fix: remove the unused user use cases, fix the specs that no longer match the code, include api/middleware and api/pkg/claims in mutation testing, log failures that password reset requests currently ignore, and reject user_id claims that don't fit a user id
This commit is contained in:
1 parent
cbb403011a
commit
3282b0580c
22 files changed
+260
-314
No files matched your search
@@ -14,8 +14,9 @@ Feature: Password and code hashing go through a domain Hasher interface
|
||||
existing user.ErrUserNotFound
|
||||
And internal/infrastructure/hashing.BcryptHasher implements hash.Hasher and is asserted
|
||||
with var _ hash.Hasher = &BcryptHasher{}
|
||||
And api/dependecyInjection.go constructs one BcryptHasher and passes it to
|
||||
CreateUserUseCase, UpdateUserUseCase and the new AuthenticateUserUseCase
|
||||
And api/dependecyInjection.go constructs one BcryptHasher and passes it to every use case
|
||||
that hashes or compares a secret: CreateUserUseCase, AuthenticateUserUseCase,
|
||||
VerifyUserUseCase, RequestPasswordResetUseCase and ResetPasswordUseCase
|
||||
And UserRepository.Create, UpdatePassword and UpdatePasswordResetCode store the string
|
||||
they are given verbatim and never call bcrypt
|
||||
And the auth Handler no longer imports golang.org/x/crypto/bcrypt; Login calls
|
||||
@@ -37,27 +38,6 @@ Feature: Password and code hashing go through a domain Hasher interface
|
||||
Then the use case returns that error
|
||||
And the repository's Create is never called
|
||||
|
||||
Scenario Outline: Updating a password or a password reset code stores the hashed value
|
||||
Given a fake hasher that hashes any value "x" to "hashed:x"
|
||||
When UpdateUserUseCase.<Method> is called with user id 1 and "<plaintext>"
|
||||
Then the repository's <Method> is called with "hashed:<plaintext>", not "<plaintext>"
|
||||
|
||||
Examples:
|
||||
| Method | plaintext |
|
||||
| UpdatePassword | newPassw0rd |
|
||||
| UpdatePasswordResetCode | reset-code-123 |
|
||||
|
||||
Scenario Outline: A hashing failure during an update persists nothing
|
||||
Given a fake hasher whose Hash always returns an error
|
||||
When UpdateUserUseCase.<Method> is called with user id 1 and "<plaintext>"
|
||||
Then the use case returns that error
|
||||
And the repository's <Method> is never called
|
||||
|
||||
Examples:
|
||||
| Method | plaintext |
|
||||
| UpdatePassword | newPassw0rd |
|
||||
| UpdatePasswordResetCode | reset-code-123 |
|
||||
|
||||
Scenario: Authenticating with the correct password returns the user
|
||||
Given a user repository containing a user "alice" with stored password "hashed:hunter2example"
|
||||
And a fake hasher whose Compare returns nil when given "hashed:hunter2example" and "hunter2example"
|
||||
|
||||
@@ -23,12 +23,6 @@ Package: `test` (black-box, alongside the existing `article_test.go` / `user_tes
|
||||
UUID, unchanged from current behaviour).
|
||||
4. Same use case with `fakeHasher` configured to always error on `Hash`: assert
|
||||
`CreateUser` returns that error and `fakeRepo.Create` was never called.
|
||||
5. `useCases.NewUpdateUserUseCase(fakeRepo, fakeHasher)`, call `UpdatePassword(1,
|
||||
"newPassw0rd")` and separately `UpdatePasswordResetCode(1, "reset-code-123")`: assert the
|
||||
fake repo receives `"hashed:newPassw0rd"` / `"hashed:reset-code-123"`, not the plaintext.
|
||||
6. Same two methods with `fakeHasher.Hash` erroring: assert the use case returns the error
|
||||
and the corresponding fake-repo method (`UpdatePassword` / `UpdatePasswordResetCode`) was
|
||||
never called.
|
||||
7. `useCases.NewAuthenticateUserUseCase(fakeRepo, fakeHasher)` (new use case): seed
|
||||
`fakeRepo` with a user "alice" whose stored `Password` is `"hashed:hunter2example"`.
|
||||
- `Authenticate("alice", "hunter2example")` with `fakeHasher.Compare` returning `nil` →
|
||||
|
||||
@@ -48,6 +48,11 @@ Feature: Anyone can request a password reset, and the response never reveals whe
|
||||
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 failure to hash the code, to store it, or to send the email is now also written to the
|
||||
process log via the stdlib log package (the same convention
|
||||
internal/infrastructure/mailer/mailRepository.go already uses for its own send failures);
|
||||
this is in addition to, not instead of, always returning nil to the caller — the HTTP response
|
||||
is unaffected by whether a failure occurred or was logged
|
||||
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
|
||||
@@ -104,6 +109,7 @@ Feature: Anyone can request a password reset, and the response never reveals whe
|
||||
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
|
||||
And a failure was written to the process log naming the mail-send failure
|
||||
|
||||
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
|
||||
@@ -148,3 +154,24 @@ Feature: Anyone can request a password reset, and the response never reveals whe
|
||||
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
|
||||
And a failure was written to the process log naming the mail-send failure
|
||||
|
||||
Scenario: The use case logs and swallows a hashing failure
|
||||
Given a user.Repository fake containing user id 3, email "alice@example.com"
|
||||
And a RequestPasswordResetUseCase built directly over that fake, a hasher set to fail on its
|
||||
next Hash call, and a mail.Repository fake
|
||||
When RequestPasswordReset is called with "alice@example.com"
|
||||
Then the call returns nil
|
||||
And the fake repository's UpdatePasswordResetCode method was never invoked
|
||||
And no email was sent
|
||||
And a failure was written to the process log naming the hashing failure
|
||||
|
||||
Scenario: The use case logs and swallows a repository failure while storing the reset code
|
||||
Given a user.Repository fake containing user id 3, email "alice@example.com", set to fail on
|
||||
its next UpdatePasswordResetCode call
|
||||
And a RequestPasswordResetUseCase built directly over that fake, a hasher, and a mail.Repository
|
||||
fake
|
||||
When RequestPasswordReset is called with "alice@example.com"
|
||||
Then the call returns nil
|
||||
And no email was sent
|
||||
And a failure was written to the process log naming the storage failure
|
||||
@@ -27,6 +27,17 @@ Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`, `stu
|
||||
can call `stubHasher{}.Compare(repo.users[id].PasswordResetCode, "code-1")` and expect an error,
|
||||
then `.Compare(..., secondRawCode)` and expect `nil`.
|
||||
|
||||
## Capturing logged failures
|
||||
|
||||
8a. `RequestPasswordReset` now logs a hash, store or mail-send failure via the stdlib `log`
|
||||
package instead of only swallowing it. Any scenario that asserts on this redirects the
|
||||
global logger's output for the duration of the call: `var buf bytes.Buffer`,
|
||||
`log.SetOutput(&buf)`, then `t.Cleanup(func() { log.SetOutput(os.Stderr) })` before invoking
|
||||
`RequestPasswordReset`. After the call, assert `buf.String()` is non-empty and contains a
|
||||
substring identifying which failure occurred (e.g. `"hash"`, `"UpdatePasswordResetCode"` /
|
||||
`"store"`, or `"Send"` / `"mail"`) — the exact wording is an implementation detail, only that
|
||||
a line was written and it's distinguishable per failure kind.
|
||||
|
||||
## Setup shared by every case
|
||||
|
||||
5. Call `validator.InitValidator()` once.
|
||||
@@ -61,12 +72,23 @@ Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`, `stu
|
||||
`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).
|
||||
Using the log-capture setup from step 8a, 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).
|
||||
Assert the captured log buffer is non-empty and its content identifies a mail-send failure.
|
||||
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.
|
||||
12a. Repo pre-populated with user id 3, email "alice@example.com". Build the use case with a
|
||||
`stubHasher{hashErr: errStubHashFailure}` so `Hash` fails. Using the log-capture setup from
|
||||
step 8a, call `RequestPasswordReset("alice@example.com")`. Assert the return value is `nil`,
|
||||
`UpdatePasswordResetCode`'s call counter is 0, `mailRepo.SentEmails` is empty, and the captured
|
||||
log buffer is non-empty and identifies a hashing failure.
|
||||
12b. Repo pre-populated with user id 3, email "alice@example.com". Set
|
||||
`repo.fail.failNext("UpdatePasswordResetCode")`. Using the log-capture setup from step 8a, call
|
||||
`RequestPasswordReset("alice@example.com")`. Assert the return value is `nil`,
|
||||
`mailRepo.SentEmails` is empty, and the captured log buffer is non-empty and identifies a
|
||||
storage failure.
|
||||
|
||||
## HTTP-level checks (fakes, per the shared setup above)
|
||||
|
||||
@@ -88,9 +110,10 @@ Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`, `stu
|
||||
`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.
|
||||
18. Seed a user "alice", email "alice@example.com". Set `mailRepo.fail.failNext("Send")`. Using the
|
||||
log-capture setup from step 8a, `POST /v1/auth/password-reset` with her email → assert status
|
||||
`200`, body identical to step 13's body, no `Set-Cookie: jwt=...` header present, and the
|
||||
captured log buffer is non-empty and identifies a mail-send failure.
|
||||
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".
|
||||
|
||||
@@ -161,30 +161,6 @@ Feature: Use case baseline characterization
|
||||
When ListUsers is called
|
||||
Then the result is the users with ids 1 and 2, and no error
|
||||
|
||||
Scenario Outline: UpdateUser delegates each operation to the repository and surfaces not-found for an unknown id
|
||||
Given a user repository fake containing a user with id 1
|
||||
When <method> is called with id <id> and <input>
|
||||
Then the result is <outcome>
|
||||
|
||||
Examples:
|
||||
| method | id | input | outcome |
|
||||
| UpdateVerificationStatus | 1 | isVerified true | the user with id 1 and is_verified true |
|
||||
| UpdateVerificationStatus | 9 | isVerified true | the repository's not-found error (gorm.ErrRecordNotFound) |
|
||||
| UpdatePassword | 1 | password "newpassword123" | the user with id 1 |
|
||||
| UpdatePassword | 9 | password "newpassword123" | the repository's not-found error (gorm.ErrRecordNotFound) |
|
||||
| UpdatePasswordResetCode | 1 | code "reset-code-123" | the user with id 1 |
|
||||
| UpdatePasswordResetCode | 9 | code "reset-code-123" | the repository's not-found error (gorm.ErrRecordNotFound) |
|
||||
|
||||
Scenario Outline: DeleteUser succeeds silently whether or not the id exists
|
||||
Given a user repository fake containing a user with id 1
|
||||
When DeleteUser is called with id <id>
|
||||
Then no error is returned
|
||||
|
||||
Examples:
|
||||
| id |
|
||||
| 1 |
|
||||
| 9 |
|
||||
|
||||
Scenario Outline: SendMail forwards its arguments verbatim to the mail repository
|
||||
Given a mail repository fake
|
||||
When SendMail is called with receiver "<receiver>", template "<template>" and some data
|
||||
@@ -201,8 +177,8 @@ Feature: Use case baseline characterization
|
||||
Then the use case returns the repository's error unchanged
|
||||
And the fake recorded no send
|
||||
|
||||
Scenario: Mutation testing runs clean on the use cases package
|
||||
Scenario: Mutation testing runs clean on the use cases package, api/middleware and api/pkg/claims
|
||||
Given the test/ suite passes via `go test -tags nofrontend ./test/... -count=1`
|
||||
When `gremlins unleash --integration --tags nofrontend --coverpkg=./internal/infrastructure/useCases/... --timeout-coefficient 10 --output tmp/gremlins.json ./internal/infrastructure/useCases` is run
|
||||
When `gremlins unleash --integration --tags nofrontend --coverpkg=./internal/infrastructure/useCases/...,./api/middleware/...,./api/pkg/claims/... --timeout-coefficient 10 --output tmp/gremlins.json ./internal/infrastructure/useCases ./api/middleware ./api/pkg/claims` is run
|
||||
Then the report contains zero mutants with status LIVED
|
||||
And the report contains zero mutants with status NOT COVERED
|
||||
@@ -25,6 +25,11 @@ Feature: Protected article routes and the offline-inclusive listing require a ve
|
||||
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 claims.UserId now also rejects a "user_id" claim whose JSON number is not a whole value in
|
||||
the inclusive range 0 to 4294967295 (math.MaxUint32): a negative number, a value with a
|
||||
fractional part (e.g. 7.5), or a value larger than 4294967295 all now return ok=false instead
|
||||
of being silently truncated or wrapped into a uint32 that could coincidentally match a
|
||||
different, real user id
|
||||
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
|
||||
@@ -108,6 +113,17 @@ Feature: Protected article routes and the offline-inclusive listing require a ve
|
||||
| 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 |
|
||||
| a validly signed jwt cookie whose "user_id" claim is the number -1 |
|
||||
| a validly signed jwt cookie whose "user_id" claim is the number 4294967296 (one past the uint32 max) |
|
||||
|
||||
Scenario: A fractional user_id claim is never truncated into impersonating a different real user
|
||||
Given a user repository containing user id 7, is_verified true
|
||||
And a validly signed jwt cookie whose claims are {"user_id": 7.5}
|
||||
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
|
||||
And no article was created
|
||||
|
||||
Scenario Outline: A request with no jwt cookie is still refused with 401, never 403
|
||||
Given no jwt cookie is sent
|
||||
|
||||
@@ -58,14 +58,28 @@ routers, so the HTTP-level checks live in a new `test/verified_account_required_
|
||||
|
||||
## 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,
|
||||
13. Table-driven over five 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).
|
||||
such user in the repo);
|
||||
- mint the jwt with `token.Encode(map[string]any{"user_id": -1.0})` (negative);
|
||||
- mint the jwt with `token.Encode(map[string]any{"user_id": float64(4294967296)})` (one past
|
||||
the uint32 maximum).
|
||||
13a. Seed `mockUserRepo` with user id 7, `IsVerified: true`; seed `mockArticleRepo` empty. Mint a
|
||||
jwt with `token.Encode(map[string]any{"user_id": 7.5})` (a fractional value that would
|
||||
previously have truncated to the real user id 7). `POST /v1/article/` with that cookie and body
|
||||
`{"title":"Valid Title","body":"Some content"}` → assert status `401`, body exactly
|
||||
`apperr.UnauthorizedErrorMessage`, and that the article repository gained no new article
|
||||
(proving the fractional claim was rejected rather than silently accepted as user 7).
|
||||
13b. Extend `test/claims_test.go` with a `TestClaimsUserId` set of subtests mirroring the existing
|
||||
`TestClaimsTokenVersion` pattern: a well-formed `uint32` claim returns `ok == true` with the
|
||||
matching value; a missing key, a non-numeric value (string), a negative value, a fractional
|
||||
value (e.g. `7.5`), and a value one past `math.MaxUint32` (`4294967296`) all return
|
||||
`ok == false`. This exercises `claims.UserId` directly, independent of the HTTP round-trip.
|
||||
|
||||
## HTTP checks: no session at all is still 401
|
||||
|
||||
|
||||
Reference in new issue
Block a user