From 7aae704d0ae40ed4892b714e888aa371e0f0bf28 Mon Sep 17 00:00:00 2001 From: Florian Sylvain Date: Tue, 29 Sep 2026 06:52:58 +0200 Subject: [PATCH] feat(API): optional display name on users --- api/dependecyInjection.go | 2 + api/handlers/auth/meHandler.go | 3 +- api/handlers/auth/verificationHandler.go | 24 +- api/handlers/user/userHandler.go | 70 +++ api/pkg/apperr/messages.go | 1 + api/pkg/publicuser/publicUser.go | 28 ++ api/router.go | 15 + features/get_current_user.feature | 7 +- features/user_display_name.feature | 242 +++++++++ features/user_display_name.qa.md | 127 +++++ features/user_response_field_hygiene.feature | 15 +- features/user_response_field_hygiene.qa.md | 9 +- internal/domain/user/userDomain.go | 2 + .../domain/user/userRepositoryInterface.go | 1 + .../persistence/mappers/userMapper.go | 2 + .../persistence/models/userModel.go | 1 + .../repositories/userRepository.go | 16 + .../useCases/UpdateUserProfile.go | 25 + test/get_current_user_http_test.go | 5 +- test/user_display_name_test.go | 471 ++++++++++++++++++ test/user_response_field_hygiene_test.go | 8 +- test/user_test.go | 17 + 22 files changed, 1051 insertions(+), 40 deletions(-) create mode 100644 api/handlers/user/userHandler.go create mode 100644 api/pkg/publicuser/publicUser.go create mode 100644 features/user_display_name.feature create mode 100644 features/user_display_name.qa.md create mode 100644 internal/infrastructure/useCases/UpdateUserProfile.go create mode 100644 test/user_display_name_test.go diff --git a/api/dependecyInjection.go b/api/dependecyInjection.go index 6b8c29a..5eb4167 100644 --- a/api/dependecyInjection.go +++ b/api/dependecyInjection.go @@ -33,6 +33,7 @@ type UseCases struct { CancelRegistrationUseCase *useCases.CancelRegistrationUseCase RequestPasswordResetUseCase *useCases.RequestPasswordResetUseCase ResetPasswordUseCase *useCases.ResetPasswordUseCase + UpdateUserProfileUseCase *useCases.UpdateUserProfileUseCase ListUsersUseCase *useCases.ListUsersUseCase SendMailUseCase *useCases.SendMailUseCase CreateImageUseCase *useCases.CreateImageUseCase @@ -88,6 +89,7 @@ func getUseCases(repos *Repositories, settings config.Settings) *UseCases { CancelRegistrationUseCase: useCases.NewCancelRegistrationUseCase(repos.UserRepo), RequestPasswordResetUseCase: useCases.NewRequestPasswordResetUseCase(repos.UserRepo, hasher, repos.MailRepo, settings.PublicURL), ResetPasswordUseCase: useCases.NewResetPasswordUseCase(repos.UserRepo, hasher), + UpdateUserProfileUseCase: useCases.NewUpdateUserProfileUseCase(repos.UserRepo), ListUsersUseCase: useCases.NewListUsersUseCase(repos.UserRepo), SendMailUseCase: useCases.NewSendMailUseCase(repos.MailRepo), CreateImageUseCase: useCases.NewCreateImageUseCase(repos.ImageRepo), diff --git a/api/handlers/auth/meHandler.go b/api/handlers/auth/meHandler.go index 001dcf7..8fc8ff8 100644 --- a/api/handlers/auth/meHandler.go +++ b/api/handlers/auth/meHandler.go @@ -3,6 +3,7 @@ package auth import ( "RenewCMS/api/pkg/apperr" "RenewCMS/api/pkg/claims" + "RenewCMS/api/pkg/publicuser" "RenewCMS/internal/domain/user" "RenewCMS/internal/infrastructure/useCases" "encoding/json" @@ -37,7 +38,7 @@ func (h *MeHandler) Me(w http.ResponseWriter, r *http.Request) { return } - body, err := json.Marshal(newPublicUser(loadedUser)) + body, err := json.Marshal(publicuser.New(loadedUser)) if err != nil { http.Error(w, "Internal server error", http.StatusInternalServerError) return diff --git a/api/handlers/auth/verificationHandler.go b/api/handlers/auth/verificationHandler.go index 42b7e39..870f1c9 100644 --- a/api/handlers/auth/verificationHandler.go +++ b/api/handlers/auth/verificationHandler.go @@ -3,13 +3,13 @@ package auth import ( "RenewCMS/api/pkg/apperr" "RenewCMS/api/pkg/claims" + "RenewCMS/api/pkg/publicuser" validator "RenewCMS/api/pkg/validator" "RenewCMS/internal/domain/user" "RenewCMS/internal/infrastructure/useCases" "encoding/json" "errors" "net/http" - "time" ) type VerificationHandler struct { @@ -22,26 +22,6 @@ type VerifyCredentials struct { Code string `json:"code" validate:"required"` } -type PublicUser struct { - ID uint32 `json:"id"` - Username string `json:"username"` - Email string `json:"email"` - IsVerified bool `json:"is_verified"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` -} - -func newPublicUser(u user.User) PublicUser { - return PublicUser{ - ID: u.ID, - Username: u.Username, - Email: u.Email, - IsVerified: u.IsVerified, - CreatedAt: u.CreatedAt, - UpdatedAt: u.UpdatedAt, - } -} - func NewVerificationHandler(verify *useCases.VerifyUserUseCase, cancel *useCases.CancelRegistrationUseCase, isProduction bool) *VerificationHandler { return &VerificationHandler{ VerifyUseCase: verify, @@ -84,7 +64,7 @@ func (h *VerificationHandler) Verify(w http.ResponseWriter, r *http.Request) { return } - body, err := json.Marshal(newPublicUser(verifiedUser)) + body, err := json.Marshal(publicuser.New(verifiedUser)) if err != nil { http.Error(w, "Internal server error", http.StatusInternalServerError) return diff --git a/api/handlers/user/userHandler.go b/api/handlers/user/userHandler.go new file mode 100644 index 0000000..57496ad --- /dev/null +++ b/api/handlers/user/userHandler.go @@ -0,0 +1,70 @@ +package user + +import ( + "RenewCMS/api/pkg/apperr" + "RenewCMS/api/pkg/claims" + "RenewCMS/api/pkg/publicuser" + validator "RenewCMS/api/pkg/validator" + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "encoding/json" + "errors" + "net/http" +) + +type Handler struct { + UpdateProfileUseCase *useCases.UpdateUserProfileUseCase +} + +type UpdateProfileRequest struct { + DisplayName *string `json:"display_name" validate:"required"` +} + +func NewHandler(updateProfile *useCases.UpdateUserProfileUseCase) *Handler { + return &Handler{ + UpdateProfileUseCase: updateProfile, + } +} + +func (h *Handler) UpdateMe(w http.ResponseWriter, r *http.Request) { + var request UpdateProfileRequest + err := json.NewDecoder(r.Body).Decode(&request) + if err != nil { + http.Error(w, apperr.BodyErrorMessage, http.StatusBadRequest) + return + } + + err = validator.Validate.Struct(request) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + userId, ok := claims.UserId(r) + if !ok { + http.Error(w, apperr.UnauthorizedErrorMessage, http.StatusUnauthorized) + return + } + + updatedUser, err := h.UpdateProfileUseCase.UpdateDisplayName(userId, *request.DisplayName) + if err != nil { + if errors.Is(err, user.ErrInvalidDisplayName) { + http.Error(w, apperr.DisplayNameErrorMessage, http.StatusBadRequest) + return + } + if errors.Is(err, user.ErrUserNotFound) { + http.Error(w, apperr.UnauthorizedErrorMessage, http.StatusUnauthorized) + return + } + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + + body, err := json.Marshal(publicuser.New(updatedUser)) + if err != nil { + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + w.Header().Set("Cache-Control", "no-store") + _, _ = w.Write(body) +} diff --git a/api/pkg/apperr/messages.go b/api/pkg/apperr/messages.go index 4b5e14f..2f221db 100644 --- a/api/pkg/apperr/messages.go +++ b/api/pkg/apperr/messages.go @@ -5,6 +5,7 @@ const BodyErrorMessage = "The request cannot be processed due to a mismatch in t const IdUint32ErrorMessage = "The server expects the ID to be in the format of an unsigned 32-bit integer (uint32)." const ArticleNotFoundErrorMessage = "Article not found" const ImageNotFoundErrorMessage = "Image not found" +const DisplayNameErrorMessage = "Display name must be 60 characters or fewer." const ArticleTitleErrorMessage = "Article title must be between 3 and 200 characters." const RegistrationClosedErrorMessage = "You are not allowed to create a user. Log in or reset database." const RegistrationFailedErrorMessage = "Registration failed. Please try again later." diff --git a/api/pkg/publicuser/publicUser.go b/api/pkg/publicuser/publicUser.go new file mode 100644 index 0000000..e7f4617 --- /dev/null +++ b/api/pkg/publicuser/publicUser.go @@ -0,0 +1,28 @@ +package publicuser + +import ( + "RenewCMS/internal/domain/user" + "time" +) + +type PublicUser struct { + ID uint32 `json:"id"` + Username string `json:"username"` + Email string `json:"email"` + IsVerified bool `json:"is_verified"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + DisplayName string `json:"display_name"` +} + +func New(u user.User) PublicUser { + return PublicUser{ + ID: u.ID, + Username: u.Username, + Email: u.Email, + IsVerified: u.IsVerified, + CreatedAt: u.CreatedAt, + UpdatedAt: u.UpdatedAt, + DisplayName: u.DisplayName, + } +} diff --git a/api/router.go b/api/router.go index 59327ad..7a4cd8e 100644 --- a/api/router.go +++ b/api/router.go @@ -6,6 +6,7 @@ import ( "RenewCMS/api/handlers/frontend" "RenewCMS/api/handlers/image" "RenewCMS/api/handlers/staticimages" + "RenewCMS/api/handlers/user" "RenewCMS/api/middleware" "RenewCMS/internal/infrastructure/config" "encoding/json" @@ -87,6 +88,19 @@ func NewAuthRouter(container *UseCases, isProduction bool, rateLimit config.Auth return r } +func NewUserRouter(container *UseCases) http.Handler { + r := chi.NewRouter() + + userHandler := user.NewHandler(container.UpdateUserProfileUseCase) + + r.Use(jwtauth.Verifier(container.Token)) + r.Use(jwtauth.Authenticator(container.Token)) + r.Use(middleware.RequireCurrentToken(container.GetUserUseCase)) + r.Use(middleware.RequireVerifiedUser(container.GetUserUseCase)) + r.Patch("/me", userHandler.UpdateMe) + return r +} + func InitBackendRoutes(container *UseCases, settings config.Settings) *chi.Mux { r := chi.NewRouter() @@ -99,6 +113,7 @@ func InitBackendRoutes(container *UseCases, settings config.Settings) *chi.Mux { r.Use(jwtauth.Verifier(container.Token)) r.Mount("/article", NewArticleRouter(container)) }) + r.Mount("/user", NewUserRouter(container)) r.Mount("/auth", NewAuthRouter(container, settings.IsProduction(), settings.AuthRateLimit, settings.TrustedProxies)) return r diff --git a/features/get_current_user.feature b/features/get_current_user.feature index ec2d090..8b956a3 100644 --- a/features/get_current_user.feature +++ b/features/get_current_user.feature @@ -9,9 +9,10 @@ Feature: A logged-in user fetches their own public profile without triggering th without being throttled like a login attempt Background: - Given the existing PublicUser struct and newPublicUser(u user.User) converter in - api/handlers/auth/verificationHandler.go, exactly the JSON shape POST /v1/auth/verify already - returns: id, username, email, is_verified, created_at, updated_at + Given the public user struct and its converter, shared by the auth and user handlers from a + package under api/pkg (features/user_display_name.feature), exactly the JSON shape + POST /v1/auth/verify returns: id, username, email, is_verified, created_at, updated_at and + display_name And this slice adds GET /v1/auth/me, handled by a new MeHandler (api/handlers/auth/meHandler.go) built with NewMeHandler(container.GetUserUseCase), the same GetUserUseCase every other protected handler already gets from api/dependecyInjection.go; no new use case or domain diff --git a/features/user_display_name.feature b/features/user_display_name.feature new file mode 100644 index 0000000..aabe03f --- /dev/null +++ b/features/user_display_name.feature @@ -0,0 +1,242 @@ +Feature: A verified user can set an optional display name that replaces their username for display only + + As one of several admins with equal access to a RenewCMS workspace + I want to give my account an optional display name, up to 60 characters, and have the API + return it wherever my own account is returned + So that people are named by a human name instead of a username, while logging in still uses + the username and no secret field ever reaches a response + + Background: + Given internal/domain/user gains the field DisplayName, json "display_name", with no gorm tag, + and the sentinel user.ErrInvalidDisplayName + And user.Repository gains UpdateDisplayName(userId uint32, displayName string) (User, error), + returning user.ErrUserNotFound when the user does not exist + And the persistence model stores the display name in a column whose default is the empty + string, so rows created before this slice still load with DisplayName "" + And a new use case, UpdateUserProfile, exposes UpdateDisplayName(userId uint32, displayName + string) (user.User, error) and depends only on user.Repository + And UpdateUserProfile trims the value with strings.TrimSpace, then counts what remains with + utf8.RuneCountInString (runes, not bytes), returns user.ErrInvalidDisplayName without calling + the repository when the count is above 60, and otherwise hands the trimmed value to + UpdateDisplayName; an empty or whitespace-only value therefore stores "" and clears the name + And api/pkg/apperr defines DisplayNameErrorMessage as "Display name must be 60 characters or + fewer." + And PATCH /v1/user/me is served by a new handler in api/handlers/user/, mounted by a new + /v1/user router in api/router.go inside a group carrying jwtauth.Verifier, + jwtauth.Authenticator, middleware.RequireCurrentToken and middleware.RequireVerifiedUser, in + that order, so an unauthenticated caller is refused with 401, a stale token with 401 + apperr.UnauthorizedErrorMessage, and an unverified caller with 403 + apperr.VerificationRequiredErrorMessage before the handler runs + And the request body is JSON {"display_name": }; the request struct field is a + *string tagged validate:"required" and is checked with validator.Validate.Struct; a body that + is not valid JSON, a display_name that is not a string, or a missing or null display_name + answers 400 + And the handler maps user.ErrInvalidDisplayName to 400 with body + apperr.DisplayNameErrorMessage, user.ErrUserNotFound to 401 apperr.UnauthorizedErrorMessage, + and any other error to 500 "Internal server error", all as plain text from http.Error + And the public user struct and its converter move out of api/handlers/auth into a shared + package under api/pkg, imported by both the auth and user handlers, so neither handler + imports the other + And on success the response is 200 and the body is the updated user in that public shape: + exactly id, username, email, is_verified, created_at, updated_at and display_name + And GET /v1/auth/me and POST /v1/auth/verify use that same public shape, so display_name is + present in both, as "" when unset + And password, password_reset_code, verification_code, verification_expiration, + password_reset_expiration and token_version never appear in any response + (features/user_response_field_hygiene.feature) + And POST /v1/auth/login still authenticates with the username only; display_name is neither + accepted nor matched there + And changing the display name does not change token_version, so existing sessions stay valid + + Scenario: A verified user sets a display name + Given a user repository containing user id 7, username "marta", email "marta@renew.example", + is_verified true, DisplayName "", TokenVersion 0 + And a valid jwt cookie for user id 7 encoding token_version 0 + When the client sends PATCH /v1/user/me with that cookie and body {"display_name": "Marta Alves"} + Then the response status is 200 + And the response body is a JSON object with exactly the keys id, username, email, is_verified, + created_at, updated_at and display_name + And the response body's id is 7, username "marta", display_name "Marta Alves" + And the user repository now holds DisplayName "Marta Alves" for user id 7 + And the user repository still holds username "marta" + + Scenario Outline: The value is trimmed before it is stored and returned + Given a user repository containing user id 7, username "marta", is_verified true, DisplayName "" + And a valid jwt cookie for user id 7 + When the client sends PATCH /v1/user/me with that cookie and body {"display_name": } + Then the response status is 200 + And the response body's display_name is + And the user repository holds DisplayName for user id 7 + + Examples: + | sent | returned | + | " Marta Alves " | "Marta Alves" | + | "\tMarta Alves\n" | "Marta Alves" | + | "Marta Alves" | "Marta Alves" | + + Scenario Outline: An empty or whitespace-only value clears the display name + Given a user repository containing user id 7, username "marta", is_verified true, DisplayName + "Marta Alves" + And a valid jwt cookie for user id 7 + When the client sends PATCH /v1/user/me with that cookie and body {"display_name": } + Then the response status is 200 + And the response body's display_name is "" + And the response body's username is "marta" + And the user repository holds DisplayName "" for user id 7 + + Examples: + | sent | + | "" | + | " " | + | "\t \n" | + + Scenario Outline: The limit is 60 Unicode characters, counted as runes + Given a user repository containing user id 7, username "marta", is_verified true, DisplayName "" + And a valid jwt cookie for user id 7 + When the client sends PATCH /v1/user/me with that cookie and a display_name of repetitions + of + Then the response status is + And + + Examples: + | count | unit | status | effect | + | 60 | "a" | 200 | the repository holds that 60-character value | + | 61 | "a" | 400 | the body is apperr.DisplayNameErrorMessage and DisplayName is still "" | + | 60 | "😀" | 200 | the repository holds that 60-rune value (240 bytes) | + | 61 | "😀" | 400 | the body is apperr.DisplayNameErrorMessage and DisplayName is still "" | + | 60 | "é" | 200 | the repository holds that 60-rune value (120 bytes) | + + Scenario: The limit applies after trimming, not before + Given a user repository containing user id 7, username "marta", is_verified true, DisplayName "" + And a valid jwt cookie for user id 7 + When the client sends PATCH /v1/user/me with that cookie and a display_name made of 5 spaces, + 60 "a" characters and 5 spaces (70 characters in total) + Then the response status is 200 + And the response body's display_name is the 60 "a" characters + + Scenario: Control characters inside the name are kept + Given a user repository containing user id 7, username "marta", is_verified true, DisplayName "" + And a valid jwt cookie for user id 7 + When the client sends PATCH /v1/user/me with that cookie and body {"display_name": "Ma\nrta"} + Then the response status is 200 + And the response body's display_name is "Ma\nrta" + + Scenario: A rejected value leaves the existing display name untouched + Given a user repository containing user id 7, username "marta", is_verified true, DisplayName + "Marta Alves" + And a valid jwt cookie for user id 7 + When the client sends PATCH /v1/user/me with that cookie and a display_name of 61 "a" characters + Then the response status is 400 + And the repository's UpdateDisplayName is never called + And the user repository still holds DisplayName "Marta Alves" for user id 7 + + Scenario Outline: A malformed body is refused with 400 + Given a user repository containing user id 7, username "marta", is_verified true, DisplayName + "Marta Alves" + And a valid jwt cookie for user id 7 + When the client sends PATCH /v1/user/me with that cookie and + Then the response status is 400 + And the repository's UpdateDisplayName is never called + And the user repository still holds DisplayName "Marta Alves" for user id 7 + + Examples: + | body | + | the raw text not json | + | {} | + | {"display_name": null} | + | {"display_name": 42} | + | {"displayName": "Marta"} | + + Scenario: The password, token version and login username are unaffected by a display name + Given a user repository containing user id 7, username "marta", is_verified true, a stored + password hash, DisplayName "", TokenVersion 0 + And a valid jwt cookie for user id 7 + When the client sends PATCH /v1/user/me with that cookie and body {"display_name": "Marta Alves"} + Then the response status is 200 + And the stored password hash for user id 7 is unchanged + And the stored TokenVersion for user id 7 is unchanged + And the same jwt cookie still gets 200 from GET /v1/auth/me + + Scenario: GET /v1/auth/me returns the display name once set + Given a user repository containing user id 7, username "marta", is_verified true, DisplayName + "Marta Alves" + And a valid jwt cookie for user id 7 + When the client sends GET /v1/auth/me with that cookie + Then the response status is 200 + And the response body has display_name "Marta Alves" and username "marta" + + Scenario: GET /v1/auth/me returns an empty display_name, not a missing key, when none is set + Given a user repository containing user id 8, username "bob", is_verified false, DisplayName "" + And a valid jwt cookie for user id 8 + When the client sends GET /v1/auth/me with that cookie + Then the response status is 200 + And the response body is a JSON object with exactly the keys id, username, email, is_verified, + created_at, updated_at and display_name + And the response body's display_name is "" + + Scenario: Logging in still uses the username only + Given a user repository containing verified user "marta" with a known password and DisplayName + "Ines Duarte" + When the client sends POST /v1/auth/login with username "Ines Duarte" and the correct password + Then the response status is 403 + And the response body is apperr.LogsErrorMessage + When the client sends POST /v1/auth/login with username "marta" and the correct password + Then the response status is 200 + + Scenario: An unverified user is refused + Given a user repository containing user id 8, username "bob", is_verified false, DisplayName "" + And a valid jwt cookie for user id 8 + When the client sends PATCH /v1/user/me with that cookie and body {"display_name": "Bob"} + Then the response status is 403 + And the response body is the apperr.VerificationRequiredErrorMessage text + And the user repository still holds DisplayName "" for user id 8 + + Scenario: No jwt cookie is refused + Given a user repository containing user id 7, username "marta", is_verified true + When the client sends PATCH /v1/user/me with no jwt cookie and body {"display_name": "Marta"} + Then the response status is 401 + And the repository's UpdateDisplayName is never called + + Scenario: A stale jwt is refused + Given a user repository containing user id 7, username "marta", is_verified true, TokenVersion 1 + And a jwt cookie for user id 7 encoding token_version 0 + When the client sends PATCH /v1/user/me with that cookie and body {"display_name": "Marta"} + Then the response status is 401 + And the response body is the apperr.UnauthorizedErrorMessage text + And the repository's UpdateDisplayName is never called + + Scenario: A jwt minted for a since-deleted account is refused + Given a user repository containing no user with id 999 + And a valid jwt cookie for user id 999 + When the client sends PATCH /v1/user/me with that cookie and body {"display_name": "Marta"} + Then the response status is 401 + And the response body is the apperr.UnauthorizedErrorMessage text + + Scenario: A repository failure while saving answers 500 + Given a user repository containing user id 7, username "marta", is_verified true + And the user repository fails on its next call to UpdateDisplayName + And a valid jwt cookie for user id 7 + When the client sends PATCH /v1/user/me with that cookie and body {"display_name": "Marta"} + Then the response status is 500 + And the response body is the text "Internal server error" + + Scenario: The user is deleted between authentication and the save + Given a user repository whose UpdateDisplayName returns user.ErrUserNotFound for user id 7 + And a valid jwt cookie for user id 7 that passes the middleware + When the client sends PATCH /v1/user/me with that cookie and body {"display_name": "Marta"} + Then the response status is 401 + And the response body is the apperr.UnauthorizedErrorMessage text + + Scenario Outline: The use case enforces the same rule without HTTP + Given a fake user repository containing user id 7 with DisplayName "Old" + When UpdateUserProfile.UpdateDisplayName is called for user id 7 with + Then the result is + + Examples: + | input | outcome | + | " Marta " | the user with DisplayName "Marta" | + | "" | the user with DisplayName "" | + | " " | the user with DisplayName "" | + | 60 "😀" runes | the user with those 60 runes | + | 61 "😀" runes | user.ErrInvalidDisplayName (errors.Is), repository not called | + | "Marta" for unknown user id 99 | user.ErrUserNotFound (errors.Is) | diff --git a/features/user_display_name.qa.md b/features/user_display_name.qa.md new file mode 100644 index 0000000..a8012ed --- /dev/null +++ b/features/user_display_name.qa.md @@ -0,0 +1,127 @@ +# QA procedure: user_display_name + +Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`, `stubHasher`) and +following the router wiring pattern in `test/get_current_user_http_test.go`. No live server, no real +SQLite file, no real SMTP. Files: `test/user_display_name_test.go` (use case and HTTP). + +## Test infrastructure additions needed first + +1. Add `UpdateDisplayName(userId uint32, displayName string) (user.User, error)` to `mockUserRepo`: + call `m.fail.trigger("UpdateDisplayName")` first, return `user.ErrUserNotFound` when the id is + absent, otherwise store `DisplayName` on the map entry and return it. Add fields + `updateDisplayNameCalls int` and `lastUpdateDisplayName string`. +2. Use `failNext("UpdateDisplayName")` for the 500 case. For the "deleted between auth and save" + case, add `failNextWith("UpdateDisplayName", user.ErrUserNotFound)` (as planned in + `features/article_title_rules.qa.md` step 1), or seed the user, then delete it from the fake after + the middleware step via a hook. +3. Any other type implementing `user.Repository` in `test/` must gain the method so the package + compiles. + +## Setup shared by every HTTP case + +4. Call `validator.InitValidator()`. +5. `getUser := useCases.NewGetUserUseCase(userRepo)`, `update := + useCases.NewUpdateUserProfileUseCase(userRepo)`, `userHandler := user.NewHandler(update)` (import + `RenewCMS/api/handlers/user` under an alias that does not collide with the domain `user` + package), `meHandler := auth.NewMeHandler(getUser)`. +6. Fresh `chi.NewRouter()` mirroring `api/router.go`, with `token := jwtauth.New("HS256", + []byte("test-secret"), nil)`: + ```go + r.Route("/v1/user", func(r chi.Router) { + r.Use(jwtauth.Verifier(token)) + r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireCurrentToken(getUser)) + r.Use(middleware.RequireVerifiedUser(getUser)) + r.Patch("/me", userHandler.UpdateMe) + }) + r.Route("/v1/auth", func(r chi.Router) { + r.Use(jwtauth.Verifier(token)) + r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireCurrentToken(getUser)) + r.Get("/me", meHandler.Me) + }) + ``` + Add the public `/v1/auth/login` route (real `auth.NewLoginHandler` over the fakes, as in the + existing login tests) for the login case. The handler method names above are indicative; use + whatever the handlers expose. +7. Mint cookies with `token.Encode(map[string]any{"user_id": uint32(id), "token_version": + uint32(v)})` set as the `jwt` cookie. +8. Decode success bodies into `map[string]json.RawMessage` (not `user.User`) so key sets can be + asserted. Error bodies are plain text from `http.Error` and carry a trailing `"\n"`. + +## Use-case checks (fake repo, no HTTP) + +9. `useCases.NewUpdateUserProfileUseCase(repo)` with user id 7 seeded, `DisplayName: "Old"`: + - `" Marta "` -> `DisplayName "Marta"` returned and stored. + - `""` and `" "` -> `DisplayName ""` stored (cleared). + - `strings.Repeat("😀", 60)` -> success. `strings.Repeat("😀", 61)` -> `errors.Is(err, + user.ErrInvalidDisplayName)`, `updateDisplayNameCalls == 0`, stored value still `"Old"`. + - Unknown id 99 -> `errors.Is(err, user.ErrUserNotFound)`. + - Repository failure via `failNext("UpdateDisplayName")` -> the error is returned unchanged. + +## HTTP checks + +10. Seed user 7 (`marta`, `marta@renew.example`, `IsVerified: true`, `DisplayName: ""`, + `TokenVersion: 0`) with a non-empty `Password` hash. `PATCH /v1/user/me` + `{"display_name":"Marta Alves"}` -> `200`. Body keys are exactly `{id, username, email, + is_verified, created_at, updated_at, display_name}`. `display_name` is `"Marta Alves"`, + `username` is `"marta"`. The repo holds `"Marta Alves"`. The stored `Password` and + `TokenVersion` are unchanged. The same cookie then gets `200` from `GET /v1/auth/me`. +11. Trim table: `" Marta Alves "` and `"\tMarta Alves\n"` -> `"Marta Alves"`; `"Marta Alves"` + keeps its interior spaces. +12. Clear table (seed `DisplayName: "Marta Alves"`): `""`, `" "`, `"\t \n"` -> `200`, response and + repo `display_name ""`, `username` still `"marta"`. +13. Length table (seed `""`): 60 x `"a"` -> `200`; 61 x `"a"` -> `400` with body + `apperr.DisplayNameErrorMessage + "\n"`; 60 x `"😀"` -> `200`; 61 x `"😀"` -> `400`; 60 x `"é"` + -> `200`. Assert `updateDisplayNameCalls == 0` on every `400` and that the stored value is + unchanged. +14. Trim-before-count: `strings.Repeat(" ", 5) + strings.Repeat("a", 60) + strings.Repeat(" ", 5)` + -> `200`, returned value is the 60 `"a"`. +15. Control characters: `{"display_name":"Ma\nrta"}` -> `200`, returned and stored value is + `"Ma\nrta"`. +16. Malformed table (seed `DisplayName: "Marta Alves"`): raw text `not json`, `{}`, + `{"display_name":null}`, `{"display_name":42}`, `{"displayName":"Marta"}` -> each `400`, + `updateDisplayNameCalls == 0`, stored value unchanged. Do not assert an exact body for these. +17. `GET /v1/auth/me` for user 7 with `DisplayName "Marta Alves"` -> `200`, `display_name` is + `"Marta Alves"`. For user 8 (`IsVerified: false`, `DisplayName ""`) -> key set is exactly the + seven keys and `display_name` is `""`, not absent. +18. Login: seed verified user `marta` with a stub-hashed password and `DisplayName: "Ines Duarte"`. + `POST /v1/auth/login` with username `"Ines Duarte"` -> `403`, body + `apperr.LogsErrorMessage + "\n"`. With username `"marta"` -> `200`. Confirm the login handler's + real status for an unknown username before asserting `403`. +19. Unverified user 8 with matching cookie -> `PATCH` -> `403`, body + `apperr.VerificationRequiredErrorMessage + "\n"`, `updateDisplayNameCalls == 0`. +20. No cookie -> `401` (do not assert the body; produced by `jwtauth.Authenticator`), + `updateDisplayNameCalls == 0`. +21. User 7 with `TokenVersion: 1`, cookie encoding `token_version 0` -> `401`, body + `apperr.UnauthorizedErrorMessage + "\n"`, `updateDisplayNameCalls == 0`. +22. Cookie for user 999 that does not exist -> `401`, same body. +23. `failNext("UpdateDisplayName")` (generic error) with a valid verified user -> `500`, body + `"Internal server error\n"`. +24. Repository returns `user.ErrUserNotFound` from `UpdateDisplayName` only (middleware passes) -> + `401`, body `apperr.UnauthorizedErrorMessage + "\n"`. +25. Sweep for secrets: on every `200` PATCH body and on `GET /v1/auth/me`, reuse + `assertNoForbiddenKeys` from `test/user_response_field_hygiene_test.go`. Add `token_version` to + that helper's forbidden keys if it does not check it yet. + +## Edits to existing tests required by the six to seven field change + +26. `test/user_response_field_hygiene_test.go` (`expectedKeys` in the verify test, around line 127): + add `"display_name"`. The count check then expects seven. The seeded user 7 has no display + name, so also assert `display_name` decodes to `""`. +27. `test/get_current_user_http_test.go` decodes into the public user struct; update its import to + the shared package. Step 17 is the authoritative shape check for `/me`. + +## Manual verification (not exercised by the Go test) + +- Start an existing dev SQLite file created before this slice, boot the server so `AutoMigrate` + runs, and confirm `GET /v1/auth/me` for the pre-existing user returns `display_name: ""` and that + login still works. Then `PATCH`, restart, and confirm the value persisted. +- Confirm by inspection that the `internal/domain/user` struct has no `gorm` tag on `DisplayName`, + the GORM tag with the empty-string default sits in `persistence/models`, `persistence/mappers` + maps both directions, and the model list in `api/dependecyInjection.go` needs no new entry (same + model, new column). +- Confirm that the new handler package constructs nothing and only receives `UpdateUserProfile` + via `NewHandler(...)`, that `dependecyInjection.go` wires the use case into the `*UseCases` + container, and that neither handler package imports the other. +- No SMTP or real embedded frontend is involved. The SPA is out of scope. diff --git a/features/user_response_field_hygiene.feature b/features/user_response_field_hygiene.feature index a09283e..91fa5f8 100644 --- a/features/user_response_field_hygiene.feature +++ b/features/user_response_field_hygiene.feature @@ -7,11 +7,11 @@ Feature: API responses never expose a user's secret fields credential or a still-valid reset/verification secret Background: - Given the public shape of a user in any API response is exactly six fields: id, username, - email, is_verified, created_at and updated_at - And password, password_reset_code, verification_code, verification_expiration and - password_reset_expiration never appear in any response body, under any key name, regardless - of their value or whether they are set + Given the public shape of a user in any API response is exactly seven fields: id, username, + email, is_verified, created_at, updated_at and display_name + And password, password_reset_code, verification_code, verification_expiration, + password_reset_expiration and token_version never appear in any response body, under any key + name, regardless of their value or whether they are set Scenario: A successful verification response carries only public fields Given a user repository containing user id 7, username "alice", email "alice@example.com", @@ -21,10 +21,11 @@ Feature: API responses never expose a user's secret fields When the client sends POST /v1/auth/verify with body {"code": "abc123"} Then the response status is 200 And the response body is a JSON object with exactly the keys id, username, email, - is_verified, created_at and updated_at + is_verified, created_at, updated_at and display_name And the response body does not contain the keys password, password_reset_code, - verification_code, verification_expiration or password_reset_expiration + verification_code, verification_expiration, password_reset_expiration or token_version And the response body's is_verified is true + And the response body's display_name is "" Scenario: A rejected verification attempt never echoes any user field Given a user repository containing user id 7, username "alice", is_verified false, whose diff --git a/features/user_response_field_hygiene.qa.md b/features/user_response_field_hygiene.qa.md index 037acf1..968e491 100644 --- a/features/user_response_field_hygiene.qa.md +++ b/features/user_response_field_hygiene.qa.md @@ -21,10 +21,11 @@ Package: `test` (black-box), extending `test/account_verification_http_test.go`' user id 7. Decode the response body into `map[string]json.RawMessage` (not into `user.User`) and assert: - the key set is exactly `{"id", "username", "email", "is_verified", "created_at", - "updated_at"}`; + "updated_at", "display_name"}`; - none of `password`, `password_reset_code`, `verification_code`, - `verification_expiration`, `password_reset_expiration` is present; - - `is_verified` unmarshals to `true`. + `verification_expiration`, `password_reset_expiration`, `token_version` is present; + - `is_verified` unmarshals to `true`; + - `display_name` unmarshals to `""`. 5. Rejected verify: same seed, `{"code":"wrong-code"}` → assert status `403` and that the raw response body bytes, treated as a string, contain none of the substrings `password`, `verification_code`, `password_reset_code`, `verification_expiration`, @@ -55,6 +56,6 @@ Package: `test` (black-box), extending `test/account_verification_http_test.go`' render; confirm by hand that no SPA code reads `password`, `verification_code` or `password_reset_code` off the verify response, since removing them from the wire format could otherwise silently break a screen that isn't covered by this Go test suite. -- Confirm the new response type lives in `api/handlers/auth` (not a change to +- Confirm the public response type lives in the shared package under `api/pkg` (not a change to `internal/domain/user/userDomain.go`'s json tags), per the project's stated preference to keep the domain struct's tags intact and shape the public response in the handler layer instead. diff --git a/internal/domain/user/userDomain.go b/internal/domain/user/userDomain.go index 75a65d4..c7e9fb9 100644 --- a/internal/domain/user/userDomain.go +++ b/internal/domain/user/userDomain.go @@ -13,6 +13,7 @@ var ErrVerificationCodeExpired = errors.New("verification code expired") var ErrAlreadyVerified = errors.New("user already verified") var ErrInvalidPasswordResetCode = errors.New("invalid password reset code") var ErrPasswordResetCodeExpired = errors.New("password reset code expired") +var ErrInvalidDisplayName = errors.New("invalid display name") type User struct { ID uint32 `json:"id"` @@ -20,6 +21,7 @@ type User struct { Password string `json:"password"` PasswordResetCode string `json:"password_reset_code"` Email string `json:"email"` + DisplayName string `json:"display_name"` IsVerified bool `json:"is_verified"` VerificationCode string `json:"verification_code"` VerificationExpiration time.Time `json:"verification_expiration"` diff --git a/internal/domain/user/userRepositoryInterface.go b/internal/domain/user/userRepositoryInterface.go index 880ed43..01e4860 100644 --- a/internal/domain/user/userRepositoryInterface.go +++ b/internal/domain/user/userRepositoryInterface.go @@ -14,4 +14,5 @@ type Repository interface { UpdatePasswordResetCode(userId uint32, code string, expiration time.Time) (User, error) ClearVerificationCode(userId uint32) (User, error) ResetPassword(userId uint32, hashedPassword string) (User, error) + UpdateDisplayName(userId uint32, displayName string) (User, error) } diff --git a/internal/infrastructure/persistence/mappers/userMapper.go b/internal/infrastructure/persistence/mappers/userMapper.go index decc969..7723138 100644 --- a/internal/infrastructure/persistence/mappers/userMapper.go +++ b/internal/infrastructure/persistence/mappers/userMapper.go @@ -26,6 +26,7 @@ func UserToDomain(m models.User) user.User { Password: m.Password, PasswordResetCode: nullableStringToDomain(m.PasswordResetCode), Email: m.Email, + DisplayName: m.DisplayName, IsVerified: m.IsVerified, VerificationCode: nullableStringToDomain(m.VerificationCode), VerificationExpiration: m.VerificationExpiration, @@ -42,6 +43,7 @@ func UserToModel(a user.User) models.User { Password: a.Password, PasswordResetCode: DomainToNullableString(a.PasswordResetCode), Email: a.Email, + DisplayName: a.DisplayName, IsVerified: a.IsVerified, VerificationCode: DomainToNullableString(a.VerificationCode), VerificationExpiration: a.VerificationExpiration, diff --git a/internal/infrastructure/persistence/models/userModel.go b/internal/infrastructure/persistence/models/userModel.go index 068b468..c411daf 100644 --- a/internal/infrastructure/persistence/models/userModel.go +++ b/internal/infrastructure/persistence/models/userModel.go @@ -10,6 +10,7 @@ type User struct { Password string `gorm:"not null"` PasswordResetCode *string `gorm:"unique"` Email string `gorm:"unique;not null"` + DisplayName string `gorm:"not null;default:''"` IsVerified bool `gorm:"default=false;not null"` VerificationCode *string `gorm:"unique"` VerificationExpiration time.Time diff --git a/internal/infrastructure/persistence/repositories/userRepository.go b/internal/infrastructure/persistence/repositories/userRepository.go index d0fd335..f8b985c 100644 --- a/internal/infrastructure/persistence/repositories/userRepository.go +++ b/internal/infrastructure/persistence/repositories/userRepository.go @@ -169,4 +169,20 @@ func (u *UserRepository) ResetPassword(userId uint32, hashedPassword string) (do return mappers.UserToDomain(localUser), nil } +func (u *UserRepository) UpdateDisplayName(userId uint32, displayName string) (domainUser.User, error) { + var localUser entity.User + err := u.db.Model(&entity.User{}).First(&localUser, userId).Error + if err != nil { + return domainUser.User{}, mapNotFoundErr(err, domainUser.ErrUserNotFound) + } + + localUser.DisplayName = displayName + err = u.db.Save(&localUser).Error + if err != nil { + return domainUser.User{}, err + } + + return mappers.UserToDomain(localUser), nil +} + var _ domainUser.Repository = &UserRepository{} diff --git a/internal/infrastructure/useCases/UpdateUserProfile.go b/internal/infrastructure/useCases/UpdateUserProfile.go new file mode 100644 index 0000000..bb1e7f4 --- /dev/null +++ b/internal/infrastructure/useCases/UpdateUserProfile.go @@ -0,0 +1,25 @@ +package useCases + +import ( + "RenewCMS/internal/domain/user" + "strings" + "unicode/utf8" +) + +const maxDisplayNameRunes = 60 + +type UpdateUserProfileUseCase struct { + userRepository user.Repository +} + +func NewUpdateUserProfileUseCase(userRepository user.Repository) *UpdateUserProfileUseCase { + return &UpdateUserProfileUseCase{userRepository} +} + +func (u *UpdateUserProfileUseCase) UpdateDisplayName(userId uint32, displayName string) (user.User, error) { + trimmed := strings.TrimSpace(displayName) + if utf8.RuneCountInString(trimmed) > maxDisplayNameRunes { + return user.User{}, user.ErrInvalidDisplayName + } + return u.userRepository.UpdateDisplayName(userId, trimmed) +} diff --git a/test/get_current_user_http_test.go b/test/get_current_user_http_test.go index b1b7737..3562855 100644 --- a/test/get_current_user_http_test.go +++ b/test/get_current_user_http_test.go @@ -4,6 +4,7 @@ import ( "RenewCMS/api/handlers/auth" "RenewCMS/api/middleware" "RenewCMS/api/pkg/apperr" + "RenewCMS/api/pkg/publicuser" validator "RenewCMS/api/pkg/validator" "RenewCMS/internal/domain/user" "RenewCMS/internal/infrastructure/useCases" @@ -81,7 +82,7 @@ func TestGetCurrentUserHttp(t *testing.T) { if got := rec.Header().Get("Cache-Control"); got != "no-store" { t.Errorf("expected Cache-Control no-store, got %q", got) } - var body auth.PublicUser + var body publicuser.PublicUser if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { t.Fatalf("failed to unmarshal body: %v", err) } @@ -111,7 +112,7 @@ func TestGetCurrentUserHttp(t *testing.T) { if got := rec.Header().Get("Cache-Control"); got != "no-store" { t.Errorf("expected Cache-Control no-store, got %q", got) } - var body auth.PublicUser + var body publicuser.PublicUser if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { t.Fatalf("failed to unmarshal body: %v", err) } diff --git a/test/user_display_name_test.go b/test/user_display_name_test.go new file mode 100644 index 0000000..35f52b0 --- /dev/null +++ b/test/user_display_name_test.go @@ -0,0 +1,471 @@ +package test + +import ( + "RenewCMS/api/handlers/auth" + userhandler "RenewCMS/api/handlers/user" + "RenewCMS/api/middleware" + "RenewCMS/api/pkg/apperr" + validator "RenewCMS/api/pkg/validator" + "RenewCMS/internal/domain/user" + "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 displayNameResponseKeys = []string{"id", "username", "email", "is_verified", "created_at", "updated_at", "display_name"} + +func newDisplayNameRouter(userRepo *mockUserRepo, token *jwtauth.JWTAuth) *chi.Mux { + hasher := &stubHasher{} + getUser := useCases.NewGetUserUseCase(userRepo) + update := useCases.NewUpdateUserProfileUseCase(userRepo) + handler := userhandler.NewHandler(update) + meHandler := auth.NewMeHandler(getUser) + loginHandler := auth.NewLoginHandler( + token, + useCases.NewCreateUserUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), + useCases.NewAuthenticateUserUseCase(userRepo, hasher), + true, + ) + + r := chi.NewRouter() + r.Route("/v1/user", func(r chi.Router) { + r.Use(jwtauth.Verifier(token)) + r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireCurrentToken(getUser)) + r.Use(middleware.RequireVerifiedUser(getUser)) + r.Patch("/me", handler.UpdateMe) + }) + r.Route("/v1/auth", func(r chi.Router) { + r.Post("/login", loginHandler.Login) + r.Group(func(r chi.Router) { + r.Use(jwtauth.Verifier(token)) + r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireCurrentToken(getUser)) + r.Get("/me", meHandler.Me) + }) + }) + return r +} + +func displayNameCookie(t *testing.T, token *jwtauth.JWTAuth, userId uint32, tokenVersion uint32) *http.Cookie { + t.Helper() + _, tokenString, err := token.Encode(map[string]any{"user_id": userId, "token_version": tokenVersion}) + if err != nil { + t.Fatalf("failed to encode jwt: %v", err) + } + return &http.Cookie{Name: "jwt", Value: tokenString} +} + +func sendDisplayNameRequest(router *chi.Mux, method, target, body string, cookie *http.Cookie) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, target, bytes.NewBufferString(body)) + if cookie != nil { + req.AddCookie(cookie) + } + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec +} + +func displayNameBody(t *testing.T, value string) string { + t.Helper() + encoded, err := json.Marshal(map[string]string{"display_name": value}) + if err != nil { + t.Fatalf("failed to encode body: %v", err) + } + return string(encoded) +} + +func seededDisplayNameRepo(displayName string) *mockUserRepo { + repo := newMockUserRepo() + repo.users[7] = user.User{ + ID: 7, + Username: "marta", + Email: "marta@renew.example", + Password: "hashed:correct-horse", + IsVerified: true, + DisplayName: displayName, + } + return repo +} + +func decodeKeys(t *testing.T, body []byte) map[string]json.RawMessage { + t.Helper() + var decoded map[string]json.RawMessage + if err := json.Unmarshal(body, &decoded); err != nil { + t.Fatalf("failed to decode body %q: %v", string(body), err) + } + return decoded +} + +func assertSevenKeys(t *testing.T, body []byte) map[string]json.RawMessage { + t.Helper() + decoded := decodeKeys(t, body) + if len(decoded) != len(displayNameResponseKeys) { + t.Errorf("expected exactly %d keys, got %d: %s", len(displayNameResponseKeys), len(decoded), string(body)) + } + for _, key := range displayNameResponseKeys { + if _, present := decoded[key]; !present { + t.Errorf("expected key %q in body %s", key, string(body)) + } + } + assertNoForbiddenKeys(t, body) + return decoded +} + +func decodedString(t *testing.T, decoded map[string]json.RawMessage, key string) string { + t.Helper() + var value string + if err := json.Unmarshal(decoded[key], &value); err != nil { + t.Fatalf("failed to decode %s: %v", key, err) + } + return value +} + +func TestUpdateUserProfileUseCase(t *testing.T) { + newRepo := func() *mockUserRepo { + repo := newMockUserRepo() + repo.users[7] = user.User{ID: 7, DisplayName: "Old"} + return repo + } + + successCases := []struct { + name string + input string + want string + }{ + {"trims surrounding whitespace", " Marta ", "Marta"}, + {"empty clears", "", ""}, + {"whitespace only clears", " ", ""}, + {"60 emoji runes", strings.Repeat("😀", 60), strings.Repeat("😀", 60)}, + } + for _, tc := range successCases { + t.Run(tc.name, func(t *testing.T) { + repo := newRepo() + got, err := useCases.NewUpdateUserProfileUseCase(repo).UpdateDisplayName(7, tc.input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.DisplayName != tc.want || repo.users[7].DisplayName != tc.want { + t.Errorf("expected %q, got %q stored %q", tc.want, got.DisplayName, repo.users[7].DisplayName) + } + }) + } + + t.Run("61 emoji runes are refused without calling the repository", func(t *testing.T) { + repo := newRepo() + _, err := useCases.NewUpdateUserProfileUseCase(repo).UpdateDisplayName(7, strings.Repeat("😀", 61)) + if !errors.Is(err, user.ErrInvalidDisplayName) { + t.Fatalf("expected ErrInvalidDisplayName, got %v", err) + } + if repo.updateDisplayNameCalls != 0 || repo.users[7].DisplayName != "Old" { + t.Errorf("repository must be untouched") + } + }) + + t.Run("unknown user", func(t *testing.T) { + _, err := useCases.NewUpdateUserProfileUseCase(newRepo()).UpdateDisplayName(99, "Marta") + if !errors.Is(err, user.ErrUserNotFound) { + t.Fatalf("expected ErrUserNotFound, got %v", err) + } + }) + + t.Run("repository failure is returned unchanged", func(t *testing.T) { + repo := newRepo() + repo.fail.failNext("UpdateDisplayName") + _, err := useCases.NewUpdateUserProfileUseCase(repo).UpdateDisplayName(7, "Marta") + if !errors.Is(err, errRepositoryFailure) { + t.Fatalf("expected repository failure, got %v", err) + } + }) +} + +func TestUserDisplayNameHttp(t *testing.T) { + validator.InitValidator() + token := jwtauth.New("HS256", []byte("test-secret"), nil) + + patch := func(t *testing.T, repo *mockUserRepo, body string) *httptest.ResponseRecorder { + t.Helper() + router := newDisplayNameRouter(repo, token) + return sendDisplayNameRequest(router, http.MethodPatch, "/v1/user/me", body, displayNameCookie(t, token, 7, 0)) + } + + t.Run("A verified user sets a display name", func(t *testing.T) { + repo := seededDisplayNameRepo("") + router := newDisplayNameRouter(repo, token) + cookie := displayNameCookie(t, token, 7, 0) + + rec := sendDisplayNameRequest(router, http.MethodPatch, "/v1/user/me", `{"display_name": "Marta Alves"}`, cookie) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + decoded := assertSevenKeys(t, rec.Body.Bytes()) + var id uint32 + _ = json.Unmarshal(decoded["id"], &id) + if id != 7 || decodedString(t, decoded, "username") != "marta" || decodedString(t, decoded, "display_name") != "Marta Alves" { + t.Errorf("unexpected body: %s", rec.Body.String()) + } + if repo.users[7].DisplayName != "Marta Alves" || repo.users[7].Username != "marta" { + t.Errorf("unexpected stored user: %+v", repo.users[7]) + } + }) + + trimCases := []struct{ name, sent, returned string }{ + {"spaces", " Marta Alves ", "Marta Alves"}, + {"tab and newline", "\tMarta Alves\n", "Marta Alves"}, + {"interior spaces kept", "Marta Alves", "Marta Alves"}, + } + for _, tc := range trimCases { + t.Run("The value is trimmed before it is stored and returned: "+tc.name, func(t *testing.T) { + repo := seededDisplayNameRepo("") + rec := patch(t, repo, displayNameBody(t, tc.sent)) + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + if got := decodedString(t, decodeKeys(t, rec.Body.Bytes()), "display_name"); got != tc.returned { + t.Errorf("expected %q, got %q", tc.returned, got) + } + if repo.users[7].DisplayName != tc.returned { + t.Errorf("expected stored %q, got %q", tc.returned, repo.users[7].DisplayName) + } + }) + } + + for _, sent := range []string{"", " ", "\t \n"} { + t.Run("An empty or whitespace-only value clears the display name: "+displayNameBody(t, sent), func(t *testing.T) { + repo := seededDisplayNameRepo("Marta Alves") + rec := patch(t, repo, displayNameBody(t, sent)) + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + decoded := decodeKeys(t, rec.Body.Bytes()) + if decodedString(t, decoded, "display_name") != "" || decodedString(t, decoded, "username") != "marta" { + t.Errorf("unexpected body: %s", rec.Body.String()) + } + if repo.users[7].DisplayName != "" { + t.Errorf("expected cleared, got %q", repo.users[7].DisplayName) + } + }) + } + + limitCases := []struct { + name string + count int + unit string + status int + }{ + {"60 a", 60, "a", http.StatusOK}, + {"61 a", 61, "a", http.StatusBadRequest}, + {"60 emoji", 60, "😀", http.StatusOK}, + {"61 emoji", 61, "😀", http.StatusBadRequest}, + {"60 e-acute", 60, "é", http.StatusOK}, + } + for _, tc := range limitCases { + t.Run("The limit is 60 Unicode characters, counted as runes: "+tc.name, func(t *testing.T) { + repo := seededDisplayNameRepo("") + value := strings.Repeat(tc.unit, tc.count) + rec := patch(t, repo, displayNameBody(t, value)) + if rec.Code != tc.status { + t.Fatalf("expected %d, got %d: %s", tc.status, rec.Code, rec.Body.String()) + } + if tc.status == http.StatusOK { + if repo.users[7].DisplayName != value { + t.Errorf("value not stored") + } + return + } + if rec.Body.String() != apperr.DisplayNameErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + if repo.users[7].DisplayName != "" || repo.updateDisplayNameCalls != 0 { + t.Errorf("repository must be untouched") + } + }) + } + + t.Run("The limit applies after trimming, not before", func(t *testing.T) { + repo := seededDisplayNameRepo("") + core := strings.Repeat("a", 60) + rec := patch(t, repo, displayNameBody(t, strings.Repeat(" ", 5)+core+strings.Repeat(" ", 5))) + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + if got := decodedString(t, decodeKeys(t, rec.Body.Bytes()), "display_name"); got != core { + t.Errorf("unexpected display_name %q", got) + } + }) + + t.Run("Control characters inside the name are kept", func(t *testing.T) { + repo := seededDisplayNameRepo("") + rec := patch(t, repo, `{"display_name": "Ma\nrta"}`) + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + if got := decodedString(t, decodeKeys(t, rec.Body.Bytes()), "display_name"); got != "Ma\nrta" { + t.Errorf("unexpected display_name %q", got) + } + if repo.users[7].DisplayName != "Ma\nrta" { + t.Errorf("unexpected stored %q", repo.users[7].DisplayName) + } + }) + + t.Run("A rejected value leaves the existing display name untouched", func(t *testing.T) { + repo := seededDisplayNameRepo("Marta Alves") + rec := patch(t, repo, displayNameBody(t, strings.Repeat("a", 61))) + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d", rec.Code) + } + if repo.updateDisplayNameCalls != 0 || repo.users[7].DisplayName != "Marta Alves" { + t.Errorf("repository must be untouched") + } + }) + + malformedBodies := []string{"not json", `{}`, `{"display_name": null}`, `{"display_name": 42}`, `{"displayName": "Marta"}`} + for _, body := range malformedBodies { + t.Run("A malformed body is refused with 400: "+body, func(t *testing.T) { + repo := seededDisplayNameRepo("Marta Alves") + rec := patch(t, repo, body) + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", rec.Code, rec.Body.String()) + } + if repo.updateDisplayNameCalls != 0 || repo.users[7].DisplayName != "Marta Alves" { + t.Errorf("repository must be untouched") + } + }) + } + + t.Run("The password, token version and login username are unaffected by a display name", func(t *testing.T) { + repo := seededDisplayNameRepo("") + router := newDisplayNameRouter(repo, token) + cookie := displayNameCookie(t, token, 7, 0) + + rec := sendDisplayNameRequest(router, http.MethodPatch, "/v1/user/me", `{"display_name": "Marta Alves"}`, cookie) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + if repo.users[7].Password != "hashed:correct-horse" || repo.users[7].TokenVersion != 0 { + t.Errorf("password or token version changed: %+v", repo.users[7]) + } + me := sendDisplayNameRequest(router, http.MethodGet, "/v1/auth/me", "", cookie) + if me.Code != http.StatusOK { + t.Errorf("expected 200 from /me, got %d", me.Code) + } + }) + + t.Run("GET /v1/auth/me returns the display name once set", func(t *testing.T) { + repo := seededDisplayNameRepo("Marta Alves") + router := newDisplayNameRouter(repo, token) + rec := sendDisplayNameRequest(router, http.MethodGet, "/v1/auth/me", "", displayNameCookie(t, token, 7, 0)) + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } + decoded := assertSevenKeys(t, rec.Body.Bytes()) + if decodedString(t, decoded, "display_name") != "Marta Alves" || decodedString(t, decoded, "username") != "marta" { + t.Errorf("unexpected body: %s", rec.Body.String()) + } + }) + + t.Run("GET /v1/auth/me returns an empty display_name, not a missing key, when none is set", func(t *testing.T) { + repo := newMockUserRepo() + repo.users[8] = user.User{ID: 8, Username: "bob", IsVerified: false} + router := newDisplayNameRouter(repo, token) + rec := sendDisplayNameRequest(router, http.MethodGet, "/v1/auth/me", "", displayNameCookie(t, token, 8, 0)) + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } + decoded := assertSevenKeys(t, rec.Body.Bytes()) + if decodedString(t, decoded, "display_name") != "" { + t.Errorf("expected empty display_name") + } + }) + + t.Run("Logging in still uses the username only", func(t *testing.T) { + repo := seededDisplayNameRepo("Ines Duarte") + router := newDisplayNameRouter(repo, token) + + rec := sendDisplayNameRequest(router, http.MethodPost, "/v1/auth/login", `{"username":"Ines Duarte","password":"correct-horse"}`, nil) + if rec.Code != http.StatusForbidden || rec.Body.String() != apperr.LogsErrorMessage+"\n" { + t.Errorf("expected 403 logs error, got %d: %q", rec.Code, rec.Body.String()) + } + rec = sendDisplayNameRequest(router, http.MethodPost, "/v1/auth/login", `{"username":"marta","password":"correct-horse"}`, nil) + if rec.Code != http.StatusOK { + t.Errorf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + }) + + t.Run("An unverified user is refused", func(t *testing.T) { + repo := newMockUserRepo() + repo.users[8] = user.User{ID: 8, Username: "bob", IsVerified: false} + router := newDisplayNameRouter(repo, token) + rec := sendDisplayNameRequest(router, http.MethodPatch, "/v1/user/me", `{"display_name": "Bob"}`, displayNameCookie(t, token, 8, 0)) + if rec.Code != http.StatusForbidden || rec.Body.String() != apperr.VerificationRequiredErrorMessage+"\n" { + t.Errorf("expected 403 verification required, got %d: %q", rec.Code, rec.Body.String()) + } + if repo.updateDisplayNameCalls != 0 || repo.users[8].DisplayName != "" { + t.Errorf("repository must be untouched") + } + }) + + t.Run("No jwt cookie is refused", func(t *testing.T) { + repo := seededDisplayNameRepo("") + router := newDisplayNameRouter(repo, token) + rec := sendDisplayNameRequest(router, http.MethodPatch, "/v1/user/me", `{"display_name": "Marta"}`, nil) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected 401, got %d", rec.Code) + } + if repo.updateDisplayNameCalls != 0 { + t.Errorf("repository must be untouched") + } + }) + + t.Run("A stale jwt is refused", func(t *testing.T) { + repo := seededDisplayNameRepo("") + u := repo.users[7] + u.TokenVersion = 1 + repo.users[7] = u + router := newDisplayNameRouter(repo, token) + rec := sendDisplayNameRequest(router, http.MethodPatch, "/v1/user/me", `{"display_name": "Marta"}`, displayNameCookie(t, token, 7, 0)) + if rec.Code != http.StatusUnauthorized || rec.Body.String() != apperr.UnauthorizedErrorMessage+"\n" { + t.Errorf("expected 401 unauthorized, got %d: %q", rec.Code, rec.Body.String()) + } + if repo.updateDisplayNameCalls != 0 { + t.Errorf("repository must be untouched") + } + }) + + t.Run("A jwt minted for a since-deleted account is refused", func(t *testing.T) { + repo := newMockUserRepo() + router := newDisplayNameRouter(repo, token) + rec := sendDisplayNameRequest(router, http.MethodPatch, "/v1/user/me", `{"display_name": "Marta"}`, displayNameCookie(t, token, 999, 0)) + if rec.Code != http.StatusUnauthorized || rec.Body.String() != apperr.UnauthorizedErrorMessage+"\n" { + t.Errorf("expected 401 unauthorized, got %d: %q", rec.Code, rec.Body.String()) + } + }) + + t.Run("A repository failure while saving answers 500", func(t *testing.T) { + repo := seededDisplayNameRepo("") + repo.fail.failNext("UpdateDisplayName") + rec := patch(t, repo, `{"display_name": "Marta"}`) + if rec.Code != http.StatusInternalServerError || rec.Body.String() != "Internal server error\n" { + t.Errorf("expected 500, got %d: %q", rec.Code, rec.Body.String()) + } + }) + + t.Run("The user is deleted between authentication and the save", func(t *testing.T) { + repo := seededDisplayNameRepo("") + repo.fail.failNextWith("UpdateDisplayName", user.ErrUserNotFound) + rec := patch(t, repo, `{"display_name": "Marta"}`) + if rec.Code != http.StatusUnauthorized || rec.Body.String() != apperr.UnauthorizedErrorMessage+"\n" { + t.Errorf("expected 401 unauthorized, got %d: %q", rec.Code, rec.Body.String()) + } + }) +} diff --git a/test/user_response_field_hygiene_test.go b/test/user_response_field_hygiene_test.go index 1fa53b6..7bb2c33 100644 --- a/test/user_response_field_hygiene_test.go +++ b/test/user_response_field_hygiene_test.go @@ -76,6 +76,7 @@ var forbiddenUserFieldKeys = []string{ "verification_code", "verification_expiration", "password_reset_expiration", + "token_version", } func assertNoForbiddenKeys(t *testing.T, body []byte) { @@ -124,7 +125,7 @@ func TestUserResponseFieldHygiene(t *testing.T) { t.Fatalf("failed to decode response: %v", err) } - expectedKeys := []string{"id", "username", "email", "is_verified", "created_at", "updated_at"} + expectedKeys := []string{"id", "username", "email", "is_verified", "created_at", "updated_at", "display_name"} if len(decoded) != len(expectedKeys) { t.Errorf("expected exactly %d keys, got %d: %v", len(expectedKeys), len(decoded), decoded) } @@ -135,6 +136,11 @@ func TestUserResponseFieldHygiene(t *testing.T) { } assertNoForbiddenKeys(t, rec.Body.Bytes()) + var displayName string + if err := json.Unmarshal(decoded["display_name"], &displayName); err != nil || displayName != "" { + t.Errorf("expected display_name to decode to an empty string, got %q (%v)", displayName, err) + } + var isVerified bool if err := json.Unmarshal(decoded["is_verified"], &isVerified); err != nil { t.Fatalf("failed to decode is_verified: %v", err) diff --git a/test/user_test.go b/test/user_test.go index 7ccb725..8b1fd29 100644 --- a/test/user_test.go +++ b/test/user_test.go @@ -44,6 +44,8 @@ type mockUserRepo struct { resetPasswordCalls int lastResetPasswordUserId uint32 lastResetPasswordHash string + updateDisplayNameCalls int + lastUpdateDisplayName string } func newMockUserRepo() *mockUserRepo { @@ -210,6 +212,21 @@ func (m *mockUserRepo) ResetPassword(userId uint32, hashedPassword string) (user return u, nil } +func (m *mockUserRepo) UpdateDisplayName(userId uint32, displayName string) (user.User, error) { + m.updateDisplayNameCalls++ + m.lastUpdateDisplayName = displayName + if err := m.fail.trigger("UpdateDisplayName"); err != nil { + return user.User{}, err + } + u, ok := m.users[userId] + if !ok { + return user.User{}, user.ErrUserNotFound + } + u.DisplayName = displayName + m.users[userId] = u + return u, nil +} + var _ user.Repository = &mockUserRepo{} func TestCreateUser(t *testing.T) {