[server] move jwt auth middleware to the root#4
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughCentralized JWT authentication: jwtauth.New is now provided via DI and applied globally to the API v1 middleware chain (with login/register bypass); individual handlers remove per-route jwtauth.New registrations and rely on the global middleware plus per-route role guards. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as Client
participant API as "API v1 Middleware"
participant JWT as "jwtauth (keyauth middleware)"
participant Users as "Users Service"
participant Handler as "Route Handler"
Client->>API: HTTP request (/api/v1/...)
API->>JWT: invoke global jwtauth
alt path is /auth/login or /auth/register
JWT-->>API: Next() -> skip validation (rgba(0,128,0,0.5))
API->>Handler: call route handler (no user in context)
else other paths
JWT->>JWT: validate token, parse claims (rgba(0,0,255,0.5))
JWT->>Users: load user by claims.userID (rgba(255,165,0,0.5))
Users-->>JWT: user or not found
alt user missing or invalid
JWT-->>API: respond 401/403
else user valid
JWT-->>API: set user in context
API->>Handler: call route handler (user available)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
🤖 Pull request artifacts
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/server/middlewares/jwtauth/jwtauth.go`:
- Around line 22-28: The Next function in jwtauth.go uses exact equality on
c.Path(), which fails for requests with trailing slashes; update the Next
callback to normalize the request path before comparing (e.g., trim trailing
slashes or canonicalize the path) and then check against the public endpoints
(login/register) so requests like "/api/v1/auth/login/" are recognized as
public; locate the Next func in jwtauth.go and replace the direct c.Path()
comparisons with a normalizedPath variable (using a trim or canonicalize
approach) and compare that to the known public paths.
In `@internal/server/module.go`:
- Around line 61-65: The middleware chain currently registers
jwtauth.ErrorsHandler() after jwtAuth so keyauth errors returned by jwtAuth
bypass the error translator; change the order in the v1.Use call so
jwtauth.ErrorsHandler() is placed before jwtAuth (keep validation.Middleware
as-is) so ErrorsHandler wraps jwtAuth and can convert
ErrInvalidToken/ErrExpiredToken into proper 401 responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c06a8d7f-9e7b-4db6-93cd-e2180e6bf285
📒 Files selected for processing (5)
internal/server/admin/users/handler.gointernal/server/auth/handler.gointernal/server/middlewares/jwtauth/jwtauth.gointernal/server/module.gointernal/server/projects/handler.go
💤 Files with no reviewable changes (3)
- internal/server/auth/handler.go
- internal/server/admin/users/handler.go
- internal/server/projects/handler.go
63f9e32 to
fe3c207
Compare
Summary by CodeRabbit