[BE-Feat] 로그인 로직에 리프레시 토큰 추가 #342
Conversation
WalkthroughThis pull request updates the authentication flow by modifying login functionality to include refresh token cookie management and introducing a refresh token endpoint. The changes propagate through the controller, service, JWT provider, and testing components, and include new configurations and utility classes for managing cookies and refresh token properties. Additionally, error handling for refresh tokens has been enhanced with a new error code, and the JWT filter has been adjusted for the refresh endpoint. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant AC as AuthController
participant AS as AuthService
participant JP as JwtProvider
C ->> AC: POST /login (credentials)
AC ->> AS: login(code, response)
AS ->> JP: createAccessToken & createRefreshToken
JP -->> AS: TokenDto {oAuthResponse, refreshTokenCookie}
AS -->> AC: TokenDto
AC ->> C: Response with OAuthResponse and refresh token cookie
sequenceDiagram
participant C as Client
participant AC as AuthController
participant AS as AuthService
participant CU as CookieUtil
participant JP as JwtProvider
C ->> AC: POST /api/v1/refresh-token (with cookie)
AC ->> AS: refreshToken(request, response)
AS ->> CU: getRefreshToken(cookies)
CU -->> AS: refresh token value
AS ->> JP: validate token & create new access token
JP -->> AS: TokenDto {new oAuthResponse, new refreshTokenCookie}
AS -->> AC: TokenDto
AC ->> C: Response with new OAuthResponse and refreshed cookie
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Definitions (1)backend/src/main/java/endolphin/backend/domain/auth/AuthService.java (1)
🔇 Additional comments (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (5)
backend/src/main/java/endolphin/backend/domain/auth/dto/TokenDto.java (1)
5-10: Consider adding JavaDoc documentation to the TokenDto classThe record structure looks good for encapsulating both the access token (via OAuthResponse) and refresh token cookie. Adding JavaDoc comments would improve the maintainability by documenting the purpose of this DTO and explaining each field, especially for other developers who might use this class in the future.
package endolphin.backend.domain.auth.dto; import jakarta.servlet.http.Cookie; +/** + * Data transfer object that encapsulates authentication tokens. + * Contains the OAuth response (with access token) and the refresh token cookie. + */ public record TokenDto( + /** + * Contains the OAuth response including the access token + */ OAuthResponse oAuthResponse, + /** + * The HTTP cookie containing the refresh token + */ Cookie refreshTokenCookie ) { }backend/src/main/java/endolphin/backend/global/util/CookieUtil.java (1)
24-36: Consider enhancing cookie retrieval with more robust error handling.The method properly handles null cookies array, but consider enhancing error handling when the refresh token cookie is not found. Some options:
- Log a warning when the cookie is missing
- Return an Optional instead of null
- Throw a specific exception type for better error traceability
- public String getRefreshToken(Cookie[] cookies) { + public Optional<String> getRefreshToken(Cookie[] cookies) { if (cookies == null) { - return null; + return Optional.empty(); } for (Cookie cookie : cookies) { if (cookie.getName().equals(properties.name())) { - return cookie.getValue(); + return Optional.ofNullable(cookie.getValue()); } } - return null; + return Optional.empty(); }backend/src/main/java/endolphin/backend/global/security/JwtProvider.java (2)
42-54: Well-implemented refresh token creation method.The
createRefreshTokenmethod is implemented correctly with proper validation attributes:
- Sets only essential claims (userId) for security
- Uses the configured refresh token validity period
- Follows the same pattern as the access token creation
I'd recommend adding a brief Javadoc comment to document the method's purpose and parameters.
55-64: Improve error handling in token validation.The current implementation throws a generic
RuntimeExceptionwhen token validation fails. Consider:
- Using a custom exception type for JWT validation errors
- Including the original exception cause for better debugging
- Providing more specific error messages based on the exception type
public Jws<Claims> validateToken(String token) { try { return Jwts.parser() .verifyWith(key) .build() .parseSignedClaims(token); } catch (JwtException | IllegalArgumentException e) { - throw new RuntimeException("Invalid or expired JWT token"); + throw new JwtAuthenticationException("Invalid or expired JWT token", e); } }This would require creating a custom
JwtAuthenticationExceptionclass that extends an appropriate exception type, providing better error handling throughout the application.backend/src/test/java/endolphin/backend/domain/auth/AuthServiceTest.java (1)
95-127: Refresh token test scenario is good, but cookie naming mismatch is confusing.
You're creating a cookie named"test"rather than"refreshToken", yet you mockcookieUtilto return"test-refresh-token". This doesn’t replicate real usage, wherecookieUtil.getRefreshToken()checks for a cookie named"refreshToken". Also, consider adding negative tests for missing/invalid cookies to increase coverage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
backend/src/main/java/endolphin/backend/domain/auth/AuthController.java(2 hunks)backend/src/main/java/endolphin/backend/domain/auth/AuthService.java(4 hunks)backend/src/main/java/endolphin/backend/domain/auth/dto/TokenDto.java(1 hunks)backend/src/main/java/endolphin/backend/global/config/CookieConfig.java(1 hunks)backend/src/main/java/endolphin/backend/global/config/RefreshTokenCookieProperties.java(1 hunks)backend/src/main/java/endolphin/backend/global/error/exception/ErrorCode.java(1 hunks)backend/src/main/java/endolphin/backend/global/security/JwtAuthFilter.java(1 hunks)backend/src/main/java/endolphin/backend/global/security/JwtProvider.java(3 hunks)backend/src/main/java/endolphin/backend/global/util/CookieUtil.java(1 hunks)backend/src/test/java/endolphin/backend/domain/auth/AuthServiceTest.java(4 hunks)backend/src/test/resources/application.yml(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
backend/src/main/java/endolphin/backend/domain/auth/AuthService.java (1)
backend/src/main/java/endolphin/backend/global/util/TimeUtil.java (1) (1)
TimeUtil(10-104)
backend/src/main/java/endolphin/backend/global/security/JwtProvider.java (1)
backend/src/main/java/endolphin/backend/global/util/TimeUtil.java (1) (1)
TimeUtil(10-104)
🔇 Additional comments (13)
backend/src/main/java/endolphin/backend/global/error/exception/ErrorCode.java (1)
53-53: Addition of INVALID_REFRESH_TOKEN error code looks goodThe new error code is correctly placed in the OAuth section, uses an appropriate HTTP status code (401 UNAUTHORIZED), and follows the existing code pattern with the "O" prefix followed by a sequential number. This aligns with the PR objective to handle refresh token validation failures properly.
backend/src/main/java/endolphin/backend/global/security/JwtAuthFilter.java (1)
66-66: Correct exclusion of refresh token endpoint from JWT authenticationAdding the refresh token endpoint to the excluded paths is necessary for the authentication flow to work properly. When a user's access token expires, they need to be able to request a new one using their refresh token without requiring a valid JWT.
backend/src/main/java/endolphin/backend/global/config/CookieConfig.java (1)
10-13:Details
✅ Verification successful
Ensure Secure flag is set when using SameSite=None
Setting SameSite=None means cookies will be sent in all cross-site requests, which could make them vulnerable to CSRF attacks. When using SameSite=None, the Secure flag should also be set to ensure cookies are only sent over HTTPS. This is especially important for refresh token cookies given their sensitive nature.
Please verify that the CookieUtil class (mentioned in the PR objectives) sets the Secure flag on the refresh token cookies:
🏁 Script executed:
#!/bin/bash # Search for the CookieUtil class and check if it sets the Secure flag on cookies fd -t f CookieUtil.java -x cat {} | grep -A 15 "createRefreshTokenCookie"Length of output: 662
Verified: Secure Flag is Configured for SameSite=None Cookies
The
CookieUtilclass correctly sets the Secure flag on the refresh token cookies by calling:refreshTokenCookie.setSecure(properties.secure());
- This confirms that when using
SameSite=None(configured inCookieConfig.java), the Secure flag is handled via theproperties.secure()value.- Please ensure that the
secureproperty is appropriately set totruein production to enforce HTTPS-only transmission.backend/src/main/java/endolphin/backend/global/config/RefreshTokenCookieProperties.java (1)
5-13: Well-structured configuration properties for refresh token cookie.The use of Java records for configuration properties is a good choice as it provides immutability and concise syntax. This class properly encapsulates all necessary cookie properties for the refresh token implementation.
backend/src/test/resources/application.yml (1)
57-64: Verify refresh token configuration values for production use.The refresh token configuration has some values that might be suitable for testing but could be problematic in production:
expired: 1000sets the token validity to just 1 secondmax-age: 0means the cookie will expire immediatelyThese values should be increased significantly for production environments to allow proper refresh token functionality.
backend/src/main/java/endolphin/backend/global/util/CookieUtil.java (1)
8-22: Clean implementation of cookie management utility.The
CookieUtilclass properly leverages the configuration properties and provides a clean method for creating refresh token cookies. The implementation correctly sets all required cookie attributes.backend/src/main/java/endolphin/backend/global/security/JwtProvider.java (2)
19-20: LGTM: Properly injecting refresh token expiration from configuration.The addition of the
refreshValidityInMsproperty with proper annotation to inject from configuration is well-implemented.
66-68: LGTM: Useful utility method for extracting user ID.This method provides a convenient way to extract the user ID from a validated token. It properly reuses the
validateTokenmethod to ensure the token is valid before attempting to extract the payload.backend/src/main/java/endolphin/backend/domain/auth/AuthService.java (2)
3-3: New imports and dependencies look consistent.
The addition ofTokenDto,CookieUtil, andHttpServletRequestdependencies, along with afinal CookieUtil cookieUtilfield, appears correctly injected via the constructor. No immediate issues noted here.Also applies to: 13-13, 15-15, 31-31
44-55: Refactoredloginmethod returningTokenDtois well-structured.
Encapsulating both the access token and the refresh token cookie inTokenDtomakes sense. Ensure that any calling code (controllers/tests) is also updated accordingly so that the refresh token is handled exclusively via secure cookies.backend/src/main/java/endolphin/backend/domain/auth/AuthController.java (1)
5-5: New imports forTokenDtoandHttpServletRequestlook good.
No issues spotted with these additional imports.Also applies to: 8-8
backend/src/test/java/endolphin/backend/domain/auth/AuthServiceTest.java (2)
7-7: Additional imports and mocks align with new token-based logic.
The use ofwhen(...)andTokenDtoreferences appears valid, preparing for the new refresh token test scenarios.Also applies to: 10-10, 19-19, 20-20, 21-21, 29-29
48-49: Mocking theCookieUtildependency is appropriate.
InjectingCookieUtilas a mock lets you accurately simulate cookie behaviors during tests. No issues noted.
| public TokenDto refresh(HttpServletRequest request) { | ||
| String refreshToken = cookieUtil.getRefreshToken(request.getCookies()); | ||
| if(refreshToken == null || refreshToken.isBlank()) { | ||
| throw new OAuthException(ErrorCode.INVALID_REFRESH_TOKEN); | ||
| } | ||
|
|
||
| //TODO: refresh token 검증 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Incomplete refresh token validation.
You've added a clear check for a null or blank refresh token, which is good. However, the //TODO: refresh token 검증 comment indicates missing signature or expiration checks. This could be a security risk.
Would you like assistance generating code to validate token integrity and expiration?
| public ResponseEntity<OAuthResponse> login(@Valid @RequestBody LoginRequest request, | ||
| HttpServletResponse response) { | ||
| TokenDto tokens = authService.login(request.code()); | ||
| OAuthResponse oAuthResponse = tokens.oAuthResponse(); | ||
|
|
||
| response.addCookie(tokens.refreshTokenCookie()); | ||
|
|
||
| return ResponseEntity.ok(oAuthResponse); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Login endpoint sets refresh token in cookie successfully, but ensure security flags.
When adding the refresh token cookie, consider marking it as HttpOnly, Secure, and SameSite to mitigate XSS and CSRF vectors. By default, these flags are not set, potentially allowing malicious scripts to access the cookie or enabling cross-site usage.
| @Operation(summary = "액세스 토큰 재발급", description = "리프레시 토큰을 사용하여 새로운 액세스 토큰을 발급받습니다.") | ||
| @PostMapping("/api/v1/refresh-token") | ||
| public ResponseEntity<OAuthResponse> refreshToken(HttpServletRequest request, | ||
| HttpServletResponse response) { | ||
|
|
||
| TokenDto newTokens = authService.refresh(request); | ||
|
|
||
| response.addCookie(newTokens.refreshTokenCookie()); | ||
|
|
||
| return ResponseEntity.ok(newTokens.oAuthResponse()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
New refresh endpoint is consistent, but apply the same cookie security measures.
Again, ensure the refresh token cookie is configured with best-practice flags (HttpOnly, Secure, SameSite) here as well. This helps protect the token from unauthorized scripts and cross-site attacks.
kwon204
left a comment
kwon204
left a comment
There was a problem hiding this comment.
refresh token 검증 시 에러는 401로 하면 될 거 같다는 생각이 듭니다
| LocalDateTime expiredAt = TimeUtil.getNow().plus(expired, ChronoUnit.MILLIS); | ||
|
|
||
| return new TokenDto(new OAuthResponse(newAccessToken, expiredAt), | ||
| cookieUtil.createRefreshTokenCookie(refreshToken)); |
There was a problem hiding this comment.
just ask; refresh token을 새로 발급하지는 않는건가요?
There was a problem hiding this comment.
제 생각에도 401에러 반환하면 될 것 같습니다~
|
고생하셨습니다~ api 연동해보고, 필요하면 커멘트로 질문 남기겠습니다 |
|
브라우저 새로고침 시 쿠키에서 리프레시 토큰이 사라집니다.. SHANA.2025-03-19.22-09-46.mp4(+25-03-20 백엔드 지인한테 물어보니 브라우저 개발자 도구에서 쿠키가 안 보여도 실제로는 헤더에 쿠키가 제대로 들어갈 거라고 합니다.
+ 프론트엔드 코드 작성해 테스트해 보았으나 매번 401 에러 반환됩니다. 문제 찾았습니다! application-dev.yml에서 쿠키에 추가로, 최초 로그인 시 backend_api 컨테이너에 아래 에러가 발생합니다. |
컨테니어에서 발생하는 에러는 영상 확인해보니 로그인 요청을 두 번 보내고 있어서 Duplicate entry 'dioo14610143@gmail.com' for key 에러가 발생하는 거 같습니다. |
아 그러면 development 환경에서 React의 Strict Mode 때문에 컴포넌트가 두 번 렌더링되어 생기는 문제인 것 같네요, production 환경에서는 발생하지 않을 문제인 것 같습니다. 확인해주셔서 감사합니다~ |
|
secure는 로컬에서는 false로 해두었습니다. 쿠키가 날아가는 건 여러가지 시도해보았는데 정확히 원인을 모르겠네요.. |
sameSite=none 일 때, secure=true 여야 쿠키가 제대로 들어간다고 합니다. application-dev.yml에서 쿠키에 secure=true 주입하고, 로컬에서 https 설정 후 테스트해보니 제대로 작동합니다. |
|
아하 그런 문제였군요 다행이네요 확인해주셔서 감사합니다. 액세스토큰검증과 리프레시 토큰 검증이 똑같이 401인데 그부분은 괜찮으신가요? |
에러 코드로 식별 가능하니 괜찮을 것 같습니다! |
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit