refac: password and code hashing & checking go through a domain hasher interface used by the use cases instead of bcrypt in the user repository and the auth handler
This commit is contained in:
1 parent
8460a078e3
commit
ff8cd5f0ec
16 files changed
+725
-88
No files matched your search
@@ -0,0 +1,112 @@
|
||||
Feature: Password and code hashing go through a domain Hasher interface
|
||||
|
||||
As the maintainer of RenewCMS
|
||||
I want password and verification/reset-code hashing done by a domain hash.Hasher
|
||||
interface, used only by use cases
|
||||
So that internal/infrastructure/persistence/repositories/userRepository.go and
|
||||
api/handlers/auth/authHandler.go stop calling bcrypt directly, and a use case can be
|
||||
tested against a fake hasher instead of a real bcrypt cost factor
|
||||
|
||||
Background:
|
||||
Given internal/domain/hash defines the interface Hasher with Hash(value string) (string, error)
|
||||
and Compare(hashed string, value string) error
|
||||
And internal/domain/user defines the sentinel user.ErrInvalidCredentials alongside the
|
||||
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 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
|
||||
AuthenticateUserUseCase.Authenticate instead of comparing a hash itself
|
||||
|
||||
Scenario: Creating a user stores a hashed password and a hashed verification code
|
||||
Given a fake hasher that hashes any value "x" to "hashed:x"
|
||||
When CreateUserUseCase.CreateUser is called with username "alice", password
|
||||
"s3cretPassw0rd" and email "alice@example.com"
|
||||
Then the repository's Create is called with a user whose Password is "hashed:s3cretPassw0rd"
|
||||
And that user's VerificationCode is "hashed:<the generated code>", not the raw code
|
||||
And the User returned by the use case has VerificationCode equal to the raw generated
|
||||
code, not the hash
|
||||
|
||||
Scenario: A hashing failure while creating a user persists nothing
|
||||
Given a fake hasher whose Hash always returns an error
|
||||
When CreateUserUseCase.CreateUser is called with username "alice", password
|
||||
"s3cretPassw0rd" and email "alice@example.com"
|
||||
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"
|
||||
When AuthenticateUserUseCase.Authenticate is called with username "alice" and password
|
||||
"hunter2example"
|
||||
Then the result is that user, with no error
|
||||
|
||||
Scenario: Authenticating with the wrong password returns a domain sentinel error
|
||||
Given a user repository containing a user "alice" with stored password "hashed:hunter2example"
|
||||
And a fake hasher whose Compare returns an error for that password
|
||||
When AuthenticateUserUseCase.Authenticate is called with username "alice" and password
|
||||
"wrongpassword12"
|
||||
Then the result is the error user.ErrInvalidCredentials
|
||||
And errors.Is(err, user.ErrInvalidCredentials) is true
|
||||
|
||||
Scenario: Authenticating an unknown username never calls the hasher
|
||||
Given a user repository with no user "bob"
|
||||
When AuthenticateUserUseCase.Authenticate is called with username "bob" and password
|
||||
"hunter2example"
|
||||
Then the result is the error user.ErrUserNotFound
|
||||
And the hasher's Compare is never called
|
||||
|
||||
Scenario Outline: Login over HTTP still can't distinguish a bad password from an unknown username
|
||||
Given a user repository containing a user "alice" with stored password "hashed:hunter2example"
|
||||
And a real BcryptHasher
|
||||
When the client sends POST /v1/auth/login with username "<username>" and password "<password>"
|
||||
Then the response status is 403
|
||||
And the response body is the text apperr.LogsErrorMessage
|
||||
And no jwt cookie is set
|
||||
|
||||
Examples:
|
||||
| username | password | case |
|
||||
| alice | wrongpassword12 | existing user, bad pass |
|
||||
| bob | hunter2example | unknown username |
|
||||
|
||||
Scenario: Registering a user stores the hashed password, never the plaintext
|
||||
Given an empty user repository
|
||||
And a real BcryptHasher
|
||||
When the client sends POST /v1/auth/register with username "alice", password
|
||||
"s3cretPassw0rd" and email "alice@example.com"
|
||||
Then the response status is 200
|
||||
And a jwt cookie is set
|
||||
And the user repository's stored Password for "alice" is not "s3cretPassw0rd"
|
||||
And BcryptHasher.Compare("<the stored value>", "s3cretPassw0rd") returns nil
|
||||
|
||||
Scenario: A hashing failure during registration surfaces as 500 and sets no cookie
|
||||
Given an empty user repository
|
||||
And a fake hasher whose Hash always returns an error
|
||||
When the client sends POST /v1/auth/register with username "alice", password
|
||||
"s3cretPassw0rd" and email "alice@example.com"
|
||||
Then the response status is 500
|
||||
And no jwt cookie is set
|
||||
@@ -0,0 +1,69 @@
|
||||
# QA procedure: hasher_domain_interface
|
||||
|
||||
Package: `test` (black-box, alongside the existing `article_test.go` / `user_test.go` /
|
||||
`use_case_baseline_test.go` / `not_found_errors_test.go`).
|
||||
|
||||
## Fakes needed
|
||||
1. A `fakeHasher` implementing `hash.Hasher`:
|
||||
- `Hash(value string) (string, error)`: by default returns `"hashed:" + value, nil`;
|
||||
configurable to return `("", someErr)` unconditionally.
|
||||
- `Compare(hashed, value string) error`: by default returns `nil` when
|
||||
`hashed == "hashed:"+value`, else a sentinel mismatch error; configurable to always
|
||||
error, and to record whether it was called.
|
||||
2. Extend the existing `mockUserRepo` (from `test/user_test.go` /
|
||||
`not_found_errors_test.go`) with call-recording flags already used elsewhere in the
|
||||
package (`createCalled`, `updatePasswordCalled`, etc.) if not already present, so tests
|
||||
can assert a hashing failure short-circuits before the repository is touched.
|
||||
|
||||
## Use-case-level checks (fake hasher + fake repo, no HTTP)
|
||||
3. `useCases.NewCreateUserUseCase(fakeRepo, fakeHasher)`, `CreateUser(...)`: assert the
|
||||
`User` argument the fake repo's `Create` receives has `Password == "hashed:s3cretPassw0rd"`
|
||||
and a `VerificationCode` that starts with `"hashed:"`; assert the `User` returned by the
|
||||
use case has a `VerificationCode` that does **not** start with `"hashed:"` (it is the raw
|
||||
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` →
|
||||
assert the returned user matches and `err` is `nil`.
|
||||
- `Authenticate("alice", "wrongpassword12")` with `fakeHasher.Compare` returning an error
|
||||
→ assert `errors.Is(err, user.ErrInvalidCredentials)` is `true`.
|
||||
- `Authenticate("bob", "hunter2example")` with no "bob" in the repo → assert
|
||||
`errors.Is(err, user.ErrUserNotFound)` is `true` and `fakeHasher`'s `Compare` was never
|
||||
invoked (record a `compareCalled` flag).
|
||||
|
||||
## HTTP checks (real BcryptHasher, in-memory fake repo, mirroring api/router.go)
|
||||
8. Call `validator.InitValidator()`.
|
||||
9. Build `hashing.NewBcryptHasher()`, wire `useCases.NewCreateUserUseCase(fakeRepo,
|
||||
bcryptHasher)` and `useCases.NewAuthenticateUserUseCase(fakeRepo, bcryptHasher)` (plus
|
||||
the existing `GetUserUseCase` / `ListUsersUseCase` the auth handler still needs), build
|
||||
`auth.NewHandler(token, get, list, create, authenticate)` (constructor signature grows by
|
||||
the new use case) with a test `jwtauth.New("HS256", []byte("test-secret"), nil)`, mount
|
||||
its routes unprotected on a fresh `chi.NewRouter()` as in `api/router.go`.
|
||||
10. Seed the fake user repo directly with a user "alice" whose `Password` field holds a real
|
||||
`bcryptHasher.Hash("hunter2example")` result (not `bcrypt.GenerateFromPassword` called
|
||||
from the test, so the test never imports bcrypt itself).
|
||||
11. `POST /v1/auth/login` with `{"username":"alice","password":"wrongpassword12"}` → assert
|
||||
status `403`, body `apperr.LogsErrorMessage + "\n"`, no `Set-Cookie: jwt=...` header.
|
||||
12. `POST /v1/auth/login` with `{"username":"bob","password":"hunter2example"}` (no such
|
||||
user) → assert identical status/body/no-cookie outcome to step 11 (regression, same as
|
||||
`not_found_errors.qa.md` steps 13–14).
|
||||
13. `POST /v1/auth/register` with `{"username":"alice","password":"s3cretPassw0rd","email":"alice@example.com"}`
|
||||
against an empty fake repo → assert status `200`, a `Set-Cookie: jwt=...` header is
|
||||
present, and the `Password` the fake repo's `Create` received is neither empty nor equal
|
||||
to `"s3cretPassw0rd"`; assert `bcryptHasher.Compare(storedPassword, "s3cretPassw0rd")`
|
||||
returns `nil`.
|
||||
14. Rebuild the handler with a `fakeHasher` (from step 1) whose `Hash` always errors in
|
||||
place of `bcryptHasher`, same empty fake repo. `POST /v1/auth/register` with the same
|
||||
valid body → assert status `500`, no `Set-Cookie` header, and the fake repo's `Create`
|
||||
was never called.
|
||||
|
||||
## Manual verification (not exercised by the Go test)
|
||||
- None. This slice has no SPA-visible behaviour and no SMTP involvement.
|
||||
Reference in new issue
Block a user