[CHORE] 회원가입 로직 수정 #392
Merged
[CHORE] 회원가입 로직 수정 #392
+376
−57
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
Closed
1 task
📝 WalkthroughWalkthrough카카오 로그인에서 신규 사용자는 임시 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OAuthCtrl as OAuthController
participant OAuthSvc as OAuthService
participant KakaoAPI as Kakao API
participant UserRepo as UserRepository
participant SessionRepo as SignupSessionRepository
Client->>OAuthCtrl: POST /kakao-login (accessCode)
OAuthCtrl->>OAuthSvc: kakaoLogin(accessCode,...)
OAuthSvc->>KakaoAPI: 사용자 정보 조회
KakaoAPI-->>OAuthSvc: email, nickname, id
alt 기존 사용자
OAuthSvc->>UserRepo: findByEmailAndSocialType(email)
UserRepo-->>OAuthSvc: User 존재
OAuthSvc->>OAuthSvc: access/refresh 토큰 생성 & refreshToken 저장
OAuthSvc-->>OAuthCtrl: KakaoLoginResponse(isNewUser=false)
else 신규 사용자
OAuthSvc->>OAuthSvc: signupToken 생성
OAuthSvc->>SessionRepo: save(signupToken, SignupSession, TTL)
SessionRepo-->>OAuthSvc: 저장 완료
OAuthSvc-->>OAuthCtrl: KakaoLoginResponse(isNewUser=true, signupToken, prefill)
end
OAuthCtrl-->>Client: ApiResponse(KakaoLoginResponse)
sequenceDiagram
participant Client
participant UserCtrl as UserController
participant OAuthSvc as OAuthService
participant SessionRepo as SignupSessionRepository
participant UserRepo as UserRepository
participant CreditRepo as CreditRepository
Client->>UserCtrl: POST /sign-up (signupToken, name, gender, birthday)
UserCtrl->>OAuthSvc: signUpWithToken(token, name, gender, birthday, resp)
OAuthSvc->>SessionRepo: consume(signupToken)
SessionRepo-->>OAuthSvc: SignupSession(email, nickname)
OAuthSvc->>UserRepo: 이메일 중복 확인
OAuthSvc->>UserRepo: save(new User)
OAuthSvc->>CreditRepo: saveAll(list of N Credits)
CreditRepo-->>OAuthSvc: 저장 완료
OAuthSvc->>OAuthSvc: access/refresh 토큰 생성 및 쿠키/헤더 설정
OAuthSvc-->>UserCtrl: username
UserCtrl-->>Client: ApiResponse(username)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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: 6
🤖 Fix all issues with AI agents
In @src/main/java/or/sopt/houme/domain/user/entity/record/SignupSession.java:
- Around line 3-7: SignupSession's kakaoId can be null; add a compact canonical
constructor on the SignupSession record to validate kakaoId is non-null (e.g.,
Objects.requireNonNull(kakaoId, "kakaoId must not be null") or throw
IllegalArgumentException) so the record cannot be created with a null
identifier; update any call sites that might pass null to provide a proper id or
handle the validation exception accordingly.
In
@src/main/java/or/sopt/houme/domain/user/repository/SignupSessionRepository.java:
- Around line 37-43: The fallback using redisTemplate.opsForValue().get +
redisTemplate.delete is not atomic and allows a race where a token can be
consumed twice; update SignupSessionRepository.consume to remove the fallback
and instead either rethrow the exception or implement an atomic consume via
RedisScript/Lua: run a script that GETs the key and conditionally DELs it in one
call (use redisTemplate.execute with DefaultRedisScript and the LUA that returns
the value), then parse that returned value into SignupSession as before; ensure
you stop using the non-atomic get+delete sequence and only rely on the atomic
get-and-delete (or a retry strategy) so tokens cannot be used twice.
- Around line 48-50: In SignupSessionRepository where you do
objectMapper.convertValue(map, SignupSession.class), guard against conversion
failures by validating the map contains required keys and wrapping convertValue
in a try-catch that returns Optional.empty() on error; specifically check for
presence/non-null of mandatory fields expected by SignupSession before calling
objectMapper.convertValue, and if you prefer, catch
IllegalArgumentException/JsonMappingException around convertValue to log the
error and return Optional.empty() instead of propagating an exception.
- Around line 23-29: In SignupSessionRepository.save, don’t silently return on
invalid inputs; validate signupToken and session at the start of the save method
and throw an IllegalArgumentException (or a specific runtime exception) with a
clear message naming the invalid parameter (e.g., "signupToken is blank" or
"session is null") instead of returning silently; ensure the exception is thrown
from the save method so callers (and tests) can detect and handle the failure.
In @src/main/java/or/sopt/houme/domain/user/service/OAuthService.java:
- Around line 214-224: In OAuthService where you create and save initial credits
(using SIGN_UP_CREDIT_COUNT, Credit.builder and creditRepository.saveAll), don't
swallow the original exception; when catching Exception e and rethrowing
CreditException(ErrorCode.CREDIT_CREATE_EXCEPTION), pass e as the cause so the
original stacktrace is preserved (e.g., use a CreditException constructor that
accepts a cause or add one if missing) and update the throw to include that
cause.
🧹 Nitpick comments (5)
src/main/java/or/sopt/houme/domain/user/service/UserServiceImpl.java (1)
45-46: 가입 시 지급 크레딧 수 상수화, 좋습니다!매직 넘버 대신 상수로 선언한 거 깔끔하네요. 혹시 나중에 운영하면서 이 값을 조정할 가능성이 있다면
application.yml같은 설정 파일로 빼는 것도 고려해볼 만합니다.src/main/java/or/sopt/houme/domain/user/entity/record/SignupSession.java (1)
8-10: 팩토리 메서드가 단순 생성자 호출만 하고 있어요
of()메서드가 현재는 생성자를 그대로 호출하는 것 외에 별다른 로직이 없는데, 이런 경우 팩토리 메서드 패턴의 이점이 크지 않습니다.위에서 제안한 것처럼 검증 로직을 추가하거나, 아니면 팩토리 메서드 없이 생성자 직접 사용도 고려해볼 수 있을 것 같아요.
src/test/java/or/sopt/houme/domain/user/controller/UserControllerTest.java (1)
118-143: 테스트 커버리지를 좀 더 보강하면 좋을 것 같아요기본적인 성공 케이스는 잘 커버하고 있는데, 몇 가지 추가 검증이 있으면 더 견고할 것 같습니다:
signUpWithToken메서드가 실제로 호출되었는지 verify 체크- Swagger 문서에서 언급한 access-token 헤더와 refresh-token 쿠키 응답 검증
- 에러 케이스 테스트 (잘못된 토큰, 이메일 중복 등)
🧪 테스트 보강 예시
+import static org.mockito.Mockito.verify; @Test @DisplayName("POST /api/v1/sign-up 요청 시 소셜 회원가입이 처리된다") void socialSignUp_Success() throws Exception { given(oAuthService.signUpWithToken( any(String.class), any(String.class), any(Gender.class), any(LocalDate.class), any(HttpServletResponse.class) )).willReturn("테스트유저"); mockMvc.perform(post("/api/v1/sign-up") .contentType(org.springframework.http.MediaType.APPLICATION_JSON) .content(""" { "signupToken": "signup-token", "name": "테스트유저", "gender": "MALE", "birthday": "2000-01-01" } """)) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(200)) .andExpect(jsonPath("$.msg").value("응답 성공")) - .andExpect(jsonPath("$.data").value("테스트유저")); + .andExpect(jsonPath("$.data").value("테스트유저")) + .andExpect(header().exists("access-token")); + + verify(oAuthService).signUpWithToken( + any(String.class), + any(String.class), + any(Gender.class), + any(LocalDate.class), + any(HttpServletResponse.class) + ); } + +@Test +@DisplayName("잘못된 signupToken으로 회원가입 시도 시 400 에러") +void socialSignUp_InvalidToken() throws Exception { + given(oAuthService.signUpWithToken( + any(String.class), + any(String.class), + any(Gender.class), + any(LocalDate.class), + any(HttpServletResponse.class) + )).willThrow(new GeneralException(ErrorCode.SIGNUP_TOKEN_INVALID)); + + mockMvc.perform(post("/api/v1/sign-up") + .contentType(org.springframework.http.MediaType.APPLICATION_JSON) + .content(""" + { + "signupToken": "invalid-token", + "name": "테스트유저", + "gender": "MALE", + "birthday": "2000-01-01" + } + """)) + .andExpect(status().isBadRequest()); +}src/main/java/or/sopt/houme/domain/user/controller/UserController.java (1)
78-107: Gender/Birthday 검증 로직이 중복되고 있어요새로운 POST /sign-up 엔드포인트(78-107)와 기존 PATCH /sign-up 엔드포인트(56-76)에서 동일한 Gender, Birthday 검증 로직이 반복되고 있습니다.
이런 검증 로직은 헬퍼 메서드로 추출하면 유지보수가 편할 것 같아요.
♻️ 검증 로직 추출 제안
+ private Gender parseGender(String genderString) { + try { + return Gender.valueOf(genderString); + } catch (IllegalArgumentException e) { + throw new GeneralException(ErrorCode.NOT_VALID_EXCEPTION); + } + } + + private LocalDate parseBirthday(String birthdayString) { + try { + return LocalDate.parse(birthdayString); + } catch (IllegalArgumentException e) { + throw new GeneralException(ErrorCode.NOT_VALID_EXCEPTION); + } + } @PatchMapping(value = "/sign-up") @Operation(summary = "자체 회원가입 API") public ResponseEntity<ApiResponse<String>> updateUser(@AuthenticationPrincipal CustomUserDetails userDetails, @RequestBody @Valid CreateUserRequest createUserRequest) { - Gender gender; - LocalDate birthday; - - try { - gender = Gender.valueOf(createUserRequest.gender()); - } catch (IllegalArgumentException e){ - throw new GeneralException(ErrorCode.NOT_VALID_EXCEPTION); - } - try { - birthday = LocalDate.parse(createUserRequest.birthday()); - } catch (IllegalArgumentException e){ - throw new GeneralException(ErrorCode.NOT_VALID_EXCEPTION); - } + Gender gender = parseGender(createUserRequest.gender()); + LocalDate birthday = parseBirthday(createUserRequest.birthday()); String username = userService.updateUser(userDetails.getUser(), createUserRequest.name(), gender, birthday); return ResponseEntity.ok(ApiResponse.ok(username)); } @PostMapping(value = "/sign-up") @Operation(summary = "소셜 자체 회원가입 API", description = "카카오 소셜로그인 완료 후 발급된 signupToken(임시토큰)과 함께 호출하면 회원을 생성합니다. <br><br>" + "성공 시 access-token 헤더와 refresh-token 쿠키를 함께 반환합니다.") public ResponseEntity<ApiResponse<String>> signUp(@RequestBody @Valid SocialSignUpRequest signUpRequest, HttpServletResponse response) { - Gender gender; - LocalDate birthday; - - try { - gender = Gender.valueOf(signUpRequest.gender()); - } catch (IllegalArgumentException e) { - throw new GeneralException(ErrorCode.NOT_VALID_EXCEPTION); - } - try { - birthday = LocalDate.parse(signUpRequest.birthday()); - } catch (IllegalArgumentException e) { - throw new GeneralException(ErrorCode.NOT_VALID_EXCEPTION); - } + Gender gender = parseGender(signUpRequest.gender()); + LocalDate birthday = parseBirthday(signUpRequest.birthday()); String username = oAuthService.signUpWithToken( signUpRequest.signupToken(), signUpRequest.name(), gender, birthday, response ); return ResponseEntity.ok(ApiResponse.ok(username)); }src/main/java/or/sopt/houme/domain/user/controller/dto/SocialSignUpRequest.java (1)
15-20: Gender를 String 대신 enum 타입으로 받는 것을 고려해보세요.현재 String + Pattern 검증 방식도 동작하지만,
Genderenum을 직접 사용하면 타입 안전성이 높아지고 Jackson이 자동으로 역직렬화/검증해줘요. 유효하지 않은 값이 들어오면HttpMessageNotReadableException이 발생하니 별도 에러 핸들러에서 처리하면 됩니다.♻️ 제안하는 변경
- @NotBlank(message = "성별은 필수 입력값입니다.") - @Pattern( - regexp = "MALE|FEMALE|NONBINARY", - message = "성별은 MALE, FEMALE, NONBINARY 중 하나여야 해요." - ) - String gender, + @NotNull(message = "성별은 필수 입력값입니다.") + Gender gender,
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/or/sopt/houme/domain/user/controller/OAuthController.javasrc/main/java/or/sopt/houme/domain/user/controller/UserController.javasrc/main/java/or/sopt/houme/domain/user/controller/dto/KakaoLoginResponse.javasrc/main/java/or/sopt/houme/domain/user/controller/dto/SocialSignUpRequest.javasrc/main/java/or/sopt/houme/domain/user/entity/record/SignupSession.javasrc/main/java/or/sopt/houme/domain/user/repository/SignupSessionRepository.javasrc/main/java/or/sopt/houme/domain/user/service/OAuthService.javasrc/main/java/or/sopt/houme/domain/user/service/UserServiceImpl.javasrc/main/java/or/sopt/houme/global/api/ErrorCode.javasrc/main/java/or/sopt/houme/global/config/SecurityConfig.javasrc/test/java/or/sopt/houme/domain/user/controller/OAuthControllerTest.javasrc/test/java/or/sopt/houme/domain/user/controller/UserControllerTest.javasrc/test/java/or/sopt/houme/domain/user/service/OAuthServiceTest.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-28T05:49:49.354Z
Learnt from: gdbs1107
Repo: TEAM-HOUME/HOUME-SERVER PR: 226
File: src/main/java/or/sopt/houme/domain/admin/controller/AdminSSRController.java:55-65
Timestamp: 2025-08-28T05:49:49.354Z
Learning: gdbs1107의 admin 인증 구조는 WhiteListConfig에서 SSR 라우트(/admin/*)를 모두 화이트리스트 처리하여 JWTFilter를 우회하고, API 라우트(/api/v1/admin/*)만 JWTFilter를 통과하도록 설계됨. 이는 Admin과 User 엔티티 분리 문제를 해결하는 합리적인 아키텍처임.
Applied to files:
src/main/java/or/sopt/houme/global/config/SecurityConfig.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (11)
src/main/java/or/sopt/houme/domain/user/service/UserServiceImpl.java (1)
195-201: 배치 크레딧 생성 로직 좋습니다!
IntStream으로 5개 크레딧을 한 번에 만들고saveAll로 저장하는 방식이 개별save호출보다 훨씬 효율적이네요.혹시 중복 호출에 대해 걱정했는데, 임시 토큰이 사용 직후 Redis에서 즉시 삭제되는 구조(
getAndDelete)라서 같은 토큰으로 여러 번 호출할 수 없습니다. 그래서 중복 생성 걱정은 없을 것 같아요.src/main/java/or/sopt/houme/global/config/SecurityConfig.java (1)
102-102: LGTM! 새로운 소셜 회원가입 플로우를 위한 화이트리스트 설정이 적절합니다POST /api/v1/sign-up을 permitAll로 설정한 게 맞는 것 같아요. 기존 PATCH /sign-up은 인증이 필요하고, 새로운 POST /sign-up은 임시 토큰 기반이라 인증 없이 접근 가능해야 하니까요.
HTTP 메서드를 명시적으로 구분한 것도 좋습니다.
src/main/java/or/sopt/houme/domain/user/controller/UserController.java (1)
98-104: 구현 확인 완료 - 토큰이 제대로 설정되고 있습니다
signUpWithToken메서드에서 access-token은 헤더로 (line 231), refresh-token은CookieUtil.addSameSiteCookie()를 통해 쿠키로 (lines 233-241) 설정되고 있습니다. Swagger 문서의 명시사항이 정확히 구현되어 있네요.src/main/java/or/sopt/houme/global/api/ErrorCode.java (1)
35-38: 새 에러 코드 적절히 활용되고 있네요 👍SIGNUP_TOKEN_INVALID와 EMAIL_ALREADY_EXISTS가 OAuthService의 signUpWithToken 메서드에서 정확한 시점에 던져지고 있습니다:
- 임시 토큰 검증 실패 시 → SIGNUP_TOKEN_INVALID (line 191)
- 이메일 중복 체크 시 → EMAIL_ALREADY_EXISTS (line 197)
HTTP 400 상태 코드도 적절하고, 메시지도 명확해서 좋습니다.
src/test/java/or/sopt/houme/domain/user/controller/OAuthControllerTest.java (1)
74-91: LGTM!테스트가 새로운
KakaoLoginResponse구조를 잘 반영하고 있어요.isNewUser,signupToken,prefill필드에 대한 검증이 적절합니다.src/main/java/or/sopt/houme/domain/user/controller/OAuthController.java (1)
47-62: LGTM!API 문서와 반환 타입이 새로운 로그인 플로우에 맞게 잘 업데이트되었어요. 기존 회원/신규 회원 분기에 대한 설명도 명확합니다.
src/test/java/or/sopt/houme/domain/user/service/OAuthServiceTest.java (2)
93-133: LGTM!신규 회원 플로우 테스트가 잘 작성되었어요.
signupToken발급,SignupSession저장, 그리고 user/token 생성이 호출되지 않는 것까지 적절히 검증하고 있습니다.
135-175: 기존 회원 테스트도 잘 되어있어요.
isNewUserfalse 검증과 JWT 토큰 발급 플로우가 잘 테스트되고 있습니다.src/main/java/or/sopt/houme/domain/user/controller/dto/KakaoLoginResponse.java (1)
3-21: 깔끔한 DTO 설계네요!Record를 활용한 immutable DTO와 factory method 패턴이 잘 적용되었어요.
newUser()와existingUser()분리도 명확합니다.src/main/java/or/sopt/houme/domain/user/service/OAuthService.java (2)
125-131: PR 목표와 충돌: 이메일 없을 때 처리가 필요해요.PR description에서 "사용자가 이메일 공유를 거부하면 이메일이 없을 수 있다"고 언급했는데, 현재 코드는 이메일이 없으면
NOT_VALID_EXCEPTION을 던지고 있어요.이메일 없이도 가입할 수 있게 하려면 별도 처리 로직이 필요하고, 이메일을 필수로 유지한다면 PR description을 업데이트하는 게 좋겠습니다. 현재 이메일을 PK처럼 사용하는 구조 때문인 것 같은데, 이 부분은 별도 이슈로 다루시는 게 맞아 보여요.
139-152: 신규 회원 플로우 로직이 잘 구현되었어요.UUID 토큰 생성, SignupSession 저장, KakaoLoginResponse 반환까지 깔끔합니다. 기존에 빈 회원 레코드가 남는 문제를 잘 해결했네요.
src/main/java/or/sopt/houme/domain/user/entity/record/SignupSession.java
Show resolved
Hide resolved
src/main/java/or/sopt/houme/domain/user/repository/SignupSessionRepository.java
Show resolved
Hide resolved
src/main/java/or/sopt/houme/domain/user/repository/SignupSessionRepository.java
Show resolved
Hide resolved
src/main/java/or/sopt/houme/domain/user/repository/SignupSessionRepository.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/or/sopt/houme/domain/user/service/UserServiceImplTest.java (1)
377-378: 배치 크레딧 생성 검증 로직 확인
saveAll로 5개의 크레딧이 배치 저장되는지 검증하는 로직이 추가되었네요. 새로운 가입 플로우와 잘 맞습니다.다만 한 가지 제안:
- Line 378의
5가 하드코딩되어 있는데,OAuthService의SIGN_UP_CREDIT_COUNT같은 상수를 참조하면 나중에 크레딧 개수가 변경될 때 테스트가 자동으로 따라갈 수 있어요. 지금은 문제없지만 유지보수 측면에서 고려해볼만 합니다.♻️ 상수 참조로 개선 (선택사항)
verify(creditRepository, times(1)) - .saveAll(argThat(credits -> credits instanceof java.util.Collection<?> c && c.size() == 5)); + .saveAll(argThat(credits -> credits instanceof java.util.Collection<?> c && c.size() == OAuthService.SIGN_UP_CREDIT_COUNT));참고:
OAuthService.SIGN_UP_CREDIT_COUNT가 접근 가능한 상수라고 가정한 예시입니다. 실제 구현에 맞게 조정해주세요.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/or/sopt/houme/domain/user/service/UserServiceImplTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
src/test/java/or/sopt/houme/domain/user/service/UserServiceImplTest.java (2)
37-38: Import 추가 확인 완료
anyList()와argThat()import 추가가 배치 저장 검증을 위해 필요한 변경이네요. 문제없습니다.
407-407: 예외 시나리오 테스트 업데이트 확인
save에서saveAll로 변경되어 배치 저장 실패 케이스를 제대로 테스트하고 있습니다. 👍
Collaborator
📝 Code Coverage
|
Collaborator
Author
|
@maehwasoo 확인 부탁드립니다~ |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/main/java/or/sopt/houme/domain/user/service/OAuthService.java:
- Around line 145-150: SignupSession currently stores a kakaoId via
SignupSession.of(...) but that value is never used in signUpWithToken and User
has no socialId field; remove the unused kakaoId from the signup flow by
deleting the kakaoId parameter/property from SignupSession (and its factory
method), update signupSessionRepository.save calls to the new signature, and
clean up any DTOs/constructors that accepted kakaoId (including the call site in
OAuthService where SignupSession.of(...) is invoked); alternatively, if you
intend to persist a social id, instead wire that value into the User creation
flow (e.g., add socialId/socialType to User and consume the stored kakaoId in
signUpWithToken) — pick one approach and make consistent changes across
SignupSession.of, signupSessionRepository.save, and signUpWithToken.
- Around line 196-198: There is a race condition between the
existsByEmail(check) and userRepository.save(...) in OAuthService; wrap the save
flow to catch database uniqueness violations (DataIntegrityViolationException)
and translate them into the same business error (throw new
UserException(ErrorCode.EMAIL_ALREADY_EXISTS)). Specifically, keep the
existsByEmail check but also surround the call that persists the entity (the
method that invokes userRepository.save(...) in OAuthService) with a try/catch
for DataIntegrityViolationException (or the underlying constraint exception) and
rethrow UserException(ErrorCode.EMAIL_ALREADY_EXISTS) so concurrent creates
return the proper client-facing error.
🧹 Nitpick comments (3)
src/main/java/or/sopt/houme/global/config/SecurityConfig.java (1)
103-103: LGTM! 회원가입 엔드포인트에 대한 public 접근 설정이 올바르게 되어 있어요.POST
/api/v1/sign-up엔드포인트가 임시 signup token을 Redis에서 검증하는 방식이라 인증 없이 접근 가능해야 하는 게 맞습니다.HttpMethod.POST로 한정한 것도 좋네요.다만 다른 화이트리스트 경로들은
WhiteListConfig에서 관리하고 있는데, 이 엔드포인트만 인라인으로 추가되어 있어요. 일관성 측면에서WhiteListConfig.userWhiteList()같은 곳에 추가하는 것도 고려해볼 수 있을 것 같아요. (큰 문제는 아니에요!)♻️ WhiteListConfig로 이동하는 방안 (선택사항)
WhiteListConfig에 회원가입 관련 화이트리스트 메서드를 추가하거나 기존userWhiteList()에 포함시키면 경로 관리가 한 곳에서 가능해집니다:// WhiteListConfig.java에 추가 public static List<String> signupWhitelist() { return List.of("/api/v1/sign-up"); }http.authorizeHttpRequests((auth)->auth - .requestMatchers(HttpMethod.POST, "/api/v1/sign-up").permitAll() + .requestMatchers(HttpMethod.POST, WhiteListConfig.signupWhitelist().toArray(new String[0])).permitAll() .requestMatchers(WhiteListConfig.swaggerWhitelist().toArray(new String[0])).permitAll()src/main/java/or/sopt/houme/domain/user/service/OAuthService.java (2)
125-131: 에러 코드가 너무 일반적이에요.이메일이 없을 때
NOT_VALID_EXCEPTION을 던지는데, 이게 너무 generic해서 클라이언트가 무슨 문제인지 파악하기 어려울 수 있어요. 이메일 동의가 필수라면 전용 에러 코드(예:EMAIL_CONSENT_REQUIRED)를 만드는 게 디버깅이나 사용자 안내에 도움이 될 것 같아요.
214-224: Exception 로깅이 빠져있어요.generic
Exception을 catch하고 있는데, 원본 예외를 로깅하지 않으면 크레딧 생성 실패 시 원인 파악이 어려워요. 최소한 에러 로그는 남겨두는 게 좋을 것 같아요.♻️ 예외 로깅 추가
try { List<Credit> newCredits = IntStream.range(0, SIGN_UP_CREDIT_COUNT) .mapToObj(i -> Credit.builder() .status(CreditStatus.ACTIVE) .user(savedUser) .build()) .toList(); creditRepository.saveAll(newCredits); } catch (Exception e) { + log.error("크레딧 생성 실패 - userId: {}", savedUser.getId(), e); throw new CreditException(ErrorCode.CREDIT_CREATE_EXCEPTION); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/or/sopt/houme/domain/user/service/OAuthService.javasrc/main/java/or/sopt/houme/domain/user/service/UserServiceImpl.javasrc/main/java/or/sopt/houme/global/config/SecurityConfig.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/or/sopt/houme/domain/user/service/UserServiceImpl.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-28T05:49:49.354Z
Learnt from: gdbs1107
Repo: TEAM-HOUME/HOUME-SERVER PR: 226
File: src/main/java/or/sopt/houme/domain/admin/controller/AdminSSRController.java:55-65
Timestamp: 2025-08-28T05:49:49.354Z
Learning: gdbs1107의 admin 인증 구조는 WhiteListConfig에서 SSR 라우트(/admin/*)를 모두 화이트리스트 처리하여 JWTFilter를 우회하고, API 라우트(/api/v1/admin/*)만 JWTFilter를 통과하도록 설계됨. 이는 Admin과 User 엔티티 분리 문제를 해결하는 합리적인 아키텍처임.
Applied to files:
src/main/java/or/sopt/houme/global/config/SecurityConfig.java
📚 Learning: 2025-06-28T04:17:44.481Z
Learnt from: gdbs1107
Repo: TEAM-HOUME/HOUME-SERVER PR: 8
File: src/main/java/or/sopt/houme/domain/user/service/JWTService.java:99-110
Timestamp: 2025-06-28T04:17:44.481Z
Learning: Redis 기반 RefreshTokenRepository에서 saveRefreshToken 메서드는 같은 userId로 호출 시 기존 값을 덮어쓰므로, 리프레시 토큰 갱신 시 순서를 "새 토큰 저장 -> 기존 토큰 삭제"로 하면 새로 저장한 토큰이 삭제되는 문제가 발생함. try/catch를 사용하여 토큰 생성/저장 로직을 감싸는 것이 안전한 해결책임.
Applied to files:
src/main/java/or/sopt/houme/domain/user/service/OAuthService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (3)
src/main/java/or/sopt/houme/domain/user/service/OAuthService.java (3)
49-66: LGTM!새로운 필드들과 설정값들이 잘 추가되었네요.
signupTokenTtlSeconds기본값 600초(10분)도 회원가입 플로우에 적절한 TTL이에요.
179-186: LGTM!기존 테스트와 호출자를 위한 backward-compatible overload가 잘 유지되어 있네요.
226-244: LGTM!JWT 토큰 발급과 쿠키 설정 로직이 기존
kakaoLogin메서드와 일관성 있게 구현되어 있어요.@Transactional경계도 적절하게 설정되어 있네요.
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.
📣 Related Issue
📝 Summary
문제 상황
의 플로우로 회원가입을 진행했었습니다. 하지만 현재 소셜로그인이 완료되고 추가정보를 기입하는 페이지에서 사용자가 이탈하게 되면 그대로 빈 사용자가 남아버리는 문제를 발견했습니다.
해결방안
발생 할 수 있는 문제
클라이언트의 개발/배포 서버가 분리되는대로 머지하도록 하겠습니다. 그 전까지 리뷰 남겨주시면 감사하겠습니다.
-> PR을 수정합니다. 회원의 이메일은 필수 동의란으로 반드시 받아야하는 값이기 때문에 문제가 되지 않습니다.
🙏 to Client
로그인은 똑같이 아래의 엔드포인트에서 시작해주시면 됩니다.
대신 반환값이 변경되었습니다.
궁금한점 있으면 말해주세요~!
Summary by CodeRabbit
새 기능
버그 수정 / 개선
테스트
✏️ Tip: You can customize this high-level summary in your review settings.