Skip to content

Conversation

@kaswhy
Copy link
Member

@kaswhy kaswhy commented Sep 3, 2025

📌 연관된 이슈

✨ 작업 내용

로그아웃 시 인증을 건너뛰어서 사용자를 못 찾는 문제를 인증 사용하도록 변경했습니다.

💬 리뷰 요구사항(선택)

Summary by CodeRabbit

  • 버그 수정
    • 로그아웃(/api/v1/auth/logout) 엔드포인트에 인증이 필수로 적용되어 비로그인 사용자는 접근이 거부됩니다.
    • 토큰에서 추출한 사용자 정보로 로그아웃을 처리해 신뢰성과 정확도를 향상했습니다.
    • 익명 사용자 요청을 명확히 차단하여 일관된 권한 오류를 반환합니다.
    • 보안 필터 적용 범위를 조정해 로그아웃 요청에도 토큰 검증이 일관되게 수행됩니다.
    • 이제 로그인 상태에서만 정상적으로 로그아웃이 가능하며, 잘못되거나 누락된 토큰은 즉시 거부됩니다.

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

coderabbitai bot commented Sep 3, 2025

Walkthrough

로그아웃 엔드포인트에 인증 요구를 명시하고, 토큰 필터에서 로그아웃 요청이 우회되지 않도록 수정했으며, 컨트롤러는 Principal 기반으로 사용자 식별 후 로그아웃을 처리하도록 변경했습니다.

Changes

Cohort / File(s) Change Summary
AuthController: Principal 기반 로그아웃 처리
src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java
- /logout@PreAuthorize("isAuthenticated()") 추가
- authenticationprincipalTokenProvider.CustomUserDetails로 캐스팅하여 userId/email 획득
- 익명/비인증 사용자 차단 가드 추가
- 관련 import 및 로그 메시지 갱신
Security: 로그아웃 인증 요구 및 필터 경로 조정
src/main/java/inha/gdgoc/global/security/SecurityConfig.java, src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java
- SecurityConfig에서 /api/v1/auth/logout.authenticated() 규칙 추가
- TokenAuthenticationFilter.shouldNotFilter에서 /api/v1/auth/logout은 우회하지 않도록 명시하여 필터 적용

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant C as Client
    participant S as SecurityFilterChain
    participant F as TokenAuthenticationFilter
    participant TP as TokenProvider/Authentication
    participant AC as AuthController
    participant US as UserService

    C->>S: POST /api/v1/auth/logout (Bearer token)
    S->>F: shouldNotFilter(request)
    Note over F: logout 경로는 우회하지 않음
    F->>TP: 토큰 추출 및 인증 설정
    F-->>S: 필터링 계속

    S->>AC: /logout 진입
    Note over AC: @PreAuthorize("isAuthenticated()")
    AC->>AC: principal 검사 (anonymous 차단)
    AC->>US: userId 기반 로그아웃 처리
    US-->>AC: 완료
    AC-->>C: 200 OK
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective Addressed Explanation
[#213] 로그아웃 호출 시 404 에러 수정

Assessment against linked issues: Out-of-scope changes

(해당 없음)

Possibly related PRs

Poem

토큰을 흔들며 퐁퐁 뛰는 토깽이 🐰
로그아웃 길에 표지판을 세웠지: “인증 필수!”
필터는 이제 비켜가지 않고 꼬박꼬박 확인하고,
컨트롤러는 주인증을 살짝 묻고 기록하고,
404는 숨어버리고 200만 남았네.
깡총, 깡총—버그여 안녕!

✨ 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-213

🪧 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 Sep 3, 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: 0

Caution

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

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

43-51: /api/v1/test 경로 불일치 확인 및 대응 필요
컨트롤러에서 /api/v1/test 매핑이 존재하지 않습니다.

  • 해당 엔드포인트가 더 이상 사용되지 않는다면 TokenAuthenticationFilter.java (shouldNotFilter)에서 startsWith("/api/v1/test/") 예외를 제거하세요.
  • 여전히 공개되어야 한다면 SecurityConfig.java의 permitAll() 목록에 "/api/v1/test/**"를 추가하세요.
🧹 Nitpick comments (5)
src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (1)

32-32: 로그아웃 엔드포인트만 필터링하도록 한 예외 처리, 방향성 적절합니다.

/api/v1/auth/logout은 이제 토큰 필터를 거치므로 SecurityConfig의 인증 요구와 일관됩니다. 다만 하드코딩된 경로 문자열은 상수로 관리하면 중복/오타를 줄일 수 있습니다.

적용 예시(이 파일 외 상단에 상수 추가 후 사용):

// 클래스 내 상단
private static final String LOGOUT_URI = "/api/v1/auth/logout";
-        if (uri.equals("/api/v1/auth/logout")) return false;
+        if (uri.equals(LOGOUT_URI)) return false;
src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java (3)

109-114: 익명 사용자 수동 체크는 중복입니다.

@PreAuthorize("isAuthenticated()")가 이미 보장하므로, 아래 가드는 제거해도 동작에 영향이 없습니다. 간결성을 위해 정리 제안드립니다.

-        // 1) 익명 방어
-        if (authentication == null
-            || !authentication.isAuthenticated()
-            || "anonymousUser".equals(authentication.getName())) {
-            throw new AuthException(UNAUTHORIZED_USER);
-        }

105-121: SecurityContext 직접 접근 대신 의존성 주입과 공용 유틸 재사용으로 단순화하세요.

메서드 파라미터로 Authentication을 주입하고, AuthService#getAuthenticationUserId를 재사용하면 중복 로직과 캐스팅 코드를 줄일 수 있습니다.

-    public ResponseEntity<ApiResponse<Void, Void>> logout() {
-        // TODO 서비스로 넘기기
-        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+    public ResponseEntity<ApiResponse<Void, Void>> logout(Authentication authentication) {
+        // TODO 서비스로 넘기기
@@
-        // 2) principal 캐스팅해서 확정적으로 userId/email 사용
-        Object principal = authentication.getPrincipal();
-        if (!(principal instanceof TokenProvider.CustomUserDetails userDetails)) {
-            throw new AuthException(UNAUTHORIZED_USER);
-        }
-
-        Long userId = userDetails.getUserId();
-        String email = userDetails.getUsername();
+        Long userId = authService.getAuthenticationUserId(authentication);
+        String email = authentication.getName();

103-140: 로그아웃 시 refresh_token 쿠키 무효화(만료) 응답도 함께 내려주세요.

DB 토큰 삭제만으로도 재발급은 막히지만, 클라이언트 UX와 불필요한 재시도(401 유발)를 줄이려면 쿠키를 즉시 만료시키는 Set-Cookie를 반환하는 것이 좋습니다.

참고 코드(응답에 쿠키 제거 헤더 추가; 필요 시 메서드 시그니처에 HttpServletResponse response 파라미터 추가):

ResponseCookie remove = ResponseCookie.from("refresh_token", "")
    .httpOnly(true)
    .secure(true)
    .sameSite("None")
    .domain(".gdgocinha.com") // 로그인 시 설정과 동일 도메인 유지
    .path("/")
    .maxAge(Duration.ZERO)
    .build();

response.addHeader(HttpHeaders.SET_COOKIE, remove.toString());
src/main/java/inha/gdgoc/global/security/SecurityConfig.java (1)

42-42: HTTP 메서드 범위를 POST로 한정하면 의도가 더 명확합니다.

현재도 문제는 없지만, 의미적으로 로그아웃은 POST에만 적용하는 편이 안전합니다.

-                .requestMatchers("/api/v1/auth/logout").authenticated()
+                .requestMatchers(HttpMethod.POST, "/api/v1/auth/logout").authenticated()
📜 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 5909b8f and c4b3d87.

📒 Files selected for processing (3)
  • src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java (3 hunks)
  • src/main/java/inha/gdgoc/global/security/SecurityConfig.java (1 hunks)
  • src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (1 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/domain/auth/controller/AuthController.java (3)
src/main/java/inha/gdgoc/global/config/jwt/TokenProvider.java (3)
  • Getter (127-136)
  • CustomUserDetails (131-134)
  • RequiredArgsConstructor (31-137)
src/main/java/inha/gdgoc/domain/auth/service/RefreshTokenService.java (1)
  • Transactional (95-114)
src/main/java/inha/gdgoc/domain/auth/service/AuthService.java (1)
  • Slf4j (33-166)
🔇 Additional comments (2)
src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java (1)

103-105: 컨트롤러 단 @PreAuthorize("isAuthenticated()") 추가, 보안 요건 충족합니다.

메서드 진입 전에 인증을 강제하므로 의도한 404 → 401/403 동작으로 정렬됩니다.

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

42-42: /api/v1/auth/logout 인증 요구 규칙 추가, 위치도 적절합니다.

permitAll("/api/v1/auth/**")보다 앞에 와서 올바르게 우선 적용됩니다.

@kaswhy kaswhy merged commit 098f1c2 into develop Sep 3, 2025
2 checks passed
@kaswhy kaswhy deleted the hotfix/issue-213 branch September 3, 2025 07:56
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.

[BUG] logout 404 에러 수정

2 participants