Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRefactors token expiration handling across JWT provider and filters: JwtTokenProvider now throws ExpiredJwtException on expiration; TokenFilter propagates ExpiredJwtException and adjusts other exception rethrows; TokenExceptionFilter serializes JSON error responses for CustomRuntimeException, ExpiredJwtException, and generic exceptions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant TF as TokenFilter
participant JP as JwtTokenProvider
participant TEF as TokenExceptionFilter
participant RS as Response
Client->>TF: HTTP Request with Access Token
TF->>JP: setAuthentication(token)
JP-->>TF: ExpiredJwtException
TF-->>TEF: propagate ExpiredJwtException
TEF->>RS: 406 JSON { code: TOKEN_EXPIRED, message }
RS-->>Client: Response
sequenceDiagram
autonumber
actor Client
participant TF as TokenFilter
participant JP as JwtTokenProvider
participant TEF as TokenExceptionFilter
participant RS as Response
Client->>TF: HTTP Request with invalid token
TF->>JP: setAuthentication(token)
JP-->>TF: JwtException / IllegalArgumentException
TF-->>TEF: propagate CustomRuntimeException(INVALID_TOKEN_TYPE)
TEF->>RS: 401 JSON { code, message }
RS-->>Client: Response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @wlgns12370, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
이 PR은 인증 필터 내에서 토큰 만료 및 일반적인 에러 핸들링 로직을 리팩토링합니다. ExpiredJwtException에 대한 특정 처리를 도입하여 만료된 토큰에 대해 더 세분화된 에러 응답을 제공하고, TokenExceptionFilter에서 ObjectMapper를 사용하여 에러 응답을 표준화된 JSON 형식으로 반환합니다. 또한, TokenFilter의 예외 흐름을 조정하여 ExpiredJwtException을 전파하고, 리프레시 토큰 재발급 불가 시 사용되는 에러 코드를 개선합니다.
Highlights
- JWT 토큰 만료 예외 처리 개선:
JwtTokenProvider에서 토큰 만료 시CustomRuntimeException대신ExpiredJwtException을 발생시키도록 변경하여, 토큰 만료에 대한 보다 구체적인 예외 처리가 가능해졌습니다. - 필터 내 에러 응답 표준화:
TokenExceptionFilter에서CustomRuntimeException,ExpiredJwtException, 그리고 일반Exception에 대해 각각 다른 HTTP 상태 코드와 함께 표준화된 JSON 형식의 에러 응답을 제공하도록 개선되었습니다.ObjectMapper를 사용하여 응답 본문을 작성합니다. - 토큰 필터 예외 전파 로직 수정:
TokenFilter에서ExpiredJwtException을 명시적으로 캐치하여 로그를 남기고 재전파함으로써,TokenExceptionFilter에서 해당 예외를 정확히 처리할 수 있도록 변경되었습니다. 또한, 리프레시 토큰 재발급 실패 시NOT_FOUND_TOKEN에러 코드를 사용하도록 수정되었습니다.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
안녕하세요. 토큰 만료 및 필터 예외 처리 리팩토링 PR 리뷰입니다.
전반적으로 ExpiredJwtException을 도입하여 만료된 토큰 예외를 명시적으로 처리하고, TokenExceptionFilter를 통해 필터단에서 발생하는 예외를 일관되게 처리하도록 개선한 점이 좋습니다.
다만, TokenExceptionFilter에서 일부 코드 중복과 로깅 누락, HTTP 상태 코드 사용에 대한 개선점이 보입니다. 또한 JwtTokenProvider의 불필요한 catch 블록을 정리하면 코드가 더 깔끔해질 것 같습니다.
자세한 내용은 각 파일의 코드 리뷰 댓글을 참고해주세요.
| .code(ex.getCode()) | ||
| .build(); | ||
|
|
||
| response.setStatus(HttpStatus.UNAUTHORIZED.value()); |
There was a problem hiding this comment.
CustomRuntimeException은 자체적으로 HttpStatus를 가지고 있습니다. 하지만 여기서는 HttpStatus.UNAUTHORIZED로 상태 코드를 고정하고 있어, 예외에 담긴 정확한 상태 코드가 무시됩니다. ex.getStatus().value()를 사용하여 동적으로 상태 코드를 설정해야 올바른 오류 응답을 클라이언트에 전달할 수 있습니다.
| response.setStatus(HttpStatus.UNAUTHORIZED.value()); | |
| response.setStatus(ex.getStatus().value()); |
| } catch (ExpiredJwtException exception) { | ||
| throw exception; | ||
| } catch (JwtException | IllegalArgumentException exception) { |
There was a problem hiding this comment.
이 catch (ExpiredJwtException exception) 블록은 예외를 잡아서 아무런 추가 작업 없이 다시 던지고 있습니다. 이는 불필요하며 코드를 복잡하게 만듭니다. 이 catch 블록을 제거해도 동작은 동일하게 유지되면서 코드가 더 간결해집니다.
| } catch (CustomRuntimeException ex) { | ||
|
|
||
| ErrorResponse errorResponse = ErrorResponse.builder() | ||
| .status(ex.getStatus().value()) | ||
| .message(ex.getMessage()) | ||
| .code(ex.getCode()) | ||
| .build(); | ||
|
|
||
| response.setStatus(HttpStatus.UNAUTHORIZED.value()); | ||
| response.setContentType(MediaType.APPLICATION_JSON_VALUE); | ||
| response.setCharacterEncoding("UTF-8"); | ||
|
|
||
| objectMapper.writeValue(response.getWriter(), errorResponse); | ||
| } catch (ExpiredJwtException ex) { | ||
| ErrorResponse errorResponse = ErrorResponse.builder() | ||
| .status(HttpStatus.NOT_ACCEPTABLE.value()) | ||
| .message("토큰이 만료되었습니다.") | ||
| .code("TOKEN_EXPIRED") | ||
| .build(); | ||
|
|
||
| response.setStatus(HttpStatus.NOT_ACCEPTABLE.value()); | ||
| response.setContentType(MediaType.APPLICATION_JSON_VALUE); | ||
| response.setCharacterEncoding("UTF-8"); | ||
| response.sendError(exception.getStatus().value(), exception.getMessage()); | ||
|
|
||
| objectMapper.writeValue(response.getWriter(), errorResponse); | ||
| } catch (Exception ex) { | ||
| ErrorResponse errorResponse = ErrorResponse.builder() | ||
| .status(HttpStatus.INTERNAL_SERVER_ERROR.value()) | ||
| .message("서버 오류가 발생했습니다.") | ||
| .code("INTERNAL_SERVER_ERROR") | ||
| .build(); | ||
|
|
||
| response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR.value()); | ||
| response.setContentType(MediaType.APPLICATION_JSON_VALUE); | ||
| response.setCharacterEncoding("UTF-8"); | ||
|
|
||
| objectMapper.writeValue(response.getWriter(), errorResponse); | ||
| } |
There was a problem hiding this comment.
현재 catch 블록들에는 코드 중복과 로깅 누락이라는 두 가지 개선점이 있습니다.
- 코드 중복: 세 개의
catch블록에서ErrorResponse를 생성하고 응답을 설정하는 코드가 거의 동일합니다. 이 로직을 별도의 private 메서드로 추출하면 중복을 줄이고 코드 가독성과 유지보수성을 높일 수 있습니다. - 로깅 누락:
catch (Exception ex)블록에서 예외를 로깅하지 않고 있습니다. 예상치 못한 오류를 추적하고 디버깅하기 어렵게 만들 수 있으므로, 클래스에@Slf4j를 추가하고log.error("...")를 사용하여 예외를 기록하는 것이 중요합니다.
아래는 리팩토링 예시입니다.
// private helper method
private void writeErrorResponse(HttpServletResponse res, HttpStatus status, String msg, String code) throws IOException {
res.setStatus(status.value());
res.setContentType(MediaType.APPLICATION_JSON_VALUE);
res.setCharacterEncoding("UTF-8");
ErrorResponse errRes = ErrorResponse.builder()
.status(status.value()).message(msg).code(code).build();
objectMapper.writeValue(res.getWriter(), errRes);
}
// catch (Exception ex) block
// log.error("Unhandled exception in filter chain", ex);
// writeErrorResponse(response, HttpStatus.INTERNAL_SERVER_ERROR, "서버 오류가 발생했습니다.", "INTERNAL_SERVER_ERROR");| ErrorResponse errorResponse = ErrorResponse.builder() | ||
| .status(HttpStatus.NOT_ACCEPTABLE.value()) | ||
| .message("토큰이 만료되었습니다.") | ||
| .code("TOKEN_EXPIRED") | ||
| .build(); | ||
|
|
||
| response.setStatus(HttpStatus.NOT_ACCEPTABLE.value()); | ||
| response.setContentType(MediaType.APPLICATION_JSON_VALUE); | ||
| response.setCharacterEncoding("UTF-8"); | ||
| response.sendError(exception.getStatus().value(), exception.getMessage()); | ||
|
|
||
| objectMapper.writeValue(response.getWriter(), errorResponse); |
There was a problem hiding this comment.
만료된 토큰에 대해 HttpStatus.NOT_ACCEPTABLE (406)을 반환하고 있습니다. 이는 일반적으로 클라이언트의 Accept 헤더와 서버가 생성할 수 있는 컨텐츠 타입이 맞지 않을 때 사용됩니다. 토큰이 만료된 경우에는 인증 실패를 의미하는 HttpStatus.UNAUTHORIZED (401)를 사용하는 것이 더 일반적이고 표준에 가깝습니다. 이전 구현에서도 UNAUTHORIZED를 사용했던 만큼, 일관성을 위해 변경을 고려해보시는 것이 좋겠습니다.
| ErrorResponse errorResponse = ErrorResponse.builder() | |
| .status(HttpStatus.NOT_ACCEPTABLE.value()) | |
| .message("토큰이 만료되었습니다.") | |
| .code("TOKEN_EXPIRED") | |
| .build(); | |
| response.setStatus(HttpStatus.NOT_ACCEPTABLE.value()); | |
| response.setContentType(MediaType.APPLICATION_JSON_VALUE); | |
| response.setCharacterEncoding("UTF-8"); | |
| response.sendError(exception.getStatus().value(), exception.getMessage()); | |
| objectMapper.writeValue(response.getWriter(), errorResponse); | |
| ErrorResponse errorResponse = ErrorResponse.builder() | |
| .status(HttpStatus.UNAUTHORIZED.value()) | |
| .message("토큰이 만료되었습니다.") | |
| .code("TOKEN_EXPIRED") | |
| .build(); | |
| response.setStatus(HttpStatus.UNAUTHORIZED.value()); | |
| response.setContentType(MediaType.APPLICATION_JSON_VALUE); | |
| response.setCharacterEncoding("UTF-8"); | |
| objectMapper.writeValue(response.getWriter(), errorResponse); |
1 participant
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
Bug Fixes
Refactor