diff --git a/api/handlers/article/articleHandler.go b/api/handlers/article/articleHandler.go index f585ed0..e08c0b2 100644 --- a/api/handlers/article/articleHandler.go +++ b/api/handlers/article/articleHandler.go @@ -13,6 +13,7 @@ import ( "strconv" "github.com/go-chi/chi/v5" + "github.com/go-chi/jwtauth/v5" ) type Handler struct { @@ -101,8 +102,19 @@ func (h *Handler) PostArticle(w http.ResponseWriter, r *http.Request) { } } -func (h *Handler) ListArticles(w http.ResponseWriter, _ *http.Request) { - articles, err := h.ListUseCase.ListOnlineArticles() +func requestHasValidJwt(r *http.Request) bool { + token, _, err := jwtauth.FromContext(r.Context()) + return token != nil && err == nil +} + +func (h *Handler) ListArticles(w http.ResponseWriter, r *http.Request) { + var articles []domain.Article + var err error + if requestHasValidJwt(r) { + articles, err = h.ListUseCase.ListAllArticles() + } else { + articles, err = h.ListUseCase.ListOnlineArticles() + } if err != nil { http.Error(w, "Failed to retrieve articles", http.StatusInternalServerError) return diff --git a/api/router.go b/api/router.go index 6153e10..016a110 100644 --- a/api/router.go +++ b/api/router.go @@ -33,10 +33,15 @@ func NewArticleRouter(container *UseCases) http.Handler { articleHandler := article.NewHandler(container.CreateArticleUseCase, container.GetArticleUseCase, container.ListArticlesUseCase, container.DeleteArticleUseCase) - r.Get("/{id}", articleHandler.GetArticle) - r.Post("/", articleHandler.PostArticle) - r.Get("/", articleHandler.ListArticles) - r.Delete("/{id}", articleHandler.DeleteArticle) + r.Group(func(r chi.Router) { + r.Get("/", articleHandler.ListArticles) + }) + r.Group(func(r chi.Router) { + r.Use(jwtauth.Authenticator(container.Token)) + r.Get("/{id}", articleHandler.GetArticle) + r.Post("/", articleHandler.PostArticle) + r.Delete("/{id}", articleHandler.DeleteArticle) + }) return r } @@ -61,7 +66,6 @@ func InitBackendRoutes(container *UseCases) *chi.Mux { r.Get("/", GetHelloWorld) r.Group(func(r chi.Router) { r.Use(jwtauth.Verifier(container.Token)) - r.Use(jwtauth.Authenticator(container.Token)) r.Mount("/article", NewArticleRouter(container)) }) r.Mount("/auth", NewAuthRouter(container)) diff --git a/features/public_article_listing.feature b/features/public_article_listing.feature new file mode 100644 index 0000000..86c1c4a --- /dev/null +++ b/features/public_article_listing.feature @@ -0,0 +1,64 @@ +Feature: Public and authenticated article listing + + As a visitor of the public site and as an authenticated RenewCMS user + I want GET /v1/article/ to return every online article to anyone, + and every article, including offline ones, to a caller with a valid jwt cookie + So that the public site never displays a draft, while an authenticated user managing + the CMS can see and act on articles that are not published yet + + Background: + Given internal/domain/article.Article has a json "is_online" field + And ListArticlesUseCase.ListOnlineArticles filters article.Filters{IsOnline: true} and + ListArticlesUseCase.ListAllArticles applies no filter, both already implemented + And before this slice, GET /v1/article/ sits inside the jwtauth Verifier + Authenticator group + 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 + 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 + requiring the router-level Authenticator middleware; article.NewHandler takes no token + And the response body is the JSON array article.Article marshals to, each element carrying its + own "is_online" value + + Scenario: An anonymous visitor only sees online articles + Given the article repository contains an online article titled "Backyard Renovation Update" with id 1 + And the same repository contains an offline article titled "Draft: Kitchen Remodel" with id 2 + When the client sends GET /v1/article/ with no jwt cookie + Then the response status is 200 + And the response body is a JSON array containing exactly the article with id 1 + And the article with id 2 is absent from the response body + + Scenario: An authenticated user sees every article, online and offline + Given a valid jwt cookie for a user with id 1 + And the article repository contains an online article titled "Backyard Renovation Update" with id 1 + And the same repository contains an offline article titled "Draft: Kitchen Remodel" with id 2 + When the client sends GET /v1/article/ with that cookie + Then the response status is 200 + And the response body is a JSON array containing both the article with id 1 and the article with id 2 + + Scenario: A malformed or expired jwt cookie falls back to the public online-only list + 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/ with that cookie + Then the response status is 200 + And the response body is a JSON array containing exactly the article with id 1 + + Scenario Outline: The rest of the article routes still require authentication + Given no jwt cookie is sent + When the client sends + Then the response status is 401 + + Examples: + | method | path | + | GET | /v1/article/1 | + | POST | /v1/article/ | + | DELETE | /v1/article/1 | + + Scenario: A repository failure still answers 500 regardless of auth state + Given the article repository fails on its next call to FindByFilters + When the client sends GET /v1/article/ with no jwt cookie + Then the response status is 500 + And the response body is the text "Failed to retrieve articles" diff --git a/features/public_article_listing.qa.md b/features/public_article_listing.qa.md new file mode 100644 index 0000000..74e8e83 --- /dev/null +++ b/features/public_article_listing.qa.md @@ -0,0 +1,54 @@ +# QA procedure: public_article_listing + +Package: `test` (black-box, extending `article_test.go`). + +## Setup shared by every case +1. Call `validator.InitValidator()` once. +2. Reuse `mockArticleRepo` (`test/article_test.go`) as-is; `FindByFilters` and the `failSwitch` + already support everything this slice needs, no fake changes expected. +3. Wire the real use cases over the fake: `useCases.NewCreateArticleUseCase`, + `useCases.NewGetArticleUseCase`, `useCases.NewListArticlesUseCase`, + `useCases.NewDeleteArticleUseCase`. Build `article.NewHandler(create, get, list, delete)`. +4. Build a test `jwtauth.New("HS256", []byte("test-secret"), nil)`. +5. Mount the routes on a fresh `chi.NewRouter()` mirroring the new `api/router.go` shape: + - `r.Route("/article", func(r chi.Router) { ... })` + - `r.Use(jwtauth.Verifier(token))` on the whole `/article` group (parses, never rejects) + - `r.Get("/", articleHandler.ListArticles)` directly under that Verifier-only group + - a nested `r.Group` adding `jwtauth.Authenticator(token)`, containing + `r.Get("/{id}", articleHandler.GetArticle)`, `r.Post("/", articleHandler.PostArticle)`, + `r.Delete("/{id}", articleHandler.DeleteArticle)` + No live server. +6. Mint a valid jwt cookie for user id 1: `token.Encode(map[string]any{"user_id": uint32(1)})`, + set as the `jwt` cookie on the relevant `httptest.NewRequest`. +7. Seed the repo for the listing cases via `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}` (same direct-map pattern as + `TestGetOnlineArticle`). + +## HTTP checks +8. `GET /v1/article/` with no cookie, repo seeded per step 7 -> status `200`; decode the JSON array + and assert it has exactly one element, id `1`; assert no element has id `2`. +9. `GET /v1/article/` with the valid cookie from step 6, same seed -> status `200`; decode the JSON + array and assert it has exactly two elements, ids `1` and `2` both present. +10. `GET /v1/article/` with a cookie named `jwt` whose value is an arbitrary non-token string (e.g. + `"not-a-real-token"`), same seed -> status `200`; assert the same single-element (id `1`) result + as step 8, proving an invalid cookie falls back to the public list rather than erroring. +11. `GET /v1/article/1` with no cookie -> status `401` (regression: single-article read still gated). +12. `POST /v1/article/` with no cookie, JSON body `{"title":"Valid Title","body":"Some content"}` -> + status `401` (regression: create still gated). +13. `DELETE /v1/article/1` with no cookie -> status `401` (regression: delete still gated). +14. Configure the fake repo's `failSwitch` so the next `FindByFilters` call returns + `errRepositoryFailure` (existing `failNext("FindByFilters")` helper). `GET /v1/article/` with no + cookie -> status `500`, body `"Failed to retrieve articles\n"`. +15. As a regression on the valid-cookie path from step 9, also assert `GET /v1/article/{id}` and + `POST`/`DELETE` still succeed as before with a valid cookie (existing coverage in + `article_test.go` / `article_title_rules_test.go` / `write_not_found_errors_test.go` already + covers this; no new assertions needed here beyond confirming the router restructuring didn't + regress them -- rerun those suites as part of `task test`). + +## Manual verification (not exercised by the Go test) +- SPA-visible behaviour: confirm the public front-office article list view (unauthenticated) only + renders online articles, and the authenticated back-office article list view renders offline + drafts too, 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. diff --git a/test/public_article_listing_test.go b/test/public_article_listing_test.go new file mode 100644 index 0000000..9e85b03 --- /dev/null +++ b/test/public_article_listing_test.go @@ -0,0 +1,204 @@ +package test + +import ( + "RenewCMS/api/handlers/article" + domainArticle "RenewCMS/internal/domain/article" + "RenewCMS/internal/infrastructure/useCases" + "bytes" + "encoding/json" + "errors" + "log" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/jwtauth/v5" + + validator "RenewCMS/api/pkg/validator" +) + +type erroringResponseWriter struct { + header http.Header +} + +func (w *erroringResponseWriter) Header() http.Header { + if w.header == nil { + w.header = http.Header{} + } + return w.header +} + +func (w *erroringResponseWriter) Write([]byte) (int, error) { + return 0, errors.New("simulated write failure") +} + +func (w *erroringResponseWriter) WriteHeader(int) {} + +func newPublicArticleListingRouter(articleRepo *mockArticleRepo, token *jwtauth.JWTAuth) *chi.Mux { + articleHandler := article.NewHandler( + useCases.NewCreateArticleUseCase(articleRepo), + useCases.NewGetArticleUseCase(articleRepo), + useCases.NewListArticlesUseCase(articleRepo), + useCases.NewDeleteArticleUseCase(articleRepo), + ) + + r := chi.NewRouter() + r.Route("/v1", func(r chi.Router) { + r.Route("/article", func(r chi.Router) { + r.Use(jwtauth.Verifier(token)) + r.Group(func(r chi.Router) { + r.Get("/", articleHandler.ListArticles) + }) + r.Group(func(r chi.Router) { + r.Use(jwtauth.Authenticator(token)) + r.Get("/{id}", articleHandler.GetArticle) + r.Post("/", articleHandler.PostArticle) + r.Delete("/{id}", articleHandler.DeleteArticle) + }) + }) + }) + return r +} + +func seedPublicArticleListingRepo() *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 TestPublicArticleListingHttp(t *testing.T) { + validator.InitValidator() + token := jwtauth.New("HS256", []byte("test-secret"), nil) + + t.Run("An anonymous visitor only sees online articles", func(t *testing.T) { + router := newPublicArticleListingRouter(seedPublicArticleListingRepo(), token) + req := httptest.NewRequest(http.MethodGet, "/v1/article/", nil) + 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 len(got) != 1 { + t.Fatalf("expected exactly one article, got %d", len(got)) + } + if got[0].ID != 1 { + t.Errorf("expected article id 1, got %d", got[0].ID) + } + }) + + t.Run("An authenticated user sees every article, online and offline", func(t *testing.T) { + router := newPublicArticleListingRouter(seedPublicArticleListingRepo(), token) + req := httptest.NewRequest(http.MethodGet, "/v1/article/", 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 len(got) != 2 { + t.Fatalf("expected exactly two articles, got %d", len(got)) + } + ids := map[uint32]bool{} + for _, a := range got { + ids[a.ID] = true + } + if !ids[1] || !ids[2] { + t.Errorf("expected ids 1 and 2, got %v", got) + } + }) + + t.Run("A malformed or expired jwt cookie falls back to the public online-only list", func(t *testing.T) { + router := newPublicArticleListingRouter(seedPublicArticleListingRepo(), token) + req := httptest.NewRequest(http.MethodGet, "/v1/article/", nil) + req.AddCookie(&http.Cookie{Name: "jwt", Value: "not-a-real-token"}) + 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 len(got) != 1 { + t.Fatalf("expected exactly one article, got %d", len(got)) + } + if got[0].ID != 1 { + t.Errorf("expected article id 1, got %d", got[0].ID) + } + }) + + restRouteCases := []struct { + name string + method string + 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}, + } + for _, tc := range restRouteCases { + t.Run(tc.name, func(t *testing.T) { + router := newPublicArticleListingRouter(seedPublicArticleListingRepo(), token) + var req *http.Request + if tc.body != nil { + req = httptest.NewRequest(tc.method, tc.path, bytes.NewReader(tc.body)) + req.Header.Set("Content-Type", "application/json") + } else { + 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", rec.Code) + } + }) + } + + t.Run("A response write failure is logged instead of crashing the handler", func(t *testing.T) { + var logOutput strings.Builder + defaultLogOutput := log.Writer() + log.SetOutput(&logOutput) + defer log.SetOutput(defaultLogOutput) + + router := newPublicArticleListingRouter(seedPublicArticleListingRepo(), token) + req := httptest.NewRequest(http.MethodGet, "/v1/article/", nil) + router.ServeHTTP(&erroringResponseWriter{}, req) + + if !strings.Contains(logOutput.String(), "Failed to write response") { + t.Errorf("expected the write failure to be logged, got %q", logOutput.String()) + } + }) + + t.Run("A repository failure still answers 500 regardless of auth state", func(t *testing.T) { + repo := seedPublicArticleListingRepo() + repo.fail.failNext("FindByFilters") + router := newPublicArticleListingRouter(repo, token) + req := httptest.NewRequest(http.MethodGet, "/v1/article/", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected status 500, got %d", rec.Code) + } + if rec.Body.String() != "Failed to retrieve articles\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) +}