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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
114
same_second_cutoff_test.go
Normal file
114
same_second_cutoff_test.go
Normal file
@@ -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))
|
||||
}
|
||||
Reference in New Issue
Block a user