From afd4467cf294e79b169942a22596f67b2fd9cd0d Mon Sep 17 00:00:00 2001 From: joel Date: Thu, 12 Sep 2024 14:46:25 +0300 Subject: [PATCH 1/4] fix: update phone admin methods --- internal/api/admin.go | 8 +++----- internal/models/factor.go | 12 ++++++------ 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/internal/api/admin.go b/internal/api/admin.go index 45b08f041..39c5dea3f 100644 --- a/internal/api/admin.go +++ b/internal/api/admin.go @@ -618,11 +618,9 @@ func (a *API) adminUserUpdateFactor(w http.ResponseWriter, r *http.Request) erro return terr } } - if params.FactorType != "" { - if params.FactorType != models.TOTP { - return badRequestError(ErrorCodeValidationFailed, "Factor Type not valid") - } - if terr := factor.UpdateFactorType(tx, params.FactorType); terr != nil { + + if params.Phone != "" && factor.IsPhoneFactor() { + if terr := factor.UpdatePhone(tx, params.Phone); terr != nil { return terr } } diff --git a/internal/models/factor.go b/internal/models/factor.go index 7c6f6dd30..ecf6b1a9f 100644 --- a/internal/models/factor.go +++ b/internal/models/factor.go @@ -32,6 +32,7 @@ func (factorState FactorState) String() string { const TOTP = "totp" const Phone = "phone" +const WebAuthn = "webauthn" type AuthenticationMethod int @@ -245,18 +246,17 @@ func (f *Factor) UpdateFriendlyName(tx *storage.Connection, friendlyName string) return tx.UpdateOnly(f, "friendly_name", "updated_at") } +func (f *Factor) UpdatePhone(tx *storage.Connection, phone string) error { + f.Phone = phone + return tx.UpdateOnly(f, "phone", "updated_at") +} + // UpdateStatus modifies the factor status func (f *Factor) UpdateStatus(tx *storage.Connection, state FactorState) error { f.Status = state.String() return tx.UpdateOnly(f, "status", "updated_at") } -// UpdateFactorType modifies the factor type -func (f *Factor) UpdateFactorType(tx *storage.Connection, factorType string) error { - f.FactorType = factorType - return tx.UpdateOnly(f, "factor_type", "updated_at") -} - func (f *Factor) DowngradeSessionsToAAL1(tx *storage.Connection) error { sessions, err := FindSessionsByFactorID(tx, f.ID) if err != nil { From c17500566de03823b1c1d2644c9fc24a59508e95 Mon Sep 17 00:00:00 2001 From: joel Date: Thu, 12 Sep 2024 14:50:53 +0300 Subject: [PATCH 2/4] fix: enforce validation on phone number passed in --- internal/models/factor.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/models/factor.go b/internal/models/factor.go index ecf6b1a9f..fc380ada8 100644 --- a/internal/models/factor.go +++ b/internal/models/factor.go @@ -32,7 +32,6 @@ func (factorState FactorState) String() string { const TOTP = "totp" const Phone = "phone" -const WebAuthn = "webauthn" type AuthenticationMethod int From f9270dfb04c6c53603cbbb955b7f3e6cfd289024 Mon Sep 17 00:00:00 2001 From: joel Date: Thu, 12 Sep 2024 14:53:57 +0300 Subject: [PATCH 3/4] fix: change admin params --- internal/api/admin.go | 8 ++++++-- internal/models/factor.go | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/api/admin.go b/internal/api/admin.go index 39c5dea3f..d6ff069ff 100644 --- a/internal/api/admin.go +++ b/internal/api/admin.go @@ -39,7 +39,7 @@ type adminUserDeleteParams struct { type adminUserUpdateFactorParams struct { FriendlyName string `json:"friendly_name"` - FactorType string `json:"factor_type"` + Phone string `json:"phone"` } type AdminListUsersResponse struct { @@ -620,7 +620,11 @@ func (a *API) adminUserUpdateFactor(w http.ResponseWriter, r *http.Request) erro } if params.Phone != "" && factor.IsPhoneFactor() { - if terr := factor.UpdatePhone(tx, params.Phone); terr != nil { + phone, err := validatePhone(params.Phone) + if err != nil { + return badRequestError(ErrorCodeValidationFailed, "Invalid phone number format (E.164 required)") + } + if terr := factor.UpdatePhone(tx, phone); terr != nil { return terr } } diff --git a/internal/models/factor.go b/internal/models/factor.go index fc380ada8..24cda188f 100644 --- a/internal/models/factor.go +++ b/internal/models/factor.go @@ -246,7 +246,7 @@ func (f *Factor) UpdateFriendlyName(tx *storage.Connection, friendlyName string) } func (f *Factor) UpdatePhone(tx *storage.Connection, phone string) error { - f.Phone = phone + f.Phone = storage.NullString(phone) return tx.UpdateOnly(f, "phone", "updated_at") } From b9e1ee375396b5b7b11172a5361dea55d14ad0e5 Mon Sep 17 00:00:00 2001 From: joel Date: Thu, 12 Sep 2024 15:26:12 +0300 Subject: [PATCH 4/4] fix: updat test --- internal/api/admin_test.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/internal/api/admin_test.go b/internal/api/admin_test.go index e2904d8bf..a2070d7e2 100644 --- a/internal/api/admin_test.go +++ b/internal/api/admin_test.go @@ -816,7 +816,7 @@ func (ts *AdminTestSuite) TestAdminUserUpdateFactor() { require.NoError(ts.T(), err, "Error making new user") require.NoError(ts.T(), ts.API.db.Create(u), "Error creating user") - f := models.NewTOTPFactor(u, "testSimpleName") + f := models.NewPhoneFactor(u, "123456789", "testSimpleName") require.NoError(ts.T(), f.SetSecret("secretkey", ts.Config.Security.DBEncryption.Encrypt, ts.Config.Security.DBEncryption.EncryptionKeyID, ts.Config.Security.DBEncryption.EncryptionKey)) require.NoError(ts.T(), ts.API.db.Create(f), "Error saving new test factor") @@ -833,20 +833,12 @@ func (ts *AdminTestSuite) TestAdminUserUpdateFactor() { ExpectedCode: http.StatusOK, }, { - Desc: "Update factor: valid factor type", + Desc: "Update Factor phone number", FactorData: map[string]interface{}{ - "friendly_name": "john", - "factor_type": models.TOTP, + "phone": "+1976154321", }, ExpectedCode: http.StatusOK, }, - { - Desc: "Update factor: invalid factor", - FactorData: map[string]interface{}{ - "factor_type": "invalid_factor", - }, - ExpectedCode: http.StatusBadRequest, - }, } // Initialize factor data