[Feat] Auth API 추가 - #19
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 13 minutes and 44 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesJWT 기반 Auth API 구현
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant AuthController as AuthController
participant AuthService as AuthService
participant UserRepository as UserRepository
participant JwtProvider as JwtProvider
participant RefreshTokenRepository as RefreshTokenRepository
Client->>AuthController: POST /api/auth/signup
AuthController->>AuthService: signup(SignupRequestDto)
AuthService->>UserRepository: existsByEmail(email)
AuthService->>UserRepository: save(User)
Client->>AuthController: POST /api/auth/login
AuthController->>AuthService: login(LoginRequestDto)
AuthService->>UserRepository: findByEmail(email)
AuthService->>JwtProvider: generateAccessToken / generateRefreshToken
AuthService->>RefreshTokenRepository: save(refreshToken)
Client->>AuthController: POST /api/auth/reissue
AuthController->>AuthService: reissue(refreshToken)
AuthService->>JwtProvider: validateToken / extractUserId
AuthService->>RefreshTokenRepository: compareAndDelete(userId, refreshToken)
AuthService->>RefreshTokenRepository: save(newRefreshToken)
Client->>AuthController: POST /api/auth/logout
AuthController->>AuthService: logout(userId)
AuthService->>RefreshTokenRepository: deleteByUserId
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/main/java/com/ilson/spotwork/common/config/SwaggerConfig.java (1)
16-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHTTP bearer 타입에서 불필요한
.name()설정20번째 줄의
.name("Authorization")은 HTTP bearer 타입의 SecurityScheme에서는 불필요합니다. OpenAPI 3 스펙에서 bearer 인증은 Authorization 헤더를 암묵적으로 사용하며,name필드는 apiKey 타입의 SecurityScheme에서만 헤더/쿼리/쿠키 이름을 지정하는 데 사용됩니다.♻️ 제안하는 수정사항
SecurityScheme securityScheme = new SecurityScheme() .type(SecurityScheme.Type.HTTP) .scheme("bearer") - .bearerFormat("JWT") - .name("Authorization"); + .bearerFormat("JWT");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/ilson/spotwork/common/config/SwaggerConfig.java` around lines 16 - 20, The SecurityScheme configuration in the SwaggerConfig class contains an unnecessary .name("Authorization") call on the securityScheme object. For HTTP bearer type authentication, the Authorization header is used implicitly per OpenAPI 3 specification, and the name field is only applicable for apiKey type SecurityScheme. Remove the .name("Authorization") method call from the SecurityScheme chain, keeping only type(), scheme(), and bearerFormat() method calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/ilson/spotwork/domain/auth/service/AuthService.java`:
- Line 45: The log statements in AuthService class at both line 45 (in the
signup completion log) and line 71 are logging email addresses as plain text
PII, which creates a security and privacy risk. Replace the direct logging of
request.getEmail() in both the log.info calls with a masked or minimized version
of the email identifier, such as using only the domain portion, a hash, or a
different non-sensitive identifier, to protect personal information from
exposure in logs.
- Around line 87-102: The refresh token rotation logic in AuthService has a race
condition because token validation and storage are not atomic operations. The
current flow retrieves the stored token, compares it, and then saves the new
token as separate steps, allowing concurrent requests with the same refresh
token to both pass validation. Replace the separate find, equality check, and
save operations with a single atomic compare-and-set operation in the
refreshTokenRepository, such as implementing a rotateIfMatch method that
validates the provided refresh token equals the stored token and only then
atomically updates it to the new token. This ensures that only one concurrent
request can successfully rotate the token, preventing multiple valid tokens from
being issued simultaneously.
- Around line 32-44: There is a race condition where two concurrent requests can
both pass the existsByEmail check and then both attempt to save, causing the
database unique constraint on the email field to be violated and throw
DataIntegrityViolationException. Wrap the userRepository.save(user) call in a
try-catch block to catch DataIntegrityViolationException and throw
CustomException(ErrorCode.DUPLICATE_EMAIL) instead, ensuring the user receives a
clear and consistent error message regardless of whether the duplicate is caught
at the application level or database level.
- Around line 36-42: The User object creation in the AuthService class uses the
role value directly from the client request via request.getRole(), which creates
a security vulnerability allowing privilege escalation where users could
register with elevated roles like ADMIN. Replace the .role(request.getRole())
assignment in the User.builder() chain with a hardcoded default role value
(typically Role.USER or similar) to enforce the server-side permission and
prevent clients from specifying arbitrary roles during registration.
---
Nitpick comments:
In `@src/main/java/com/ilson/spotwork/common/config/SwaggerConfig.java`:
- Around line 16-20: The SecurityScheme configuration in the SwaggerConfig class
contains an unnecessary .name("Authorization") call on the securityScheme
object. For HTTP bearer type authentication, the Authorization header is used
implicitly per OpenAPI 3 specification, and the name field is only applicable
for apiKey type SecurityScheme. Remove the .name("Authorization") method call
from the SecurityScheme chain, keeping only type(), scheme(), and bearerFormat()
method calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 52a9e3af-0017-4148-a515-53c7406c2667
📒 Files selected for processing (12)
src/main/java/com/ilson/spotwork/common/config/SwaggerConfig.javasrc/main/java/com/ilson/spotwork/common/entity/BaseEntity.javasrc/main/java/com/ilson/spotwork/domain/auth/controller/AuthController.javasrc/main/java/com/ilson/spotwork/domain/auth/dto/LoginRequestDto.javasrc/main/java/com/ilson/spotwork/domain/auth/dto/SignupRequestDto.javasrc/main/java/com/ilson/spotwork/domain/auth/dto/TokenResponseDto.javasrc/main/java/com/ilson/spotwork/domain/auth/service/AuthService.javasrc/main/java/com/ilson/spotwork/domain/user/entity/User.javasrc/main/java/com/ilson/spotwork/domain/user/repository/UserRepository.javasrc/main/java/com/ilson/spotwork/domain/user/service/UserService.javasrc/main/java/com/ilson/spotwork/infra/security/SecurityConfig.javasrc/main/java/com/ilson/spotwork/infra/security/jwt/JwtAuthenticationFilter.java
3 tasks
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
1 participant
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.
관련 이슈
closes #18
구현 내용
변경 사항
체크리스트
Summary by CodeRabbit
Release Notes