[refactor] #115 JWT 기반 인증 필터 구현 및 RequestAttribute 기반 인증 적용#142
[refactor] #115 JWT 기반 인증 필터 구현 및 RequestAttribute 기반 인증 적용#142
Conversation
|
""" WalkthroughThis update introduces a refactor of JWT authentication handling across the application. Controllers now receive the authenticated user ID via a request attribute instead of parsing it from the Authorization header. Core JWT logic is modularized into new components: Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant JwtAuthFilter
participant JwtParser
participant Controller
Client->>JwtAuthFilter: HTTP request with Authorization header
JwtAuthFilter->>JwtParser: Parse and validate JWT token
JwtParser-->>JwtAuthFilter: userId (if valid)
JwtAuthFilter->>Controller: Set request attribute "userId"
Controller->>Controller: Access userId via @RequestAttribute
Controller-->>Client: Response
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope or unrelated code changes were detected in this pull request. Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Test Results45 tests 45 ✅ 1s ⏱️ Results for commit b3814d5. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
docker-compose.yaml (1)
1-1: Specify Compose file version more preciselyUsing
version: "3"is valid, but consider pinning to a more specific minor or patch-level (e.g.,3.8) for clarity about feature support and backward compatibility.src/main/java/org/festimate/team/infra/jwt/JwtAuthFilter.java (1)
29-31: Consider more specific exception handling.While logging authentication failures is good practice, catching all exceptions might hide important issues like configuration problems or token provider failures. Consider catching more specific exceptions and potentially failing fast for critical issues.
Consider catching specific JWT-related exceptions separately from general exceptions:
try { Long userId = jwtParser.getUserIdFromToken(token); request.setAttribute(USER_ID, userId); -} catch (Exception e) { - log.warn("JWT 인증 실패: {}", e.getMessage()); +} catch (FestimateException e) { + log.warn("JWT 인증 실패: {}", e.getMessage()); +} catch (Exception e) { + log.error("예상치 못한 JWT 처리 오류: {}", e.getMessage()); }src/main/java/org/festimate/team/infra/config/SecurityConfig.java (1)
24-25: Document or configure CORS settings.The CORS configuration is currently empty. This might be intentional, but it should be documented or properly configured based on your application's requirements.
Consider either documenting the intention or adding proper CORS configuration:
-.cors(cors -> { -}) +.cors(cors -> { + // TODO: Configure CORS based on application requirements + // or remove if not needed +})src/main/java/org/festimate/team/infra/jwt/JwtTokenProvider.java (1)
56-58: Consider caching the secret key.The
getSecretKey()method recreates the key from bytes on every call. For better performance, consider caching theSecretKeyinstance since it's immutable.+ private SecretKey secretKey; + public SecretKey getSecretKey() { - return Keys.hmacShaKeyFor(jwtProperties.getSecretKey().getBytes(StandardCharsets.UTF_8)); + if (secretKey == null) { + secretKey = Keys.hmacShaKeyFor(jwtProperties.getSecretKey().getBytes(StandardCharsets.UTF_8)); + } + return secretKey; }src/main/java/org/festimate/team/infra/jwt/JwtParser.java (1)
41-46: Consider null safety improvement.The platform ID extraction handles null values but could be more explicit about the validation.
public String getPlatformIdFromToken(String token) { Claims claims = parseClaims(token); Object platformId = claims.get(PLATFORM_ID); - if (platformId != null) return platformId.toString(); + if (platformId != null && !platformId.toString().trim().isEmpty()) { + return platformId.toString(); + } + log.error("platformId claim missing or empty in token"); throw new FestimateException(ResponseError.INVALID_TOKEN); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.gitignore(1 hunks)docker-compose.yaml(1 hunks)src/main/java/org/festimate/team/api/admin/AdminController.java(1 hunks)src/main/java/org/festimate/team/api/auth/AuthController.java(1 hunks)src/main/java/org/festimate/team/api/facade/LoginFacade.java(3 hunks)src/main/java/org/festimate/team/api/facade/SignUpFacade.java(2 hunks)src/main/java/org/festimate/team/api/festival/FestivalController.java(1 hunks)src/main/java/org/festimate/team/api/matching/MatchingController.java(1 hunks)src/main/java/org/festimate/team/api/participant/ParticipantController.java(1 hunks)src/main/java/org/festimate/team/api/point/PointController.java(1 hunks)src/main/java/org/festimate/team/api/user/UserController.java(1 hunks)src/main/java/org/festimate/team/infra/config/SecurityConfig.java(1 hunks)src/main/java/org/festimate/team/infra/jwt/JwtAuthFilter.java(1 hunks)src/main/java/org/festimate/team/infra/jwt/JwtParser.java(1 hunks)src/main/java/org/festimate/team/infra/jwt/JwtService.java(1 hunks)src/main/java/org/festimate/team/infra/jwt/JwtTokenProvider.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
src/main/java/org/festimate/team/infra/jwt/JwtAuthFilter.java (1)
src/main/java/org/festimate/team/infra/jwt/JwtService.java (1)
RequiredArgsConstructor(10-31)
src/main/java/org/festimate/team/api/festival/FestivalController.java (1)
src/main/java/org/festimate/team/global/response/ResponseBuilder.java (1)
ResponseBuilder(5-19)
src/main/java/org/festimate/team/api/user/UserController.java (1)
src/main/java/org/festimate/team/global/response/ResponseBuilder.java (1)
ResponseBuilder(5-19)
src/main/java/org/festimate/team/infra/jwt/JwtParser.java (3)
src/main/java/org/festimate/team/infra/jwt/JwtService.java (1)
RequiredArgsConstructor(10-31)src/main/java/org/festimate/team/infra/jwt/JwtAuthFilter.java (1)
Slf4j(12-36)src/main/java/org/festimate/team/infra/jwt/JwtTokenProvider.java (1)
Component(13-59)
src/main/java/org/festimate/team/api/matching/MatchingController.java (1)
src/main/java/org/festimate/team/global/response/ResponseBuilder.java (1)
ResponseBuilder(5-19)
src/main/java/org/festimate/team/api/admin/AdminController.java (1)
src/main/java/org/festimate/team/global/response/ResponseBuilder.java (1)
ResponseBuilder(5-19)
src/main/java/org/festimate/team/api/participant/ParticipantController.java (1)
src/main/java/org/festimate/team/global/response/ResponseBuilder.java (1)
ResponseBuilder(5-19)
🔇 Additional comments (27)
docker-compose.yaml (3)
4-5: Good: Pinning MySQL version and naming containerSpecifying
mysql:8.0ensures a stable database version, and a consistentcontainer_nameimproves manageability.
9-9: Port mapping is correctExposing
"3306:3306"aligns host and container ports as expected.
11-12: Persisted volumes and restart policyMounting
./mysql-dataensures data persistence, andrestart: unless-stoppedhelps maintain service availability.src/main/java/org/festimate/team/api/auth/AuthController.java (1)
29-29: LGTM! Improved logging clarity.The log message change from "Code" to "kakaoAccessToken" makes the logging more descriptive and improves debugging experience.
src/main/java/org/festimate/team/api/facade/LoginFacade.java (2)
9-9: LGTM! Improved separation of concerns.The refactoring to use
JwtTokenProviderinstead ofJwtServicefollows the single responsibility principle by delegating token creation to a dedicated component.Also applies to: 18-18
31-31: LGTM! Consistent token creation delegation.All token creation operations are now properly delegated to
JwtTokenProvider, maintaining consistency across the facade.Also applies to: 34-34, 42-42
src/main/java/org/festimate/team/api/facade/SignUpFacade.java (2)
12-12: LGTM! Consistent refactoring pattern.The migration from
JwtServicetoJwtTokenProviderfollows the same pattern asLoginFacade, creating architectural consistency across facade classes.Also applies to: 19-19
39-40: LGTM! Proper token creation delegation.Token creation operations are correctly delegated to
JwtTokenProvider, maintaining the same functionality with improved separation of concerns.src/main/java/org/festimate/team/infra/config/SecurityConfig.java (2)
27-27: Review permissive authorization configuration.The current configuration permits all requests without authentication. Ensure this aligns with your security requirements, especially for production environments.
Please verify that
permitAll()is the intended authorization strategy for all endpoints, or if specific endpoints should require authentication.
28-28: JWT filter integration looks good.The JWT authentication filter is correctly positioned before the
UsernamePasswordAuthenticationFilterin the filter chain, which will allow JWT-based authentication to be processed first.src/main/java/org/festimate/team/infra/jwt/JwtTokenProvider.java (1)
13-59: Well-structured JWT token provider implementation.The
JwtTokenProviderclass demonstrates good separation of concerns and follows consistent patterns across all token creation methods. The use ofJwtPropertiesfor configuration and proper HMAC SHA key generation are excellent practices.src/main/java/org/festimate/team/infra/jwt/JwtParser.java (1)
21-32: JWT parsing implementation looks solid.The
parseClaimsmethod correctly uses the JJWT library for token validation and provides appropriate error handling with custom exceptions.src/main/java/org/festimate/team/api/festival/FestivalController.java (2)
21-21: Clean refactor to centralized authentication pattern.The change from manual JWT token parsing to
@RequestAttribute("userId")injection is an excellent architectural improvement that separates authentication concerns from controller logic.
30-30: Consistent implementation across endpoints.Good consistency in applying the same authentication pattern across both endpoints in this controller.
src/main/java/org/festimate/team/api/user/UserController.java (3)
59-59: Good refactor to centralized authentication.The change to use
@RequestAttribute("userId")simplifies the controller logic and aligns with the architectural improvements seen across other controllers.
67-67: Consistent application of the new pattern.Excellent consistency in applying the same authentication approach across both authenticated endpoints.
32-32:Details
✅ Verification successful
Verify JwtService dependency is still needed.
The
JwtServicedependency is retained, which appears correct since it's used in thesignUpmethod (line 50) for extracting platform user ID from tokens. This is a different use case than the authenticated endpoints.Let me verify that JwtService is actually used in this controller:
🏁 Script executed:
#!/bin/bash # Description: Check if JwtService is used in UserController # Expected: Should find usage in signup method rg -A 3 -B 3 "jwtService\." src/main/java/org/festimate/team/api/user/UserController.javaLength of output: 436
JwtService dependency is required
Confirmed that
jwtService.extractPlatformUserIdFromToken(accessToken)is invoked in thesignUpmethod (around line 50) to retrieve the platform user ID. No changes needed.src/main/java/org/festimate/team/api/matching/MatchingController.java (3)
21-21: Excellent refactor implementation.The migration to
@RequestAttribute("userId")is cleanly implemented and removes authentication complexity from the controller layer.
30-30: Consistent pattern application.Good consistency in applying the same authentication approach across all endpoints.
39-39: Complete migration to centralized authentication.All endpoints in this controller have been successfully migrated to the new authentication pattern, demonstrating thorough refactoring.
src/main/java/org/festimate/team/infra/jwt/JwtService.java (4)
12-12: Appropriate annotation change.The change from
@Componentto@Serviceis semantically correct since this class provides business logic services rather than just being a generic Spring component.
18-26: Clean delegation in reIssueToken method.The method effectively delegates JWT operations to the appropriate specialized components while maintaining the core business logic for token reissuance. The flow is clear and follows good separation of concerns.
28-30: Consistent delegation pattern.The
extractPlatformUserIdFromTokenmethod correctly delegates toJwtParser, maintaining consistency with the overall refactoring approach.
14-15:Details
✅ Verification successful
Good architectural refactor using delegation pattern.
The introduction of
JwtTokenProviderandJwtParseras dependencies follows the single responsibility principle and improves code organization by separating token creation from token parsing concerns.Let me verify that these new JWT components exist and are properly implemented:
🏁 Script executed:
#!/bin/bash # Description: Verify the new JWT components exist and are properly configured # Expected: Should find JwtTokenProvider and JwtParser classes echo "Checking for JwtTokenProvider:" fd "JwtTokenProvider.java" --type f echo "Checking for JwtParser:" fd "JwtParser.java" --type f echo "Verifying they are Spring components:" rg -l "@(Component|Service)" | xargs rg -l "class Jwt(TokenProvider|Parser)"Length of output: 602
Verification Successful – JWT Components Present and Configured
Both
JwtTokenProviderandJwtParserclasses were found undersrc/main/java/org/festimate/team/infra/jwt/and are properly annotated as Spring components. This confirms the refactor cleanly delegates token creation toJwtTokenProviderand parsing toJwtParser, adhering to the single responsibility principle.No further changes required—approving these updates.
src/main/java/org/festimate/team/api/participant/ParticipantController.java (2)
22-22: Excellent refactoring - centralized authentication approach.The change from manual JWT parsing to
@RequestAttribute("userId")is a significant improvement that centralizes authentication concerns in the filter layer, following the separation of concerns principle.
32-32: Consistent implementation across all endpoints.All endpoints consistently use the new
@RequestAttribute("userId")pattern, ensuring uniform authentication handling throughout the controller. This eliminates code duplication and reduces the risk of authentication-related bugs.Also applies to: 42-42, 52-52, 61-61, 71-71, 81-81
src/main/java/org/festimate/team/api/admin/AdminController.java (1)
26-26: Consistent and secure authentication refactoring.All admin endpoints consistently implement the new authentication pattern using
@RequestAttribute("userId"). This is particularly important for admin endpoints which require robust authentication, and the centralized approach reduces the attack surface by eliminating duplicate JWT parsing logic across multiple endpoints.Also applies to: 35-35, 43-43, 52-52, 62-62, 72-72, 82-82, 92-92
2hyunjinn
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/java/org/festimate/team/api/facade/LoginFacadeTest.java (1)
24-24: Consider removing unused JwtService mock if no longer needed.Similar to SignUpFacadeTest, the
JwtServicemock is declared but no longer used for token creation. This should be cleaned up ifJwtServiceno longer has testable responsibilities in this class.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/java/org/festimate/team/api/facade/LoginFacadeTest.java(4 hunks)src/test/java/org/festimate/team/api/facade/SignUpFacadeTest.java(3 hunks)
🔇 Additional comments (7)
src/test/java/org/festimate/team/api/facade/SignUpFacadeTest.java (3)
12-12: LGTM: Import added for the new JWT token provider.The import of
JwtTokenProvideraligns with the JWT authentication refactoring objectives.
32-33: LGTM: Mock properly added for JwtTokenProvider.The new mock field is correctly declared and will be used for token creation in the refactored architecture.
77-78: LGTM: Token creation calls updated to use JwtTokenProvider.The method calls are correctly updated to use the new
jwtTokenProvidermock instead ofjwtService, which aligns with the separation of responsibilities in the JWT refactoring.src/test/java/org/festimate/team/api/facade/LoginFacadeTest.java (4)
8-8: LGTM: Import added for the new JWT token provider.The import of
JwtTokenProvideris consistent with the JWT refactoring across test files.
26-27: LGTM: Mock properly added for JwtTokenProvider.The new mock field is correctly declared for the refactored JWT token creation.
47-48: LGTM: Regular token creation updated to use JwtTokenProvider.The method calls for access and refresh token creation are correctly updated to use the new token provider.
66-67: LGTM: Temporary token creation updated to use JwtTokenProvider.The temporary token creation methods are also correctly updated, ensuring comprehensive coverage of all token types.
src/test/java/org/festimate/team/api/facade/SignUpFacadeTest.java
Outdated
Show resolved
Hide resolved
1 participant
📌 PR 제목
[refactor] #115 JWT 기반 인증 필터 구현 및 RequestAttribute 기반 인증 적용
📌 PR 내용
🛠 작업 내용
🔍 관련 이슈
Closes #115
📸 스크린샷 (Optional)
📚 레퍼런스 (Optional)
Spring Security에서 JWT 인증/인가 – RequestAttribute 실전 적용기
데굴데굴 개발자의 기록: SpringBoot 3 JWT 인증/인가 구조
길은 가면, 뒤에 있다: JWT란?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Chores