refac: reamaining auth surface rewrite, T0
This commit is contained in:
1 parent
6d510d2805
commit
aa865af2e9
50 files changed
+3723
-149
No files matched your search
@@ -0,0 +1,127 @@
|
||||
Feature: A logged-in user verifies their account with the emailed code
|
||||
|
||||
As the maintainer of RenewCMS
|
||||
I want a logged-in user to confirm ownership of their account by submitting the verification
|
||||
code that CreateUser already emailed them, have a wrong or expired code refused without
|
||||
changing anything, have the stored code cleared the moment it is successfully used so it
|
||||
cannot be replayed, and have a token that carries no usable user_id answered with 401 instead
|
||||
of a panic
|
||||
So that account verification is a real security check, not a formality, and a malformed or
|
||||
hand-crafted token can never crash the server
|
||||
|
||||
Background:
|
||||
Given CreateUserUseCase.CreateUser already stores a bcrypt hash of a random verification code
|
||||
on the new user, alongside VerificationExpiration set to two hours after creation, and emails
|
||||
the raw code to the user (features/registration_gate_and_verification_email.feature)
|
||||
And this slice adds POST /v1/auth/verify, mounted in api/router.go inside a group that applies
|
||||
jwtauth.Verifier and jwtauth.Authenticator to the auth router, so a request with no jwt
|
||||
cookie, an expired jwt, or a jwt with a bad signature never reaches the handler and is
|
||||
answered 401 by jwtauth.Authenticator itself, as already happens for /v1/article routes
|
||||
And the request body is decoded into a VerifyCredentials struct with one field, Code, validated
|
||||
required by validator.Validate.Struct before anything else
|
||||
And a new use case, VerifyUserUseCase (internal/infrastructure/useCases/VerifyUser.go), is
|
||||
injected into auth.Handler as VerifyUseCase; it depends only on user.Repository and
|
||||
hash.Hasher and exposes Verify(userId uint32, code string) (user.User, error)
|
||||
And VerifyUserUseCase.Verify loads the user by id, refuses with the new sentinel
|
||||
user.ErrInvalidVerificationCode when the stored VerificationCode is empty (already used, or
|
||||
never set) or hash.Hasher.Compare finds the submitted code does not match the stored hash,
|
||||
refuses with the new sentinel user.ErrVerificationCodeExpired when
|
||||
time.Now() is after the stored VerificationExpiration, and otherwise calls a new
|
||||
user.Repository method, ClearVerificationCode(userId uint32) (user.User, error), that sets
|
||||
IsVerified true and VerificationCode "" (stored as SQL NULL, the same convention as
|
||||
PasswordResetCode in features/unverified_users_and_reregistration.feature) in one write
|
||||
And today, auth.Handler.IsVerified reads token.PrivateClaims()["user_id"].(float64) with a bare
|
||||
type assertion that panics if the claim is absent or not a number; this slice does not touch
|
||||
IsVerified, but the new Verify handler extracts user_id from the already-verified token's
|
||||
claims safely, and answers 401 with a new apperr.UnauthorizedErrorMessage constant whenever
|
||||
the claim is missing, is not a number, or does not match any user in the repository, instead
|
||||
of panicking or trusting the value
|
||||
And a wrong or expired code is mapped to 403 with a new apperr.VerificationFailedErrorMessage
|
||||
constant, the same status family Login already uses for a rejected credential
|
||||
And on success the handler responds 200 with the updated user as JSON (is_verified true,
|
||||
verification_code "")
|
||||
|
||||
Scenario: A logged-in user verifies their account with the correct emailed code
|
||||
Given a user repository containing user id 7, username "alice", is_verified false, whose stored
|
||||
VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is one hour
|
||||
from now
|
||||
And a valid jwt cookie for user id 7
|
||||
When the client sends POST /v1/auth/verify with body {"code": "abc123"}
|
||||
Then the response status is 200
|
||||
And the response body is the user as JSON with is_verified true and verification_code ""
|
||||
And the user repository's record for id 7 now has is_verified true and an empty
|
||||
verification_code
|
||||
|
||||
Scenario: A wrong code is refused and nothing changes
|
||||
Given a user repository containing user id 7, username "alice", is_verified false, whose stored
|
||||
VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is one hour
|
||||
from now
|
||||
And a valid jwt cookie for user id 7
|
||||
When the client sends POST /v1/auth/verify with body {"code": "wrong-code"}
|
||||
Then the response status is 403
|
||||
And the response body is "The verification code is invalid or has expired."
|
||||
And the user repository's record for id 7 still has is_verified false and the same stored
|
||||
verification_code
|
||||
|
||||
Scenario: An expired code is refused and nothing changes
|
||||
Given a user repository containing user id 7, username "alice", is_verified false, whose stored
|
||||
VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is one hour
|
||||
in the past
|
||||
And a valid jwt cookie for user id 7
|
||||
When the client sends POST /v1/auth/verify with body {"code": "abc123"}
|
||||
Then the response status is 403
|
||||
And the response body is "The verification code is invalid or has expired."
|
||||
And the user repository's record for id 7 still has is_verified false and the same stored
|
||||
verification_code
|
||||
|
||||
Scenario: The code is cleared once used, so it cannot be replayed
|
||||
Given a user repository containing user id 7, username "alice", is_verified false, whose stored
|
||||
VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is one hour
|
||||
from now
|
||||
And a valid jwt cookie for user id 7
|
||||
When the client sends POST /v1/auth/verify with body {"code": "abc123"}
|
||||
Then the response status is 200
|
||||
When the client sends POST /v1/auth/verify again with body {"code": "abc123"}
|
||||
Then the response status is 403
|
||||
And the response body is "The verification code is invalid or has expired."
|
||||
|
||||
Scenario Outline: A token without a valid user_id returns 401 instead of crashing
|
||||
Given a user repository containing user id 7, username "alice", is_verified false, whose stored
|
||||
VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is one hour
|
||||
from now
|
||||
And <token description>
|
||||
When the client sends POST /v1/auth/verify with body {"code": "abc123"}
|
||||
Then the response status is 401
|
||||
And the response body is the new apperr.UnauthorizedErrorMessage text
|
||||
And the handler does not panic
|
||||
And the user repository's record for id 7 still has is_verified false
|
||||
|
||||
Examples:
|
||||
| token description |
|
||||
| a validly signed jwt cookie whose claims carry no "user_id" key at all |
|
||||
| a validly signed jwt cookie whose "user_id" claim is the string "seven" |
|
||||
| a validly signed jwt cookie whose "user_id" claim is the number 999, which does not match any user |
|
||||
|
||||
Scenario: The use case itself enforces the rule, independently of any handler
|
||||
Given a user.Repository fake containing user id 3 with VerificationCode equal to the bcrypt
|
||||
hash of "right-code" and VerificationExpiration one hour from now
|
||||
And a VerifyUserUseCase built directly over that fake and a hasher
|
||||
When Verify is called with userId 3 and code "wrong-code"
|
||||
Then the call returns user.ErrInvalidVerificationCode, asserted with errors.Is
|
||||
And the fake repository's ClearVerificationCode method was never invoked
|
||||
|
||||
Scenario: The use case rejects an expired code independently of any handler
|
||||
Given a user.Repository fake containing user id 3 with VerificationCode equal to the bcrypt
|
||||
hash of "right-code" and VerificationExpiration one hour in the past
|
||||
And a VerifyUserUseCase built directly over that fake and a hasher
|
||||
When Verify is called with userId 3 and code "right-code"
|
||||
Then the call returns user.ErrVerificationCodeExpired, asserted with errors.Is
|
||||
And the fake repository's ClearVerificationCode method was never invoked
|
||||
|
||||
Scenario: The use case succeeds and clears the code independently of any handler
|
||||
Given a user.Repository fake containing user id 3 with VerificationCode equal to the bcrypt
|
||||
hash of "right-code" and VerificationExpiration one hour from now
|
||||
And a VerifyUserUseCase built directly over that fake and a hasher
|
||||
When Verify is called with userId 3 and code "right-code"
|
||||
Then the call returns a user with IsVerified true and VerificationCode ""
|
||||
And the fake repository's ClearVerificationCode method was invoked exactly once with userId 3
|
||||
@@ -0,0 +1,84 @@
|
||||
# QA procedure: account_verification
|
||||
|
||||
Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`, `stubHasher`).
|
||||
|
||||
## Test infrastructure additions needed first
|
||||
|
||||
1. Add a `ClearVerificationCode(userId uint32) (user.User, error)` method to `mockUserRepo`
|
||||
mirroring the existing `UpdateVerificationStatus`/`UpdatePasswordResetCode` pattern: honor
|
||||
`m.fail.trigger("ClearVerificationCode")`, look the user up (return `user.ErrUserNotFound` if
|
||||
absent), set `IsVerified = true` and `VerificationCode = ""`, store it back, return it. Track a
|
||||
call counter/last-argument on the mock (or reuse the existing fail-switch style) so scenarios can
|
||||
assert it was or wasn't invoked.
|
||||
2. `stubHasher.Compare(hashed, value)` already returns `nil` only when `hashed == "hashed:"+value`
|
||||
and an error otherwise, so seed `VerificationCode` in fixtures as `"hashed:abc123"` to represent
|
||||
"the bcrypt hash of abc123", and submit `"abc123"` as the matching code.
|
||||
|
||||
## Setup shared by every case
|
||||
|
||||
3. Call `validator.InitValidator()` once.
|
||||
4. Use-case-level checks build `useCases.NewVerifyUserUseCase(repo, &stubHasher{})` where `repo` is
|
||||
a `mockUserRepo` pre-populated directly via `repo.users[id] = user.User{...}`.
|
||||
5. HTTP-level checks wire `mockUserRepo`, `useCases.NewVerifyUserUseCase(repo, &stubHasher{})` and
|
||||
the other existing use cases into `auth.NewHandler(...)` (extended with the new `VerifyUseCase`
|
||||
parameter) with a test `jwtauth.New("HS256", []byte("test-secret"), nil)`. Mount
|
||||
`POST /v1/auth/verify` on a fresh `chi.NewRouter()` wrapped in `jwtauth.Verifier(token)` then
|
||||
`jwtauth.Authenticator(token)`, mirroring the protected group `api/router.go` adds inside
|
||||
`NewAuthRouter`. No live server. Mint jwt cookies with `token.Encode(map[string]any{"user_id":
|
||||
uint32(7)})` for the valid-token cases, following the convention in
|
||||
`features/article_publishing.qa.md`.
|
||||
|
||||
## Use-case-level checks (no HTTP)
|
||||
|
||||
6. Repo has user id 3 with `VerificationCode: "hashed:right-code"`,
|
||||
`VerificationExpiration: time.Now().Add(time.Hour)`. Call `Verify(3, "wrong-code")`. Assert
|
||||
`errors.Is(err, user.ErrInvalidVerificationCode)`. Assert `ClearVerificationCode` was never
|
||||
called on the repo.
|
||||
7. Same repo, `VerificationExpiration: time.Now().Add(-time.Hour)`. Call `Verify(3, "right-code")`.
|
||||
Assert `errors.Is(err, user.ErrVerificationCodeExpired)`. Assert `ClearVerificationCode` was
|
||||
never called.
|
||||
8. Same repo, `VerificationExpiration: time.Now().Add(time.Hour)`. Call `Verify(3, "right-code")`.
|
||||
Assert `err == nil`, `result.IsVerified == true`, `result.VerificationCode == ""`. Assert the
|
||||
repo's stored record for id 3 now has `IsVerified == true` and `VerificationCode == ""`.
|
||||
9. Repo has user id 3 with `VerificationCode: ""` (already verified/used). Call
|
||||
`Verify(3, "anything")`. Assert `errors.Is(err, user.ErrInvalidVerificationCode)`.
|
||||
|
||||
## HTTP-level checks (fakes, per the shared setup above)
|
||||
|
||||
10. Seed `mockUserRepo` with user id 7, username "alice", `IsVerified: false`,
|
||||
`VerificationCode: "hashed:abc123"`, `VerificationExpiration: time.Now().Add(time.Hour)`. Mint a
|
||||
valid jwt cookie for user id 7. `POST /v1/auth/verify` with `{"code":"abc123"}` → assert status
|
||||
`200`, response body decodes to a user JSON with `is_verified: true` and
|
||||
`verification_code: ""`, and the repo's record for id 7 now has `IsVerified == true` and
|
||||
`VerificationCode == ""`.
|
||||
11. Same seed, wrong code `{"code":"wrong-code"}` → assert status `403`, body exactly
|
||||
`"The verification code is invalid or has expired."`, and the repo's record for id 7 is
|
||||
unchanged (`IsVerified == false`, `VerificationCode == "hashed:abc123"`).
|
||||
12. Same seed but `VerificationExpiration: time.Now().Add(-time.Hour)`, correct code
|
||||
`{"code":"abc123"}` → assert status `403`, same body text as above, and the repo's record for
|
||||
id 7 unchanged.
|
||||
13. Replay: repeat case 10's successful call, then immediately `POST /v1/auth/verify` again with
|
||||
the same body `{"code":"abc123"}` on the same router/repo → assert the second response status
|
||||
is `403` with the same body text (the cleared code no longer matches).
|
||||
14. Table-driven over three "no valid user_id" cases, each against the case-10 seed and asserting
|
||||
status `401` and a response body equal to the new `apperr.UnauthorizedErrorMessage` constant,
|
||||
without the handler panicking (a panic inside `httptest`/chi would fail the test run directly,
|
||||
so simply completing with the asserted status is sufficient proof):
|
||||
- mint the jwt with `token.Encode(map[string]any{"role": "user"})` (no `user_id` key at all);
|
||||
- mint the jwt with `token.Encode(map[string]any{"user_id": "seven"})` (string, not a number);
|
||||
- mint the jwt with `token.Encode(map[string]any{"user_id": uint32(999)})` (well-formed but no
|
||||
such user in the repo).
|
||||
After each sub-case, assert the repo's record for id 7 is still `IsVerified == false`.
|
||||
15. A request with no `jwt` cookie at all against the same router → assert status `401` (produced
|
||||
by `jwtauth.Authenticator` itself, before the handler runs); do not assert an exact body for
|
||||
this one case since it is the library's own default text, not `apperr.UnauthorizedErrorMessage`.
|
||||
|
||||
## Manual verification (not exercised by the Go test)
|
||||
|
||||
- The real SMTP-delivered verification link and the SPA's `/register/validate?c=...` page actually
|
||||
calling `POST /v1/auth/verify` with the code parsed from the URL, and redirecting or showing a
|
||||
message on success/failure, are out of scope for this slice's automated test and should be
|
||||
checked by hand against a running dev build.
|
||||
- The pre-existing bare type assertion in `auth.Handler.IsVerified`
|
||||
(`token.PrivateClaims()["user_id"].(float64)`) is untouched by this slice and remains a latent
|
||||
panic if that helper is ever wired into a route; it is not exercised here.
|
||||
@@ -0,0 +1,112 @@
|
||||
Feature: A logged-in, unverified user cancels their pending registration
|
||||
|
||||
As the maintainer of RenewCMS
|
||||
I want a logged-in user who has not yet verified their account to be able to delete that
|
||||
account and clear their session in one call, while a user who is already verified cannot use
|
||||
this route to delete their own account, and a token that carries no usable user_id is answered
|
||||
with 401 instead of a panic
|
||||
So that someone who registered by mistake, or changed their mind before verifying, can free their
|
||||
username and email immediately (features/unverified_users_and_reregistration.feature), without
|
||||
exposing a general self-delete for verified accounts
|
||||
|
||||
Background:
|
||||
Given this slice adds DELETE /v1/auth/register, mounted in api/router.go inside the same
|
||||
protected group NewAuthRouter already applies to POST /v1/auth/verify: jwtauth.Verifier then
|
||||
jwtauth.Authenticator, so a request with no jwt cookie, an expired jwt, or a jwt with a bad
|
||||
signature never reaches the handler and is answered 401 by jwtauth.Authenticator itself
|
||||
And the handler extracts user_id from the already-verified token's claims using the same safe
|
||||
pattern auth.Handler.Verify already established (features/account_verification.feature):
|
||||
401 with apperr.UnauthorizedErrorMessage whenever the "user_id" claim is missing, is not a
|
||||
number, or does not match any user in the repository, never a panic and never trusting the
|
||||
raw claim value
|
||||
And a new use case, CancelRegistrationUseCase (internal/infrastructure/useCases/CancelRegistration.go),
|
||||
is injected into auth.Handler as CancelRegistrationUseCase; it depends only on user.Repository
|
||||
and exposes CancelRegistration(userId uint32) error
|
||||
And CancelRegistrationUseCase.CancelRegistration loads the user by id, returns the existing
|
||||
sentinel user.ErrUserNotFound when no such user exists, refuses with a new sentinel
|
||||
user.ErrAlreadyVerified when the loaded user's IsVerified is already true, and otherwise calls
|
||||
user.Repository.Delete(userId), whose GORM implementation removes the row entirely rather than
|
||||
soft-deleting it (features/unverified_users_and_reregistration.feature), so the same username
|
||||
and email are immediately free for a new registration
|
||||
And the handler maps user.ErrUserNotFound to 401 with apperr.UnauthorizedErrorMessage, the same
|
||||
status and body Verify already uses for an unresolvable user_id, and maps
|
||||
user.ErrAlreadyVerified to 403 with a new apperr.AlreadyVerifiedErrorMessage constant
|
||||
And on success the handler clears the "jwt" cookie exactly the way auth.Handler.Logout already
|
||||
does (Max-Age -1, empty value, Path "/") and responds 200 with {"message": "..."}
|
||||
And on any error path the handler returns before touching the "jwt" cookie, so an unresolved or
|
||||
already-verified request leaves the caller's existing session untouched
|
||||
|
||||
Scenario: An unverified user cancels their pending registration
|
||||
Given a user repository containing user id 7, username "alice", email "alice@example.com",
|
||||
is_verified false
|
||||
And a valid jwt cookie for user id 7
|
||||
When the client sends DELETE /v1/auth/register with that cookie
|
||||
Then the response status is 200
|
||||
And the response body is {"message": "..."}
|
||||
And the response clears the "jwt" cookie
|
||||
And the user repository no longer contains user id 7; Get(7) returns user.ErrUserNotFound
|
||||
|
||||
Scenario: Cancelling frees the username and email for a new registration
|
||||
Given a user repository containing user id 7, username "alice", email "alice@example.com",
|
||||
is_verified false
|
||||
And a valid jwt cookie for user id 7
|
||||
When the client sends DELETE /v1/auth/register with that cookie
|
||||
Then the response status is 200
|
||||
When the client sends POST /v1/auth/register with username "alice", password
|
||||
"hunter2example12", email "alice@example.com"
|
||||
Then the response status is 200
|
||||
And a jwt cookie is set
|
||||
And the newly created user's id is different from 7
|
||||
|
||||
Scenario: An already-verified user cannot cancel their account this way
|
||||
Given a user repository containing user id 7, username "alice", email "alice@example.com",
|
||||
is_verified true
|
||||
And a valid jwt cookie for user id 7
|
||||
When the client sends DELETE /v1/auth/register with that cookie
|
||||
Then the response status is 403
|
||||
And the response body is the new apperr.AlreadyVerifiedErrorMessage text
|
||||
And the response does not clear the "jwt" cookie
|
||||
And the user repository's record for id 7 is unchanged: is_verified true, the row still exists
|
||||
|
||||
Scenario Outline: A token without a valid user_id returns 401 instead of crashing
|
||||
Given a user repository containing user id 7, username "alice", is_verified false
|
||||
And <token description>
|
||||
When the client sends DELETE /v1/auth/register with that cookie
|
||||
Then the response status is 401
|
||||
And the response body is the apperr.UnauthorizedErrorMessage text
|
||||
And the handler does not panic
|
||||
And the response does not clear the "jwt" cookie
|
||||
And the user repository's record for id 7 still exists
|
||||
|
||||
Examples:
|
||||
| token description |
|
||||
| a validly signed jwt cookie whose claims carry no "user_id" key at all |
|
||||
| a validly signed jwt cookie whose "user_id" claim is the string "seven" |
|
||||
| a validly signed jwt cookie whose "user_id" claim is the number 999, which does not match any user |
|
||||
|
||||
Scenario: A request with no jwt cookie never reaches the handler
|
||||
Given a user repository containing user id 7, username "alice", is_verified false
|
||||
And no jwt cookie is sent
|
||||
When the client sends DELETE /v1/auth/register without a cookie
|
||||
Then the response status is 401
|
||||
And the user repository's record for id 7 still exists
|
||||
|
||||
Scenario: The use case deletes an unverified user, independently of any handler
|
||||
Given a user.Repository fake containing user id 3, is_verified false
|
||||
And a CancelRegistrationUseCase built directly over that fake
|
||||
When CancelRegistration is called with userId 3
|
||||
Then the call returns nil
|
||||
And the fake repository no longer contains user id 3
|
||||
|
||||
Scenario: The use case refuses to delete an already-verified user, independently of any handler
|
||||
Given a user.Repository fake containing user id 3, is_verified true
|
||||
And a CancelRegistrationUseCase built directly over that fake
|
||||
When CancelRegistration is called with userId 3
|
||||
Then the call returns user.ErrAlreadyVerified, asserted with errors.Is
|
||||
And the fake repository still contains user id 3
|
||||
|
||||
Scenario: The use case reports an unknown user, independently of any handler
|
||||
Given a user.Repository fake containing no user with id 42
|
||||
And a CancelRegistrationUseCase built directly over that fake
|
||||
When CancelRegistration is called with userId 42
|
||||
Then the call returns user.ErrUserNotFound, asserted with errors.Is
|
||||
@@ -0,0 +1,76 @@
|
||||
# QA procedure: cancel_pending_registration
|
||||
|
||||
Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`) and following the
|
||||
router/handler wiring pattern already used in `test/account_verification_http_test.go`.
|
||||
|
||||
## Test infrastructure reused, none new needed
|
||||
|
||||
`mockUserRepo` already supports deleting an entry (used by the existing `DeleteUserUseCase` tests
|
||||
in `test/use_case_baseline_test.go`) and returning `user.ErrUserNotFound` from `Get` once a user is
|
||||
gone. No new mock methods are required; `CancelRegistrationUseCase` only calls `Get` and `Delete`,
|
||||
both of which `mockUserRepo` already implements.
|
||||
|
||||
## Setup shared by every case
|
||||
|
||||
1. Call `validator.InitValidator()` once.
|
||||
2. Use-case-level checks build `useCases.NewCancelRegistrationUseCase(repo)` where `repo` is a
|
||||
`mockUserRepo` pre-populated directly via `repo.users[id] = user.User{...}`.
|
||||
3. HTTP-level checks wire `mockUserRepo`, `useCases.NewCancelRegistrationUseCase(repo)` and the
|
||||
other existing use cases into `auth.NewHandler(...)` (extended with the new
|
||||
`CancelRegistrationUseCase` parameter) with a test `jwtauth.New("HS256", []byte("test-secret"),
|
||||
nil)`. Mount `DELETE /v1/auth/register` on a fresh `chi.NewRouter()` wrapped in
|
||||
`jwtauth.Verifier(token)` then `jwtauth.Authenticator(token)`, mirroring the protected group
|
||||
`api/router.go` adds inside `NewAuthRouter` (the same group `POST /v1/auth/verify` already sits
|
||||
in). Mint jwt cookies with `token.Encode(map[string]any{"user_id": uint32(7)})` for the
|
||||
valid-token cases.
|
||||
|
||||
## Use-case-level checks (no HTTP)
|
||||
|
||||
4. Repo has user id 3, `IsVerified: false`. Call `CancelRegistration(3)`. Assert `err == nil`.
|
||||
Assert `repo.users` no longer has key 3 (or `repo.Get(3)` returns `user.ErrUserNotFound`).
|
||||
5. Repo has user id 3, `IsVerified: true`. Call `CancelRegistration(3)`. Assert
|
||||
`errors.Is(err, user.ErrAlreadyVerified)`. Assert the repo still has user id 3 with
|
||||
`IsVerified: true`.
|
||||
6. Repo has no user with id 42. Call `CancelRegistration(42)`. Assert
|
||||
`errors.Is(err, user.ErrUserNotFound)`.
|
||||
|
||||
## HTTP-level checks (fakes, per the shared setup above)
|
||||
|
||||
7. Seed `mockUserRepo` with user id 7, username "alice", email "alice@example.com",
|
||||
`IsVerified: false`. Mint a valid jwt cookie for user id 7. `DELETE /v1/auth/register` with that
|
||||
cookie → assert status `200`, response body decodes to a JSON object with a `message` field,
|
||||
the response has a `Set-Cookie` header for `jwt` with `MaxAge < 0` (or an `Expires` in the past)
|
||||
and an empty `Value` (parse with `http.ParseSetCookie`, mirroring
|
||||
`secureAttributeFromResponse` in `test/runtime_settings_injection_http_test.go`), and
|
||||
`repo.Get(7)` now returns `user.ErrUserNotFound`.
|
||||
8. Re-registration after cancel: repeat case 7's cancel call on the same router/repo, then
|
||||
`POST /v1/auth/register` with `{"username":"alice","password":"hunter2example12","email":"alice@example.com"}`
|
||||
→ assert status `200`, a `jwt` `Set-Cookie` header is present with a non-empty value, and the
|
||||
newly created user's id (read back via `repo.GetByUsername("alice")` or the use case) is
|
||||
different from `7`.
|
||||
9. Seed `mockUserRepo` with user id 7, username "alice", email "alice@example.com",
|
||||
`IsVerified: true`. Mint a valid jwt cookie for user id 7. `DELETE /v1/auth/register` with that
|
||||
cookie → assert status `403`, body exactly the `apperr.AlreadyVerifiedErrorMessage` constant,
|
||||
no `Set-Cookie` header for `jwt` is present in the response, and `repo.Get(7)` still returns the
|
||||
user with `IsVerified: true`.
|
||||
10. Table-driven over three "no valid user_id" cases, each against a fresh seed of user id 7,
|
||||
`IsVerified: false`, and asserting status `401`, a response body equal to
|
||||
`apperr.UnauthorizedErrorMessage`, no `jwt` `Set-Cookie` header in the response, and
|
||||
`repo.Get(7)` still succeeding afterward:
|
||||
- mint the jwt with `token.Encode(map[string]any{"role": "user"})` (no `user_id` key at all);
|
||||
- mint the jwt with `token.Encode(map[string]any{"user_id": "seven"})` (string, not a number);
|
||||
- mint the jwt with `token.Encode(map[string]any{"user_id": uint32(999)})` (well-formed but no
|
||||
such user in the repo).
|
||||
11. A request with no `jwt` cookie at all against the same router → assert status `401` (produced
|
||||
by `jwtauth.Authenticator` itself, before the handler runs); do not assert an exact body for
|
||||
this case, and assert `repo.Get(7)` still succeeds afterward.
|
||||
|
||||
## Manual verification (not exercised by the Go test)
|
||||
|
||||
- The SPA-visible "cancel registration" affordance (wherever it is surfaced to a logged-in,
|
||||
unverified user, e.g. on the pending-verification screen) actually calling
|
||||
`DELETE /v1/auth/register` and redirecting to a logged-out state on success is out of scope for
|
||||
this slice's automated test and should be checked by hand against a running dev build.
|
||||
- End-to-end confirmation against the real SQLite file that the row is physically gone (not just
|
||||
hidden by a soft-delete) is already covered at the repository level by
|
||||
`features/unverified_users_and_reregistration.feature` and is not re-verified here.
|
||||
@@ -0,0 +1,150 @@
|
||||
Feature: Anyone can request a password reset, and the response never reveals whether the email exists
|
||||
|
||||
As the maintainer of RenewCMS
|
||||
I want POST /v1/auth/password-reset to always answer with the same status and body regardless of
|
||||
whether the submitted email belongs to an account, and, when it does, to email that account a
|
||||
reset code that expires one hour after it was generated
|
||||
So that the endpoint cannot be used to discover which email addresses are registered, and a
|
||||
legitimate reset link always uses the server's own configured public URL
|
||||
|
||||
Background:
|
||||
Given POST /v1/auth/password-reset is public and mounted alongside /register, /login and
|
||||
/logout in NewAuthRouter, outside the jwt-protected group
|
||||
And the request body is decoded into a RequestPasswordResetCredentials struct with one field,
|
||||
Email, validated required,email by validator.Validate.Struct before anything else
|
||||
And a decode failure or a validation failure (missing or malformed email) is answered exactly
|
||||
like every other auth route: apperr.BodyErrorMessage with 400 for a decode failure, or the
|
||||
validator's error text with 400 for a validation failure; only the "does this email exist"
|
||||
question is hidden, not the shape of the request itself
|
||||
And this slice adds a new use case, RequestPasswordResetUseCase
|
||||
(internal/infrastructure/useCases/RequestPasswordReset.go), injected into auth.Handler as
|
||||
RequestPasswordResetUseCase; its constructor takes user.Repository, hash.Hasher, mail.Repository
|
||||
and the configured public URL string, the same shape as CreateUserUseCase, with the public URL
|
||||
coming from config.Settings.PublicURL through api/dependecyInjection.go
|
||||
And RequestPasswordReset(email string) error looks the user up with user.Repository.GetByEmail;
|
||||
when no user has that email, or GetByEmail returns any other error, it does nothing further and
|
||||
returns nil
|
||||
And when a user is found, RequestPasswordReset generates a random raw code with uuid.NewString()
|
||||
(the same generator CreateUser uses for the verification code), hashes it with hash.Hasher.Hash,
|
||||
and stores the hash together with a fresh expiration of time.Now().Add(1 * time.Hour) by calling
|
||||
user.Repository.UpdatePasswordResetCode(userId uint32, code string, expiration time.Time)
|
||||
(user.User, error) — this slice adds the expiration parameter to that existing repository
|
||||
method and to UpdateUserUseCase.UpdatePasswordResetCode, which gains the same parameter and
|
||||
passes it straight through, and adds a new PasswordResetExpiration time.Time field to
|
||||
user.User, to internal/infrastructure/persistence/models.User (a plain, non-nullable column,
|
||||
the same shape as VerificationExpiration) and to the UserToDomain/UserToModel mappers; no new
|
||||
entry is needed in api/dependecyInjection.go's AutoMigrate call because models.User is already
|
||||
migrated there
|
||||
And storing a new code and expiration always overwrites whatever was stored before, so only the
|
||||
most recently requested code and expiration are ever valid; a user who requests a reset twice
|
||||
can only use the second code, and the first one no longer matches once overwritten
|
||||
And after storing succeeds, RequestPasswordReset calls mail.Repository.Send with the user's
|
||||
email, template "passwordReset" (the already-existing
|
||||
internal/infrastructure/mailer/templates/passwordReset.html, which renders a link from
|
||||
"{{.Host}}/register/reset/validate?c={{.VerificationCode}}&email={{.Email}}"), and a
|
||||
MailPasswordResetData value whose Host is the injected public URL, whose VerificationCode is
|
||||
the raw (unhashed) code, and whose Email is the user's email
|
||||
And RequestPasswordReset never returns a non-nil error to its caller: a failure to hash the
|
||||
code, to store it, or to send the email is swallowed the same way a not-found email is, so the
|
||||
handler can never tell those cases apart either — sending the request always looks the same to
|
||||
the caller regardless of whether the email exists or whether the mail send happened to succeed
|
||||
And a reset applies the same way regardless of the user's IsVerified status: an unverified user
|
||||
can request and receive a password reset email exactly like a verified one, since resetting a
|
||||
password is unrelated to email verification
|
||||
And auth.Handler.RequestPasswordReset calls RequestPasswordResetUseCase.RequestPasswordReset and
|
||||
then always responds 200 with the same body,
|
||||
{"message": "If an account exists for that email, a password reset link has been sent."},
|
||||
never sets or clears the jwt cookie, and never branches on the use case's return value
|
||||
And this slice does not add the confirmation endpoint that consumes the code (the page behind
|
||||
"/register/reset/validate?c=...&email=..." and the POST call it would make); only the request
|
||||
step is in scope
|
||||
|
||||
Scenario: Requesting a reset for an existing email sends the reset email and returns the generic message
|
||||
Given the user repository contains a user "alice" with email "alice@example.com" and IsVerified true
|
||||
When the client sends POST /v1/auth/password-reset with body {"email": "alice@example.com"}
|
||||
Then the response status is 200
|
||||
And the response body is {"message": "If an account exists for that email, a password reset link has been sent."}
|
||||
And exactly one email was sent to "alice@example.com" using template "passwordReset"
|
||||
And that email's data has VerificationCode equal to the raw code stored in the repository's
|
||||
hashed PasswordResetCode for alice, not the hash itself, and Email equal to "alice@example.com"
|
||||
|
||||
Scenario: Requesting a reset for an email with no account returns the identical response and sends no email
|
||||
Given the user repository is empty
|
||||
When the client sends POST /v1/auth/password-reset with body {"email": "nobody@example.com"}
|
||||
Then the response status is 200
|
||||
And the response body is {"message": "If an account exists for that email, a password reset link has been sent."}
|
||||
And no email was sent
|
||||
|
||||
Scenario: The reset email's Host reflects the injected public URL
|
||||
Given a config.Settings value with PublicURL "https://cms.example.com"
|
||||
And the auth handler is built from that settings value
|
||||
And the user repository contains a user "alice" with email "alice@example.com"
|
||||
When the client sends POST /v1/auth/password-reset with body {"email": "alice@example.com"}
|
||||
Then the email sent to "alice@example.com" has data.Host equal to "https://cms.example.com"
|
||||
|
||||
Scenario: The stored reset code expires one hour after creation
|
||||
Given the user repository contains a user "alice" with email "alice@example.com"
|
||||
When the client sends POST /v1/auth/password-reset with body {"email": "alice@example.com"}
|
||||
Then alice's stored PasswordResetExpiration is within 5 seconds of time.Now().Add(1 * time.Hour)
|
||||
|
||||
Scenario: A second request overwrites the first code, invalidating it
|
||||
Given the user repository contains a user "alice" with email "alice@example.com"
|
||||
When the client sends POST /v1/auth/password-reset with body {"email": "alice@example.com"}
|
||||
Then exactly one email has been sent, carrying raw code "code-1" in its VerificationCode data
|
||||
When the client sends POST /v1/auth/password-reset again with body {"email": "alice@example.com"}
|
||||
Then exactly two emails have been sent in total, and the second one's VerificationCode data
|
||||
differs from "code-1"
|
||||
And alice's stored, hashed PasswordResetCode no longer matches "code-1" but matches the raw code
|
||||
from the second email
|
||||
|
||||
Scenario: A mail-send failure for an existing user still returns the same generic response
|
||||
Given the user repository contains a user "alice" with email "alice@example.com"
|
||||
And the mail repository is set to fail on its next Send call
|
||||
When the client sends POST /v1/auth/password-reset with body {"email": "alice@example.com"}
|
||||
Then the response status is 200
|
||||
And the response body is {"message": "If an account exists for that email, a password reset link has been sent."}
|
||||
And no jwt cookie is set
|
||||
|
||||
Scenario: An unverified user can request a password reset exactly like a verified one
|
||||
Given the user repository contains a user "bob" with email "bob@example.com" and IsVerified false
|
||||
When the client sends POST /v1/auth/password-reset with body {"email": "bob@example.com"}
|
||||
Then the response status is 200
|
||||
And the response body is {"message": "If an account exists for that email, a password reset link has been sent."}
|
||||
And exactly one email was sent to "bob@example.com" using template "passwordReset"
|
||||
|
||||
Scenario Outline: A malformed request is still rejected, since this is not an enumeration concern
|
||||
When the client sends POST /v1/auth/password-reset with body <body>
|
||||
Then the response status is 400
|
||||
|
||||
Examples:
|
||||
| body |
|
||||
| {} |
|
||||
| {"email": "not-an-email"} |
|
||||
| not-json |
|
||||
|
||||
Scenario: The use case itself hashes the code, stores it with a one-hour expiration, and sends the email
|
||||
Given a user.Repository fake containing user id 3, email "alice@example.com"
|
||||
And a RequestPasswordResetUseCase built directly over that fake, a hasher, a mail.Repository
|
||||
fake and public URL "https://cms.example.com"
|
||||
When RequestPasswordReset is called with "alice@example.com"
|
||||
Then the call returns nil
|
||||
And the fake repository's UpdatePasswordResetCode method was invoked exactly once with userId 3,
|
||||
a hashed code, and an expiration within 5 seconds of time.Now().Add(1 * time.Hour)
|
||||
And exactly one email was sent to "alice@example.com" using template "passwordReset" whose data
|
||||
carries the same raw code that was hashed, and Host "https://cms.example.com"
|
||||
|
||||
Scenario: The use case swallows an unknown email and returns nil without touching the repository or mailer
|
||||
Given an empty user.Repository fake
|
||||
And a RequestPasswordResetUseCase built directly over that fake, a hasher, and a mail.Repository fake
|
||||
When RequestPasswordReset is called with "nobody@example.com"
|
||||
Then the call returns nil
|
||||
And the fake repository's UpdatePasswordResetCode method was never invoked
|
||||
And no email was sent
|
||||
|
||||
Scenario: The use case swallows a mail repository failure and still returns nil
|
||||
Given a user.Repository fake containing user id 3, email "alice@example.com"
|
||||
And a RequestPasswordResetUseCase built directly over that fake, a hasher, and a mail.Repository
|
||||
fake set to fail on its next Send call
|
||||
When RequestPasswordReset is called with "alice@example.com"
|
||||
Then the call returns nil
|
||||
And the fake repository's UpdatePasswordResetCode method was invoked exactly once with userId 3
|
||||
@@ -0,0 +1,111 @@
|
||||
# QA procedure: request_password_reset
|
||||
|
||||
Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`, `stubHasher`) and
|
||||
`test/mail_test.go` (`mockMailRepo`, `recordedMail`, `failSwitch`).
|
||||
|
||||
## Test infrastructure additions needed first
|
||||
|
||||
1. Update `mockUserRepo.UpdatePasswordResetCode` to the new three-argument signature
|
||||
`UpdatePasswordResetCode(userId uint32, code string, expiration time.Time) (user.User, error)`:
|
||||
honor `m.fail.trigger("UpdatePasswordResetCode")`, look the user up (return `user.ErrUserNotFound`
|
||||
if absent), set both `PasswordResetCode = code` and `PasswordResetExpiration = expiration`, store
|
||||
it back, return it. Track a call counter and the last `(userId, code, expiration)` triple (mirroring
|
||||
the existing `clearVerificationCodeCalls`/`clearVerificationCodeUserIds` fields) so scenarios can
|
||||
assert it was or wasn't invoked and with what arguments.
|
||||
2. Update every existing caller of the old two-argument signature to pass an expiration: this is a
|
||||
breaking, cross-cutting change reaching `internal/infrastructure/useCases/UpdateUser.go`,
|
||||
`internal/infrastructure/persistence/repositories/userRepository.go`,
|
||||
`internal/domain/user/userRepositoryInterface.go`, `test/hasher_test.go` (the
|
||||
`UpdatePasswordResetCode` case in `TestUpdateUserUseCaseHashesUpdatedValues`),
|
||||
`test/write_not_found_errors_test.go`, and `test/use_case_baseline_test.go`. None of these are new
|
||||
to this slice; note them here so the coder updates all call sites together instead of missing one.
|
||||
3. Add `PasswordResetExpiration time.Time` to `user.User`, `models.User` and both mapper functions,
|
||||
the same shape as `VerificationExpiration` (a plain, non-nullable column with no explicit `gorm`
|
||||
tag beyond the struct field).
|
||||
4. `stubHasher.Compare(hashed, value)` returns `nil` only when `hashed == "hashed:"+value`, so a
|
||||
scenario asserting "the stored code no longer matches the first raw code but matches the second"
|
||||
can call `stubHasher{}.Compare(repo.users[id].PasswordResetCode, "code-1")` and expect an error,
|
||||
then `.Compare(..., secondRawCode)` and expect `nil`.
|
||||
|
||||
## Setup shared by every case
|
||||
|
||||
5. Call `validator.InitValidator()` once.
|
||||
6. Use-case-level checks build
|
||||
`useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, "https://cms.example.com")`
|
||||
where `repo` is a `mockUserRepo` and `mailRepo` is a `mockMailRepo`. Use `mailRepo.fail.failNext("Send")`
|
||||
to force the next `Send` call to fail.
|
||||
7. HTTP-level checks wire `mockUserRepo`, `mockMailRepo`,
|
||||
`useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, publicURL)` and the other
|
||||
existing use cases into `auth.NewHandler(...)` (extended with the new
|
||||
`RequestPasswordResetUseCase` parameter) with a test `jwtauth.New("HS256", []byte("test-secret"),
|
||||
nil)`. Mount `POST /v1/auth/password-reset` unprotected on a fresh `chi.NewRouter()` mirroring the
|
||||
public group `NewAuthRouter` already uses for `/register`, `/login` and `/logout`. No live server,
|
||||
no real SMTP. Default `publicURL` for cases that don't care about its value is
|
||||
`"https://cms.example.com"`.
|
||||
8. The expected response body for every "generic response" assertion is the exact JSON
|
||||
`{"message":"If an account exists for that email, a password reset link has been sent."}`; decode
|
||||
both the expected and actual bodies (or compare the raw bytes exactly, matching how `Login` and
|
||||
`Register`'s success bodies are asserted elsewhere) rather than doing a loose substring check.
|
||||
|
||||
## Use-case-level checks (no HTTP)
|
||||
|
||||
9. Repo pre-populated with one user, id 3, email "alice@example.com". Call
|
||||
`RequestPasswordReset("alice@example.com")`. Assert the return value is `nil`. Assert
|
||||
`UpdatePasswordResetCode` was invoked exactly once with `userId == 3`, a `code` matching
|
||||
`"hashed:" + <raw code>` and an `expiration` within 5 seconds of `time.Now().Add(time.Hour)`.
|
||||
Assert `mailRepo.SentEmails` has exactly one entry: `Address == "alice@example.com"`,
|
||||
`Template == "passwordReset"`, and its `Data` (asserted via a type assertion to
|
||||
`useCases.MailPasswordResetData`) has `VerificationCode` equal to the same raw code that hashes to
|
||||
the stored value, and `Host == "https://cms.example.com"`.
|
||||
10. On an empty repo, call `RequestPasswordReset("nobody@example.com")`. Assert the return value is
|
||||
`nil`. Assert `UpdatePasswordResetCode`'s call counter is still 0. Assert `mailRepo.SentEmails` is
|
||||
empty.
|
||||
11. Repo pre-populated with user id 3, email "alice@example.com". Set `mailRepo.fail.failNext("Send")`.
|
||||
Call `RequestPasswordReset("alice@example.com")`. Assert the return value is `nil` (not the mail
|
||||
repository's failure). Assert `UpdatePasswordResetCode` was still invoked exactly once (the store
|
||||
happens before the send).
|
||||
12. Repo pre-populated with two unverified users, "alice" and "bob" (`IsVerified: false`). Call
|
||||
`RequestPasswordReset` for alice's email. Assert the return value is `nil` and exactly one email
|
||||
was sent, showing IsVerified plays no role in the use case's behavior.
|
||||
|
||||
## HTTP-level checks (fakes, per the shared setup above)
|
||||
|
||||
13. Seed `mockUserRepo` with user "alice", email "alice@example.com", `IsVerified: true`.
|
||||
`POST /v1/auth/password-reset` with `{"email":"alice@example.com"}` → assert status `200`, body
|
||||
exactly matches the generic JSON from step 8, and `mockMailRepo.SentEmails` has exactly one entry
|
||||
with `Address == "alice@example.com"` and `Template == "passwordReset"`.
|
||||
14. On an empty `mockUserRepo`, `POST /v1/auth/password-reset` with `{"email":"nobody@example.com"}` →
|
||||
assert status `200`, body byte-for-byte identical to step 13's body, and
|
||||
`mockMailRepo.SentEmails` is empty.
|
||||
15. Build the handler with `publicURL = "https://cms.example.com"`. Seed a user "alice",
|
||||
email "alice@example.com". `POST /v1/auth/password-reset` with `{"email":"alice@example.com"}` →
|
||||
assert the recorded mail's `Data.Host` equals `"https://cms.example.com"`.
|
||||
16. Seed a user "alice", email "alice@example.com". `POST /v1/auth/password-reset` with her email →
|
||||
assert `mockUserRepo`'s stored record has `PasswordResetExpiration` within 5 seconds of
|
||||
`time.Now().Add(time.Hour)`.
|
||||
17. Seed a user "alice", email "alice@example.com". `POST /v1/auth/password-reset` with her email
|
||||
twice in a row. Assert `mockMailRepo.SentEmails` has exactly two entries and their
|
||||
`VerificationCode` data values differ. Assert the repo's stored `PasswordResetCode` no longer
|
||||
matches the first email's raw code via `stubHasher{}.Compare` (expect an error) but does match
|
||||
the second email's raw code (expect `nil`).
|
||||
18. Seed a user "alice", email "alice@example.com". Set `mailRepo.fail.failNext("Send")`.
|
||||
`POST /v1/auth/password-reset` with her email → assert status `200`, body identical to step 13's
|
||||
body, and no `Set-Cookie: jwt=...` header present.
|
||||
19. Seed a user "bob", email "bob@example.com", `IsVerified: false`. `POST /v1/auth/password-reset`
|
||||
with his email → assert status `200`, body identical to step 13's body, and exactly one email was
|
||||
sent to "bob@example.com" with template "passwordReset".
|
||||
20. Table-driven malformed-request cases against a `mockUserRepo` seeded with "alice" so a leak would
|
||||
be visible if it occurred: `{}` (missing email) and `{"email":"not-an-email"}` → assert status
|
||||
`400`. A non-JSON body `not-json` → assert status `400`. For all three, assert no email was sent.
|
||||
|
||||
## Manual verification (not exercised by the Go test)
|
||||
|
||||
- The real SMTP send and the rendered `passwordReset.html` link are never exercised automatically;
|
||||
verify by hand against a real or sandboxed SMTP server that the link matches `PUBLIC_URL` from the
|
||||
deployed `.env` and that visiting `/register/reset/validate?c=...&email=...` reaches the SPA.
|
||||
- This slice does not add the confirmation endpoint that consumes the code, nor any SPA page/route
|
||||
for it; both are out of scope and unverified here.
|
||||
- The response-time difference between an existing and a non-existing email (hashing, storing and
|
||||
emailing only happen when a user is found) is a known, unaddressed timing side channel; this slice
|
||||
only equalizes the HTTP status and body, not response latency, and no automated timing assertion is
|
||||
included since the suite has no precedent for timing-sensitive tests.
|
||||
@@ -0,0 +1,183 @@
|
||||
Feature: A user sets a new password with their email and reset code, and the code cannot be replayed
|
||||
|
||||
As the maintainer of RenewCMS
|
||||
I want POST /v1/auth/password-reset/confirm to accept the email, reset code and new password
|
||||
from the link RequestPasswordReset already emailed, set the new password only when the code
|
||||
matches and has not expired, clear the stored code the moment it is used so it can never be
|
||||
replayed, and answer an unknown email exactly like a wrong or expired code
|
||||
So that a forgotten password can be reset without exposing which emails are registered, and a
|
||||
reset code is single-use by construction rather than by convention
|
||||
|
||||
Background:
|
||||
Given RequestPasswordResetUseCase (features/request_password_reset.feature) already stores a
|
||||
bcrypt hash of a random reset code on the user, together with PasswordResetExpiration set to
|
||||
one hour after the request, and emails the raw code and the user's email in a link to
|
||||
"{PublicURL}/register/reset/validate?c={code}&email={email}"
|
||||
And this slice adds POST /v1/auth/password-reset/confirm, mounted alongside /register, /login,
|
||||
/logout and /password-reset in NewAuthRouter, public and outside the jwt-protected group,
|
||||
since a user resetting a forgotten password cannot be expected to present a jwt
|
||||
And the request body is decoded into a ResetPasswordCredentials struct with three fields —
|
||||
Email (validated required,email), Code (validated required) and Password (validated
|
||||
required,min=8, the same length rule RegisterCredentials already applies to its own Password
|
||||
field) — by validator.Validate.Struct before anything else
|
||||
And a decode failure or a validation failure is answered exactly like every other auth route:
|
||||
apperr.BodyErrorMessage with 400 for a decode failure, or the validator's error text with 400
|
||||
for a validation failure; these are shape checks on the request itself, not the
|
||||
"does this account or code exist" question this slice otherwise hides
|
||||
And this slice adds a new use case, ResetPasswordUseCase
|
||||
(internal/infrastructure/useCases/ResetPassword.go), injected into auth.Handler as
|
||||
ResetPasswordUseCase; its constructor takes only user.Repository and hash.Hasher, the same
|
||||
shape as VerifyUserUseCase's constructor
|
||||
And ResetPassword(email, code, newPassword string) error looks the user up with
|
||||
user.Repository.GetByEmail; when no user has that email, or GetByEmail returns any other
|
||||
error, it refuses with the new sentinel user.ErrInvalidPasswordResetCode instead of letting
|
||||
the lookup failure itself distinguish the case, so an unknown email produces exactly the same
|
||||
outcome as a wrong code
|
||||
And when a user is found, ResetPassword refuses with the same user.ErrInvalidPasswordResetCode
|
||||
when the stored PasswordResetCode is empty (never requested, or already used) or when
|
||||
hash.Hasher.Compare finds the submitted code does not match the stored hash — the same
|
||||
empty-string-never-matches rule VerifyUserUseCase already applies to VerificationCode, so a
|
||||
cleared code short-circuits before any hash comparison happens, and is never compared against
|
||||
a hash of the empty string
|
||||
And ResetPassword refuses with the new sentinel user.ErrPasswordResetCodeExpired when
|
||||
time.Now() is after the stored PasswordResetExpiration; the repository is not touched in
|
||||
either refusal case
|
||||
And otherwise ResetPassword hashes newPassword with hash.Hasher.Hash and calls a new
|
||||
user.Repository method, ResetPassword(userId uint32, hashedPassword string) (user.User,
|
||||
error), that in one write sets Password to the hash, PasswordResetCode to "" (stored as SQL
|
||||
NULL, the same convention VerificationCode already uses) and PasswordResetExpiration to the
|
||||
zero time.Time — the same "clear together in one write" pattern ClearVerificationCode already
|
||||
uses for IsVerified and VerificationCode
|
||||
And UpdateUserUseCase.UpdatePasswordResetCode, the older generic path that hashes whatever code
|
||||
it is given, no longer hashes an empty code: given "" it passes "" straight through to
|
||||
user.Repository.UpdatePasswordResetCode (stored as SQL NULL) without calling hash.Hasher.Hash,
|
||||
so no code path can ever store a hash of the empty string as a reset code
|
||||
And because the code is cleared rather than replaced with a hash of an empty string, and an
|
||||
empty stored code never matches any submitted code, the same code can never be used a second
|
||||
time even if an attacker knows it; requesting a new reset (RequestPasswordReset) is the only
|
||||
way to obtain a usable code afterward
|
||||
And auth.Handler.ResetPassword maps both user.ErrInvalidPasswordResetCode and
|
||||
user.ErrPasswordResetCodeExpired to the exact same response — 403 with a new
|
||||
apperr.PasswordResetFailedErrorMessage constant — so a client can never tell an unknown email
|
||||
apart from a wrong or expired code; any other error is answered 500 "Internal server error"
|
||||
And on success the handler responds 200 with
|
||||
{"message": "Your password has been reset. You can now log in."}; it never sets or clears the
|
||||
jwt cookie and never returns the user record, so a successful reset does not log the caller in
|
||||
And this slice does not touch RequestPasswordReset, CreateUser or any SPA page; the SPA route
|
||||
behind "/register/reset/validate?c=...&email=..." calling this endpoint remains a manual/SPA
|
||||
concern
|
||||
|
||||
Scenario: A correct, unexpired code sets the new password and cannot be reused
|
||||
Given the user repository contains a user "alice" with email "alice@example.com", whose stored
|
||||
PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is
|
||||
one hour from now
|
||||
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 response body is {"message": "Your password has been reset. You can now log in."}
|
||||
And no jwt cookie is set
|
||||
And the user repository's record for alice now has a Password hash that is no longer the old
|
||||
one, an empty PasswordResetCode, and a zero PasswordResetExpiration
|
||||
When the client sends POST /v1/auth/password-reset/confirm again with the same body
|
||||
Then the response status is 403
|
||||
And the response body is the new apperr.PasswordResetFailedErrorMessage text
|
||||
And the user repository's record for alice still has the password set by the first request
|
||||
|
||||
Scenario: A wrong code is refused with the generic message and nothing changes
|
||||
Given the user repository contains a user "alice" with email "alice@example.com", whose stored
|
||||
PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is
|
||||
one hour from now
|
||||
When the client sends POST /v1/auth/password-reset/confirm with body
|
||||
{"email": "alice@example.com", "code": "wrong-code", "password": "newpassword1"}
|
||||
Then the response status is 403
|
||||
And the response body is the new apperr.PasswordResetFailedErrorMessage text
|
||||
And the user repository's record for alice is unchanged: the same stored PasswordResetCode and
|
||||
the same Password hash as before
|
||||
|
||||
Scenario: An expired code is refused with the same generic message and nothing changes
|
||||
Given the user repository contains a user "alice" with email "alice@example.com", whose stored
|
||||
PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is
|
||||
one hour in the past
|
||||
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 403
|
||||
And the response body is the new apperr.PasswordResetFailedErrorMessage text
|
||||
And the user repository's record for alice is unchanged
|
||||
|
||||
Scenario: An unknown email is refused with the exact same status and body as a wrong code
|
||||
Given the user repository is empty
|
||||
When the client sends POST /v1/auth/password-reset/confirm with body
|
||||
{"email": "nobody@example.com", "code": "any-code", "password": "newpassword1"}
|
||||
Then the response status is 403
|
||||
And the response body is the new apperr.PasswordResetFailedErrorMessage text, byte-for-byte
|
||||
identical to the body a wrong code produces for a known email
|
||||
|
||||
Scenario: A user who never requested a reset gets the same refusal, since the stored code is empty
|
||||
Given the user repository contains a user "alice" with email "alice@example.com" and an empty
|
||||
PasswordResetCode
|
||||
When the client sends POST /v1/auth/password-reset/confirm with body
|
||||
{"email": "alice@example.com", "code": "anything", "password": "newpassword1"}
|
||||
Then the response status is 403
|
||||
And the response body is the new apperr.PasswordResetFailedErrorMessage text
|
||||
|
||||
Scenario Outline: A malformed or too-short request is rejected before any lookup happens
|
||||
When the client sends POST /v1/auth/password-reset/confirm with body <body>
|
||||
Then the response status is 400
|
||||
|
||||
Examples:
|
||||
| body |
|
||||
| {} |
|
||||
| {"email": "not-an-email", "code": "c", "password": "newpassword1"} |
|
||||
| {"email": "alice@example.com", "code": "", "password": "newpassword1"} |
|
||||
| {"email": "alice@example.com", "code": "reset-code-1", "password": "short"} |
|
||||
| not-json |
|
||||
|
||||
Scenario: The use case itself sets the password and clears the code in one write, independently of any handler
|
||||
Given a user.Repository fake containing user id 3, email "alice@example.com", 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 ResetPassword method was invoked exactly once with userId 3 and a
|
||||
hash of "newpassword1"
|
||||
|
||||
Scenario: The use case rejects a wrong code without touching the repository's ResetPassword method
|
||||
Given a user.Repository fake containing user id 3, email "alice@example.com", 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", "wrong-code" and "newpassword1"
|
||||
Then the call returns user.ErrInvalidPasswordResetCode, asserted with errors.Is
|
||||
And the fake repository's ResetPassword method was never invoked
|
||||
|
||||
Scenario: The use case rejects an expired code without touching the repository's ResetPassword method
|
||||
Given a user.Repository fake containing user id 3, email "alice@example.com", whose
|
||||
PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is
|
||||
one hour in the past
|
||||
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 user.ErrPasswordResetCodeExpired, asserted with errors.Is
|
||||
And the fake repository's ResetPassword method was never invoked
|
||||
|
||||
Scenario: The use case treats an unknown email the same as a wrong code
|
||||
Given an empty user.Repository fake
|
||||
And a ResetPasswordUseCase built directly over that fake and a hasher
|
||||
When ResetPassword is called with "nobody@example.com", "any-code" and "newpassword1"
|
||||
Then the call returns user.ErrInvalidPasswordResetCode, asserted with errors.Is
|
||||
And the fake repository's ResetPassword method was never invoked
|
||||
|
||||
Scenario: The use case treats an empty stored code the same as a wrong code, never calling Compare with an empty hash
|
||||
Given a user.Repository fake containing user id 3, email "alice@example.com", whose
|
||||
PasswordResetCode is empty
|
||||
And a ResetPasswordUseCase built directly over that fake and a hasher
|
||||
When ResetPassword is called with "alice@example.com", "anything" and "newpassword1"
|
||||
Then the call returns user.ErrInvalidPasswordResetCode, asserted with errors.Is
|
||||
And the fake repository's ResetPassword method was never invoked
|
||||
|
||||
Scenario: The generic reset-code update stores an empty code as empty instead of hashing it
|
||||
Given a user.Repository fake containing user id 3, email "alice@example.com"
|
||||
And an UpdateUserUseCase built directly over that fake and a hasher
|
||||
When UpdatePasswordResetCode is called with userId 3, code "" and the zero time.Time
|
||||
Then the hasher's Hash method was never invoked
|
||||
And the fake repository's stored PasswordResetCode for id 3 is ""
|
||||
@@ -0,0 +1,108 @@
|
||||
# QA procedure: reset_password
|
||||
|
||||
Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`, `stubHasher`).
|
||||
|
||||
## Test infrastructure additions needed first
|
||||
|
||||
1. Add a `ResetPassword(userId uint32, hashedPassword string) (user.User, error)` method to
|
||||
`mockUserRepo`, mirroring the `ClearVerificationCode`/`UpdatePasswordResetCode` pattern: honor
|
||||
`m.fail.trigger("ResetPassword")`, look the user up (return `user.ErrUserNotFound` if absent),
|
||||
set `Password = hashedPassword`, `PasswordResetCode = ""` and `PasswordResetExpiration =
|
||||
time.Time{}`, store it back, return it. Track a call counter and the last `(userId,
|
||||
hashedPassword)` pair (e.g. `resetPasswordCalls`, `lastResetPasswordUserId`,
|
||||
`lastResetPasswordHash`), the same shape as the existing `clearVerificationCodeCalls` fields, so
|
||||
scenarios can assert it was or wasn't invoked and with what arguments.
|
||||
2. `stubHasher.Compare(hashed, value)` returns `nil` only when `hashed == "hashed:"+value` and an
|
||||
error otherwise, and `stubHasher.Hash(value)` returns `"hashed:"+value`, so seed
|
||||
`PasswordResetCode` in fixtures as `"hashed:reset-code-1"` to represent "the bcrypt hash of
|
||||
reset-code-1", and submit `"reset-code-1"` as the matching code.
|
||||
|
||||
## Setup shared by every case
|
||||
|
||||
3. Call `validator.InitValidator()` once.
|
||||
4. Use-case-level checks build `useCases.NewResetPasswordUseCase(repo, &stubHasher{})` where `repo`
|
||||
is a `mockUserRepo` pre-populated directly via `repo.users[id] = user.User{...}`.
|
||||
5. HTTP-level checks wire `mockUserRepo`, `useCases.NewResetPasswordUseCase(repo, &stubHasher{})`
|
||||
and the other existing use cases into `auth.NewHandler(...)` (extended with the new
|
||||
`ResetPasswordUseCase` parameter) with a test `jwtauth.New("HS256", []byte("test-secret"), nil)`.
|
||||
Mount `POST /v1/auth/password-reset/confirm` unprotected on a fresh `chi.NewRouter()` mirroring
|
||||
the public group `NewAuthRouter` already uses for `/register`, `/login`, `/logout` and
|
||||
`/password-reset`. No live server, no real SMTP.
|
||||
6. The expected response body for every "generic refusal" assertion is the exact plain-text value
|
||||
of `apperr.PasswordResetFailedErrorMessage`, compared byte-for-byte via `http.Error`'s output
|
||||
(trailing newline included, the same way `test/account_verification_http_test.go` compares
|
||||
`apperr.VerificationFailedErrorMessage`).
|
||||
|
||||
## Use-case-level checks (no HTTP)
|
||||
|
||||
7. Repo has user id 3, email "alice@example.com", `PasswordResetCode: "hashed:reset-code-1"`,
|
||||
`PasswordResetExpiration: time.Now().Add(time.Hour)`. Call `ResetPassword("alice@example.com",
|
||||
"reset-code-1", "newpassword1")`. Assert the return value is `nil`. Assert the fake's
|
||||
`ResetPassword` method was invoked exactly once with `userId == 3` and a hash equal to
|
||||
`"hashed:newpassword1"`.
|
||||
8. Same seed. Call `ResetPassword("alice@example.com", "wrong-code", "newpassword1")`. Assert
|
||||
`errors.Is(err, user.ErrInvalidPasswordResetCode)`. Assert the fake's `ResetPassword` method was
|
||||
never invoked.
|
||||
9. Same seed but `PasswordResetExpiration: time.Now().Add(-time.Hour)`. Call
|
||||
`ResetPassword("alice@example.com", "reset-code-1", "newpassword1")`. Assert
|
||||
`errors.Is(err, user.ErrPasswordResetCodeExpired)`. Assert the fake's `ResetPassword` method was
|
||||
never invoked.
|
||||
10. On an empty repo, call `ResetPassword("nobody@example.com", "any-code", "newpassword1")`.
|
||||
Assert `errors.Is(err, user.ErrInvalidPasswordResetCode)`. Assert the fake's `ResetPassword`
|
||||
method was never invoked.
|
||||
11. Repo has user id 3, email "alice@example.com", `PasswordResetCode: ""`. Call
|
||||
`ResetPassword("alice@example.com", "anything", "newpassword1")`. Assert
|
||||
`errors.Is(err, user.ErrInvalidPasswordResetCode)`. Assert the fake's `ResetPassword` method
|
||||
was never invoked.
|
||||
11b. Repo has user id 3, email "alice@example.com". Build `UpdateUserUseCase` over it with a
|
||||
hasher that counts `Hash` calls. Call `UpdatePasswordResetCode(3, "", time.Time{})`. Assert
|
||||
`Hash` was never invoked and the stored `PasswordResetCode` for id 3 is `""`.
|
||||
|
||||
## HTTP-level checks (fakes, per the shared setup above)
|
||||
|
||||
12. Seed `mockUserRepo` with user "alice", email "alice@example.com",
|
||||
`PasswordResetCode: "hashed:reset-code-1"`, `PasswordResetExpiration:
|
||||
time.Now().Add(time.Hour)`. `POST /v1/auth/password-reset/confirm` with
|
||||
`{"email":"alice@example.com","code":"reset-code-1","password":"newpassword1"}` → assert
|
||||
status `200`, body exactly `{"message":"Your password has been reset. You can now log in."}`,
|
||||
and no `Set-Cookie: jwt=...` header present. Then assert the repo's record for alice has
|
||||
`Password == "hashed:newpassword1"`, `PasswordResetCode == ""` and `PasswordResetExpiration`
|
||||
equal to `time.Time{}`.
|
||||
13. Immediately repeat step 12's exact request on the same router/repo (the code was already
|
||||
cleared) → assert status `403`, body equal to `apperr.PasswordResetFailedErrorMessage`, and the
|
||||
repo's record for alice still has `Password == "hashed:newpassword1"` (the value set by the
|
||||
first request, unchanged by the second).
|
||||
14. Seed a fresh `mockUserRepo`/router with user "alice" as in step 12. `POST
|
||||
/v1/auth/password-reset/confirm` with `{"email":"alice@example.com","code":"wrong-code","password":"newpassword1"}`
|
||||
→ assert status `403`, body equal to `apperr.PasswordResetFailedErrorMessage`, and the repo's
|
||||
record for alice is unchanged (`PasswordResetCode` still `"hashed:reset-code-1"`, `Password`
|
||||
still its original seeded value).
|
||||
15. Seed with `PasswordResetExpiration: time.Now().Add(-time.Hour)`. `POST
|
||||
/v1/auth/password-reset/confirm` with the correct code → assert status `403`, body equal to
|
||||
`apperr.PasswordResetFailedErrorMessage`, and the record is unchanged.
|
||||
16. On an empty `mockUserRepo`, `POST /v1/auth/password-reset/confirm` with
|
||||
`{"email":"nobody@example.com","code":"any-code","password":"newpassword1"}` → assert status
|
||||
`403` and body byte-for-byte identical to step 14's body.
|
||||
17. Seed a user "alice" with `PasswordResetCode: ""`. `POST /v1/auth/password-reset/confirm` with
|
||||
`{"email":"alice@example.com","code":"anything","password":"newpassword1"}` → assert status
|
||||
`403`, body equal to `apperr.PasswordResetFailedErrorMessage`.
|
||||
18. Table-driven over the five malformed/too-short bodies from the feature's Examples table, each
|
||||
against a `mockUserRepo` seeded with "alice" (`PasswordResetCode: "hashed:reset-code-1"`,
|
||||
unexpired) so a leak would be visible if it occurred: `{}`, `{"email":"not-an-email",...}`,
|
||||
`{"email":"alice@example.com","code":"",...}`, a `password` shorter than 8 characters, and the
|
||||
non-JSON body `not-json` → assert status `400` for all five, and assert the repo's record for
|
||||
alice is unchanged in every case (in particular `ResetPassword`'s call counter stays at 0).
|
||||
|
||||
## Manual verification (not exercised by the Go test)
|
||||
|
||||
- The SPA page behind `/register/reset/validate?c=...&email=...` actually reading the query
|
||||
parameters, prompting for a new password, and calling
|
||||
`POST /v1/auth/password-reset/confirm` is out of scope for this slice's automated test and
|
||||
should be checked by hand against a running dev build.
|
||||
- The response-time difference between an unknown email (short-circuits at `GetByEmail`) and a
|
||||
known email with a wrong code (also compares a hash) is a known, unaddressed timing side
|
||||
channel; this slice only equalizes the HTTP status and body, not response latency, matching the
|
||||
same caveat already recorded in `features/request_password_reset.qa.md`.
|
||||
- This slice does not log the user in on a successful reset; verify by hand, if desired, that the
|
||||
SPA redirects to the login page rather than assuming an authenticated session after a successful
|
||||
reset.
|
||||
@@ -0,0 +1,165 @@
|
||||
Feature: Protected article routes and the offline-inclusive listing require a verified account
|
||||
|
||||
As the maintainer of RenewCMS
|
||||
I want every protected article route, and the "see offline articles too" behavior of
|
||||
GET /v1/article/, to require an account that has completed email verification, not merely
|
||||
a valid jwt session
|
||||
So that a user who registered but never confirmed their email cannot create, read, edit,
|
||||
publish, unpublish, delete or attach images to articles, and cannot see unpublished drafts
|
||||
through the listing endpoint, while a verified user's existing workflow is unchanged
|
||||
|
||||
Background:
|
||||
Given before this slice, the protected group inside NewArticleRouter (api/router.go) applies
|
||||
only jwtauth.Verifier then jwtauth.Authenticator to GET /v1/article/{id}, POST /v1/article/,
|
||||
DELETE /v1/article/{id}, PATCH /v1/article/{id}, PATCH /v1/article/{id}/publish,
|
||||
PATCH /v1/article/{id}/unpublish and POST /v1/article/{id}/image, so any caller with a
|
||||
validly signed, unexpired jwt cookie reaches the handler regardless of that account's
|
||||
is_verified status
|
||||
And this slice adds a third middleware, api/middleware.RequireVerifiedUser(getUser
|
||||
*useCases.GetUserUseCase), mounted after jwtauth.Authenticator in that same group, so the
|
||||
order becomes Verifier, Authenticator, RequireVerifiedUser for all seven routes above
|
||||
And RequireVerifiedUser reads user_id from the request's jwt claims using a new exported
|
||||
helper, api/pkg/claims.UserId(r *http.Request) (uint32, bool), which replaces the
|
||||
identical private userIdFromClaims method api/handlers/auth.Handler.Verify and
|
||||
CancelRegistration already used (features/account_verification.feature,
|
||||
features/cancel_pending_registration.feature); those two handlers are updated to call the
|
||||
shared helper, and their observable 401 behavior on a missing or non-numeric user_id claim
|
||||
is unchanged
|
||||
And when claims.UserId reports no usable user_id, RequireVerifiedUser answers 401 with
|
||||
apperr.UnauthorizedErrorMessage without calling GetUserUseCase, the same body and status the
|
||||
protected auth routes already use for the same condition
|
||||
And otherwise RequireVerifiedUser calls GetUserUseCase.GetUser(userId); when that returns
|
||||
user.ErrUserNotFound (a token minted for an account since deleted), it answers 401 with
|
||||
apperr.UnauthorizedErrorMessage; when it returns any other error, it answers 500 with body
|
||||
"Internal server error"; when it returns a user whose IsVerified is false, it answers 403
|
||||
with a new apperr.VerificationRequiredErrorMessage constant, "You must verify your account
|
||||
to access this resource."; when IsVerified is true, it calls next.ServeHTTP unchanged, so
|
||||
the wrapped handler runs exactly as it did before this slice
|
||||
And this slice deletes the dead api/handlers/auth.Handler.IsVerified method: it was never
|
||||
wired to any route, and it performed the bare token.PrivateClaims()["user_id"].(float64)
|
||||
assertion this slice avoids everywhere else; RequireVerifiedUser is the behavior it was
|
||||
meant to provide, so it is removed rather than kept unused
|
||||
And this slice also changes GET /v1/article/, which sits under jwtauth.Verifier only (no
|
||||
Authenticator, per features/public_article_listing.feature) and therefore is never rejected
|
||||
outright: article.Handler gains a GetUserUseCase field, wired from container.GetUserUseCase
|
||||
in api/router.go's NewArticleRouter, and Handler.ListArticles now calls ListAllArticles only
|
||||
when the request carries a resolvable user_id (via the same claims.UserId helper) whose
|
||||
GetUserUseCase.GetUser lookup succeeds and returns IsVerified true; any other case (no jwt,
|
||||
malformed jwt, unresolvable user_id, deleted account, or an account that exists but is not
|
||||
yet verified) falls back to ListOnlineArticles, the same fallback
|
||||
features/public_article_listing.feature already documents for a malformed cookie; this
|
||||
listing route never answers 401 or 403 for an unverified caller, it just narrows the result
|
||||
And a repository failure while ListArticles resolves the caller (GetUserUseCase.GetUser
|
||||
returning an error other than user.ErrUserNotFound) is treated the same as "not verified"
|
||||
for the purpose of choosing which list to show, so the listing still answers 200 with the
|
||||
online-only array rather than failing the request; only a failure inside ListOnlineArticles
|
||||
or ListAllArticles itself still answers 500, unchanged from features/public_article_listing.feature
|
||||
|
||||
Scenario Outline: An authenticated but unverified user is refused on every protected article route
|
||||
Given a user repository containing user id 1, is_verified false
|
||||
And a valid jwt cookie for user id 1
|
||||
And the article repository contains an article with id 1 titled "Backyard Renovation Update"
|
||||
When the client sends <method> <path> with that cookie
|
||||
Then the response status is 403
|
||||
And the response body is the apperr.VerificationRequiredErrorMessage text
|
||||
|
||||
Examples:
|
||||
| method | path |
|
||||
| GET | /v1/article/1 |
|
||||
| POST | /v1/article/ |
|
||||
| DELETE | /v1/article/1 |
|
||||
| PATCH | /v1/article/1 |
|
||||
| PATCH | /v1/article/1/publish |
|
||||
| PATCH | /v1/article/1/unpublish |
|
||||
|
||||
Scenario: An unverified user is also refused when attaching an image
|
||||
Given a user repository containing user id 1, is_verified false
|
||||
And a valid jwt cookie for user id 1
|
||||
And the article repository contains an article with id 1 titled "Backyard Renovation Update"
|
||||
When the client sends POST /v1/article/1/image with that cookie and a multipart form field
|
||||
"file" holding a file named "before.png" with content type "image/png"
|
||||
Then the response status is 403
|
||||
And the response body is the apperr.VerificationRequiredErrorMessage text
|
||||
And the image repository's Create is never called
|
||||
|
||||
Scenario: A verified user still exercises the protected routes exactly as before
|
||||
Given a user repository containing user id 1, is_verified true
|
||||
And a valid jwt cookie for user id 1
|
||||
When the client sends POST /v1/article/ with that cookie and body
|
||||
{"title": "Valid Title", "body": "Some content"}
|
||||
Then the response status is 201
|
||||
And the response body is the created article as JSON
|
||||
|
||||
Scenario Outline: A token that resolves to no usable account still answers 401, never 403
|
||||
Given a user repository containing user id 1, is_verified false
|
||||
And <token description>
|
||||
And the article repository contains an article with id 1 titled "Backyard Renovation Update"
|
||||
When the client sends POST /v1/article/ with that cookie and body
|
||||
{"title": "Valid Title", "body": "Some content"}
|
||||
Then the response status is 401
|
||||
And the response body is the apperr.UnauthorizedErrorMessage text
|
||||
|
||||
Examples:
|
||||
| token description |
|
||||
| a validly signed jwt cookie whose claims carry no "user_id" key at all |
|
||||
| a validly signed jwt cookie whose "user_id" claim is the string "one" |
|
||||
| a validly signed jwt cookie whose "user_id" claim is the number 999, which does not match any user |
|
||||
|
||||
Scenario Outline: A request with no jwt cookie is still refused with 401, never 403
|
||||
Given no jwt cookie is sent
|
||||
And the article repository contains an article with id 1 titled "Backyard Renovation Update"
|
||||
When the client sends <method> <path> without a cookie
|
||||
Then the response status is 401
|
||||
|
||||
Examples:
|
||||
| method | path |
|
||||
| GET | /v1/article/1 |
|
||||
| POST | /v1/article/ |
|
||||
|
||||
Scenario: A repository failure while resolving the caller's verification status answers 500
|
||||
Given a valid jwt cookie for user id 1
|
||||
And the user repository fails on its next call to Get with an error other than
|
||||
user.ErrUserNotFound
|
||||
And the article repository contains an article with id 1 titled "Backyard Renovation Update"
|
||||
When the client sends DELETE /v1/article/1 with that cookie
|
||||
Then the response status is 500
|
||||
And the response body is the text "Internal server error"
|
||||
And the article repository's article with id 1 still exists
|
||||
|
||||
Scenario: Listing falls back to the online-only list for an authenticated but unverified caller
|
||||
Given a user repository containing user id 1, is_verified false
|
||||
And a valid jwt cookie for user id 1
|
||||
And the article repository contains an online article titled "Backyard Renovation Update" with id 1
|
||||
And the same repository contains an offline article titled "Draft: Kitchen Remodel" with id 2
|
||||
When the client sends GET /v1/article/ with that cookie
|
||||
Then the response status is 200
|
||||
And the response body is a JSON array containing exactly the article with id 1
|
||||
And the article with id 2 is absent from the response body
|
||||
|
||||
Scenario: Listing still returns every article for an authenticated and verified caller
|
||||
Given a user repository containing user id 1, is_verified true
|
||||
And a valid jwt cookie for user id 1
|
||||
And the article repository contains an online article titled "Backyard Renovation Update" with id 1
|
||||
And the same repository contains an offline article titled "Draft: Kitchen Remodel" with id 2
|
||||
When the client sends GET /v1/article/ with that cookie
|
||||
Then the response status is 200
|
||||
And the response body is a JSON array containing both the article with id 1 and the article with id 2
|
||||
|
||||
Scenario: Listing falls back to the online-only list when the caller's account no longer exists
|
||||
Given the user repository contains no user with id 999
|
||||
And a valid jwt cookie for user id 999
|
||||
And the article repository contains an online article titled "Backyard Renovation Update" with id 1
|
||||
And the same repository contains an offline article titled "Draft: Kitchen Remodel" with id 2
|
||||
When the client sends GET /v1/article/ with that cookie
|
||||
Then the response status is 200
|
||||
And the response body is a JSON array containing exactly the article with id 1
|
||||
|
||||
Scenario: Listing falls back to the online-only list rather than failing when the user lookup errors
|
||||
Given a valid jwt cookie for user id 1
|
||||
And the user repository fails on its next call to Get with an error other than
|
||||
user.ErrUserNotFound
|
||||
And the article repository contains an online article titled "Backyard Renovation Update" with id 1
|
||||
And the same repository contains an offline article titled "Draft: Kitchen Remodel" with id 2
|
||||
When the client sends GET /v1/article/ with that cookie
|
||||
Then the response status is 200
|
||||
And the response body is a JSON array containing exactly the article with id 1
|
||||
@@ -0,0 +1,120 @@
|
||||
# QA procedure: verified_account_required
|
||||
|
||||
Package: `test` (black-box), extending `test/article_test.go` (`mockArticleRepo`) and
|
||||
`test/user_test.go` (`mockUserRepo`). This slice's router mount touches both the article and auth
|
||||
routers, so the HTTP-level checks live in a new `test/verified_account_required_http_test.go`.
|
||||
|
||||
## Test infrastructure additions needed first
|
||||
|
||||
1. `mockUserRepo.Get` already honors `m.fail.trigger("Get")` and already returns
|
||||
`user.ErrUserNotFound` for an absent id (`test/user_test.go`), so no new mock methods are
|
||||
required on the user side.
|
||||
2. `mockArticleRepo` already supports seeding via `repo.articles[id] = article.Article{...}` and a
|
||||
`failNext(...)` helper (`test/article_test.go`); no changes needed there either.
|
||||
3. Build the middleware under test directly: `middleware.RequireVerifiedUser(getUser
|
||||
*useCases.GetUserUseCase) func(http.Handler) http.Handler`, exported from `api/middleware`, and
|
||||
`claims.UserId(r *http.Request) (uint32, bool)`, exported from `api/pkg/claims`. Construct
|
||||
`useCases.NewGetUserUseCase(repo)` over a `mockUserRepo`.
|
||||
|
||||
## Setup shared by every case
|
||||
|
||||
4. Call `validator.InitValidator()` once.
|
||||
5. Build a test `jwtauth.New("HS256", []byte("test-secret"), nil)`.
|
||||
6. Wire `article.NewHandler(create, get, list, delete, update, getUser)` (extended with the new
|
||||
`GetUserUseCase` parameter) over a `mockArticleRepo` and the `mockUserRepo`/`GetUserUseCase`
|
||||
from step 3.
|
||||
7. Mount the routes on a fresh `chi.NewRouter()` mirroring the new `api/router.go` shape:
|
||||
- `r.Use(jwtauth.Verifier(token))` on the whole `/article` group (parses, never rejects)
|
||||
- `r.Get("/", articleHandler.ListArticles)` directly under that Verifier-only group
|
||||
- a nested `r.Group` adding `jwtauth.Authenticator(token)` then
|
||||
`middleware.RequireVerifiedUser(getUser)`, containing `r.Get("/{id}", ...)`,
|
||||
`r.Post("/", ...)`, `r.Delete("/{id}", ...)`, `r.Patch("/{id}", ...)`,
|
||||
`r.Patch("/{id}/publish", ...)`, `r.Patch("/{id}/unpublish", ...)`,
|
||||
`r.Post("/{id}/image", ...)`
|
||||
No live server.
|
||||
8. Mint jwt cookies with `token.Encode(map[string]any{"user_id": uint32(1)})` for the valid-token
|
||||
cases, following the convention in `features/article_publishing.qa.md`.
|
||||
9. Seed `mockUserRepo` per scenario via `repo.users[1] = user.User{ID: 1, IsVerified: ...}`.
|
||||
|
||||
## HTTP checks: protected routes reject an unverified caller
|
||||
|
||||
10. Seed `mockUserRepo` with user id 1, `IsVerified: false`; seed `mockArticleRepo` with article id
|
||||
1. Table-driven over `GET /v1/article/1`, `POST /v1/article/` (body
|
||||
`{"title":"Valid Title","body":"Some content"}`), `DELETE /v1/article/1`,
|
||||
`PATCH /v1/article/1` (body `{"body":"Updated"}`), `PATCH /v1/article/1/publish`,
|
||||
`PATCH /v1/article/1/unpublish`, each sent with the valid cookie from step 8 → assert status
|
||||
`403` and body exactly `apperr.VerificationRequiredErrorMessage`. Assert the article repository
|
||||
was not mutated by any of the write calls.
|
||||
11. Same seed, `POST /v1/article/1/image` with the valid cookie and a multipart form field `"file"`
|
||||
holding `before.png` / `image/png` → assert status `403`, body
|
||||
`apperr.VerificationRequiredErrorMessage`, and the image repository's `Create` is never called
|
||||
(mirroring the assertion style in `test/upload_article_image_test.go`).
|
||||
|
||||
## HTTP checks: a verified caller is unaffected
|
||||
|
||||
12. Seed `mockUserRepo` with user id 1, `IsVerified: true`. `POST /v1/article/` with the valid
|
||||
cookie and body `{"title":"Valid Title","body":"Some content"}` → assert status `201` and the
|
||||
response body decodes to the created `article.Article`.
|
||||
|
||||
## HTTP checks: an unresolvable caller still answers 401, not 403
|
||||
|
||||
13. Table-driven over three token shapes, each against a fresh seed of user id 1,
|
||||
`IsVerified: false`, sent as `POST /v1/article/` with body
|
||||
`{"title":"Valid Title","body":"Some content"}`, asserting status `401` and body exactly
|
||||
`apperr.UnauthorizedErrorMessage`:
|
||||
- mint the jwt with `token.Encode(map[string]any{"role": "user"})` (no `user_id` key at all);
|
||||
- mint the jwt with `token.Encode(map[string]any{"user_id": "one"})` (string, not a number);
|
||||
- mint the jwt with `token.Encode(map[string]any{"user_id": uint32(999)})` (well-formed but no
|
||||
such user in the repo).
|
||||
|
||||
## HTTP checks: no session at all is still 401
|
||||
|
||||
14. `GET /v1/article/1` and `POST /v1/article/` with no `jwt` cookie → assert status `401` for
|
||||
both (produced by `jwtauth.Authenticator` itself, before `RequireVerifiedUser` runs); do not
|
||||
assert an exact body.
|
||||
|
||||
## HTTP checks: a user-repository failure while resolving the caller
|
||||
|
||||
15. Mint a valid jwt cookie for user id 1. Configure `mockUserRepo`'s `failSwitch` so the next call
|
||||
to `Get` returns a generic error (not `user.ErrUserNotFound`), via the existing
|
||||
`failNext("Get")`-style helper. `DELETE /v1/article/1` with that cookie, article repository
|
||||
seeded with article id 1 → assert status `500`, body exactly `"Internal server error"`, and
|
||||
that the article repository's article with id 1 still exists (the delete use case was never
|
||||
reached).
|
||||
|
||||
## HTTP checks: the offline-inclusive listing is gated by verification
|
||||
|
||||
16. Seed `mockUserRepo` with user id 1, `IsVerified: false`; seed `mockArticleRepo` with an online
|
||||
article id 1 and an offline article id 2. `GET /v1/article/` with the valid cookie from step 8
|
||||
→ assert status `200`; decode the JSON array and assert it has exactly one element, id `1`.
|
||||
17. Same article seed, `mockUserRepo` user id 1 with `IsVerified: true` → `GET /v1/article/` with
|
||||
the same cookie → assert status `200`; decode the JSON array and assert both ids `1` and `2`
|
||||
are present.
|
||||
18. Same article seed, no user with id 999 in `mockUserRepo`, jwt cookie minted for user id 999 →
|
||||
`GET /v1/article/` with that cookie → assert status `200`; assert exactly id `1` is present
|
||||
(fallback to online-only on `user.ErrUserNotFound`).
|
||||
19. Same article seed, valid cookie for user id 1, `mockUserRepo`'s `failSwitch` set so the next
|
||||
`Get` call returns a generic error → `GET /v1/article/` with that cookie → assert status `200`;
|
||||
assert exactly id `1` is present (a user-lookup failure falls back to online-only rather than
|
||||
failing the request).
|
||||
|
||||
## Regression check on existing fixtures
|
||||
|
||||
20. `features/article_publishing.feature`, `features/edit_article_body.feature`,
|
||||
`features/upload_article_image.feature` and the corresponding `test/*_test.go` files mint a
|
||||
valid jwt cookie for "a user with id 1" without asserting anything about that user's
|
||||
`IsVerified` status. After this slice, those HTTP tests' `mockUserRepo` fixture must seed user
|
||||
id 1 with `IsVerified: true` (or the equivalent id used in each file), or every one of those
|
||||
protected-route assertions will start failing with `403` instead of the expected success/404/400
|
||||
status. This is a required update to those test files' setup, not a new behavior; call it out
|
||||
explicitly when touching them so the regression is caught at review time rather than by a
|
||||
surprise `task test` failure.
|
||||
|
||||
## Manual verification (not exercised by the Go test)
|
||||
|
||||
- The SPA-visible behaviour of an unverified, logged-in user landing on the back-office article
|
||||
screens (create/edit/publish/image-upload) and seeing a 403 instead of the normal editing UI,
|
||||
and the front-office/back-office listing showing only published articles to that same unverified
|
||||
session, is out of scope for this slice's automated test and should be checked by hand against a
|
||||
running dev build with an unverified test account.
|
||||
- No SMTP involvement.
|
||||
Reference in new issue
Block a user