-
Notifications
You must be signed in to change notification settings - Fork 0
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
refactor/#214 cors 설정 수정 #215
Conversation
Walkthrough이 PR은 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Test Coverage Report
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #215 +/- ##
==========================================
Coverage 92.15% 92.15%
Complexity 145 145
==========================================
Files 49 49
Lines 408 408
Branches 4 4
==========================================
Hits 376 376
Misses 27 27
Partials 5 5 Continue to review full report in Codecov by Sentry.
|
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.
빠른 작업 좋습니다 👍
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
🔭 Outside diff range comments (1)
aics-common/src/main/java/kgu/developers/common/config/SecurityConfig.java (1)
101-101
: 💡 Verification agent🧩 Analysis chain
URL 형식 확인 필요
마지막 허용 URL
https://aics-client-dev.vercel.app/
에 슬래시(/)가 포함되어 있습니다. 다른 모든 URL은 슬래시 없이 설정되어 있으므로, 일관성을 위해 확인이 필요합니다.URL 형식이 맞는지 확인하기 위해 다음 명령어를 실행해볼 수 있습니다:
이 슬래시가 브라우저에서 CORS 요청을 처리할 때 문제를 일으킬 수 있는지 확인하세요:
🏁 Script executed:
#!/bin/bash # URL이 실제로 존재하는지 확인하고 리다이렉션 여부 체크 curl -I https://aics-client-dev.vercel.app/ curl -I https://aics-client-dev.vercel.appLength of output: 1981
🌐 Web query:
Does trailing slash in CORS allowedOrigins affect browser requests?
💡 Result:
Yes, trailing slashes in CORS
allowedOrigins
configurations can affect browser requests due to how browsers and servers handle origin validation. Here's a detailed analysis:Key Considerations
Origin Format
Browsers send theOrigin
header without trailing slashes (e.g.,https://example.com
), even if the URL includes one. CORS origins are defined as a combination of scheme, hostname, and port – no path component is allowed [2][9].Matching Behavior
Servers compare the browser'sOrigin
header against configuredallowedOrigins
exactly. A trailing slash creates a mismatch:
- ✅ Matches:
allowedOrigins: "https://example.com"
vs browser'sOrigin: https://example.com
- ❌ Fails:
allowedOrigins: "https://example.com/"
vs browser'sOrigin: https://example.com
[4][5]Common Pitfalls
- Express.js: A trailing slash in
origin: "https://domain-a.com/"
prevents theAccess-Control-Allow-Origin
header from being sent [4].- AWS/Cloud Services: Bucket CORS configurations with trailing slashes fail validation checks [5][7].
- Apollo/Spring: Explicit warnings in documentation to avoid trailing slashes in origin patterns [2][9].
Workarounds and Solutions
Scenario Problem Solution Single origin Trailing slash in config Remove slash ( https://example.com
→https://example.com
) [4][9]Wildcard subdomains Patterns like https://*.company.domain
Use regex patterns or framework-specific normalization (e.g., Spring's CorsConfiguration
trailing slash trimming) [1]Multiple origins Mixing paths/ports List origins explicitly without trailing slashes or paths [2][9] Framework-Specific Guidance
Express.js:
// ❌ Incorrect origin: "https://domain-a.com/" // ✅ Correct origin: "https://domain-a.com"ASP.NET Core:
// Automatically handles origin validation without paths policy.WithOrigins("https://example.com");Spring Framework:
Implements pattern matching that ignores trailing slashes in some configurations [1][8].Why This Matters
Mismatched origins cause hard-to-debug CORS failures where:
- Preflight requests succeed (200 OK) but lack CORS headers [6][8]
- Browsers block requests with console errors like:
Access-Control-Allow-Origin header value 'X' not equal to supplied origin 'Y'
[5]Always validate CORS configurations using browser developer tools to inspect both
Origin
request headers andAccess-Control-Allow-Origin
response headers [4][6].Citations:
- 1: Ignore trailing slash in CorsConfiguration origin patterns spring-projects/spring-framework#26892
- 2: https://www.apollographql.com/docs/graphos/routing/security/cors
- 3: https://tiffanybbrown.com/2011/12/dom-access-control-using-cors/index.html
- 4: https://dev.to/sepiyush/troubleshooting-cors-issues-in-expressjs-a-simple-misconfiguration-2e4k
- 5: https://repost.aws/questions/QU3C0ZRB87Tne7HF434VtsUw/access-control-allow-origin-header-value-not-equal-to-the-supplied-origin-post-method
- 6: https://learn.microsoft.com/en-us/aspnet/core/security/cors?view=aspnetcore-9.0
- 7: https://www.googlecloudcommunity.com/gc/Infrastructure-Compute-Storage/CORS-settings-has-no-effect/m-p/766266
- 8: https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/security/cors.md
- 9: https://www.apollographql.com/docs/apollo-server/security/cors
- 10: Weird Chrome-only, macOS-only problem regarding trailing slashes in servers.json shlinkio/shlink-web-client#380
CORS 설정: trailing slash 문제 수정 필요
- 보안 구성 파일(
aics-common/src/main/java/kgu/developers/common/config/SecurityConfig.java
, 101번째 줄)에서 마지막 허용 URL이"https://aics-client-dev.vercel.app/"
처럼 trailing slash가 포함되어 있습니다.- 테스트결과 두 URL(슬래시 포함/미포함) 모두 HTTP/2 200 응답을 반환하나, 웹 쿼리 결과와 CORS 동작 원리에 따르면 브라우저는 trailing slash 없이 Origin 헤더를 전송합니다.
- 이로 인해, 서버 측
allowedOrigins
설정에 trailing slash가 포함되면 브라우저의 Origin과 불일치하여 CORS 정책 상 차단될 가능성이 있습니다.- 다른 모든 URL이 슬래시 없이 설정된 점을 고려하여, 일관성을 위해 해당 URL에서 trailing slash를 제거하는 것을 권장합니다.
🧹 Nitpick comments (1)
aics-common/src/main/java/kgu/developers/common/config/SecurityConfig.java (1)
87-106
: CORS 설정 개선 고려사항현재 CORS 설정은 모든 헤더(
"*"
)와 모든 HTTP 메서드("*"
)를 허용하고 있습니다. 개발 환경에서는 편리하지만, 보안을 위해 프로덕션 환경에서는 필요한 헤더와 메서드만 명시적으로 허용하는 것이 좋습니다.프로덕션 환경에서는 다음과 같이 필요한 헤더와 메서드만 명시적으로 설정하는 것을 고려해보세요:
- config.setAllowedHeaders(Collections.singletonList("*")); - config.setAllowedMethods(Collections.singletonList("*")); + config.setAllowedHeaders(Arrays.asList("Origin", "Content-Type", "Accept", "Authorization")); + config.setAllowedMethods(Arrays.asList("GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH"));또한 환경(개발/테스트/프로덕션)에 따라 다른 CORS 설정을 적용하는 방식도 고려해볼 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aics-common/src/main/java/kgu/developers/common/config/SecurityConfig.java
(1 hunks)
🔇 Additional comments (1)
aics-common/src/main/java/kgu/developers/common/config/SecurityConfig.java (1)
94-94
: CORS 설정에 새로운 로컬 개발 서버가 정상적으로 추가되었습니다.CORS 허용 목록에
http://localhost:3001
을 추가하여 해당 오리진에서의 API 요청을 정상적으로 처리할 수 있게 되었습니다. 이는 PR 목표와 일치하며 로컬 개발 환경을 확장하는 데 도움이 됩니다.
Summary
cors 설정 수정
Tasks