From a484d0e9a2f29db8cee338370ba961f6c2c14f75 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 22 Oct 2026 22:56:38 +0200 Subject: [PATCH] fix: reset-password and the admin password set stamp tokens_valid_after in whole seconds, so a token minted in the same second is accepted JWT iat is whole seconds and every cutoff check is iat before tokens_valid_after, so a sub-second cutoff refused the login that followed a reset within the same second. POST /reset-password and the admin Users password set now truncate the cutoff to the second, like RevokeUserAccess; email_verified_at keeps full precision. Change-password and bouncer's comparison are unchanged. --- controllers/api_controller.go | 7 +- controllers/users_admin_controller.go | 6 +- same_second_cutoff_test.go | 114 ++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 same_second_cutoff_test.go diff --git a/controllers/api_controller.go b/controllers/api_controller.go index 4423acd..e64fc71 100644 --- a/controllers/api_controller.go +++ b/controllers/api_controller.go @@ -417,11 +417,14 @@ func ResetPassword(app *backpack.App) http.HandlerFunc { } // The reset code was mailed to the account's address, so the write // that stores the new password also records the proof of that - // address (D-27). A reset that fails here records nothing. + // address (D-27). A reset that fails here records nothing. The + // session cutoff is truncated to whole seconds, as in + // RevokeUserAccess, because JWT iat has whole-second precision, so + // the login that follows the reset is not refused. now := time.Now() if err := gdb.WithContext(r.Context()).Model(&models.User{}).Where("id = ?", user.ID).Updates(map[string]any{ "password": hashed, - "tokens_valid_after": now, + "tokens_valid_after": now.Truncate(time.Second), "email_verified_at": now, }).Error; err != nil { writeOpaque500(w) diff --git a/controllers/users_admin_controller.go b/controllers/users_admin_controller.go index c923f9a..f2eaa1c 100644 --- a/controllers/users_admin_controller.go +++ b/controllers/users_admin_controller.go @@ -241,8 +241,10 @@ func (c usersAdminController) FormBeforeUpdate(ctx context.Context, model any) e } if changed { // An admin password reset ends every session of the user, as the - // user's own change-password does. - now := time.Now() + // user's own change-password does. The cutoff is truncated to whole + // seconds, as in RevokeUserAccess, because JWT iat has whole-second + // precision, so the user's next login in that second is not refused. + now := time.Now().Truncate(time.Second) user.TokensValidAfter = &now } return nil diff --git a/same_second_cutoff_test.go b/same_second_cutoff_test.go new file mode 100644 index 0000000..d4c2147 --- /dev/null +++ b/same_second_cutoff_test.go @@ -0,0 +1,114 @@ +package user + +import ( + "net/http" + "strconv" + "testing" + "time" + + "git.golem15.com/golem15/sm-user-plugin/classes" + "git.golem15.com/golem15/sm-user-plugin/models" +) + +// TestPasswordCutoffSameSecond pins the whole-second tokens_valid_after +// cutoff of POST /reset-password and of the admin Users password set: JWT +// iat has one-second resolution, so a token minted in the second of the +// reset is accepted on every check site (the jwt guard, /fetch, /refresh) +// and a token from an earlier second is refused. +func TestPasswordCutoffSameSecond(t *testing.T) { + e := newAdminEnv(t) + + t.Run("reset-password", func(t *testing.T) { + user := e.seedUser(t, "cutoff-reset", nil) + code, err := classes.IssueResetCode(t.Context(), e.gdb, user) + if err != nil { + t.Fatal(err) + } + const next = "reset-password-same-second" + waitForFreshSecond(t) + rec := e.call(t, http.MethodPost, "/_user/api/v1/reset-password", "", map[string]string{ + "code": strconv.FormatUint(uint64(user.ID), 10) + "!" + code, + "password": next, + "password_confirmation": next, + }) + if rec.Code != http.StatusOK || rec.Body.String() != `{"message":"Password has been reset"}` { + t.Fatalf("reset = %d %s", rec.Code, rec.Body.String()) + } + got := e.reload(t, user.ID) + if got.EmailVerifiedAt == nil { + t.Fatal("reset did not record email_verified_at") + } + assertCutoffSameSecond(t, e, got, next) + }) + + t.Run("admin password set", func(t *testing.T) { + user := e.seedUser(t, "cutoff-admin", nil) + token := e.admin(t, permAccessUsers) + const next = "admin-password-same-second" + waitForFreshSecond(t) + e.expect(t, http.StatusOK, http.MethodPut, userPath(user.ID, ""), token, map[string]any{ + "name": "Cutoff admin", "email": user.Email, "password": next, "password_confirmation": next, + }) + assertCutoffSameSecond(t, e, e.reload(t, user.ID), next) + }) +} + +// assertCutoffSameSecond: the stored cutoff is a whole second; a token whose +// iat is that second works at jwt.auth, /fetch and /refresh, one issued a +// second earlier is refused at all three, and a login with password works. +func assertCutoffSameSecond(t *testing.T, e *adminEnv, user *models.User, password string) { + t.Helper() + if user.TokensValidAfter == nil { + t.Fatal("tokens_valid_after is not set") + } + cutoff := *user.TokensValidAfter + if !cutoff.Equal(cutoff.Truncate(time.Second)) { + t.Fatalf("tokens_valid_after %s is not a whole second", cutoff.Format(time.RFC3339Nano)) + } + sub := userSub(user) + + same := userAudienceToken(t, sub, "same-"+adminStamp(), cutoff, cutoff.Add(time.Hour)) + if code := e.guarded(t, same, ""); code != http.StatusOK { + t.Fatalf("jwt.auth with iat = cutoff: %d", code) + } + if code := e.fetchCode(t, same); code != http.StatusOK { + t.Fatalf("fetch with iat = cutoff: %d", code) + } + // Refresh last: it rotates and revokes the token. + if rec := e.refresh(t, same); rec.Code != http.StatusOK { + t.Fatalf("refresh with iat = cutoff: %d %s", rec.Code, rec.Body.String()) + } + + earlier := cutoff.Add(-time.Second) + old := userAudienceToken(t, sub, "earlier-"+adminStamp(), earlier, cutoff.Add(time.Hour)) + if code := e.guarded(t, old, ""); code != http.StatusUnauthorized { + t.Fatalf("jwt.auth with iat a second before the cutoff: %d", code) + } + if code := e.fetchCode(t, old); code != http.StatusUnauthorized { + t.Fatalf("fetch with iat a second before the cutoff: %d", code) + } + assertRefreshRefused(t, e.refresh(t, old)) + + status, token := e.userLogin(t, user.Email, password) + if status != http.StatusOK || token == "" { + t.Fatalf("login after the password change = %d", status) + } + if code := e.guarded(t, token, ""); code != http.StatusOK { + t.Fatalf("jwt.auth with the fresh login token: %d", code) + } + if code := e.fetchCode(t, token); code != http.StatusOK { + t.Fatalf("fetch with the fresh login token: %d", code) + } + if iat, err := parseToken(t, token).GetIssuedAt(); err == nil && iat != nil { + t.Logf("login iat %d, cutoff %d, same second: %v", iat.Unix(), cutoff.Unix(), iat.Unix() == cutoff.Unix()) + } +} + +// waitForFreshSecond sleeps until 10ms after the next whole second, so the +// password change, the reload and the login usually fall within one second. +// The deterministic proof is the crafted-token pair, not this alignment. +func waitForFreshSecond(t *testing.T) { + t.Helper() + now := time.Now() + time.Sleep(now.Truncate(time.Second).Add(time.Second + 10*time.Millisecond).Sub(now)) +}