Skip to content

Conversation

@kaswhy
Copy link
Member

@kaswhy kaswhy commented Aug 30, 2025

📌 연관된 이슈

✨ 작업 내용

  • filter에 PATCH 추가
  • filter, secruityConfig 허용 경로 일치
  • skipPaths 블록 삭제

💬 리뷰 요구사항(선택)

Summary by CodeRabbit

  • New Features

    • API에서 PATCH 메서드를 공식 지원하여 부분 업데이트가 가능해졌습니다.
  • Bug Fixes / 변경사항

    • 브라우저의 CORS 사전 요청(OPTIONS)을 허용하여 교차 도메인 통신 안정성이 향상되었습니다.
    • 인증 필터 동작이 간소화되어 이전에 필터를 우회하던 일부 엔드포인트가 더 이상 자동으로 제외되지 않습니다. 이로 인해 비밀번호 재설정 등 비인증 흐름이 영향받을 수 있으니 유의하세요.

@kaswhy kaswhy self-assigned this Aug 30, 2025
@kaswhy kaswhy added the bug Something isn't working label Aug 30, 2025
@coderabbitai
Copy link

coderabbitai bot commented Aug 30, 2025

Walkthrough

SecurityConfig에서 OPTIONS 요청을 명시적으로 permitAll로 허용하고 CORS 허용 메서드에 PATCH를 추가했으며, TokenAuthenticationFilter에서는 내부의 skip 리스트 로직을 제거해 모든 엔드포인트에 대해 토큰 검사 흐름을 단순화했습니다. 공개 API 시그니처 변경은 없습니다.

Changes

Cohort / File(s) Change Summary
Security config 업데이트
src/main/java/inha/gdgoc/global/security/SecurityConfig.java
HttpMethod.OPTIONS를 명시해 프리플라이트(OPTIONS)를 전역 허용으로 추가, CORS 허용 메서드에 PATCH 포함, 일부 엔드포인트의 permit 목록에서 /api/v1/password-reset/** 제거, 포매팅 정리.
인증 필터 정리
src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java
내부 skip 리스트(skipPaths) 및 관련 List import 제거로 doFilterInternal 흐름 단순화 — 특정 경로를 내부에서 예외 처리하지 않음. 토큰 추출·인증·실패 처리 로직과 shouldNotFilter 서명에는 변경 없음. 로그 메시지는 토큰 존재 여부로 변경.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant B as Client/Browser
  participant S as SecurityFilterChain
  participant F as TokenAuthenticationFilter
  participant A as AuthenticationManager
  participant C as SecurityContext

  rect rgba(220,235,255,0.35)
  Note over B,S: CORS Preflight
  B->>S: OPTIONS /api/...
  S-->>B: 200 (permitAll, 인증 불필요)
  end

  rect rgba(235,255,220,0.35)
  Note over B,S: 일반 요청 흐름
  B->>S: GET/POST/PUT/PATCH /api/...
  S->>F: 필터 호출
  F->>F: Authorization 헤더에서 토큰 추출
  alt 토큰 존재
    F->>A: 인증 시도
    alt 인증 성공
      A-->>F: Authentication
      F->>C: SecurityContext에 설정
      F-->>S: 다음 필터로 진행
    else 인증 실패
      F->>C: SecurityContext.clear()
      F-->>S: 다음 필터로 진행
    end
  else 토큰 없음
    F-->>S: 토큰 없이 다음 필터로 진행
  end
  S-->>B: 컨트롤러 처리 결과
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • [HOTFIX] 모든 OPTIONS 요청 허용 #207: OPTIONS/CORS 처리 및 TokenAuthenticationFilter의 예외/스킵 로직 변경을 다루는 PR로, CORS 허용과 필터 예외 처리 방식의 교차 검토가 필요합니다.

Poem

작은 토끼가 말하네,
"문은 OPTIONS로 활짝 열리고,
필터는 가벼운 발걸음으로 지나가네.
패치 하나 더하고 로그는 짧게,
깡충 — 코드가 웃으니 나도 웃네." 🐰✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hotfix/issue-206

🪧 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.
    • 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.
  • 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 the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit 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

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • 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.

@github-actions
Copy link

github-actions bot commented Aug 30, 2025

CI status
✅ Assemble 성공
✅ Test 성공

Copy link

@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: 3

🧹 Nitpick comments (1)
src/main/java/inha/gdgoc/global/security/SecurityConfig.java (1)

93-97: 허용 Origin 재검토(배포/프리뷰 도메인 관리 제안)

Vercel 프리뷰/브랜치 도메인이 수시로 바뀔 수 있습니다. 서브도메인 확장을 고려한다면 setAllowedOriginPatterns로 패턴 기반 허용을 검토하세요. 운영·스테이징 분리도 함께 점검 권장.

예:

-        config.setAllowedOrigins(List.of(
-            "https://gdgocinha.com",
-            "https://www.gdgocinha.com",
-            "https://typing-game-alpha-umber.vercel.app"
-        ));
+        config.setAllowedOriginPatterns(List.of(
+            "https://gdgocinha.com",
+            "https://www.gdgocinha.com",
+            "https://*.vercel.app"
+        ));

주의: allowCredentials(true) 사용 시 *는 금지이며, 패턴은 Spring에서만 동작(브라우저 응답에는 구체 Origin이 반환)합니다.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6b7304a and 618efac.

📒 Files selected for processing (2)
  • src/main/java/inha/gdgoc/global/security/SecurityConfig.java (3 hunks)
  • src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-30T10:43:25.860Z
Learnt from: kaswhy
PR: GDGoCINHA/24-2_GDGoC_Server#207
File: src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java:31-39
Timestamp: 2025-08-30T10:43:25.860Z
Learning: In the GDGoCINHA project, the following API paths are intentionally designed as public endpoints that should not require authentication: /api/v1/auth/, /api/v1/test/, /api/v1/game/, /api/v1/apply/, /api/v1/check/. These paths are correctly excluded from the TokenAuthenticationFilter in shouldNotFilter() method.

Applied to files:

  • src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java
📚 Learning: 2025-08-30T10:43:25.860Z
Learnt from: kaswhy
PR: GDGoCINHA/24-2_GDGoC_Server#207
File: src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java:31-39
Timestamp: 2025-08-30T10:43:25.860Z
Learning: The GDGoCINHA project has a consistent security architecture where public endpoints are handled in two layers: 1) SecurityConfig uses .permitAll() for paths like /api/v1/auth/**, /api/v1/game/**, /api/v1/apply/**, /api/v1/check/** and 2) TokenAuthenticationFilter excludes the same paths in shouldNotFilter(). This dual-layer approach is correct and intentional.

Applied to files:

  • src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java
  • src/main/java/inha/gdgoc/global/security/SecurityConfig.java
🧬 Code graph analysis (1)
src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (1)
src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java (1)
  • Slf4j (47-174)
🔇 Additional comments (6)
src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (1)

44-47: @NotNull 주석 추가는 가독성과 계약 명시 측면에서 👍

시그니처에 명시적으로 null 금지를 표현해 방어적 코딩에 도움이 됩니다.

src/main/java/inha/gdgoc/global/security/SecurityConfig.java (5)

10-10: OPTIONS 허용을 위한 HttpMethod import 추가 적절

CORS 프리플라이트 처리를 위한 준비가 잘 되어 있습니다.


41-41: 프리플라이트(OPTIONS) 전면 허용 OK

requestMatchers(HttpMethod.OPTIONS, "/**").permitAll()로 브라우저 프리플라이트가 필터/인증을 우회할 수 있어 CORS 오류 완화에 효과적입니다.


36-40: CORS + Stateless 설정 일관성 👍

csrf/formLogin/httpBasic 비활성화와 stateless 세션 정책 설정이 토큰 기반 인증 흐름과 잘 맞습니다.


98-98: CORS 허용 메서드에 PATCH 추가 적절

프론트 요청과의 정합성 확보에 필요합니다.


100-101: 허용 헤더 목록 충분

Authorization 포함되어 있어 Bearer 토큰 시나리오에 문제 없습니다.

@kaswhy kaswhy merged commit 0abadec into develop Aug 30, 2025
1 of 2 checks passed
@kaswhy kaswhy deleted the hotfix/issue-206 branch August 30, 2025 11:59
Copy link

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (2)

80-84: getAccessToken에서 IllegalArgumentException 발생 시 500 위험

제어문자 포함 토큰에서 IllegalArgumentException이 던져지면 doFilterInternal 상단에서 잡히지 않아 500으로 번질 수 있습니다. 비정상 Authorization 헤더는 400/401로 처리하거나 최소한 인증 시도 스킵(null 반환)으로 내려주는 편이 안전합니다.

다음처럼 예외 대신 경고 로그 후 null 반환을 권장합니다:

-        for (char c : token.toCharArray()) {
-            if (c < 32) {
-                log.info("토큰에 유효하지 않은 제어 문자가 포함되어 있습니다.");
-                throw new IllegalArgumentException("토큰에 유효하지 않은 제어 문자가 포함되어 있습니다.");
-            }
-        }
+        for (char c : token.toCharArray()) {
+            if (c < 32) {
+                log.warn("토큰에 유효하지 않은 제어 문자가 포함되어 있습니다. uri={}", request.getRequestURI());
+                return null;
+            }
+        }

대안: doFilterInternal에서 getAccessToken 호출을 try-catch로 감싸 400/401을 직접 내려도 됩니다. 필요하시면 해당 형태로도 패치 제안 드리겠습니다.


30-38: /api/v1/test/** 경로 동기화 필요
TokenAuthenticationFilter.shouldNotFilter에는 /api/v1/test/가 제외 대상인데, SecurityConfig.authorizeHttpRequests의 .requestMatchers(...).permitAll() 목록에 빠져 있습니다. 두 위치 목록을 일치시켜야 합니다.
CORS 설정: PATCH 메서드는 이미 config.setAllowedMethods(List.of(..., "PATCH"))에 포함되어 있어 별도 조치 불필요합니다.

♻️ Duplicate comments (1)
src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (1)

48-48: 액세스 토큰 원문 로깅 제거 잘 반영됨

토큰 존재 여부만 로그로 남기는 방향으로 수정되어 보안 관점에서 적절합니다. 이전 리뷰 취지와 일치합니다.

🧹 Nitpick comments (2)
src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (2)

72-77: Authorization 스킴 대소문자 무시 처리 제안

RFC 상 인증 스킴은 대소문자 비의존입니다. 현재 startsWith("Bearer ")는 소문자 "bearer "를 거부합니다.

-        if (bearerToken == null || !bearerToken.startsWith(TOKEN_PREFIX)) {
-            return null;
-        }
-
-        String token = bearerToken.substring(TOKEN_PREFIX.length());
+        if (bearerToken == null) {
+            return null;
+        }
+        int prefixLen = TOKEN_PREFIX.length();
+        if (bearerToken.length() < prefixLen
+                || !bearerToken.regionMatches(true, 0, TOKEN_PREFIX, 0, prefixLen)) {
+            return null;
+        }
+        String token = bearerToken.substring(prefixLen);

54-54: 성공 로그 레벨을 DEBUG로 낮추는 것을 권장

요청마다 INFO 로그가 과다해질 수 있습니다. 운영 환경에서는 DEBUG가 적절합니다.

-                log.info("JWT 인증 성공: {}", authentication.getName());
+                log.debug("JWT 인증 성공: {}", authentication.getName());
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 618efac and f4f0473.

📒 Files selected for processing (2)
  • src/main/java/inha/gdgoc/global/security/SecurityConfig.java (3 hunks)
  • src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/inha/gdgoc/global/security/SecurityConfig.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-30T10:43:25.860Z
Learnt from: kaswhy
PR: GDGoCINHA/24-2_GDGoC_Server#207
File: src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java:31-39
Timestamp: 2025-08-30T10:43:25.860Z
Learning: The GDGoCINHA project has a consistent security architecture where public endpoints are handled in two layers: 1) SecurityConfig uses .permitAll() for paths like /api/v1/auth/**, /api/v1/game/**, /api/v1/apply/**, /api/v1/check/** and 2) TokenAuthenticationFilter excludes the same paths in shouldNotFilter(). This dual-layer approach is correct and intentional.

Applied to files:

  • src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java
📚 Learning: 2025-08-30T10:43:25.860Z
Learnt from: kaswhy
PR: GDGoCINHA/24-2_GDGoC_Server#207
File: src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java:31-39
Timestamp: 2025-08-30T10:43:25.860Z
Learning: In the GDGoCINHA project, the following API paths are intentionally designed as public endpoints that should not require authentication: /api/v1/auth/, /api/v1/test/, /api/v1/game/, /api/v1/apply/, /api/v1/check/. These paths are correctly excluded from the TokenAuthenticationFilter in shouldNotFilter() method.

Applied to files:

  • src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java
🧬 Code graph analysis (1)
src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (1)
src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java (1)
  • Slf4j (47-174)
🔇 Additional comments (1)
src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (1)

43-46: 메서드 시그니처의 @NotNull 추가, OK

NPE 방지·정적분석 이점 모두 확보. 별도 이슈 없음.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants