diff --git a/Taskfile.yml b/Taskfile.yml index b917c24..9fe1085 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -57,7 +57,7 @@ tasks: desc: Gremlins mutation testing of the use cases → tmp/gremlins.json cmds: - mkdir -p tmp - - gremlins unleash --integration --tags nofrontend --coverpkg=./internal/infrastructure/useCases/... --timeout-coefficient 10 --output tmp/gremlins.json ./internal/infrastructure/useCases + - 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 lint: desc: gofmt check, go vet, eslint (--fix) and vue-tsc diff --git a/api/dependecyInjection.go b/api/dependecyInjection.go index ec5d9bb..6b8c29a 100644 --- a/api/dependecyInjection.go +++ b/api/dependecyInjection.go @@ -28,13 +28,11 @@ type UseCases struct { DeleteArticleUseCase *useCases.DeleteArticleUseCase GetUserUseCase *useCases.GetUserUseCase CreateUserUseCase *useCases.CreateUserUseCase - UpdateUserUseCase *useCases.UpdateUserUseCase AuthenticateUserUseCase *useCases.AuthenticateUserUseCase VerifyUserUseCase *useCases.VerifyUserUseCase CancelRegistrationUseCase *useCases.CancelRegistrationUseCase RequestPasswordResetUseCase *useCases.RequestPasswordResetUseCase ResetPasswordUseCase *useCases.ResetPasswordUseCase - DeleteUserUseCase *useCases.DeleteUserUseCase ListUsersUseCase *useCases.ListUsersUseCase SendMailUseCase *useCases.SendMailUseCase CreateImageUseCase *useCases.CreateImageUseCase @@ -85,13 +83,11 @@ func getUseCases(repos *Repositories, settings config.Settings) *UseCases { DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(repos.ArticleRepo, repos.ImageRepo), GetUserUseCase: useCases.NewGetUserUseCase(repos.UserRepo), CreateUserUseCase: useCases.NewCreateUserUseCase(repos.UserRepo, hasher, repos.MailRepo, settings.PublicURL), - UpdateUserUseCase: useCases.NewUpdateUserUseCase(repos.UserRepo, hasher), AuthenticateUserUseCase: useCases.NewAuthenticateUserUseCase(repos.UserRepo, hasher), VerifyUserUseCase: useCases.NewVerifyUserUseCase(repos.UserRepo, hasher), CancelRegistrationUseCase: useCases.NewCancelRegistrationUseCase(repos.UserRepo), RequestPasswordResetUseCase: useCases.NewRequestPasswordResetUseCase(repos.UserRepo, hasher, repos.MailRepo, settings.PublicURL), ResetPasswordUseCase: useCases.NewResetPasswordUseCase(repos.UserRepo, hasher), - DeleteUserUseCase: useCases.NewDeleteUserUseCase(repos.UserRepo), ListUsersUseCase: useCases.NewListUsersUseCase(repos.UserRepo), SendMailUseCase: useCases.NewSendMailUseCase(repos.MailRepo), CreateImageUseCase: useCases.NewCreateImageUseCase(repos.ImageRepo), diff --git a/api/pkg/claims/claims.go b/api/pkg/claims/claims.go index 6955866..46558e8 100644 --- a/api/pkg/claims/claims.go +++ b/api/pkg/claims/claims.go @@ -1,6 +1,7 @@ package claims import ( + "math" "net/http" "github.com/go-chi/jwtauth/v5" @@ -22,6 +23,10 @@ func UserId(r *http.Request) (uint32, bool) { return 0, false } + if userIdFloat < 0 || userIdFloat > math.MaxUint32 || userIdFloat != math.Trunc(userIdFloat) { + return 0, false + } + return uint32(userIdFloat), true } diff --git a/features/hasher_domain_interface.feature b/features/hasher_domain_interface.feature index 03cef79..4c5448f 100644 --- a/features/hasher_domain_interface.feature +++ b/features/hasher_domain_interface.feature @@ -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. is called with user id 1 and "" - 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" diff --git a/features/hasher_domain_interface.qa.md b/features/hasher_domain_interface.qa.md index 360b1c4..24d0202 100644 --- a/features/hasher_domain_interface.qa.md +++ b/features/hasher_domain_interface.qa.md @@ -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` → diff --git a/features/request_password_reset.feature b/features/request_password_reset.feature index a6f2f6d..a900b46 100644 --- a/features/request_password_reset.feature +++ b/features/request_password_reset.feature @@ -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 diff --git a/features/request_password_reset.qa.md b/features/request_password_reset.qa.md index 0ccc9d9..7b8dfb0 100644 --- a/features/request_password_reset.qa.md +++ b/features/request_password_reset.qa.md @@ -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". diff --git a/features/use_case_baseline.feature b/features/use_case_baseline.feature index d71d18e..6ad3466 100644 --- a/features/use_case_baseline.feature +++ b/features/use_case_baseline.feature @@ -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 diff --git a/features/verified_account_required.feature b/features/verified_account_required.feature index 53bd91f..8f2c30d 100644 --- a/features/verified_account_required.feature +++ b/features/verified_account_required.feature @@ -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 diff --git a/features/verified_account_required.qa.md b/features/verified_account_required.qa.md index 38a62b7..423b3e8 100644 --- a/features/verified_account_required.qa.md +++ b/features/verified_account_required.qa.md @@ -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 diff --git a/internal/infrastructure/useCases/DeleteUser.go b/internal/infrastructure/useCases/DeleteUser.go deleted file mode 100644 index 82be445..0000000 --- a/internal/infrastructure/useCases/DeleteUser.go +++ /dev/null @@ -1,15 +0,0 @@ -package useCases - -import "RenewCMS/internal/domain/user" - -type DeleteUserUseCase struct { - userRepository user.Repository -} - -func NewDeleteUserUseCase(userRepository user.Repository) *DeleteUserUseCase { - return &DeleteUserUseCase{userRepository} -} - -func (g *DeleteUserUseCase) DeleteUser(userId uint32) error { - return g.userRepository.Delete(userId) -} diff --git a/internal/infrastructure/useCases/RequestPasswordReset.go b/internal/infrastructure/useCases/RequestPasswordReset.go index d1f0c09..9c8a58d 100644 --- a/internal/infrastructure/useCases/RequestPasswordReset.go +++ b/internal/infrastructure/useCases/RequestPasswordReset.go @@ -4,6 +4,7 @@ import ( "RenewCMS/internal/domain/hash" "RenewCMS/internal/domain/mail" "RenewCMS/internal/domain/user" + "log" "time" "github.com/google/uuid" @@ -36,18 +37,23 @@ func (g *RequestPasswordResetUseCase) RequestPasswordReset(email string) error { hashedCode, err := g.hasher.Hash(rawCode) if err != nil { + log.Println("error when hashing the password reset code:", err) return nil } if _, err := g.userRepository.UpdatePasswordResetCode(foundUser.ID, hashedCode, time.Now().Add(time.Hour)); err != nil { + log.Println("error when calling UpdatePasswordResetCode to store the password reset code:", err) return nil } - _ = g.mailRepository.Send(foundUser.Email, "passwordReset", MailPasswordResetData{ + if err := g.mailRepository.Send(foundUser.Email, "passwordReset", MailPasswordResetData{ Host: g.publicURL, VerificationCode: rawCode, Email: foundUser.Email, - }) + }); err != nil { + log.Println("error when Send-ing the password reset mail:", err) + return nil + } return nil } diff --git a/internal/infrastructure/useCases/UpdateUser.go b/internal/infrastructure/useCases/UpdateUser.go deleted file mode 100644 index dac0815..0000000 --- a/internal/infrastructure/useCases/UpdateUser.go +++ /dev/null @@ -1,39 +0,0 @@ -package useCases - -import ( - "RenewCMS/internal/domain/hash" - "RenewCMS/internal/domain/user" - "time" -) - -type UpdateUserUseCase struct { - userRepository user.Repository - hasher hash.Hasher -} - -func NewUpdateUserUseCase(userRepository user.Repository, hasher hash.Hasher) *UpdateUserUseCase { - return &UpdateUserUseCase{userRepository, hasher} -} - -func (g *UpdateUserUseCase) UpdateVerificationStatus(userId uint32, isVerified bool) (user.User, error) { - return g.userRepository.UpdateVerificationStatus(userId, isVerified) -} - -func (g *UpdateUserUseCase) UpdatePasswordResetCode(userId uint32, code string, expiration time.Time) (user.User, error) { - if code == "" { - return g.userRepository.UpdatePasswordResetCode(userId, "", expiration) - } - hashedCode, err := g.hasher.Hash(code) - if err != nil { - return user.User{}, err - } - return g.userRepository.UpdatePasswordResetCode(userId, hashedCode, expiration) -} - -func (g *UpdateUserUseCase) UpdatePassword(userId uint32, password string) (user.User, error) { - hashedPassword, err := g.hasher.Hash(password) - if err != nil { - return user.User{}, err - } - return g.userRepository.UpdatePassword(userId, hashedPassword) -} diff --git a/test/claims_test.go b/test/claims_test.go index 9631605..bd84382 100644 --- a/test/claims_test.go +++ b/test/claims_test.go @@ -2,6 +2,7 @@ package test import ( "RenewCMS/api/pkg/claims" + "math" "net/http" "net/http/httptest" "testing" @@ -65,3 +66,88 @@ func TestClaimsTokenVersion(t *testing.T) { } }) } + +func TestClaimsUserId(t *testing.T) { + token := jwtauth.New("HS256", []byte("test-secret"), nil) + + t.Run("returns the numeric user_id claim", func(t *testing.T) { + req := requestWithJwtClaims(t, token, map[string]any{"user_id": uint32(7)}) + + got, ok := claims.UserId(req) + if !ok { + t.Fatalf("expected ok true") + } + if got != 7 { + t.Errorf("expected 7, got %d", got) + } + }) + + t.Run("returns ok false when the user_id key is missing", func(t *testing.T) { + req := requestWithJwtClaims(t, token, map[string]any{"role": "user"}) + + _, ok := claims.UserId(req) + if ok { + t.Errorf("expected ok false") + } + }) + + t.Run("returns ok false when user_id is not numeric", func(t *testing.T) { + req := requestWithJwtClaims(t, token, map[string]any{"user_id": "one"}) + + _, ok := claims.UserId(req) + if ok { + t.Errorf("expected ok false") + } + }) + + t.Run("returns ok false when user_id is negative", func(t *testing.T) { + req := requestWithJwtClaims(t, token, map[string]any{"user_id": -1.0}) + + _, ok := claims.UserId(req) + if ok { + t.Errorf("expected ok false") + } + }) + + t.Run("returns ok false when user_id has a fractional part", func(t *testing.T) { + req := requestWithJwtClaims(t, token, map[string]any{"user_id": 7.5}) + + _, ok := claims.UserId(req) + if ok { + t.Errorf("expected ok false") + } + }) + + t.Run("returns ok true when user_id is exactly zero", func(t *testing.T) { + req := requestWithJwtClaims(t, token, map[string]any{"user_id": float64(0)}) + + got, ok := claims.UserId(req) + if !ok { + t.Fatalf("expected ok true") + } + if got != 0 { + t.Errorf("expected 0, got %d", got) + } + }) + + t.Run("returns ok true when user_id is exactly the uint32 max", func(t *testing.T) { + req := requestWithJwtClaims(t, token, map[string]any{"user_id": float64(math.MaxUint32)}) + + got, ok := claims.UserId(req) + if !ok { + t.Fatalf("expected ok true") + } + if got != math.MaxUint32 { + t.Errorf("expected %d, got %d", uint32(math.MaxUint32), got) + } + }) + + t.Run("returns ok false when user_id is one past the uint32 max", func(t *testing.T) { + req := requestWithJwtClaims(t, token, map[string]any{"user_id": float64(4294967296)}) + + _, ok := claims.UserId(req) + if ok { + t.Errorf("expected ok false") + } + }) +} diff --git a/test/hasher_test.go b/test/hasher_test.go index ab07ef2..ba6ea79 100644 --- a/test/hasher_test.go +++ b/test/hasher_test.go @@ -14,7 +14,6 @@ import ( "net/http/httptest" "strings" "testing" - "time" "github.com/go-chi/chi/v5" "github.com/go-chi/jwtauth/v5" @@ -77,64 +76,6 @@ func TestCreateUserUseCaseHashesPasswordAndVerificationCode(t *testing.T) { }) } -func TestUpdateUserUseCaseHashesUpdatedValues(t *testing.T) { - cases := []struct { - method string - plaintext string - run func(uc *useCases.UpdateUserUseCase, plaintext string) (user.User, error) - stored func(u user.User) string - }{ - { - method: "UpdatePassword", - plaintext: "newPassw0rd", - run: func(uc *useCases.UpdateUserUseCase, plaintext string) (user.User, error) { - return uc.UpdatePassword(1, plaintext) - }, - stored: func(u user.User) string { return u.Password }, - }, - { - method: "UpdatePasswordResetCode", - plaintext: "reset-code-123", - run: func(uc *useCases.UpdateUserUseCase, plaintext string) (user.User, error) { - return uc.UpdatePasswordResetCode(1, plaintext, time.Now().Add(time.Hour)) - }, - stored: func(u user.User) string { return u.PasswordResetCode }, - }, - } - - for _, tc := range cases { - t.Run(tc.method+" stores the hashed value", func(t *testing.T) { - repo := newMockUserRepo() - repo.users[1] = user.User{ID: 1} - uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) - - _, err := tc.run(uc, tc.plaintext) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - expected := "hashed:" + tc.plaintext - if got := tc.stored(repo.users[1]); got != expected { - t.Errorf("expected stored value %q, got %q", expected, got) - } - }) - - t.Run(tc.method+" a hashing failure persists nothing", func(t *testing.T) { - repo := newMockUserRepo() - repo.users[1] = user.User{ID: 1} - uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{hashErr: errStubHashFailure}) - - _, err := tc.run(uc, tc.plaintext) - if !errors.Is(err, errStubHashFailure) { - t.Errorf("expected the hashing error, got %v", err) - } - if got := tc.stored(repo.users[1]); got != "" { - t.Errorf("expected the repository update to never be called, got %q", got) - } - }) - } -} - func TestAuthenticateUserUseCase(t *testing.T) { newRepo := func() *mockUserRepo { repo := newMockUserRepo() diff --git a/test/request_password_reset_http_test.go b/test/request_password_reset_http_test.go index d6163bf..873e606 100644 --- a/test/request_password_reset_http_test.go +++ b/test/request_password_reset_http_test.go @@ -172,6 +172,8 @@ func TestRequestPasswordResetHTTP(t *testing.T) { mailRepo.fail.failNext("Send") router := newRequestPasswordResetRouter(userRepo, mailRepo, "https://cms.example.com") + logBuf := captureRequestPasswordResetLog(t) + recorder := sendPasswordResetRequest(router, `{"email": "alice@example.com"}`) if recorder.Code != http.StatusOK { @@ -183,6 +185,12 @@ func TestRequestPasswordResetHTTP(t *testing.T) { if recorder.Header().Get("Set-Cookie") != "" { t.Errorf("expected no jwt cookie to be set, got %s", recorder.Header().Get("Set-Cookie")) } + if logBuf.Len() == 0 { + t.Fatalf("expected a failure to be written to the process log") + } + if !strings.Contains(strings.ToLower(logBuf.String()), "send") && !strings.Contains(strings.ToLower(logBuf.String()), "mail") { + t.Errorf("expected the log to name the mail-send failure, got %q", logBuf.String()) + } }) t.Run("An unverified user can request a password reset exactly like a verified one", func(t *testing.T) { diff --git a/test/request_password_reset_test.go b/test/request_password_reset_test.go index f0e642a..6d423a1 100644 --- a/test/request_password_reset_test.go +++ b/test/request_password_reset_test.go @@ -3,10 +3,22 @@ package test import ( "RenewCMS/internal/domain/user" "RenewCMS/internal/infrastructure/useCases" + "bytes" + "log" + "os" + "strings" "testing" "time" ) +func captureRequestPasswordResetLog(t *testing.T) *bytes.Buffer { + t.Helper() + var buf bytes.Buffer + log.SetOutput(&buf) + t.Cleanup(func() { log.SetOutput(os.Stderr) }) + return &buf +} + func newRequestPasswordResetTestUserRepo() *mockUserRepo { repo := newMockUserRepo() repo.users[3] = user.User{ID: 3, Email: "alice@example.com"} @@ -79,6 +91,7 @@ func TestRequestPasswordResetUseCase(t *testing.T) { mailRepo := newMockMailRepo() mailRepo.fail.failNext("Send") uc := useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, "https://cms.example.com") + logBuf := captureRequestPasswordResetLog(t) err := uc.RequestPasswordReset("alice@example.com") if err != nil { @@ -87,12 +100,19 @@ func TestRequestPasswordResetUseCase(t *testing.T) { if repo.updatePasswordResetCodeCalls != 1 { t.Errorf("expected UpdatePasswordResetCode to be called once, got %d", repo.updatePasswordResetCodeCalls) } + if logBuf.Len() == 0 { + t.Fatalf("expected a failure to be logged") + } + if !strings.Contains(strings.ToLower(logBuf.String()), "send") && !strings.Contains(strings.ToLower(logBuf.String()), "mail") { + t.Errorf("expected the log to identify a mail-send failure, got %q", logBuf.String()) + } }) - t.Run("The use case swallows a hash failure and returns nil without storing or sending", func(t *testing.T) { + t.Run("The use case logs and swallows a hashing failure", func(t *testing.T) { repo := newRequestPasswordResetTestUserRepo() mailRepo := newMockMailRepo() uc := useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{hashErr: errStubHashFailure}, mailRepo, "https://cms.example.com") + logBuf := captureRequestPasswordResetLog(t) err := uc.RequestPasswordReset("alice@example.com") if err != nil { @@ -104,13 +124,20 @@ func TestRequestPasswordResetUseCase(t *testing.T) { if len(mailRepo.SentEmails) != 0 { t.Errorf("expected no email to be sent, got %d", len(mailRepo.SentEmails)) } + if logBuf.Len() == 0 { + t.Fatalf("expected a failure to be logged") + } + if !strings.Contains(strings.ToLower(logBuf.String()), "hash") { + t.Errorf("expected the log to identify a hashing failure, got %q", logBuf.String()) + } }) - t.Run("The use case swallows a store failure and returns nil without sending", func(t *testing.T) { + t.Run("The use case logs and swallows a repository failure while storing the reset code", func(t *testing.T) { repo := newRequestPasswordResetTestUserRepo() repo.fail.failNext("UpdatePasswordResetCode") mailRepo := newMockMailRepo() uc := useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, "https://cms.example.com") + logBuf := captureRequestPasswordResetLog(t) err := uc.RequestPasswordReset("alice@example.com") if err != nil { @@ -119,6 +146,12 @@ func TestRequestPasswordResetUseCase(t *testing.T) { if len(mailRepo.SentEmails) != 0 { t.Errorf("expected no email to be sent, got %d", len(mailRepo.SentEmails)) } + if logBuf.Len() == 0 { + t.Fatalf("expected a failure to be logged") + } + if !strings.Contains(strings.ToLower(logBuf.String()), "updatepasswordresetcode") && !strings.Contains(strings.ToLower(logBuf.String()), "store") { + t.Errorf("expected the log to identify a storage failure, got %q", logBuf.String()) + } }) t.Run("An unverified user can request a password reset exactly like a verified one", func(t *testing.T) { diff --git a/test/reset_password_test.go b/test/reset_password_test.go index c3c6913..ee88146 100644 --- a/test/reset_password_test.go +++ b/test/reset_password_test.go @@ -164,22 +164,4 @@ func TestResetPasswordUseCase(t *testing.T) { } }) - t.Run("The generic reset-code update stores an empty code as empty instead of hashing it", func(t *testing.T) { - repo := newMockUserRepo() - repo.users[3] = user.User{ID: 3, Email: "alice@example.com"} - hasher := &stubHasher{} - uc := useCases.NewUpdateUserUseCase(repo, hasher) - - _, err := uc.UpdatePasswordResetCode(3, "", time.Time{}) - - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if hasher.hashCalls != 0 { - t.Errorf("expected Hash to never be called, got %d calls", hasher.hashCalls) - } - if repo.users[3].PasswordResetCode != "" { - t.Errorf("expected stored PasswordResetCode to be empty, got %q", repo.users[3].PasswordResetCode) - } - }) } diff --git a/test/runtime_settings_injection_http_test.go b/test/runtime_settings_injection_http_test.go index 7506ca1..fc053e7 100644 --- a/test/runtime_settings_injection_http_test.go +++ b/test/runtime_settings_injection_http_test.go @@ -145,10 +145,8 @@ func newRuntimeSettingsContainer(t *testing.T) *api.UseCases { DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(articleRepo, imageRepo), GetUserUseCase: useCases.NewGetUserUseCase(userRepo), CreateUserUseCase: useCases.NewCreateUserUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"), - UpdateUserUseCase: useCases.NewUpdateUserUseCase(userRepo, hasher), AuthenticateUserUseCase: useCases.NewAuthenticateUserUseCase(userRepo, hasher), CancelRegistrationUseCase: useCases.NewCancelRegistrationUseCase(userRepo), - DeleteUserUseCase: useCases.NewDeleteUserUseCase(userRepo), ListUsersUseCase: useCases.NewListUsersUseCase(userRepo), SendMailUseCase: useCases.NewSendMailUseCase(mailRepo), CreateImageUseCase: useCases.NewCreateImageUseCase(imageRepo), diff --git a/test/use_case_baseline_test.go b/test/use_case_baseline_test.go index caab5bf..e02cad2 100644 --- a/test/use_case_baseline_test.go +++ b/test/use_case_baseline_test.go @@ -588,101 +588,6 @@ func TestListUsersReturnsEveryUser(t *testing.T) { } } -func TestUpdateUserDelegatesEachOperation(t *testing.T) { - newRepo := func() *mockUserRepo { - repo := newMockUserRepo() - repo.users[1] = user.User{ID: 1} - return repo - } - - t.Run("UpdateVerificationStatus_id1", func(t *testing.T) { - repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) - result, err := uc.UpdateVerificationStatus(1, true) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if result.ID != 1 || !result.IsVerified { - t.Errorf("unexpected result: %+v", result) - } - }) - - t.Run("UpdateVerificationStatus_id9", func(t *testing.T) { - repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) - _, err := uc.UpdateVerificationStatus(9, true) - if !errors.Is(err, user.ErrUserNotFound) { - t.Errorf("expected user.ErrUserNotFound, got %v", err) - } - }) - - t.Run("UpdatePassword_id1", func(t *testing.T) { - repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) - result, err := uc.UpdatePassword(1, "newpassword123") - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if result.ID != 1 { - t.Errorf("expected user id 1, got %d", result.ID) - } - }) - - t.Run("UpdatePassword_id9", func(t *testing.T) { - repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) - _, err := uc.UpdatePassword(9, "newpassword123") - if !errors.Is(err, user.ErrUserNotFound) { - t.Errorf("expected user.ErrUserNotFound, got %v", err) - } - }) - - t.Run("UpdatePasswordResetCode_id1", func(t *testing.T) { - repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) - result, err := uc.UpdatePasswordResetCode(1, "reset-code-123", time.Now().Add(time.Hour)) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if result.ID != 1 { - t.Errorf("expected user id 1, got %d", result.ID) - } - }) - - t.Run("UpdatePasswordResetCode_id9", func(t *testing.T) { - repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) - _, err := uc.UpdatePasswordResetCode(9, "reset-code-123", time.Now().Add(time.Hour)) - if !errors.Is(err, user.ErrUserNotFound) { - t.Errorf("expected user.ErrUserNotFound, got %v", err) - } - }) -} - -func TestDeleteUserReportsMissingUser(t *testing.T) { - t.Run("id_1", func(t *testing.T) { - repo := newMockUserRepo() - repo.users[1] = user.User{ID: 1} - uc := useCases.NewDeleteUserUseCase(repo) - - err := uc.DeleteUser(1) - if err != nil { - t.Errorf("expected no error, got %v", err) - } - }) - - t.Run("id_9", func(t *testing.T) { - repo := newMockUserRepo() - repo.users[1] = user.User{ID: 1} - uc := useCases.NewDeleteUserUseCase(repo) - - err := uc.DeleteUser(9) - if !errors.Is(err, user.ErrUserNotFound) { - t.Errorf("expected user.ErrUserNotFound, got %v", err) - } - }) -} - func TestSendMailForwardsArgumentsVerbatim(t *testing.T) { cases := []struct { receiver string diff --git a/test/user_test.go b/test/user_test.go index 8994cc6..7ccb725 100644 --- a/test/user_test.go +++ b/test/user_test.go @@ -233,17 +233,3 @@ func TestCreateUser(t *testing.T) { t.Errorf("Verification code was not generated") } } - -func TestUpdateUserVerification(t *testing.T) { - repo := newMockUserRepo() - repo.users[1] = user.User{ID: 1, IsVerified: false} - uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) - - res, err := uc.UpdateVerificationStatus(1, true) - if err != nil { - t.Fatalf("Update failed: %v", err) - } - if !res.IsVerified { - t.Errorf("User should be verified") - } -} diff --git a/test/verified_account_required_http_test.go b/test/verified_account_required_http_test.go index 9d79aa4..8b066d2 100644 --- a/test/verified_account_required_http_test.go +++ b/test/verified_account_required_http_test.go @@ -196,6 +196,8 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { {"a validly signed jwt cookie whose claims carry no user_id key at all", map[string]any{"role": "user"}}, {`a validly signed jwt cookie whose user_id claim is the string "one"`, map[string]any{"user_id": "one"}}, {"a validly signed jwt cookie whose user_id claim is the number 999, which does not match any user", map[string]any{"user_id": uint32(999)}}, + {"a validly signed jwt cookie whose user_id claim is the number -1", map[string]any{"user_id": -1.0}}, + {"a validly signed jwt cookie whose user_id claim is the number 4294967296 (one past the uint32 max)", map[string]any{"user_id": float64(4294967296)}}, } for _, tc := range unresolvableTokenCases { t.Run("A token that resolves to no usable account still answers 401, never 403: "+tc.name, func(t *testing.T) { @@ -217,6 +219,28 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { }) } + t.Run("A fractional user_id claim is never truncated into impersonating a different real user", func(t *testing.T) { + articleRepo := newMockArticleRepo() + userRepo := newMockUserRepo() + userRepo.users[7] = domainUser.User{ID: 7, IsVerified: true} + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + + req := newVerifiedAccountRequiredRequest(http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`) + req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": 7.5})) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected status 401, got %d, body %q", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.UnauthorizedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + if len(articleRepo.articles) != 0 { + t.Errorf("expected no article to be created, got %+v", articleRepo.articles) + } + }) + noCookieCases := []struct { method string path string