Refactor : html(google oauth)로 리다이렉팅되는 문제 해결 #57
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough인증 및 토큰 관리 로직이 리팩토링되어, 서비스 계층의 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant AuthService
participant Redis
participant JwtFilter
Client->>AuthController: /auth/logout (Authorization 헤더 포함)
AuthController->>AuthService: logout(userId, token)
AuthService->>Redis: 토큰 블랙리스트 등록 및 리프레시 토큰 삭제
AuthService-->>AuthController: LogoutResponse
AuthController-->>Client: 응답
Client->>AuthController: /auth/refresh (Authorization 헤더 포함)
AuthController->>AuthService: refreshToken(token)
AuthService->>Redis: 리프레시 토큰 조회 및 검증
AuthService-->>AuthController: RefreshTokenResponse
AuthController-->>Client: 응답
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/main/java/org/ezcode/codetest/application/usermanagement/auth/service/AuthService.java (1)
118-136:⚠️ Potential issueRedis 실패를 삼켜 버리면 운영 문제를 조기에 인지하기 어렵습니다
catch (Exception e)에서 로그만 남기고 성공을 리턴하면 실제로 토큰이 블랙리스트에 등록되지 않아 보안 구멍이 발생할 수 있습니다.
- 재시도 로직 또는
- 실패 시에도 사용자에게 “일시적 오류” 를 알리고 재로그인 요구
를 고려해 주세요.
🧹 Nitpick comments (5)
src/main/java/org/ezcode/codetest/common/security/util/JwtFilter.java (1)
108-110: 성능은 충분하지만, WHITE_LIST 가 작아도 매 요청Arrays.stream()사용은 불필요
정적Set으로 변환 후contains/stream().anyMatch비교에서O(n)→O(1)로 단순화할 수 있습니다.src/main/java/org/ezcode/codetest/presentation/usermanagement/AuthController.java (2)
55-58:replace("Bearer ", "")는 토큰 내부 문자열까지 제거할 수 있습니다
토큰 값에Bearer가 포함될 경우(드물지만 가능) 전부 치환됩니다.- .map(h -> h.replace("Bearer ", "")) + .map(h -> h.substring(7)) // "Bearer ".length == 7
66-72: 중복 코드 추출 제안
토큰 헤더 파싱 로직이 logout·refresh 두 곳에 동일하게 존재합니다.private String extractToken(HttpServletRequest)헬퍼 메서드로 분리하면 중복을 제거하고 테스트하기도 수월합니다.src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java (2)
94-116: 에러 메시지 하드코딩은 예외 케이스를 과도하게 단순화합니다
"토큰이 필요합니다"로 고정하면 만료·서명 오류·지원되지 않는 토큰 등 구분이 불가합니다.
authException.getMessage()활용 또는 커스텀 예외 코드/메시지를 포함시켜 클라이언트가 원인을 식별할 수 있도록 개선이 필요합니다.
118-140:AccessDeniedHandler도 동일하게 세분화 필요
현재는 모든 403을"접근 권한이 없습니다"로 반환합니다. 세밀한 권한 정책(등급 제한 등)이 있다면 코드와 메시지를 함께 내려주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/org/ezcode/codetest/application/usermanagement/auth/service/AuthService.java(3 hunks)src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java(3 hunks)src/main/java/org/ezcode/codetest/common/security/util/JwtFilter.java(3 hunks)src/main/java/org/ezcode/codetest/presentation/usermanagement/AuthController.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/org/ezcode/codetest/common/security/util/JwtFilter.java (1)
49-56: startsWith 매칭은 의도치 않은 우회 가능성을 열 수 있습니다
예)/auth/signupExtra요청도/auth/signup으로 시작하므로 필터를 우회합니다. 필요하다면AntPathMatcher로"/auth/signup"정확 매칭 또는"/auth/signup/**"패턴 매칭을 명시적으로 지정하세요.
Comment on lines
+35
to
+43
| // 토큰 검증을 건너뛸 경로들 | ||
| private static final String[] WHITE_LIST = { | ||
| "/auth/signin", | ||
| "/auth/signup", | ||
| "/auth/refresh", | ||
| "/swagger-ui", | ||
| "/v3/api-docs", | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
화이트리스트 하드코딩은 유지‧보수 비용을 높입니다
SecurityConfig 에서도 동일(혹은 더 광범위한) 경로를 permitAll() 로 선언하고 있어 두 곳이 불필요하게 중복됩니다. 경로가 추가/변경될 때 한쪽만 수정되면 필터와 인가 규칙이 불일치할 위험이 있습니다.
- 공통 상수 클래스로 분리하거나
application.yml의 리스트를 주입받아 사용하도록 리팩터링하면 추후 운영 중 경로가 늘어나더라도 한 군데만 수정하면 됩니다.
🤖 Prompt for AI Agents
In src/main/java/org/ezcode/codetest/common/security/util/JwtFilter.java around
lines 35 to 43, the whitelist paths are hardcoded, causing duplication with
SecurityConfig and increasing maintenance risk. Refactor by extracting these
paths into a shared constant class or configure them in application.yml and
inject the list into JwtFilter. This centralizes path management, ensuring
consistency and easier updates.
Collaborator
Author
There was a problem hiding this comment.
고도화할떄 별도 클래스로 분리할게
There was a problem hiding this comment.
@minjee2758 네, 알겠습니다. 차후 고도화 단계에서 공통 클래스로 분리하시려는 계획 확인했습니다.
추적 용으로 별도 이슈를 만들어 두면 놓치지 않고 진행하실 수 있는데, 이슈 생성을 도와드릴까요? 필요 없으시면 그대로 두셔도 됩니다.
Comment on lines
52
to
60
| @AuthenticationPrincipal AuthUser authUser, | ||
| HttpServletRequest request) { | ||
| return ResponseEntity.status(HttpStatus.OK).body(authService.logout(authUser.getId(), request)); | ||
|
|
||
| String token = Optional.ofNullable(request.getHeader("Authorization")) | ||
| .map(h -> h.replace("Bearer ", "")) | ||
| .orElseThrow(()-> new AuthException(AuthExceptionCode.INVALID_AUTHORIZATION_HEADER)); | ||
|
|
||
| return ResponseEntity.status(HttpStatus.OK).body(authService.logout(authUser.getId(), token)); | ||
| } |
There was a problem hiding this comment.
@AuthenticationPrincipal 가 null 일 수 있어 NPE 위험
/auth/** 가 permitAll() 로 열려 있으므로 토큰 없이 호출되면 authUser 가 주입되지 않습니다.
Objects.requireNonNull(authUser, "로그아웃은 인증이 필요합니다");또는 SecurityConfig 에서 /auth/logout 을 인증 필요 경로로 이동하세요.
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/presentation/usermanagement/AuthController.java
around lines 52 to 60, the parameter annotated with @AuthenticationPrincipal
(authUser) can be null because the /auth/** endpoints are configured with
permitAll(), allowing calls without a token. To fix the potential
NullPointerException, add a null check for authUser at the start of the method
using Objects.requireNonNull(authUser, "로그아웃은 인증이 필요합니다"); or alternatively,
update the SecurityConfig to require authentication for the /auth/logout
endpoint so that authUser is always injected.
Collaborator
Author
There was a problem hiding this comment.
테스트하면서 편하려고 일단 풀어놨어
There was a problem hiding this comment.
@minjee2758 네, 확인했습니다! 테스트용으로 잠시 개방해 두신 것이라면 이해됩니다. 다만
- 토큰 없이
/auth/logout을 호출할 때authUser가null이면 NPE가 발생해 500 응답이 내려갈 수 있으니, 테스트 중에도 간단한 널 체크로 401/403 정도를 내려주면 디버깅이 수월할 거예요.
if (authUser == null) {
throw new AuthException(AuthExceptionCode.UNAUTHORIZED); // 혹은 커스텀 메시지
}- 나중에 배포 단계에서 잊지 않고 다시 인증 필요 경로로 옮길 수 있도록
// TODO: release 시 permitAll 해제같은 주석을 남겨 두시는 것도 권장드립니다.
추가로, 프로덕션 전환 시점에 이 항목을 추적할 이슈가 필요하시면 말씀해 주세요. 🤖
Comment on lines
+56
to
+58
| .exceptionHandling(except -> except | ||
| .authenticationEntryPoint(customAuthenticationEntryPoint()) | ||
| .accessDeniedHandler(customAccessDeniedHandler())) |
There was a problem hiding this comment.
/auth/** 전체를 permitAll() 로 두면 보호가 필요한 API(예: 로그아웃)가 열려 있습니다
JWT 필터로 인증 객체가 주입되지 않더라도 컨트롤러 접근이 허용됩니다. /auth/logout 만큼은 authenticated() 로 분리하거나 메서드에 @PreAuthorize 를 추가해 무조건 인증을 보장하세요.
🤖 Prompt for AI Agents
In src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java
around lines 56 to 58, the current configuration permits all requests to
/auth/**, which unintentionally exposes protected APIs like /auth/logout. To fix
this, modify the security rules to exclude /auth/logout from permitAll() by
requiring authentication for it, either by configuring it with authenticated()
in the security filter chain or by adding @PreAuthorize annotations on the
logout controller method to enforce authentication.
Comment on lines
+140
to
150
| public RefreshTokenResponse refreshToken(String token) { | ||
| log.info("서비스 입장"); | ||
|
|
||
| if (bearToken == null || !bearToken.startsWith("Bearer ")) { | ||
| throw new AuthException(AuthExceptionCode.INVALID_AUTHORIZATION_HEADER); | ||
| } | ||
| String refreshToken = jwtUtil.substringToken(bearToken); | ||
|
|
||
| if (!jwtUtil.validateToken(refreshToken)){ | ||
| throw new AuthException(AuthExceptionCode.INVALID_REFRESH_TOKEN); | ||
| } | ||
|
|
||
| Long userId = jwtUtil.getUserId(refreshToken); | ||
| Long userId = jwtUtil.getUserId(token); | ||
|
|
||
| log.info("유저 아이디 가져옴"); | ||
| String savedToken = redisTemplate.opsForValue().get("RefreshToken:" + userId); | ||
|
|
||
| if (savedToken==null || !savedToken.equals(refreshToken)){ | ||
| log.info("저장된 토큰 가져옴"); | ||
| if (savedToken==null || !savedToken.equals(token)){ | ||
| log.error("저장된 토큰 없음"); | ||
| throw new AuthException(AuthExceptionCode.INVALID_REFRESH_TOKEN); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refresh 토큰 자체 유효성 검증이 부족합니다
Redis 비교 전에 jwtUtil.validateToken(token) 과 만료 여부를 검사하지 않으면 만료된 RefreshToken 으로도 새 AccessToken 이 발급될 수 있습니다.
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/application/usermanagement/auth/service/AuthService.java
around lines 140 to 150, the refreshToken method lacks validation of the refresh
token's validity and expiration before comparing it with the stored token in
Redis. To fix this, add a call to jwtUtil.validateToken(token) before fetching
the saved token from Redis, and if the token is invalid or expired, throw an
AuthException with the appropriate error code to prevent issuing a new access
token with an expired refresh token.
pokerbearkr
approved these changes
Jun 13, 2025
NCookies
approved these changes
Jun 13, 2025
Kimminu7
approved these changes
Jun 13, 2025
3 tasks
thezz9
pushed a commit
that referenced
this pull request
Jun 14, 2025
* refactor : logout HttpServeletRequest를 컨트롤러에서 String으로 추출 후 service로 전달 * refactor : whitelist 등록 * refactor : 에러 발생 시, html응답 대신 Json응답으로 받을 수 있도록 리팩토링 * refactor : requestMapping으로 앞에 일괄적으로 /api 달기
thezz9
added a commit
that referenced
this pull request
Jun 14, 2025
* build : update build.gradle for Redis * refactor : Judge0 비동기 병렬 요청 구현 * refactor : 비동기 병렬 요청 구현을 위한 Application, dto * refactor : 비동기 병렬 요청 구현을 위한 config 파일 * feat : Redis Stream 구현 * refactor : 병렬 요청에서 SSE 구별을 위한 인메모리 저장 * refactor : 테스트용 UI * refactor : 그룹핑 처리 DTO로 책임 분리 * refactor : Judge0 polling 예외처리 고도화 * refactor : api 경로 수정 * refactor : 서비스 로직 리팩토링 * refactor : 모델 객체 추가 * refactor : 예외 코드 추가 * Refactor : html(google oauth)로 리다이렉팅되는 문제 해결 (#57) * refactor : logout HttpServeletRequest를 컨트롤러에서 String으로 추출 후 service로 전달 * refactor : whitelist 등록 * refactor : 에러 발생 시, html응답 대신 Json응답으로 받을 수 있도록 리팩토링 * refactor : requestMapping으로 앞에 일괄적으로 /api 달기 * feat : 게임 캐릭터 생성 기능, 아이템 생성 기능, 아이템 뽑기, 스테이터스 확인, 아이템 장착 기능 추가 (#58) * feat : 게임 도메인 엔티티 생성 및 스킬, 아이템 효과 정의 * feat : 게임 도메인 서비스, 아이템 뽑기 도메인서비스 추가, 랜덤 인카운터 추가 * feat : 아이템 장착, 뽑기, 인벤토리 오픈, 스탯 확인, 캐릭터 생성 기능 추가 # Conflicts: # src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java * chore : properties 수정 * chore : properties 수정 * chore : securityconfig 오타수정 * chore : 오타수정 * chore : 오타수정 * chore : 무수히 많은 오타수정, 누락된 어노테이션 추가 * docs : 환경변수 추가 * docs : 오타수정 --------- Co-authored-by: pokerbearkr <[email protected]> * Refactor : Filter WhiteList 수정 (#59) * refactor : api 경로 추가, oauth 경로 수정 * refactor : whitelist 경로 수정 * refactor : redirect url 수정 * refactor : Submission Stream --------- Co-authored-by: MIN <[email protected]> Co-authored-by: chat26666 <[email protected]> Co-authored-by: pokerbearkr <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
작업 내용
변경 사항
트러블 슈팅
1. JwtFilter
문제: 인증이 필요없는 경로에서도 JWT 필터가 동작하여 인증 검사 수행함
-> 화이트리스트 경로는 JWT 검증 생략함
2. HTML 반환 오류
문제: OAuth2 설정으로 인해 인증되지 않은 요청이 자동으로 Google 로그인으로 리다이렉트됨
-> AuthenticationEntryPoint와 AccessDeniedHandler 설정으로 JSON 응답 우선 처리함
해결해야 할 문제
참고 사항
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit
/api가 추가되어 URL 구조가 일관되게 변경되었습니다.