fix: reading a single article follows the same rule as the list: anyone can read an online article, a verified user can also read an offline one, and an unknown or hidden id returns 404

This commit is contained in:
fsylvain committed 2026-09-24 16:32:40 +02:00
1 parent 2ee28203ab
commit 81c4b62175
11 files changed
+510 -28

No files matched your search

+6 -1
View File
@@ -53,7 +53,12 @@ func (h *Handler) GetArticle(w http.ResponseWriter, r *http.Request) {
return
}
article, err := h.GetUseCase.GetOnlineArticle(id)
var article domain.Article
if h.callerIsVerified(r) {
article, err = h.GetUseCase.GetArticle(id)
} else {
article, err = h.GetUseCase.GetOnlineArticle(id)
}
if err != nil {
if errors.Is(err, domain.ErrArticleNotFound) {
http.Error(w, apperr.ArticleNotFoundErrorMessage, http.StatusNotFound)
+1 -1
View File
@@ -36,11 +36,11 @@ func NewArticleRouter(container *UseCases) http.Handler {
r.Group(func(r chi.Router) {
r.Get("/", articleHandler.ListArticles)
r.Get("/{id}", articleHandler.GetArticle)
})
r.Group(func(r chi.Router) {
r.Use(jwtauth.Authenticator(container.Token))
r.Use(middleware.RequireVerifiedUser(container.GetUserUseCase))
r.Get("/{id}", articleHandler.GetArticle)
r.Post("/", articleHandler.PostArticle)
r.Delete("/{id}", articleHandler.DeleteArticle)
r.Patch("/{id}", articleHandler.UpdateArticleBody)
+10 -5
View File
@@ -11,6 +11,10 @@ Feature: Domain not-found errors instead of raw GORM errors
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
And a later slice, features/public_article_read.feature, moves GET /v1/article/{id} into the
Verifier-only group and makes it return an offline article to a verified caller instead of
404, mirroring the listing endpoint's visibility rule; the two scenarios below that depended
on the older, stricter rule are updated accordingly
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
@@ -42,10 +46,10 @@ Feature: Domain not-found errors instead of raw GORM errors
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
Scenario: Fetching an offline article as an anonymous or unverified caller still answers 404 with the same message
Given no jwt cookie is sent
And the article repository contains an article with id 2, is_online false
When the client sends GET /v1/article/2 with that cookie
When the client sends GET /v1/article/2 without a cookie
Then the response status is 404
And the response body is the text "Article not found"
@@ -56,11 +60,12 @@ Feature: Domain not-found errors instead of raw GORM errors
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
Scenario: An unknown article id answers 404 even without authentication
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
Then the response status is 404
And the response body is the text "Article not found"
Scenario Outline: Login never reveals whether the username exists
Given a user repository containing a user "alice" with password "hunter2example"
+5 -3
View File
@@ -14,8 +14,11 @@ Feature: Public and authenticated article listing
in api/router.go along with GET /v1/article/{id}, POST /v1/article/ and DELETE /v1/article/{id},
so an anonymous request to any of them answers 401, not an online-only list
And this slice moves GET /v1/article/ under jwtauth.Verifier only, so a missing or invalid jwt
cookie no longer blocks the request; GET /v1/article/{id}, POST /v1/article/ and
DELETE /v1/article/{id} keep requiring jwtauth.Verifier + Authenticator, unchanged
cookie no longer blocks the request; POST /v1/article/ and DELETE /v1/article/{id} keep
requiring jwtauth.Verifier + Authenticator, unchanged
And a later slice, features/public_article_read.feature, moves GET /v1/article/{id} into the
same Verifier-only group as GET /v1/article/, so the 401 Examples table below is updated
accordingly
And Handler.ListArticles decides between ListAllArticles and ListOnlineArticles by checking the
request for a valid jwt, read with jwtauth.FromContext from the token the router-level
jwtauth.Verifier (header and cookie sources) placed in the request context, rather than
@@ -53,7 +56,6 @@ Feature: Public and authenticated article listing
Examples:
| method | path |
| GET | /v1/article/1 |
| POST | /v1/article/ |
| DELETE | /v1/article/1 |
+132
View File
@@ -0,0 +1,132 @@
Feature: Single article read follows the same online/offline visibility rule as the list
As a visitor of the public site and as an authenticated RenewCMS user
I want GET /v1/article/{id} to return an online article to anyone, and an offline article
only to a caller resolved as verified, with an unknown id and a hidden offline id
answering the exact same 404
So that the public site can read a single published article through the API, and a verified
user can open their own unpublished draft to edit it, while a draft never leaks its existence
to anyone else
Background:
Given internal/domain/article.Article has a json "is_online" field and the sentinel
article.ErrArticleNotFound
And GetArticleUseCase.GetArticle(id) returns the article regardless of is_online, and
GetArticleUseCase.GetOnlineArticle(id) returns article.ErrArticleNotFound when the article
exists but is offline, both already implemented and unchanged by this slice
And before this slice, GET /v1/article/{id} sits inside the jwtauth Verifier + Authenticator +
middleware.RequireVerifiedUser group in api/router.go, so a missing jwt cookie answers 401, an
authenticated-but-unverified caller answers 403, and even a verified caller always resolves
through GetOnlineArticle, so an offline article answers 404 no matter who asks
(features/not_found_errors.feature, features/verified_account_required.feature)
And this slice moves GET /v1/article/{id} out of that group and into the jwtauth.Verifier-only
group that already holds GET /v1/article/ (features/public_article_listing.feature), so a
missing or invalid jwt cookie no longer blocks the request; POST /v1/article/,
DELETE /v1/article/{id}, PATCH /v1/article/{id}, PATCH /v1/article/{id}/publish,
PATCH /v1/article/{id}/unpublish and POST /v1/article/{id}/image keep requiring
Verifier + Authenticator + RequireVerifiedUser, unchanged
And Handler.GetArticle reuses the existing Handler.callerIsVerified(r) helper already used by
Handler.ListArticles: when the request resolves to a verified account it calls
GetUseCase.GetArticle(id); otherwise (no jwt, malformed jwt, unresolvable user_id, a deleted
account, or an account that exists but is not yet verified) it calls
GetUseCase.GetOnlineArticle(id), exactly the same fallback rule
features/verified_account_required.feature already documents for the listing endpoint
And an id that does not parse as a uint32 still answers 400 with apperr.IdUint32ErrorMessage
before any lookup happens, for every caller, unchanged
And a domain article.ErrArticleNotFound, whether the id is truly unknown or is an offline
article hidden from this caller, still answers 404 with apperr.ArticleNotFoundErrorMessage
("Article not found"), the identical body either way, so no response distinguishes the two
And a repository failure other than article.ErrArticleNotFound still answers 500 with the text
"Internal server error", regardless of caller state, unchanged
And this slice amends features/verified_account_required.feature: the Examples table of
"An authenticated but unverified user is refused on every protected article route" and of "A
request with no jwt cookie is still refused with 401, never 403" both drop their
"GET /v1/article/1" row, since that route no longer applies RequireVerifiedUser or requires a
cookie at all
And this slice amends features/not_found_errors.feature: "Fetching an offline article still
answers 404 with the same message" now holds only for an anonymous or unverified caller, not
a verified one, and "The article route still requires authentication before existence is even
checked" is replaced by this feature's public-visibility scenarios below
Scenario: An anonymous visitor can read an online article
Given the article repository contains an online article titled "Backyard Renovation Update" with id 1
When the client sends GET /v1/article/1 with no jwt cookie
Then the response status is 200
And the response body is the article's JSON representation
Scenario: An anonymous visitor is told an offline article does not exist
Given the article repository contains an offline article titled "Draft: Kitchen Remodel" with id 2
When the client sends GET /v1/article/2 with no jwt cookie
Then the response status is 404
And the response body is the text "Article not found"
Scenario: An anonymous visitor is told an unknown id does not exist
Given the article repository has no article with id 9999
When the client sends GET /v1/article/9999 with no jwt cookie
Then the response status is 404
And the response body is the text "Article not found"
Scenario: A verified user can open their own offline draft
Given a user repository containing user id 1, is_verified true
And a valid jwt cookie for user id 1
And the article repository contains an offline article titled "Draft: Kitchen Remodel" with id 2
When the client sends GET /v1/article/2 with that cookie
Then the response status is 200
And the response body is the article's JSON representation
Scenario: A verified user still reads an online article normally
Given a user repository containing user id 1, is_verified true
And a valid jwt cookie for user id 1
And the article repository contains an online article titled "Backyard Renovation Update" with id 1
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: A verified user requesting an unknown id gets the same not-found response
Given a user repository containing user id 1, is_verified true
And a valid jwt cookie for user 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: An authenticated but unverified user is told an offline article does not exist, not refused with 403
Given a user repository containing user id 1, is_verified false
And a valid jwt cookie for user id 1
And the article repository contains an offline article titled "Draft: Kitchen Remodel" with id 2
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: A malformed or expired jwt cookie falls back to the public online-only visibility
Given a jwt cookie whose value is not a token issued by JWT_SECRET
And the article repository contains an online article with id 1 and an offline article with id 2
When the client sends GET /v1/article/1 with that cookie
Then the response status is 200
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: An id that isn't a valid uint32 still answers 400, regardless of caller
When the client sends GET /v1/article/not-a-number with no jwt cookie
Then the response status is 400
And the response body is the text apperr.IdUint32ErrorMessage
Scenario Outline: The rest of the article routes still require a verified account
Given no jwt cookie is sent
When the client sends <method> <path>
Then the response status is 401
Examples:
| method | path |
| POST | /v1/article/ |
| DELETE | /v1/article/1 |
| PATCH | /v1/article/1 |
| PATCH | /v1/article/1/publish |
| PATCH | /v1/article/1/unpublish |
Scenario: A repository failure still answers 500 regardless of caller state
Given the article repository fails on its next call to Get
When the client sends GET /v1/article/1 with no jwt cookie
Then the response status is 500
And the response body is the text "Internal server error"
+73
View File
@@ -0,0 +1,73 @@
# QA procedure: public_article_read
Package: `test` (black-box, extending `article_test.go`, `verified_account_required_http_test.go`
and `not_found_errors_test.go`).
## Setup shared by every case
1. Call `validator.InitValidator()` once.
2. Reuse `mockArticleRepo` (`test/article_test.go`) and `mockUserRepo` (`test/verified_account_required_http_test.go`)
as-is; no fake changes expected.
3. Wire the real use cases over the fakes: `useCases.NewGetArticleUseCase`, `useCases.NewGetUserUseCase`,
plus `NewCreateArticleUseCase`/`NewListArticlesUseCase`/`NewDeleteArticleUseCase`/
`NewUpdateArticleUseCase` for the regression routes. Build
`article.NewHandler(create, get, list, delete, update, getUser)`.
4. Build a test `jwtauth.New("HS256", []byte("test-secret"), nil)`.
5. Mount routes on a fresh `chi.NewRouter()` mirroring the amended `api/router.go` shape:
- `r.Route("/v1", func(r chi.Router) { r.Route("/article", func(r chi.Router) { ... }) })`
- `r.Use(jwtauth.Verifier(token))` on the whole `/article` group
- `r.Get("/", articleHandler.ListArticles)` and `r.Get("/{id}", articleHandler.GetArticle)`
directly under that Verifier-only group
- a nested `r.Group` adding `jwtauth.Authenticator(token)` then
`middleware.RequireVerifiedUser(getUserUseCase)`, containing `r.Post("/", ...)`,
`r.Delete("/{id}", ...)`, `r.Patch("/{id}", ...)`, `r.Patch("/{id}/publish", ...)`,
`r.Patch("/{id}/unpublish", ...)`
No live server.
6. Mint a valid jwt cookie for user id 1 via `token.Encode(map[string]any{"user_id": uint32(1)})`.
7. Seed the article repo directly, e.g. `repo.articles[1] = article.Article{ID: 1, Title:
"Backyard Renovation Update", IsOnline: true}` and `repo.articles[2] = article.Article{ID: 2,
Title: "Draft: Kitchen Remodel", IsOnline: false}`.
8. Seed the user repo with `repo.users[1] = user.User{ID: 1, IsVerified: <true|false>}` as each case needs.
## HTTP checks
9. `GET /v1/article/1` (online), no cookie -> 200, decode body, assert `id == 1`.
10. `GET /v1/article/2` (offline), no cookie -> 404, body `"Article not found\n"`.
11. `GET /v1/article/9999` (no article with that id), no cookie -> 404, body `"Article not found\n"`.
12. `GET /v1/article/2`, cookie for user 1 with `IsVerified: true` -> 200, decode body, assert `id == 2`.
13. `GET /v1/article/1`, cookie for user 1 with `IsVerified: true` -> 200, `id == 1`.
14. `GET /v1/article/9999`, cookie for user 1 with `IsVerified: true` -> 404, body `"Article not found\n"`.
15. `GET /v1/article/2`, cookie for user 1 with `IsVerified: false` -> 404, body `"Article not found\n"`
(never 403).
16. `GET /v1/article/1` and `GET /v1/article/2`, cookie named `jwt` whose value is an arbitrary
non-token string (e.g. `"not-a-real-token"`) -> 200 for id 1, 404 for id 2 (same as step 9/10,
proving the invalid cookie falls back to public visibility).
17. `GET /v1/article/not-a-number`, no cookie -> 400, body `apperr.IdUint32ErrorMessage + "\n"`.
18. Regression, no cookie: `POST /v1/article/` -> 401; `DELETE /v1/article/1` -> 401;
`PATCH /v1/article/1` -> 401; `PATCH /v1/article/1/publish` -> 401;
`PATCH /v1/article/1/unpublish` -> 401.
19. Configure the fake repo's `failSwitch` so the next `Get` call returns `errRepositoryFailure`
(`failNext("Get")`). `GET /v1/article/1` with no cookie -> 500, body `"Internal server error\n"`.
## Required amendments to existing tests/specs (part of this slice, not optional cleanup)
20. `test/verified_account_required_http_test.go`: move `r.Get("/{id}", articleHandler.GetArticle)`
in `newVerifiedAccountRequiredRouter` out of the `Authenticator` + `RequireVerifiedUser` group
into the outer Verifier-only group (mirroring step 5 above); remove the
`{http.MethodGet, "/v1/article/1", ""}` row from `protectedRouteCases` and the
`{http.MethodGet, "/v1/article/1"}` row from `noCookieCases` in `TestVerifiedAccountRequiredHttp`,
since that route no longer produces 403 or 401.
21. `test/not_found_errors_test.go`: move `r.Get("/{id}", articleHandler.GetArticle)` in
`newNotFoundErrorsRouter` the same way; change the `"Fetching an offline article still answers
404 with the same message"` subtest to use an unverified or anonymous caller (it currently uses
`newVerifiedUserRepo()`, which will now 200 instead of 404); replace `"The article route still
requires authentication before existence is even checked"` with an assertion that an anonymous
request for an unknown id now answers 404 (covered by step 11 above, so this subtest can instead
be deleted in favor of the new test file, or repurposed to check `GET /v1/article/1` with no
cookie still answers 200 as a cross-file regression note).
`features/verified_account_required.feature` and `features/not_found_errors.feature` get the
corresponding Examples-table/scenario edits described in this feature's Background.
## Manual verification (not exercised by the Go test)
- SPA-visible behaviour: confirm the public front-office single-article view (unauthenticated) can
open a published article and 404s on a draft URL, and the authenticated back-office view can open
its own unpublished draft for editing, against the real embedded frontend and a real SQLite file.
Not exercised by `task test`; requires `task dev` or a built `task build` binary.
- No SMTP involvement.
+5 -2
View File
@@ -54,6 +54,11 @@ Feature: Protected article routes and the offline-inclusive listing require a ve
for the purpose of choosing which list to show, so the listing still answers 200 with the
online-only array rather than failing the request; only a failure inside ListOnlineArticles
or ListAllArticles itself still answers 500, unchanged from features/public_article_listing.feature
And a later slice, features/public_article_read.feature, moves GET /v1/article/{id} out of the
Verifier + Authenticator + RequireVerifiedUser group entirely and into the Verifier-only group
next to GET /v1/article/, so that route no longer answers 401 for a missing cookie or 403 for
an unverified caller; the "GET /v1/article/1" row is therefore removed from both Examples
tables below
Scenario Outline: An authenticated but unverified user is refused on every protected article route
Given a user repository containing user id 1, is_verified false
@@ -65,7 +70,6 @@ Feature: Protected article routes and the offline-inclusive listing require a ve
Examples:
| method | path |
| GET | /v1/article/1 |
| POST | /v1/article/ |
| DELETE | /v1/article/1 |
| PATCH | /v1/article/1 |
@@ -113,7 +117,6 @@ Feature: Protected article routes and the offline-inclusive listing require a ve
Examples:
| method | path |
| GET | /v1/article/1 |
| POST | /v1/article/ |
Scenario: A repository failure while resolving the caller's verification status answers 500
+8 -11
View File
@@ -3,7 +3,6 @@ package test
import (
"RenewCMS/api/handlers/article"
"RenewCMS/api/handlers/auth"
"RenewCMS/api/middleware"
"RenewCMS/api/pkg/apperr"
validator "RenewCMS/api/pkg/validator"
domainArticle "RenewCMS/internal/domain/article"
@@ -150,13 +149,9 @@ func newNotFoundErrorsRouter(articleRepo *mockArticleRepo, userRepo *mockUserRep
r := chi.NewRouter()
r.Route("/v1", func(r chi.Router) {
r.Group(func(r chi.Router) {
r.Route("/article", func(r chi.Router) {
r.Use(jwtauth.Verifier(token))
r.Use(jwtauth.Authenticator(token))
r.Use(middleware.RequireVerifiedUser(getUserUseCase))
r.Route("/article", func(r chi.Router) {
r.Get("/{id}", articleHandler.GetArticle)
})
r.Get("/{id}", articleHandler.GetArticle)
})
r.Route("/auth", func(r chi.Router) {
r.Post("/login", authHandler.Login)
@@ -203,7 +198,6 @@ func TestNotFoundErrorsHttp(t *testing.T) {
t.Run("Fetching an offline article still answers 404 with the same message", func(t *testing.T) {
router := newNotFoundErrorsRouter(newArticleRepo(), newVerifiedUserRepo(), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/2", nil)
setJwtCookie(t, req, token, 1)
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
@@ -234,14 +228,17 @@ func TestNotFoundErrorsHttp(t *testing.T) {
}
})
t.Run("The article route still requires authentication before existence is even checked", func(t *testing.T) {
t.Run("An unknown article id answers 404 even without authentication", 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)
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())
}
})
+1 -2
View File
@@ -56,11 +56,11 @@ func newPublicArticleListingRouter(articleRepo *mockArticleRepo, token *jwtauth.
r.Use(jwtauth.Verifier(token))
r.Group(func(r chi.Router) {
r.Get("/", articleHandler.ListArticles)
r.Get("/{id}", articleHandler.GetArticle)
})
r.Group(func(r chi.Router) {
r.Use(jwtauth.Authenticator(token))
r.Use(middleware.RequireVerifiedUser(getUserUseCase))
r.Get("/{id}", articleHandler.GetArticle)
r.Post("/", articleHandler.PostArticle)
r.Delete("/{id}", articleHandler.DeleteArticle)
})
@@ -155,7 +155,6 @@ func TestPublicArticleListingHttp(t *testing.T) {
path string
body []byte
}{
{"GET /v1/article/1 with no jwt cookie answers 401", http.MethodGet, "/v1/article/1", nil},
{"POST /v1/article/ with no jwt cookie answers 401", http.MethodPost, "/v1/article/", []byte(`{"title":"Valid Title","body":"Some content"}`)},
{"DELETE /v1/article/1 with no jwt cookie answers 401", http.MethodDelete, "/v1/article/1", nil},
}
+268
View File
@@ -0,0 +1,268 @@
package test
import (
"RenewCMS/api/handlers/article"
"RenewCMS/api/middleware"
"RenewCMS/api/pkg/apperr"
validator "RenewCMS/api/pkg/validator"
domainArticle "RenewCMS/internal/domain/article"
domainUser "RenewCMS/internal/domain/user"
"RenewCMS/internal/infrastructure/useCases"
"bytes"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"github.com/go-chi/chi/v5"
"github.com/go-chi/jwtauth/v5"
)
func newPublicArticleReadRouter(articleRepo *mockArticleRepo, userRepo *mockUserRepo, token *jwtauth.JWTAuth) *chi.Mux {
getUserUseCase := useCases.NewGetUserUseCase(userRepo)
articleHandler := article.NewHandler(
useCases.NewCreateArticleUseCase(articleRepo),
useCases.NewGetArticleUseCase(articleRepo),
useCases.NewListArticlesUseCase(articleRepo),
useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()),
useCases.NewUpdateArticleUseCase(articleRepo),
getUserUseCase,
)
r := chi.NewRouter()
r.Route("/v1", func(r chi.Router) {
r.Route("/article", func(r chi.Router) {
r.Use(jwtauth.Verifier(token))
r.Get("/", articleHandler.ListArticles)
r.Get("/{id}", articleHandler.GetArticle)
r.Group(func(r chi.Router) {
r.Use(jwtauth.Authenticator(token))
r.Use(middleware.RequireVerifiedUser(getUserUseCase))
r.Post("/", articleHandler.PostArticle)
r.Delete("/{id}", articleHandler.DeleteArticle)
r.Patch("/{id}", articleHandler.UpdateArticleBody)
r.Patch("/{id}/publish", articleHandler.PublishArticle)
r.Patch("/{id}/unpublish", articleHandler.UnpublishArticle)
})
})
})
return r
}
func seedPublicArticleReadRepo() *mockArticleRepo {
repo := newMockArticleRepo()
repo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update", IsOnline: true}
repo.articles[2] = domainArticle.Article{ID: 2, Title: "Draft: Kitchen Remodel", IsOnline: false}
return repo
}
func newPublicArticleReadUserRepo(isVerified bool) *mockUserRepo {
repo := newMockUserRepo()
repo.users[1] = domainUser.User{ID: 1, IsVerified: isVerified}
return repo
}
func mintPublicArticleReadCookie(t *testing.T, token *jwtauth.JWTAuth, userId uint32) *http.Cookie {
t.Helper()
_, tokenString, err := token.Encode(map[string]any{"user_id": userId})
if err != nil {
t.Fatalf("failed to encode jwt: %v", err)
}
return &http.Cookie{Name: "jwt", Value: tokenString}
}
func assertPublicArticleReadResponse(t *testing.T, rec *httptest.ResponseRecorder, expectedId uint32) {
t.Helper()
if rec.Code != http.StatusOK {
t.Fatalf("expected status 200, got %d, body %q", rec.Code, rec.Body.String())
}
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 != expectedId {
t.Errorf("expected article id %d, got %d", expectedId, got.ID)
}
}
func assertPublicArticleReadNotFound(t *testing.T, rec *httptest.ResponseRecorder) {
t.Helper()
if rec.Code != http.StatusNotFound {
t.Fatalf("expected status 404, got %d, body %q", rec.Code, rec.Body.String())
}
if rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" {
t.Errorf("unexpected body: %q", rec.Body.String())
}
}
func TestPublicArticleReadHttp(t *testing.T) {
validator.InitValidator()
token := jwtauth.New("HS256", []byte("test-secret"), nil)
t.Run("An anonymous visitor can read an online article", func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newMockUserRepo(), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/1", nil)
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
assertPublicArticleReadResponse(t, rec, 1)
})
t.Run("An anonymous visitor is told an offline article does not exist", func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newMockUserRepo(), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/2", nil)
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
assertPublicArticleReadNotFound(t, rec)
})
t.Run("An anonymous visitor is told an unknown id does not exist", func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newMockUserRepo(), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/9999", nil)
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
assertPublicArticleReadNotFound(t, rec)
})
t.Run("A verified user can open their own offline draft", func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newPublicArticleReadUserRepo(true), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/2", nil)
req.AddCookie(mintPublicArticleReadCookie(t, token, 1))
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
assertPublicArticleReadResponse(t, rec, 2)
})
t.Run("A verified user still reads an online article normally", func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newPublicArticleReadUserRepo(true), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/1", nil)
req.AddCookie(mintPublicArticleReadCookie(t, token, 1))
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
assertPublicArticleReadResponse(t, rec, 1)
})
t.Run("A verified user requesting an unknown id gets the same not-found response", func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newPublicArticleReadUserRepo(true), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/9999", nil)
req.AddCookie(mintPublicArticleReadCookie(t, token, 1))
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
assertPublicArticleReadNotFound(t, rec)
})
t.Run("An authenticated but unverified user is told an offline article does not exist, not refused with 403", func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newPublicArticleReadUserRepo(false), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/2", nil)
req.AddCookie(mintPublicArticleReadCookie(t, token, 1))
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
assertPublicArticleReadNotFound(t, rec)
})
t.Run("A malformed or expired jwt cookie falls back to the public online-only visibility", func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newMockUserRepo(), token)
cookie := &http.Cookie{Name: "jwt", Value: "not-a-real-token"}
onlineReq := httptest.NewRequest(http.MethodGet, "/v1/article/1", nil)
onlineReq.AddCookie(cookie)
onlineRec := httptest.NewRecorder()
router.ServeHTTP(onlineRec, onlineReq)
if onlineRec.Code != http.StatusOK {
t.Fatalf("expected status 200, got %d, body %q", onlineRec.Code, onlineRec.Body.String())
}
offlineReq := httptest.NewRequest(http.MethodGet, "/v1/article/2", nil)
offlineReq.AddCookie(cookie)
offlineRec := httptest.NewRecorder()
router.ServeHTTP(offlineRec, offlineReq)
assertPublicArticleReadNotFound(t, offlineRec)
})
t.Run("An id that isn't a valid uint32 still answers 400, regardless of caller", func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newMockUserRepo(), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/not-a-number", nil)
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != http.StatusBadRequest {
t.Fatalf("expected status 400, got %d, body %q", rec.Code, rec.Body.String())
}
if rec.Body.String() != apperr.IdUint32ErrorMessage+"\n" {
t.Errorf("unexpected body: %q", rec.Body.String())
}
})
protectedRouteCases := []struct {
method string
path string
}{
{http.MethodPost, "/v1/article/"},
{http.MethodDelete, "/v1/article/1"},
{http.MethodPatch, "/v1/article/1"},
{http.MethodPatch, "/v1/article/1/publish"},
{http.MethodPatch, "/v1/article/1/unpublish"},
}
for _, tc := range protectedRouteCases {
t.Run("The rest of the article routes still require a verified account: "+tc.method+" "+tc.path, func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newMockUserRepo(), token)
req := httptest.NewRequest(tc.method, tc.path, nil)
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != http.StatusUnauthorized {
t.Fatalf("expected status 401, got %d, body %q", rec.Code, rec.Body.String())
}
})
}
t.Run("A response write failure is reported instead of crashing the handler", func(t *testing.T) {
router := newPublicArticleReadRouter(seedPublicArticleReadRepo(), newMockUserRepo(), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/1", nil)
originalStdout := os.Stdout
readEnd, writeEnd, err := os.Pipe()
if err != nil {
t.Fatalf("failed to create pipe: %v", err)
}
os.Stdout = writeEnd
router.ServeHTTP(&erroringResponseWriter{}, req)
writeEnd.Close()
os.Stdout = originalStdout
var captured bytes.Buffer
if _, err := io.Copy(&captured, readEnd); err != nil {
t.Fatalf("failed to read captured stdout: %v", err)
}
if !strings.Contains(captured.String(), "Failed to write response") {
t.Errorf("expected the write failure to be reported, got %q", captured.String())
}
})
t.Run("A repository failure still answers 500 regardless of caller state", func(t *testing.T) {
repo := seedPublicArticleReadRepo()
repo.fail.failNext("Get")
router := newPublicArticleReadRouter(repo, newMockUserRepo(), token)
req := httptest.NewRequest(http.MethodGet, "/v1/article/1", nil)
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != http.StatusInternalServerError {
t.Fatalf("expected status 500, got %d, body %q", rec.Code, rec.Body.String())
}
if rec.Body.String() != "Internal server error\n" {
t.Errorf("unexpected body: %q", rec.Body.String())
}
})
}
+1 -3
View File
@@ -42,10 +42,10 @@ func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mo
r.Route("/article", func(r chi.Router) {
r.Use(jwtauth.Verifier(token))
r.Get("/", articleHandler.ListArticles)
r.Get("/{id}", articleHandler.GetArticle)
r.Group(func(r chi.Router) {
r.Use(jwtauth.Authenticator(token))
r.Use(middleware.RequireVerifiedUser(getUserUseCase))
r.Get("/{id}", articleHandler.GetArticle)
r.Post("/", articleHandler.PostArticle)
r.Delete("/{id}", articleHandler.DeleteArticle)
r.Patch("/{id}", articleHandler.UpdateArticleBody)
@@ -97,7 +97,6 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
path string
body string
}{
{http.MethodGet, "/v1/article/1", ""},
{http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`},
{http.MethodDelete, "/v1/article/1", ""},
{http.MethodPatch, "/v1/article/1", `{"body": "Updated"}`},
@@ -222,7 +221,6 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
method string
path string
}{
{http.MethodGet, "/v1/article/1"},
{http.MethodPost, "/v1/article/"},
}
for _, tc := range noCookieCases {