Conversation
Summary of ChangesHello, 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! 이 Pull Request는 애플리케이션의 전반적인 인증 흐름과 사용자 권한 처리 방식을 개선하는 데 중점을 둡니다. 일반 로그인 및 OAuth2 로그인 모두에서 사용자 이메일을 기반으로 한 인증을 강화하고, JWT 토큰에 사용자 역할을 명시적으로 포함시켜 권한 관리를 더욱 세밀하게 만들었습니다. 또한, 보안 설정을 모듈화하고 CORS 정책을 별도로 관리하여 시스템의 유연성과 유지보수성을 향상했습니다. 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
|
There was a problem hiding this comment.
Code Review
This PR includes important changes to improve authentication and authorization logic. Overall, the changes are positive, such as using DTOs and cleaning up the logic. However, some security and performance issues were found. In particular, passing tokens as URL parameters after OAuth2 login is a significant security risk and must be corrected. Also, there are opportunities to improve performance by removing duplicate database queries and Redis storage logic. Finally, the compilation error caused by separating the CORS configuration into CorsConfig must also be fixed. Please refer to the individual code review comments for details.
I am having trouble creating individual review comments. Click here to see my feedback.
src/main/java/com/aibe/team2/domain/auth/util/OAuth2SuccessHandler.java (51-56)
Passing JWT tokens in URL query parameters is a significant security vulnerability. URLs can be exposed in server logs, browser history, etc., which can lead to token theft. It is recommended to use the previous implementation of transmitting tokens via secure HttpOnly cookies.
Also, the redirect URL is hardcoded. It is better to manage this value in a configuration file such as application.properties for maintainability.
org.springframework.http.ResponseCookie accessTokenCookie = org.springframework.http.ResponseCookie.from("accessToken", accessToken)
.path("/")
.httpOnly(true)
.secure(true)
.sameSite("Lax")
.maxAge(3600) // 1시간, 설정값으로 관리하는 것을 권장합니다.
.build();
org.springframework.http.ResponseCookie refreshTokenCookie = org.springframework.http.ResponseCookie.from("refreshToken", refreshToken)
.path("/")
.httpOnly(true)
.secure(true)
.sameSite("Lax")
.maxAge(604800) // 7일, 설정값으로 관리하는 것을 권장합니다.
.build();
response.addHeader(org.springframework.http.HttpHeaders.SET_COOKIE, accessTokenCookie.toString());
response.addHeader(org.springframework.http.HttpHeaders.SET_COOKIE, refreshTokenCookie.toString());
// TODO: targetUrl을 설정 파일에서 관리하도록 변경
String targetUrl = "http://localhost:5173/AIBE4_FinalProject_Team2_FE/oauth/callback";References
- To prevent token theft, do not pass JWT tokens in URL query parameters. Instead, transmit them using secure, HTTP-only cookies.
src/main/java/com/aibe/team2/global/config/SecurityConfig.java (105-117)
After moving the corsConfigurationSource Bean to CorsConfig, the corsConfigurationSource() method call on line 50 of SecurityConfig was not updated. This will cause a compilation error. You should inject CorsConfigurationSource as a dependency into SecurityConfig and use it.
src/main/java/com/aibe/team2/domain/auth/controller/AuthController.java (49-55)
The Authentication object returned by authenticationManager.authenticate() already contains user information. Utilizing this information can reduce unnecessary database calls to retrieve the user. You can obtain the Member object by retrieving CustomUserDetails via authentication.getPrincipal().
Authentication authentication = authenticationManager.authenticate(
new UsernamePasswordAuthenticationToken(email, password)
);
// 2. 사용자 조회
com.aibe.team2.domain.auth.dto.CustomUserDetails userDetails = (com.aibe.team2.domain.auth.dto.CustomUserDetails) authentication.getPrincipal();
Member member = userDetails.getMember();
src/main/java/com/aibe/team2/domain/auth/controller/AuthController.java (61-64)
The jwtTokenProvider.createRefreshToken() method already saves the refresh token to Redis. Therefore, there is no need to save it again here. This code should be removed.
src/main/java/com/aibe/team2/domain/auth/controller/AuthController.java (134-137)
There is no need to make an additional database call to retrieve the Member object. You can directly extract the role information from the refresh token. This reduces unnecessary database queries and improves performance.
String role = jwtTokenProvider.getRole(refreshToken);
String newAccessToken = jwtTokenProvider.createAccessToken(email, role);
src/main/java/com/aibe/team2/global/config/CorsConfig.java (18-20)
The allowed Origin is hardcoded. Considering deployment to various environments such as development, staging, and production, it is recommended to configure this value in the application.properties file and inject it using the @Value annotation.
관련 이슈
작업 내용
체크 리스트