feat(auth): 이메일 인증 및 리프레시 토큰 Redis 관리 구현 (#14, #15) - #28
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
crolvlee
changed the title
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds Redis-backed email verification (Gmail SMTP, 6-digit code, 5-min TTL) as a mandatory signup prerequisite. Implements Token Rotation for refresh tokens stored in Redis with TTL. Updates ChangesEmail Verification and Token Rotation Auth Flow
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant EmailVerificationService
participant AuthService
participant Redis
participant JavaMailSender
rect rgba(70, 130, 180, 0.5)
note over Client,JavaMailSender: Email Verification (pre-signup)
Client->>AuthController: POST /auth/email/send {email}
AuthController->>EmailVerificationService: sendCode(email)
EmailVerificationService->>Redis: SET email:verify:{email} code EX 300
EmailVerificationService->>JavaMailSender: send(SimpleMailMessage)
Client->>AuthController: POST /auth/email/verify {email, code}
AuthController->>EmailVerificationService: verifyCode(email, code)
EmailVerificationService->>Redis: GET email:verify:{email}
EmailVerificationService->>Redis: DEL email:verify:{email}
EmailVerificationService->>Redis: SET email:verified:{email} "true" EX 600
end
rect rgba(60, 179, 113, 0.5)
note over Client,Redis: Signup gated by verified flag
Client->>AuthController: POST /auth/signup {email, password}
AuthController->>AuthService: signup(request)
AuthService->>Redis: GET email:verified:{email}
AuthService-->>Client: 200 OK or 403 EMAIL_NOT_VERIFIED
end
rect rgba(210, 105, 30, 0.5)
note over Client,Redis: Login and Token Rotation
Client->>AuthController: POST /auth/login
AuthController->>AuthService: login(request)
AuthService->>Redis: SET refresh:{email} refreshToken EX ttl
AuthService-->>Client: {accessToken, refreshToken}
Client->>AuthController: POST /auth/refresh {refreshToken}
AuthController->>AuthService: refresh(refreshToken)
AuthService->>Redis: GET refresh:{email}
AuthService->>Redis: SET refresh:{email} newRefreshToken EX ttl
AuthService-->>Client: {accessToken, refreshToken}
Client->>AuthController: POST /auth/logout
AuthController->>AuthService: logout(email)
AuthService->>Redis: DEL refresh:{email}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 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📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/java/com/ssafy/salmanhae/service/auth/AuthService.java (1)
34-45:⚠️ Potential issue | 🟠 MajorAdd missing duplicate email check to signup method.
The signup method checks email verification status but does not verify whether the email is already registered. This allows duplicate email registration attempts to fail at the database layer instead of returning a proper API error response with
EMAIL_ALREADY_EXISTS.Add a check using the existing
userDao.findByEmail()method and throwApiException(ErrorCode.EMAIL_ALREADY_EXISTS)if the email is already registered:Proposed fix
public void signup(String email, String password, String nickname) { if (!Boolean.TRUE.toString().equals(redisTemplate.opsForValue().get("email:verified:" + email))) { throw new ApiException(ErrorCode.EMAIL_NOT_VERIFIED); } + if (userDao.findByEmail(email) != null) { + throw new ApiException(ErrorCode.EMAIL_ALREADY_EXISTS); + } User user = User.builder() .email(email)🤖 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 `@backend/src/main/java/com/ssafy/salmanhae/service/auth/AuthService.java` around lines 34 - 45, The signup method needs to check for duplicate email registration before creating a new user. After the existing email verification check, add a duplicate email check by calling userDao.findByEmail() with the email parameter and throw ApiException(ErrorCode.EMAIL_ALREADY_EXISTS) if it returns a non-null result. Place this check before the User object construction so duplicate emails are caught early and return the proper API error response instead of failing at the database layer.
🧹 Nitpick comments (3)
backend/src/main/java/com/ssafy/salmanhae/service/auth/EmailVerificationService.java (1)
28-38: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider rate limiting and error handling for email dispatch.
The
sendCodemethod lacks rate limiting, allowing potential abuse where an attacker could trigger excessive email sends to any address. Additionally,mailSender.send()can throwMailExceptionwhich propagates as an unhandled 500 error.Consider:
- Adding per-email rate limiting (e.g., check Redis for recent sends before allowing another)
- Wrapping mail send in try-catch to provide a cleaner error response
♻️ Example rate limiting approach
+ private static final String RATE_LIMIT_PREFIX = "email:rate:"; + public void sendCode(String email) { + String rateLimitKey = RATE_LIMIT_PREFIX + email; + if (Boolean.TRUE.equals(redisTemplate.hasKey(rateLimitKey))) { + throw new ApiException(ErrorCode.TOO_MANY_REQUESTS); + } + String code = String.format("%06d", new SecureRandom().nextInt(1_000_000)); redisTemplate.opsForValue().set(VERIFY_PREFIX + email, code, Duration.ofMinutes(5)); + redisTemplate.opsForValue().set(rateLimitKey, "1", Duration.ofMinutes(1)); SimpleMailMessage message = new SimpleMailMessage(); // ... - mailSender.send(message); + try { + mailSender.send(message); + } catch (MailException e) { + redisTemplate.delete(VERIFY_PREFIX + email); + throw new ApiException(ErrorCode.EMAIL_SEND_FAILED); + } }🤖 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 `@backend/src/main/java/com/ssafy/salmanhae/service/auth/EmailVerificationService.java` around lines 28 - 38, The sendCode method needs two improvements: First, add rate limiting by checking Redis before sending a new code to prevent abuse by checking if a recent send exists within a time window (e.g., 1 minute) using the VERIFY_PREFIX pattern and returning early if found. Second, wrap the mailSender.send() call in a try-catch block to handle MailException gracefully and prevent unhandled 500 errors, allowing you to log the error and throw or return a more appropriate response to the caller.backend/src/main/java/com/ssafy/salmanhae/controller/auth/AuthController.java (1)
36-39: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueAdd null guard for defensive safety.
While
@AuthenticationPrincipalshould always resolve to a non-null user on authenticated endpoints, adding a null check prevents a potential NPE if the filter chain ever misbehaves.🛡️ Optional defensive check
`@PostMapping`("/logout") public ResponseEntity<ApiResponse<Void>> logout(`@AuthenticationPrincipal` User user) { + if (user == null) { + throw new ApiException(ErrorCode.UNAUTHORIZED); + } authService.logout(user.getEmail()); return ResponseEntity.ok(ApiResponse.ok()); }🤖 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 `@backend/src/main/java/com/ssafy/salmanhae/controller/auth/AuthController.java` around lines 36 - 39, The logout method in AuthController lacks a null guard for the user parameter, which could lead to a potential NPE if the authentication filter chain misbehaves. Add a null check at the beginning of the logout method to verify that the user parameter is not null before calling authService.logout(user.getEmail()), and return an appropriate error response if the user is null.backend/src/main/resources/application.properties (1)
4-33: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winExpand the required-env note.
The
Required:comment only lists the Supabase DB trio, butjwt.secretand the mail credentials below also have no fallback. Please either enumerate the full env contract here or point readers to the backend.envexample.🤖 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 `@backend/src/main/resources/application.properties` around lines 4 - 33, The Required comment at the beginning of the file only lists the Supabase database environment variables (SUPABASE_DB_URL, SUPABASE_DB_USERNAME, SUPABASE_DB_PASSWORD) but omits other mandatory environment variables that lack fallback values. Update the comment to either enumerate all required environment variables including JWT_SECRET, MAIL_USERNAME, and MAIL_PASSWORD, or alternatively update it to reference a .env.example file in the backend directory that documents the complete environment configuration contract for developers.
🤖 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 `@backend/src/main/java/com/ssafy/salmanhae/common/exception/ErrorCode.java`:
- Around line 8-15: The ErrorCode enum contains a duplicate constant declaration
of PROPERTY_NOT_FOUND appearing twice in the enum body. Remove the duplicate
PROPERTY_NOT_FOUND constant (the second occurrence at line 15) to ensure all
enum constant names are unique, which is required by Java language
specification. Since both declarations are identical with the same HttpStatus
and message, keeping only one will resolve the compilation failure.
In
`@backend/src/main/java/com/ssafy/salmanhae/model/dto/auth/EmailSendRequest.java`:
- Around line 5-7: The email field in the EmailSendRequest class lacks
validation annotations, allowing null, blank, or invalid email formats to bypass
validation at the DTO boundary. Add appropriate validation annotations to the
email field (such as `@NotBlank` to ensure the field is not empty and `@Email` to
validate the email format) to ensure request validation happens early and
returns clean 4xx responses instead of allowing invalid data to reach the
service layer.
In
`@backend/src/main/java/com/ssafy/salmanhae/model/dto/auth/EmailVerifyRequest.java`:
- Around line 5-9: The EmailVerifyRequest class lacks field validation
constraints, allowing malformed payloads with blank or invalid email and code
values to be accepted. Add validation annotations to the email field (such as
`@NotBlank` and `@Email` to ensure non-empty valid email format) and to the code
field (such as `@NotBlank` to ensure non-empty code). These annotations will cause
Spring to validate the request at binding time and automatically return 4xx
validation errors for invalid payloads before they reach service logic.
In
`@backend/src/main/java/com/ssafy/salmanhae/model/dto/auth/RefreshRequest.java`:
- Around line 5-7: The RefreshRequest class currently has no validation
constraints on the refreshToken field, allowing empty or blank values to pass
through to the controller. Add the `@NotBlank` validation annotation to the
refreshToken field in the RefreshRequest DTO so that Spring will automatically
validate the input at the request level and return a 4xx error response for
empty or blank refresh tokens before they reach the controller business logic.
In `@backend/src/test/java/com/ssafy/salmanhae/service/auth/AuthServiceTest.java`:
- Line 95: In the AuthServiceTest class, the line with
when(jwtUtil.getEmail(REFRESH_TOKEN)).thenThrow(ExpiredJwtException.class) is
attempting to throw the exception class directly, but ExpiredJwtException
requires constructor parameters (Header, Claims, String) and has no no-arg
constructor. Replace the class reference with an explicit instance of
ExpiredJwtException by creating a new instance with the required parameters and
pass that instance to thenThrow() instead of the class.
In
`@backend/src/test/java/com/ssafy/salmanhae/service/auth/EmailVerificationServiceTest.java`:
- Around line 48-53: In the sendCode_savesCodeToRedisAndSendsEmail test method,
replace the anyString() argument in the verify(valueOps).set() call with an
ArgumentCaptor to capture the actual verification code value, then add an
assertion to verify that the captured code matches the regex pattern for exactly
6 digits (^\d{6}$). This ensures the sendCode method generates codes in the
correct format rather than accepting any arbitrary string.
In `@docs/10_SECURITY_POLICY.md`:
- Around line 15-29: The markdown file contains fenced code blocks without
language tags, which violates the markdownlint MD040 rule. Add language
identifiers to all bare code fences throughout the file - specifically at the
blocks starting around lines 15, 33, 43, 53, and 64. For each fenced code block
marked with just ```, add an appropriate language tag such as ```text or ```http
immediately after the opening backticks to indicate the code language and ensure
the documentation passes linting validation.
---
Outside diff comments:
In `@backend/src/main/java/com/ssafy/salmanhae/service/auth/AuthService.java`:
- Around line 34-45: The signup method needs to check for duplicate email
registration before creating a new user. After the existing email verification
check, add a duplicate email check by calling userDao.findByEmail() with the
email parameter and throw ApiException(ErrorCode.EMAIL_ALREADY_EXISTS) if it
returns a non-null result. Place this check before the User object construction
so duplicate emails are caught early and return the proper API error response
instead of failing at the database layer.
---
Nitpick comments:
In
`@backend/src/main/java/com/ssafy/salmanhae/controller/auth/AuthController.java`:
- Around line 36-39: The logout method in AuthController lacks a null guard for
the user parameter, which could lead to a potential NPE if the authentication
filter chain misbehaves. Add a null check at the beginning of the logout method
to verify that the user parameter is not null before calling
authService.logout(user.getEmail()), and return an appropriate error response if
the user is null.
In
`@backend/src/main/java/com/ssafy/salmanhae/service/auth/EmailVerificationService.java`:
- Around line 28-38: The sendCode method needs two improvements: First, add rate
limiting by checking Redis before sending a new code to prevent abuse by
checking if a recent send exists within a time window (e.g., 1 minute) using the
VERIFY_PREFIX pattern and returning early if found. Second, wrap the
mailSender.send() call in a try-catch block to handle MailException gracefully
and prevent unhandled 500 errors, allowing you to log the error and throw or
return a more appropriate response to the caller.
In `@backend/src/main/resources/application.properties`:
- Around line 4-33: The Required comment at the beginning of the file only lists
the Supabase database environment variables (SUPABASE_DB_URL,
SUPABASE_DB_USERNAME, SUPABASE_DB_PASSWORD) but omits other mandatory
environment variables that lack fallback values. Update the comment to either
enumerate all required environment variables including JWT_SECRET,
MAIL_USERNAME, and MAIL_PASSWORD, or alternatively update it to reference a
.env.example file in the backend directory that documents the complete
environment configuration contract for developers.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 63741900-6eb0-4dcc-8267-48df7affe477
📒 Files selected for processing (24)
AGENTS.mdCLAUDE.mdbackend/.env.examplebackend/pom.xmlbackend/src/main/java/com/ssafy/salmanhae/common/exception/ErrorCode.javabackend/src/main/java/com/ssafy/salmanhae/config/SecurityConfig.javabackend/src/main/java/com/ssafy/salmanhae/controller/auth/AuthController.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/auth/EmailSendRequest.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/auth/EmailVerifyRequest.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/auth/RefreshRequest.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/auth/RefreshResponse.javabackend/src/main/java/com/ssafy/salmanhae/service/auth/AuthService.javabackend/src/main/java/com/ssafy/salmanhae/service/auth/EmailVerificationService.javabackend/src/main/resources/application.propertiesbackend/src/test/java/com/ssafy/salmanhae/controller/auth/AuthControllerTest.javabackend/src/test/java/com/ssafy/salmanhae/controller/auth/AuthHttpIntegrationTest.javabackend/src/test/java/com/ssafy/salmanhae/service/auth/AuthServiceTest.javabackend/src/test/java/com/ssafy/salmanhae/service/auth/EmailVerificationServiceTest.javabackend/src/test/resources/application-test.propertiesdocs/02_ARCHITECTURE.mddocs/03_ADR.mddocs/08_API_SPEC.mddocs/10_SECURITY_POLICY.mdfrontend/.env.example
💤 Files with no reviewable changes (2)
- backend/.env.example
- frontend/.env.example
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
변경 내용
POST /api/v1/auth/email/send— Gmail SMTP로 이메일 인증 코드(6자리) 발송, Redis에 5분 TTL로 저장POST /api/v1/auth/email/verify— 인증 코드 검증 후 완료 플래그를 Redis에 10분 TTL로 저장POST /api/v1/auth/signup— 이메일 인증 완료 여부 체크 추가 (미완료 시 403)POST /api/v1/auth/refresh— 리프레시 토큰 갱신 구현, Token Rotation 적용 (액세스 + 리프레시 모두 재발급)POST /api/v1/auth/login— 로그인 시 리프레시 토큰을 Redis에 저장 (refresh:{email}, TTL 7일)POST /api/v1/auth/logout— Redis에서 리프레시 토큰 삭제로 실제 무효화 구현application-local.properties제거,backend/.env·frontend/.env방식으로 환경변수 통일origin/develop머지 및application.properties충돌 해결 (AI agent 설정 추가)연결 이슈
closes #14
closes #15
테스트
EmailVerificationServiceTest,AuthServiceTest)AuthControllerTest,AuthHttpIntegrationTest) — 총 30개리뷰 포인트
Summary by CodeRabbit
Release Notes