-
-
Notifications
You must be signed in to change notification settings - Fork 70
fix(#218): allow multiple CORS origins on Next.js server and update dependencies #263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe middleware for the Next.js backend server was refactored to support multiple allowed CORS origins by reading a comma-separated list from an environment variable. The function signature was updated to accept a request parameter, and CORS headers were set dynamically based on the request's origin. Additionally, a development dependency was updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware
participant NextApp
Client->>Middleware: Sends HTTP request (with Origin header)
Middleware->>Middleware: Extracts Origin header
Middleware->>Middleware: Checks if Origin is in allowed list
alt OPTIONS preflight
Middleware-->>Client: Responds with CORS headers (if allowed)
else Other methods
Middleware->>NextApp: Forwards request
NextApp-->>Middleware: Returns response
Middleware-->>Client: Responds with CORS headers (if allowed)
end
Assessment against linked issues
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/cli/templates/backend/server/next/src/middleware.ts (2)
24-34
: Consider adding trim() when processing origins from environment variables.The origin comparison could be more robust by trimming whitespace from the allowed origins. This would prevent issues when environment variables contain spaces after commas.
- const allowedOrigins = process.env.CORS_ORIGINS?.split(",") ?? []; + const allowedOrigins = process.env.CORS_ORIGINS?.split(",").map(origin => origin.trim()) ?? [];
9-12
: Consider adding a default origin for local development.If
CORS_ORIGINS
is not set in the environment, the allowed origins array will be empty, which might cause issues during local development. Consider adding a default development origin when the environment variable is not set.- const allowedOrigins = process.env.CORS_ORIGINS?.split(",") ?? []; + const allowedOrigins = process.env.CORS_ORIGINS?.split(",") ?? + (process.env.NODE_ENV === 'development' ? ['http://localhost:3000'] : []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
bun.lock
is excluded by!**/*.lock
📒 Files selected for processing (2)
apps/cli/templates/backend/server/next/src/middleware.ts
(1 hunks)package.json
(1 hunks)
🔇 Additional comments (6)
package.json (1)
19-19
: Dependency version update looks good.The update from
@changesets/cli@^2.29.3
to@changesets/cli@^2.29.4
is a minor patch version change that likely includes bug fixes and maintenance improvements. This is a low-risk change that aligns with the PR objectives.apps/cli/templates/backend/server/next/src/middleware.ts (5)
1-1
: Import statement updated correctly.The import statement now properly imports both
NextRequest
andNextResponse
from 'next/server', which is necessary for the updated middleware function signature that accepts a request parameter.
3-7
: Good extraction of CORS options into a reusable object.Extracting CORS headers into a dedicated object improves code readability and maintainability. The CORS configuration includes all necessary headers with appropriate values, and correctly expands the allowed methods to include PUT and DELETE as mentioned in the PR objectives.
9-12
: Multiple CORS origins handling implemented correctly.The middleware function now properly extracts the origin from request headers and checks it against a list of allowed origins from the environment variable. The implementation follows best security practices by only allowing specific origins instead of using wildcards.
14-22
: Proper handling of preflight OPTIONS requests.The code correctly identifies and handles preflight OPTIONS requests by returning an immediate response with appropriate CORS headers. The conditional inclusion of the Access-Control-Allow-Origin header only for allowed origins is a good security practice.
30-32
: Efficient header setting with Object.entries.Using
Object.entries
to iterate through the CORS options and set headers is a clean and maintainable approach. This ensures all required CORS headers are consistently applied.
This Pull Request (PR) introduces a robust solution for handling Cross-Origin Resource Sharing (CORS) on the Next.js server, enabling secure communication with multiple specified frontend origins. Additionally, it updates several key project dependencies to their latest stable versions, ensuring better performance, security, and access to new features.
✨ Changes Introduced
1. CORS Configuration (
pages/_middleware.ts
)allowedOrigins
. This ensures that only trusted frontends can interact with the API, enhancing security.allowedOrigins
array, theAccess-Control-Allow-Origin
header is set to the specific requesting origin. This is crucial for proper CORS behavior and avoids issues with wildcard origins.2. Code Refactoring
I spotted a few potential issues:
3. Dependency Updates (
package.json
)@changesets/cli
: Updated from 2.29.3 to 2.29.4.Fixed: #218
Summary by CodeRabbit
New Features
Refactor
Chores
@changesets/cli
development dependency to the latest version.