refac: authenticated user can list all articles, including offline ones, while the public list only shows online ones
This commit is contained in:
1 parent
6a2ca07b66
commit
790d4e6da6
5 files changed
+345
-7
No files matched your search
@@ -13,6 +13,7 @@ import (
|
|||||||
"strconv"
|
"strconv"
|
||||||
|
|
||||||
"github.com/go-chi/chi/v5"
|
"github.com/go-chi/chi/v5"
|
||||||
|
"github.com/go-chi/jwtauth/v5"
|
||||||
)
|
)
|
||||||
|
|
||||||
type Handler struct {
|
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) {
|
func requestHasValidJwt(r *http.Request) bool {
|
||||||
articles, err := h.ListUseCase.ListOnlineArticles()
|
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 {
|
if err != nil {
|
||||||
http.Error(w, "Failed to retrieve articles", http.StatusInternalServerError)
|
http.Error(w, "Failed to retrieve articles", http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
|
|||||||
+9
-5
@@ -33,10 +33,15 @@ func NewArticleRouter(container *UseCases) http.Handler {
|
|||||||
|
|
||||||
articleHandler := article.NewHandler(container.CreateArticleUseCase, container.GetArticleUseCase, container.ListArticlesUseCase, container.DeleteArticleUseCase)
|
articleHandler := article.NewHandler(container.CreateArticleUseCase, container.GetArticleUseCase, container.ListArticlesUseCase, container.DeleteArticleUseCase)
|
||||||
|
|
||||||
r.Get("/{id}", articleHandler.GetArticle)
|
r.Group(func(r chi.Router) {
|
||||||
r.Post("/", articleHandler.PostArticle)
|
r.Get("/", articleHandler.ListArticles)
|
||||||
r.Get("/", articleHandler.ListArticles)
|
})
|
||||||
r.Delete("/{id}", articleHandler.DeleteArticle)
|
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
|
return r
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -61,7 +66,6 @@ func InitBackendRoutes(container *UseCases) *chi.Mux {
|
|||||||
r.Get("/", GetHelloWorld)
|
r.Get("/", GetHelloWorld)
|
||||||
r.Group(func(r chi.Router) {
|
r.Group(func(r chi.Router) {
|
||||||
r.Use(jwtauth.Verifier(container.Token))
|
r.Use(jwtauth.Verifier(container.Token))
|
||||||
r.Use(jwtauth.Authenticator(container.Token))
|
|
||||||
r.Mount("/article", NewArticleRouter(container))
|
r.Mount("/article", NewArticleRouter(container))
|
||||||
})
|
})
|
||||||
r.Mount("/auth", NewAuthRouter(container))
|
r.Mount("/auth", NewAuthRouter(container))
|
||||||
|
|||||||
@@ -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 <method> <path>
|
||||||
|
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"
|
||||||
@@ -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.
|
||||||
@@ -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())
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
Reference in new issue
Block a user