fix: API responses never include a user's password, verification code or reset code hashes, or their expiry dates; verifying an account returns only the public user fields

This commit is contained in:
fsylvain committed 2026-09-24 15:39:50 +02:00
1 parent aa865af2e9
commit 2ee28203ab
4 files changed
+389 -1

No files matched your search

+25 -1
View File
@@ -52,6 +52,26 @@ type ResetPasswordCredentials struct {
Password string `json:"password" validate:"required,min=8"`
}
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 (h *Handler) removeJwtCookie(w http.ResponseWriter) {
http.SetCookie(w, &http.Cookie{
Name: "jwt",
@@ -205,7 +225,11 @@ func (h *Handler) Verify(w http.ResponseWriter, r *http.Request) {
return
}
body, _ := json.Marshal(verifiedUser)
body, err := json.Marshal(newPublicUser(verifiedUser))
if err != nil {
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
_, _ = w.Write(body)
}
@@ -0,0 +1,61 @@
Feature: API responses never expose a user's secret fields
As the maintainer of RenewCMS
I want every HTTP response that includes user data to carry only public fields, never a
password hash, a verification or password-reset code hash, or their expiry timestamps
So that a bug, a future endpoint, or a client inspecting a response body can never recover a
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
Scenario: A successful verification response carries only public fields
Given a user repository containing user id 7, username "alice", email "alice@example.com",
is_verified false, whose stored VerificationCode is the bcrypt hash of "abc123" and whose
VerificationExpiration is one hour from now
And a valid jwt cookie for user id 7
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
And the response body does not contain the keys password, password_reset_code,
verification_code, verification_expiration or password_reset_expiration
And the response body's is_verified is true
Scenario: A rejected verification attempt never echoes any user field
Given a user repository containing user id 7, username "alice", is_verified false, whose
stored VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is
one hour from now
And a valid jwt cookie for user id 7
When the client sends POST /v1/auth/verify with body {"code": "wrong-code"}
Then the response status is 403
And the response body is the plain-text apperr.VerificationFailedErrorMessage, not JSON, and
contains no user field at all
Scenario: Registering while no verified account exists never echoes any user field
Given the user repository is empty
When the client sends POST /v1/auth/register with username "bob", password "hunter2example12",
email "bob@example.com"
Then the response status is 200
And the response body contains no key named password, password_reset_code,
verification_code, verification_expiration or password_reset_expiration
Scenario Outline: No other auth endpoint under /v1/auth leaks a user field
Given a user repository containing user id 7, username "alice", email "alice@example.com",
is_verified true, with a non-empty PasswordResetCode or VerificationCode where the endpoint
under test would set one
When the client sends <method> <path> with <body>
Then the response status is <status>
And the response body contains no key named password, password_reset_code,
verification_code, verification_expiration or password_reset_expiration
Examples:
| method | path | body | status |
| POST | /v1/auth/login | {"username":"alice","password":"hunter2example12"} | 200 |
| POST | /v1/auth/logout | {} | 200 |
| POST | /v1/auth/password-reset | {"email":"alice@example.com"} | 200 |
| POST | /v1/auth/password-reset/confirm | {"email":"alice@example.com","code":"wrong","password":"hunter2example12"} | 403 |
@@ -0,0 +1,60 @@
# QA procedure: user_response_field_hygiene
Package: `test` (black-box), extending `test/account_verification_http_test.go`'s
`newAccountVerificationRouter` / `newAccountVerificationTestUserRepo` helpers and
`test/user_test.go`'s `mockUserRepo` / `stubHasher`.
## Setup shared by every case
1. Call `validator.InitValidator()` once.
2. Reuse `newAccountVerificationRouter(userRepo, mailRepo, token)` from
`test/account_verification_http_test.go`, which wires `auth.NewHandler(...)` with fakes over
a fresh `chi.NewRouter()` mirroring `api/router.go`'s `/v1/auth` group (public routes plus the
`jwtauth.Verifier` + `Authenticator`-wrapped `verify` route). No live server, no real SQLite,
no real SMTP.
3. `newAccountVerificationTestUserRepo()` seeds user id 7 with a non-empty `VerificationCode`
(`"hashed:abc123"`) and `VerificationExpiration` one hour out, matching the existing fixture.
## Response-shape check for POST /v1/auth/verify (the fix)
4. Successful verify: `POST /v1/auth/verify` with `{"code":"abc123"}` and a valid jwt cookie for
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"}`;
- none of `password`, `password_reset_code`, `verification_code`,
`verification_expiration`, `password_reset_expiration` is present;
- `is_verified` unmarshals to `true`.
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`,
`password_reset_expiration` (the body is plain text from `http.Error`, so this is a substring
check, not a JSON-key check).
## Registration check (its own case, correctly gated)
6. `POST /v1/auth/register` against an **empty** `mockUserRepo` (the registration gate only
allows register while no verified account exists) with
`{"username":"bob","password":"hunter2example12","email":"bob@example.com"}` → assert status
`200` and that the raw response body string contains none of the six sensitive substrings
from step 5.
## Table-driven sweep of the other auth endpoints (regression guard, not the fix)
7. For each of `POST /v1/auth/login`, `/logout`, `/password-reset`, `/password-reset/confirm`,
seed a `mockUserRepo`/`mockMailRepo` pair with a verified user "alice" (`IsVerified: true`,
matching the Scenario Outline's Given, since these endpoints don't depend on the registration
gate), send the request, and assert the raw response body string contains none of the six
sensitive substrings from step 5. This should already pass today since these handlers only
ever write a `{"message": ...}` body; the point of the test is to pin that fact down so a
future change can't quietly regress it.
## Manual verification (not exercised by the Go test)
- The SPA's post-verification screen (`web/src/`) only ever needs `username`/`is_verified` to
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
`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.
+243
View File
@@ -0,0 +1,243 @@
package test
import (
"RenewCMS/api/handlers/auth"
"RenewCMS/api/pkg/apperr"
validator "RenewCMS/api/pkg/validator"
"RenewCMS/internal/domain/user"
"RenewCMS/internal/infrastructure/useCases"
"bytes"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/go-chi/chi/v5"
"github.com/go-chi/jwtauth/v5"
)
func newUserFieldHygieneRouter(userRepo *mockUserRepo, mailRepo *mockMailRepo, token *jwtauth.JWTAuth) *chi.Mux {
hasher := &stubHasher{}
authHandler := auth.NewHandler(
token,
useCases.NewGetUserUseCase(userRepo),
useCases.NewListUsersUseCase(userRepo),
useCases.NewCreateUserUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"),
useCases.NewAuthenticateUserUseCase(userRepo, hasher),
useCases.NewVerifyUserUseCase(userRepo, hasher),
useCases.NewCancelRegistrationUseCase(userRepo),
useCases.NewRequestPasswordResetUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"),
useCases.NewResetPasswordUseCase(userRepo, hasher),
true,
)
r := chi.NewRouter()
r.Route("/v1", func(r chi.Router) {
r.Route("/auth", func(r chi.Router) {
r.Post("/register", authHandler.Register)
r.Post("/login", authHandler.Login)
r.Post("/logout", authHandler.Logout)
r.Post("/password-reset", authHandler.RequestPasswordReset)
r.Post("/password-reset/confirm", authHandler.ResetPassword)
r.Group(func(r chi.Router) {
r.Use(jwtauth.Verifier(token))
r.Use(jwtauth.Authenticator(token))
r.Post("/verify", authHandler.Verify)
})
})
})
return r
}
func newFieldHygieneVerifiedAliceRepo() *mockUserRepo {
repo := newMockUserRepo()
repo.users[7] = user.User{
ID: 7,
Username: "alice",
Email: "alice@example.com",
Password: "hashed:hunter2example12",
IsVerified: true,
PasswordResetCode: "hashed:some-reset-code",
PasswordResetExpiration: time.Now().Add(time.Hour),
VerificationCode: "hashed:some-verification-code",
VerificationExpiration: time.Now().Add(time.Hour),
}
repo.nextID = 7
return repo
}
var forbiddenUserFieldKeys = []string{
"password",
"password_reset_code",
"verification_code",
"verification_expiration",
"password_reset_expiration",
}
func assertNoForbiddenKeys(t *testing.T, body []byte) {
t.Helper()
var decoded map[string]json.RawMessage
if err := json.Unmarshal(body, &decoded); err != nil {
return
}
for _, key := range forbiddenUserFieldKeys {
if _, present := decoded[key]; present {
t.Errorf("expected no key %q in response body, got %s", key, string(body))
}
}
}
func TestUserResponseFieldHygiene(t *testing.T) {
validator.InitValidator()
token := jwtauth.New("HS256", []byte("test-secret"), nil)
t.Run("A successful verification response carries only public fields", func(t *testing.T) {
repo := newMockUserRepo()
repo.users[7] = user.User{
ID: 7,
Username: "alice",
Email: "alice@example.com",
IsVerified: false,
VerificationCode: "hashed:abc123",
VerificationExpiration: time.Now().Add(time.Hour),
}
repo.nextID = 7
router := newUserFieldHygieneRouter(repo, newMockMailRepo(), token)
body, _ := json.Marshal(map[string]string{"code": "abc123"})
req := httptest.NewRequest(http.MethodPost, "/v1/auth/verify", bytes.NewReader(body))
_, tokenString, _ := token.Encode(map[string]any{"user_id": uint32(7)})
req.AddCookie(&http.Cookie{Name: "jwt", Value: tokenString})
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())
}
var decoded map[string]json.RawMessage
if err := json.Unmarshal(rec.Body.Bytes(), &decoded); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
expectedKeys := []string{"id", "username", "email", "is_verified", "created_at", "updated_at"}
if len(decoded) != len(expectedKeys) {
t.Errorf("expected exactly %d keys, got %d: %v", len(expectedKeys), len(decoded), decoded)
}
for _, key := range expectedKeys {
if _, present := decoded[key]; !present {
t.Errorf("expected key %q in response body", key)
}
}
assertNoForbiddenKeys(t, rec.Body.Bytes())
var isVerified bool
if err := json.Unmarshal(decoded["is_verified"], &isVerified); err != nil {
t.Fatalf("failed to decode is_verified: %v", err)
}
if !isVerified {
t.Errorf("expected is_verified true")
}
})
t.Run("A rejected verification attempt never echoes any user field", func(t *testing.T) {
repo := newMockUserRepo()
repo.users[7] = user.User{
ID: 7,
Username: "alice",
IsVerified: false,
VerificationCode: "hashed:abc123",
VerificationExpiration: time.Now().Add(time.Hour),
}
repo.nextID = 7
router := newUserFieldHygieneRouter(repo, newMockMailRepo(), token)
body, _ := json.Marshal(map[string]string{"code": "wrong-code"})
req := httptest.NewRequest(http.MethodPost, "/v1/auth/verify", bytes.NewReader(body))
_, tokenString, _ := token.Encode(map[string]any{"user_id": uint32(7)})
req.AddCookie(&http.Cookie{Name: "jwt", Value: tokenString})
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != http.StatusForbidden {
t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String())
}
if rec.Body.String() != apperr.VerificationFailedErrorMessage+"\n" {
t.Errorf("expected plain-text apperr.VerificationFailedErrorMessage body, got %q", rec.Body.String())
}
var decoded map[string]json.RawMessage
if err := json.Unmarshal(rec.Body.Bytes(), &decoded); err == nil {
t.Errorf("expected non-JSON body, but it decoded as JSON: %v", decoded)
}
})
t.Run("Registering while no verified account exists never echoes any user field", func(t *testing.T) {
repo := newMockUserRepo()
router := newUserFieldHygieneRouter(repo, newMockMailRepo(), token)
body := `{"username":"bob","password":"hunter2example12","email":"bob@example.com"}`
req := httptest.NewRequest(http.MethodPost, "/v1/auth/register", bytes.NewBufferString(body))
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())
}
assertNoForbiddenKeys(t, rec.Body.Bytes())
})
outlineCases := []struct {
name string
method string
path string
body string
status int
}{
{
name: "login",
method: http.MethodPost,
path: "/v1/auth/login",
body: `{"username":"alice","password":"hunter2example12"}`,
status: http.StatusOK,
},
{
name: "logout",
method: http.MethodPost,
path: "/v1/auth/logout",
body: `{}`,
status: http.StatusOK,
},
{
name: "password-reset",
method: http.MethodPost,
path: "/v1/auth/password-reset",
body: `{"email":"alice@example.com"}`,
status: http.StatusOK,
},
{
name: "password-reset/confirm",
method: http.MethodPost,
path: "/v1/auth/password-reset/confirm",
body: `{"email":"alice@example.com","code":"wrong","password":"hunter2example12"}`,
status: http.StatusForbidden,
},
}
for _, tc := range outlineCases {
t.Run("No other auth endpoint under /v1/auth leaks a user field: "+tc.name, func(t *testing.T) {
repo := newFieldHygieneVerifiedAliceRepo()
router := newUserFieldHygieneRouter(repo, newMockMailRepo(), token)
req := httptest.NewRequest(tc.method, tc.path, bytes.NewBufferString(tc.body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != tc.status {
t.Fatalf("expected status %d, got %d: %s", tc.status, rec.Code, rec.Body.String())
}
assertNoForbiddenKeys(t, rec.Body.Bytes())
})
}
}