Skip to content

Commit 8f5a13c

Browse files
committed
chore: no current password for recovery
1 parent 78a56ba commit 8f5a13c

2 files changed

Lines changed: 108 additions & 8 deletions

File tree

internal/api/user.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -168,15 +168,27 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
168168
if user.HasPassword() {
169169
// current password required when updating password
170170
if config.Security.UpdatePasswordRequireCurrentPassword {
171-
if params.CurrentPassword == nil || *params.CurrentPassword == "" {
172-
return apierrors.NewBadRequestError(apierrors.ErrorCodeCurrentPasswordRequired, "Current password required when setting new password.")
171+
// user may be in a password reset flow, where they do not have a currentPassword
172+
isRecoverySession := false
173+
for _, claim := range session.AMRClaims {
174+
// password recovery flows can be via otp or a magic link, check if the current session
175+
// was created with one of those
176+
if claim.GetAuthenticationMethod() == "otp" || claim.GetAuthenticationMethod() == "magiclink" {
177+
isRecoverySession = true
178+
break
179+
}
173180
}
174-
isCurrentPasswordCorrect, _, err := user.Authenticate(ctx, db, *params.CurrentPassword, config.Security.DBEncryption.DecryptionKeys, false, "")
175-
if err != nil {
176-
return err
177-
}
178-
if !isCurrentPasswordCorrect {
179-
return apierrors.NewBadRequestError(apierrors.ErrorCodeCurrentPasswordMismatch, "Current password required when setting new password.")
181+
if !isRecoverySession {
182+
if params.CurrentPassword == nil || *params.CurrentPassword == "" {
183+
return apierrors.NewBadRequestError(apierrors.ErrorCodeCurrentPasswordRequired, "Current password required when setting new password.")
184+
}
185+
isCurrentPasswordCorrect, _, err := user.Authenticate(ctx, db, *params.CurrentPassword, config.Security.DBEncryption.DecryptionKeys, false, "")
186+
if err != nil {
187+
return err
188+
}
189+
if !isCurrentPasswordCorrect {
190+
return apierrors.NewBadRequestError(apierrors.ErrorCodeCurrentPasswordMismatch, "Current password required when setting new password.")
191+
}
180192
}
181193
}
182194
auth, _, err := user.Authenticate(ctx, db, password, config.Security.DBEncryption.DecryptionKeys, false, "")

internal/api/user_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,94 @@ func (ts *UserTestSuite) TestUserUpdatePassword() {
402402
}
403403
}
404404

405+
func (ts *UserTestSuite) TestUserUpdatePasswordViaRecovery() {
406+
ts.Config.Security.UpdatePasswordRequireCurrentPassword = true
407+
ts.Config.SMTP.MaxFrequency = 60
408+
u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud)
409+
require.NoError(ts.T(), err)
410+
u.RecoverySentAt = &time.Time{}
411+
require.NoError(ts.T(), ts.API.db.Update(u))
412+
413+
type expected struct {
414+
code int
415+
isAuthenticated bool
416+
}
417+
418+
var cases = []struct {
419+
desc string
420+
newPassword string
421+
currentPassword string
422+
recoveryType models.AuthenticationMethod
423+
expected expected
424+
}{
425+
{
426+
desc: "Current password not required in OTP recovery flow",
427+
newPassword: "newpassword123",
428+
recoveryType: models.OTP,
429+
expected: expected{code: http.StatusOK, isAuthenticated: true},
430+
},
431+
{
432+
desc: "Current password not required in magiclink recovery flow",
433+
newPassword: "newpassword456",
434+
recoveryType: models.MagicLink,
435+
expected: expected{code: http.StatusOK, isAuthenticated: true},
436+
},
437+
{
438+
desc: "Current password required for any other claim",
439+
newPassword: "newpassword456",
440+
recoveryType: models.EmailChange,
441+
expected: expected{code: http.StatusBadRequest, isAuthenticated: true},
442+
},
443+
}
444+
445+
for _, c := range cases {
446+
ts.Run(c.desc, func() {
447+
require.NoError(ts.T(), models.ClearAllOneTimeTokensForUser(ts.API.db, u.ID))
448+
449+
// Create a session
450+
session, err := models.NewSession(u.ID, nil)
451+
require.NoError(ts.T(), err)
452+
require.NoError(ts.T(), ts.API.db.Create(session))
453+
454+
// Add AMR claim to session to simulate recovery flow
455+
require.NoError(ts.T(), models.AddClaimToSession(ts.API.db, session.ID, c.recoveryType))
456+
457+
// Reload session with AMR claims
458+
session, err = models.FindSessionByID(ts.API.db, session.ID, true)
459+
require.NoError(ts.T(), err)
460+
require.NotEmpty(ts.T(), session.AMRClaims, "Session should have AMR claims")
461+
462+
// Generate access token with the recovery authentication method
463+
req := httptest.NewRequest(http.MethodPut, "http://localhost/user", nil)
464+
token, _, err := ts.API.generateAccessToken(req, ts.API.db, u, &session.ID, c.recoveryType)
465+
require.NoError(ts.T(), err)
466+
467+
// Update password without current password
468+
userUpdateBody := map[string]string{"password": c.newPassword}
469+
var buffer bytes.Buffer
470+
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(userUpdateBody))
471+
472+
req = httptest.NewRequest(http.MethodPut, "http://localhost/user", &buffer)
473+
req.Header.Set("Content-Type", "application/json")
474+
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
475+
476+
// Setup response recorder
477+
w := httptest.NewRecorder()
478+
ts.API.handler.ServeHTTP(w, req)
479+
require.Equal(ts.T(), c.expected.code, w.Code)
480+
481+
// Verify password was updated
482+
u, err = models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud)
483+
require.NoError(ts.T(), err)
484+
485+
isAuthenticated, _, err := u.Authenticate(context.Background(), ts.API.db, c.newPassword, ts.API.config.Security.DBEncryption.DecryptionKeys, ts.API.config.Security.DBEncryption.Encrypt, ts.API.config.Security.DBEncryption.EncryptionKeyID)
486+
require.NoError(ts.T(), err)
487+
488+
require.Equal(ts.T(), c.expected.isAuthenticated, isAuthenticated)
489+
})
490+
}
491+
}
492+
405493
func (ts *UserTestSuite) TestUserUpdatePasswordNoReauthenticationRequired() {
406494
ts.Config.Security.UpdatePasswordRequireCurrentPassword = false
407495
u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud)

0 commit comments

Comments
 (0)