[BE] SISC1-209 [FIX] 소셜 로그인 로직 수정#103
Conversation
WalkthroughOAuth2 인증 흐름을 Spring Security의 내장 OAuth2 지원으로 재구성합니다. AuthController에서 수동 OAuth 엔드포인트를 제거하고, CustomOAuth2UserService, OAuth2SuccessHandler 및 업데이트된 SecurityConfig를 통해 Google, Kakao, GitHub 통합을 구현합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend
participant SecurityConfig
participant CustomOAuth2UserService
participant UserRepository
participant JwtProvider
participant OAuth2SuccessHandler
User->>Frontend: OAuth 로그인 클릭
Frontend->>SecurityConfig: GET /oauth2/authorization/{provider}
SecurityConfig->>User: OAuth 제공자 로그인 페이지로 리다이렉트
User->>SecurityConfig: 인가 코드 반환
SecurityConfig->>CustomOAuth2UserService: loadUser(OAuth2UserRequest)
CustomOAuth2UserService->>UserRepository: 기존 사용자 조회 또는 생성
UserRepository-->>CustomOAuth2UserService: User 반환
CustomOAuth2UserService-->>SecurityConfig: DefaultOAuth2User 반환
SecurityConfig->>OAuth2SuccessHandler: onAuthenticationSuccess()
OAuth2SuccessHandler->>JwtProvider: 액세스/리프레시 토큰 발급
OAuth2SuccessHandler->>Frontend: 토큰 포함 리다이렉트
Frontend-->>User: 로그인 완료
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/java/org/sejongisc/backend/auth/controller/AuthController.java (1)
1-337: Spring Security OAuth2 구성이 불완전합니다. 세 가지 중요한 문제를 해결해야 합니다.
application.yml에 OAuth2 제공자 설정 누락 (CRITICAL)
spring.security.oauth2.client.registration구성이 없어서 OAuth2 로그인이 작동하지 않습니다- 다음 구성을 application.yml에 추가해야 합니다:
spring: security: oauth2: client: registration: google: client-id: ${GOOGLE_CLIENT_ID} client-secret: ${GOOGLE_CLIENT_SECRET} scope: profile,email kakao: client-id: ${KAKAO_CLIENT_ID} client-secret: ${KAKAO_CLIENT_SECRET} client-authentication-method: post authorization-grant-type: authorization_code redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}" scope: profile_nickname,profile_image,account_email github: client-id: ${GITHUB_CLIENT_ID} client-secret: ${GITHUB_CLIENT_SECRET} scope: user:email provider: kakao: authorization-uri: https://kauth.kakao.com/oauth/authorize token-uri: https://kauth.kakao.com/oauth/token user-info-uri: https://kapi.kakao.com/v2/user/me user-name-attribute: idAuthController의 미사용 import 제거 (라인 23-25)
GithubUserInfoAdapter,GoogleUserInfoAdapter,KakaoUserInfoAdapter는 이제 사용되지 않으므로 제거하세요- 이들은 이전의 수동 OAuth 구현 관련 코드입니다
OAuth2SuccessHandler의 하드코딩된 리다이렉트 URL 외부화
- 현재
http://localhost:5173/oauth/success로 하드코딩되어 있습니다- 이를
application.yml의oauth.redirect-url프로퍼티로 외부화하고@Value주입을 통해 사용하세요 (개발/프로덕션 환경 분리)
🧹 Nitpick comments (8)
backend/src/main/java/org/sejongisc/backend/user/service/UserService.java (1)
30-32: OAuth upsert 메서드 파라미터 네이밍 정합성 확인 제안
UserService인터페이스는providerId를 쓰고, 구현체UserServiceImpl.upsertOAuthUser는providerUid라는 네이밍을 사용하고 있습니다. 타입 시그니처 관점에서는 문제 없지만, 유지보수 시 혼동될 수 있어 통일해 두는 편이 좋겠습니다.또, 현재 PR 범위 내에서 이 메서드가 실제로 어디서 호출되는지 보이지 않는데, 당장 쓰이지 않는다면 주석으로 용도를 명시하거나, 추후 사용 시점에 맞춰 추가해도 될지 한번 같이 검토해 보면 좋겠습니다.
backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java (2)
55-66: OAuth2 로그인 실패 redirect URL의 환경 분리 제안
failureHandler에서"http://localhost:5173/oauth/fail"을 하드코딩하고 있어 로컬 외 환경(스테이징/운영)으로 확장 시 매번 코드 변경이 필요해 보입니다. 성공 redirect URL도OAuth2SuccessHandler쪽에 별도로 하드코딩되어 있어, 공통 설정(예:application.yml프로퍼티)로 빼두면 환경 전환이 훨씬 수월할 것 같습니다.
107-109: CORS에서setAllowedOrigins와setAllowedOriginPatterns("*")동시 사용 재검토 제안
config.setAllowedOrigins(List.of("http://localhost:5173"))와setAllowedOriginPatterns(List.of("*"))를 함께 설정해서, 사실상 모든 Origin을 허용하는 설정이 될 수 있습니다. 특히setAllowCredentials(true)까지 켜져 있어, 운영 환경 기준으로는 필요한 Origin만 명시하는 방향이 더 안전할 것 같습니다. 로컬 개발용과 운영용 설정을 profile로 분리하는 것도 고려해 볼 만합니다.backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java (1)
52-68: User 재조회 방식은 동작상 문제 없으나, 필요 시 최적화 여지 있음
DefaultOAuth2User의 attributes에서userId를 꺼낸 뒤 다시UserRepository.findById로 조회하는 방식은 동작상 큰 문제는 없습니다. 다만 role 정보까지 attributes에 함께 넣는 형태로CustomOAuth2UserService를 확장하면, 여기서 DB 재조회 없이 토큰 생성을 할 수도 있어 약간의 최적화 여지는 있어 보입니다. (필수는 아니고, 추후 성능/복잡도 요구에 따라 고려할 수 있는 수준입니다.)backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java (1)
52-77: provider별 매핑/유저 생성 로직은 동작상 타당하나, edge 케이스와 중복 코드 고려 필요몇 가지 포인트가 있습니다:
Kakao 속성 NPE 가능성
account = (Map) attrs.get("kakao_account"),profile = (Map) account.get("profile")에서 해당 키가 없으면 NPE가 발생할 수 있습니다. 카카오 쪽 스펙 변경이나 동의항목 조합에 따라 필드가 빠질 수 있어,null체크 후 적절한 fallback을 두는 편이 안전합니다.placeholder 이메일 중복 가능성
Kakao/GitHub에서 이메일이 없을 때"no-email@kakao.com","no-email@github.com"을 고정 문자열로 저장하고 있는데,User.email에 unique 제약이 걸려 있다면 두 번째 사용자가 가입되는 시점에 DB 제약 에러가 날 수 있습니다.
- 예: providerUid 기반 가짜 이메일(
providerUid + "@kakao.local") 사용- 혹은 이메일 nullable + 별도 식별자 기반 UX 설계
등의 방식을 한 번 고려해 보는 게 좋겠습니다.UserServiceImpl.upsertOAuthUser와 로직 중복
이 클래스의 upsert 로직과UserServiceImpl.upsertOAuthUser가 상당히 유사합니다. 순환참조 때문에UserService를 직접 호출하지 않도록 설계한 것은 이해되지만, 장기적으로는
- 공통 도메인 서비스/헬퍼로 분리하거나
- repository 레벨의 공통 팩토리 메서드 등으로 추출
해서 중복을 줄이면 유지보수성이 좋아질 것 같습니다.동작 자체는 문제 없어 보이지만, 위 세 부분은 추후 운영/확장 시 리스크가 될 수 있어 한 번쯤 같이 정리해 보면 좋겠습니다.
Also applies to: 86-108
backend/src/main/java/org/sejongisc/backend/user/service/UserServiceImpl.java (2)
4-16: OAuth 관련 import 추가는 현재 구현과 약간 불일치이 파일 상단에
SimpleGrantedAuthority,DefaultOAuth2UserService,OAuth2UserRequest,OAuth2AuthenticationException,DefaultOAuth2User,OAuth2User등이 import 되어 있지만, 본문에서는 실제로 사용되는 것은AuthProvider뿐입니다.
빌드/포매터에서 자동 정리가 된다면 그대로 두어도 무방하지만, 사용하지 않는 import는 IDE 설정에 따라 warning이 될 수 있어 한 번 정리해 두시면 좋겠습니다.Also applies to: 36-37
328-356: upsertOAuthUser의 provider 문자열 검증 및 공통 로직화 제안
upsertOAuthUser구현은 전체적으로CustomOAuth2UserService와 동일한 패턴으로 잘 작성되어 있습니다만, 두 가지 정도만 같이 보면 좋겠습니다.
provider 문자열 검증
AuthProvider.valueOf(provider.toUpperCase())에서provider값이 enum과 일치하지 않으면IllegalArgumentException으로 바로 500이 날 수 있습니다.
- 이 메서드가 외부 입력에 직접 노출되는 경로(컨트롤러 등)에서 호출된다면,
try/catch로 감싸CustomException(ErrorCode.INVALID_INPUT)등으로 매핑해 주는 것을 고려해 주세요.- 내부에서만 안전한 값으로 호출된다면, 메서드-level Javadoc이나 주석으로 “provider는 GOOGLE/KAKAO/GITHUB 등 enum 이름 기반”이라는 전제를 명시해 두는 것도 도움이 됩니다.
CustomOAuth2UserService와의 로직 중복
OAuth 계정 조회 후 없으면User+UserOauthAccount를 생성하는 패턴이CustomOAuth2UserService와 거의 동일합니다. 추후 유지보수를 생각하면, 공통 헬퍼/도메인 서비스로 추출해서 두 곳에서 재사용하도록 리팩터링하는 방안도 검토해 보시면 좋겠습니다.기능적으로는 문제 없어 보이며, 위 부분은 안정성을 조금 더 높이고 중복을 줄이기 위한 제안입니다.
backend/src/main/java/org/sejongisc/backend/auth/controller/AuthController.java (1)
23-25: 미사용 OAuth 어댑터 임포트를 제거하세요.OAuth 로직이 Spring Security로 이관되면서 이 어댑터들은 더 이상 AuthController에서 사용되지 않습니다. 코드 정리를 위해 제거하는 것이 좋습니다.
다음 diff를 적용하여 미사용 임포트를 제거하세요:
-import org.sejongisc.backend.auth.oauth.GithubUserInfoAdapter; -import org.sejongisc.backend.auth.oauth.GoogleUserInfoAdapter; -import org.sejongisc.backend.auth.oauth.KakaoUserInfoAdapter;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
backend/src/main/java/org/sejongisc/backend/auth/controller/AuthController.java(1 hunks)backend/src/main/java/org/sejongisc/backend/auth/dto/SignupRequest.java(1 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java(1 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java(1 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java(5 hunks)backend/src/main/java/org/sejongisc/backend/user/dto/PasswordResetCommitRequest.java(1 hunks)backend/src/main/java/org/sejongisc/backend/user/dto/PasswordResetSendRequest.java(1 hunks)backend/src/main/java/org/sejongisc/backend/user/dto/PasswordResetVerifyRequest.java(1 hunks)backend/src/main/java/org/sejongisc/backend/user/dto/UserIdFindRequest.java(1 hunks)backend/src/main/java/org/sejongisc/backend/user/dto/UserInfoResponse.java(1 hunks)backend/src/main/java/org/sejongisc/backend/user/dto/UserUpdateRequest.java(1 hunks)backend/src/main/java/org/sejongisc/backend/user/service/UserService.java(1 hunks)backend/src/main/java/org/sejongisc/backend/user/service/UserServiceImpl.java(3 hunks)backend/src/main/resources/application.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java (2)
backend/src/main/java/org/sejongisc/backend/auth/controller/AuthController.java (1)
Slf4j(40-337)backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java (1)
Slf4j(25-120)
backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java (1)
backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/CustomUserDetailsService.java (1)
RequiredArgsConstructor(16-38)
🔇 Additional comments (12)
backend/src/main/resources/application.yml (1)
15-16: LGTM!파일 끝에 빈 줄을 추가하여 파일 포맷 컨벤션을 준수했습니다.
backend/src/main/java/org/sejongisc/backend/user/dto/UserUpdateRequest.java (1)
36-36: LGTM!비밀번호 예시값을 대문자를 포함하도록 업데이트하여 더 강력한 비밀번호 패턴을 제시하고 있습니다.
backend/src/main/java/org/sejongisc/backend/user/dto/PasswordResetCommitRequest.java (1)
3-3: LGTM!Swagger 문서화를 위한 @Schema 어노테이션이 추가되어 API 문서의 가독성이 향상되었습니다. 비밀번호 요구사항이 명확하게 설명되어 있으며, 예시값도 검증 패턴과 일치합니다.
Also applies to: 9-27
backend/src/main/java/org/sejongisc/backend/user/dto/PasswordResetVerifyRequest.java (1)
3-3: LGTM!Swagger 문서화를 위한 @Schema 어노테이션이 추가되어 API 문서의 가독성이 향상되었습니다.
Also applies to: 10-21
backend/src/main/java/org/sejongisc/backend/user/dto/PasswordResetSendRequest.java (1)
3-3: LGTM!Swagger 문서화를 위한 @Schema 어노테이션이 추가되어 API 문서의 가독성이 향상되었습니다.
Also applies to: 9-12
backend/src/main/java/org/sejongisc/backend/user/dto/UserIdFindRequest.java (1)
3-3: LGTM!Record를 멀티라인 형식으로 재구성하고 Swagger 문서화를 위한 @Schema 어노테이션을 추가했습니다. 전화번호 검증 패턴(
^010\\d{8}$)과 예시값("01098765432")이 일치하며, 명확한 설명이 제공되고 있습니다.Also applies to: 7-21
backend/src/main/java/org/sejongisc/backend/auth/dto/SignupRequest.java (2)
43-43: LGTM!비밀번호 예시값을 대문자를 포함하도록 업데이트하여 더 강력한 비밀번호 패턴을 제시하고 있습니다.
51-51: Role 예시값 변경 확인role 예시값이 "USER"에서 "TEAM_MEMBER"로 변경되었습니다. Role enum에 해당 값이 정의되어 있는지 확인이 필요합니다(UserInfoResponse.java의 리뷰 코멘트 참조).
backend/src/main/java/org/sejongisc/backend/user/dto/UserInfoResponse.java (1)
50-50: Role enum에 TEAM_MEMBER 값이 정의되어 있음을 확인했습니다검증 결과,
backend/src/main/java/org/sejongisc/backend/user/entity/Role.java의 Role enum에 TEAM_MEMBER 값이 명시적으로 정의되어 있습니다. 따라서UserInfoResponse.java에서 example 값을 "TEAM_MEMBER"로 변경한 것은 올바른 수정입니다.backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java (2)
17-19: OAuth2 AuthorizationRequestRepository 및 핸들러 주입 구조는 무난해 보입니다
HttpSessionOAuth2AuthorizationRequestRepository를 Bean으로 분리하고,CustomOAuth2UserService및OAuth2SuccessHandler를 생성자 주입하는 구조는 Spring OAuth2Login 전형적인 구성이라 특별한 이슈는 없어 보입니다.Also applies to: 37-43
94-96: 세션 정책(IF_REQUIRED) 변경이 JWT 기반 설계와 맞는지 확인 필요
SessionCreationPolicy.STATELESS에서IF_REQUIRED로 바꾸면서, OAuth2 인증 과정에서 HttpSession을 사용할 수 있게 된 반면, 일반 JWT 기반 요청에서도 세션이 생성될 수 있는 여지가 생겼습니다. 의도적으로 “필요 시에만 세션 허용”을 선택한 것인지, 기존에 완전한 stateless 동작을 가정한 부분이 없는지 한 번 점검해 보시면 좋겠습니다.backend/src/main/java/org/sejongisc/backend/auth/controller/AuthController.java (1)
119-119: 역할 값 변경이 적절합니다.OpenAPI 예제에서 role 값을 "USER"에서 "TEAM_MEMBER"로 변경한 것은 시스템의 역할 열거형 변경과 일치하며, 다른 엔드포인트 예제와도 일관성을 유지합니다.
backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java
Show resolved
Hide resolved
| // 7. 프론트로 redirect | ||
| String redirectUrl = "http://localhost:5173/oauth/success" | ||
| + "?accessToken=" + accessToken | ||
| + "&name=" + URLEncoder.encode(name, StandardCharsets.UTF_8) | ||
| + "&userId=" + userId; |
There was a problem hiding this comment.
accessToken을 URL 쿼리 파라미터로 전달하는 방식은 보안상 위험
redirectUrl에
?accessToken=...&name=...&userId=...
형태로 accessToken을 쿼리스트링으로 노출하고 있는데, 이 방식은 다음과 같은 보안 리스크가 큽니다:
- URL이 브라우저 히스토리, 서버/프록시 로그, 리퍼러 헤더 등에 그대로 남을 수 있음.
- 제3자에게 URL이 유출되면 accessToken 탈취로 직결될 수 있음.
가능하다면:
- accessToken도 refresh와 마찬가지로 HttpOnly 쿠키로 전달하고, 리다이렉트 URL에는 민감정보를 포함하지 않거나
- 리다이렉트 없이 JSON 응답으로 access/refresh 토큰을 내려 프론트에서 처리
하는 식으로 토큰을 URL 쿼리 파라미터에 두지 않는 방향으로 구조를 바꾸는 것을 강하게 추천드립니다.
🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java
around lines 80 to 84, the code is putting accessToken in the redirect URL query
string which exposes it to browser history, logs and referrers; instead, stop
including the access token in the redirect URL and deliver tokens via a secure
HttpOnly cookie or by returning a JSON response: set the access (and refresh)
token as cookies with HttpOnly, Secure, SameSite=strict/lax and appropriate
expiry (or send a JSON body containing tokens if you remove the redirect flow),
then build the redirect URL without any sensitive query params (only
non-sensitive state if needed); ensure no logging or referrer headers include
the token and remove any URL encoding of tokens so tokens are never present in
the query string.
2 participants
Summary by CodeRabbit
릴리스 노트
New Features
Documentation