Cors Issue Resolved#5
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the CORS configuration to reflect the request origin and enables credential support. The review identifies a critical security vulnerability where allowing credentials while reflecting any origin could lead to unauthorized authenticated requests. Additionally, the explicit inclusion of the OPTIONS method is noted as redundant.
| if (!process.env.CORS_ORIGINS?.trim()) return true; // true = reflect any origin | ||
| const list = process.env.CORS_ORIGINS.split(',') | ||
| .map((o) => o.trim()) | ||
| .filter(Boolean); | ||
| return list.length > 0 ? list : ['*']; | ||
| return list.length > 0 ? list : true; |
There was a problem hiding this comment.
Reflecting any origin by returning true is a security risk when credentials: true is enabled in the CORS configuration. This allows any website to make authenticated requests to your API on behalf of a user. It is highly recommended to use a specific whitelist of allowed domains instead of reflecting the request origin.
| 'tenantid', | ||
| 'academicyearid', | ||
| ], | ||
| credentials: true, |
There was a problem hiding this comment.
Enabling credentials: true while the origin is configured to reflect any requester (via true in app.config.ts) creates a security vulnerability. This configuration allows any malicious website to perform authenticated actions on behalf of your users. If credentials are required, the origin must be restricted to a trusted whitelist.
| app.enableCors({ | ||
| origin: corsOrigins, | ||
| methods: ['GET', 'POST', 'PUT', 'DELETE', 'PATCH'], | ||
| methods: ['GET', 'POST', 'PUT', 'DELETE', 'PATCH', 'OPTIONS'], |
There was a problem hiding this comment.
No description provided.