-
Notifications
You must be signed in to change notification settings - Fork 0
[feat] Google OAuth2 로그인 구현 #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughGoogle OAuth2 로그인을 위한 전반적인 코드 추가 및 기존 회원가입/로그인 기능 비활성화가 이루어졌습니다. OAuth2 관련 DTO, 서비스, 핸들러, 리포지토리, 시큐리티 설정이 새로 도입되었고, 데이터베이스 및 OAuth2 클라이언트 설정이 환경 변수 기반으로 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant SpringApp
participant Google
User->>Browser: /login/oauth2/authorization/google 접속
Browser->>Google: OAuth2 인증 요청 (redirect)
Google->>Browser: 인증 후 callback (redirect_uri)
Browser->>SpringApp: 인증 코드 전달
SpringApp->>Google: 인증 코드로 토큰 요청
Google->>SpringApp: 액세스 토큰 반환
SpringApp->>SpringApp: CustomOAuth2UserService.loadUser()로 사용자 정보 조회/저장
SpringApp->>SpringApp: CustomSuccessHandler.onAuthenticationSuccess()로 JWT 토큰 생성 및 쿠키 설정
SpringApp->>Browser: 리다이렉트 및 쿠키 반환
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 19
🔭 Outside diff range comments (2)
src/main/java/com/opendata/domain/register/controller/RegisterController.java (1)
1-28: 회원가입 컨트롤러 비활성화 접근 방식을 재검토해 주세요.전체 컨트롤러를 주석 처리하는 것보다는 다음 중 하나를 고려해 보세요:
- OAuth2 마이그레이션이 완료되면 파일을 삭제
- 임시 비활성화라면 마이그레이션 계획에 대한 문서화 추가
- 단계적 마이그레이션을 위한 명확한 전략 수립
주석 처리된 코드는 유지보수 부담을 증가시키고 혼란을 야기할 수 있습니다.
src/main/java/com/opendata/domain/oauth2/filter/OAuth2RedirectUriCookieFilter.java (1)
1-42: 불필요해진 OAuth2RedirectUriCookieFilter 삭제 필요OAuth2 리디렉트 URI 저장 로직이 이미
src/main/java/com/opendata/domain/oauth2/repository/OAuth2CookieAuthorizationRequestRepository.java
내loadAuthorizationRequest및saveAuthorizationRequest메서드에서 구현되어 있습니다.따라서 더 이상 사용되지 않는 주석 처리된 필터 클래스를 정리하세요:
• src/main/java/com/opendata/domain/oauth2/filter/OAuth2RedirectUriCookieFilter.java – 파일 삭제 또는 완전 제거
이렇게 하면 코드 유지보수성과 가독성이 향상됩니다.
♻️ Duplicate comments (1)
src/main/java/com/opendata/domain/login/controller/LoginController.java (1)
1-22: 로그인 컨트롤러 비활성화에 대한 동일한 권장사항입니다.RegisterController와 마찬가지로, OAuth2 마이그레이션이 완료되면 주석 처리된 코드를 삭제하거나 명확한 마이그레이션 전략을 문서화하는 것을 권장합니다. 일관된 코드 정리가 필요합니다.
🧹 Nitpick comments (7)
src/main/resources/application.yml (1)
30-32: 운영 환경에서의 로그 레벨을 고려하세요.DEBUG 레벨 로깅은 개발 환경에서만 사용하고, 운영 환경에서는 INFO 이상으로 설정하는 것이 좋습니다.
프로필별 설정을 고려해보세요:
logging: level: - com.opendata.domain.oauth2.repository: DEBUG + com.opendata.domain.oauth2.repository: ${OAUTH2_LOG_LEVEL:INFO}src/main/java/com/opendata/domain/user/entity/User.java (1)
26-27: name 필드에 검증을 추가하는 것을 고려하세요.OAuth2 제공자로부터 받은 이름이 null이거나 빈 값일 가능성이 있습니다.
- @Column(name = "name") + @Column(name = "name", nullable = false) + @NotBlank private String name;src/main/java/com/opendata/domain/oauth2/dto/user/CustomOAuth2User.java (1)
25-36: 권한 생성 로직을 간소화할 수 있습니다익명 클래스 대신 람다식이나 SimpleGrantedAuthority를 사용하여 코드를 더 간결하게 만들 수 있습니다.
다음과 같이 개선할 수 있습니다:
+import org.springframework.security.core.authority.SimpleGrantedAuthority; @Override public Collection<? extends GrantedAuthority> getAuthorities() { - - Collection<GrantedAuthority> collection = new ArrayList<>(); - - collection.add(new GrantedAuthority() { - - @Override - public String getAuthority() { - - return userDto.getRole(); - } - }); - - return collection; + return List.of(new SimpleGrantedAuthority(userDto.getRole())); }src/main/java/com/opendata/global/config/SecurityConfig.java (3)
27-28: 사용하지 않는 import 제거 필요
OAuth2AuthorizationRequestRedirectFilter와OAuth2LoginAuthenticationFilter는 현재 사용되지 않고 있습니다. 주석 처리된 코드와 관련된 것으로 보이므로 제거하는 것이 좋겠습니다.-import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestRedirectFilter; -import org.springframework.security.oauth2.client.web.OAuth2LoginAuthenticationFilter;
84-85: 응답의 charset 설정 추가 필요JSON 응답에 UTF-8 charset을 명시적으로 설정하면 한글 등 다국어 처리가 안전해집니다.
-res.setContentType("application/json"); +res.setContentType("application/json; charset=UTF-8");
109-111: 주석 처리된 코드 정리 필요OAuth2 로그인으로 완전히 전환했다면 주석 처리된 기존 로그인 관련 코드를 제거하는 것이 좋겠습니다. 필요시 버전 관리 시스템에서 복구할 수 있습니다.
-// .addFilterBefore(oAuth2RedirectUriCookieFilter, OAuth2AuthorizationRequestRedirectFilter.class); -// .addFilterAfter(new JwtFilter(jwtUtil,userDetailsService), OAuth2LoginAuthenticationFilter.class); -// .addFilterAt(new LoginFilter(authenticationManager(authenticationConfiguration), jwtUtil, cookieUtil, userRepository), UsernamePasswordAuthenticationFilter.class);src/main/java/com/opendata/domain/oauth2/repository/OAuth2CookieAuthorizationRequestRepository.java (1)
103-111: 직렬화 방식 개선 고려
SerializationUtils는 보안 취약점이 있을 수 있습니다. JSON 직렬화 방식을 고려해보세요.더 안전한 JSON 기반 직렬화 구현을 원하시면 도와드릴 수 있습니다. 새로운 이슈를 생성할까요?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
build.gradle(1 hunks)src/main/java/com/opendata/domain/login/controller/LoginController.java(1 hunks)src/main/java/com/opendata/domain/oauth2/dto/GoogleResponse.java(1 hunks)src/main/java/com/opendata/domain/oauth2/dto/OAuth2Response.java(1 hunks)src/main/java/com/opendata/domain/oauth2/dto/user/CustomOAuth2User.java(1 hunks)src/main/java/com/opendata/domain/oauth2/dto/user/UserDto.java(1 hunks)src/main/java/com/opendata/domain/oauth2/filter/OAuth2RedirectUriCookieFilter.java(1 hunks)src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java(1 hunks)src/main/java/com/opendata/domain/oauth2/repository/OAuth2CookieAuthorizationRequestRepository.java(1 hunks)src/main/java/com/opendata/domain/oauth2/service/CustomOAuth2UserService.java(1 hunks)src/main/java/com/opendata/domain/register/controller/RegisterController.java(1 hunks)src/main/java/com/opendata/domain/register/dto/RegisterRequest.java(1 hunks)src/main/java/com/opendata/domain/register/service/RegisterService.java(1 hunks)src/main/java/com/opendata/domain/user/entity/User.java(2 hunks)src/main/java/com/opendata/global/config/SecurityConfig.java(4 hunks)src/main/java/com/opendata/global/security/CustomUserDetails.java(1 hunks)src/main/java/com/opendata/global/security/CustomUserDetailsService.java(0 hunks)src/main/resources/application.yml(2 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/opendata/global/security/CustomUserDetailsService.java
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/opendata/domain/register/controller/RegisterController.java (1)
src/main/java/com/opendata/domain/tourspot/controller/TourSpotController.java (1)
TourSpotController(15-34)
src/main/java/com/opendata/domain/oauth2/repository/OAuth2CookieAuthorizationRequestRepository.java (1)
src/main/java/com/opendata/global/jwt/CookieUtil.java (1)
CookieUtil(13-66)
src/main/java/com/opendata/domain/oauth2/filter/OAuth2RedirectUriCookieFilter.java (3)
src/main/java/com/opendata/global/jwt/JwtFilter.java (1)
JwtFilter(19-52)src/main/java/com/opendata/global/jwt/LoginFilter.java (3)
LoginFilter(24-77)successfulAuthentication(59-75)LoginFilter(31-36)src/main/java/com/opendata/global/jwt/CookieUtil.java (1)
CookieUtil(13-66)
🪛 ast-grep (0.38.6)
src/main/java/com/opendata/domain/oauth2/repository/OAuth2CookieAuthorizationRequestRepository.java
[warning] 81-81: A cookie was detected without setting the 'secure' flag. The 'secure' flag for cookies prevents the client from transmitting the cookie over insecure channels such as HTTP. Set the 'secure' flag by calling '.setSecure(true);'.
Context: response.addCookie(cookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
[warning] 88-88: A cookie was detected without setting the 'secure' flag. The 'secure' flag for cookies prevents the client from transmitting the cookie over insecure channels such as HTTP. Set the 'secure' flag by calling '.setSecure(true);'.
Context: response.addCookie(cookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
[warning] 81-81: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(cookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 88-88: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(cookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 88-88: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(cookie);
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java
[warning] 57-57: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(createCookie("access", token));
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 58-58: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(createCookie("refresh", refresh));
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 57-57: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(createCookie("access", token));
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
[warning] 58-58: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(createCookie("refresh", refresh));
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
🔇 Additional comments (8)
build.gradle (1)
32-32: OAuth2 클라이언트 의존성 활성화가 적절합니다.OAuth2 로그인 기능 구현을 위해 필요한 의존성이 올바르게 활성화되었습니다. Spring Boot의 의존성 관리를 통해 적절한 버전이 자동으로 선택됩니다.
src/main/java/com/opendata/domain/oauth2/dto/OAuth2Response.java (1)
3-12: OAuth2 응답을 위한 깔끔한 인터페이스 설계입니다.다양한 OAuth2 프로바이더(Google 등)를 지원하기 위한 표준 계약을 잘 정의했습니다. 메서드명이 명확하고 일관성이 있어 유지보수에 도움이 됩니다.
src/main/java/com/opendata/global/security/CustomUserDetails.java (1)
37-39: OAuth2 전환에 따른getPassword()null 반환 검증 완료현재 코드베이스에서 다음을 확인했습니다:
DaoAuthenticationProvider,AuthenticationProvider,passwordEncoder관련 실제 등록 코드가 전혀 없으며RegisterService등 로컬 인증·회원가입 기능은 모두 주석 처리되어 있습니다.따라서
CustomUserDetails#getPassword()가null을 반환해도 Spring Security OAuth2 로그인 플로우에는 전혀 영향이 없습니다. 이 변경을 승인합니다.src/main/java/com/opendata/domain/user/entity/User.java (1)
17-17: ID 자동 생성 전략이 적절합니다.
@GeneratedValue(strategy = GenerationType.IDENTITY)추가로 OAuth2를 통해 생성되는 새 사용자의 ID가 자동으로 할당됩니다.src/main/java/com/opendata/global/config/SecurityConfig.java (3)
50-53: OAuth2 관련 의존성 주입이 적절히 구현됨OAuth2 로그인 기능을 위한 필수 컴포넌트들이 올바르게 주입되었습니다.
94-95: OAuth2 관련 경로 권한 설정이 적절함OAuth2 인증 흐름에 필요한 경로들이 올바르게 공개 접근 가능하도록 설정되었습니다.
98-105: OAuth2 로그인 설정이 완벽하게 구현됨커스텀 리포지토리, 사용자 서비스, 성공 핸들러가 모두 적절히 설정되어 OAuth2 인증 흐름이 완전하게 구성되었습니다.
src/main/java/com/opendata/domain/oauth2/repository/OAuth2CookieAuthorizationRequestRepository.java (1)
31-54: OAuth2 인증 요청 저장 로직이 적절히 구현됨null 체크와 파라미터 처리가 올바르게 구현되었습니다. redirect_uri와 mode를 별도 쿠키로 저장하는 방식도 적절합니다.
| return attribute.get("sub").toString(); | ||
| } | ||
|
|
||
| @Override | ||
| public String getEmail() { | ||
|
|
||
| return attribute.get("email").toString(); | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
|
|
||
| return attribute.get("name").toString(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
null 체크 없이 toString() 호출로 인한 NPE 위험
attribute.get() 메서드가 null을 반환하거나 값 자체가 null인 경우 NullPointerException이 발생할 수 있습니다.
다음과 같이 방어적 프로그래밍을 적용하세요:
@Override
public String getProviderId() {
-
- return attribute.get("sub").toString();
+ Object sub = attribute.get("sub");
+ return sub != null ? sub.toString() : "";
}
@Override
public String getEmail() {
-
- return attribute.get("email").toString();
+ Object email = attribute.get("email");
+ return email != null ? email.toString() : "";
}
@Override
public String getName() {
-
- return attribute.get("name").toString();
+ Object name = attribute.get("name");
+ return name != null ? name.toString() : "";
}📝 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.
| return attribute.get("sub").toString(); | |
| } | |
| @Override | |
| public String getEmail() { | |
| return attribute.get("email").toString(); | |
| } | |
| @Override | |
| public String getName() { | |
| return attribute.get("name").toString(); | |
| } | |
| @Override | |
| public String getProviderId() { | |
| Object sub = attribute.get("sub"); | |
| return sub != null ? sub.toString() : ""; | |
| } | |
| @Override | |
| public String getEmail() { | |
| Object email = attribute.get("email"); | |
| return email != null ? email.toString() : ""; | |
| } | |
| @Override | |
| public String getName() { | |
| Object name = attribute.get("name"); | |
| return name != null ? name.toString() : ""; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/opendata/domain/oauth2/dto/GoogleResponse.java between
lines 23 and 36, the code calls toString() directly on values returned by
attribute.get() without checking for null, which risks NullPointerException. To
fix this, add null checks before calling toString(), returning a default value
or null if the attribute is missing or null, thereby applying defensive
programming to avoid NPEs.
| public class UserDto { | ||
| private String name; | ||
| private String email; | ||
| private String role; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
DTO의 안정성과 사용성을 개선하세요.
현재 구현에 몇 가지 개선점이 있습니다:
- 이메일 형식 검증이 없습니다
- role을 String으로 사용하면 일관성 문제가 발생할 수 있습니다
- 편의를 위한 생성자가 없습니다
다음과 같이 개선해보세요:
package com.opendata.domain.oauth2.dto.user;
+import jakarta.validation.constraints.Email;
+import jakarta.validation.constraints.NotBlank;
import lombok.Getter;
import lombok.Setter;
+import lombok.AllArgsConstructor;
+import lombok.NoArgsConstructor;
@Getter
@Setter
+@NoArgsConstructor
+@AllArgsConstructor
public class UserDto {
+ @NotBlank
private String name;
+
+ @Email
+ @NotBlank
private String email;
+
+ @NotBlank
private String role;
}또는 role에 대해 enum을 고려해보세요:
public enum UserRole {
USER, ADMIN
}🤖 Prompt for AI Agents
In src/main/java/com/opendata/domain/oauth2/dto/user/UserDto.java around lines 8
to 12, improve the DTO by adding email format validation using annotations or
manual checks, replace the role String with a UserRole enum to ensure
consistency, and add constructors including a no-args and all-args constructor
for easier instantiation and usage.
src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java
Outdated
Show resolved
Hide resolved
| String token = jwtUtil.createAccess(email); | ||
| String refresh = jwtUtil.createRefresh(email); | ||
|
|
||
| response.addCookie(createCookie("access", token)); | ||
| response.addCookie(createCookie("refresh", refresh)); | ||
| //response.addCookie(createCookie("google_access_token", accessToken)); | ||
| String targetUrl = authRequestRepository.loadRedirectUri(request); | ||
| authRequestRepository.removeCookies(response); | ||
|
|
||
| response.sendRedirect(targetUrl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
JWT 토큰 생성 및 리다이렉트 처리에 에러 핸들링 추가 필요
JWT 토큰 생성 실패나 리다이렉트 실패 시 적절한 에러 처리가 없습니다.
-String token = jwtUtil.createAccess(email);
-String refresh = jwtUtil.createRefresh(email);
-
-response.addCookie(createCookie("access", token));
-response.addCookie(createCookie("refresh", refresh));
-//response.addCookie(createCookie("google_access_token", accessToken));
-String targetUrl = authRequestRepository.loadRedirectUri(request);
-authRequestRepository.removeCookies(response);
-
-response.sendRedirect(targetUrl);
+try {
+ String token = jwtUtil.createAccess(email);
+ String refresh = jwtUtil.createRefresh(email);
+
+ response.addCookie(createCookie("access", token));
+ response.addCookie(createCookie("refresh", refresh));
+
+ String targetUrl = authRequestRepository.loadRedirectUri(request);
+ if (targetUrl == null || targetUrl.isEmpty()) {
+ targetUrl = "/"; // 기본 리다이렉트 URL
+ }
+ authRequestRepository.removeCookies(response);
+
+ response.sendRedirect(targetUrl);
+} catch (Exception e) {
+ log.error("OAuth2 인증 성공 처리 중 오류 발생: ", e);
+ response.sendRedirect("/error");
+}📝 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.
| String token = jwtUtil.createAccess(email); | |
| String refresh = jwtUtil.createRefresh(email); | |
| response.addCookie(createCookie("access", token)); | |
| response.addCookie(createCookie("refresh", refresh)); | |
| //response.addCookie(createCookie("google_access_token", accessToken)); | |
| String targetUrl = authRequestRepository.loadRedirectUri(request); | |
| authRequestRepository.removeCookies(response); | |
| response.sendRedirect(targetUrl); | |
| try { | |
| String token = jwtUtil.createAccess(email); | |
| String refresh = jwtUtil.createRefresh(email); | |
| response.addCookie(createCookie("access", token)); | |
| response.addCookie(createCookie("refresh", refresh)); | |
| String targetUrl = authRequestRepository.loadRedirectUri(request); | |
| if (targetUrl == null || targetUrl.isEmpty()) { | |
| targetUrl = "/"; // 기본 리다이렉트 URL | |
| } | |
| authRequestRepository.removeCookies(response); | |
| response.sendRedirect(targetUrl); | |
| } catch (Exception e) { | |
| log.error("OAuth2 인증 성공 처리 중 오류 발생: ", e); | |
| response.sendRedirect("/error"); | |
| } |
🧰 Tools
🪛 ast-grep (0.38.6)
[warning] 57-57: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(createCookie("access", token));
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 58-58: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(createCookie("refresh", refresh));
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 57-57: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(createCookie("access", token));
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
[warning] 58-58: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(createCookie("refresh", refresh));
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
🤖 Prompt for AI Agents
In src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java
around lines 55 to 64, there is no error handling for failures during JWT token
creation or response redirection. Add try-catch blocks around the token creation
and redirect logic to catch exceptions, log the errors appropriately, and handle
the failure gracefully, such as sending an error response or redirecting to an
error page.
| //package com.opendata.domain.register.service; | ||
| // | ||
| //import com.opendata.domain.register.dto.RegisterRequest; | ||
| //import com.opendata.domain.user.repository.UserRepository; | ||
| //import lombok.RequiredArgsConstructor; | ||
| //import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; | ||
| //import org.springframework.stereotype.Service; | ||
| // | ||
| // | ||
| //@Service | ||
| //@RequiredArgsConstructor | ||
| //public class RegisterService { | ||
| // private final BCryptPasswordEncoder passwordEncoder; | ||
| // private final UserRepository userRepository; | ||
| // | ||
| // public String registerUser(RegisterRequest registerRequest){ | ||
| // userRepository.save(registerRequest.from(passwordEncoder.encode(registerRequest.password()))); | ||
| // return "회원가입 성공"; | ||
| // } | ||
| // | ||
| // public boolean checkEmail(String email){ | ||
| // return userRepository.findUserByEmail(email) == null; | ||
| // } | ||
| //} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
기존 회원가입 서비스 코드의 처리 방식을 명확히 하세요.
전체 서비스가 주석 처리되어 있는데, OAuth2 전환으로 인해 이 코드가 더 이상 필요하지 않다면 완전히 제거하는 것이 좋겠습니다. 임시적인 비활성화라면 TODO 주석으로 의도를 명확히 표시해주세요.
만약 완전한 제거를 원한다면:
-//package com.opendata.domain.register.service;
-//
-//import com.opendata.domain.register.dto.RegisterRequest;
-//import com.opendata.domain.user.repository.UserRepository;
-//import lombok.RequiredArgsConstructor;
-//import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
-//import org.springframework.stereotype.Service;
-//
-//
-//@Service
-//@RequiredArgsConstructor
-//public class RegisterService {
-// private final BCryptPasswordEncoder passwordEncoder;
-// private final UserRepository userRepository;
-//
-// public String registerUser(RegisterRequest registerRequest){
-// userRepository.save(registerRequest.from(passwordEncoder.encode(registerRequest.password())));
-// return "회원가입 성공";
-// }
-//
-// public boolean checkEmail(String email){
-// return userRepository.findUserByEmail(email) == null;
-// }
-//}임시 비활성화라면:
+// TODO: OAuth2 전환 완료 후 제거 예정 - 기존 회원가입 로직
//package com.opendata.domain.register.service;📝 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.
| //package com.opendata.domain.register.service; | |
| // | |
| //import com.opendata.domain.register.dto.RegisterRequest; | |
| //import com.opendata.domain.user.repository.UserRepository; | |
| //import lombok.RequiredArgsConstructor; | |
| //import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; | |
| //import org.springframework.stereotype.Service; | |
| // | |
| // | |
| //@Service | |
| //@RequiredArgsConstructor | |
| //public class RegisterService { | |
| // private final BCryptPasswordEncoder passwordEncoder; | |
| // private final UserRepository userRepository; | |
| // | |
| // public String registerUser(RegisterRequest registerRequest){ | |
| // userRepository.save(registerRequest.from(passwordEncoder.encode(registerRequest.password()))); | |
| // return "회원가입 성공"; | |
| // } | |
| // | |
| // public boolean checkEmail(String email){ | |
| // return userRepository.findUserByEmail(email) == null; | |
| // } | |
| //} |
| //package com.opendata.domain.register.service; | |
| // | |
| //import com.opendata.domain.register.dto.RegisterRequest; | |
| //import com.opendata.domain.user.repository.UserRepository; | |
| //import lombok.RequiredArgsConstructor; | |
| //import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; | |
| //import org.springframework.stereotype.Service; | |
| // | |
| // | |
| //@Service | |
| //@RequiredArgsConstructor | |
| //public class RegisterService { | |
| // private final BCryptPasswordEncoder passwordEncoder; | |
| // private final UserRepository userRepository; | |
| // | |
| // public String registerUser(RegisterRequest registerRequest){ | |
| // userRepository.save(registerRequest.from(passwordEncoder.encode(registerRequest.password()))); | |
| // return "회원가입 성공"; | |
| // } | |
| // | |
| // public boolean checkEmail(String email){ | |
| // return userRepository.findUserByEmail(email) == null; | |
| // } | |
| //} | |
| // TODO: OAuth2 전환 완료 후 제거 예정 - 기존 회원가입 로직 | |
| //package com.opendata.domain.register.service; | |
| // | |
| //import com.opendata.domain.register.dto.RegisterRequest; | |
| //import com.opendata.domain.user.repository.UserRepository; | |
| //import lombok.RequiredArgsConstructor; | |
| //import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; | |
| //import org.springframework.stereotype.Service; | |
| // | |
| // | |
| //@Service | |
| //@RequiredArgsConstructor | |
| //public class RegisterService { | |
| // private final BCryptPasswordEncoder passwordEncoder; | |
| // private final UserRepository userRepository; | |
| // | |
| // public String registerUser(RegisterRequest registerRequest){ | |
| // userRepository.save(registerRequest.from(passwordEncoder.encode(registerRequest.password()))); | |
| // return "회원가입 성공"; | |
| // } | |
| // | |
| // public boolean checkEmail(String email){ | |
| // return userRepository.findUserByEmail(email) == null; | |
| // } | |
| //} |
🤖 Prompt for AI Agents
In src/main/java/com/opendata/domain/register/service/RegisterService.java lines
1 to 24, the entire RegisterService class is commented out. If this service is
no longer needed due to OAuth2 migration, remove the commented code entirely to
clean up the codebase. If you intend to keep it temporarily disabled, add a
clear TODO comment explaining why it is commented out and when it should be
revisited or removed.
| public static User create(String email, String name, Long membershipId) { | ||
| return User.builder() | ||
| .email(email) | ||
| .name(name) | ||
| .membershipId(membershipId) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
create 메서드의 매개변수 검증을 추가하세요.
OAuth2 플로우에서 null 값이 전달될 가능성을 대비해 검증 로직을 추가하는 것이 좋습니다.
public static User create(String email, String name, Long membershipId) {
+ if (email == null || email.trim().isEmpty()) {
+ throw new IllegalArgumentException("Email cannot be null or empty");
+ }
+ if (name == null || name.trim().isEmpty()) {
+ throw new IllegalArgumentException("Name cannot be null or empty");
+ }
+ if (membershipId == null) {
+ throw new IllegalArgumentException("MembershipId cannot be null");
+ }
+
return User.builder()
.email(email)
- .name(name)
+ .name(name.trim())
.membershipId(membershipId)
.build();
}📝 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.
| public static User create(String email, String name, Long membershipId) { | |
| return User.builder() | |
| .email(email) | |
| .name(name) | |
| .membershipId(membershipId) | |
| .build(); | |
| } | |
| public static User create(String email, String name, Long membershipId) { | |
| if (email == null || email.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Email cannot be null or empty"); | |
| } | |
| if (name == null || name.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Name cannot be null or empty"); | |
| } | |
| if (membershipId == null) { | |
| throw new IllegalArgumentException("MembershipId cannot be null"); | |
| } | |
| return User.builder() | |
| .email(email) | |
| .name(name.trim()) | |
| .membershipId(membershipId) | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/opendata/domain/user/entity/User.java between lines 29 and
35, the create method lacks validation for its parameters. Add null checks for
email, name, and membershipId at the start of the method, throwing appropriate
exceptions if any are null, to prevent invalid User objects from being created
during OAuth2 flows.
| public void updateUserInfo(String name) { | ||
| this.name = name; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
updateUserInfo 메서드에도 검증을 추가하세요.
사용자 정보 업데이트 시에도 유효성 검증이 필요합니다.
public void updateUserInfo(String name) {
+ if (name == null || name.trim().isEmpty()) {
+ throw new IllegalArgumentException("Name cannot be null or empty");
+ }
- this.name = name;
+ this.name = name.trim();
}📝 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.
| public void updateUserInfo(String name) { | |
| this.name = name; | |
| } | |
| public void updateUserInfo(String name) { | |
| if (name == null || name.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Name cannot be null or empty"); | |
| } | |
| this.name = name.trim(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/opendata/domain/user/entity/User.java around lines 36 to
38, the updateUserInfo method lacks validation for the input name. Add
validation logic to check that the name parameter is not null or empty before
assigning it to this.name. If the validation fails, throw an appropriate
exception or handle the error to prevent invalid user data updates.
src/main/resources/application.yml
Outdated
| client-name: google | ||
| client-id: ${GOOGLE_CLIENT_ID} | ||
| client-secret: ${GOOGLE_CLIENT_SECRET} | ||
| redirect-uri: "http://localhost:8080/login/oauth2/code/google" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
리다이렉트 URI를 환경별로 설정 가능하게 만드세요.
현재 localhost로 하드코딩되어 있어 운영 환경에서 문제가 될 수 있습니다.
- redirect-uri: "http://localhost:8080/login/oauth2/code/google"
+ redirect-uri: ${OAUTH2_REDIRECT_URI:http://localhost:8080/login/oauth2/code/google}📝 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.
| redirect-uri: "http://localhost:8080/login/oauth2/code/google" | |
| redirect-uri: ${OAUTH2_REDIRECT_URI:http://localhost:8080/login/oauth2/code/google} |
🤖 Prompt for AI Agents
In src/main/resources/application.yml at line 23, the redirect-uri is hardcoded
to localhost, which is not suitable for different environments. Modify the
configuration to use a placeholder or environment variable for the redirect-uri
so it can be set differently for each environment (e.g., development, staging,
production). Update the YAML to reference this variable instead of a fixed URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java (2)
55-65: JWT 토큰 생성 및 리다이렉트 처리에 에러 핸들링 추가 필요JWT 토큰 생성 실패나 리다이렉트 실패 시 적절한 에러 처리가 없습니다.
67-76: 쿠키 보안 설정 개선 필요현재 쿠키 설정에 CSRF 방어를 위한 SameSite 속성이 누락되어 있고, Secure 플래그가 주석 처리되어 있습니다.
🧹 Nitpick comments (1)
src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java (1)
47-53: 사용하지 않는 변수들을 제거하세요
accessToken과role변수가 추출되었지만 사용되지 않습니다. 사용하지 않는다면 제거하세요.-String accessToken = authorizedClient.getAccessToken().getTokenValue(); - - -Collection<? extends GrantedAuthority> authorities = authentication.getAuthorities(); -Iterator<? extends GrantedAuthority> iterator = authorities.iterator(); -GrantedAuthority auth = iterator.next(); -String role = auth.getAuthority();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java(1 hunks)
🧰 Additional context used
🪛 ast-grep (0.38.6)
src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java
[warning] 57-57: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(createCookie("access", token));
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 58-58: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(createCookie("refresh", refresh));
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 57-57: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(createCookie("access", token));
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
[warning] 58-58: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(createCookie("refresh", refresh));
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java
Outdated
Show resolved
Hide resolved
|
코드래빗 리뷰 반영 ㄱㄱ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java (2)
55-64: JWT 토큰 생성 및 리다이렉트 처리에 에러 핸들링 추가 필요JWT 토큰 생성 실패나 리다이렉트 실패 시 적절한 에러 처리가 없습니다.
67-76: 쿠키 보안 설정 개선 필요현재 쿠키 설정에 CSRF 방어를 위한 SameSite 속성이 누락되어 있고, Secure 플래그가 주석 처리되어 있습니다.
🧹 Nitpick comments (1)
src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java (1)
11-11: 사용하지 않는 import 제거 필요
@Setter어노테이션이 import되었지만 실제로 사용되지 않습니다.-import lombok.Setter;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/opendata/domain/oauth2/dto/user/CustomOAuth2User.java(1 hunks)src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java(1 hunks)src/main/resources/application.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/resources/application.yml
- src/main/java/com/opendata/domain/oauth2/dto/user/CustomOAuth2User.java
🧰 Additional context used
🪛 ast-grep (0.38.6)
src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java
[warning] 57-57: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(createCookie("access", token));
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
[warning] 58-58: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(createCookie("refresh", refresh));
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
[warning] 57-57: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(createCookie("access", token));
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 58-58: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(createCookie("refresh", refresh));
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
| Collection<? extends GrantedAuthority> authorities = authentication.getAuthorities(); | ||
| Iterator<? extends GrantedAuthority> iterator = authorities.iterator(); | ||
| GrantedAuthority auth = iterator.next(); | ||
| String role = auth.getAuthority(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
권한 추출 시 잠재적 오류 처리 필요
authorities가 비어있을 경우 iterator.next()에서 NoSuchElementException이 발생할 수 있습니다.
-Collection<? extends GrantedAuthority> authorities = authentication.getAuthorities();
-Iterator<? extends GrantedAuthority> iterator = authorities.iterator();
-GrantedAuthority auth = iterator.next();
-String role = auth.getAuthority();
+Collection<? extends GrantedAuthority> authorities = authentication.getAuthorities();
+String role = authorities.isEmpty() ? "ROLE_USER" : authorities.iterator().next().getAuthority();📝 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.
| Collection<? extends GrantedAuthority> authorities = authentication.getAuthorities(); | |
| Iterator<? extends GrantedAuthority> iterator = authorities.iterator(); | |
| GrantedAuthority auth = iterator.next(); | |
| String role = auth.getAuthority(); | |
| Collection<? extends GrantedAuthority> authorities = authentication.getAuthorities(); | |
| String role = authorities.isEmpty() | |
| ? "ROLE_USER" | |
| : authorities.iterator().next().getAuthority(); |
🤖 Prompt for AI Agents
In src/main/java/com/opendata/domain/oauth2/handler/CustomSuccessHandler.java
around lines 50 to 53, the code retrieves the first authority without checking
if the authorities collection is empty, which can cause a
NoSuchElementException. To fix this, add a check to ensure that authorities is
not empty before calling iterator.next(), and handle the empty case
appropriately to avoid the exception.
📌 PR 개요
✅ 변경사항
🔍 체크리스트
📎 관련 이슈
Closes #38
💬 기타 참고사항
Summary by CodeRabbit
신규 기능
버그 수정
리팩터링
스타일/기타