Wrap /user/{username} in RequireAuth middleware (closes #60) #71
Reference in New Issue
Block a user
Delete Branch "issue-60-user-requireauth"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Enforces authentication for the
/user/{username}route group at the middleware layer, matching every other authenticated route group.Changes
internal/server/routes.go(setupUserRoutes): addedr.Use(s.mw.RequireAuth())immediately after the existingr.Use(s.mw.CSRF())on the/user/{username}group, so auth is enforced by design (CSRF first, then RequireAuth) — consistent with/sourcesand/source/{sourceID}.internal/handlers/profile.go(HandleProfile): removed the now-dead unauthenticated-redirect branch (RequireAuth guarantees an authenticated session before the handler runs). The handler still reads the username and user id from the session for the own-profile-only check; a request for another user's profile still returns 403. The session-retrieval error is now handled as a 500.Tests (
internal/handlers/profile_test.go)/user/{username}is redirected to/pages/loginat the middleware layer and never reaches the endpoint handler (routing-level test replicating the CSRF + RequireAuth chain)Validation
docker build .(fmt-check, lint, test, build) passes.Closes #60
What changed
internal/server/routes.go— insetupUserRoutes(), addedr.Use(s.mw.RequireAuth())immediately after the existingr.Use(s.mw.CSRF())on the/user/{username}route group. Relative order is CSRF first, then RequireAuth, matching/sourcesand/source/{sourceID}.internal/handlers/profile.go— inHandleProfile, removed the now-unreachable "not authenticated -> redirect to/pages/login" branch (RequireAuth guarantees an authenticated session before the handler runs). The handler still reads the username and user id from the session for the own-profile-only comparison and template data; a request for another user's profile still returns 403. A session-retrieval error is now surfaced as a 500 rather than a redirect.internal/handlers/profile_test.go(new) — three tests:TestHandleProfile_OwnProfile_OK: own profile returns 200TestHandleProfile_OtherProfile_Forbidden: another user's profile returns 403TestUserRoute_Unauthenticated_RedirectedByMiddleware: replicates the route group's CSRF + RequireAuth chain and asserts an unauthenticated request to/user/{username}returns 303 to/pages/loginand never reaches the endpoint handlerValidation
docker build .(runs fmt-check, lint, test, build) completed successfully:Build exited 0; no lint findings, all tests pass.
Independent review — PASS (merge-ready)
Reviewed the diff against the issue spec and repo policies; posture was adversarial.
Correctness / spec
internal/server/routes.gosetupUserRoutes()now appliesRequireAuth()immediately afterCSRF()on/user/{username}, matching the/sourcesand/source/{sourceID}ordering. Meets the definition of done.internal/handlers/profile.goHandleProfiledrops the now-dead unauthenticated-redirect branch; asession.Geterror returns 500; the own-profile-only 403 is unchanged. Correct givenRequireAuthruns first./pages/loginwithout reaching the handler.Policy
make fmtclean,docker build .green (fmt-check + lint + test + build). No AI/tooling references. Commit subject closes the issue.One non-blocking observation
TestUserRoute_Unauthenticated_RedirectedByMiddlewarereconstructs the route group (CSRFthenRequireAuth) inside the test rather than exercising the productionsetupUserRoutes, so it would not catch a future accidental removal of theRequireAuthline fromroutes.go. It does correctly prove the middleware chain's behaviour. Acceptable for a one-line wiring change given there is no full-server route test seam today; worth tightening if such a seam is added later.Verdict: meets the bar. Marking merge-ready and handing to @sneak for final review.