Skip to content

Commit f6c8d98

Browse files
J0cemalkilic
authored andcommitted
fix: update mfa admin methods (#1774)
## What kind of change does this PR introduce? Update admin MFA methods to allow an admin to update a phone factor's phone number. Also disallows and removes factor type as an updatable field. Having the factor type field is redundant as it previously allowed for update of only one factor type (TOTP).
1 parent 0aeb45a commit f6c8d98

File tree

3 files changed

+15
-22
lines changed

3 files changed

+15
-22
lines changed

internal/api/admin.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ type adminUserDeleteParams struct {
3939

4040
type adminUserUpdateFactorParams struct {
4141
FriendlyName string `json:"friendly_name"`
42-
FactorType string `json:"factor_type"`
42+
Phone string `json:"phone"`
4343
}
4444

4545
type AdminListUsersResponse struct {
@@ -618,11 +618,13 @@ func (a *API) adminUserUpdateFactor(w http.ResponseWriter, r *http.Request) erro
618618
return terr
619619
}
620620
}
621-
if params.FactorType != "" {
622-
if params.FactorType != models.TOTP {
623-
return badRequestError(ErrorCodeValidationFailed, "Factor Type not valid")
621+
622+
if params.Phone != "" && factor.IsPhoneFactor() {
623+
phone, err := validatePhone(params.Phone)
624+
if err != nil {
625+
return badRequestError(ErrorCodeValidationFailed, "Invalid phone number format (E.164 required)")
624626
}
625-
if terr := factor.UpdateFactorType(tx, params.FactorType); terr != nil {
627+
if terr := factor.UpdatePhone(tx, phone); terr != nil {
626628
return terr
627629
}
628630
}

internal/api/admin_test.go

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,7 @@ func (ts *AdminTestSuite) TestAdminUserUpdateFactor() {
816816
require.NoError(ts.T(), err, "Error making new user")
817817
require.NoError(ts.T(), ts.API.db.Create(u), "Error creating user")
818818

819-
f := models.NewTOTPFactor(u, "testSimpleName")
819+
f := models.NewPhoneFactor(u, "123456789", "testSimpleName")
820820
require.NoError(ts.T(), f.SetSecret("secretkey", ts.Config.Security.DBEncryption.Encrypt, ts.Config.Security.DBEncryption.EncryptionKeyID, ts.Config.Security.DBEncryption.EncryptionKey))
821821
require.NoError(ts.T(), ts.API.db.Create(f), "Error saving new test factor")
822822

@@ -833,20 +833,12 @@ func (ts *AdminTestSuite) TestAdminUserUpdateFactor() {
833833
ExpectedCode: http.StatusOK,
834834
},
835835
{
836-
Desc: "Update factor: valid factor type",
836+
Desc: "Update Factor phone number",
837837
FactorData: map[string]interface{}{
838-
"friendly_name": "john",
839-
"factor_type": models.TOTP,
838+
"phone": "+1976154321",
840839
},
841840
ExpectedCode: http.StatusOK,
842841
},
843-
{
844-
Desc: "Update factor: invalid factor",
845-
FactorData: map[string]interface{}{
846-
"factor_type": "invalid_factor",
847-
},
848-
ExpectedCode: http.StatusBadRequest,
849-
},
850842
}
851843

852844
// Initialize factor data

internal/models/factor.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -245,18 +245,17 @@ func (f *Factor) UpdateFriendlyName(tx *storage.Connection, friendlyName string)
245245
return tx.UpdateOnly(f, "friendly_name", "updated_at")
246246
}
247247

248+
func (f *Factor) UpdatePhone(tx *storage.Connection, phone string) error {
249+
f.Phone = storage.NullString(phone)
250+
return tx.UpdateOnly(f, "phone", "updated_at")
251+
}
252+
248253
// UpdateStatus modifies the factor status
249254
func (f *Factor) UpdateStatus(tx *storage.Connection, state FactorState) error {
250255
f.Status = state.String()
251256
return tx.UpdateOnly(f, "status", "updated_at")
252257
}
253258

254-
// UpdateFactorType modifies the factor type
255-
func (f *Factor) UpdateFactorType(tx *storage.Connection, factorType string) error {
256-
f.FactorType = factorType
257-
return tx.UpdateOnly(f, "factor_type", "updated_at")
258-
}
259-
260259
func (f *Factor) DowngradeSessionsToAAL1(tx *storage.Connection) error {
261260
sessions, err := FindSessionsByFactorID(tx, f.ID)
262261
if err != nil {

0 commit comments

Comments
 (0)