refactor(user): 토큰 전달 방식 수정 #54
Conversation
|
""" WalkthroughOAuth2 로그인 성공 시 토큰 전달 방식이 변경되었습니다. 리프레시 토큰은 HttpOnly 쿠키에 저장되고, 액세스 토큰은 프론트엔드 URL 쿼리 파라미터로 전달됩니다. 기존 JSON 응답 대신 쿠키 설정과 리다이렉트를 통해 토큰이 전달되며, 코드가 일부 간소화되었습니다. Changes
Poem
""" 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🪛 ast-grep (0.38.1)src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java[warning] 58-58: 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 'refreshTokenCookie.setSecure(true);'. (cookie-secure-flag-false-java) [warning] 61-61: 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 (cookie-missing-samesite-java) 🔇 Additional comments (1)
✨ 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: 2
🧹 Nitpick comments (1)
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java (1)
48-50: 토큰 생성 로직 개선사항 제안토큰 생성 로직은 정상적으로 작동하지만, 매직 넘버와 상수 관리 측면에서 개선할 수 있습니다.
상수를 별도로 정의하여 유지보수성을 높여보세요:
+private static final long ACCESS_TOKEN_EXPIRY = 30 * 60 * 1000L; // 30분 +private static final long REFRESH_TOKEN_EXPIRY = 30L * 24 * 60 * 60 * 1000L; // 30일 -String accessToken = jwtUtil.createAccessToken("accessToken", userId, role, 30 * 60 * 1000L); // 30분 -String refreshToken = jwtUtil.createRefreshToken("refreshToken", userId, 30L * 24 * 60 * 60 * 1000L); // 30일 +String accessToken = jwtUtil.createAccessToken("accessToken", userId, role, ACCESS_TOKEN_EXPIRY); +String refreshToken = jwtUtil.createRefreshToken("refreshToken", userId, REFRESH_TOKEN_EXPIRY);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java(2 hunks)
🧰 Additional context used
🪛 ast-grep (0.38.1)
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java
[warning] 61-61: 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(refreshTokenCookie);
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: 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 'refreshTokenCookie.setSecure(true);'.
Context: refreshTokenCookie.setSecure(false);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-secure-flag-false-java)
🔇 Additional comments (2)
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java (2)
22-22: Cookie import 추가 확인완료HttpOnly 쿠키 구현을 위한 필수 import가 적절히 추가되었습니다.
43-46: 코드 간소화가 잘 적용되었습니다사용자 정보 추출 로직이 더 간결하고 읽기 쉽게 개선되었네요. 중간 변수를 제거하여 코드가 깔끔해졌습니다.
| // 3. 프론트엔드로 리다이렉트 (accessToken만 쿼리로 전달) | ||
| String targetUrl = "http://localhost:5173/login/success?accessToken=" + accessToken; | ||
| response.sendRedirect(targetUrl); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
리다이렉트 URL 하드코딩 문제 해결 필요
현재 프론트엔드 URL이 하드코딩되어 있어서 개발/스테이징/운영 환경별로 다른 URL을 사용하기 어려울 것 같습니다. 또한 쿼리 파라미터로 access token을 전달하는 것도 보안상 고려사항이 있습니다.
다음과 같은 개선을 권장합니다:
-// 3. 프론트엔드로 리다이렉트 (accessToken만 쿼리로 전달)
-String targetUrl = "http://localhost:5173/login/success?accessToken=" + accessToken;
-response.sendRedirect(targetUrl);
+// 3. 프론트엔드로 리다이렉트 (환경별 URL 설정)
+String frontendUrl = getFrontendUrl(); // 환경별 설정에서 가져오기
+String targetUrl = frontendUrl + "/login/success";
+
+// Access token도 쿠키로 전달 (보안 강화)
+Cookie accessTokenCookie = new Cookie("accessToken", accessToken);
+accessTokenCookie.setHttpOnly(false); // JS에서 접근 가능해야 함
+accessTokenCookie.setSecure(true);
+accessTokenCookie.setPath("/");
+accessTokenCookie.setMaxAge(30 * 60); // 30분
+response.addCookie(accessTokenCookie);
+
+response.sendRedirect(targetUrl);환경별 URL 설정을 위해 application.yml에 설정값 추가도 고려해보세요:
app:
frontend:
url: ${FRONTEND_URL:http://localhost:5173}🤖 Prompt for AI Agents
In
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java
around lines 64 to 66, the redirect URL is hardcoded, which limits flexibility
across environments and poses security concerns by passing the access token as a
query parameter. To fix this, externalize the frontend URL by adding a
configuration property in application.yml (e.g., app.frontend.url) and inject
this property into the handler class. Then, construct the redirect URL
dynamically using the injected frontend URL. Additionally, consider a more
secure method to transmit the access token instead of including it in the query
string.
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java (1)
68-70: 하드코딩된 URL과 보안 문제가 여전히 존재합니다이전 리뷰에서 지적된 문제들이 아직 해결되지 않았습니다:
- 프론트엔드 URL이 하드코딩되어 환경별 배포에 제약이 있습니다
- URL 쿼리 파라미터로 액세스 토큰을 전달하는 것은 보안상 위험합니다 (브라우저 히스토리, 서버 로그, Referer 헤더 등에 노출 가능)
환경 설정 기반 URL 구성과 더 안전한 토큰 전달 방식을 고려해주세요:
-// 3. 프론트엔드로 리다이렉트 (accessToken만 쿼리로 전달) -String targetUrl = "http://localhost:5173/login/success?accessToken=" + accessToken; -response.sendRedirect(targetUrl); +// 3. 액세스 토큰도 쿠키로 전달 (보안 강화) +Cookie accessTokenCookie = new Cookie("accessToken", accessToken); +accessTokenCookie.setHttpOnly(false); // JS에서 접근 가능해야 함 +accessTokenCookie.setSecure(true); +accessTokenCookie.setPath("/"); +accessTokenCookie.setMaxAge(30 * 60); // 30분 +response.addCookie(accessTokenCookie); + +// 4. 환경별 설정된 프론트엔드 URL로 리다이렉트 +String frontendUrl = getFrontendUrl(); // @Value로 주입받은 설정값 사용 +String targetUrl = frontendUrl + "/login/success"; +response.sendRedirect(targetUrl);application.yml에 환경별 설정 추가도 권장드립니다:
app: frontend: url: ${FRONTEND_URL:http://localhost:5173}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java(2 hunks)
🧰 Additional context used
🪛 ast-grep (0.38.1)
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java
[warning] 58-58: 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 'refreshTokenCookie.setSecure(true);'.
Context: refreshTokenCookie.setSecure(false);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-secure-flag-false-java)
🔇 Additional comments (3)
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java (3)
22-22: Cookie 임포트 추가 승인HttpOnly 쿠키 구현을 위한 필수 임포트가 적절히 추가되었습니다.
43-46: 코드 간소화가 잘 적용되었습니다중간 변수를 제거하고 직접적인 접근 방식으로 코드를 깔끔하게 정리하셨네요. 가독성이 향상되었습니다.
48-50: JWT 토큰 생성 로직 확인액세스 토큰과 리프레시 토큰의 만료 시간 설정이 적절합니다. 액세스 토큰 30분, 리프레시 토큰 30일은 보안과 사용성의 균형을 잘 맞춘 설정입니다.
| // 2. refreshToken을 HttpOnly 쿠키로 설정 | ||
| Cookie refreshTokenCookie = new Cookie("refreshToken", refreshToken); | ||
| refreshTokenCookie.setHttpOnly(true); // JS 접근 불가 | ||
| refreshTokenCookie.setSecure(false); // 운영환경 https라면 true로 변경 필요 | ||
| refreshTokenCookie.setPath("/"); | ||
| refreshTokenCookie.setMaxAge(30 * 24 * 60 * 60); // 30일 | ||
| // CSRF 방지를 위한 SameSite 속성 추가 (Servlet 3.1+ 지원시) | ||
| response.setHeader("Set-Cookie", | ||
| String.format("%s=%s; HttpOnly; Secure; Path=/; Max-Age=%d; SameSite=Lax", | ||
| "refreshToken", refreshToken, 30 * 24 * 60 * 60)); | ||
| response.addCookie(refreshTokenCookie); |
There was a problem hiding this comment.
쿠키 보안 설정에 일관성 문제가 있습니다
쿠키 설정에서 보안 관련 설정이 서로 상충되고 있습니다. 59번 줄에서는 setSecure(false)로 설정하고 있지만, 64번 줄의 Set-Cookie 헤더에서는 Secure 플래그를 true로 설정하고 있어 혼란을 야기할 수 있습니다.
다음과 같이 일관성 있게 수정해주세요:
Cookie refreshTokenCookie = new Cookie("refreshToken", refreshToken);
refreshTokenCookie.setHttpOnly(true);
-refreshTokenCookie.setSecure(false); // 운영환경 https라면 true로 변경 필요
+refreshTokenCookie.setSecure(true); // HTTPS 환경에서 보안 강화
refreshTokenCookie.setPath("/");
refreshTokenCookie.setMaxAge(30 * 24 * 60 * 60);
-// CSRF 방지를 위한 SameSite 속성 추가 (Servlet 3.1+ 지원시)
-response.setHeader("Set-Cookie",
- String.format("%s=%s; HttpOnly; Secure; Path=/; Max-Age=%d; SameSite=Lax",
- "refreshToken", refreshToken, 30 * 24 * 60 * 60));
response.addCookie(refreshTokenCookie);
+
+// SameSite 속성을 위한 추가 헤더 설정
+response.addHeader("Set-Cookie",
+ response.getHeader("Set-Cookie") + "; SameSite=Lax");📝 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.
| // 2. refreshToken을 HttpOnly 쿠키로 설정 | |
| Cookie refreshTokenCookie = new Cookie("refreshToken", refreshToken); | |
| refreshTokenCookie.setHttpOnly(true); // JS 접근 불가 | |
| refreshTokenCookie.setSecure(false); // 운영환경 https라면 true로 변경 필요 | |
| refreshTokenCookie.setPath("/"); | |
| refreshTokenCookie.setMaxAge(30 * 24 * 60 * 60); // 30일 | |
| // CSRF 방지를 위한 SameSite 속성 추가 (Servlet 3.1+ 지원시) | |
| response.setHeader("Set-Cookie", | |
| String.format("%s=%s; HttpOnly; Secure; Path=/; Max-Age=%d; SameSite=Lax", | |
| "refreshToken", refreshToken, 30 * 24 * 60 * 60)); | |
| response.addCookie(refreshTokenCookie); | |
| // 2. refreshToken을 HttpOnly 쿠키로 설정 | |
| Cookie refreshTokenCookie = new Cookie("refreshToken", refreshToken); | |
| refreshTokenCookie.setHttpOnly(true); // JS 접근 불가 | |
| refreshTokenCookie.setSecure(true); // HTTPS 환경에서 보안 강화 | |
| refreshTokenCookie.setPath("/"); | |
| refreshTokenCookie.setMaxAge(30 * 24 * 60 * 60); // 30일 | |
| response.addCookie(refreshTokenCookie); | |
| // SameSite 속성을 위한 추가 헤더 설정 | |
| response.addHeader("Set-Cookie", | |
| response.getHeader("Set-Cookie") + "; SameSite=Lax"); |
🧰 Tools
🪛 ast-grep (0.38.1)
[warning] 58-58: 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 'refreshTokenCookie.setSecure(true);'.
Context: refreshTokenCookie.setSecure(false);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-secure-flag-false-java)
🤖 Prompt for AI Agents
In
src/main/java/com/example/gtable/global/security/oauth2/OAuth2LoginSuccessHandler.java
around lines 56 to 66, the cookie security settings are inconsistent because
setSecure(false) is used on the Cookie object while the Set-Cookie header
includes the Secure flag. To fix this, ensure both the Cookie object and the
Set-Cookie header consistently set the Secure attribute, ideally setting
setSecure(true) if using HTTPS, and update the header accordingly to avoid
conflicting security configurations.
작업 요약
Issue Link
문제점 및 어려움
해결 방안
Reference
Summary by CodeRabbit