fix: resetting the password or cancelling a registration ends existing sessions: a token issued before the change is refused
This commit is contained in:
fsylvain committed
1 parent
81c4b62175
commit
f3ec3d2fa2
15 files changed
+844
-13
No files matched your search
@@ -0,0 +1,172 @@
|
||||
Feature: A password reset invalidates every jwt issued before it, and a cancelled registration keeps doing so
|
||||
|
||||
As the maintainer of RenewCMS
|
||||
I want a jwt minted before a user resets their password to stop working the moment the reset
|
||||
succeeds, while a jwt obtained by logging in again afterward keeps working, and I want the
|
||||
session a cancelled registration already destroys today to keep being destroyed once this
|
||||
slice's new check is layered on top of it
|
||||
So that resetting a password — usually done because a session or credential may be compromised —
|
||||
actually ends every session an attacker might be holding, which is the whole reason people
|
||||
reset a password in the first place
|
||||
|
||||
Background:
|
||||
Given user.User gains a new field, TokenVersion uint32, defaulting to 0 for every existing and
|
||||
newly created user
|
||||
And user.Repository.ResetPassword(userId, hashedPassword) now increments TokenVersion by 1 in
|
||||
the same single write that already sets Password and clears PasswordResetCode and
|
||||
PasswordResetExpiration (features/reset_password.feature), so every successful password
|
||||
reset advances the counter by exactly one
|
||||
And auth.Handler.SetJwtCookie now takes the full user.User instead of a bare userId, and encodes
|
||||
the jwt's claims as {"user_id": user.ID, "token_version": user.TokenVersion}; Login and
|
||||
Register, which already hold the freshly loaded or created user.User at their call site, pass
|
||||
it through unchanged otherwise
|
||||
And a new helper, claims.TokenVersion(r), reads the "token_version" claim from the verified jwt
|
||||
the same way claims.UserId already reads "user_id": missing key or a non-numeric value both
|
||||
return ok=false, never a panic
|
||||
And this slice adds a new middleware, middleware.RequireCurrentToken(getUser
|
||||
*useCases.GetUserUseCase), mounted immediately after jwtauth.Authenticator in both protected
|
||||
groups api/router.go already defines: NewArticleRouter's group (ahead of the existing
|
||||
RequireVerifiedUser) and NewAuthRouter's group (ahead of Verify and CancelRegistration)
|
||||
And RequireCurrentToken resolves user_id with claims.UserId (401 apperr.UnauthorizedErrorMessage
|
||||
if absent or malformed, the same response RequireVerifiedUser already gives), loads the user
|
||||
with getUser.GetUser(userId) (401 apperr.UnauthorizedErrorMessage on user.ErrUserNotFound, 500
|
||||
"Internal server error" on any other error), resolves token_version with claims.TokenVersion
|
||||
(401 apperr.UnauthorizedErrorMessage if absent or malformed), and 401s with
|
||||
apperr.UnauthorizedErrorMessage when that value does not equal the loaded user's TokenVersion;
|
||||
only when all four checks pass does it call next
|
||||
And cancelling a pending registration (features/cancel_pending_registration.feature) already
|
||||
deletes the user row outright, and every protected handler already re-resolves the user by id
|
||||
on each request, so RequireCurrentToken's user.ErrUserNotFound branch already covers a
|
||||
pre-cancellation token without any change to CancelRegistrationUseCase or user.Repository.Delete;
|
||||
this slice only adds regression scenarios for that existing behaviour
|
||||
And nothing in this slice changes RequireVerifiedUser's own IsVerified check, or the status
|
||||
codes and bodies any existing scenario already asserts for a missing or malformed user_id
|
||||
claim; RequireCurrentToken only adds a new way to fail before those checks run
|
||||
|
||||
Scenario: A jwt minted before a password reset is refused afterward, but a fresh login's jwt is accepted
|
||||
Given a user repository containing user id 7, username "alice", email "alice@example.com",
|
||||
is_verified true, password hash of "oldpassword1", TokenVersion 0, whose stored
|
||||
PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is
|
||||
one hour from now
|
||||
And a valid jwt cookie for user id 7 encoding token_version 0
|
||||
When the client sends POST /v1/article with that cookie and body
|
||||
{"title": "A valid title", "body": "Some article body"}
|
||||
Then the response status is 200
|
||||
When the client sends POST /v1/auth/password-reset/confirm with body
|
||||
{"email": "alice@example.com", "code": "reset-code-1", "password": "newpassword1"}
|
||||
Then the response status is 200
|
||||
And the user repository's record for id 7 now has TokenVersion 1
|
||||
When the client sends POST /v1/article again with the same pre-reset cookie and body
|
||||
{"title": "Another title", "body": "Some other body"}
|
||||
Then the response status is 401
|
||||
And the response body is the apperr.UnauthorizedErrorMessage text
|
||||
And no article was created by that request
|
||||
When the client sends POST /v1/auth/login with body
|
||||
{"username": "alice", "password": "newpassword1"}
|
||||
Then the response status is 200
|
||||
And a jwt cookie is set whose token_version claim is 1
|
||||
When the client sends POST /v1/article with that new cookie and body
|
||||
{"title": "Another title", "body": "Some other body"}
|
||||
Then the response status is 200
|
||||
|
||||
Scenario: A stale jwt is also refused on the auth group's own protected routes, not just article routes
|
||||
Given a user repository containing user id 7, username "alice", email "alice@example.com",
|
||||
is_verified false, TokenVersion 0, whose stored PasswordResetCode is the bcrypt hash of
|
||||
"reset-code-1" and whose PasswordResetExpiration is one hour from now
|
||||
And a valid jwt cookie for user id 7 encoding token_version 0
|
||||
When the client sends POST /v1/auth/password-reset/confirm with body
|
||||
{"email": "alice@example.com", "code": "reset-code-1", "password": "newpassword1"}
|
||||
Then the response status is 200
|
||||
When the client sends POST /v1/auth/verify with that pre-reset cookie and body {"code": "anything"}
|
||||
Then the response status is 401
|
||||
And the response body is the apperr.UnauthorizedErrorMessage text
|
||||
When the client sends DELETE /v1/auth/register with that same pre-reset cookie
|
||||
Then the response status is 401
|
||||
And the response body is the apperr.UnauthorizedErrorMessage text
|
||||
And the user repository's record for id 7 still exists
|
||||
|
||||
Scenario: Resetting the password twice invalidates both the original jwt and the jwt from the interim login
|
||||
Given a user repository containing user id 7, username "alice", email "alice@example.com",
|
||||
is_verified true, TokenVersion 0, whose stored PasswordResetCode is the bcrypt hash of
|
||||
"reset-code-1" and whose PasswordResetExpiration is one hour from now
|
||||
And a valid jwt cookie for user id 7 encoding token_version 0, called "original cookie"
|
||||
When the client sends POST /v1/auth/password-reset/confirm with body
|
||||
{"email": "alice@example.com", "code": "reset-code-1", "password": "newpassword1"}
|
||||
Then the response status is 200
|
||||
And the user repository's record for id 7 now has TokenVersion 1
|
||||
When the client sends POST /v1/auth/login with body
|
||||
{"username": "alice", "password": "newpassword1"}
|
||||
Then a jwt cookie is set whose token_version claim is 1, called "interim cookie"
|
||||
Given the user repository's record for id 7 now also has PasswordResetCode set to the bcrypt
|
||||
hash of "reset-code-2" and PasswordResetExpiration one hour from now
|
||||
When the client sends POST /v1/auth/password-reset/confirm with body
|
||||
{"email": "alice@example.com", "code": "reset-code-2", "password": "newerpassword1"}
|
||||
Then the response status is 200
|
||||
And the user repository's record for id 7 now has TokenVersion 2
|
||||
When the client sends POST /v1/article with the "original cookie" and body
|
||||
{"title": "A valid title", "body": "Some article body"}
|
||||
Then the response status is 401
|
||||
When the client sends POST /v1/article with the "interim cookie" and body
|
||||
{"title": "A valid title", "body": "Some article body"}
|
||||
Then the response status is 401
|
||||
When the client sends POST /v1/auth/login with body
|
||||
{"username": "alice", "password": "newerpassword1"}
|
||||
Then a jwt cookie is set whose token_version claim is 2
|
||||
When the client sends POST /v1/article with that cookie and body
|
||||
{"title": "A valid title", "body": "Some article body"}
|
||||
Then the response status is 200
|
||||
|
||||
Scenario: Resetting one user's password never invalidates another user's jwt
|
||||
Given a user repository containing user id 7, username "alice", is_verified true, TokenVersion
|
||||
0, whose stored PasswordResetCode is the bcrypt hash of "reset-code-1" and whose
|
||||
PasswordResetExpiration is one hour from now
|
||||
And a user repository also containing user id 9, username "bob", is_verified true, TokenVersion 0
|
||||
And a valid jwt cookie for user id 9 encoding token_version 0
|
||||
When the client sends POST /v1/auth/password-reset/confirm with body
|
||||
{"email": "alice@example.com", "code": "reset-code-1", "password": "newpassword1"}
|
||||
Then the response status is 200
|
||||
And the user repository's record for id 9 still has TokenVersion 0
|
||||
When the client sends POST /v1/article with bob's cookie and body
|
||||
{"title": "A valid title", "body": "Some article body"}
|
||||
Then the response status is 200
|
||||
|
||||
Scenario Outline: A cancelled registration still refuses a jwt minted before the cancellation, on every protected route
|
||||
Given a user repository containing user id 7, username "alice", email "alice@example.com",
|
||||
is_verified false, TokenVersion 0
|
||||
And a valid jwt cookie for user id 7 encoding token_version 0
|
||||
When the client sends DELETE /v1/auth/register with that cookie
|
||||
Then the response status is 200
|
||||
When the client sends <method> <path> with the same pre-cancellation cookie <and body>
|
||||
Then the response status is 401
|
||||
And the response body is the apperr.UnauthorizedErrorMessage text
|
||||
|
||||
Examples:
|
||||
| method | path | and body |
|
||||
| POST | /v1/auth/verify | and body {"code": "anything"} |
|
||||
| DELETE | /v1/auth/register | and no body |
|
||||
|
||||
Scenario: A jwt with no token_version claim at all is refused, treated the same as a mismatch
|
||||
Given a user repository containing user id 7, username "alice", is_verified true, TokenVersion 0
|
||||
And a validly signed jwt cookie whose claims are exactly {"user_id": 7}, with no token_version key
|
||||
When the client sends POST /v1/article with that cookie and body
|
||||
{"title": "A valid title", "body": "Some article body"}
|
||||
Then the response status is 401
|
||||
And the response body is the apperr.UnauthorizedErrorMessage text
|
||||
|
||||
Scenario: A jwt whose token_version claim is a string, not a number, is refused instead of crashing
|
||||
Given a user repository containing user id 7, username "alice", is_verified true, TokenVersion 0
|
||||
And a validly signed jwt cookie whose claims are {"user_id": 7, "token_version": "0"}
|
||||
When the client sends POST /v1/article with that cookie and body
|
||||
{"title": "A valid title", "body": "Some article body"}
|
||||
Then the response status is 401
|
||||
And the response body is the apperr.UnauthorizedErrorMessage text
|
||||
And the handler does not panic
|
||||
|
||||
Scenario: The use case itself advances TokenVersion by one on every successful reset, independently of any handler
|
||||
Given a user.Repository fake containing user id 3, email "alice@example.com", TokenVersion 5,
|
||||
whose PasswordResetCode is the bcrypt hash of "reset-code-1" and whose
|
||||
PasswordResetExpiration is one hour from now
|
||||
And a ResetPasswordUseCase built directly over that fake and a hasher
|
||||
When ResetPassword is called with "alice@example.com", "reset-code-1" and "newpassword1"
|
||||
Then the call returns nil
|
||||
And the fake repository's record for id 3 now has TokenVersion 6
|
||||
@@ -0,0 +1,93 @@
|
||||
# QA procedure: stale_token_invalidation
|
||||
|
||||
Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`) and reusing the
|
||||
router/handler wiring pattern from `test/account_verification_http_test.go` and
|
||||
`test/reset_password_http_test.go` (or equivalent).
|
||||
|
||||
## Test infrastructure changes needed
|
||||
|
||||
1. `mockUserRepo` needs a `TokenVersion uint32` field on its stored `user.User` values, and its
|
||||
`ResetPassword` fake must increment it by 1 alongside setting the password hash and clearing the
|
||||
reset code, mirroring the production contract this slice adds.
|
||||
2. HTTP-level checks wire `mockUserRepo`, `useCases.NewResetPasswordUseCase(repo, hasher)`,
|
||||
`useCases.NewAuthenticateUserUseCase(repo, hasher)`, `useCases.NewGetUserUseCase(repo)`, the
|
||||
article use cases, and the rest of the existing container into `auth.NewHandler(...)` and
|
||||
`article.NewHandler(...)` with a test `jwtauth.New("HS256", []byte("test-secret"), nil)`.
|
||||
3. Build two `chi.NewRouter()` instances (or one router mounting both) that mirror
|
||||
`api/router.go`:
|
||||
- the article protected group: `jwtauth.Verifier(token)` → (mounted once, top-level, per
|
||||
`InitBackendRoutes`) → inside the group: `jwtauth.Authenticator(token)` →
|
||||
`middleware.RequireCurrentToken(getUserUseCase)` → `middleware.RequireVerifiedUser(getUserUseCase)`
|
||||
→ `POST /article`;
|
||||
- the auth protected group: `jwtauth.Verifier(token)` → `jwtauth.Authenticator(token)` →
|
||||
`middleware.RequireCurrentToken(getUserUseCase)` → `POST /verify`, `DELETE /register`;
|
||||
- the auth public routes `POST /login`, `POST /password-reset/confirm`, `POST /register` mounted
|
||||
unprotected, as today.
|
||||
4. Call `validator.InitValidator()` once per test.
|
||||
5. Mint jwt cookies directly with `token.Encode(map[string]any{"user_id": uint32(7), "token_version":
|
||||
uint32(0)})` for "pre-reset" cookies; decode a `Set-Cookie` response header's jwt with
|
||||
`jwtauth.New(...).Decode(tokenString)` or by parsing the JWT payload to assert its
|
||||
`token_version` claim, mirroring how `secureAttributeFromResponse` in
|
||||
`test/runtime_settings_injection_http_test.go` already parses `Set-Cookie`.
|
||||
|
||||
## HTTP-level checks
|
||||
|
||||
6. Seed user id 7 "alice", `is_verified: true`, `TokenVersion: 0`, `PasswordResetCode` = bcrypt hash
|
||||
of "reset-code-1", `PasswordResetExpiration` one hour ahead. Mint a jwt cookie encoding
|
||||
`{"user_id": 7, "token_version": 0}`.
|
||||
- `POST /v1/article` with that cookie and a valid body → assert `200`.
|
||||
- `POST /v1/auth/password-reset/confirm` with
|
||||
`{"email":"alice@example.com","code":"reset-code-1","password":"newpassword1"}` → assert `200`
|
||||
and `repo.users[7].TokenVersion == 1`.
|
||||
- `POST /v1/article` again with the *same* pre-reset cookie → assert `401`, body equals
|
||||
`apperr.UnauthorizedErrorMessage`, and the article use case's create path was not invoked
|
||||
(article count in the fake repo unchanged).
|
||||
- `POST /v1/auth/login` with `{"username":"alice","password":"newpassword1"}` → assert `200`, a
|
||||
`jwt` `Set-Cookie` is present, and decoding it shows `token_version == 1`.
|
||||
- `POST /v1/article` with that new cookie → assert `200`.
|
||||
7. Same seed as case 6 minus the article calls. After the password reset:
|
||||
- `POST /v1/auth/verify` with the pre-reset cookie and `{"code":"anything"}` → assert `401`, body
|
||||
equals `apperr.UnauthorizedErrorMessage`.
|
||||
- `DELETE /v1/auth/register` with the pre-reset cookie → assert `401`, body equals
|
||||
`apperr.UnauthorizedErrorMessage`, and `repo.Get(7)` still succeeds.
|
||||
8. Double-reset case: seed as in 6, reset once (`TokenVersion` → 1), log in (capture "interim"
|
||||
cookie, `token_version: 1`), re-seed `PasswordResetCode` to bcrypt hash of "reset-code-2" with a
|
||||
fresh expiration, reset again with `{"email":"alice@example.com","code":"reset-code-2","password":"newerpassword1"}`
|
||||
→ assert `TokenVersion == 2`. Assert both the original (`token_version: 0`) and interim
|
||||
(`token_version: 1`) cookies now get `401` on `POST /v1/article`, and a jwt minted after logging
|
||||
in with `newerpassword1` (`token_version: 2`) gets `200`.
|
||||
9. Isolation case: seed user id 7 "alice" as in 6 and user id 9 "bob", `is_verified: true`,
|
||||
`TokenVersion: 0`. Mint a cookie for user id 9 encoding `token_version: 0`. Reset alice's
|
||||
password. Assert `repo.users[9].TokenVersion` is still `0`, and `POST /v1/article` with bob's
|
||||
cookie still returns `200`.
|
||||
10. Cancelled-registration regression, table-driven over `POST /v1/auth/verify` (body
|
||||
`{"code":"anything"}`) and `DELETE /v1/auth/register` (no body): seed user id 7 "alice",
|
||||
`is_verified: false`, `TokenVersion: 0`. Mint a cookie encoding `{"user_id":7,"token_version":0}`.
|
||||
`DELETE /v1/auth/register` with it → assert `200`. Then send the pre-cancellation cookie against
|
||||
each of the two routes in turn (fresh repo/router state per sub-case, or re-running the cancel
|
||||
step) → assert `401` and body `apperr.UnauthorizedErrorMessage` for both.
|
||||
11. Missing-claim case: seed user id 7, `is_verified: true`, `TokenVersion: 0`. Mint a jwt with
|
||||
`token.Encode(map[string]any{"user_id": uint32(7)})` (no `token_version` key at all).
|
||||
`POST /v1/article` → assert `401`, body `apperr.UnauthorizedErrorMessage`.
|
||||
12. Wrong-type case: mint a jwt with
|
||||
`token.Encode(map[string]any{"user_id": uint32(7), "token_version": "0"})` (string, not a
|
||||
number). `POST /v1/article` → assert `401`, body `apperr.UnauthorizedErrorMessage`, and confirm
|
||||
the test process does not panic (no `recover` needed — a crash would fail the test run outright).
|
||||
|
||||
## Use-case-level checks (no HTTP)
|
||||
|
||||
13. `mockUserRepo` seeded with user id 3, email "alice@example.com", `TokenVersion: 5`,
|
||||
`PasswordResetCode` = bcrypt hash of "reset-code-1", expiration one hour ahead. Build
|
||||
`ResetPasswordUseCase` directly over the fake. Call
|
||||
`ResetPassword("alice@example.com", "reset-code-1", "newpassword1")` → assert `err == nil` and
|
||||
`repo.users[3].TokenVersion == 6`.
|
||||
|
||||
## Manual verification (not exercised by the Go test)
|
||||
|
||||
- The SPA behaviour of being logged out / redirected to login when an API call comes back `401`
|
||||
after a password reset or a cancelled registration (wherever that's handled globally, e.g. an
|
||||
axios/fetch interceptor) is out of scope for this slice's automated test and should be checked by
|
||||
hand against a running dev build.
|
||||
- Confirming against the real SQLite file that `TokenVersion` persists correctly through
|
||||
`gorm.AutoMigrate` for existing rows (defaulting to `0`) is a manual check against a real
|
||||
migration, not re-verified by the in-memory fakes here.
|
||||
Reference in new issue
Block a user