diff --git a/api/dependecyInjection.go b/api/dependecyInjection.go index ac4d964..0bf3c5f 100644 --- a/api/dependecyInjection.go +++ b/api/dependecyInjection.go @@ -5,6 +5,7 @@ import ( domainImage "RenewCMS/internal/domain/image" domainMail "RenewCMS/internal/domain/mail" domainUser "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/hashing" "RenewCMS/internal/infrastructure/mailer" "RenewCMS/internal/infrastructure/persistence/models" persistence "RenewCMS/internal/infrastructure/persistence/repositories" @@ -18,20 +19,21 @@ import ( ) type UseCases struct { - Token *jwtauth.JWTAuth - CreateArticleUseCase *useCases.CreateArticleUseCase - GetArticleUseCase *useCases.GetArticleUseCase - ListArticlesUseCase *useCases.ListArticlesUseCase - UpdateArticleUseCase *useCases.UpdateArticleUseCase - DeleteArticleUseCase *useCases.DeleteArticleUseCase - GetUserUseCase *useCases.GetUserUseCase - CreateUserUseCase *useCases.CreateUserUseCase - UpdateUserUseCase *useCases.UpdateUserUseCase - DeleteUserUseCase *useCases.DeleteUserUseCase - ListUsersUseCase *useCases.ListUsersUseCase - SendMailUseCase *useCases.SendMailUseCase - CreateImageUseCase *useCases.CreateImageUseCase - DeleteImageUseCase *useCases.DeleteImageUseCase + Token *jwtauth.JWTAuth + CreateArticleUseCase *useCases.CreateArticleUseCase + GetArticleUseCase *useCases.GetArticleUseCase + ListArticlesUseCase *useCases.ListArticlesUseCase + UpdateArticleUseCase *useCases.UpdateArticleUseCase + DeleteArticleUseCase *useCases.DeleteArticleUseCase + GetUserUseCase *useCases.GetUserUseCase + CreateUserUseCase *useCases.CreateUserUseCase + UpdateUserUseCase *useCases.UpdateUserUseCase + AuthenticateUserUseCase *useCases.AuthenticateUserUseCase + DeleteUserUseCase *useCases.DeleteUserUseCase + ListUsersUseCase *useCases.ListUsersUseCase + SendMailUseCase *useCases.SendMailUseCase + CreateImageUseCase *useCases.CreateImageUseCase + DeleteImageUseCase *useCases.DeleteImageUseCase } type Repositories struct { @@ -68,21 +70,24 @@ func getRepositories(db *gorm.DB) *Repositories { } func getUseCases(repos *Repositories) *UseCases { + hasher := hashing.NewBcryptHasher() + return &UseCases{ - Token: jwtauth.New("HS256", []byte(os.Getenv("JWT_SECRET")), nil), - CreateArticleUseCase: useCases.NewCreateArticleUseCase(repos.ArticleRepo), - GetArticleUseCase: useCases.NewGetArticleUseCase(repos.ArticleRepo), - ListArticlesUseCase: useCases.NewListArticlesUseCase(repos.ArticleRepo), - UpdateArticleUseCase: useCases.NewUpdateArticleUseCase(repos.ArticleRepo), - DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(repos.ArticleRepo), - GetUserUseCase: useCases.NewGetUserUseCase(repos.UserRepo), - CreateUserUseCase: useCases.NewCreateUserUseCase(repos.UserRepo), - UpdateUserUseCase: useCases.NewUpdateUserUseCase(repos.UserRepo), - DeleteUserUseCase: useCases.NewDeleteUserUseCase(repos.UserRepo), - ListUsersUseCase: useCases.NewListUsersUseCase(repos.UserRepo), - SendMailUseCase: useCases.NewSendMailUseCase(repos.MailRepo), - CreateImageUseCase: useCases.NewCreateImageUseCase(repos.ImageRepo), - DeleteImageUseCase: useCases.NewDeleteImageUseCase(repos.ImageRepo), + Token: jwtauth.New("HS256", []byte(os.Getenv("JWT_SECRET")), nil), + CreateArticleUseCase: useCases.NewCreateArticleUseCase(repos.ArticleRepo), + GetArticleUseCase: useCases.NewGetArticleUseCase(repos.ArticleRepo), + ListArticlesUseCase: useCases.NewListArticlesUseCase(repos.ArticleRepo), + UpdateArticleUseCase: useCases.NewUpdateArticleUseCase(repos.ArticleRepo), + DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(repos.ArticleRepo), + GetUserUseCase: useCases.NewGetUserUseCase(repos.UserRepo), + CreateUserUseCase: useCases.NewCreateUserUseCase(repos.UserRepo, hasher), + UpdateUserUseCase: useCases.NewUpdateUserUseCase(repos.UserRepo, hasher), + AuthenticateUserUseCase: useCases.NewAuthenticateUserUseCase(repos.UserRepo, hasher), + DeleteUserUseCase: useCases.NewDeleteUserUseCase(repos.UserRepo), + ListUsersUseCase: useCases.NewListUsersUseCase(repos.UserRepo), + SendMailUseCase: useCases.NewSendMailUseCase(repos.MailRepo), + CreateImageUseCase: useCases.NewCreateImageUseCase(repos.ImageRepo), + DeleteImageUseCase: useCases.NewDeleteImageUseCase(repos.ImageRepo), } } diff --git a/api/handlers/auth/authHandler.go b/api/handlers/auth/authHandler.go index 682d3b8..15e0ae7 100644 --- a/api/handlers/auth/authHandler.go +++ b/api/handlers/auth/authHandler.go @@ -3,7 +3,6 @@ package auth import ( "RenewCMS/api/pkg/apperr" validator "RenewCMS/api/pkg/validator" - "RenewCMS/internal/domain/user" "RenewCMS/internal/infrastructure/useCases" "encoding/json" "net/http" @@ -11,14 +10,14 @@ import ( "time" "github.com/go-chi/jwtauth/v5" - "golang.org/x/crypto/bcrypt" ) type Handler struct { - Token *jwtauth.JWTAuth - GetUseCase *useCases.GetUserUseCase - ListUseCase *useCases.ListUsersUseCase - CreateUseCase *useCases.CreateUserUseCase + Token *jwtauth.JWTAuth + GetUseCase *useCases.GetUserUseCase + ListUseCase *useCases.ListUsersUseCase + CreateUseCase *useCases.CreateUserUseCase + AuthenticateUseCase *useCases.AuthenticateUserUseCase } type RegisterCredentials struct { @@ -48,12 +47,13 @@ func removeJwtCookie(w http.ResponseWriter) { }) } -func NewHandler(token *jwtauth.JWTAuth, get *useCases.GetUserUseCase, list *useCases.ListUsersUseCase, create *useCases.CreateUserUseCase) *Handler { +func NewHandler(token *jwtauth.JWTAuth, get *useCases.GetUserUseCase, list *useCases.ListUsersUseCase, create *useCases.CreateUserUseCase, authenticate *useCases.AuthenticateUserUseCase) *Handler { return &Handler{ - Token: token, - GetUseCase: get, - ListUseCase: list, - CreateUseCase: create, + Token: token, + GetUseCase: get, + ListUseCase: list, + CreateUseCase: create, + AuthenticateUseCase: authenticate, } } @@ -113,20 +113,6 @@ func (h *Handler) IsVerified(r *http.Request) bool { return currentUser.IsVerified } -func (h *Handler) GetUserFromCredentials(credentials LoginCredentials) (user.User, error) { - dbUser, err := h.GetUseCase.GetUserByUsername(credentials.Username) - if err != nil { - return user.User{}, err - } - - err = bcrypt.CompareHashAndPassword([]byte(dbUser.Password), []byte(credentials.Password)) - if err != nil { - return user.User{}, err - } - - return dbUser, nil -} - func (h *Handler) Login(w http.ResponseWriter, r *http.Request) { var credentials LoginCredentials @@ -142,7 +128,7 @@ func (h *Handler) Login(w http.ResponseWriter, r *http.Request) { return } - dbUser, err := h.GetUserFromCredentials(credentials) + dbUser, err := h.AuthenticateUseCase.Authenticate(credentials.Username, credentials.Password) if err != nil { http.Error(w, apperr.LogsErrorMessage, http.StatusForbidden) return diff --git a/api/router.go b/api/router.go index 68d8ecc..6153e10 100644 --- a/api/router.go +++ b/api/router.go @@ -43,7 +43,7 @@ func NewArticleRouter(container *UseCases) http.Handler { func NewAuthRouter(container *UseCases) http.Handler { r := chi.NewRouter() - authHandler := auth.NewHandler(container.Token, container.GetUserUseCase, container.ListUsersUseCase, container.CreateUserUseCase) + authHandler := auth.NewHandler(container.Token, container.GetUserUseCase, container.ListUsersUseCase, container.CreateUserUseCase, container.AuthenticateUserUseCase) r.Post("/register", authHandler.Register) r.Post("/login", authHandler.Login) diff --git a/features/hasher_domain_interface.feature b/features/hasher_domain_interface.feature new file mode 100644 index 0000000..03cef79 --- /dev/null +++ b/features/hasher_domain_interface.feature @@ -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:", 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. 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" + 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 diff --git a/features/hasher_domain_interface.qa.md b/features/hasher_domain_interface.qa.md new file mode 100644 index 0000000..360b1c4 --- /dev/null +++ b/features/hasher_domain_interface.qa.md @@ -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. diff --git a/internal/domain/hash/hashDomain.go b/internal/domain/hash/hashDomain.go new file mode 100644 index 0000000..5a29514 --- /dev/null +++ b/internal/domain/hash/hashDomain.go @@ -0,0 +1,6 @@ +package hash + +type Hasher interface { + Hash(value string) (string, error) + Compare(hashed string, value string) error +} diff --git a/internal/domain/user/userDomain.go b/internal/domain/user/userDomain.go index 86c00bc..97c03be 100644 --- a/internal/domain/user/userDomain.go +++ b/internal/domain/user/userDomain.go @@ -6,6 +6,7 @@ import ( ) var ErrUserNotFound = errors.New("user not found") +var ErrInvalidCredentials = errors.New("invalid credentials") type User struct { ID uint32 `json:"id"` diff --git a/internal/infrastructure/hashing/BcryptHasher.go b/internal/infrastructure/hashing/BcryptHasher.go new file mode 100644 index 0000000..8ee1ad7 --- /dev/null +++ b/internal/infrastructure/hashing/BcryptHasher.go @@ -0,0 +1,29 @@ +package hashing + +import ( + "RenewCMS/internal/domain/hash" + + "golang.org/x/crypto/bcrypt" +) + +const bcryptCost = 12 + +type BcryptHasher struct{} + +func NewBcryptHasher() *BcryptHasher { + return &BcryptHasher{} +} + +func (b *BcryptHasher) Hash(value string) (string, error) { + hashed, err := bcrypt.GenerateFromPassword([]byte(value), bcryptCost) + if err != nil { + return "", err + } + return string(hashed), nil +} + +func (b *BcryptHasher) Compare(hashed string, value string) error { + return bcrypt.CompareHashAndPassword([]byte(hashed), []byte(value)) +} + +var _ hash.Hasher = &BcryptHasher{} diff --git a/internal/infrastructure/persistence/repositories/userRepository.go b/internal/infrastructure/persistence/repositories/userRepository.go index 8bb4adf..ede74bf 100644 --- a/internal/infrastructure/persistence/repositories/userRepository.go +++ b/internal/infrastructure/persistence/repositories/userRepository.go @@ -5,7 +5,6 @@ import ( "RenewCMS/internal/infrastructure/persistence/mappers" entity "RenewCMS/internal/infrastructure/persistence/models" - "golang.org/x/crypto/bcrypt" "gorm.io/gorm" ) @@ -28,14 +27,11 @@ func (u *UserRepository) Get(id uint32) (domainUser.User, error) { } func (u *UserRepository) Create(user domainUser.User) (domainUser.User, error) { - hashedPassword, _ := bcrypt.GenerateFromPassword([]byte(user.Password), 12) - hashedVerificationCode, _ := bcrypt.GenerateFromPassword([]byte(user.VerificationCode), 12) - creationResult := u.db.Create(&entity.User{ Username: user.Username, - Password: string(hashedPassword), + Password: user.Password, Email: user.Email, - VerificationCode: string(hashedVerificationCode), + VerificationCode: user.VerificationCode, VerificationExpiration: user.VerificationExpiration, }) if creationResult.Error != nil { @@ -108,8 +104,7 @@ func (u *UserRepository) UpdatePassword(userId uint32, password string) (domainU return domainUser.User{}, err } - hashedPassword, _ := bcrypt.GenerateFromPassword([]byte(password), 12) - localUser.Password = string(hashedPassword) + localUser.Password = password err = u.db.Save(&localUser).Error if err != nil { return domainUser.User{}, err @@ -125,8 +120,7 @@ func (u *UserRepository) UpdatePasswordResetCode(userId uint32, code string) (do return domainUser.User{}, err } - hashedCode, _ := bcrypt.GenerateFromPassword([]byte(code), 12) - localUser.PasswordResetCode = string(hashedCode) + localUser.PasswordResetCode = code err = u.db.Save(&localUser).Error if err != nil { return domainUser.User{}, err diff --git a/internal/infrastructure/useCases/AuthenticateUser.go b/internal/infrastructure/useCases/AuthenticateUser.go new file mode 100644 index 0000000..9376bb2 --- /dev/null +++ b/internal/infrastructure/useCases/AuthenticateUser.go @@ -0,0 +1,28 @@ +package useCases + +import ( + "RenewCMS/internal/domain/hash" + "RenewCMS/internal/domain/user" +) + +type AuthenticateUserUseCase struct { + userRepository user.Repository + hasher hash.Hasher +} + +func NewAuthenticateUserUseCase(userRepository user.Repository, hasher hash.Hasher) *AuthenticateUserUseCase { + return &AuthenticateUserUseCase{userRepository, hasher} +} + +func (a *AuthenticateUserUseCase) Authenticate(username string, password string) (user.User, error) { + dbUser, err := a.userRepository.GetByUsername(username) + if err != nil { + return user.User{}, err + } + + if err := a.hasher.Compare(dbUser.Password, password); err != nil { + return user.User{}, user.ErrInvalidCredentials + } + + return dbUser, nil +} diff --git a/internal/infrastructure/useCases/CreateUser.go b/internal/infrastructure/useCases/CreateUser.go index 10ede94..c35373a 100644 --- a/internal/infrastructure/useCases/CreateUser.go +++ b/internal/infrastructure/useCases/CreateUser.go @@ -1,6 +1,7 @@ package useCases import ( + "RenewCMS/internal/domain/hash" "RenewCMS/internal/domain/user" "time" @@ -9,6 +10,7 @@ import ( type CreateUserUseCase struct { userRepository user.Repository + hasher hash.Hasher } type CreateUserCommand struct { @@ -17,20 +19,30 @@ type CreateUserCommand struct { Email string } -func NewCreateUserUseCase(userRepository user.Repository) *CreateUserUseCase { - return &CreateUserUseCase{userRepository} +func NewCreateUserUseCase(userRepository user.Repository, hasher hash.Hasher) *CreateUserUseCase { + return &CreateUserUseCase{userRepository, hasher} } func (g *CreateUserUseCase) CreateUser(createUser CreateUserCommand) (user.User, error) { rawUuid := uuid.NewString() + hashedPassword, err := g.hasher.Hash(createUser.Password) + if err != nil { + return user.User{}, err + } + + hashedVerificationCode, err := g.hasher.Hash(rawUuid) + if err != nil { + return user.User{}, err + } + createdUser, err := g.userRepository.Create(user.User{ Username: createUser.Username, - Password: createUser.Password, + Password: hashedPassword, PasswordResetCode: "", Email: createUser.Email, IsVerified: false, - VerificationCode: rawUuid, + VerificationCode: hashedVerificationCode, VerificationExpiration: time.Now().Add(2 * time.Hour), }) if err != nil { diff --git a/internal/infrastructure/useCases/UpdateUser.go b/internal/infrastructure/useCases/UpdateUser.go index 49643d5..4caf505 100644 --- a/internal/infrastructure/useCases/UpdateUser.go +++ b/internal/infrastructure/useCases/UpdateUser.go @@ -1,13 +1,17 @@ package useCases -import "RenewCMS/internal/domain/user" +import ( + "RenewCMS/internal/domain/hash" + "RenewCMS/internal/domain/user" +) type UpdateUserUseCase struct { userRepository user.Repository + hasher hash.Hasher } -func NewUpdateUserUseCase(userRepository user.Repository) *UpdateUserUseCase { - return &UpdateUserUseCase{userRepository} +func NewUpdateUserUseCase(userRepository user.Repository, hasher hash.Hasher) *UpdateUserUseCase { + return &UpdateUserUseCase{userRepository, hasher} } func (g *UpdateUserUseCase) UpdateVerificationStatus(userId uint32, isVerified bool) (user.User, error) { @@ -15,9 +19,17 @@ func (g *UpdateUserUseCase) UpdateVerificationStatus(userId uint32, isVerified b } func (g *UpdateUserUseCase) UpdatePasswordResetCode(userId uint32, code string) (user.User, error) { - return g.userRepository.UpdatePasswordResetCode(userId, code) + hashedCode, err := g.hasher.Hash(code) + if err != nil { + return user.User{}, err + } + return g.userRepository.UpdatePasswordResetCode(userId, hashedCode) } func (g *UpdateUserUseCase) UpdatePassword(userId uint32, password string) (user.User, error) { - return g.userRepository.UpdatePassword(userId, password) + hashedPassword, err := g.hasher.Hash(password) + if err != nil { + return user.User{}, err + } + return g.userRepository.UpdatePassword(userId, hashedPassword) } diff --git a/test/hasher_test.go b/test/hasher_test.go new file mode 100644 index 0000000..71b63bd --- /dev/null +++ b/test/hasher_test.go @@ -0,0 +1,362 @@ +package test + +import ( + "RenewCMS/api/handlers/auth" + "RenewCMS/api/pkg/apperr" + validator "RenewCMS/api/pkg/validator" + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/hashing" + "RenewCMS/internal/infrastructure/useCases" + "bytes" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/jwtauth/v5" +) + +var errStubHashFailure = errors.New("stub hash failure") + +type spyHasher struct { + stubHasher + compareCalls int +} + +func (s *spyHasher) Compare(hashed string, value string) error { + s.compareCalls++ + return s.stubHasher.Compare(hashed, value) +} + +func TestCreateUserUseCaseHashesPasswordAndVerificationCode(t *testing.T) { + t.Run("stores hashed password and hashed verification code, returns raw code", func(t *testing.T) { + repo := newMockUserRepo() + uc := useCases.NewCreateUserUseCase(repo, &stubHasher{}) + + result, err := uc.CreateUser(useCases.CreateUserCommand{ + Username: "alice", + Password: "s3cretPassw0rd", + Email: "alice@example.com", + }) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + stored := repo.users[result.ID] + if stored.Password != "hashed:s3cretPassw0rd" { + t.Errorf("expected stored password %q, got %q", "hashed:s3cretPassw0rd", stored.Password) + } + if stored.VerificationCode == "" || stored.VerificationCode == result.VerificationCode { + t.Errorf("expected stored verification code to be hashed, got %q", stored.VerificationCode) + } + if stored.VerificationCode != "hashed:"+result.VerificationCode { + t.Errorf("expected stored verification code %q, got %q", "hashed:"+result.VerificationCode, stored.VerificationCode) + } + }) + + t.Run("a hashing failure persists nothing", func(t *testing.T) { + repo := newMockUserRepo() + uc := useCases.NewCreateUserUseCase(repo, &stubHasher{hashErr: errStubHashFailure}) + + _, err := uc.CreateUser(useCases.CreateUserCommand{ + Username: "alice", + Password: "s3cretPassw0rd", + Email: "alice@example.com", + }) + + if !errors.Is(err, errStubHashFailure) { + t.Errorf("expected the hashing error, got %v", err) + } + if len(repo.users) != 0 { + t.Errorf("expected no user stored, got %d", len(repo.users)) + } + }) +} + +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) + }, + 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() + repo.users[1] = user.User{ID: 1, Username: "alice", Password: "hashed:hunter2example"} + return repo + } + + t.Run("correct password returns the user with no error", func(t *testing.T) { + repo := newRepo() + uc := useCases.NewAuthenticateUserUseCase(repo, &stubHasher{}) + + result, err := uc.Authenticate("alice", "hunter2example") + 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("wrong password returns user.ErrInvalidCredentials", func(t *testing.T) { + repo := newRepo() + uc := useCases.NewAuthenticateUserUseCase(repo, &stubHasher{}) + + _, err := uc.Authenticate("alice", "wrongpassword12") + if !errors.Is(err, user.ErrInvalidCredentials) { + t.Errorf("expected user.ErrInvalidCredentials, got %v", err) + } + }) + + t.Run("unknown username never calls the hasher", func(t *testing.T) { + repo := newMockUserRepo() + hasher := &spyHasher{} + uc := useCases.NewAuthenticateUserUseCase(repo, hasher) + + _, err := uc.Authenticate("bob", "hunter2example") + if !errors.Is(err, user.ErrUserNotFound) { + t.Errorf("expected user.ErrUserNotFound, got %v", err) + } + if hasher.compareCalls != 0 { + t.Errorf("expected the hasher's Compare to never be called, got %d calls", hasher.compareCalls) + } + }) +} + +func TestBcryptHasherHashAndCompare(t *testing.T) { + hasher := hashing.NewBcryptHasher() + + t.Run("hashing then comparing the same value succeeds", func(t *testing.T) { + hashed, err := hasher.Hash("hunter2example") + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if err := hasher.Compare(hashed, "hunter2example"); err != nil { + t.Errorf("expected the comparison to succeed, got %v", err) + } + }) + + t.Run("comparing against the wrong value fails", func(t *testing.T) { + hashed, err := hasher.Hash("hunter2example") + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if err := hasher.Compare(hashed, "wrongpassword12"); err == nil { + t.Errorf("expected the comparison to fail") + } + }) + + t.Run("hashing a value over 72 bytes fails", func(t *testing.T) { + tooLong := strings.Repeat("a", 73) + if _, err := hasher.Hash(tooLong); err == nil { + t.Errorf("expected an error for a value over 72 bytes") + } + }) +} + +func newHasherRouter(userRepo *mockUserRepo, hasher *stubHasher) *chi.Mux { + token := jwtauth.New("HS256", []byte("test-secret"), nil) + authHandler := auth.NewHandler( + token, + useCases.NewGetUserUseCase(userRepo), + useCases.NewListUsersUseCase(userRepo), + useCases.NewCreateUserUseCase(userRepo, hasher), + useCases.NewAuthenticateUserUseCase(userRepo, hasher), + ) + + r := chi.NewRouter() + r.Route("/v1/auth", func(r chi.Router) { + r.Post("/login", authHandler.Login) + r.Post("/register", authHandler.Register) + }) + return r +} + +func newRealHasherRouter(userRepo *mockUserRepo) *chi.Mux { + token := jwtauth.New("HS256", []byte("test-secret"), nil) + hasher := hashing.NewBcryptHasher() + authHandler := auth.NewHandler( + token, + useCases.NewGetUserUseCase(userRepo), + useCases.NewListUsersUseCase(userRepo), + useCases.NewCreateUserUseCase(userRepo, hasher), + useCases.NewAuthenticateUserUseCase(userRepo, hasher), + ) + + r := chi.NewRouter() + r.Route("/v1/auth", func(r chi.Router) { + r.Post("/login", authHandler.Login) + r.Post("/register", authHandler.Register) + }) + return r +} + +func TestLoginOverHttpCannotDistinguishBadPasswordFromUnknownUsername(t *testing.T) { + validator.InitValidator() + + newAliceRepo := func() *mockUserRepo { + repo := newMockUserRepo() + hashed, err := hashing.NewBcryptHasher().Hash("hunter2example") + if err != nil { + t.Fatalf("failed to hash password: %v", err) + } + repo.users[1] = user.User{ID: 1, Username: "alice", Password: hashed} + return repo + } + + cases := []struct { + name string + username string + password string + }{ + {"wrong password for existing user", "alice", "wrongpassword12"}, + {"nonexistent username", "bob", "hunter2example"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + router := newRealHasherRouter(newAliceRepo()) + body, err := json.Marshal(map[string]string{"username": tc.username, "password": tc.password}) + if err != nil { + t.Fatalf("failed to marshal body: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/v1/auth/login", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d", rec.Code) + } + if rec.Body.String() != apperr.LogsErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + if rec.Header().Get("Set-Cookie") != "" { + t.Errorf("expected no Set-Cookie header, got %q", rec.Header().Get("Set-Cookie")) + } + }) + } +} + +func TestRegisteringAUserStoresTheHashedPasswordNeverThePlaintext(t *testing.T) { + validator.InitValidator() + + repo := newMockUserRepo() + router := newRealHasherRouter(repo) + + body, err := json.Marshal(map[string]string{ + "username": "alice", + "password": "s3cretPassw0rd", + "email": "alice@example.com", + }) + if err != nil { + t.Fatalf("failed to marshal body: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/v1/auth/register", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Header().Get("Set-Cookie") == "" { + t.Errorf("expected a jwt cookie to be set") + } + + stored, err := repo.GetByUsername("alice") + if err != nil { + t.Fatalf("expected the user to be stored, got %v", err) + } + if stored.Password == "s3cretPassw0rd" { + t.Errorf("expected the stored password to not be the plaintext") + } + + hasher := hashing.NewBcryptHasher() + if err := hasher.Compare(stored.Password, "s3cretPassw0rd"); err != nil { + t.Errorf("expected the stored password to match the plaintext once compared, got %v", err) + } +} + +func TestAHashingFailureDuringRegistrationSurfacesAs500AndSetsNoCookie(t *testing.T) { + validator.InitValidator() + + repo := newMockUserRepo() + router := newHasherRouter(repo, &stubHasher{hashErr: errStubHashFailure}) + + body, err := json.Marshal(map[string]string{ + "username": "alice", + "password": "s3cretPassw0rd", + "email": "alice@example.com", + }) + if err != nil { + t.Fatalf("failed to marshal body: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/v1/auth/register", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected status 500, got %d", rec.Code) + } + if rec.Header().Get("Set-Cookie") != "" { + t.Errorf("expected no Set-Cookie header, got %q", rec.Header().Get("Set-Cookie")) + } +} diff --git a/test/not_found_errors_test.go b/test/not_found_errors_test.go index 62209c8..b79f785 100644 --- a/test/not_found_errors_test.go +++ b/test/not_found_errors_test.go @@ -7,6 +7,7 @@ import ( validator "RenewCMS/api/pkg/validator" domainArticle "RenewCMS/internal/domain/article" domainUser "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/hashing" "RenewCMS/internal/infrastructure/persistence/models" persistence "RenewCMS/internal/infrastructure/persistence/repositories" "RenewCMS/internal/infrastructure/useCases" @@ -20,7 +21,6 @@ import ( "github.com/glebarez/sqlite" "github.com/go-chi/chi/v5" "github.com/go-chi/jwtauth/v5" - "golang.org/x/crypto/bcrypt" "gorm.io/gorm" ) @@ -130,11 +130,13 @@ func newNotFoundErrorsRouter(articleRepo *mockArticleRepo, userRepo *mockUserRep useCases.NewListArticlesUseCase(articleRepo), useCases.NewDeleteArticleUseCase(articleRepo), ) + hasher := hashing.NewBcryptHasher() authHandler := auth.NewHandler( token, useCases.NewGetUserUseCase(userRepo), useCases.NewListUsersUseCase(userRepo), - useCases.NewCreateUserUseCase(userRepo), + useCases.NewCreateUserUseCase(userRepo, hasher), + useCases.NewAuthenticateUserUseCase(userRepo, hasher), ) r := chi.NewRouter() @@ -235,11 +237,11 @@ func TestNotFoundErrorsHttp(t *testing.T) { newAliceUserRepo := func() *mockUserRepo { repo := newMockUserRepo() - hashed, err := bcrypt.GenerateFromPassword([]byte("hunter2example"), 12) + hashed, err := hashing.NewBcryptHasher().Hash("hunter2example") if err != nil { t.Fatalf("failed to hash password: %v", err) } - repo.users[1] = domainUser.User{ID: 1, Username: "alice", Password: string(hashed)} + repo.users[1] = domainUser.User{ID: 1, Username: "alice", Password: hashed} return repo } diff --git a/test/use_case_baseline_test.go b/test/use_case_baseline_test.go index 56060fb..704c217 100644 --- a/test/use_case_baseline_test.go +++ b/test/use_case_baseline_test.go @@ -433,7 +433,7 @@ func TestDeleteImageRequiresExistence(t *testing.T) { func TestCreateUserAssignsVerificationCode(t *testing.T) { repo := newMockUserRepo() - uc := useCases.NewCreateUserUseCase(repo) + uc := useCases.NewCreateUserUseCase(repo, &stubHasher{}) before := time.Now() result, err := uc.CreateUser(useCases.CreateUserCommand{ @@ -480,7 +480,7 @@ func TestCreateUserAssignsVerificationCode(t *testing.T) { func TestCreateUserPropagatesRepositoryFailure(t *testing.T) { repo := newMockUserRepo() repo.fail.failNext("Create") - uc := useCases.NewCreateUserUseCase(repo) + uc := useCases.NewCreateUserUseCase(repo, &stubHasher{}) _, err := uc.CreateUser(useCases.CreateUserCommand{ Username: "alice", @@ -590,7 +590,7 @@ func TestUpdateUserDelegatesEachOperation(t *testing.T) { t.Run("UpdateVerificationStatus_id1", func(t *testing.T) { repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo) + uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) result, err := uc.UpdateVerificationStatus(1, true) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -602,7 +602,7 @@ func TestUpdateUserDelegatesEachOperation(t *testing.T) { t.Run("UpdateVerificationStatus_id9", func(t *testing.T) { repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo) + uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) _, err := uc.UpdateVerificationStatus(9, true) if !errors.Is(err, gorm.ErrRecordNotFound) { t.Errorf("expected gorm.ErrRecordNotFound, got %v", err) @@ -611,7 +611,7 @@ func TestUpdateUserDelegatesEachOperation(t *testing.T) { t.Run("UpdatePassword_id1", func(t *testing.T) { repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo) + uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) result, err := uc.UpdatePassword(1, "newpassword123") if err != nil { t.Fatalf("expected no error, got %v", err) @@ -623,7 +623,7 @@ func TestUpdateUserDelegatesEachOperation(t *testing.T) { t.Run("UpdatePassword_id9", func(t *testing.T) { repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo) + uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) _, err := uc.UpdatePassword(9, "newpassword123") if !errors.Is(err, gorm.ErrRecordNotFound) { t.Errorf("expected gorm.ErrRecordNotFound, got %v", err) @@ -632,7 +632,7 @@ func TestUpdateUserDelegatesEachOperation(t *testing.T) { t.Run("UpdatePasswordResetCode_id1", func(t *testing.T) { repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo) + uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) result, err := uc.UpdatePasswordResetCode(1, "reset-code-123") if err != nil { t.Fatalf("expected no error, got %v", err) @@ -644,7 +644,7 @@ func TestUpdateUserDelegatesEachOperation(t *testing.T) { t.Run("UpdatePasswordResetCode_id9", func(t *testing.T) { repo := newRepo() - uc := useCases.NewUpdateUserUseCase(repo) + uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) _, err := uc.UpdatePasswordResetCode(9, "reset-code-123") if !errors.Is(err, gorm.ErrRecordNotFound) { t.Errorf("expected gorm.ErrRecordNotFound, got %v", err) diff --git a/test/user_test.go b/test/user_test.go index 66d0517..6d9668d 100644 --- a/test/user_test.go +++ b/test/user_test.go @@ -3,12 +3,31 @@ package test import ( "RenewCMS/internal/domain/user" "RenewCMS/internal/infrastructure/useCases" + "errors" "sort" "testing" "gorm.io/gorm" ) +type stubHasher struct { + hashErr error +} + +func (s *stubHasher) Hash(value string) (string, error) { + if s.hashErr != nil { + return "", s.hashErr + } + return "hashed:" + value, nil +} + +func (s *stubHasher) Compare(hashed string, value string) error { + if hashed == "hashed:"+value { + return nil + } + return errors.New("hash mismatch") +} + type mockUserRepo struct { users map[uint32]user.User nextID uint32 @@ -134,7 +153,7 @@ var _ user.Repository = &mockUserRepo{} func TestCreateUser(t *testing.T) { repo := newMockUserRepo() - uc := useCases.NewCreateUserUseCase(repo) + uc := useCases.NewCreateUserUseCase(repo, &stubHasher{}) cmd := useCases.CreateUserCommand{ Username: "testuser", @@ -157,7 +176,7 @@ func TestCreateUser(t *testing.T) { func TestUpdateUserVerification(t *testing.T) { repo := newMockUserRepo() repo.users[1] = user.User{ID: 1, IsVerified: false} - uc := useCases.NewUpdateUserUseCase(repo) + uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) res, err := uc.UpdateVerificationStatus(1, true) if err != nil {