Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions internal/api/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.")
}
}
}
Expand Down
23 changes: 23 additions & 0 deletions internal/api/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ func (ts *UserTestSuite) TestUserUpdatePasswordViaRecovery() {
newPassword string
currentPassword string
recoveryType models.AuthenticationMethod
staleSession bool
expected expected
}{
{
Expand All @@ -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 {
Expand All @@ -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)
Expand Down
Loading