Conversation
Contributor
WalkthroughOAuth 로그인 성공 핸들러가 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler as OAuth2AuthenticationSuccessHandler
participant Service as OAuthLoginPolicyService
participant UserRepo as UserRepository
participant TokenRepo as RefreshTokenRepository
participant JwtHandler
Client->>Handler: OAuth2 인증 성공
Handler->>Service: issueTokenIfNoActiveSession(jwtUserClaim)
Service->>UserRepo: findByIdForUpdate(userId)<br/>(비관적 잠금)
UserRepo-->>Service: User (잠금 획득)
Service->>TokenRepo: findByUserId(userId)
TokenRepo-->>Service: RefreshToken (또는 null)
alt 활성 세션 존재 (토큰 미만료)
Service-->>Handler: Optional.empty()
Handler-->>Client: 리다이렉트 (error=already_logged_in)
else 활성 세션 없음 또는 만료
Service->>TokenRepo: delete(oldToken)
Service->>JwtHandler: issueToken(accessToken, refreshToken)
JwtHandler-->>Service: Token
Service-->>Handler: Optional.of(Token)
Handler-->>Client: 리다이렉트 (accessToken, refreshToken)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/gpt/geumpumtabackend/global/oauth/service/OAuthLoginPolicyService.java`:
- Around line 19-30: Add `@Transactional`(readOnly = true) at the class level of
OAuthLoginPolicyService to make read-only the default transaction behavior, and
keep the existing `@Transactional` on the write method issueTokenIfNoActiveSession
so it overrides the class-level readOnly setting for write operations; this
ensures future read-only methods follow the correct default while the
token-issuing method still executes a writable transaction.
In `@src/main/java/com/gpt/geumpumtabackend/user/repository/UserRepository.java`:
- Around line 32-34: The current findByIdForUpdate method locks rows for any
user ID but does not exclude soft-deleted users (deletedAt), allowing
masked/withdrawn users to obtain tokens; modify the method/query to filter out
soft-deleted records (e.g., add "AND u.deletedAt IS NULL" to the JPQL) or rely
on the BaseEntity soft-delete setup (ensure User extends BaseEntity annotated
with `@SQLDelete` and `@Where`(clause = "deleted_at IS NULL") so the
PESSIMISTIC_WRITE query respects soft deletes), and update the repository method
findByIdForUpdate to use the non-deleted constraint so masked users are not
returned.
In
`@src/test/java/com/gpt/geumpumtabackend/unit/oauth/service/OAuthLoginPolicyServiceTest.java`:
- Around line 25-26: The test class OAuthLoginPolicyServiceTest currently uses
`@ExtendWith`(MockitoExtension.class) which bypasses shared test setup; change the
class to extend BaseUnitTest instead (remove the explicit `@ExtendWith`
annotation) so it inherits common test configuration and helpers from
BaseUnitTest; ensure imports and any Mockito/AssertJ usage remain compatible
after switching the class signature to extend BaseUnitTest.
- Around line 44-101: Add a test in OAuthLoginPolicyServiceTest to cover the "no
existing refresh token" branch: mock userRepository.findByIdForUpdate(userId) to
return Optional.of(mock(User.class)) and
refreshTokenRepository.findByUserId(userId) to return Optional.empty(), then
call oAuthLoginPolicyService.issueTokenIfNoActiveSession(claim) and assert the
returned Optional contains the Token produced by jwtHandler.createTokens(claim);
also verify jwtHandler.createTokens(claim) was called and
refreshTokenRepository.deleteByUserId(userId) was never called. This ensures the
issueTokenIfNoActiveSession path for new logins is tested.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96bbe7a8-4fbc-48be-9198-200232bd5e9f
📒 Files selected for processing (4)
src/main/java/com/gpt/geumpumtabackend/global/oauth/handler/OAuth2AuthenticationSuccessHandler.javasrc/main/java/com/gpt/geumpumtabackend/global/oauth/service/OAuthLoginPolicyService.javasrc/main/java/com/gpt/geumpumtabackend/user/repository/UserRepository.javasrc/test/java/com/gpt/geumpumtabackend/unit/oauth/service/OAuthLoginPolicyServiceTest.java
src/main/java/com/gpt/geumpumtabackend/global/oauth/service/OAuthLoginPolicyService.java
Show resolved
Hide resolved
src/main/java/com/gpt/geumpumtabackend/user/repository/UserRepository.java
Show resolved
Hide resolved
src/test/java/com/gpt/geumpumtabackend/unit/oauth/service/OAuthLoginPolicyServiceTest.java
Show resolved
Hide resolved
src/test/java/com/gpt/geumpumtabackend/unit/oauth/service/OAuthLoginPolicyServiceTest.java
Show resolved
Hide resolved
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
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.
🚀 1. 개요
📝 2. 주요 변경 사항
Optional<User> findByIdForUpdate(@Param("userId") Long userId);에 비관적 락을 추가해issueTokenIfNoActiveSession메서드 동시 호출을 차단합니다.{state에서 복원된 redirectUri}?error=already_logged_in경로로 리다이렉트가 진행됩니다/📸 3. 스크린샷 (API 테스트 결과)
Summary by CodeRabbit
리팩토링