From 93722eb1d372a0ea27d7067a13e0a3cd33121169 Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Thu, 26 Feb 2026 09:24:07 +0100 Subject: [PATCH 1/3] fix: add limit to recovery session A session would be deemed a recovery session indefinitely long. This timeboxes the recovery to 15 minutes. --- internal/api/user.go | 16 ++++++++++------ internal/api/user_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/internal/api/user.go b/internal/api/user.go index 0fc2a1b5e..70139c70b 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 { diff --git a/internal/api/user_test.go b/internal/api/user_test.go index 59f8585e2..27edc3345 100644 --- a/internal/api/user_test.go +++ b/internal/api/user_test.go @@ -289,6 +289,27 @@ func (ts *UserTestSuite) TestUserUpdatePassword() { notRecentlyLoggedIn.ID).Exec(), ) + // create a recovery session (OTP) created recently (within 15 minutes) + recentRecoverySession, err := models.NewSession(u.ID, nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.API.db.Create(recentRecoverySession)) + require.NoError(ts.T(), models.AddClaimToSession(ts.API.db, recentRecoverySession.ID, models.OTP)) + recentRecoverySession, err = models.FindSessionByID(ts.API.db, recentRecoverySession.ID, true) + require.NoError(ts.T(), err) + + // create a recovery session (OTP) whose created_at is older than 15 minutes + staleRecoverySession, err := models.NewSession(u.ID, nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.API.db.Create(staleRecoverySession)) + require.NoError(ts.T(), models.AddClaimToSession(ts.API.db, staleRecoverySession.ID, models.OTP)) + require.NoError(ts.T(), ts.API.db.RawQuery( + "update "+staleRecoverySession.TableName()+" set created_at = ? where id = ?", + time.Now().Add(-20*time.Minute), + staleRecoverySession.ID).Exec(), + ) + staleRecoverySession, err = models.FindSessionByID(ts.API.db, staleRecoverySession.ID, true) + require.NoError(ts.T(), err) + type expected struct { code int isAuthenticated bool @@ -365,6 +386,24 @@ func (ts *UserTestSuite) TestUserUpdatePassword() { sessionId: r.SessionId, expected: expected{code: http.StatusBadRequest, isAuthenticated: false}, }, + { + desc: "Current password not required for recent recovery session (OTP, within 15 minutes)", + newPassword: "newpassword123", + nonce: "", + requireReauthentication: false, + requireCurrentPassword: true, + sessionId: &recentRecoverySession.ID, + expected: expected{code: http.StatusOK, isAuthenticated: true}, + }, + { + desc: "Current password required for stale recovery session (OTP, older than 15 minutes)", + newPassword: "newpassword456", + nonce: "", + requireReauthentication: false, + requireCurrentPassword: true, + sessionId: &staleRecoverySession.ID, + expected: expected{code: http.StatusBadRequest, isAuthenticated: false}, + }, } for _, c := range cases { From 2aa70020a8032e150614399c2bd33bad18d7c2e8 Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Thu, 26 Feb 2026 09:30:54 +0100 Subject: [PATCH 2/3] fix: more user friendly message --- internal/api/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/user.go b/internal/api/user.go index 70139c70b..a32b50dc9 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -191,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.") } } } From 61a08e884f48375f75dd2f4d51459119d2307e66 Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Thu, 26 Feb 2026 09:48:23 +0100 Subject: [PATCH 3/3] chore: move test cases --- internal/api/user_test.go | 62 +++++++++++++++------------------------ 1 file changed, 23 insertions(+), 39 deletions(-) diff --git a/internal/api/user_test.go b/internal/api/user_test.go index 27edc3345..03edda25c 100644 --- a/internal/api/user_test.go +++ b/internal/api/user_test.go @@ -289,27 +289,6 @@ func (ts *UserTestSuite) TestUserUpdatePassword() { notRecentlyLoggedIn.ID).Exec(), ) - // create a recovery session (OTP) created recently (within 15 minutes) - recentRecoverySession, err := models.NewSession(u.ID, nil) - require.NoError(ts.T(), err) - require.NoError(ts.T(), ts.API.db.Create(recentRecoverySession)) - require.NoError(ts.T(), models.AddClaimToSession(ts.API.db, recentRecoverySession.ID, models.OTP)) - recentRecoverySession, err = models.FindSessionByID(ts.API.db, recentRecoverySession.ID, true) - require.NoError(ts.T(), err) - - // create a recovery session (OTP) whose created_at is older than 15 minutes - staleRecoverySession, err := models.NewSession(u.ID, nil) - require.NoError(ts.T(), err) - require.NoError(ts.T(), ts.API.db.Create(staleRecoverySession)) - require.NoError(ts.T(), models.AddClaimToSession(ts.API.db, staleRecoverySession.ID, models.OTP)) - require.NoError(ts.T(), ts.API.db.RawQuery( - "update "+staleRecoverySession.TableName()+" set created_at = ? where id = ?", - time.Now().Add(-20*time.Minute), - staleRecoverySession.ID).Exec(), - ) - staleRecoverySession, err = models.FindSessionByID(ts.API.db, staleRecoverySession.ID, true) - require.NoError(ts.T(), err) - type expected struct { code int isAuthenticated bool @@ -386,24 +365,6 @@ func (ts *UserTestSuite) TestUserUpdatePassword() { sessionId: r.SessionId, expected: expected{code: http.StatusBadRequest, isAuthenticated: false}, }, - { - desc: "Current password not required for recent recovery session (OTP, within 15 minutes)", - newPassword: "newpassword123", - nonce: "", - requireReauthentication: false, - requireCurrentPassword: true, - sessionId: &recentRecoverySession.ID, - expected: expected{code: http.StatusOK, isAuthenticated: true}, - }, - { - desc: "Current password required for stale recovery session (OTP, older than 15 minutes)", - newPassword: "newpassword456", - nonce: "", - requireReauthentication: false, - requireCurrentPassword: true, - sessionId: &staleRecoverySession.ID, - expected: expected{code: http.StatusBadRequest, isAuthenticated: false}, - }, } for _, c := range cases { @@ -459,6 +420,7 @@ func (ts *UserTestSuite) TestUserUpdatePasswordViaRecovery() { newPassword string currentPassword string recoveryType models.AuthenticationMethod + staleSession bool expected expected }{ { @@ -479,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 { @@ -493,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)