Refactor : Filter WhiteList 수정 #59
Conversation
|
""" Walkthrough인증 및 보안 관련 엔드포인트의 URL 경로가 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend (login-page.html)
participant AuthViewController
participant GoogleOAuth2
User->>Frontend (login-page.html): 방문 및 Google 로그인 버튼 클릭
Frontend->>AuthViewController: /login 요청
AuthViewController-->>User: http://localhost:8080/oauth2/authorization/google 반환
User->>GoogleOAuth2: OAuth2 인증 플로우 시작
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/org/ezcode/codetest/application/usermanagement/auth/service/AuthService.java(1 hunks)src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java(1 hunks)src/main/java/org/ezcode/codetest/common/security/util/JwtFilter.java(1 hunks)src/main/java/org/ezcode/codetest/presentation/usermanagement/view/AuthViewController.java(1 hunks)src/main/resources/templates/login-page.html(1 hunks)
🔇 Additional comments (2)
src/main/resources/templates/login-page.html (1)
35-37: Google 로그인 URL 수정 적용 확인됨
백엔드의 OAuth2 엔드포인트(/oauth2/authorization/google)와 일치하도록 href가 수정됐습니다. 문제 없습니다.src/main/java/org/ezcode/codetest/common/security/util/JwtFilter.java (1)
37-40: 로그아웃·비밀번호 재설정 등 추가 엔드포인트 확인 필요
화이트리스트가/api/auth/signin|signup|refresh세 가지만 허용하고 있습니다. 로그아웃(/api/auth/logout)이나 비밀번호 재설정 등의 신규 엔드포인트를 도입할 경우 필터를 통과하지 못해 401이 반환될 수 있으니, API 스펙을 다시 점검해 추가 여부를 검토해 주세요.
| "/api/auth/**", | ||
| "/login", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
화이트리스트 경로 하드코딩 분산 ⇒ 중앙집중화 필요
/api/auth/** 경로를 SecurityConfig와 JwtFilter 두 곳에서 각각 문자열로 관리하고 있습니다. 경로 변경 시 두 파일이 불일치할 위험이 있으니 상수를 별도 클래스로 분리하거나 application.yml에 프로퍼티로 두고 주입하는 방식으로 중복을 제거하세요.
🤖 Prompt for AI Agents
In src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java
around lines 64 to 65, the whitelist path "/api/auth/**" is hardcoded both here
and in JwtFilter, causing duplication and risk of inconsistency. To fix this,
extract the path string into a shared constant in a separate class or define it
as a property in application.yml and inject it into both SecurityConfig and
JwtFilter, ensuring centralized management of the whitelist path.
There was a problem hiding this comment.
웅 나중에 클래스로 분리할게
There was a problem hiding this comment.
@minjee2758 네, 이해했습니다! 현재 PR에 집중하시고 리팩토링은 나중에 진행하시면 됩니다.
혹시 이 개선사항을 추적하기 위해 별도 이슈를 생성해 드릴까요? 나중에 놓치지 않고 처리하실 수 있도록 도움이 될 것 같습니다.
ᕦ(๑^╹^๑)ᕤ
| log.info("유저 아이디 가져옴 id : {}", userId); | ||
| String savedToken = redisTemplate.opsForValue().get("RefreshToken:" + userId); | ||
| log.info("저장된 토큰 가져옴"); | ||
| log.info("저장된 토큰 가져옴 {}", savedToken); | ||
| if (savedToken==null || !savedToken.equals(token)){ |
There was a problem hiding this comment.
민감 정보(리프레시 토큰) 로그 출력은 보안 위험
log.info("저장된 토큰 가져옴 {}", savedToken); 로 리프레시 토큰 전체를 남기면 토큰 탈취 위험이 있습니다. 토큰 앞·뒤 몇 글자만 마스킹하거나 로그 레벨을 DEBUG 이하로 낮추고 운영 환경에서는 끄는 것이 안전합니다.
-log.info("저장된 토큰 가져옴 {}", savedToken);
+log.debug("저장된 토큰 조회 완료. length={}", savedToken != null ? savedToken.length() : 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| log.info("유저 아이디 가져옴 id : {}", userId); | |
| String savedToken = redisTemplate.opsForValue().get("RefreshToken:" + userId); | |
| log.info("저장된 토큰 가져옴"); | |
| log.info("저장된 토큰 가져옴 {}", savedToken); | |
| if (savedToken==null || !savedToken.equals(token)){ | |
| log.info("유저 아이디 가져옴 id : {}", userId); | |
| String savedToken = redisTemplate.opsForValue().get("RefreshToken:" + userId); | |
| log.debug("저장된 토큰 조회 완료. length={}", savedToken != null ? savedToken.length() : 0); | |
| if (savedToken == null || !savedToken.equals(token)) { |
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/application/usermanagement/auth/service/AuthService.java
around lines 145 to 148, the code logs the full refresh token which poses a
security risk. Modify the logging to mask the token by showing only the first
and last few characters with the middle part replaced by asterisks or similar,
or change the log level to DEBUG and ensure it is disabled in production
environments to prevent token exposure.
There was a problem hiding this comment.
개발할떄 필요해서 해놨어 토끼얌
There was a problem hiding this comment.
@minjee2758 개발 중에 디버깅할 때 필요하다는 점 이해해요! 👍
다만 운영 환경으로 배포될 때를 고려해서, 환경별로 로그 레벨을 다르게 설정하는 것도 좋은 방법이에요:
if (log.isDebugEnabled()) {
log.debug("저장된 토큰 가져옴 {}", savedToken);
} else {
log.info("저장된 토큰 조회 완료");
}또는 application.yml에서 개발/운영 환경별로 로그 레벨을 다르게 설정하시면 개발할 때는 상세한 로그를, 운영에서는 안전한 로그를 사용할 수 있어요.
하지만 지금 개발 단계에서 필요하시다면 그대로 두셔도 괜찮습니다! 😊
src/main/java/org/ezcode/codetest/presentation/usermanagement/view/AuthViewController.java
Outdated
Show resolved
Hide resolved
|
충돌나영 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/org/ezcode/codetest/common/security/util/ExceptionHandlingFilter.java (1)
34-36:JwtException블록 로그 레벨 및 메시지 통일 고려
JwtException은 서명 오류·만료·지원되지 않는 토큰 등 모든 하위 예외의 슈퍼클래스입니다.
이미 세 가지 상세 예외를 앞에서 처리했으므로, 여기서는 미처 분류하지 못한 JWT 예외만 들어옵니다.
로그 레벨이warn이고, 앞선 블록들은info라 레벨이 달라 모니터링 시 혼선이 있을 수 있습니다. 전부warn또는info로 통일하는 편이 관리 측면에서 낫습니다.
또한 사용자 메시지도 한글·영문 혼용 대신 일관성을 유지해 주세요.-log.warn("JWT 관련 예외", e); +log.info("Unhandled JWT 예외", e); // 또는 전 블록을 warn 으로 상향
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/ezcode/codetest/common/security/util/ExceptionHandlingFilter.java(2 hunks)src/main/java/org/ezcode/codetest/common/security/util/JwtFilter.java(1 hunks)src/main/java/org/ezcode/codetest/presentation/usermanagement/view/AuthViewController.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/ezcode/codetest/common/security/util/JwtFilter.java
- src/main/java/org/ezcode/codetest/presentation/usermanagement/view/AuthViewController.java
🔇 Additional comments (1)
src/main/java/org/ezcode/codetest/common/security/util/ExceptionHandlingFilter.java (1)
37-41: Generic Exception 재전파 정책 점검 필요
IOException | ServletException뿐 아니라Exception까지 그대로 throw 하도록 변경했는데,
컨트롤러 레이어에서 해당 예외를 처리하지 않으면 서블릿 컨테이너 전역 오류 페이지로 넘어가며 스택트레이스가 사용자에게 노출될 위험이 있습니다.
- 컨트롤러에
@ExceptionHandler가 준비돼 있는지 확인하거나,- 여기서
HttpServletResponse.sendError(500, ...)로 매핑 후 로그만 남기도록 하는 방식을 검토해 주세요.외부로 노출되지 않는 커스텀 예외로 감싸 재전파하는 것도 고려할 수 있습니다.
* refactor : api 경로 추가, oauth 경로 수정 * refactor : whitelist 경로 수정 * refactor : redirect url 수정
* 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]>
작업 내용
WhiteList 목록에 /api 경로 추가
-> 기존 PR에 반영되지 않아 로그인 이슈가 있었습니다!
해결해야 할 문제
참고 사항
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit
Summary by CodeRabbit
신규 기능
버그 수정
스타일
리팩터
/api/auth/**로 통합 및 정비되었습니다.