Skip to content
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

Merged
merged 1 commit into from
Mar 4, 2025
Merged

refactor/#214 cors 설정 수정 #215

merged 1 commit into from
Mar 4, 2025

Conversation

minjo-on
Copy link
Contributor

@minjo-on minjo-on commented Mar 4, 2025

Summary

cors 설정 수정

Tasks

  • localhost:3001 추가

@minjo-on minjo-on added the 🔨refactor refactoring code label Mar 4, 2025
@minjo-on minjo-on requested a review from a team March 4, 2025 08:51
@minjo-on minjo-on self-assigned this Mar 4, 2025
@minjo-on minjo-on linked an issue Mar 4, 2025 that may be closed by this pull request
1 task
Copy link
Contributor

coderabbitai bot commented Mar 4, 2025

Walkthrough

이 PR은 SecurityConfig.java 파일의 CORS 설정 업데이트를 포함하며, corsConfigurationSource() 메서드에 http://localhost:3001을 추가하여 로컬 개발 서버의 요청 허용 범위를 확장합니다. 또한, 더 이상 사용되지 않는 org.springframework.beans.factory.annotation.Value 임포트가 제거되었습니다.

Changes

파일 경로 변경 내용
aics-common/.../SecurityConfig.java CORS 설정 내 허용 원본에 http://localhost:3001 추가, 사용되지 않는 @Value 임포트 제거

Possibly related PRs

  • fix: cors 이슈 픽스 #200: SecurityConfig.javacorsConfigurationSource() 메서드 변경으로 CORS 요청 허용 원본 업데이트 관련 작업 수행.

Suggested labels

🐞bug

Suggested reviewers

  • LeeShinHaeng
✨ Finishing Touches
  • 📝 Generate Docstrings

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

github-actions bot commented Mar 4, 2025

Test Coverage Report

Overall Project 94.74% 🍏

There is no coverage information present for the Files changed

Copy link

codecov bot commented Mar 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Impacted file tree graph

@@            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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b10879...f57719b. Read the comment docs.

Copy link
Contributor

@LeeShinHaeng LeeShinHaeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

빠른 작업 좋습니다 👍

@minjo-on minjo-on merged commit 10abc65 into develop Mar 4, 2025
4 of 5 checks passed
@minjo-on minjo-on deleted the refactor/#214-cors branch March 4, 2025 08:54
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.app

Length 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

  1. Origin Format
    Browsers send the Origin 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].

  2. Matching Behavior
    Servers compare the browser's Origin header against configured allowedOrigins exactly. A trailing slash creates a mismatch:

    • ✅ Matches: allowedOrigins: "https://example.com" vs browser's Origin: https://example.com
    • ❌ Fails: allowedOrigins: "https://example.com/" vs browser's Origin: https://example.com [4][5]
  3. Common Pitfalls

    • Express.js: A trailing slash in origin: "https://domain-a.com/" prevents the Access-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.comhttps://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:

  1. Preflight requests succeed (200 OK) but lack CORS headers [6][8]
  2. 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 and Access-Control-Allow-Origin response headers [4][6].

Citations:


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

📥 Commits

Reviewing files that changed from the base of the PR and between 2b10879 and f57719b.

📒 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 목표와 일치하며 로컬 개발 환경을 확장하는 데 도움이 됩니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨refactor refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cors 수정
2 participants