From 8460a078e381b8c34e7c66586c00e262e37a094b Mon Sep 17 00:00:00 2001 From: Florian Sylvain Date: Wed, 23 Sep 2026 18:55:05 +0200 Subject: [PATCH] refac: missing user or article is reported with a domain not-found error instead of a raw GORM error, and the API answers 404 for it --- api/handlers/article/articleHandler.go | 2 +- api/pkg/apperr/messages.go | 1 + features/not_found_errors.feature | 75 +++++ features/not_found_errors.qa.md | 26 ++ features/use_case_baseline.feature | 12 +- internal/domain/user/userDomain.go | 3 + .../repositories/articleRepository.go | 4 +- .../persistence/repositories/notFound.go | 14 + .../repositories/userRepository.go | 6 +- test/article_test.go | 4 +- test/not_found_errors_test.go | 278 ++++++++++++++++++ test/use_case_baseline_test.go | 27 +- test/user_test.go | 6 +- 13 files changed, 426 insertions(+), 32 deletions(-) create mode 100644 features/not_found_errors.feature create mode 100644 features/not_found_errors.qa.md create mode 100644 internal/infrastructure/persistence/repositories/notFound.go create mode 100644 test/not_found_errors_test.go diff --git a/api/handlers/article/articleHandler.go b/api/handlers/article/articleHandler.go index 2c229e7..34f803e 100644 --- a/api/handlers/article/articleHandler.go +++ b/api/handlers/article/articleHandler.go @@ -46,7 +46,7 @@ func (h *Handler) GetArticle(w http.ResponseWriter, r *http.Request) { article, err := h.GetUseCase.GetOnlineArticle(uint32(id)) if err != nil { if errors.Is(err, domain.ErrArticleNotFound) { - http.Error(w, "Article not found", http.StatusNotFound) + http.Error(w, apperr.ArticleNotFoundErrorMessage, http.StatusNotFound) return } http.Error(w, "Internal server error", http.StatusInternalServerError) diff --git a/api/pkg/apperr/messages.go b/api/pkg/apperr/messages.go index 27d6604..9d86fb3 100644 --- a/api/pkg/apperr/messages.go +++ b/api/pkg/apperr/messages.go @@ -3,3 +3,4 @@ package apperr const LogsErrorMessage = "Access to the requested resource is forbidden due to incorrect password and/or username." const BodyErrorMessage = "The request cannot be processed due to a mismatch in the format of the body." const IdUint32ErrorMessage = "The server expects the ID to be in the format of an unsigned 32-bit integer (uint32)." +const ArticleNotFoundErrorMessage = "Article not found" diff --git a/features/not_found_errors.feature b/features/not_found_errors.feature new file mode 100644 index 0000000..7898233 --- /dev/null +++ b/features/not_found_errors.feature @@ -0,0 +1,75 @@ +Feature: Domain not-found errors instead of raw GORM errors + + As the maintainer of RenewCMS + I want a missing article or user reported as a domain not-found error + So that the API answers 404 for it instead of leaking a raw GORM error as a 500, and so that + a caller can rely on errors.Is(err, article.ErrArticleNotFound) or errors.Is(err, user.ErrUserNotFound) + + Background: + Given internal/domain/article defines the sentinel article.ErrArticleNotFound + And internal/domain/user defines the sentinel user.ErrUserNotFound + And ArticleRepository.Get and ArticleRepository.GetByName return article.ErrArticleNotFound, not gorm.ErrRecordNotFound, when no row matches + And UserRepository.Get, UserRepository.GetByUsername and UserRepository.GetByEmail return user.ErrUserNotFound, not gorm.ErrRecordNotFound, when no row matches + And GET /v1/article/{id} sits behind the jwtauth Verifier and Authenticator group, and requires a valid jwt cookie + + Scenario Outline: The article repository reports a missing article as a domain error + Given a repository backed by an in-memory SQLite database containing an article titled "Backyard Renovation Update" with id 1 + When is called with + Then the result is the error article.ErrArticleNotFound + And errors.Is(err, gorm.ErrRecordNotFound) is false + + Examples: + | method | input | + | Get | id 9 | + | GetByName | "Unknown Title" | + + Scenario Outline: The user repository reports a missing user as a domain error + Given a repository backed by an in-memory SQLite database containing a user with id 1, username "alice" and email "alice@example.com" + When is called with + Then the result is the error user.ErrUserNotFound + And errors.Is(err, gorm.ErrRecordNotFound) is false + + Examples: + | method | input | + | Get | id 9 | + | GetByUsername | "bob" | + | GetByEmail | "bob@example.com" | + + Scenario: Fetching a nonexistent article answers 404, not 500 + Given a valid jwt cookie for a user with id 1 + And the article repository has no article with id 9999 + When the client sends GET /v1/article/9999 with that cookie + Then the response status is 404 + And the response body is the text "Article not found" + + Scenario: Fetching an offline article still answers 404 with the same message + Given a valid jwt cookie for a user with id 1 + And the article repository contains an article with id 2, is_online false + When the client sends GET /v1/article/2 with that cookie + Then the response status is 404 + And the response body is the text "Article not found" + + Scenario: Fetching an online article still answers 200 + Given a valid jwt cookie for a user with id 1 + And the article repository contains an article with id 1, is_online true + When the client sends GET /v1/article/1 with that cookie + Then the response status is 200 + And the response body is the article's JSON representation + + Scenario: The article route still requires authentication before existence is even checked + Given no jwt cookie is sent + And the article repository has no article with id 9999 + When the client sends GET /v1/article/9999 without a cookie + Then the response status is 401 + + Scenario Outline: Login never reveals whether the username exists + Given a user repository containing a user "alice" with password "hunter2example" + When the client sends POST /v1/auth/login with username "" and 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 | diff --git a/features/not_found_errors.qa.md b/features/not_found_errors.qa.md new file mode 100644 index 0000000..ebb5750 --- /dev/null +++ b/features/not_found_errors.qa.md @@ -0,0 +1,26 @@ +# QA procedure: not_found_errors + +Package: `test` (black-box, alongside the existing `article_test.go` / `user_test.go` / `use_case_baseline_test.go`). + +## Setup shared by every case +1. Call `validator.InitValidator()` once. +2. Build in-memory fakes for `article.Repository` and `user.Repository` (extend the existing `mockArticleRepo` / `mockUserRepo` patterns in `test/`), updated so that a lookup by an unknown id/name/username/email now returns `article.ErrArticleNotFound` / `user.ErrUserNotFound` instead of `gorm.ErrRecordNotFound` — mirroring the fixed real repositories. +3. Wire the real use cases (`useCases.NewGetArticleUseCase`, `useCases.NewCreateArticleUseCase` if needed, `useCases.NewGetUserUseCase`, `useCases.NewListUsersUseCase`, `useCases.NewCreateUserUseCase`) over those fakes. +4. Build `article.NewHandler(...)` and `auth.NewHandler(...)` from those use cases plus a test `jwtauth.New("HS256", []byte("test-secret"), nil)`. +5. Mount a fresh `chi.NewRouter()` mirroring `api/router.go`: the article handler's routes inside a group wrapped by `jwtauth.Verifier(token)` + `jwtauth.Authenticator(token)`, the auth handler's routes unprotected. No live server. + +## Repository-level checks (real persistence repositories, in-memory SQLite, no HTTP) +6. Open a `gorm.DB` with `sqlite.Open("file:" + t.Name() + "?mode=memory&cache=shared")` (one named in-memory DB per test, so tests never share rows) (the project's `github.com/glebarez/sqlite` driver, never the real `DB_FILE`), and `db.AutoMigrate(&models.Article{})`. Construct a real `persistence.NewArticleRepository(db)`, seed it with an article id 1 via `Create`. Call `Get(9)` directly; assert `errors.Is(err, article.ErrArticleNotFound)` is true and `errors.Is(err, gorm.ErrRecordNotFound)` is false. Repeat for `GetByName("Unknown Title")`. Close the in-memory DB (or let it drop) at the end of the test. +7. Open a second `gorm.DB` the same way with `db.AutoMigrate(&models.User{})`. Construct a real `persistence.NewUserRepository(db)`, seed it with a user id 1 / username "alice" / email "alice@example.com" via `Create`. Call `Get(9)`, `GetByUsername("bob")`, `GetByEmail("bob@example.com")` directly; assert each returns `user.ErrUserNotFound` and not `gorm.ErrRecordNotFound`. + +## HTTP checks (fakes, per the shared setup above) +8. Mint a valid jwt cookie for user id 1 with the test `jwtauth.JWTAuth` (encode with `token.Encode(map[string]any{"user_id": uint32(1)})` and set it as the `jwt` cookie on the `httptest.NewRequest`). +9. `GET /v1/article/9999` with that cookie, empty repo id 9999 → assert status `404`, body `"Article not found\n"` (http.Error appends a trailing newline). +10. `GET /v1/article/2` with that cookie, repo seeded with an offline article id 2 → assert status `404`, body `"Article not found\n"` (regression: unchanged from before this fix). +11. `GET /v1/article/1` with that cookie, repo seeded with an online article id 1 → assert status `200`, body decodes to the article JSON with id 1. +12. `GET /v1/article/9999` with **no** cookie → assert status `401` (regression: jwtauth still gates the route). +13. `POST /v1/auth/login` with `{"username":"alice","password":"wrongpassword12"}` (repo seeded with user "alice"/bcrypt-hashed "hunter2example") → assert status `403`, body equals `apperr.LogsErrorMessage + "\n"`, no `Set-Cookie: jwt=...` header present. +14. `POST /v1/auth/login` with `{"username":"bob","password":"hunter2example"}` (no such user in the repo) → assert status `403`, **identical** body to step 13, no `Set-Cookie` header — proves the domain-error change didn't turn this into a distinguishable 404/500 and didn't start leaking existence. + +## Manual verification (not exercised by the Go test) +- None. This slice has no SPA-visible behavior and no SMTP involvement; the SPA's article-detail view already treats any non-2xx as "not found" generically (verify only if the SPA has an article-detail page — otherwise skip). diff --git a/features/use_case_baseline.feature b/features/use_case_baseline.feature index 883e286..d71d18e 100644 --- a/features/use_case_baseline.feature +++ b/features/use_case_baseline.feature @@ -40,10 +40,10 @@ Feature: Use case baseline characterization | method | id | outcome | | GetArticle | 1 | the article with id 1 | | GetArticle | 2 | the article with id 2 (GetArticle does not hide offline articles) | - | GetArticle | 9 | the repository's not-found error (gorm.ErrRecordNotFound) | + | GetArticle | 9 | the error article.ErrArticleNotFound | | GetOnlineArticle | 1 | the article with id 1 | | GetOnlineArticle | 2 | the error article.ErrArticleNotFound | - | GetOnlineArticle | 9 | the repository's not-found error (gorm.ErrRecordNotFound), not article.ErrArticleNotFound | + | GetOnlineArticle | 9 | the error article.ErrArticleNotFound | Scenario Outline: GetArticleByName delegates verbatim to the repository Given an article repository fake containing an article titled "Backyard Renovation Update" with id 1 @@ -53,7 +53,7 @@ Feature: Use case baseline characterization Examples: | name | outcome | | Backyard Renovation Update | the article with id 1 | - | Unknown Title | the repository's not-found error (gorm.ErrRecordNotFound) | + | Unknown Title | the error article.ErrArticleNotFound | Scenario Outline: ListArticles filters by is_online Given an article repository fake containing: @@ -150,11 +150,11 @@ Feature: Use case baseline characterization Examples: | method | input | outcome | | GetUser | 1 | the user with id 1 | - | GetUser | 9 | the repository's not-found error (gorm.ErrRecordNotFound) | + | GetUser | 9 | the error user.ErrUserNotFound | | GetUserByUsername | alice | the user with id 1 | - | GetUserByUsername | bob | the repository's not-found error (gorm.ErrRecordNotFound) | + | GetUserByUsername | bob | the error user.ErrUserNotFound | | GetUserByEmail | alice@example.com | the user with id 1 | - | GetUserByEmail | bob@example.com | the repository's not-found error (gorm.ErrRecordNotFound) | + | GetUserByEmail | bob@example.com | the error user.ErrUserNotFound | Scenario: ListUsers returns every user in the fake Given a user repository fake containing users with ids 1 and 2 diff --git a/internal/domain/user/userDomain.go b/internal/domain/user/userDomain.go index ba3e50f..86c00bc 100644 --- a/internal/domain/user/userDomain.go +++ b/internal/domain/user/userDomain.go @@ -1,9 +1,12 @@ package user import ( + "errors" "time" ) +var ErrUserNotFound = errors.New("user not found") + type User struct { ID uint32 `json:"id"` Username string `json:"username"` diff --git a/internal/infrastructure/persistence/repositories/articleRepository.go b/internal/infrastructure/persistence/repositories/articleRepository.go index 55429e0..953e21a 100644 --- a/internal/infrastructure/persistence/repositories/articleRepository.go +++ b/internal/infrastructure/persistence/repositories/articleRepository.go @@ -38,7 +38,7 @@ func (a *ArticleRepository) Get(id uint32) (domain.Article, error) { var article entity.Article err := a.db.Model(&entity.Article{}).Preload("Images").First(&article, id).Error if err != nil { - return domain.Article{}, err + return domain.Article{}, mapNotFoundErr(err, domain.ErrArticleNotFound) } return mappers.ArticleToDomain(article), nil @@ -48,7 +48,7 @@ func (a *ArticleRepository) GetByName(name string) (domain.Article, error) { var article entity.Article err := a.db.Model(&entity.Article{}).Where("title = ?", name).First(&article).Error if err != nil { - return domain.Article{}, err + return domain.Article{}, mapNotFoundErr(err, domain.ErrArticleNotFound) } return mappers.ArticleToDomain(article), nil diff --git a/internal/infrastructure/persistence/repositories/notFound.go b/internal/infrastructure/persistence/repositories/notFound.go new file mode 100644 index 0000000..331d321 --- /dev/null +++ b/internal/infrastructure/persistence/repositories/notFound.go @@ -0,0 +1,14 @@ +package persistence + +import ( + "errors" + + "gorm.io/gorm" +) + +func mapNotFoundErr(err error, sentinel error) error { + if errors.Is(err, gorm.ErrRecordNotFound) { + return sentinel + } + return err +} diff --git a/internal/infrastructure/persistence/repositories/userRepository.go b/internal/infrastructure/persistence/repositories/userRepository.go index decde19..8bb4adf 100644 --- a/internal/infrastructure/persistence/repositories/userRepository.go +++ b/internal/infrastructure/persistence/repositories/userRepository.go @@ -21,7 +21,7 @@ func (u *UserRepository) Get(id uint32) (domainUser.User, error) { var localUser entity.User err := u.db.Model(&entity.User{}).First(&localUser, id).Error if err != nil { - return domainUser.User{}, err + return domainUser.User{}, mapNotFoundErr(err, domainUser.ErrUserNotFound) } return mappers.UserToDomain(localUser), nil @@ -69,7 +69,7 @@ func (u *UserRepository) GetByUsername(username string) (domainUser.User, error) var localUser entity.User err := u.db.Model(&entity.User{}).Where("username = ?", username).First(&localUser).Error if err != nil { - return domainUser.User{}, err + return domainUser.User{}, mapNotFoundErr(err, domainUser.ErrUserNotFound) } return mappers.UserToDomain(localUser), nil @@ -79,7 +79,7 @@ func (u *UserRepository) GetByEmail(email string) (domainUser.User, error) { var localUser entity.User err := u.db.Model(&entity.User{}).Where("email = ?", email).First(&localUser).Error if err != nil { - return domainUser.User{}, err + return domainUser.User{}, mapNotFoundErr(err, domainUser.ErrUserNotFound) } return mappers.UserToDomain(localUser), nil diff --git a/test/article_test.go b/test/article_test.go index 508336c..158e288 100644 --- a/test/article_test.go +++ b/test/article_test.go @@ -55,7 +55,7 @@ func (m *mockArticleRepo) Get(id uint32) (article.Article, error) { } a, ok := m.articles[id] if !ok { - return article.Article{}, gorm.ErrRecordNotFound + return article.Article{}, article.ErrArticleNotFound } return a, nil } @@ -70,7 +70,7 @@ func (m *mockArticleRepo) GetByName(name string) (article.Article, error) { return a, nil } } - return article.Article{}, gorm.ErrRecordNotFound + return article.Article{}, article.ErrArticleNotFound } func (m *mockArticleRepo) GetAll() []article.Article { diff --git a/test/not_found_errors_test.go b/test/not_found_errors_test.go new file mode 100644 index 0000000..62209c8 --- /dev/null +++ b/test/not_found_errors_test.go @@ -0,0 +1,278 @@ +package test + +import ( + "RenewCMS/api/handlers/article" + "RenewCMS/api/handlers/auth" + "RenewCMS/api/pkg/apperr" + validator "RenewCMS/api/pkg/validator" + domainArticle "RenewCMS/internal/domain/article" + domainUser "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/persistence/models" + persistence "RenewCMS/internal/infrastructure/persistence/repositories" + "RenewCMS/internal/infrastructure/useCases" + "bytes" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/glebarez/sqlite" + "github.com/go-chi/chi/v5" + "github.com/go-chi/jwtauth/v5" + "golang.org/x/crypto/bcrypt" + "gorm.io/gorm" +) + +func openInMemoryDb(t *testing.T) *gorm.DB { + t.Helper() + db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{}) + if err != nil { + t.Fatalf("failed to open in-memory database: %v", err) + } + return db +} + +func TestArticleRepositoryReportsMissingArticleAsDomainError(t *testing.T) { + db := openInMemoryDb(t) + if err := db.AutoMigrate(&models.Article{}); err != nil { + t.Fatalf("failed to migrate: %v", err) + } + repo := persistence.NewArticleRepository(db) + if _, err := repo.Create(domainArticle.Article{Title: "Backyard Renovation Update", Body: "Body"}); err != nil { + t.Fatalf("failed to seed article: %v", err) + } + + t.Run("Get id 9", func(t *testing.T) { + _, err := repo.Get(9) + if !errors.Is(err, domainArticle.ErrArticleNotFound) { + t.Errorf("expected article.ErrArticleNotFound, got %v", err) + } + if errors.Is(err, gorm.ErrRecordNotFound) { + t.Errorf("expected the domain error, not gorm.ErrRecordNotFound") + } + }) + + t.Run("GetByName Unknown Title", func(t *testing.T) { + _, err := repo.GetByName("Unknown Title") + if !errors.Is(err, domainArticle.ErrArticleNotFound) { + t.Errorf("expected article.ErrArticleNotFound, got %v", err) + } + if errors.Is(err, gorm.ErrRecordNotFound) { + t.Errorf("expected the domain error, not gorm.ErrRecordNotFound") + } + }) + + t.Run("Get propagates a non-not-found database error unchanged", func(t *testing.T) { + sqlDb, err := db.DB() + if err != nil { + t.Fatalf("failed to get sql db: %v", err) + } + if err := sqlDb.Close(); err != nil { + t.Fatalf("failed to close db: %v", err) + } + + _, err = repo.Get(1) + if err == nil { + t.Fatal("expected an error, got nil") + } + if errors.Is(err, domainArticle.ErrArticleNotFound) { + t.Errorf("expected a non-not-found error, got domain.ErrArticleNotFound") + } + }) +} + +func TestUserRepositoryReportsMissingUserAsDomainError(t *testing.T) { + db := openInMemoryDb(t) + if err := db.AutoMigrate(&models.User{}); err != nil { + t.Fatalf("failed to migrate: %v", err) + } + repo := persistence.NewUserRepository(db) + if _, err := repo.Create(domainUser.User{Username: "alice", Password: "hunter2example", Email: "alice@example.com", VerificationCode: "code"}); err != nil { + t.Fatalf("failed to seed user: %v", err) + } + + t.Run("Get id 9", func(t *testing.T) { + _, err := repo.Get(9) + if !errors.Is(err, domainUser.ErrUserNotFound) { + t.Errorf("expected user.ErrUserNotFound, got %v", err) + } + if errors.Is(err, gorm.ErrRecordNotFound) { + t.Errorf("expected the domain error, not gorm.ErrRecordNotFound") + } + }) + + t.Run("GetByUsername bob", func(t *testing.T) { + _, err := repo.GetByUsername("bob") + if !errors.Is(err, domainUser.ErrUserNotFound) { + t.Errorf("expected user.ErrUserNotFound, got %v", err) + } + if errors.Is(err, gorm.ErrRecordNotFound) { + t.Errorf("expected the domain error, not gorm.ErrRecordNotFound") + } + }) + + t.Run("GetByEmail bob@example.com", func(t *testing.T) { + _, err := repo.GetByEmail("bob@example.com") + if !errors.Is(err, domainUser.ErrUserNotFound) { + t.Errorf("expected user.ErrUserNotFound, got %v", err) + } + if errors.Is(err, gorm.ErrRecordNotFound) { + t.Errorf("expected the domain error, not gorm.ErrRecordNotFound") + } + }) +} + +func newNotFoundErrorsRouter(articleRepo *mockArticleRepo, userRepo *mockUserRepo, token *jwtauth.JWTAuth) *chi.Mux { + articleHandler := article.NewHandler( + useCases.NewCreateArticleUseCase(articleRepo), + useCases.NewGetArticleUseCase(articleRepo), + useCases.NewListArticlesUseCase(articleRepo), + useCases.NewDeleteArticleUseCase(articleRepo), + ) + authHandler := auth.NewHandler( + token, + useCases.NewGetUserUseCase(userRepo), + useCases.NewListUsersUseCase(userRepo), + useCases.NewCreateUserUseCase(userRepo), + ) + + r := chi.NewRouter() + r.Route("/v1", func(r chi.Router) { + r.Group(func(r chi.Router) { + r.Use(jwtauth.Verifier(token)) + r.Use(jwtauth.Authenticator(token)) + r.Route("/article", func(r chi.Router) { + r.Get("/{id}", articleHandler.GetArticle) + }) + }) + r.Route("/auth", func(r chi.Router) { + r.Post("/login", authHandler.Login) + }) + }) + return r +} + +func setJwtCookie(t *testing.T, req *http.Request, token *jwtauth.JWTAuth, userId uint32) { + t.Helper() + _, tokenString, err := token.Encode(map[string]any{"user_id": userId}) + if err != nil { + t.Fatalf("failed to encode jwt: %v", err) + } + req.AddCookie(&http.Cookie{Name: "jwt", Value: tokenString}) +} + +func TestNotFoundErrorsHttp(t *testing.T) { + validator.InitValidator() + token := jwtauth.New("HS256", []byte("test-secret"), nil) + + newArticleRepo := func() *mockArticleRepo { + repo := newMockArticleRepo() + repo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update", IsOnline: true} + repo.articles[2] = domainArticle.Article{ID: 2, Title: "Offline Article", IsOnline: false} + return repo + } + + t.Run("Fetching a nonexistent article answers 404, not 500", func(t *testing.T) { + router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token) + req := httptest.NewRequest(http.MethodGet, "/v1/article/9999", nil) + setJwtCookie(t, req, token, 1) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("expected status 404, got %d", rec.Code) + } + if rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) + + t.Run("Fetching an offline article still answers 404 with the same message", func(t *testing.T) { + router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token) + req := httptest.NewRequest(http.MethodGet, "/v1/article/2", nil) + setJwtCookie(t, req, token, 1) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("expected status 404, got %d", rec.Code) + } + if rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) + + t.Run("Fetching an online article still answers 200", func(t *testing.T) { + router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token) + req := httptest.NewRequest(http.MethodGet, "/v1/article/1", nil) + setJwtCookie(t, req, token, 1) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", rec.Code) + } + var got domainArticle.Article + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if got.ID != 1 { + t.Errorf("expected article id 1, got %d", got.ID) + } + }) + + t.Run("The article route still requires authentication before existence is even checked", func(t *testing.T) { + router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token) + req := httptest.NewRequest(http.MethodGet, "/v1/article/9999", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected status 401, got %d", rec.Code) + } + }) + + newAliceUserRepo := func() *mockUserRepo { + repo := newMockUserRepo() + hashed, err := bcrypt.GenerateFromPassword([]byte("hunter2example"), 12) + if err != nil { + t.Fatalf("failed to hash password: %v", err) + } + repo.users[1] = domainUser.User{ID: 1, Username: "alice", Password: string(hashed)} + return repo + } + + loginCases := []struct { + name string + username string + password string + }{ + {"wrong password for existing user", "alice", "wrongpassword12"}, + {"nonexistent username", "bob", "hunter2example"}, + } + + for _, tc := range loginCases { + t.Run("Login never reveals whether the username exists: "+tc.name, func(t *testing.T) { + router := newNotFoundErrorsRouter(newArticleRepo(), newAliceUserRepo(), token) + 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")) + } + }) + } +} diff --git a/test/use_case_baseline_test.go b/test/use_case_baseline_test.go index 22a5a45..56060fb 100644 --- a/test/use_case_baseline_test.go +++ b/test/use_case_baseline_test.go @@ -111,8 +111,8 @@ func TestGetArticleAndGetOnlineArticle(t *testing.T) { repo := newRepo() uc := useCases.NewGetArticleUseCase(repo) _, err := uc.GetArticle(9) - if !errors.Is(err, gorm.ErrRecordNotFound) { - t.Errorf("expected gorm.ErrRecordNotFound, got %v", err) + if !errors.Is(err, article.ErrArticleNotFound) { + t.Errorf("expected article.ErrArticleNotFound, got %v", err) } }) @@ -141,11 +141,8 @@ func TestGetArticleAndGetOnlineArticle(t *testing.T) { repo := newRepo() uc := useCases.NewGetArticleUseCase(repo) _, err := uc.GetOnlineArticle(9) - if !errors.Is(err, gorm.ErrRecordNotFound) { - t.Errorf("expected gorm.ErrRecordNotFound, got %v", err) - } - if errors.Is(err, article.ErrArticleNotFound) { - t.Errorf("expected the raw not-found error, not article.ErrArticleNotFound") + if !errors.Is(err, article.ErrArticleNotFound) { + t.Errorf("expected article.ErrArticleNotFound, got %v", err) } }) } @@ -173,8 +170,8 @@ func TestGetArticleByNameDelegatesVerbatim(t *testing.T) { repo := newRepo() uc := useCases.NewGetArticleUseCase(repo) _, err := uc.GetArticleByName("Unknown Title") - if !errors.Is(err, gorm.ErrRecordNotFound) { - t.Errorf("expected gorm.ErrRecordNotFound, got %v", err) + if !errors.Is(err, article.ErrArticleNotFound) { + t.Errorf("expected article.ErrArticleNotFound, got %v", err) } }) } @@ -519,8 +516,8 @@ func TestGetUserReadsByIdUsernameOrEmail(t *testing.T) { repo := newRepo() uc := useCases.NewGetUserUseCase(repo) _, err := uc.GetUser(9) - if !errors.Is(err, gorm.ErrRecordNotFound) { - t.Errorf("expected gorm.ErrRecordNotFound, got %v", err) + if !errors.Is(err, user.ErrUserNotFound) { + t.Errorf("expected user.ErrUserNotFound, got %v", err) } }) @@ -540,8 +537,8 @@ func TestGetUserReadsByIdUsernameOrEmail(t *testing.T) { repo := newRepo() uc := useCases.NewGetUserUseCase(repo) _, err := uc.GetUserByUsername("bob") - if !errors.Is(err, gorm.ErrRecordNotFound) { - t.Errorf("expected gorm.ErrRecordNotFound, got %v", err) + if !errors.Is(err, user.ErrUserNotFound) { + t.Errorf("expected user.ErrUserNotFound, got %v", err) } }) @@ -561,8 +558,8 @@ func TestGetUserReadsByIdUsernameOrEmail(t *testing.T) { repo := newRepo() uc := useCases.NewGetUserUseCase(repo) _, err := uc.GetUserByEmail("bob@example.com") - if !errors.Is(err, gorm.ErrRecordNotFound) { - t.Errorf("expected gorm.ErrRecordNotFound, got %v", err) + if !errors.Is(err, user.ErrUserNotFound) { + t.Errorf("expected user.ErrUserNotFound, got %v", err) } }) } diff --git a/test/user_test.go b/test/user_test.go index 29bf793..66d0517 100644 --- a/test/user_test.go +++ b/test/user_test.go @@ -34,7 +34,7 @@ func (m *mockUserRepo) Get(id uint32) (user.User, error) { } u, ok := m.users[id] if !ok { - return user.User{}, gorm.ErrRecordNotFound + return user.User{}, user.ErrUserNotFound } return u, nil } @@ -49,7 +49,7 @@ func (m *mockUserRepo) GetByUsername(username string) (user.User, error) { return u, nil } } - return user.User{}, gorm.ErrRecordNotFound + return user.User{}, user.ErrUserNotFound } func (m *mockUserRepo) GetByEmail(email string) (user.User, error) { @@ -62,7 +62,7 @@ func (m *mockUserRepo) GetByEmail(email string) (user.User, error) { return u, nil } } - return user.User{}, gorm.ErrRecordNotFound + return user.User{}, user.ErrUserNotFound } func (m *mockUserRepo) GetAll() []user.User {