Add local HTTPS dev docs, improve auth, update redirects and error handling #146
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… signup redirects; update SignupTokenService to use Duration for token TTL; enhance CookieService with improved cookie handling and security attributes; modify SecurityConfig to allow HTTPS localhost; and update AuthConstants for consistent token duration management. These changes aim to improve user experience, security, and maintainability across the authentication system.
…agement and security attributes. This change ensures that cookies are correctly associated with the specified domain, aligning with best practices for cookie handling.
…thods to streamline cookie management. This change enhances code clarity while maintaining existing security attributes.
…TL, enhancing code clarity and maintainability. Remove the hardcoded TTL value from AuthConstants to streamline token duration management. Update CookieService to improve cookie creation logging for better debugging and monitoring.
… clarity in member handling. Update SignupTokenService to change token TTL from milliseconds to seconds for better consistency. Enhance CookieService to set domain for cookies, improving cookie management and security attributes.
…IGN_UP_TOKEN_NOT_FOUND for missing tokens. Introduce SIGN_UP_TOKEN_NOT_FOUND error code in ErrorCode for better clarity. Remove unnecessary logging in CookieService's cookie creation method to streamline code. Optimize terms agreement validation in MemberUtils for improved performance and clarity.
…arious files. Enhance OAuthLoginService error handling to throw specific OAUTH_AUTHENTICATION_FAILED exception. Modify SecurityConfig to allow a new local domain for CORS. Refactor CookieProperties to change configuration prefix and streamline cookie settings. Update RedirectProperties to support context-aware URLs based on request type. These changes aim to improve code clarity, error handling, and configuration management.
Walkthrough이 변경은 로컬 HTTPS 개발 환경 문서 추가, 인증 및 리다이렉션 로직 개선, 쿠키 속성 및 SameSite 정책 통합, 리다이렉션 URL의 로컬/원격 분기 도입, 에러 코드 및 예외 처리 업데이트, 그리고 불필요한 코드 및 상수 제거를 포함한다. Mr. T 스타일로 말하자면, 깔끔하게 리팩터링했구먼! Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend (Next.js)
participant Backend (Spring Boot)
participant SignupTokenService
participant HttpResponseService
User->>Frontend (Next.js): 소셜 로그인 시도
Frontend->>Backend: OAuth 인증 요청
Backend->>SignupTokenService: createAndSetToken(oAuthUser, nonce)
SignupTokenService-->>Backend: 토큰 생성 및 쿠키 설정
Backend->>HttpResponseService: redirectToSignup(oAuthUser.email)
HttpResponseService-->>User: /signup?email=... 리다이렉트
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ 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
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.gitignore (1)
44-47: 이 멍청이야, 불필요한 빈 줄들을 정리해라!.vscode/ 뒤에 추가된 빈 줄들이 너무 많다. 하나만 있으면 충분하다구!
### VS Code ### .vscode/ - - +docs/infra/local-https-setup.md (1)
1-149: I pity the fool이 HTTPS 로컬 개발 환경을 안 쓰는구먼!이 문서는 로컬 HTTPS 개발 환경 구성에 대한 매우 상세하고 유용한 가이드입니다. mkcert를 사용한 인증서 생성부터 Next.js와 Spring Boot 설정까지 완벽하게 다루고 있습니다!
몇 가지 마크다운 스타일 개선을 제안합니다:
- ```bash + ```text C:\Windows\System32\drivers\etc\hosts-
+text
127.0.0.1 local.juulabel.com-``` +```text juulabel-front/ ├── local.juulabel.com.pem ├── local.juulabel.com-key.pem ├── server.cjs-### ✅
mkcert설치 후 인증서 생성:
+### ✅mkcert설치 후 인증서 생성-#### 📌 내용 추가:
+#### 📌 내용 추가-### 📁 프로젝트 루트에 파일 배치:
+### 📁 프로젝트 루트에 파일 배치<details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 18-18: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 22-22: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 24-24: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 32-32: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 47-47: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 49-49: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 16aad4a30553d74655ed6b7365860b76c020fa22 and 57a928c2e3aa75baf4ac7f7f2c0a24e762587b53. </details> <details> <summary>📒 Files selected for processing (16)</summary> * `.gitignore` (2 hunks) * `docs/infra/local-https-setup.md` (1 hunks) * `src/main/java/com/juu/juulabel/auth/service/AuthService.java` (1 hunks) * `src/main/java/com/juu/juulabel/auth/service/OAuthLoginService.java` (1 hunks) * `src/main/java/com/juu/juulabel/auth/service/SignupTokenService.java` (4 hunks) * `src/main/java/com/juu/juulabel/common/auth/SignupTokenAuthenticationStrategy.java` (1 hunks) * `src/main/java/com/juu/juulabel/common/config/SecurityConfig.java` (1 hunks) * `src/main/java/com/juu/juulabel/common/constants/AuthConstants.java` (1 hunks) * `src/main/java/com/juu/juulabel/common/exception/code/ErrorCode.java` (1 hunks) * `src/main/java/com/juu/juulabel/common/http/CookieService.java` (5 hunks) * `src/main/java/com/juu/juulabel/common/http/HttpResponseService.java` (1 hunks) * `src/main/java/com/juu/juulabel/common/http/RequestDataExtractor.java` (9 hunks) * `src/main/java/com/juu/juulabel/common/properties/CookieProperties.java` (1 hunks) * `src/main/java/com/juu/juulabel/common/properties/RedirectProperties.java` (1 hunks) * `src/main/java/com/juu/juulabel/member/util/MemberUtils.java` (2 hunks) * `src/main/java/com/juu/juulabel/redis/UserSessionManager.java` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code Graph Analysis (1)</summary> <details> <summary>src/main/java/com/juu/juulabel/common/properties/RedirectProperties.java (1)</summary><blockquote> <details> <summary>src/main/java/com/juu/juulabel/common/properties/CookieProperties.java (1)</summary> * `Data` (12-67) </details> </blockquote></details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/infra/local-https-setup.md</summary> 18-18: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 22-22: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 24-24: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 32-32: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 47-47: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 49-49: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 ast-grep (0.38.1)</summary> <details> <summary>src/main/java/com/juu/juulabel/common/http/CookieService.java</summary> [warning] 56-56: 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) --- [warning] 75-75: 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(removeCookie); Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES] - https://owasp.org/www-community/HttpOnly (cookie-missing-httponly-java) --- [warning] 56-56: 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] 75-75: 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(removeCookie); Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES] - https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application (cookie-missing-samesite-java) --- [warning] 56-56: 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] 75-75: 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(removeCookie); Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES] - https://owasp.org/www-community/controls/SecureCookieAttribute (cookie-missing-secure-flag-java) </details> </details> </details> <details> <summary>🔇 Additional comments (22)</summary><blockquote> <details> <summary>.gitignore (1)</summary> `9-9`: **이 바보 같은 놈아, HTTPS 설정을 위한 키스토어 파일을 무시하는 건 좋은 판단이다!** *.p12 패턴 추가는 로컬 HTTPS 설정에서 생성되는 PKCS12 키스토어 파일들을 적절히 무시한다, 구먼! </details> <details> <summary>src/main/java/com/juu/juulabel/common/config/SecurityConfig.java (1)</summary> `60-60`: **이 멍청한 놈아, 로컬 HTTPS 개발 환경을 지원하는 건 똑똑한 판단이다!** CORS 허용 원본에 "https://local.juulabel.com:3000" 추가는 로컬 개발에서 HTTPS를 테스트할 수 있게 해준다. 프로덕션과 유사한 환경에서 테스트하는 게 현명하다구! </details> <details> <summary>src/main/java/com/juu/juulabel/redis/UserSessionManager.java (1)</summary> `64-64`: **이 바보야, 세션 생성 후 signup 토큰을 정리하는 건 현명한 생각이다!** 인증 토큰을 설정한 후 signup 토큰 쿠키를 명시적으로 제거하는 건 토큰 생명주기 관리를 개선한다. 사용자가 signup을 완료하면 더 이상 필요 없는 토큰을 깔끔하게 정리하는구먼! </details> <details> <summary>src/main/java/com/juu/juulabel/common/auth/SignupTokenAuthenticationStrategy.java (1)</summary> `42-42`: **이 멍청이야, 더 정확한 에러 코드를 사용하는 건 똑똑한 개선이다!** SIGN_UP_SESSION_EXPIRED에서 SIGN_UP_TOKEN_NOT_FOUND로 변경하는 건 토큰이 단순히 없을 때 더 정확한 에러를 보여준다. 디버깅할 때 훨씬 도움이 되는구먼! </details> <details> <summary>src/main/java/com/juu/juulabel/auth/service/OAuthLoginService.java (1)</summary> `46-47`: **예외 처리 개선이 훌륭하구먼!** 이 변경사항이 마음에 들어! 더 구체적인 에러 코드 `OAUTH_AUTHENTICATION_FAILED`와 적절한 에러 로깅을 추가했군. 이전의 일반적인 `INVALID_AUTHENTICATION`보다 훨씬 나은 접근이야. 디버깅할 때 이런 구체적인 정보가 있으면 문제를 빠르게 찾을 수 있지! </details> <details> <summary>src/main/java/com/juu/juulabel/common/constants/AuthConstants.java (1)</summary> `13-13`: **매직 넘버에 주석 추가한 거 좋아!** `USER_SESSION_TTL` 상수에 "// 7 days" 주석을 추가한 것이 현명한 판단이야! 매직 넘버를 보고 무엇을 의미하는지 바로 알 수 있어서 코드 가독성이 훨씬 좋아졌어. 이런 작은 개선이 코드 유지보수를 쉽게 만들지! </details> <details> <summary>src/main/java/com/juu/juulabel/common/http/RequestDataExtractor.java (1)</summary> `26-26`: **Javadoc 포맷팅 개선 잘했어!** 메서드 설명 뒤에 빈 줄을 추가해서 Javadoc 가독성을 높인 거 훌륭해! 이런 일관된 포맷팅이 코드 품질을 올려주지. 작은 디테일이지만 중요한 개선이야! Also applies to: 31-31, 44-44, 53-53, 62-62, 68-68, 71-71, 78-78, 91-91, 94-94, 108-108, 120-120, 132-132, 142-142, 152-152, 159-159 </details> <details> <summary>src/main/java/com/juu/juulabel/auth/service/AuthService.java (1)</summary> `102-103`: **이 변경사항들이 훌륭하구먼! I pity the fool who doesn't appreciate clean API design!** `createAndSetToken` 메서드로 통합하고 리다이렉트에 이메일 파라미터를 추가한 것은 매우 좋은 개선이야! 두 메서드 모두에서 일관성 있게 적용된 것도 훌륭하고, 이렇게 하면 코드가 더 깔끔해지고 사용자 경험도 개선되는구먼! Also applies to: 110-111 </details> <details> <summary>src/main/java/com/juu/juulabel/auth/service/SignupTokenService.java (2)</summary> `3-3`: **Duration import 추가가 현명한 선택이구먼!** 타입 안전성을 위해 `Duration`을 사용하는 것은 fool-proof한 방법이야! --- `40-40`: **TTL 상수 도입이 정말 멋지구먼! I pity the fool who uses magic numbers!** `SIGN_UP_TOKEN_TTL` 상수를 도입해서 매직 넘버를 제거하고, 생성자와 쿠키 만료 시간에서 일관되게 사용한 것이 훌륭해! 15분은 회원가입 토큰으로 적절한 시간이고, 코드 유지보수성도 크게 향상되었구먼! Also applies to: 52-52, 73-73 </details> <details> <summary>src/main/java/com/juu/juulabel/member/util/MemberUtils.java (1)</summary> `146-165`: **이 성능 최적화가 정말 멋지구먼! I pity the fool who doesn't use anyMatch!** `anyMatch`를 사용해서 조기 종료를 구현한 것이 훌륭해! 리스트를 수집하는 대신 첫 번째 조건 불일치에서 바로 멈추니까 성능이 크게 향상되었구먼! 코드 가독성도 더 좋아졌고, 이런 최적화가 진정한 프로의 실력이야! </details> <details> <summary>src/main/java/com/juu/juulabel/common/exception/code/ErrorCode.java (1)</summary> `49-49`: **새로운 에러 코드들이 정말 멋지구먼! I pity the fool who uses generic error messages!** `OAUTH_AUTHENTICATION_FAILED`와 `SIGN_UP_TOKEN_NOT_FOUND` 에러 코드 추가가 훌륭해! 구체적이고 명확한 에러 메시지로 디버깅이 쉬워지고, 사용자 경험도 개선될 거야! HTTP 상태 코드도 적절하게 `BAD_REQUEST`로 설정되어 있구먼! Also applies to: 52-52 </details> <details> <summary>src/main/java/com/juu/juulabel/common/properties/RedirectProperties.java (4)</summary> `7-7`: **I pity the fool이 제대로 된 의존성 주입을 안 하는구먼!** @RequiredArgsConstructor와 final 필드를 사용한 생성자 주입이 깔끔하게 구현되었습니다. 불변성과 명시적 의존성 선언이 좋습니다! Also applies to: 10-10, 14-14, 18-18 --- `20-23`: **환경별 URL 분리가 멋지구먼!** 로컬과 원격 환경을 위한 별도 URL 필드 추가가 개발 환경 유연성을 크게 향상시킵니다. 이제 Mr. T도 로컬에서 편하게 개발할 수 있겠구먼! --- `45-58`: **Context-aware URL 처리가 멋진 구현이구먼!** RequestDataExtractor.isLocalRequest()를 활용한 동적 URL 선택 로직이 매우 깔끔합니다. 개발과 운영 환경 간의 완벽한 분리를 제공합니다! --- `30-30`: **기존 메서드들이 새로운 로직을 완벽하게 활용하고 있구먼!** 모든 URL 생성 메서드가 새로운 context-aware 메서드를 사용하도록 일관성 있게 업데이트되었습니다. 깔끔한 리팩터링입니다! Also applies to: 34-34, 38-38, 42-42 </details> <details> <summary>src/main/java/com/juu/juulabel/common/properties/CookieProperties.java (3)</summary> `14-14`: **Spring Boot 표준 prefix 사용이 멋지구먼!** `server.servlet.session.cookie` prefix로 변경하여 Spring Boot의 표준 구성과 일치시킨 것이 훌륭합니다. 이제 기본 세션 쿠키 설정과 통합되어 더욱 일관성 있는 구성이 가능합니다! --- `21-21`: **필드 초기화 제거가 깔끔하구먼!** 하드코딩된 기본값 제거로 인해 application.yml에서의 완전한 제어가 가능해졌습니다. 설정 파일 중심의 구성이 Mr. T가 좋아하는 방식입니다! Also applies to: 27-27, 33-33, 40-40, 46-46 --- `52-66`: **명시적 getter 메서드 추가가 좋은 판단이구먼!** @Data와 함께 명시적 getter를 제공하여 API 명확성을 높였습니다. sameSite 필드 통합으로 설정이 단순화된 것도 훌륭합니다! </details> <details> <summary>src/main/java/com/juu/juulabel/common/http/CookieService.java (3)</summary> `56-56`: **I pity the fool이 통합된 쿠키 생성을 안 쓰는구먼!** createCookie 메서드로 통합하여 모든 쿠키에 일관된 보안 속성을 설정하는 것이 훌륭합니다. HttpOnly, Secure, SameSite 모든 속성이 CookieProperties를 통해 올바르게 설정되고 있습니다! 정적 분석 도구의 경고는 false positive입니다. 실제 코드를 보면: - line 128: `cookie.setHttpOnly(cookieProperties.isHttpOnly())` - line 129: `cookie.setSecure(isSecure)` - line 131: `cookie.setAttribute("SameSite", cookieProperties.getSameSite())` 모든 보안 속성이 올바르게 설정되고 있습니다! Also applies to: 122-134 <details> <summary>🧰 Tools</summary> <details> <summary>🪛 ast-grep (0.38.1)</summary> [warning] 56-56: 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) --- [warning] 56-56: 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] 56-56: 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) </details> </details> --- `75-75`: **쿠키 제거 로직도 깔끔하게 단순화되었구먼!** createRemovalCookie 메서드가 CookieProperties의 설정을 일관되게 사용하여 제거 쿠키를 생성합니다. 보안 속성도 동일하게 적용되어 완벽합니다! Also applies to: 139-151 <details> <summary>🧰 Tools</summary> <details> <summary>🪛 ast-grep (0.38.1)</summary> [warning] 75-75: 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(removeCookie); Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES] - https://owasp.org/www-community/HttpOnly (cookie-missing-httponly-java) --- [warning] 75-75: 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(removeCookie); Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES] - https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application (cookie-missing-samesite-java) --- [warning] 75-75: 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(removeCookie); Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES] - https://owasp.org/www-community/controls/SecureCookieAttribute (cookie-missing-secure-flag-java) </details> </details> --- `53-62`: **에러 핸들링과 로깅이 Mr. T 스타일로 완벽하구먼!** HttpContext가 없는 경우의 적절한 경고 로그와 함께 안전한 처리가 구현되었습니다. 디버깅을 위한 상세한 로그도 훌륭합니다! Also applies to: 70-81 <details> <summary>🧰 Tools</summary> <details> <summary>🪛 ast-grep (0.38.1)</summary> [warning] 56-56: 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) --- [warning] 56-56: 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] 56-56: 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) </details> </details> </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
42
to
43
| public void redirectToSignup(String email) { | ||
| redirect(redirectProperties.getSignupUrl() + "?email=" + email); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
이메일 파라미터 추가는 좋은데, URL 인코딩이 필요해!
회원가입 리다이렉션에 이메일을 전달하는 기능 개선은 훌륭해! 하지만 사용자 입력을 URL에 직접 연결하는 건 위험할 수 있어. 이메일에 특수문자가 있으면 URL이 깨질 수 있고, 보안 문제도 생길 수 있지!
다음과 같이 URL 인코딩을 추가하는 게 좋겠어:
- public void redirectToSignup(String email) {
- redirect(redirectProperties.getSignupUrl() + "?email=" + email);
+ public void redirectToSignup(String email) {
+ try {
+ String encodedEmail = URLEncoder.encode(email, StandardCharsets.UTF_8);
+ redirect(redirectProperties.getSignupUrl() + "?email=" + encodedEmail);
+ } catch (Exception e) {
+ log.error("Failed to encode email for signup redirect", e);
+ redirect(redirectProperties.getSignupUrl());
+ }그리고 import도 추가해야 해:
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;🤖 Prompt for AI Agents
In src/main/java/com/juu/juulabel/common/http/HttpResponseService.java around
lines 42-43, the email parameter is appended directly to the URL without
encoding, which can break the URL or cause security issues if the email contains
special characters. Fix this by applying URL encoding to the email parameter
using URLEncoder.encode(email, StandardCharsets.UTF_8.toString()) before
appending it to the URL. Also, add the necessary imports: java.net.URLEncoder
and java.nio.charset.StandardCharsets.
Comment on lines
+86
to
+90
| public boolean isLocalRequest() { | ||
| HttpServletRequest request = httpContextService.getCurrentRequest(); | ||
| String serverName = request.getServerName(); | ||
| return serverName.contains("local"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
로컬 요청 감지 로직이 좀 너무 단순해!
환경별 URL 선택을 위한 isLocalRequest() 메서드 추가는 좋은 아이디어야! 하지만 단순히 서버 이름에 "local"이 포함되어 있는지만 확인하는 건 너무 광범위할 수 있어. 예를 들어 "localhost.production.com" 같은 도메인도 true를 반환할 수 있지!
더 정확한 체크를 위해 다음과 같이 개선하는 게 어때:
public boolean isLocalRequest() {
HttpServletRequest request = httpContextService.getCurrentRequest();
String serverName = request.getServerName();
- return serverName.contains("local");
+ return serverName.equals("localhost") ||
+ serverName.equals("127.0.0.1") ||
+ serverName.startsWith("local.") ||
+ serverName.endsWith(".local");
}📝 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.
Suggested change
| public boolean isLocalRequest() { | |
| HttpServletRequest request = httpContextService.getCurrentRequest(); | |
| String serverName = request.getServerName(); | |
| return serverName.contains("local"); | |
| } | |
| public boolean isLocalRequest() { | |
| HttpServletRequest request = httpContextService.getCurrentRequest(); | |
| String serverName = request.getServerName(); | |
| return serverName.equals("localhost") || | |
| serverName.equals("127.0.0.1") || | |
| serverName.startsWith("local.") || | |
| serverName.endsWith(".local"); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/juu/juulabel/common/http/RequestDataExtractor.java around
lines 86 to 90, the isLocalRequest() method currently returns true if the server
name contains "local", which is too broad and can misclassify domains like
"localhost.production.com". To fix this, refine the logic to check if the server
name exactly matches "localhost" or "127.0.0.1", or matches a stricter pattern
for local environments, ensuring more precise detection of local requests.
…factor signup URL generation to include email parameter; update MemberUtils to throw new error code for duplicate alcohol types.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
문서화
신규 기능
버그 수정
기능 개선
환경 설정