Main#65
Conversation
[hotfix] 구글 로그인 연동 문제 해결
develop to main
swagger patter matcher 추가 api filter chain 에 /api 경로 추가로 basic auth가 정상 작동하도록 수정
hotfix: swagger 403 에러 수정
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! 이 PR은 Google OAuth 로그인 처리 로직을 개선하고 Spring Security 설정을 리팩토링하여 애플리케이션의 보안 구성을 더욱 명확하고 효율적으로 만들었습니다. 특히, OAuth 리다이렉트 URL 처리의 견고성을 높이고, Swagger와 일반 API 엔드포인트에 대한 보안 정책을 분리하여 관리의 용이성을 확보했습니다. Highlights
Changelog
Ignored Files
Activity
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 pull request aims to improve redirect URL handling in the OAuth service and clarify security policies for Swagger and API endpoints by separating Spring Security configuration into two SecurityFilterChains. While the PR introduces a change to decode the state parameter for redirect URL validation, a security audit identified an Open Redirect vulnerability in the OAuth callback logic, indicating that the validation might still be insufficient. Additionally, the audit highlighted other critical issues, including the exposure of a potentially sensitive /env endpoint to the public and the disabling of CSRF protection while using cookie-based authentication. These vulnerabilities should be addressed to protect user data and maintain application integrity.
| .anyRequest().authenticated() | ||
| ); | ||
| .securityMatcher("/api/**") | ||
| .csrf(AbstractHttpConfigurer::disable) |
There was a problem hiding this comment.
CSRF protection is explicitly disabled for the API (/api/**). However, the JwtAuthenticationFilter is configured to resolve the authentication token from cookies. This combination makes the application vulnerable to Cross-Site Request Forgery (CSRF) attacks, as an attacker can trick a logged-in user's browser into making unauthorized requests to the API.
| http.authorizeHttpRequests(auth -> auth | ||
| .requestMatchers(HttpMethod.OPTIONS, "/**").permitAll() | ||
| .requestMatchers(HttpMethod.GET, GetPermitPatterns).permitAll() | ||
| .requestMatchers(PermitAllPatterns).permitAll() |
There was a problem hiding this comment.
The PermitAllPatterns array includes the /env path, which is then permitted for all users in the apiFilterChain. In Spring Boot applications, the /env endpoint (often part of Actuator) can expose sensitive environment variables, including database credentials, API keys, and other secrets. Exposing this endpoint publicly is a significant security risk.
|
|
||
| String decodedRedirectUrl = UriUtils.decode(state, StandardCharsets.UTF_8); | ||
|
|
||
| validateRedirectUrl(decodedRedirectUrl); |
There was a problem hiding this comment.
The loginWithGoogle method decodes the state parameter and uses it as a redirect URL. The validation performed by validateRedirectUrl uses startsWith, which can be bypassed (e.g., http://localhost:3000.evil.com starts with http://localhost:3000). This allows an attacker to redirect users to a malicious site after a successful login.
| } | ||
| else { | ||
| .authorizeHttpRequests(auth -> auth.anyRequest().authenticated()) | ||
| .httpBasic(withDefaults()); |
There was a problem hiding this comment.
#️⃣ 연관된 이슈
#️⃣ 작업 내용
#️⃣ 테스트 결과
#️⃣ 변경 사항 체크리스트
#️⃣ 스크린샷 (선택)
#️⃣ 리뷰 요구사항 (선택)
📎 참고 자료 (선택)