diff --git a/internal/api/user.go b/internal/api/user.go index 0fc2a1b5e..a32b50dc9 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -170,12 +170,16 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { if config.Security.UpdatePasswordRequireCurrentPassword { // user may be in a password reset flow, where they do not have a currentPassword isRecoverySession := false - for _, claim := range session.AMRClaims { - // password recovery flows can be via otp or a magic link, check if the current session - // was created with one of those - if claim.GetAuthenticationMethod() == "otp" || claim.GetAuthenticationMethod() == "magiclink" { - isRecoverySession = true - break + + // it is only a recovery session if it was recently created + if time.Now().Before(session.CreatedAt.Add(15*time.Minute)) { + for _, claim := range session.AMRClaims { + // password recovery flows can be via otp or a magic link, check if the current session + // was created with one of those + if claim.GetAuthenticationMethod() == "otp" || claim.GetAuthenticationMethod() == "magiclink" { + isRecoverySession = true + break + } } } if !isRecoverySession { @@ -187,7 +191,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { return err } if !isCurrentPasswordCorrect { - return apierrors.NewBadRequestError(apierrors.ErrorCodeCurrentPasswordMismatch, "Current password required when setting new password.") + return apierrors.NewBadRequestError(apierrors.ErrorCodeCurrentPasswordMismatch, "A valid current password required when setting new password.") } } } diff --git a/internal/api/user_test.go b/internal/api/user_test.go index 59f8585e2..03edda25c 100644 --- a/internal/api/user_test.go +++ b/internal/api/user_test.go @@ -420,6 +420,7 @@ func (ts *UserTestSuite) TestUserUpdatePasswordViaRecovery() { newPassword string currentPassword string recoveryType models.AuthenticationMethod + staleSession bool expected expected }{ { @@ -440,6 +441,20 @@ func (ts *UserTestSuite) TestUserUpdatePasswordViaRecovery() { recoveryType: models.EmailChange, expected: expected{code: http.StatusBadRequest, isAuthenticated: true}, }, + { + desc: "Current password not required for recent OTP recovery session (within 15 minutes)", + newPassword: "newpassword789", + recoveryType: models.OTP, + staleSession: false, + expected: expected{code: http.StatusOK, isAuthenticated: true}, + }, + { + desc: "Current password required for stale OTP recovery session (older than 15 minutes)", + newPassword: "newpassword789", + recoveryType: models.OTP, + staleSession: true, + expected: expected{code: http.StatusBadRequest, isAuthenticated: true}, + }, } for _, c := range cases { @@ -454,6 +469,14 @@ func (ts *UserTestSuite) TestUserUpdatePasswordViaRecovery() { // Add AMR claim to session to simulate recovery flow require.NoError(ts.T(), models.AddClaimToSession(ts.API.db, session.ID, c.recoveryType)) + if c.staleSession { + require.NoError(ts.T(), ts.API.db.RawQuery( + "update "+session.TableName()+" set created_at = ? where id = ?", + time.Now().Add(-20*time.Minute), + session.ID).Exec(), + ) + } + // Reload session with AMR claims session, err = models.FindSessionByID(ts.API.db, session.ID, true) require.NoError(ts.T(), err)