Feat/#7 소셜 로그인(네이버, 카카오, 구글) 기능 구현#14
Conversation
# Conflicts: # src/main/java/com/whereyouad/WhereYouAd/domains/user/exception/code/UserErrorCode.java
WalkthroughOAuth2를 활용한 소셜 로그인(Google, Naver, Kakao) 기능을 구현합니다. 제공자별 응답 매핑, 인증 성공 처리기, 사용자 저장소 통합을 포함하며 JWT 기반 토큰과 쿠키로 세션을 관리합니다. Changes
Sequence DiagramsequenceDiagram
actor User as 사용자
participant FE as Frontend
participant OAuth2 as OAuth2 Provider<br/>(Google/Naver/Kakao)
participant Spring as Spring OAuth2 Flow
participant Service as CustomOAuth2UserService
participant DB as Database<br/>(User, AuthProviderAccount)
participant Handler as OAuth2SuccessHandler
participant JWT as JwtTokenProvider
User->>FE: 소셜 로그인 버튼 클릭
FE->>OAuth2: 인증 요청
OAuth2->>FE: 사용자 동의 화면
User->>OAuth2: 동의
OAuth2->>Spring: 인증 코드 반환
Spring->>Service: loadUser(userRequest)
Service->>OAuth2: 사용자 정보 요청
OAuth2-->>Service: 제공자별 응답 반환
Service->>Service: OAuth2Response로 매핑<br/>(GoogleResponse/NaverResponse/KaKaoResponse)
Service->>DB: providerId로 AuthProviderAccount 조회
alt 기존 계정 존재
Service->>DB: User 조회 및 프로필 업데이트
else 신규 계정
Service->>DB: 이메일로 기존 User 확인
alt 기존 User 없음
Service->>DB: 새로운 User 생성 (ROLE_USER)
end
Service->>DB: AuthProviderAccount 생성 및 저장
end
Service-->>Spring: CustomOAuth2User 반환
Spring->>Handler: onAuthenticationSuccess 호출
Handler->>JWT: 토큰 생성 (email 기반)
JWT-->>Handler: access_token, refresh_token
Handler->>DB: RefreshToken 저장/업데이트
Handler->>FE: 쿠키 설정<br/>(access_token, refresh_token)
Handler->>FE: 리다이렉트
FE-->>User: 로그인 완료
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
추가 검토 포인트 (시니어 리뷰어 의견)✅ 잘 설계된 부분
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | PR 제목은 소셜 로그인(네이버, 카카오, 구글) 기능 구현이라는 명확한 변경 사항을 요약하고 있으며, 전체 변경셋의 핵심을 잘 나타냅니다. |
| Description check | ✅ Passed | PR 설명은 템플릿의 필수 섹션(관련 이슈, 개요, 작업 내용, 테스트 결과, 체크리스트)을 모두 포함하고 있으며, 상세한 스크린샷으로 구현 결과를 명확히 입증합니다. |
| Linked Issues check | ✅ Passed | 변경사항이 #7 이슈의 모든 요구사항을 충족합니다: 세 플랫폼 설정 완료, auth_provider_account 엔티티 추가, OAuth2 서비스 및 핸들러 구현으로 JWT 발급까지 전체 파이프라인 구현. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 소셜 로그인 기능 구현 범위 내에 있으며, User 엔티티의 phoneNumber 제약 조건 완화는 해당 기능 지원을 위한 필수 변경입니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/exception/code/UserErrorCode.java`:
- Around line 15-21: The enum error code strings are inconsistent with the
project's naming convention; update the string values for NOT_PROVIDE_SOCIAL and
USER_NOT_FOUND in UserErrorCode so they include the missing underscore(s) to
match the existing pattern (e.g., change "USER400_4" to "USER_400_4" and
"USER404_1" to "USER_404_1"), ensuring USER_EMAIL_NOT_VERIFIED remains
"USER_401_1".
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/entity/AuthProviderAccount.java`:
- Around line 11-12: Add a DB-level unique constraint for the social account key
(provider + providerId) to prevent duplicate records: modify the
AuthProviderAccount entity (class AuthProviderAccount) to declare a unique
constraint on the provider and providerId columns via the
`@Table`(uniqueConstraints=...) annotation using the actual column names (e.g.,
"provider" and the column name mapped from the providerId field), and also add a
corresponding DB migration (ALTER TABLE ... ADD CONSTRAINT ... UNIQUE or a
unique index) to enforce it in the database.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/repository/AuthProviderAccountRepository.java`:
- Around line 6-8: In AuthProviderAccountRepository, change the method signature
from findByProviderId(String username) to Optional<AuthProviderAccount>
findByProviderId(String providerId) (rename parameter to providerId and return
Optional to avoid nulls); then update all callers (e.g., places checking
existAccount == null) to handle Optional properly
(isPresent()/orElseThrow()/ifPresent) and adjust AuthAccountConverter usage so
it stores/queries the raw providerId (e.g., "123") alongside the provider enum
rather than a combined "google_123" string to avoid duplicated data
responsibility.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/CustomOAuth2User.java`:
- Around line 16-19: The getAttributes() override in CustomOAuth2User currently
returns null which can cause NPEs; change CustomOAuth2User.getAttributes() to
return an immutable empty Map (e.g., Collections.emptyMap()) or a new empty Map
instance instead of null so callers never receive null; update the method
implementation in the CustomOAuth2User class to return a non-null empty map.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/KaKaoResponse.java`:
- Around line 30-38: getEmail()과 getName() 에서 kakaoAccount 또는 profile 이 null이거나
내부 값이 null일 때 NPE(또는 "null" 문자열 반환)가 발생합니다; 수정 방법: getEmail()과 getName()에서
kakaoAccount 및 profile 객체가 null인지 검사하고, 내부 Map.get("email") /
Map.get("nickname") 결과도 null인지 확인한 뒤 안전한 기본값(예: null 또는 빈 문자열)이나 Optional로 반환하도록
변경하세요; 관련 식별자: 메서드 getEmail(), getName(), 필드 kakaoAccount, profile을 찾아 해당 null
체크 로직을 추가하세요.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/NaverResponse.java`:
- Around line 26-34: NaverResponse's getEmail and getName currently call
String.valueOf(attribute.get(...)) which turns null into the literal "null";
change both getEmail() and getName() in class NaverResponse to mirror
KaKaoResponse's null-safety by checking attribute.get("email") and
attribute.get("name") for null and returning null (or an empty string if your
app expects that) instead of the string "null" — specifically, retrieve the
value from attribute, if it's non-null return its toString(), otherwise return
null; update getEmail() and getName() accordingly.
- Around line 11-14: The NaverResponse constructor sets this.attribute =
(Map<String, Object>) attribute.get("response") which can be null and later
cause NPEs in the class's getters; update the NaverResponse(Map<String,Object>
attribute) constructor to check attribute.get("response") for null and handle it
safely (e.g., assign Collections.emptyMap() or a new HashMap<>() as a default,
or throw a clear IllegalArgumentException) so subsequent getter methods on the
NaverResponse instance won't throw NPEs; ensure you reference the constructor in
class NaverResponse and the field attribute when making the change.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/OAuth2UserInfo.java`:
- Around line 9-16: The fields in OAuth2UserInfo (name, email, role, provider,
providerId) are declared public; change them to private to restore encapsulation
while keeping the existing `@Getter` usage on the class (or add `@Getter` above the
class if missing) so callers can read values but not mutate them directly; if
external mutation is required, add explicit setters or a constructor/factory
method on OAuth2UserInfo instead of making fields public.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/handler/OAuth2AuthenticationSuccessHandler.java`:
- Around line 53-60: The access token cookie is created in
OAuth2AuthenticationSuccessHandler as ResponseCookie accessTokenCookie with
httpOnly(false) and secure(false); change httpOnly to true (httpOnly(true)) to
prevent JavaScript access, and make the secure flag configurable rather than
hardcoded (replace secure(false) with secure(<env/config boolean>) sourced from
a property or Environment bean, e.g., an isProduction or cookie.secure config
used in the handler), keeping path/maxAge/sameSite as-is; update any tests or
local dev config to allow non-secure during development.
🧹 Nitpick comments (10)
.env.example (1)
31-31: 파일 끝에 빈 줄 추가를 권장합니다.POSIX 표준에 따르면 텍스트 파일은 newline으로 끝나야 합니다. 일부 도구에서 경고가 발생할 수 있어요.
💡 수정 제안
# 6. Frontend Redirect URL OAUTH2_REDIRECT_URL=http://localhost:3000/oauth2/redirect +src/main/resources/application.yml (1)
55-55: OAuth2 redirect-uri가 하드코딩되어 있습니다.현재
localhost:8080으로 고정되어 있어 개발 환경에서는 문제없지만, 스테이징/프로덕션 배포 시에는 환경별로 다른 도메인을 사용해야 합니다. 환경 변수로 분리하는 것을 권장합니다.💡 수정 제안
.env.example에 추가:OAUTH2_BASE_URL=http://localhost:8080
application.yml에서:naver: - redirect-uri: "http://localhost:8080/login/oauth2/code/naver" + redirect-uri: "${OAUTH2_BASE_URL}/login/oauth2/code/naver" google: - redirect-uri: "http://localhost:8080/login/oauth2/code/google" + redirect-uri: "${OAUTH2_BASE_URL}/login/oauth2/code/google" kakao: - redirect-uri: "http://localhost:8080/login/oauth2/code/kakao" + redirect-uri: "${OAUTH2_BASE_URL}/login/oauth2/code/kakao"Also applies to: 65-65, 75-75
src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/entity/User.java (1)
46-49:updateProfile메서드에 null 체크 추가를 권장합니다.현재 구현은 name이 null이어도 그대로 저장될 수 있어요.
@Column(nullable = false)로 name이 정의되어 있으므로, DB 제약 조건 위반 예외가 발생할 수 있습니다. 방어적 코딩으로 미리 검증하는 것이 좋습니다.또한 메서드명이
updateProfile인데 name만 업데이트하고 있어서,updateName으로 변경하거나 향후 다른 필드도 업데이트할 계획이라면 파라미터를 확장하는 것도 고려해보세요.💡 수정 제안
// 소셜 로그인 시 유저 프로필 최신화 - public void updateProfile(String name){ - this.name = name; + public void updateName(String name) { + if (name == null || name.isBlank()) { + return; // 또는 예외를 던지거나, 기존 값 유지 + } + this.name = name; }또는 여러 필드를 업데이트할 계획이라면:
public void updateProfile(String name, String profileImageUrl) { if (name != null && !name.isBlank()) { this.name = name; } if (profileImageUrl != null) { this.profileImageUrl = profileImageUrl; } }src/main/java/com/whereyouad/WhereYouAd/global/security/jwt/JwtTokenProvider.java (1)
43-57: Java 17 Pattern Matching을 활용하면 더 깔끔해집니다현재 코드는
instanceof체크 후 별도로 캐스팅하고 있는데, Java 17의 Pattern Matching for instanceof를 사용하면 코드가 더 간결해지고 타입 안전성도 높아집니다.♻️ Java 17 Pattern Matching 적용 제안
// 로그인 종류에 따라 이메일(식별자) 추출 String email; Object principal = authentication.getPrincipal(); - if (principal instanceof CustomOAuth2User) { + if (principal instanceof CustomOAuth2User oauth2User) { // 소셜 로그인: CustomOAuth2User에서 이메일 추출 - email = ((CustomOAuth2User) principal).getEmail(); - } else if (principal instanceof CustomUserDetails) { + email = oauth2User.getEmail(); + } else if (principal instanceof CustomUserDetails userDetails) { // 일반 로그인: UserDetails의 username(email) 추출 - email = ((CustomUserDetails) principal).getUsername(); + email = userDetails.getUsername(); } else { // 그 외의 경우 (기본값) email = authentication.getName(); }As per coding guidelines: "Java 17의 최신 문법(Record, Switch Expression, Text Block 등)을 적절히 사용하는지 확인하라."
src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/mapper/AuthAccountConverter.java (1)
7-16: 유틸리티 클래스에 private 생성자를 추가해주세요
AuthAccountConverter는 static 메서드만 가진 유틸리티 클래스입니다. 이런 경우 인스턴스화를 방지하기 위해 private 생성자를 추가하는 것이 좋습니다. 이렇게 하면 실수로new AuthAccountConverter()를 호출하는 것을 컴파일 타임에 방지할 수 있어요.♻️ private 생성자 추가 제안
public class AuthAccountConverter { + + private AuthAccountConverter() { + // 유틸리티 클래스 인스턴스화 방지 + } + // dto -> entity public static AuthProviderAccount toAuthProviderAccount(Provider provider, String providerId, User user){src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/mapper/UserConverter.java (1)
8-18: 필드 직접 접근과 getter 접근이 혼용되어 있어요.Line 13에서는
authUserResponseDTO.getEmail()로 getter를 사용하고, Line 14에서는authUserResponseDTO.name으로 필드를 직접 접근하고 있습니다. 이런 일관성 없는 접근 방식은 나중에OAuth2UserInfo클래스가 변경될 때 버그로 이어질 수 있어요.또한, 유틸리티 클래스는 인스턴스화를 방지하기 위해 private 생성자를 추가하는 것이 좋습니다.
♻️ 권장 수정 사항
public class UserConverter { + + private UserConverter() { + // 유틸리티 클래스 인스턴스화 방지 + } // dto -> entity public static User toSocialUser(OAuth2UserInfo authUserResponseDTO) { return User.builder() .email(authUserResponseDTO.getEmail()) - .name(authUserResponseDTO.name) + .name(authUserResponseDTO.getName()) .status(UserStatus.ACTIVE) .isEmailVerified(true) .build(); }src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/handler/OAuth2AuthenticationSuccessHandler.java (1)
35-38: 타입 캐스팅에 방어적 코드를 추가하면 더 안전해요.
authentication.getPrincipal()이CustomOAuth2User가 아닌 경우ClassCastException이 발생합니다. Spring Security OAuth2 플로우 내에서는 안전하지만, 디버깅을 위해 타입 체크를 추가하면 좋습니다.♻️ 방어적 코딩 제안
- CustomOAuth2User oAuth2User = (CustomOAuth2User) authentication.getPrincipal(); + Object principal = authentication.getPrincipal(); + if (!(principal instanceof CustomOAuth2User oAuth2User)) { + throw new IllegalStateException("Unexpected principal type: " + principal.getClass()); + }src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/service/CustomOAuth2UserService.java (2)
46-54: Java 17 switch expression 사용을 권장합니다.현재 if-else 체인 대신 Java 17의 switch expression을 사용하면 코드가 더 간결하고 가독성이 좋아집니다. 또한 새로운 Provider가 추가될 때 컴파일 타임에 누락을 방지할 수 있어요.
♻️ Switch expression으로 리팩토링 제안
- OAuth2Response oAuth2Response = null; - - if (provider == Provider.NAVER) { - oAuth2Response = new NaverResponse(oAuth2User.getAttributes()); - } else if (provider == Provider.GOOGLE) { - oAuth2Response = new GoogleResponse(oAuth2User.getAttributes()); - } else if (provider == Provider.KAKAO){ - oAuth2Response = new KaKaoResponse(oAuth2User.getAttributes()); - } else { - throw new UserSignUpException(UserErrorCode.NOT_PROVIDE_SOCIAL); - } + OAuth2Response oAuth2Response = switch (provider) { + case NAVER -> new NaverResponse(oAuth2User.getAttributes()); + case GOOGLE -> new GoogleResponse(oAuth2User.getAttributes()); + case KAKAO -> new KaKaoResponse(oAuth2User.getAttributes()); + default -> throw new UserSignUpException(UserErrorCode.NOT_PROVIDE_SOCIAL); + };코딩 가이드라인에 따라 Java 17의 최신 문법(Switch Expression)을 적절히 사용하는지 확인하였습니다.
59-59: Spring Data JPA 모범 사례: Optional 반환으로 현대화하기현재 코드는 명시적인 null 체크(
if (existAccount == null))로 안전하게 처리되고 있어서 NPE 위험은 없습니다. 다만 Spring Data JPA의 현대적 패턴에 맞춰AuthProviderAccountRepository의findByProviderId메서드를Optional<AuthProviderAccount>를 반환하도록 개선하면, 의도가 더 명확해지고 코드 스타일이 더 일관성 있게 됩니다.예를 들어:
- 현재:
AuthProviderAccount existAccount = authProviderAccountRepository.findByProviderId(providerId); if (existAccount == null) { ... }- 개선 후:
authProviderAccountRepository.findByProviderId(providerId).ifPresentOrElse(account -> { ... }, () -> { ... })또는 더 간단하게
ifPresent()또는orElseGet()을 활용할 수 있어서 코드가 더 함수형 스타일이 됩니다. 이미 다른 곳에서userRepository.findUserByEmail()이 Optional을 반환하고 있으므로(66번 줄), Repository 메서드들 간의 일관성을 높이는 것도 좋은 개선입니다.src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/CustomOAuth2User.java (1)
21-32: Java 17 스타일로 간결하게 리팩토링할 수 있습니다.
GrantedAuthority는 함수형 인터페이스이므로 람다로 변환 가능하고, 단일 요소 컬렉션은List.of()로 더 간결하게 표현할 수 있어요.♻️ 람다와 List.of()를 사용한 리팩토링
`@Override` public Collection<? extends GrantedAuthority> getAuthorities() { - Collection<GrantedAuthority> collection = new ArrayList<>(); - - collection.add(new GrantedAuthority() { - `@Override` - public String getAuthority() { - return authUserDTO.getRole(); - } - }); - return collection; + return List.of((GrantedAuthority) () -> authUserDTO.getRole()); }또는 더 읽기 쉽게:
`@Override` public Collection<? extends GrantedAuthority> getAuthorities() { return List.of(new SimpleGrantedAuthority(authUserDTO.getRole())); }
SimpleGrantedAuthority를 사용하면 Spring Security의 표준 구현체를 활용할 수 있어서 더 좋습니다.
src/main/java/com/whereyouad/WhereYouAd/domains/user/exception/code/UserErrorCode.java
Outdated
Show resolved
Hide resolved
| @Entity | ||
| @Table(name = "auth_provider_account") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
provider + providerId 조합에 Unique 제약 조건 추가를 권장합니다
동일한 소셜 계정(provider + providerId 조합)은 DB에서 유일해야 합니다. 현재는 테이블 레벨에서 이 제약이 없어서 동시성 이슈나 버그로 인해 중복 레코드가 생성될 수 있습니다.
예를 들어, Google 계정 "12345"가 두 번 저장되는 상황을 방지해야 합니다.
🔧 Unique 제약 조건 추가 제안
`@Entity`
-@Table(name = "auth_provider_account")
+@Table(name = "auth_provider_account",
+ uniqueConstraints = `@UniqueConstraint`(
+ name = "uk_provider_provider_id",
+ columnNames = {"provider", "provider_id"}
+ ))
`@Getter`이렇게 하면 DB 레벨에서 데이터 무결성이 보장되고, 중복 삽입 시도 시 예외가 발생하여 문제를 조기에 발견할 수 있습니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/entity/AuthProviderAccount.java`
around lines 11 - 12, Add a DB-level unique constraint for the social account
key (provider + providerId) to prevent duplicate records: modify the
AuthProviderAccount entity (class AuthProviderAccount) to declare a unique
constraint on the provider and providerId columns via the
`@Table`(uniqueConstraints=...) annotation using the actual column names (e.g.,
"provider" and the column name mapped from the providerId field), and also add a
corresponding DB migration (ALTER TABLE ... ADD CONSTRAINT ... UNIQUE or a
unique index) to enforce it in the database.
| public interface AuthProviderAccountRepository extends JpaRepository<AuthProviderAccount, Long> { | ||
| AuthProviderAccount findByProviderId(String username); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# AuthProviderAccountRepository 파일 위치 및 내용 확인
find . -name "AuthProviderAccountRepository.java" -type fRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 185
🏁 Script executed:
# 해당 파일의 전체 내용 확인
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/repository/AuthProviderAccountRepository.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 508
🏁 Script executed:
# findByProviderId 사용처 확인
rg -n "findByProviderId" --type=java -B2 -A2Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 1423
🏁 Script executed:
# AuthProviderAccount 도메인 모델 확인
find . -name "AuthProviderAccount.java" -type fRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 171
🏁 Script executed:
# Provider enum/class 구조 확인
find . -name "Provider.java" -type fRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 157
🏁 Script executed:
# AuthProviderAccount 엔티티 전체 내용 확인
cat -n ./src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/entity/AuthProviderAccount.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 1476
🏁 Script executed:
# Provider enum 확인
cat -n ./src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/constant/Provider.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 1171
🏁 Script executed:
# AuthProviderAccount 엔티티에 `@Unique` 또는 제약조건 확인
rg -n "AuthProviderAccount" --type=java -A20 | head -60Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 8292
🏁 Script executed:
# CustomOAuth2UserService에서 findByProviderId 사용 후 처리 방식 확인
rg -n "findByProviderId" --type=java -A5 | head -30Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 1185
🏁 Script executed:
# CustomOAuth2UserService의 전체 loadUser 메서드 확인
cat -n src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/service/CustomOAuth2UserService.java | head -100Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 5351
🏁 Script executed:
# providerId 생성 및 저장 방식 전체 확인
rg -n "provider_" --type=java -B3 -A3Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 2677
🏁 Script executed:
# AuthProviderAccount에 복합 유니크 제약이 있는지 확인
rg -n "@UniqueConstraint|@Unique" --type=javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
# 데이터베이스 스키마 또는 마이그레이션 파일 확인
find . -name "*.sql" -o -name "*migration*" | head -20Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
# Liquibase 또는 Flyway 마이그레이션 파일 확인
find . -path "*/db/migration/*" -o -path "*/db/changelog/*" 2>/dev/nullRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
# 전체 프로젝트 구조에서 SQL 파일 확인
find . -name "*.xml" -o -name "*.sql" | grep -i "liqui\|flyway\|migration" | head -20Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
# AuthAccountConverter의 저장 로직이 providerId를 어떻게 처리하는지 다시 확인
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/mapper/AuthAccountConverter.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 832
🏁 Script executed:
# 혹시 다른 repository 메서드나 쿼리 메서드가 있는지 확인
rg -n "findBy" src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/repository/ --type=javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 421
🏁 Script executed:
# 마지막 확인: providerId에 실제로 "google_123" 형태로 저장되는지 User entity도 확인
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/entity/User.java | head -50Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 1862
🏁 Script executed:
# OAuth2UserInfo 구조를 확인하여 providerId가 어떤 값인지 확인
find . -name "OAuth2UserInfo.java" -type fRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 161
🏁 Script executed:
# AuthAccountConverter 저장할 때 provider와 providerId가 어떻게 저장되는지 다시 확인
rg -n "AuthAccountConverter.toAuthProviderAccount" --type=java -B2 -A5Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 1240
🏁 Script executed:
# 마지막 확인: OAuth2UserInfo에서 providerId가 combined string인지 확인
cat -n ./src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/OAuth2UserInfo.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 548
파라미터명과 반환 타입을 개선해야 합니다
-
파라미터명 오류:
findByProviderId(String username)에서 파라미터명이username으로 되어 있는데, 실제로는"google_123"형태의 combined providerId입니다. 파라미터명을providerId로 수정해주세요. -
Optional 반환 권장: 현재는 null을 반환하는데, 59번 라인의
if (existAccount == null)체크로 NPE 위험이 있습니다.Optional<AuthProviderAccount> findByProviderId(String providerId)로 변경하면 null safe한 코드가 됩니다. -
설계 개선 제안: 현재 providerId 컬럼에 "google_123" 같은 combined 문자열을 저장하고 있는데, 엔티티는 provider(enum)도 따로 가지고 있어서 데이터 중복이 발생합니다. 다음처럼 개선하면 더 명확합니다:
🔧 개선된 Repository 방식
public interface AuthProviderAccountRepository extends JpaRepository<AuthProviderAccount, Long> {
- AuthProviderAccount findByProviderId(String username);
+ Optional<AuthProviderAccount> findByProviderAndProviderId(Provider provider, String providerId);
}그러면 저장할 때도:
// 현재 (combined string)
AuthProviderAccount account = AuthAccountConverter.toAuthProviderAccount(
provider, "google_123", user
);
// 개선된 방식 (분리된 데이터)
AuthProviderAccount account = AuthAccountConverter.toAuthProviderAccount(
provider, "123", user // providerId는 순수 ID만
);이렇게 하면 데이터베이스 정규화가 개선되고, provider와 providerId의 책임이 명확해집니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/repository/AuthProviderAccountRepository.java`
around lines 6 - 8, In AuthProviderAccountRepository, change the method
signature from findByProviderId(String username) to
Optional<AuthProviderAccount> findByProviderId(String providerId) (rename
parameter to providerId and return Optional to avoid nulls); then update all
callers (e.g., places checking existAccount == null) to handle Optional properly
(isPresent()/orElseThrow()/ifPresent) and adjust AuthAccountConverter usage so
it stores/queries the raw providerId (e.g., "123") alongside the provider enum
rather than a combined "google_123" string to avoid duplicated data
responsibility.
| @Override | ||
| public Map<String, Object> getAttributes() { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
getAttributes()가 null을 반환하면 NPE 위험이 있습니다.
Spring Security 내부 또는 다른 컴포넌트에서 getAttributes()를 호출할 때 null을 반환하면 NullPointerException이 발생할 수 있어요. 빈 Map을 반환하는 것이 더 안전합니다.
🐛 수정 제안
`@Override`
public Map<String, Object> getAttributes() {
- return null;
+ return Map.of();
}🤖 Prompt for AI Agents
In
`@src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/CustomOAuth2User.java`
around lines 16 - 19, The getAttributes() override in CustomOAuth2User currently
returns null which can cause NPEs; change CustomOAuth2User.getAttributes() to
return an immutable empty Map (e.g., Collections.emptyMap()) or a new empty Map
instance instead of null so callers never receive null; update the method
implementation in the CustomOAuth2User class to return a non-null empty map.
| @Override | ||
| public String getEmail() { | ||
| return String.valueOf(kakaoAccount.get("email")); | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return String.valueOf(profile.get("nickname")); | ||
| } |
There was a problem hiding this comment.
NullPointerException 위험이 있어요! 🚨
kakaoAccount나 profile이 null일 경우 getEmail()과 getName()에서 NPE가 발생합니다. 카카오 로그인에서 사용자가 이메일 제공에 동의하지 않으면 kakaoAccount.get("email")이 null을 반환하거나, kakaoAccount 자체가 없을 수 있어요.
예를 들어, 사용자가 카카오 로그인 시 "이메일 제공 선택 안함"을 선택하면 서버가 500 에러를 반환하게 됩니다.
🐛 Null 안전성 개선 제안
`@Override`
public String getEmail() {
+ if (kakaoAccount == null) {
+ return null;
+ }
+ Object email = kakaoAccount.get("email");
+ return email != null ? String.valueOf(email) : null;
- return String.valueOf(kakaoAccount.get("email"));
}
`@Override`
public String getName() {
+ if (profile == null) {
+ return null;
+ }
+ Object nickname = profile.get("nickname");
+ return nickname != null ? String.valueOf(nickname) : null;
- return String.valueOf(profile.get("nickname"));
}🤖 Prompt for AI Agents
In
`@src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/KaKaoResponse.java`
around lines 30 - 38, getEmail()과 getName() 에서 kakaoAccount 또는 profile 이 null이거나
내부 값이 null일 때 NPE(또는 "null" 문자열 반환)가 발생합니다; 수정 방법: getEmail()과 getName()에서
kakaoAccount 및 profile 객체가 null인지 검사하고, 내부 Map.get("email") /
Map.get("nickname") 결과도 null인지 확인한 뒤 안전한 기본값(예: null 또는 빈 문자열)이나 Optional로 반환하도록
변경하세요; 관련 식별자: 메서드 getEmail(), getName(), 필드 kakaoAccount, profile을 찾아 해당 null
체크 로직을 추가하세요.
| // 네이버는 response라는 안에 실제 데이터 있어서 추출해서 저장 | ||
| public NaverResponse(Map<String, Object> attribute) { | ||
| this.attribute = (Map<String, Object>) attribute.get("response"); | ||
| } |
There was a problem hiding this comment.
생성자에서 NPE 위험이 있어요! 🚨
attribute.get("response")가 null을 반환하면 this.attribute가 null이 되어, 이후 모든 getter 호출에서 NPE가 발생합니다. 네이버 API 응답이 예상과 다른 형식일 경우 로그인 전체가 실패하게 됩니다.
🐛 Null 체크 추가 제안
// 네이버는 response라는 안에 실제 데이터 있어서 추출해서 저장
public NaverResponse(Map<String, Object> attribute) {
- this.attribute = (Map<String, Object>) attribute.get("response");
+ Object response = attribute.get("response");
+ if (response == null) {
+ throw new IllegalArgumentException("Naver OAuth2 response is missing 'response' field");
+ }
+ this.attribute = (Map<String, Object>) response;
}📝 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.
| // 네이버는 response라는 안에 실제 데이터 있어서 추출해서 저장 | |
| public NaverResponse(Map<String, Object> attribute) { | |
| this.attribute = (Map<String, Object>) attribute.get("response"); | |
| } | |
| // 네이버는 response라는 안에 실제 데이터 있어서 추출해서 저장 | |
| public NaverResponse(Map<String, Object> attribute) { | |
| Object response = attribute.get("response"); | |
| if (response == null) { | |
| throw new IllegalArgumentException("Naver OAuth2 response is missing 'response' field"); | |
| } | |
| this.attribute = (Map<String, Object>) response; | |
| } |
🤖 Prompt for AI Agents
In
`@src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/NaverResponse.java`
around lines 11 - 14, The NaverResponse constructor sets this.attribute =
(Map<String, Object>) attribute.get("response") which can be null and later
cause NPEs in the class's getters; update the NaverResponse(Map<String,Object>
attribute) constructor to check attribute.get("response") for null and handle it
safely (e.g., assign Collections.emptyMap() or a new HashMap<>() as a default,
or throw a clear IllegalArgumentException) so subsequent getter methods on the
NaverResponse instance won't throw NPEs; ensure you reference the constructor in
class NaverResponse and the field attribute when making the change.
| @Override | ||
| public String getEmail() { | ||
| return String.valueOf(attribute.get("email")); | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return String.valueOf(attribute.get("name")); | ||
| } |
There was a problem hiding this comment.
getter 메서드들도 null 안전성 고려가 필요해요.
KaKaoResponse와 동일하게, attribute.get("email")이나 attribute.get("name")이 null일 경우 "null" 문자열이 반환됩니다. 네이버에서 이메일 제공 동의를 하지 않은 사용자의 경우 DB에 "null"이라는 문자열이 저장될 수 있어요.
🤖 Prompt for AI Agents
In
`@src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/NaverResponse.java`
around lines 26 - 34, NaverResponse's getEmail and getName currently call
String.valueOf(attribute.get(...)) which turns null into the literal "null";
change both getEmail() and getName() in class NaverResponse to mirror
KaKaoResponse's null-safety by checking attribute.get("email") and
attribute.get("name") for null and returning null (or an empty string if your
app expects that) instead of the string "null" — specifically, retrieve the
value from attribute, if it's non-null return its toString(), otherwise return
null; update getEmail() and getName() accordingly.
src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/OAuth2UserInfo.java
Show resolved
Hide resolved
...whereyouad/WhereYouAd/global/security/oauth2/handler/OAuth2AuthenticationSuccessHandler.java
Show resolved
Hide resolved
src/main/java/com/whereyouad/WhereYouAd/domains/user/exception/code/UserErrorCode.java
Outdated
Show resolved
Hide resolved
| import com.whereyouad.WhereYouAd.domains.user.persistence.entity.User; | ||
| import com.whereyouad.WhereYouAd.global.security.oauth2.dto.OAuth2Response; | ||
|
|
||
| public class UserConverter { |
There was a problem hiding this comment.
P4: domains/user/application/mapper 밑에 제가 회원가입 DTO 변환할 때 사용한 UserConverter 클래스랑 이름이 겹쳐서 헷갈릴 수 있을 것 같은데 OAuthConverter 등으로 변경하면 어떨까요?
There was a problem hiding this comment.
아 그렇군요 제가 위치를 착각했던 것 같습니다! 수정하겠습니다! 감사합니다!
|
|
||
| // 로그인 종류에 따라 이메일(식별자) 추출 | ||
| String email; | ||
| Object principal = authentication.getPrincipal(); |
There was a problem hiding this comment.
P4: 기존 로그인이랑 소셜 로그인 구현 구분 꼼꼼히 잘해주신거 같아요!!
src/main/java/com/whereyouad/WhereYouAd/global/security/oauth2/dto/OAuth2UserInfo.java
Show resolved
Hide resolved
3 participants
📌 관련 이슈
🚀 개요
📄 작업 내용
📸 스크린샷 / 테스트 결과 (선택)
0. 시작 DB 상태
1. 구글 로그인(http://localhost:8080/oauth2/authorization/google 접속)
2. 네이버 로그인(http://localhost:8080/oauth2/authorization/naver 접속)
3. 카카오 로그인(http://localhost:8080/oauth2/authorization/kakao 접속)
-> 네이버 메일과 동일하기 때문에 새로운 유저 생성 X
4. 쿠키를 통해 발급 받은 access token을 통해 swagger test
✅ 체크리스트
🔍 리뷰 포인트 (Review Points)
Summary by CodeRabbit
릴리스 노트
새로운 기능
설정
✏️ Tip: You can customize this high-level summary in your review settings.