[URECA-58] Feat: 회원탈퇴 - 소셜 로그인 끊기 - #35
Conversation
This workflow automatically prefixes pull request titles and new commits with the Jira issue key extracted from the branch name.
Updated issue template title and branch description for clarity.
* chore: domain/auth 구조 및 global/security 구조 분리 * chore: OAuth 공통부분 파일 작성 * chore: KakaoOAuthClient, GoogleOAuthClient 파일 생성 * chore: PR 템플릿 업로드 * chore: OAuthService 시그니처 변경, OAuthProvider Enum 도입, OAuthController 엔드포인트 정리, DTO 불변성 개선 * chore: OAuthController 입력 검증 추가, GlobalExceptionHandler 추가 * chore: GlobalExceptionHandler 에러 발생 시 내부 구현 노출 문제 및 로깅 누락 해결
* Feat: User 엔티티 생성 * Feat OAuthClient 생성 및 Google, Kakao, Naver OAuthClient 구현 * Feat: OAuthServiceImpl 및 UserMapper.xml 구현 -> 로그인 요청 확인 및 DB 연동 * Fix: GoogleOAuthClient, NaverOAuthClient에 name에 대한 fallback 추가
* Feat: User 엔티티 생성 * Feat OAuthClient 생성 및 Google, Kakao, Naver OAuthClient 구현 * Feat: OAuthServiceImpl 및 UserMapper.xml 구현 -> 로그인 요청 확인 및 DB 연동 * Fix: GoogleOAuthClient, NaverOAuthClient에 name에 대한 fallback 추가 * Fix: name처리 * Feat: WebMvcConfig 추가 - CORS 설정 * Feat: JWT 발급 로직 구현 * Feat: JWT 발급 로직 구현 * Feat: JWT 검증: JWT 인증 필터 및 보안 설정 적용 * Feat: JWT 검증: JWT 인증 필터 및 보안 설정 적용 * Feat: JWT 인증 예외 401로 정리 + RefreshToken 설계/저장 * Feat: refresh api 구현 * Feat: 리프레쉬토큰 DB 저장 및 RefreshTokenMapper에 resultMap 적용 * Feat: Refresh Rotation 구현 * Feat: 로드아웃 구현 * Docs: .coderabbit.yaml 파일 수정 * Fix: JJWT 버전 수정 및 의ì¡올바르지 않은 ´ì 의존성 제거 * Fix: 쿠키 보안 설정 개선 및 SameSite 속성 추가 * Fix: 로그아웃 엔드포인트 보호 추가 및 로그아웃 서비스 레이어 결합도 제거 * Fix: ãrefreshtoken을 body로 부터 분리 * Fix: ãrefreRefreshTokenServiceImpl 수정 * Fix CookieUtils 유틸 분리 * Fix: 사ìChore하지 않는 import 정리 * Fix RefreshTokenServiceImpl 죽은 코드 제거 및 jjwt 버전 업그레이드 * Fix: 모든 쿠키 생성/삭제 로직을 CookieUtils로 통일 * Fix: Nitpick Comment 정리 * Fix: OAuthServiceImpl 패키지 헬퍼 메소드 분리 * Fix: JWT 서명 검증 먼저하도록 수정 * Fix: userId 변수 중복 선언 수정 * Fix: Refresh Token Rotation에서 Race Condition 해소 * Chore: 들여쓰기 수정 * Fix: 예외사항 범위 확장 * Fix: DB/JWT 시간 통일 및 Access/Refresh Token 타입 Claim 분리 * Fix: REFRESH_TOKEN_INVALID 적용 범위 확장
* Feat: User 엔티티 생성 * Feat OAuthClient 생성 및 Google, Kakao, Naver OAuthClient 구현 * Feat: OAuthServiceImpl 및 UserMapper.xml 구현 -> 로그인 요청 확인 및 DB 연동 * Fix: GoogleOAuthClient, NaverOAuthClient에 name에 대한 fallback 추가 * Fix: name처리 * Feat: WebMvcConfig 추가 - CORS 설정 * Feat: JWT 발급 로직 구현 * Feat: JWT 발급 로직 구현 * Feat: JWT 검증: JWT 인증 필터 및 보안 설정 적용 * Feat: JWT 검증: JWT 인증 필터 및 보안 설정 적용 * Feat: JWT 인증 예외 401로 정리 + RefreshToken 설계/저장 * Feat: refresh api 구현 * Feat: 리프레쉬토큰 DB 저장 및 RefreshTokenMapper에 resultMap 적용 * Feat: Refresh Rotation 구현 * Feat: 로드아웃 구현 * Docs: .coderabbit.yaml 파일 수정 * Fix: JJWT 버전 수정 및 의ì¡올바르지 않은 ´ì 의존성 제거 * Fix: 쿠키 보안 설정 개선 및 SameSite 속성 추가 * Fix: 로그아웃 엔드포인트 보호 추가 및 로그아웃 서비스 레이어 결합도 제거 * Fix: ãrefreshtoken을 body로 부터 분리 * Fix: ãrefreRefreshTokenServiceImpl 수정 * Fix CookieUtils 유틸 분리 * Fix: 사ìChore하지 않는 import 정리 * Fix RefreshTokenServiceImpl 죽은 코드 제거 및 jjwt 버전 업그레이드 * Fix: 모든 쿠키 생성/삭제 로직을 CookieUtils로 통일 * Fix: Nitpick Comment 정리 * Fix: OAuthServiceImpl 패키지 헬퍼 메소드 분리 * Fix: JWT 서명 검증 먼저하도록 수정 * Fix: userId 변수 중복 선언 수정 * Fix: Refresh Token Rotation에서 Race Condition 해소 * Chore: 들여쓰기 수정 * Fix: 예외사항 범위 확장 * Fix: DB/JWT 시간 통일 및 Access/Refresh Token 타입 Claim 분리 * Fix: REFRESH_TOKEN_INVALID 적용 범위 확장 * fix: spring, gradle 버전 다운그레이드
* feat: stt 1차 짧은 대화 성공 * fix: stt m4a to wav convert 추가 --------- Co-authored-by: 40food <40food@naver.com>
* chore: remove secrets and use env variables * URECA-48: remove secrets and use env variables * URECA-48: restore summary and gemini feature code (without secrets) * fix: STT 오류 고려한 Gemini 요약 프롬프트 보강 * fix: 오타 수정 * fix: Gemini 응답의 null 값에 대한 방어 로직 * fix: SummaryRequest 역직렬화 및 입력 검증 보강 * fix: Summary 저장 트랜잭션 처리 추가
* fix: jwt필터에서 logout엔드í �¬인트 제거 * fix: users 테이블 관련 수정 * fix: refresh_tokens 테이블 관련 수정 * feat: 카카오 로그인 시 이메일, 닉네임 저장 가능하도록 ì수정
📝 WalkthroughWalkthrough각 OAuth 클라이언트가 사용자 정보와 토큰 정보를 함께 반환하도록 Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@src/main/java/com/ureca/unity/domain/auth/model/OAuthToken.java`:
- Around line 10-16: The OAuthToken class currently stores accessToken and
refreshToken in plaintext; implement application-level encryption by adding a
MyBatis TypeHandler (or integrate Spring Security Crypto) that encrypts
accessToken and refreshToken on insert/update and decrypts on select, update
OAuthTokenMapper.xml to use those handlers, and update OAuthServiceImpl where
tokens are persisted to call the secure persistence flow (or use repository
methods that trigger the TypeHandler) instead of storing raw values;
additionally ensure logs never print token fields (mask accessToken/refreshToken
as ***REDACTED*** in any logging code) and coordinate with ops to enable
DB-level encryption (TDE/column encryption) for the oauth_tokens table and
review token expiry/refresh logic for security.
In
`@src/main/java/com/ureca/unity/domain/auth/service/oauth/GoogleOAuthClient.java`:
- Around line 41-43: In GoogleOAuthClient where accessToken, refreshToken and
expiresIn are extracted (variables accessToken, refreshToken, expiresIn),
replace the direct Long.valueOf(String.valueOf(token.get("expires_in"))) with a
defensive parse: check for null, trim the string, and attempt Long.parseLong
inside a try-catch that catches NumberFormatException; on failure set expiresIn
to null (or a safe default) and log or warn the parsing issue (using your class
logger) so malformed/non-numeric expires_in values don’t crash the flow. Ensure
the change only affects the expires_in parsing logic and preserves existing
behavior for access_token and refresh_token.
In `@src/main/java/com/ureca/unity/domain/auth/service/OAuthServiceImpl.java`:
- Around line 45-48: The in-memory anyUser isn't updated after calling
userMapper.restoreById(...) so anyUser.getDeletedAt() remains non-null and
createLoginResult(...) gets the wrong requiresOnboarding value; fix by
determining restoration explicitly: after calling restoreById in
OAuthServiceImpl either re-fetch the user from the database (e.g.,
loadById(anyUser.getId())) before calling createLoginResult, or set a boolean
restored = true before calling createLoginResult and pass restored (or use
restored ? false : (anyUser.getDeletedAt() != null)) as the requiresOnboarding
argument to createLoginResult so restored users don't get forced into
onboarding.
In
`@src/main/java/com/ureca/unity/domain/user/service/unlink/GoogleUnlinkClient.java`:
- Around line 46-51: The catch block in GoogleUnlinkClient currently rewraps all
exceptions (including existing CustomException and specific
OAUTH_TOKEN_NOT_FOUND cases) into SOCIAL_UNLINK_FAILED; change the catch to
rethrow existing CustomException instances and only wrap other exceptions: in
the catch(Exception e) check "if (e instanceof CustomException) throw
(CustomException) e;" otherwise throw new
CustomException(ErrorCode.SOCIAL_UNLINK_FAILED, e) (or similar constructor) so
original CustomException and its error code remain intact while
non-CustomException errors are wrapped; apply this change around the response
check and the catch block that surrounds res.getStatusCode() handling.
In
`@src/main/java/com/ureca/unity/domain/user/service/unlink/NaverUnlinkClient.java`:
- Around line 25-50: The RestTemplate is created directly in NaverUnlinkClient
(private final RestTemplate restTemplate = new RestTemplate()) which lacks
connection/read timeouts; change to accept a RestTemplate bean via constructor
injection (remove the new RestTemplate instantiation) and configure a single
RestTemplate bean using RestTemplateBuilder with setConnectTimeout and
setReadTimeout (per the suggested RestTemplateConfig), then update
NaverUnlinkClient.unlink (and the analogous GoogleUnlinkClient and
KakaoUnlinkClient classes) to use the injected restTemplate instead of creating
new instances so all OAuth clients share the timeout-configured bean.
In `@src/main/java/com/ureca/unity/domain/user/service/UserServiceImpl.java`:
- Around line 22-35: The withdraw method in UserServiceImpl currently calls
socialUnlinkService.unlink(user, token) inside the `@Transactional` boundary which
can leave DB and external provider state inconsistent; refactor so the
revoke/unlink runs after DB commit (use TransactionSynchronization.afterCommit
or publish a domain event consumed by a `@TransactionalEventListener` to perform
the unlink asynchronously with retries) and keep the DB deletion
(deleteByUserId, softDeleteById) inside the existing transaction; also change
the OAuth token handling around oAuthTokenService.find so that if a user has no
provider token (e.g., LOCAL/non-social account) you skip the unlink step instead
of throwing OAUTH_TOKEN_NOT_FOUND and still proceed with deletion.
- Around line 25-31: The withdraw() flow currently always queries
oAuthTokenService.find using user.getProvider(), but User.provider is nullable
and non-OAuth accounts should not trigger OAuth unlink; update withdraw() to
first validate provider (e.g., check user.getProvider() != null and that it
denotes a social provider) before calling oAuthTokenService.find or invoking
SocialUnlinkServiceImpl; if provider is null or not a social type, skip the
OAuth token lookup/unlink branch (and only perform local cleanup), ensuring you
reference the User object, withdraw(), user.getProvider(),
oAuthTokenService.find(...) and SocialUnlinkServiceImpl when making the
conditional branch.
In `@src/main/resources/application.yml`:
- Line 46: The current cookie.secure default is false which risks sending
cookies over HTTP if COOKIE_SECURE is missing; change the default to true by
updating the property reference for cookie.secure to default to true (use the
environment variable COOKIE_SECURE as the override) and add a development
profile override (e.g., set cookie.secure to false in the dev profile file) or
alternatively ensure application-prod.yml sets cookie.secure: true so production
always uses secure cookies; update any documentation or deployment checklist to
require COOKIE_SECURE when profiles are not used.
- Line 43: The CORS allowed-origins handling may fail when environment values
contain spaces after commas; in WebMvcConfig.java update the
.allowedOrigins(...) call that currently uses allowedOrigins.split(",") to trim
entries—either map(String::trim) over Arrays.stream(allowedOrigins.split(","))
and collect to String[] or use allowedOrigins.split("\\s*,\\s*")—so each origin
has surrounding whitespace removed before passing to .allowedOrigins.
In `@src/main/resources/mapper/ouath/OAuthTokenMapper.xml`:
- Around line 17-20: Rename the mapper directory from "ouath" to "oauth" to fix
the typo so the mapper file with the <insert id="insert"> mapping for
oauth_tokens is located under mapper/oauth; confirm any build/resource
references are updated. Keep the existing insert mapping (INSERT INTO
oauth_tokens ...) and do not change the upsert pattern in service (delete→insert
with `@Transactional`). Add a DB-level unique constraint on (user_id, provider)
for the oauth_tokens table via your migration/schema (so duplicates are
prevented outside the service layer) and verify the migration file is
committed/updated accordingly.
🧹 Nitpick comments (19)
src/main/resources/mapper/ouath/OAuthTokenMapper.xml (1)
1-6: 디렉토리명 오타:ouath→oauth파일 경로가
mapper/ouath/로 되어 있는데,mapper/oauth/가 올바른 철자입니다. 기능에는 영향이 없지만 일관성과 검색 편의를 위해 수정을 권장합니다.src/main/java/com/ureca/unity/domain/auth/dto/OAuthTokenInfo.java (1)
3-7: nullable 필드에 대한 명시적 표기 권장주석으로 nullable 여부를 명시한 것은 좋지만,
@Nullable어노테이션을 사용하면 IDE 지원과 정적 분석에 도움이 됩니다.import org.springframework.lang.Nullable; public record OAuthTokenInfo( String accessToken, `@Nullable` String refreshToken, `@Nullable` Long expiresInSeconds ) {}Spring의
@Nullable또는 Jakarta의@Nullable을 사용할 수 있습니다. 공식 문서: Spring Null-Safetysrc/main/java/com/ureca/unity/domain/user/service/unlink/SocialUnlinkClient.java (1)
6-8: LGTM! 인터페이스 설계가 명확합니다.각 provider별 구현체에서 토큰 처리 방식이 다르더라도 일관된 인터페이스를 유지하는 것은 좋은 설계입니다.
주석을 Javadoc 형식으로 변경하면 API 문서화에 도움이 됩니다:
📝 Javadoc 형식 제안
public interface SocialUnlinkClient { + /** + * 소셜 계정 연결을 해제합니다. + * + * `@param` user 연결 해제할 사용자 + * `@param` token OAuth 토큰 (Google/Naver는 필수, Kakao는 내부적으로 미사용될 수 있음) + * `@throws` CustomException SOCIAL_UNLINK_FAILED 연결 해제 실패 시 + */ - void unlink(User user, OAuthToken token); // token nullable 아님(구글/네이버), 카카오는 token 없어도 되지만 여기선 받게 둠 + void unlink(User user, OAuthToken token); }src/main/java/com/ureca/unity/domain/user/service/unlink/SocialUnlinkServiceImpl.java (1)
19-24: provider 소문자 변환은 Locale.ROOT로 고정하면 안전합니다.
기본 로케일에 따라toLowerCase()결과가 달라져 키 매칭이 실패할 수 있습니다.Locale.ROOT로 고정하고trim()을 함께 적용하면 더 안전합니다. Java 공식 문서의 로케일 처리 가이드 참고 부탁드립니다.♻️ 제안 변경
+import java.util.Locale; ... - String provider = user.getProvider() == null ? "" : user.getProvider().toLowerCase(); + String provider = user.getProvider() == null + ? "" + : user.getProvider().trim().toLowerCase(Locale.ROOT);src/main/java/com/ureca/unity/domain/auth/service/OAuthTokenServiceImpl.java (1)
24-27: 읽기 전용 트랜잭션 힌트 추가를 권장합니다.
find메서드에@Transactional(readOnly = true)를 추가하면 DB 최적화에 도움이 됩니다. Spring 공식 문서에서도 읽기 전용 작업에 이 설정을 권장합니다.♻️ 개선 제안
+ `@Transactional`(readOnly = true) `@Override` public Optional<OAuthToken> find(Long userId, String provider) { return oAuthTokenMapper.findByUserIdAndProvider(userId, provider); }src/main/java/com/ureca/unity/domain/auth/service/oauth/NaverOAuthClient.java (3)
35-35: RestTemplate을 Spring Bean으로 주입받는 것을 권장합니다.직접 인스턴스화하면 커넥션 풀 설정, 타임아웃 구성, 테스트 시 Mock 주입이 어렵습니다.
@Configuration에서RestTemplateBean을 정의하고 주입받으세요.♻️ 개선 방향
- private final RestTemplate restTemplate = new RestTemplate(); + private final RestTemplate restTemplate;별도 Configuration 클래스에서:
`@Bean` public RestTemplate restTemplate() { // 타임아웃, 커넥션 풀 등 설정 가능 return new RestTemplate(); }
41-43: 토큰 파싱 시 타입 안전성을 개선하세요.
expires_in값이Integer로 반환될 수 있어Long.valueOf(String.valueOf(...))보다Number인터페이스를 활용하면 더 안전합니다.♻️ 개선 제안
String accessToken = token.get("access_token").toString(); String refreshToken = token.get("refresh_token") != null ? String.valueOf(token.get("refresh_token")) : null; - Long expiresIn = token.get("expires_in") != null ? Long.valueOf(String.valueOf(token.get("expires_in"))) : null; + Long expiresIn = token.get("expires_in") != null ? ((Number) token.get("expires_in")).longValue() : null;
77-77: 예외 타입을 프로젝트의CustomException으로 통일하세요.다른 서비스들은
CustomException과ErrorCode를 사용하는데, 여기서는IllegalArgumentException을 던지고 있습니다. 일관된 예외 처리를 위해 통일이 필요합니다.♻️ 개선 제안
if (response.getBody() == null || response.getBody().get("access_token") == null) { - throw new IllegalArgumentException("Failed to retrieve Naver access token"); + throw new CustomException(ErrorCode.OAUTH_TOKEN_NOT_FOUND); }
fetchUserInfo메서드(Line 99)도 동일하게 수정하세요.src/main/java/com/ureca/unity/domain/auth/service/oauth/KakaoOAuthClient.java (3)
35-35: NaverOAuthClient와 동일하게 RestTemplate Bean 주입을 권장합니다.테스트 용이성과 설정 일관성을 위해 직접 인스턴스화 대신 Bean 주입 방식으로 변경하세요.
38-51: 토큰 추출 로직이 NaverOAuthClient와 중복됩니다.
authenticate메서드의 토큰 파싱 로직(Lines 41-43)이 NaverOAuthClient와 동일합니다. 향후 리팩토링 시 공통 유틸리티 메서드나 부모 클래스로 추출을 고려해보세요.
79-80: 예외 타입을CustomException으로 통일하세요.NaverOAuthClient와 동일하게
IllegalArgumentException대신 프로젝트 표준인CustomException을 사용하세요.src/main/java/com/ureca/unity/domain/auth/service/OAuthServiceImpl.java (1)
39-61: 중첩된 Optional 체인의 가독성 개선을 고려해보세요.로직은 올바르게 동작하지만, 3단계 중첩된
map/orElseGet체인은 추적하기 어렵습니다. 별도의 private 메서드로 분리하면 테스트와 유지보수가 쉬워집니다.src/main/java/com/ureca/unity/domain/user/service/unlink/KakaoUnlinkClient.java (3)
25-25:OAuthToken token파라미터가 사용되지 않습니다.Kakao는 Admin API를 사용하므로 토큰이 필요 없지만,
SocialUnlinkClient인터페이스 시그니처와의 일관성을 위해 포함된 것으로 보입니다. 의도적인 설계라면 주석으로 명시하거나, 사용하지 않는 파라미터에@SuppressWarnings("unused")를 추가하세요.♻️ 개선 제안
`@Override` - public void unlink(User user, OAuthToken token) { + public void unlink(User user, OAuthToken token) { // token unused: Kakao uses Admin API
22-22: RestTemplate을 Bean으로 주입받으세요.OAuth 클라이언트들과 동일하게, 테스트 용이성과 설정 일관성을 위해 직접 인스턴스화 대신 Bean 주입을 권장합니다.
GoogleUnlinkClient와NaverUnlinkClient도 동일한 패턴을 사용하고 있어 일괄 리팩토링이 효과적입니다.
47-49: 원본 예외 정보를 보존하세요.현재 catch 블록에서 원본 예외를 무시하고 있어 디버깅이 어렵습니다. 다만
CustomException의 현재 생성자는 cause 매개변수를 지원하지 않으므로, 아래 두 가지 방식 중 하나로 개선하세요.방식 1: CustomException에 cause 생성자 추가 (권장)
CustomException 클래스에 다음 생성자를 추가합니다:
public CustomException(ErrorCode errorCode, Throwable cause) { super(errorCode.getMessage(), cause); this.errorCode = errorCode; }그 후 KakaoUnlinkClient에서:
} catch (Exception e) { throw new CustomException(ErrorCode.SOCIAL_UNLINK_FAILED, e); }방식 2: initCause() 활용
CustomException을 수정하지 않으려면:
} catch (Exception e) { throw new CustomException(ErrorCode.SOCIAL_UNLINK_FAILED).initCause(e); }방식 1이 더 명확하고 Java 표준 예외 처리 관례에 부합합니다. 스택 트레이스 수집 및 로깅 시스템에서 원본 예외를 제대로 추적할 수 있습니다.
src/main/java/com/ureca/unity/domain/auth/service/oauth/GoogleOAuthClient.java (2)
35-35:RestTemplate을 Bean으로 주입받는 것을 권장합니다.현재
new RestTemplate()으로 인라인 생성하고 있는데, 이렇게 하면 테스트 시 모킹이 어렵고, 타임아웃이나 인터셉터 같은 공통 설정을 적용하기 어렵습니다. Spring에서 관리하는 Bean으로 주입받으면 설정을 중앙에서 관리할 수 있습니다.Spring 공식 문서에서
RestTemplateBuilder를 통한 설정 방법을 참고하세요.
53-79: 타입 안전성을 위해ParameterizedTypeReference사용을 고려해보세요.현재
@SuppressWarnings({"rawtypes", "unchecked"})로 경고를 억제하고 있지만,ParameterizedTypeReference를 사용하면 타입 안전하게 처리할 수 있습니다. 현재 구현도 동작에 문제는 없으니 선택적 개선 사항입니다.src/main/java/com/ureca/unity/domain/user/service/unlink/GoogleUnlinkClient.java (2)
14-18:@RequiredArgsConstructor가 불필요하고,RestTemplate은 주입을 권장합니다.현재 클래스에 생성자 주입이 필요한
final필드가 없어서@RequiredArgsConstructor는 의미가 없습니다.RestTemplate을 Bean으로 주입받으면 테스트도 용이해지고 애노테이션도 유의미해집니다.KakaoUnlinkClient, NaverUnlinkClient도 동일한 패턴을 사용하고 있으니, 향후 일괄 리팩토링을 고려해보세요.
♻️ 제안하는 수정 방법
`@Component`("googleUnlink") -@RequiredArgsConstructor public class GoogleUnlinkClient implements SocialUnlinkClient { - private final RestTemplate restTemplate = new RestTemplate(); + private final RestTemplate restTemplate; + + public GoogleUnlinkClient(RestTemplate restTemplate) { + this.restTemplate = restTemplate; + }또는
RestTemplateBean이 이미 정의되어 있다면:`@Component`("googleUnlink") `@RequiredArgsConstructor` public class GoogleUnlinkClient implements SocialUnlinkClient { - private final RestTemplate restTemplate = new RestTemplate(); + private final RestTemplate restTemplate;
40-40: Revoke URL을 설정 파일로 외부화하는 것을 고려해보세요.다른 OAuth 클라이언트들(GoogleOAuthClient 등)은
@Value로 URL을 주입받고 있습니다. 일관성과 유지보수를 위해 revoke URL도application.yml에서 관리하면 환경별 설정 변경이 용이해집니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/com/ureca/unity/domain/auth/service/oauth/GoogleOAuthClient.java`:
- Line 37: GoogleOAuthClient currently creates a RestTemplate instance directly
(private final RestTemplate restTemplate = new RestTemplate()), which bypasses
the RestTemplate bean (and its timeouts) and hampers testing; change that field
to a constructor-injected dependency (keep private final RestTemplate
restTemplate without new) so Spring injects the RestTemplateConfig bean via the
existing `@RequiredArgsConstructor`, and update the excessive indentation inside
the if block in the method using restTemplate (the same if-block around the
OAuth response handling) to normal indentation.
In `@src/main/java/com/ureca/unity/domain/auth/service/OAuthServiceImpl.java`:
- Around line 78-90: The upsert/save calls (oAuthTokenService.upsert(...) and
saveRefreshToken(...)) can throw unchecked DataAccessException and currently
bubble up, aborting login; wrap these calls inside a narrow try-catch in
OAuthServiceImpl (or the methods that call them) to catch DataAccessException
(or Exception), log a clear error/warn including exception details and context
(userId, provider) and allow the method to continue returning the issued access
token; if your intended policy is to make DB persistence mandatory instead,
instead throw a custom LoginPersistenceException after logging and mark
transactional boundaries with `@Transactional` on the enclosing method—pick one
policy and implement the corresponding try-catch+logging or
transactional+explicit exception as described.
🧹 Nitpick comments (6)
src/main/java/com/ureca/unity/domain/user/service/UserServiceImpl.java (1)
3-13: 사용하지 않는 import 정리가 필요합니다.
RefreshTokenMapper(line 3)와Transactional(line 13)은 현재 클래스에서 사용되지 않습니다. 트랜잭션 처리가UserWithdrawTxService로 이동했으니 정리해 주세요.♻️ 제안하는 수정
package com.ureca.unity.domain.user.service; -import com.ureca.unity.domain.auth.mapper.RefreshTokenMapper; import com.ureca.unity.domain.auth.model.OAuthToken; import com.ureca.unity.domain.auth.service.OAuthTokenService; import com.ureca.unity.domain.user.mapper.UserMapper; import com.ureca.unity.domain.user.model.User; import com.ureca.unity.domain.user.service.unlink.SocialUnlinkService; import com.ureca.unity.global.exception.CustomException; import com.ureca.unity.global.exception.ErrorCode; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional;src/main/java/com/ureca/unity/global/config/OAuthTokenCrypto.java (2)
24-32: 키 유도 방식 개선을 권장합니다.현재 secret을 UTF-8 바이트로 변환 후 32바이트로 패딩(0으로 채움)하는 방식은 짧은 secret의 엔트로피를 활용하지 못합니다.
권장 개선안: PBKDF2, HKDF 같은 표준 KDF(Key Derivation Function)를 사용하면 짧은 secret에서도 강력한 키를 유도할 수 있습니다.
♻️ HKDF를 사용한 키 유도 예시
+import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; + public OAuthTokenCrypto(`@Value`("${security.oauth-token.secret}") String secret) { - byte[] raw = secret.getBytes(StandardCharsets.UTF_8); - this.keyBytes = new byte[32]; - for (int i = 0; i < this.keyBytes.length; i++) { - this.keyBytes[i] = i < raw.length ? raw[i] : 0; - } + // HKDF-SHA256 간소화 버전 (또는 BouncyCastle HKDF 사용) + this.keyBytes = deriveKey(secret.getBytes(StandardCharsets.UTF_8), 32); } + +private byte[] deriveKey(byte[] ikm, int length) { + try { + Mac mac = Mac.getInstance("HmacSHA256"); + mac.init(new SecretKeySpec(ikm, "HmacSHA256")); + byte[] prk = mac.doFinal("oauth-token-encryption".getBytes(StandardCharsets.UTF_8)); + mac.init(new SecretKeySpec(prk, "HmacSHA256")); + return Arrays.copyOf(mac.doFinal(new byte[]{1}), length); + } catch (Exception e) { + throw new IllegalStateException("Key derivation failed", e); + } +}참고: NIST SP 800-108 KDF 권장사항
75-78: 복호화 실패 시 로깅 추가를 권장합니다.마이그레이션 단계에서 평문 데이터를 허용하는 방어 로직은 이해되지만, 운영 환경에서 실제 복호화 오류를 감지하기 어렵습니다.
🔍 로깅 추가 제안
} catch (Exception e) { // 방어: 기존 데이터가 평문/다른 포맷이면 그냥 원문 반환(마이그레이션 단계에서 유용) + log.warn("OAuth token decryption failed, returning original value (migration mode)", e); return enc; }
@Slf4j어노테이션과 함께 사용하세요.src/main/java/com/ureca/unity/domain/auth/service/OAuthServiceImpl.java (1)
4-5: 와일드카드 import 사용현재
com.ureca.unity.domain.auth.dto.*형태의 와일드카드 import를 사용하고 있습니다. 기능상 문제는 없지만, 명시적 import를 사용하면 어떤 클래스가 실제로 사용되는지 파악하기 쉬워 코드 가독성과 유지보수성이 향상됩니다.src/main/java/com/ureca/unity/domain/auth/service/oauth/GoogleOAuthClient.java (2)
63-90:@SuppressWarnings사용에 대한 개선 제안현재 raw type 경고를 억제하고 있는데, 제네릭 타입을 명시하면 타입 안전성을 높일 수 있습니다.
♻️ 제안하는 수정 방법
ParameterizedTypeReference를 사용하여 타입 안전성 확보:- `@SuppressWarnings`({"rawtypes", "unchecked"}) private Map<String, Object> getTokenResponse(String code) { // ... 기존 코드 ... - ResponseEntity<Map> response = restTemplate.exchange( + ResponseEntity<Map<String, Object>> response = restTemplate.exchange( tokenUri, HttpMethod.POST, request, - Map.class + new ParameterizedTypeReference<Map<String, Object>>() {} ); // ... null check ... - return (Map<String, Object>) response.getBody(); + return response.getBody(); }Spring의
ParameterizedTypeReference를 사용하면 런타임에도 제네릭 타입 정보가 유지되어 안전한 역직렬화가 가능합니다. Spring 공식 문서 참고하세요.
93-124: fetchUserInfo 메서드 구현 검토사용자 정보 조회 로직이 잘 구현되어 있습니다. null 체크와 기본값 처리가 적절합니다.
한 가지 고려사항:
fetchUserInfo메서드도getTokenResponse와 동일하게 raw typeMap을 사용하고 있습니다. 일관성을 위해 동일한 방식으로 개선하면 좋겠습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/com/ureca/unity/global/exception/ErrorCode.java`:
- Line 23: The AUTH_STORAGE_FAILED enum constant currently uses a misleading
message about social login; update its user-facing message in the ErrorCode enum
(AUTH_STORAGE_FAILED) to reflect a storage failure (e.g., "저장에 실패했습니다. 잠시 후 다시
시도해주세요.") while keeping the HttpStatus.INTERNAL_SERVER_ERROR code unchanged so
the cause matches the message shown to users and operators.
🧹 Nitpick comments (2)
src/main/java/com/ureca/unity/global/exception/ErrorCode.java (1)
20-21: OAUTH_TOKEN_NOT_FOUND의 상태 코드(400) 재검토 권장
토큰 미존재는 “잘못된 요청”이라기보다 “재인증 필요(401)” 또는 “리소스 부재(404)”로 보는 게 더 자연스러울 수 있습니다. API 계약과 일관되게 맞춰 주세요. (상태 코드 기준은 공식 HTTP 문서를 참고 권장)src/main/java/com/ureca/unity/domain/auth/service/OAuthServiceImpl.java (1)
43-71: provider 문자열 소스 일원화로 토큰 키 불일치 방지사용자 조회는
userInfo.getProvider()를 사용하지만, 토큰 저장에는provider.value()를 전달합니다. 두 값이 불일치하면 OAuthToken이 다른 provider 키로 저장되어 unlink/조회가 실패할 수 있습니다. 동일한 providerKey를 쓰거나 불일치 시 예외를 던지는 방어 로직을 추천합니다.🛠️ 제안 수정
OAuthAuthResult auth = oAuthClient.authenticate(authorizationCode); OAuthUserInfo userInfo = auth.userInfo(); OAuthTokenInfo tokenInfo = auth.tokenInfo(); +String providerKey = userInfo.getProvider(); +if (!providerKey.equals(provider.value())) { + throw new CustomException(ErrorCode.INVALID_OAUTH_PROVIDER); +} return userMapper - .findByProviderAndProviderId(userInfo.getProvider(), userInfo.getProviderId()) - .map(u -> createLoginResult(u.getId(), false, provider.value(), tokenInfo)) + .findByProviderAndProviderId(providerKey, userInfo.getProviderId()) + .map(u -> createLoginResult(u.getId(), false, providerKey, tokenInfo)) .orElseGet(() -> - userMapper.findAnyByProviderAndProviderId(userInfo.getProvider(), userInfo.getProviderId()) + userMapper.findAnyByProviderAndProviderId(providerKey, userInfo.getProviderId()) .map(anyUser -> { boolean wasDeleted = anyUser.getDeletedAt() != null; if (wasDeleted) { userMapper.restoreById( anyUser.getId(), userInfo.getEmail(), userInfo.getName() ); } // 재가입이면 true, 기존이면 false - return createLoginResult(anyUser.getId(), wasDeleted, provider.value(), tokenInfo); + return createLoginResult(anyUser.getId(), wasDeleted, providerKey, tokenInfo); }) .orElseGet(() -> { User newUser = User.builder() .provider(userInfo.getProvider()) .providerId(userInfo.getProviderId()) .email(userInfo.getEmail()) .name(userInfo.getName()) .role("ROLE_USER") .build(); userMapper.insert(newUser); - return createLoginResult(newUser.getId(), true, provider.value(), tokenInfo); + return createLoginResult(newUser.getId(), true, providerKey, tokenInfo); }) );
Key Changes
작업 내역
이전에 회원탈퇴 기능에 소셜 로그인 끊기를 구현하지 않았습니다.
이는 사용자 기준에서는 Unity 서비스에서 회원 탈퇴를 한 것이었으나, 소셜 서비스 입장에서는 Unity 서비스 여전히 연결되는 이슈를 만듭니다.
또한, 사용자가 Unity에 재가입시 동의 화면이 등장하지 않게 되며 이는 UX적으로 찜찜함을 느끼게 할 수 있습니다.
소셜 서비스에서도 서비스 탈퇴 시 연결 끊기를 수행할 것을 권고하고 있습니다.
close: [URECA-58] Feat: 회원탈퇴 - 소셜 로그인 끊기 #34
💬 공유사항 to 리뷰어
비고
Summary by CodeRabbit
새로운 기능
설정
개선
오류 코드
✏️ Tip: You can customize this high-level summary in your review settings.