Secure backend data access with signed auth tokens and RBAC#48
Secure backend data access with signed auth tokens and RBAC#48fuzziecoder merged 1 commit intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR implements Issue Changes
Sequence DiagramsequenceDiagram
participant User as User/Client
participant Server as Server
participant Auth as Auth System
User->>Server: POST /api/login (credentials)
Server->>Auth: generateAuthToken(user)
Auth->>Auth: signToken(payload, secret)
Auth->>Auth: toBase64Url(signature)
Auth-->>Server: signed token
Server-->>User: { token: "signed.bearer.token" }
User->>Server: GET /api/orders (Authorization: Bearer token)
Server->>Auth: verifyAuthToken(token, secret)
Auth->>Auth: validate signature in constant time
Auth-->>Server: { userId, role, isValid }
Server->>Server: enforce role-based access (user vs admin)
Server-->>User: filtered orders
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } | ||
|
|
||
| if (method === 'GET' && path.startsWith('/api/bills/')) { | ||
| const authedUser = getUserFromAuthHeader(req.headers.authorization); | ||
| if (!authedUser || authedUser.role !== 'admin') { | ||
| sendJson(res, 403, { error: 'Forbidden' }); | ||
| return; |
There was a problem hiding this comment.
🟡 Admin-only endpoints return 403 instead of 401 for unauthenticated requests
The GET /api/bills/:spotId and DELETE /api/users/:userId endpoints return HTTP 403 (Forbidden) when the request has no token or an invalid/expired token, instead of HTTP 401 (Unauthorized). Per HTTP semantics, 401 means "you are not authenticated" while 403 means "you are authenticated but lack permission." Returning 403 for unauthenticated requests conflates these two conditions.
Root Cause and Impact
At backend/server.js:339-345 and backend/server.js:350-354, the check if (!authedUser || authedUser.role !== 'admin') collapses both the unauthenticated case (!authedUser) and the unauthorized case (role !== 'admin') into a single 403 response.
Compare with the other protected endpoints like GET /api/orders at backend/server.js:252-256 and POST /api/orders at backend/server.js:300-303, which correctly return 401 first for unauthenticated users, and only check authorization afterward.
For example, a request with no Authorization header to GET /api/bills/spot-1 returns {"error": "Forbidden"} with status 403, when it should return {"error": "Unauthorized"} with status 401. This makes it harder for API clients to distinguish between "need to log in" vs "logged in but insufficient privileges," and breaks the pattern established by the other endpoints in this same PR.
| } | |
| if (method === 'GET' && path.startsWith('/api/bills/')) { | |
| const authedUser = getUserFromAuthHeader(req.headers.authorization); | |
| if (!authedUser || authedUser.role !== 'admin') { | |
| sendJson(res, 403, { error: 'Forbidden' }); | |
| return; | |
| const authedUser = getUserFromAuthHeader(req.headers.authorization); | |
| if (!authedUser) { | |
| sendJson(res, 401, { error: 'Unauthorized' }); | |
| return; | |
| } | |
| if (authedUser.role !== 'admin') { | |
| sendJson(res, 403, { error: 'Forbidden' }); | |
| return; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Motivation
Description
demo-token-*tokens with HMAC-SHA256 signed bearer tokens that includesub,role, andexpclaims and added constant-time signature verification and expiry checks inbackend/server.js.GET /api/ordersandGET /api/orders/:idrestrict non-admins to their own orders,POST /api/ordersblocks non-admin creation for other users, andGET /api/bills/:spotIdandDELETE /api/users/:userIdare admin-only; updated CORS to allowAuthorizationheader and added env-driven origin viaCORS_ALLOW_ORIGIN.generateAuthToken/verifyAuthTokenand new env-configurable settingsAUTH_TOKEN_SECRETandAUTH_TOKEN_TTL_SECONDSinbackend/server.js.backend/env.jsto ESM import style and expanded thezodenv schema to include the new security and rate-limit variables, and updatedbackend/README.mdto document the new auth behavior and env vars.backend/server.js,backend/env.js,backend/README.md.Testing
node --check backend/server.jswhich succeeded.node --check backend/env.jswhich succeeded.dotenvnot installed), so the end-to-end smoke test did not complete.Codex Task
Summary by CodeRabbit
New Features
Configuration