Conversation
Summary of ChangesHello @Dockerel, 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은 애플리케이션의 핵심 아키텍처를 재정비하고, 인증 시스템을 JWT 기반으로 전면 교체하며, 데이터 모델을 간소화하는 대규모 리팩토링을 포함합니다. 또한, API 응답 및 예외 처리 방식을 표준화하고, API 문서화 도구를 도입하여 개발 효율성과 코드 품질을 향상시키는 데 중점을 두었습니다. 이러한 변화는 향후 기능 확장 및 유지보수성을 크게 개선할 것입니다. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
|
|
||
| // refreshToken 생성 및 저장 | ||
| String newRefreshToken = jwtProvider.createRefreshToken(email); | ||
| memberRepository.saveRefreshToken(refreshToken, email, JwtProvider.REFRESH_TOKEN_EXPIRE_SECONDS); |
There was a problem hiding this comment.
reissueTokens 메서드에서 새로운 리프레시 토큰을 생성한 후, Redis에 저장할 때 이전 리프레시 토큰(refreshToken)을 사용하고 있습니다. newRefreshToken을 사용해야 정상적으로 토큰 재발급이 계속 이루어질 수 있습니다.
| memberRepository.saveRefreshToken(refreshToken, email, JwtProvider.REFRESH_TOKEN_EXPIRE_SECONDS); | |
| memberRepository.saveRefreshToken(newRefreshToken, email, JwtProvider.REFRESH_TOKEN_EXPIRE_SECONDS); |
| @ExceptionHandler(ServletRequestBindingException.class) | ||
| public ResponseEntity<ApiResponse<Object>> servletRequestBindingException(ServletRequestBindingException e) { | ||
| ApiResponse<Object> response = ApiResponse.error(new ErrorMessage(ErrorType.USER_LOGIN_REQUIRED_ERROR)); | ||
| return new ResponseEntity<>(response, ErrorType.BINDING_ERROR.getStatus()); | ||
| } |
There was a problem hiding this comment.
servletRequestBindingException 핸들러에서 USER_LOGIN_REQUIRED_ERROR를 사용하고 있는데, 이 에러 타입의 HTTP 상태는 UNAUTHORIZED(401)입니다. 하지만 응답을 생성할 때는 BINDING_ERROR의 상태인 BAD_REQUEST(400)를 사용하고 있어 불일치가 발생합니다. USER_LOGIN_REQUIRED_ERROR의 상태 코드를 사용하도록 수정해야 합니다.
| @ExceptionHandler(ServletRequestBindingException.class) | |
| public ResponseEntity<ApiResponse<Object>> servletRequestBindingException(ServletRequestBindingException e) { | |
| ApiResponse<Object> response = ApiResponse.error(new ErrorMessage(ErrorType.USER_LOGIN_REQUIRED_ERROR)); | |
| return new ResponseEntity<>(response, ErrorType.BINDING_ERROR.getStatus()); | |
| } | |
| @ExceptionHandler(ServletRequestBindingException.class) | |
| public ResponseEntity<ApiResponse<Object>> servletRequestBindingException(ServletRequestBindingException e) { | |
| ApiResponse<Object> response = ApiResponse.error(new ErrorMessage(ErrorType.USER_LOGIN_REQUIRED_ERROR)); | |
| return new ResponseEntity<>(response, ErrorType.USER_LOGIN_REQUIRED_ERROR.getStatus()); | |
| } |
build.gradle
Outdated
| implementation group: 'org.springdoc', name: 'springdoc-openapi-starter-webmvc-ui', version: '2.7.0' | ||
|
|
||
| // security | ||
| implementation 'org.springframework.security:spring-security-crypto:6.0.3' |
There was a problem hiding this comment.
spring-security-crypto 의존성의 버전을 6.0.3으로 직접 지정하셨습니다. Spring Boot에서는 dependency-management 플러그인을 통해 의존성들의 호환되는 버전을 관리해주는 것이 일반적입니다. 버전을 직접 명시하면 다른 의존성과의 충돌이 발생할 수 있으니, 버전을 제거하여 Spring Boot의 BOM(Bill of Materials)이 관리하도록 하는 것을 권장합니다.
implementation 'org.springframework.security:spring-security-crypto'
| } catch (Exception e) { | ||
| throw new KnuChatbotException(ErrorType.USER_INVALID_REFRESH_TOKEN_ERROR); | ||
| } |
There was a problem hiding this comment.
validateToken 메서드에서 Exception으로 예외를 잡고 있습니다. 너무 광범위한 예외 처리이며, JWT 파싱과 관련 없는 다른 런타임 예외까지 잡을 수 있습니다. io.jsonwebtoken.JwtException과 같이 더 구체적인 예외를 잡도록 수정하는 것이 좋습니다. getClaimsFromAccessToken 메서드(85-87 라인)도 마찬가지입니다.
| } catch (Exception e) { | |
| throw new KnuChatbotException(ErrorType.USER_INVALID_REFRESH_TOKEN_ERROR); | |
| } | |
| } catch (io.jsonwebtoken.JwtException e) { | |
| throw new KnuChatbotException(ErrorType.USER_INVALID_REFRESH_TOKEN_ERROR); | |
| } |
| @@ -0,0 +1,12 @@ | |||
| package knu_chatbot.application.util; | |||
|
|
|||
| public abstract class AuthConst { | |||
There was a problem hiding this comment.
상수만 포함하는 유틸리티 클래스는 abstract로 선언하기보다 final로 선언하고 private 생성자를 추가하여 상속과 인스턴스화를 모두 막는 것이 일반적입니다.
| public abstract class AuthConst { | |
| public final class AuthConst { | |
| private AuthConst() {} |
| } | ||
|
|
||
| public static <T> ApiResponse<T> success(T data) { | ||
| return new ApiResponse(ResultType.SUCCESS, data, null); |
There was a problem hiding this comment.
new ApiResponse(...)에서 raw type을 사용하고 있습니다. 타입 안정성을 위해 다이아몬드 연산자 <>를 사용하는 것이 좋습니다. error 메서드도 마찬가지입니다.
| return new ApiResponse(ResultType.SUCCESS, data, null); | |
| return new ApiResponse<>(ResultType.SUCCESS, data, null); |
| return new OpenAPI() | ||
| .components(new Components()) | ||
| .info(createApiInfo()) | ||
| .addSecurityItem(new SecurityRequirement().addList(BEARER_AUTH)) | ||
| .schemaRequirement(BEARER_AUTH, createSecurityScheme()); |
There was a problem hiding this comment.
schemaRequirement() 메서드는 deprecated 되었습니다. components()에 addSecuritySchemes()를 사용하여 보안 스키마를 등록하는 것이 좋습니다.
| return new OpenAPI() | |
| .components(new Components()) | |
| .info(createApiInfo()) | |
| .addSecurityItem(new SecurityRequirement().addList(BEARER_AUTH)) | |
| .schemaRequirement(BEARER_AUTH, createSecurityScheme()); | |
| return new OpenAPI() | |
| .components(new Components() | |
| .addSecuritySchemes(BEARER_AUTH, createSecurityScheme())) | |
| .info(createApiInfo()) | |
| .addSecurityItem(new SecurityRequirement().addList(BEARER_AUTH)); |
| private String getRefreshTokenFromCookie(HttpServletRequest request) { | ||
| String refreshToken = null; | ||
|
|
||
| Cookie[] cookies = request.getCookies(); | ||
| if (cookies != null) { | ||
| for (Cookie cookie : cookies) { | ||
| if (cookie.getName().equals(REFRESH_TOKEN)) { | ||
| refreshToken = cookie.getValue(); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (refreshToken == null) { | ||
| throw new KnuChatbotException(ErrorType.USER_COOKIE_REFRESH_TOKEN_ERROR); | ||
| } | ||
|
|
||
| return refreshToken; | ||
| } |
There was a problem hiding this comment.
getRefreshTokenFromCookie 메서드에서 쿠키를 찾기 위해 직접 반복문을 사용하고 있습니다. java.util.Arrays.stream()을 사용하면 코드를 더 간결하고 선언적으로 만들 수 있습니다.
private String getRefreshTokenFromCookie(HttpServletRequest request) {
Cookie[] cookies = request.getCookies();
if (cookies == null) {
throw new KnuChatbotException(ErrorType.USER_COOKIE_REFRESH_TOKEN_ERROR);
}
return java.util.Arrays.stream(cookies)
.filter(cookie -> REFRESH_TOKEN.equals(cookie.getName()))
.findFirst()
.map(Cookie::getValue)
.orElseThrow(() -> new KnuChatbotException(ErrorType.USER_COOKIE_REFRESH_TOKEN_ERROR));
}|
|
||
| @NotBlank(message = "이메일은 필수입니다.") | ||
| @Email(message = "이메일 형태가 아닙니다.") | ||
| String email; |
There was a problem hiding this comment.
email 필드가 private으로 선언되지 않았습니다. 캡슐화 원칙을 지키기 위해 필드는 private으로 선언하는 것이 좋습니다.
| String email; | |
| private String email; |
| DISCORD_ENVIRONMENT=dev No newline at end of file | ||
| DISCORD_ENVIRONMENT=dev | ||
|
|
||
| SECRET_KEY=hello_world_hello_world_hello_world No newline at end of file |
There was a problem hiding this comment.
개발 환경이라도 hello_world...와 같이 예측하기 쉬운 시크릿 키를 사용하는 것은 안전하지 않습니다. 더 강력하고 무작위적인 문자열을 사용하고, Git에 커밋되지 않도록 환경 변수나 다른 secrets management 도구를 통해 관리하는 것을 권장합니다.
1 participant
#️⃣ 연관된 이슈
x
📚 배경
x
📝 작업 내용
💬 리뷰 요구사항
x
✏ Git Close
x