Refactor auth API with Redis token management and service separation #141
Conversation
- Consolidated tag annotations in AdminController and MemberController for consistency. - Improved formatting in SecurityConfig and JwtAuthorizationFilter for better readability. - Added constants for access and refresh token durations in AuthConstants. - Updated error codes in ErrorCode for better clarity on refresh token issues. - Refactored JwtTokenProvider to streamline token creation and validation processes. - Enhanced MemberService with clearer method documentation and improved member retrieval logic.
…nto refactor/auth
- Added Redis support for token storage in JwtTokenProvider. - Introduced device ID handling in refresh token logic to improve security. - Updated AuthConstants to include new constants for refresh token handling. - Refactored HttpRequestUtil for better IP and device ID extraction. - Removed deprecated AuthController and consolidated member-related services for improved maintainability. - Added error handling for missing device ID in requests. - Enhanced MemberService to delegate responsibilities to specialized services for better separation of concerns.
- Updated Redis configuration for enhanced token storage and management. - Removed deprecated classes and methods related to refresh token handling. - Consolidated authentication logic into a single AuthService for better maintainability. - Improved error handling for device ID mismatches and token reuse detection. - Enhanced security by implementing refresh token rotation and validation mechanisms. - Updated GlobalExceptionHandler to manage new error codes and exceptions effectively. - Refactored JwtTokenProvider to streamline token creation and hashing processes. - Improved CORS configuration and security settings in SecurityConfig for better API protection.
Walkthrough이 변경 사항은 인증 API 리팩터링, Redis 기반 토큰 관리 도입, 유틸리티 및 서비스 계층 분리, 그리고 보안 설정 개선을 포함한다. Redis 연동, Lua 스크립트 기반 토큰 로테이션, 도메인 서비스 분리, Swagger 문서화, 그리고 다양한 추출 유틸리티 클래스가 추가되었다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant AuthController
participant AuthService
participant TokenService
participant RedisRefreshTokenRepository
participant Redis (ElastiCache)
Client->>AuthController: POST /v1/api/auth/login
AuthController->>AuthService: login(request)
AuthService->>TokenService: createTokenPair(member, ...)
TokenService->>RedisRefreshTokenRepository: save(refreshToken)
RedisRefreshTokenRepository->>Redis: Lua Script (save_refresh_token.lua)
Redis-->>RedisRefreshTokenRepository: OK
TokenService-->>AuthService: TokenPair
AuthService-->>AuthController: LoginResponse
AuthController-->>Client: 200 OK + Set-Cookie(Refresh-Token)
sequenceDiagram
actor Client
participant AuthController
participant AuthService
participant TokenService
participant RedisRefreshTokenRepository
participant Redis (ElastiCache)
Client->>AuthController: POST /v1/api/auth/refresh (with Refresh-Token Cookie)
AuthController->>AuthService: refresh(refreshToken)
AuthService->>TokenService: rotateRefreshToken(refreshToken)
TokenService->>RedisRefreshTokenRepository: rotate(refreshToken, hashedOldToken)
RedisRefreshTokenRepository->>Redis: Lua Script (rotate_refresh_token.lua)
Redis-->>RedisRefreshTokenRepository: OK or Error
TokenService-->>AuthService: New TokenPair
AuthService-->>AuthController: RefreshResponse
AuthController-->>Client: 200 OK + Set-Cookie(Refresh-Token)
Poem
|
coderabbitai
bot
changed the title
- Added links to PR #139 in relevant documentation files for better traceability. - Cleaned up .gitignore by removing duplicate entries and ensuring proper formatting.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 39
♻️ Duplicate comments (1)
src/main/java/com/juu/juulabel/auth/controller/AuthController.java (1)
60-67: 또 다른 사용하지 않는 member 파라미터예요!logout 메서드에서도 같은 문제가 있어요. 일관성이 없어요, fool!
🧹 Nitpick comments (25)
src/main/java/com/juu/juulabel/common/dto/response/RefreshResponse.java (1)
3-5: 야, 이 바보야! 빈 줄들을 정리해라!레코드 내부에 불필요한 빈 줄들이 있다니, 내가 이런 걸 봐야 하다니 불쌍해!
public record RefreshResponse(String accessToken) { - }src/main/resources/scripts/save_refresh_token.lua (1)
1-32: I pity the fool who doesn't appreciate this solid token saving script, but let me break it down for ya!이 스크립트는 기본적인 토큰 저장 로직을 잘 구현했지만, 몇 가지 개선사항이 있습니다:
- 에러 처리 부족: Redis 명령어 실행 중 실패할 경우에 대한 처리가 없습니다
- 원자성 보장: 현재 구조는 좋지만 중간에 실패하면 일관성 문제가 발생할 수 있습니다
다음과 같은 개선을 고려해보세요:
-- Store token -- Save the new token hash -redis.call("HSET", newTokenKey, "memberId", memberId, "clientId", clientId, "deviceId", deviceId, "ipAddress", - ipAddress, "userAgent", userAgent, "revoked", 0) -redis.call("EXPIRE", newTokenKey, ttl) +local hsetResult = redis.call("HSET", newTokenKey, "memberId", memberId, "clientId", clientId, "deviceId", deviceId, "ipAddress", + ipAddress, "userAgent", userAgent, "revoked", 0) +if hsetResult == 0 then + return { err = "FAILED_TO_SAVE_TOKEN" } +end +redis.call("EXPIRE", newTokenKey, ttl)src/main/resources/scripts/login_refresh_token.lua (1)
31-40: New token creation looks good, but I got some concerns, fool!새 토큰 생성 로직은 올바르지만 한 가지 확인이 필요합니다:
- 기존 토큰 무효화와 새 토큰 생성 사이에 타이밍 이슈가 발생할 수 있습니다
- 만약 새 토큰 생성이 실패하면 사용자가 모든 토큰을 잃을 수 있습니다
더 안전한 접근을 위해 다음을 고려해보세요:
+-- First create new token +redis.call("HSET", newTokenKey, "memberId", memberId, "clientId", clientId, "deviceId", deviceId, "ipAddress", + ipAddress, "userAgent", userAgent, "revoked", 0) +redis.call("EXPIRE", newTokenKey, ttl) + -- Revoke all old tokens for _, key in ipairs(oldTokenKeys) do if redis.call("EXISTS", key) == 1 then redis.call("HSET", key, "revoked", 1) else redis.call("SREM", indexKey, key) -- clean up dead keys end end --- Save the new token hash -redis.call("HSET", newTokenKey, "memberId", memberId, "clientId", clientId, "deviceId", deviceId, "ipAddress", - ipAddress, "userAgent", userAgent, "revoked", 0)docs/infra/aws-elasticache-redis-local-setup.md (1)
47-53: I pity the fool who doesn't use proper CLI syntax! This command needs some love!AWS CLI 명령어가 정확하지만 몇 가지 개선사항이 있습니다:
더 안전하고 읽기 쉬운 형태로 개선할 수 있습니다:
```bash aws ssm start-session \ --target i-xxxxxxxxxxxxxxxxx \ --document-name AWS-StartPortForwardingSession \ - --parameters '{"portNumber":["6379"],"localPortNumber":["6379"]}' \ + --parameters 'portNumber=6379,localPortNumber=6379' \ --profile dev-redisJSON 형태보다 간단한 key=value 형태가 더 읽기 쉽고 실수를 줄입니다.
src/main/java/com/juu/juulabel/auth/executor/RedisScriptName.java (1)
3-18: 이 enum은 제대로 구현되었다, fool!Redis 스크립트 이름을 executor 이름에 매핑하는 깔끔한 구조다. 네이밍 컨벤션도 일관되고 "ScriptExecutor" 접미사도 적절하다. I ain't got no complaints about this implementation, sucka!
각 enum 값이 명확한 목적을 가지고 있고, 생성자와 getter 메서드도 표준적이다.
클래스 레벨 Javadoc 추가를 고려해보자:
+/** + * Redis 스크립트 실행자 이름을 정의하는 enum + */ public enum RedisScriptName {src/main/java/com/juu/juulabel/common/util/DeviceIdExtractor.java (1)
12-12: Listen up, fool! 이 상수는 중앙 집중화가 필요해 보입니다!
DEVICE_ID_HEADER_NAME상수가 여기에 하드코딩되어 있는데, AI 요약에서 언급된AuthConstants에 이미 디바이스 ID 관련 상수들이 정의되어 있다고 합니다. 일관성을 위해 이 상수를AuthConstants로 이동하는 것을 고려해보세요.src/main/java/com/juu/juulabel/common/util/UserAgentExtractor.java (1)
23-26: I pity the fool who doesn't handle null values! User-Agent 헤더가 없을 때 null 반환 가능성을 고려하세요.getCurrentRequest()에서 가져온 요청의 User-Agent 헤더가 없을 경우 null이 반환될 수 있습니다. 보안 및 로깅 목적으로 사용되는 것 같은데, null 체크나 기본값 제공을 고려해보세요.
다음과 같이 개선할 수 있습니다:
public static String getUserAgent() { HttpServletRequest request = getCurrentRequest(); - return request.getHeader(HttpHeaders.USER_AGENT); + String userAgent = request.getHeader(HttpHeaders.USER_AGENT); + return userAgent != null ? userAgent : "Unknown"; }src/main/java/com/juu/juulabel/common/util/AbstractHttpUtil.java (1)
54-60: 더 구체적인 예외 처리가 필요해, fool!
INTERNAL_SERVER_ERROR는 너무 일반적이야. I pity the fool who gets this generic error! 더 구체적인 에러 코드를 사용하거나 커스텀 예외를 만들어.protected static <T> T getFromRequestAttributes(Function<ServletRequestAttributes, T> extractor) { return Optional.ofNullable(RequestContextHolder.getRequestAttributes()) .filter(ServletRequestAttributes.class::isInstance) .map(ServletRequestAttributes.class::cast) .map(extractor) - .orElseThrow(() -> new BaseException(ErrorCode.INTERNAL_SERVER_ERROR)); + .orElseThrow(() -> new BaseException(ErrorCode.REQUEST_CONTEXT_NOT_AVAILABLE)); }src/main/java/com/juu/juulabel/auth/executor/RevokeRefreshTokenByIndexKeyExecutor.java (1)
32-35: 도달할 수 없는 코드가 있어, you crazy fool!
handleRedisException(e)다음의throw e는 절대 실행되지 않을 거야. I pity the fool who wrote unreachable code!handleRedisException이 항상 예외를 던지는 것 같아.} catch (RedisSystemException e) { handleRedisException(e); - throw e; // This line will never be reached due to the exception thrown above }src/main/java/com/juu/juulabel/member/service/MemberLookupService.java (2)
28-31: 파라미터 검증이 없어, you fool!
memberId가 null이나 음수일 때를 대비한 검증이 없어. I pity the fool who passes invalid data! 파라미터 검증을 추가해야 해.public Member findById(Long memberId) { + if (memberId == null || memberId <= 0) { + throw new BaseException(ErrorCode.INVALID_PARAMETER); + } return memberJpaRepository.findById(memberId) .orElseThrow(() -> new BaseException(ErrorCode.MEMBER_NOT_FOUND)); }
38-40: 메소드 이름이 일관성이 없어, fool!
findById는 Optional을 리턴할 것 같은데 실제로는 예외를 던지고,getMemberByEmail은 get으로 시작해. I pity the fool who can't maintain naming consistency! 일관성을 맞춰야 해.- public Member getMemberByEmail(String email) { + public Member findByEmail(String email) { return memberReader.getByEmail(email); }src/main/java/com/juu/juulabel/auth/executor/ScriptRegistry.java (1)
15-18: 빈 컬렉션에 대한 검증이 없어, fool!
executors리스트가 비어있거나 null일 때를 대비한 검증이 없어. I pity the fool who doesn't validate inputs! 초기화 시점에 검증을 추가해야 해.public ScriptRegistry(List<RedisScriptExecutor<?, ?>> executors) { + if (executors == null || executors.isEmpty()) { + throw new IllegalArgumentException("Executors list cannot be null or empty"); + } this.scripts = executors.stream() .collect(Collectors.toMap(e -> e.getClass().getSimpleName(), Function.identity())); }src/main/java/com/juu/juulabel/auth/service/RiskAssessment.java (2)
20-23: 하드코딩된 임계값을 상수로 빼는 게 어때, fool?0.8이라는 매직 넘버를 직접 사용하는 건 좋지 않아! 나중에 변경이 필요할 때를 대비해서 상수로 정의하는 게 낫겠어.
+private static final double HIGH_RISK_THRESHOLD = 0.8; + public boolean isHighRisk() { - /* e.g., score > 0.8 */ - return score > 0.8; + return score > HIGH_RISK_THRESHOLD; }
25-27: 이 메서드는 불필요해 보이는군, fool!
@Getter어노테이션이 이미isFamilyShouldBeCompromised()getter를 생성하고 있어. 이 메서드는 중복이야!-public boolean isFamilyCompromised() { - return familyShouldBeCompromised; -}src/main/java/com/juu/juulabel/auth/domain/RefreshToken.java (2)
16-16: Serializable 구현했으면 serialVersionUID도 추가해야지, fool!직렬화 호환성을 위해 serialVersionUID를 명시적으로 선언하는 게 좋아!
public class RefreshToken implements Serializable { + private static final long serialVersionUID = 1L;
58-60: clientId.toString() 대신 getValue()를 쓰는 게 더 명확해!ClientId enum에 getValue() 메서드가 있으니까 그걸 사용하는 게 더 좋겠어, fool!
public List<String> getArgs() { - return List.of(memberId.toString(), clientId.toString(), deviceId, ipAddress, userAgent, ttl.toString()); + return List.of(memberId.toString(), clientId.getValue(), deviceId, ipAddress, userAgent, ttl.toString()); }src/main/java/com/juu/juulabel/common/constants/AuthConstants.java (1)
20-21: 토큰 만료 시간을 환경별로 설정할 수 있게 하는 건 어때?프로덕션과 개발 환경에서 다른 만료 시간을 사용하고 싶을 수도 있어, fool! 설정 파일에서 읽어오는 방식을 고려해봐!
// application.yml에서 설정 가능하도록: // auth: // token: // access-duration: P1D // refresh-duration: P30Dsrc/main/java/com/juu/juulabel/auth/executor/LoginRefreshTokenScriptExecutor.java (1)
40-42: 도달할 수 없는 코드다, sucka!라인 41의
throw e;는 절대 실행되지 않을 것이다.handleRedisException(e)가 이미 예외를 던지기 때문이다.이 불필요한 코드를 제거해라:
} catch (RedisSystemException e) { handleRedisException(e); - throw e; // This line will never be reached due to the exception thrown above }src/main/java/com/juu/juulabel/member/service/MemberProfileService.java (1)
87-95: I pity the fool who doesn't handle empty collections properly!빈 컬렉션 처리가 잘 되어 있지만, 메서드명을 더 명확하게 할 수 있습니다.
메서드명을 더 구체적으로 만들어보세요:
- private void updateMemberAlcoholTypes(Member member, List<Long> alcoholTypeIds) { + private void saveMemberAlcoholTypes(Member member, List<Long> alcoholTypeIds) {src/main/java/com/juu/juulabel/common/config/SecurityConfig.java (1)
114-115: CORS 설정 중복이다, fool!
addAllowedMethod("*")와setAllowedMethods(Arrays.asList(...))가 중복되어 있다. I pity the fool who writes redundant code!다음 중 하나만 사용하라:
config.addAllowedHeader("*"); -config.addAllowedMethod("*"); config.setAllowedMethods(Arrays.asList("GET", "POST", "PUT", "DELETE", "OPTIONS"));src/main/java/com/juu/juulabel/auth/executor/SaveRefreshTokenScriptExecutor.java (1)
42-43: 의미없는 주석이다, fool!"This line will never be reached"라는 주석은 불필요하다. I pity the fool who writes obvious comments!
주석을 제거하라:
} catch (RedisSystemException e) { handleRedisException(e); - throw e; // This line will never be reached due to the exception thrown above + throw e; }src/main/java/com/juu/juulabel/member/util/MemberUtils.java (1)
39-51: 주석 위치가 엉망이다, fool!Javadoc 주석이 메서드가 아니라 필드 위에 있다. I pity the fool who can't organize comments properly!
주석을 올바른 위치로 이동하라:
- /** - * 회원-주종 관계 목록 생성 - * - * @param member 회원 - * @param alcoholTypeIdList 주종 ID 목록 - * @param alcoholTypeReader 주종 조회 리포지토리 - * @return 회원-주종 관계 목록 - */ private final TermsReader termsReader; private final MemberAlcoholTypeWriter memberAlcoholTypeWriter; private final AlcoholTypeReader alcoholTypeReader; private final MemberTermsWriter memberTermsWriter; + /** + * 회원-주종 관계 목록 생성 + * + * @param member 회원 + * @param alcoholTypeIdList 주종 ID 목록 + * @param alcoholTypeReader 주종 조회 리포지토리 + * @return 회원-주종 관계 목록 + */ public List<MemberAlcoholType> getMemberAlcoholTypeList(Member member, List<Long> alcoholTypeIdList,src/main/java/com/juu/juulabel/common/provider/JwtTokenProvider.java (2)
64-66: 불필요한 null 체크를 제거하세요!
role파라미터는 항상member.getRole().name()으로 전달되니까 null일 수 없어요. 이 fool들을 위한 불필요한 체크예요!role이 null일 수 없으니 제거하거나, 더 의미있는 검증으로 바꾸세요:
- if (role != null) { - builder.claim(ROLE_CLAIM, role); - } + builder.claim(ROLE_CLAIM, role);
145-147: 더 명확한 에러 메시지를 사용하세요!SHA-256은 모든 JVM에서 지원되는 표준 알고리즘이에요. 이 예외는 절대 발생하지 않을 거예요, 하지만 메시지는 더 구체적으로 만들어야 해요!
- throw new IllegalStateException("SHA-256 not available", e); + throw new IllegalStateException("Failed to initialize SHA-256 algorithm for token hashing", e);src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java (1)
54-76: IP 추출 로직이 훌륭하지만 성능을 개선할 수 있다고, 바보야!현재 스트림을 두 번 실행하는 구조(public IP 우선, fallback으로 모든 IP)는 비효율적일 수 있습니다. 특히 헤더가 많을 때 성능 이슈가 있을 수 있어요.
public static String getClientIpAddress() { HttpServletRequest request = getCurrentRequest(); + + String bestPublicIp = null; + String bestAnyIp = null; - return IP_HEADER_CANDIDATES.stream() - .map(request::getHeader) - .filter(ip -> ip != null && !ip.isEmpty() && !UNKNOWN.equalsIgnoreCase(ip)) - .map(ip -> ip.split(",")[0].trim()) - .filter(IpAddressExtractor::isValidIpAddress) - .filter(IpAddressExtractor::isPublicIpAddress) - .findFirst() - .orElseGet(() -> { /* fallback logic */ }); + for (String header : IP_HEADER_CANDIDATES) { + String ip = request.getHeader(header); + if (ip != null && !ip.isEmpty() && !UNKNOWN.equalsIgnoreCase(ip)) { + ip = ip.split(",")[0].trim(); + if (isValidIpAddress(ip)) { + if (bestAnyIp == null) bestAnyIp = ip; + if (isPublicIpAddress(ip) && bestPublicIp == null) { + bestPublicIp = ip; + break; + } + } + } + } + + return bestPublicIp != null ? bestPublicIp : + (bestAnyIp != null ? bestAnyIp : + (request.getRemoteAddr() != null ? request.getRemoteAddr() : "unknown")); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/main/generated/com/juu/juulabel/report/QReport.javais excluded by!**/generated/**
📒 Files selected for processing (53)
.gitignore(1 hunks)build.gradle(2 hunks)docs/infra/aws-elasticache-redis-local-setup.md(1 hunks)docs/pr/PR-139-refactor---auth-api.md(1 hunks)src/main/java/com/juu/juulabel/JuulabelApplication.java(1 hunks)src/main/java/com/juu/juulabel/admin/AdminController.java(2 hunks)src/main/java/com/juu/juulabel/auth/controller/AuthApiDocs.java(1 hunks)src/main/java/com/juu/juulabel/auth/controller/AuthController.java(1 hunks)src/main/java/com/juu/juulabel/auth/domain/ClientId.java(1 hunks)src/main/java/com/juu/juulabel/auth/domain/RefreshToken.java(1 hunks)src/main/java/com/juu/juulabel/auth/executor/LoginRefreshTokenScriptExecutor.java(1 hunks)src/main/java/com/juu/juulabel/auth/executor/RedisScriptExecutor.java(1 hunks)src/main/java/com/juu/juulabel/auth/executor/RedisScriptName.java(1 hunks)src/main/java/com/juu/juulabel/auth/executor/RevokeRefreshTokenByIndexKeyExecutor.java(1 hunks)src/main/java/com/juu/juulabel/auth/executor/RotateRefreshTokenScriptExecutor.java(1 hunks)src/main/java/com/juu/juulabel/auth/executor/SaveRefreshTokenScriptExecutor.java(1 hunks)src/main/java/com/juu/juulabel/auth/executor/ScriptRegistry.java(1 hunks)src/main/java/com/juu/juulabel/auth/repository/RedisRefreshTokenRepository.java(1 hunks)src/main/java/com/juu/juulabel/auth/repository/RefreshTokenRepository.java(1 hunks)src/main/java/com/juu/juulabel/auth/service/AuthService.java(1 hunks)src/main/java/com/juu/juulabel/auth/service/FraudDetectionService.java(1 hunks)src/main/java/com/juu/juulabel/auth/service/RiskAssessment.java(1 hunks)src/main/java/com/juu/juulabel/auth/service/TokenService.java(1 hunks)src/main/java/com/juu/juulabel/common/config/RedisConfig.java(1 hunks)src/main/java/com/juu/juulabel/common/config/SecurityConfig.java(1 hunks)src/main/java/com/juu/juulabel/common/config/WebConfig.java(1 hunks)src/main/java/com/juu/juulabel/common/constants/AuthConstants.java(2 hunks)src/main/java/com/juu/juulabel/common/converter/ProviderConverter.java(1 hunks)src/main/java/com/juu/juulabel/common/dto/response/MemberProfileResponse.java(0 hunks)src/main/java/com/juu/juulabel/common/dto/response/RefreshResponse.java(1 hunks)src/main/java/com/juu/juulabel/common/exception/InvalidParamException.java(0 hunks)src/main/java/com/juu/juulabel/common/exception/code/ErrorCode.java(2 hunks)src/main/java/com/juu/juulabel/common/exception/handler/GlobalExceptionHandler.java(3 hunks)src/main/java/com/juu/juulabel/common/filter/JwtAuthorizationFilter.java(3 hunks)src/main/java/com/juu/juulabel/common/provider/JwtTokenProvider.java(2 hunks)src/main/java/com/juu/juulabel/common/util/AbstractHttpUtil.java(1 hunks)src/main/java/com/juu/juulabel/common/util/AuthorizationExtractor.java(1 hunks)src/main/java/com/juu/juulabel/common/util/DeviceIdExtractor.java(1 hunks)src/main/java/com/juu/juulabel/common/util/HttpResponseUtil.java(1 hunks)src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java(1 hunks)src/main/java/com/juu/juulabel/common/util/UserAgentExtractor.java(1 hunks)src/main/java/com/juu/juulabel/member/controller/MemberController.java(1 hunks)src/main/java/com/juu/juulabel/member/request/OAuthUserInfo.java(1 hunks)src/main/java/com/juu/juulabel/member/service/MemberContentService.java(1 hunks)src/main/java/com/juu/juulabel/member/service/MemberLookupService.java(1 hunks)src/main/java/com/juu/juulabel/member/service/MemberProfileService.java(1 hunks)src/main/java/com/juu/juulabel/member/service/MemberService.java(1 hunks)src/main/java/com/juu/juulabel/member/util/MemberUtils.java(1 hunks)src/main/java/com/juu/juulabel/tastingnote/domain/TastingNote.java(3 hunks)src/main/resources/scripts/login_refresh_token.lua(1 hunks)src/main/resources/scripts/revoke_refresh_token_by_index_key.lua(1 hunks)src/main/resources/scripts/rotate_refresh_token.lua(1 hunks)src/main/resources/scripts/save_refresh_token.lua(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/juu/juulabel/common/dto/response/MemberProfileResponse.java
- src/main/java/com/juu/juulabel/common/exception/InvalidParamException.java
🧰 Additional context used
🪛 ast-grep (0.38.1)
src/main/java/com/juu/juulabel/common/util/HttpResponseUtil.java
[warning] 19-19: A cookie was detected without setting the 'secure' flag. The 'secure' flag for cookies prevents the client from transmitting the cookie over insecure channels such as HTTP. Set the 'secure' flag by calling '.setSecure(true);'.
Context: response.addCookie(cookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
[warning] 19-19: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(cookie);
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
[warning] 19-19: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(cookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
src/main/java/com/juu/juulabel/auth/executor/RotateRefreshTokenScriptExecutor.java
[warning] 52-52: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: HttpResponseUtil.addCookie(AuthConstants.REFRESH_TOKEN_HEADER_NAME, "", 0);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
src/main/java/com/juu/juulabel/auth/executor/RedisScriptExecutor.java
[warning] 30-30: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: HttpResponseUtil.addCookie(AuthConstants.REFRESH_TOKEN_HEADER_NAME, "", 0);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
src/main/java/com/juu/juulabel/auth/service/TokenService.java
[warning] 121-124: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: HttpResponseUtil.addCookie(
AuthConstants.REFRESH_TOKEN_HEADER_NAME,
token,
(int) AuthConstants.REFRESH_TOKEN_DURATION.getSeconds());
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 128-128: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: HttpResponseUtil.addCookie(AuthConstants.REFRESH_TOKEN_HEADER_NAME, "", 0);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
🔇 Additional comments (50)
src/main/java/com/juu/juulabel/member/request/OAuthUserInfo.java (1)
6-6: 훌륭한 변경이야, fool! memberId 필드 추가는 정말 현명한 결정이다!토큰 관리와 회원 연결을 위한 memberId 필드 추가는 인증 리팩터링의 핵심적인 개선사항이다. I pity the fool who doesn't see the value in proper member-token association!
이 변경사항은 다음과 같은 이점을 제공한다:
- 새로운 refresh token rotation 전략 지원
- Redis 기반 토큰 관리에서 회원별 토큰 추적 가능
- 보안 강화를 위한 토큰-회원 매핑 개선
build.gradle (2)
35-37: 좋아, fool! Redis 의존성 추가는 완벽한 선택이다!spring-boot-starter-data-redis 추가는 새로운 토큰 관리 전략을 위한 필수적인 변경사항이다. I pity the fool who tries to implement secure token rotation without proper Redis support!
54-56: 훌륭해, fool! GeoIP2 라이브러리로 보안을 한층 더 강화했군!MaxMind GeoIP2 라이브러리 추가는 위치 기반 보안 기능을 위한 현명한 선택이다. 의심스러운 로그인 시도나 비정상적인 접근을 탐지하는 데 도움이 될 거야. I pity the fool who doesn't implement geo-based security measures!
src/main/java/com/juu/juulabel/JuulabelApplication.java (2)
5-5: 완벽한 import야, fool! Redis 기능을 제대로 활성화하려면 이게 필요하다!EnableRedisRepositories import는 Redis 리포지토리 기능을 사용하기 위한 필수 구성요소다.
8-8: 훌륭해, fool! @EnableRedisRepositories 어노테이션으로 Redis 리포지토리를 제대로 활성화했군!이 어노테이션은 Spring Data Redis 리포지토리들이 자동으로 감지되고 구성되도록 해준다. I pity the fool who forgets to enable Redis repositories when implementing token management with Redis!
새로운 RedisRefreshTokenRepository와 다른 Redis 기반 컴포넌트들이 제대로 작동하려면 이 설정이 반드시 필요하다.
src/main/resources/scripts/login_refresh_token.lua (1)
19-29: I pity the fool who leaves old tokens lying around! This cleanup logic is SOLID!이 토큰 무효화 및 정리 로직은 보안 관점에서 매우 좋습니다:
- 기존 토큰 무효화: 로그인 시 이전 토큰들을 모두 revoked=1로 설정
- 데드 키 정리: 존재하지 않는 키들을 인덱스에서 제거하는 자동 정리 기능
이런 방식은 refresh token rotation 보안 전략에 완벽하게 부합합니다!
src/main/resources/scripts/rotate_refresh_token.lua (2)
56-62: This validation logic is TIGHT! I approve, fool!HMGET를 사용해서 한 번에 여러 필드를 가져오는 것은 효율적입니다. 토큰 존재 여부 확인 로직도 올바릅니다.
64-78: These security checks are SOLID GOLD! Device validation is crucial!보안 검증 로직이 훌륭합니다:
- 토큰 무효화 상태 확인: 이미 무효화된 토큰 사용 시 모든 토큰 무효화
- 디바이스 ID 검증: 다른 디바이스에서 토큰 사용 시도 시 보안 조치
이런 보안 조치는 토큰 탈취 공격을 효과적으로 방어할 수 있습니다!
docs/pr/PR-139-refactor---auth-api.md (1)
1-97: 이 문서는 정말 멋지다, fool!인증 API 리팩터링에 대한 포괄적이고 체계적인 설명이다. Refresh Token Rotation 전략, Redis 기반 토큰 관리, 그리고 보안 강화 방안들이 명확하게 설명되어 있어서 I ain't got no problems with this documentation, sucka!
특히 기존 경로에서 새로운 경로로의 마이그레이션 테이블과 보안 고려사항들이 잘 정리되어 있다.
src/main/java/com/juu/juulabel/common/util/AuthorizationExtractor.java (1)
9-16: 이 클래스 디자인은 괜찮다, fool!final 클래스에 private 생성자를 사용해서 인스턴스화를 방지하는 것은 올바른 유틸리티 클래스 패턴이다. I appreciate that you're doing it right, sucka!
src/main/java/com/juu/juulabel/admin/AdminController.java (1)
16-18: I pity the fool who don't appreciate clean code! 이 변경사항들이 좋아 보이네요!코드 포맷팅이 더 일관성 있고 읽기 쉽게 개선되었습니다. 특히 어노테이션을 한 줄로 정리하고 메서드 파라미터를 분리한 것이 좋습니다.
Also applies to: 27-28
src/main/java/com/juu/juulabel/common/util/DeviceIdExtractor.java (1)
27-34: 이 구현은 A급이야! I approve, fool!디바이스 ID 추출 로직이 견고하게 구현되었습니다:
- null과 빈 문자열 검증이 적절합니다
- trim() 처리로 공백 문제를 방지합니다
- 명확한 에러 코드로 예외를 처리합니다
src/main/java/com/juu/juulabel/common/exception/handler/GlobalExceptionHandler.java (2)
53-56: Smart move, fool! 404는 에러가 아니야!
NoResourceFoundException의 로그 레벨을error에서warn으로 변경한 것이 적절합니다. 404 응답은 정상적인 흐름의 일부이므로 에러로 취급할 필요가 없습니다.
75-89: 이 예외 처리는 완벽해! Mr. T approves!
MethodArgumentTypeMismatchException핸들러의 구현이 훌륭합니다:
- cause chain을 순회하여
BaseException을 찾는 로직이 영리합니다- 커스텀 에러 코드를 우선적으로 사용하여 더 정확한 에러 응답을 제공합니다
- 다른 핸들러들과 일관된 패턴을 유지합니다
src/main/java/com/juu/juulabel/common/util/UserAgentExtractor.java (1)
9-16: Good work on the utility pattern, fool! 유틸리티 클래스 구조가 올바르게 구현되었습니다.private constructor와 static 메서드를 사용한 유틸리티 패턴이 올바르게 적용되어 있고, AbstractHttpUtil을 상속받아 일관성을 유지하고 있습니다.
src/main/java/com/juu/juulabel/common/exception/code/ErrorCode.java (2)
24-24: I approve this addition, fool! NOT_FOUND 에러 코드가 적절히 추가되었습니다.공통적으로 사용될 수 있는 NOT_FOUND 에러 코드가 올바른 HTTP 상태코드(404)와 함께 추가되어 리소스를 찾을 수 없는 상황을 처리할 수 있게 되었습니다.
37-43: Perfect authentication error codes, you beautiful fool! 인증 관련 에러 코드들이 체계적으로 추가되었습니다.새로운 인증 에러 코드들이 토큰 로테이션 및 디바이스 ID 검증 로직에 필요한 모든 시나리오를 커버하고 있습니다:
- DEVICE_ID_REQUIRED: 필수 헤더 누락
- REFRESH_TOKEN_NOT_FOUND: 토큰 부재
- DEVICE_ID_MISMATCH: 디바이스 불일치 보안 검증
- REFRESH_TOKEN_INVALID: 유효하지 않은 토큰
- REFRESH_TOKEN_REUSE_DETECTED: 토큰 재사용 보안 위협 감지
각각 적절한 HTTP 상태코드와 명확한 한국어 메시지를 가지고 있어 인증 시스템의 보안성과 사용자 경험을 향상시킵니다.
src/main/java/com/juu/juulabel/common/config/WebConfig.java (1)
9-18: I pity the fool who doesn't appreciate clean configuration! 웹 설정이 깔끔하게 구현되었습니다.Spring MVC 설정이 올바르게 구현되어 있습니다:
- @configuration과 WebMvcConfigurer 사용으로 적절한 Spring 설정 패턴 적용
- 생성자 주입을 통한 의존성 관리
- ProviderConverter 등록으로 OAuth 프로바이더 문자열 자동 변환 지원
단순하고 명확한 목적을 가진 설정 클래스로 유지보수성이 좋습니다.
src/main/java/com/juu/juulabel/auth/domain/ClientId.java (2)
6-21: Beautiful enum design, fool! ClientId 열거형이 잘 설계되었습니다.클라이언트 타입을 나타내는 enum이 올바르게 구현되어 있습니다:
- 명확한 클라이언트 타입 정의 (WEB, IOS, ANDROID, ADMIN)
- 적절한 문자열 값 매핑
- 캡슐화된 value 필드와 getter 메서드
- 타입 안전성 제공
토큰 관리 시스템에서 클라이언트별 구분이 가능하게 하여 보안성을 향상시킵니다.
22-26: 🛠️ Refactor suggestionI pity the fool who doesn't validate input! from() 메서드에 null 체크를 추가하세요.
현재 from() 메서드는 null 입력값에 대한 처리가 없어 NullPointerException이 발생할 수 있습니다. 안전한 변환을 위해 null 체크를 추가해야 합니다.
다음과 같이 개선할 수 있습니다:
public static Optional<ClientId> from(String value) { + if (value == null) { + return Optional.empty(); + } return Arrays.stream(values()) .filter(c -> c.value.equals(value)) .findFirst(); }Likely an incorrect or invalid review comment.
src/main/java/com/juu/juulabel/common/constants/AuthConstants.java (1)
15-16: RFC 6648 참조 주석 추가한 건 잘했어!커스텀 헤더에 X- 접두사를 사용하지 않는 이유를 명확히 설명해줘서 좋아! I approve this, fool!
src/main/java/com/juu/juulabel/auth/repository/RefreshTokenRepository.java (1)
6-33: 이 인터페이스는 좋다, fool!깔끔하고 잘 설계된 repository 인터페이스다. 메서드 시그니처가 명확하고 문서화도 적절하다. 토큰 생명주기 관리를 위한 모든 필수 작업들이 포함되어 있다.
src/main/java/com/juu/juulabel/tastingnote/domain/TastingNote.java (1)
69-75: 좋은 포맷팅 개선이다, sucka!메서드 파라미터를 각각 별도 라인으로 분리한 것은 가독성을 크게 향상시킨다. 이런 일관된 코드 스타일링이 바로 프로페셔널한 개발자가 하는 일이다!
Also applies to: 89-95
src/main/java/com/juu/juulabel/auth/executor/RedisScriptExecutor.java (1)
15-28: 에러 핸들링 로직은 훌륭하다, fool!예외 unwrapping 로직이 잘 구현되어 있고,
RedisCommandExecutionException을 적절히 찾아내는 방식이 좋다. 이런 방어적 프로그래밍이 바로 내가 좋아하는 스타일이다!src/main/java/com/juu/juulabel/member/controller/MemberController.java (3)
44-48: I approve of this fool's endpoint! That's some fine API design!내 정보 조회 엔드포인트가 잘 구현되었습니다. 인증 처리와 문서화가 적절합니다.
50-54: I approve of this fool's space endpoint! Clean and simple!내 공간 조회 엔드포인트도 깔끔하게 구현되었습니다. 일관된 패턴을 따르고 있습니다.
56-62: I approve of this fool's profile viewing! Good security check!타 유저 프로필 조회 엔드포인트가 잘 구현되었습니다. 인증된 사용자만 접근할 수 있도록 보안이 적절히 처리되었습니다.
src/main/java/com/juu/juulabel/member/service/MemberProfileService.java (3)
49-52: I approve of this fool's nickname check! Simple and effective!닉네임 중복 확인 메서드가 깔끔하게 구현되었습니다. readOnly 트랜잭션 사용도 적절합니다.
57-71: I approve of this fool's profile update! Well structured transaction!프로필 업데이트 로직이 잘 구성되었습니다. 트랜잭션 범위와 알코올 타입 업데이트 처리가 적절합니다.
143-164: I approve of this fool's caching strategy! That's smart optimization!타 유저 프로필 조회에 캐싱을 적용한 것이 성능 최적화에 좋습니다.
unless = "#result == null"조건도 적절합니다.src/main/java/com/juu/juulabel/auth/service/TokenService.java (2)
32-41: I approve of this fool's token creation! That's solid security!토큰 쌍 생성 로직이 잘 구현되었습니다. 액세스 토큰과 리프레시 토큰을 적절히 분리하고 해싱을 사용한 것이 보안에 좋습니다.
68-81: I approve of this fool's token rotation! That's proper lifecycle management!토큰 로테이션 로직이 보안 모범 사례를 잘 따르고 있습니다. 기존 토큰 무효화와 새 토큰 생성이 원자적으로 처리됩니다.
src/main/java/com/juu/juulabel/auth/executor/RotateRefreshTokenScriptExecutor.java (1)
54-62: 에러 처리가 훌륭하다!각 Redis 스크립트 에러에 대한 구체적인 예외 처리가 잘 되어 있다. I respect the fool who handles errors properly!
src/main/java/com/juu/juulabel/member/util/MemberUtils.java (2)
96-97: 성능 최적화가 훌륭하다!약관 ID를 키로 하는 맵으로 변환하여 조회 성능을 개선한 것이 좋다. I respect the fool who thinks about performance!
108-110: 검증 로직이 견고하다!필수 약관에 대한 검증과 적절한 예외 처리가 잘 되어 있다. I respect the fool who validates properly!
src/main/java/com/juu/juulabel/auth/controller/AuthController.java (1)
50-57:Details
❓ Verification inconclusive
사용하지 않는 member 파라미터가 있어요!
@AuthenticationPrincipal Member member가 선언되었지만 메서드에서 사용하지 않아요. Spring Security 검증을 위한 건지 확인이 필요해요!이 파라미터가 필요한지 확인하세요:
- Spring Security 인증 검증용이라면 주석을 추가하세요
- 불필요하다면 제거하세요
- 추가 검증이 필요하다면 로직을 구현하세요
🏁 Script executed:
#!/bin/bash # AuthApiDocs 인터페이스에서 member 파라미터 사용 확인 ast-grep --pattern 'interface AuthApiDocs { $$$ refresh($$$Member member$$$) { $$$ } $$$ }'Length of output: 105
잘 봐라, fool! 아직 AuthApiDocs 인터페이스에서
refresh메서드 시그니처를 못 찾았으니, 다시 한 번 위치랑 파라미터 포함 여부를 확인해보자고! 아래 스크립트를 돌려봐:
🏁 Script executed:
#!/bin/bash # AuthApiDocs.java 파일 경로 찾기 및 refresh 메서드 시그니처 확인 auth_docs=$(fd AuthApiDocs.java) if [ -z "$auth_docs" ]; then echo "AuthApiDocs.java 파일을 찾을 수 없습니다." exit 1 fi echo "Found AuthApiDocs: $auth_docs" # 인터페이스 선언부와 refresh 메서드 시그니처 출력 sed -n '/interface AuthApiDocs/,/}/p' "$auth_docs" rg "refresh\\s*\\(" -n "$auth_docs"Length of output: 964
이 바보야!
AuthApiDocs.refresh메서드 전체 시그니처를 봐야 뭘 할지 알지. 아래 스크립트로 65~75라인을 출력해 봐라!
🏁 Script executed:
#!/bin/bash # AuthApiDocs.refresh 메서드 전체 시그니처 확인 file=src/main/java/com/juu/juulabel/auth/controller/AuthApiDocs.java if [ ! -f "$file" ]; then echo "파일을 찾을 수 없습니다: $file" exit 1 fi # 65~75라인 출력 sed -n '65,75p' "$file"Length of output: 951
멍청아, refresh 메서드의
member파라미터 사용 여부를 검증해라!
I pity the fool who ignores this!이 파라미터가 메서드 바디에서 실제로 사용되지 않습니다. 다음 위치를 확인해 보세요:
- src/main/java/com/juu/juulabel/auth/controller/AuthController.java (refresh 메서드)
- src/main/java/com/juu/juulabel/auth/controller/AuthApiDocs.java (refresh 메서드 시그니처)
이 파라미터가 Spring Security 인증 검증용인지, 불필요한지, 혹은 추가 검증 로직이 필요한지 확인하세요:
- 필요하다면 주석으로 의도를 명시하거나
@Parameter(hidden = true)등으로 문서에서 숨기기- 불필요하다면 구현과 문서 양쪽에서 모두 제거
- 추가 검증 로직이 필요하다면 해당 로직을 구현
src/main/java/com/juu/juulabel/auth/controller/AuthApiDocs.java (3)
32-34: 이 바보 같은 문서화가 완벽하다고!Swagger 태그와 RequestMapping이 깔끔하게 정의되어 있고, API 문서화 구조가 매우 체계적입니다. 인증 API의 전체적인 설계가 잘 드러나 있어요.
36-45: OAuth 로그인 문서화가 훌륭하다고, 바보야!Provider를 PathVariable로 받는 설계가 RESTful하고, 응답 헤더에 대한 설명도 명확합니다. 계정 존재 여부에 따른 refresh token 발급 로직이 잘 문서화되어 있어요.
56-69: 토큰 갱신 로직의 보안 설계가 인상적이다, 바보야!토큰 재사용 감지(403)와 즉시 삭제 정책이 잘 문서화되어 있습니다. 이는 refresh token rotation 패턴의 보안 모범 사례를 제대로 따르고 있어요.
src/main/java/com/juu/juulabel/auth/service/AuthService.java (2)
34-41: 의존성 주입이 깔끔하다고, 바보야!RequiredArgsConstructor를 사용한 의존성 주입이 깔끔하고, 책임이 잘 분리된 repository와 service들이 주입되어 있습니다.
110-114: 탈퇴 회원 검증 로직이 훌륭하다고!이메일 기반의 탈퇴 회원 검증이 적절하게 구현되어 있고, 명확한 예외 처리가 되어 있습니다.
src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java (1)
170-178: 신뢰도 평가 로직이 스마트하다고!헤더별 신뢰도 평가와 public IP 여부에 따른 동적 평가가 잘 구현되어 있습니다. 보안과 모니터링에 유용한 정보를 제공해요.
src/main/java/com/juu/juulabel/member/service/MemberContentService.java (3)
31-37: 서비스 클래스 설계가 깔끔하다고, 바보야!책임이 명확하게 분리되어 있고, 회원 콘텐츠 관련 작업들이 잘 그룹화되어 있습니다. 주석으로 기능별 구분도 명확해요.
105-121: 전통주 저장/취소 토글 로직이 훌륭하다고!Optional을 사용한 토글 로직이 깔끔하고, 반환값으로 저장/취소 상태를 명확하게 전달하고 있습니다. 코드 가독성도 매우 좋아요.
53-59: 읽기 전용 트랜잭션 사용이 훌륭하다고!조회 메서드들에
@Transactional(readOnly = true)를 적절히 사용하여 성능 최적화와 의도를 명확하게 표현했습니다.src/main/java/com/juu/juulabel/member/service/MemberService.java (5)
12-22: 파사드 패턴 구현이 깔끔하다, fool!이 리팩터링은 정말 멋지다! 의존성 분리가 제대로 되어 있고, JavaDoc 문서화도 완벽하다. I pity the fool who doesn't appreciate this clean architecture!
세 개의 전문화된 서비스 클래스로 책임을 분리한 것이 매우 좋다:
MemberProfileService: 프로필 관련 작업MemberLookupService: 회원 조회 작업MemberContentService: 콘텐츠 관련 작업
24-62: 프로필 관련 메서드들의 위임이 완벽하다!모든 프로필 관련 메서드들이
MemberProfileService로 깔끔하게 위임되고 있다. Transaction 어노테이션도 적절하게 사용되었고, JavaDoc 문서화도 훌륭하다. I ain't got time for messy code, but this is clean as a whistle!각 메서드의 책임이 명확하고 단일 책임 원칙을 잘 따르고 있다.
64-78: 회원 조회 로직의 위임이 정확하다!
MemberLookupService로의 위임이 완벽하다. ID와 이메일로 회원을 조회하는 두 가지 방법 모두 적절하게 처리되고 있다. I pity the fool who thinks this delegation pattern isn't gold!읽기 전용 트랜잭션 설정도 올바르게 적용되어 있다.
80-130: 콘텐츠 관련 메서드들의 위임이 훌륭하다!
MemberContentService로의 모든 위임이 완벽하게 구현되어 있다. 일상생활, 시음노트, 전통주 관련 모든 기능이 적절하게 분리되었다.특히
saveAlcoholicDrinks메서드의 boolean 반환값 문서화가 매우 명확하다 - "true if saved, false if unsaved". I got no time for ambiguous return values, and this ain't one of them!
16-131:Details
✅ Verification successful
전체적인 파사드 패턴 구현을 검증해보자!
이 리팩터링이 정말로 모든 의존성이 올바르게 주입되고 사용되는지 확인해보자. I pity the fool who doesn't verify their refactoring!
다음 스크립트를 실행하여 세 개의 서비스 클래스가 실제로 존재하고 올바르게 구현되었는지 확인해보자:
🏁 Script executed:
#!/bin/bash # Description: 파사드 패턴에서 사용되는 세 개의 서비스 클래스들이 존재하는지 확인 echo "=== MemberProfileService 확인 ===" fd "MemberProfileService.java" --exec cat {} \; | head -20 echo -e "\n=== MemberLookupService 확인 ===" fd "MemberLookupService.java" --exec cat {} \; | head -20 echo -e "\n=== MemberContentService 확인 ===" fd "MemberContentService.java" --exec cat {} \; | head -20 echo -e "\n=== 세 서비스 클래스들이 모두 @Service 어노테이션을 가지고 있는지 확인 ===" rg -A 5 "@Service" | rg "MemberProfileService|MemberLookupService|MemberContentService"Length of output: 5601
파사드 패턴 구현 검증 완료
MemberProfileService, MemberLookupService, MemberContentService가 모두 존재하며 @service 어노테이션도 제대로 적용된 것을 확인했습니다. 리팩터링 후 의존성 주입에는 이상이 없습니다. I pity the fool who doesn't!
src/main/java/com/juu/juulabel/common/converter/ProviderConverter.java
Outdated
Show resolved
Hide resolved
src/main/resources/scripts/revoke_refresh_token_by_index_key.lua
Outdated
Show resolved
Hide resolved
src/main/resources/scripts/revoke_refresh_token_by_index_key.lua
Outdated
Show resolved
Hide resolved
src/main/java/com/juu/juulabel/common/filter/JwtAuthorizationFilter.java
Show resolved
Hide resolved
src/main/java/com/juu/juulabel/auth/repository/RedisRefreshTokenRepository.java
Show resolved
Hide resolved
src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java
Outdated
Show resolved
Hide resolved
- Removed unused generated QueryDSL classes related to alcohol, daily life, and member domains to streamline the codebase. - Updated build.gradle to remove unnecessary dependencies and improve formatting. - Enhanced code readability by adjusting indentation and spacing in various configuration and utility classes. - Improved Redis token revocation script for better performance with large datasets by switching from KEYS to SCAN.
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
build.gradle (1)
74-78: 💡 Verification agent🧩 Analysis chain
QueryDSL 설정 제거를 확인해라, 바보야!
QueryDSL 관련 의존성은 여전히 있는데 빌드 설정만 제거되었다니! I pity the fool who leaves inconsistent configurations! 프로젝트에서 QueryDSL을 완전히 제거하는 건지, 아니면 다른 방식으로 설정하는 건지 확인이 필요하다.
다음 스크립트로 QueryDSL 사용 여부를 확인해라:
🏁 Script executed:
#!/bin/bash # QueryDSL 관련 파일과 사용처 확인 echo "=== QueryDSL 생성된 파일 확인 ===" fd -t f -e java "Q.*\.java" echo "=== QueryDSL 사용처 확인 ===" rg -A 3 -B 3 "import.*Q[A-Z]" echo "=== QueryDSL JPAQueryFactory 사용 확인 ===" rg -A 3 "JPAQueryFactory"Length of output: 55835
QueryDSL 설정 제거 불완전: 코드에 여전히 QueryDSL 사용 발견! I pity the fool who ignores this!
QuerydslConfig와 다수의 Repository에서 Q 클래스 및 JPAQueryFactory 참조가 확인되었습니다:
- src/main/java/com/juu/juulabel/common/config/QuerydslConfig.java
- src/main/java/com/juu/juulabel/**/repository/query/*QueryRepository.java
- src/main/java/com/juu/juulabel/**/domain/Q*.java
빌드 설정만 제거된 상태로 QueryDSL 의존성을 남겨두면 빌드 오류가 발생합니다.
- QueryDSL을 완전히 제거하려면 위 클래스들의 Q 타입 참조와 JPAQueryFactory 사용 코드를 모두 삭제하세요.
- QueryDSL을 유지할 계획이라면 build.gradle에 implementation 및 annotationProcessor 설정을 반드시 유지/복원해야 합니다.
바보 같은 실수를 그만두고 명확한 방향을 정하세요!
♻️ Duplicate comments (1)
src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java (1)
38-40: 🛠️ Refactor suggestionIPv6 패턴이 개선되었지만 여전히 완벽하지 않아, 바보야!
이전 댓글에서 지적된 IPv6 정규식이 개선되었지만, 여전히 모든 유효한 IPv6 형식을 완전히 커버하지 못할 수 있어. IPv6 mixed notation (예:
::ffff:192.0.2.1)이나 zone identifier 같은 특수 케이스들이 빠져있어!
InetAddress.getByName()이 이미 완벽한 검증을 제공하는데 정규식과 중복으로 사용하고 있어. 더 안전하고 간단한 방법을 고려해봐야 해!- // IPv6 pattern (simplified) - private static final Pattern IPV6_PATTERN = Pattern.compile( - "^(([0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|:((:[0-9a-fA-F]{1,4}){1,7}|:)|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4})$"); + // Remove IPv6 pattern - InetAddress.getByName() handles all valid IPv6 formats그리고
isValidIpAddress메서드를 단순화해:private static boolean isValidIpAddress(String ip) { if (ip == null || ip.trim().isEmpty()) { return false; } try { InetAddress.getByName(ip); - return IPV4_PATTERN.matcher(ip).matches() || IPV6_PATTERN.matcher(ip).matches(); + return true; // InetAddress.getByName() already validates } catch (UnknownHostException e) { return false; } }
🧹 Nitpick comments (2)
src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java (2)
57-75: 복잡한 스트림 체인이 성능을 잡아먹고 있어, 바보야!중첩된 스트림과 중복된 로직이 성능에 악영향을 줄 수 있어. 같은 헤더를 두 번 검사하고 있고, fallback 로직이 너무 복잡해!
public static String getClientIpAddress() { HttpServletRequest request = getCurrentRequest(); - - return IP_HEADER_CANDIDATES.stream() - .map(request::getHeader) - .filter(ip -> ip != null && !ip.isEmpty() && !UNKNOWN.equalsIgnoreCase(ip)) - .map(ip -> ip.split(",")[0].trim()) // Take first IP from comma-separated list - .filter(IpAddressExtractor::isValidIpAddress) - .filter(IpAddressExtractor::isPublicIpAddress) // Prefer public IPs - .findFirst() - .orElseGet(() -> { - // Fallback: try to get any valid IP (including private) - String fallbackIp = IP_HEADER_CANDIDATES.stream() - .map(request::getHeader) - .filter(ip -> ip != null && !ip.isEmpty() && !UNKNOWN.equalsIgnoreCase(ip)) - .map(ip -> ip.split(",")[0].trim()) - .filter(IpAddressExtractor::isValidIpAddress) - .findFirst() - .orElse(request.getRemoteAddr()); - - return fallbackIp != null ? fallbackIp : "unknown"; - }); + + String publicIp = null; + String anyValidIp = null; + + for (String headerName : IP_HEADER_CANDIDATES) { + String headerValue = request.getHeader(headerName); + if (headerValue != null && !headerValue.isEmpty() && !UNKNOWN.equalsIgnoreCase(headerValue)) { + String ip = headerValue.split(",")[0].trim(); + if (isValidIpAddress(ip)) { + if (anyValidIp == null) anyValidIp = ip; + if (isPublicIpAddress(ip)) { + publicIp = ip; + break; // Found public IP, use it immediately + } + } + } + } + + if (publicIp != null) return publicIp; + if (anyValidIp != null) return anyValidIp; + + String remoteAddr = request.getRemoteAddr(); + return remoteAddr != null ? remoteAddr : "unknown"; }
183-205: 데이터 클래스에 불변성 보장이 약해, 바보야!필드들이 final인 건 좋지만, defensive copying이나 validation이 없어! null 값이 들어올 수 있고 toString(), equals(), hashCode() 메서드도 없어!
public static class IpAddressInfo { private final String ipAddress; private final String sourceHeader; private final ReliabilityLevel reliability; public IpAddressInfo(String ipAddress, String sourceHeader, ReliabilityLevel reliability) { - this.ipAddress = ipAddress; - this.sourceHeader = sourceHeader; - this.reliability = reliability; + this.ipAddress = ipAddress != null ? ipAddress : "unknown"; + this.sourceHeader = sourceHeader != null ? sourceHeader : "unknown"; + this.reliability = reliability != null ? reliability : ReliabilityLevel.LOW; } public String getIpAddress() { return ipAddress; } public String getSourceHeader() { return sourceHeader; } public ReliabilityLevel getReliability() { return reliability; } + + @Override + public String toString() { + return String.format("IpAddressInfo{ip='%s', source='%s', reliability=%s}", + ipAddress, sourceHeader, reliability); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + IpAddressInfo that = (IpAddressInfo) obj; + return Objects.equals(ipAddress, that.ipAddress) && + Objects.equals(sourceHeader, that.sourceHeader) && + reliability == that.reliability; + } + + @Override + public int hashCode() { + return Objects.hash(ipAddress, sourceHeader, reliability); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (42)
src/main/generated/com/juu/juulabel/alcohol/domain/QAlcoholType.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QAlcoholTypeColor.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QAlcoholTypeFlavor.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QAlcoholTypeScent.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QAlcoholTypeSensory.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QAlcoholicDrinks.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QAlcoholicDrinksIngredient.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QAlcoholicDrinksPairing.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QBrewery.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QColor.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QFlavor.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QFlavorLevel.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QIngredient.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QPairing.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QScent.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QSensory.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/alcohol/domain/QSensoryLevel.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/category/domain/QCategory.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/common/base/QBaseCreatedTimeEntity.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/common/base/QBaseTimeEntity.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/dailylife/domain/QDailyLife.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/dailylife/domain/QDailyLifeComment.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/dailylife/domain/QDailyLifeCommentLike.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/dailylife/domain/QDailyLifeImage.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/dailylife/domain/QDailyLifeLike.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/follow/domain/QFollow.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/member/domain/QMember.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/member/domain/QMemberAlcoholType.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/member/domain/QMemberAlcoholicDrinks.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/member/domain/QMemberTerms.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/member/domain/QWithdrawalRecord.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/notification/domain/QNotification.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/tastingnote/domain/QTastingNote.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/tastingnote/domain/QTastingNoteComment.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/tastingnote/domain/QTastingNoteCommentLike.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/tastingnote/domain/QTastingNoteFlavorLevel.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/tastingnote/domain/QTastingNoteImage.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/tastingnote/domain/QTastingNoteLike.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/tastingnote/domain/QTastingNoteScent.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/tastingnote/domain/QTastingNoteSensoryLevel.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/tastingnote/domain/embedded/QAlcoholicDrinksSnapshot.javais excluded by!**/generated/**src/main/generated/com/juu/juulabel/terms/domain/QTerms.javais excluded by!**/generated/**
📒 Files selected for processing (6)
build.gradle(1 hunks)src/main/java/com/juu/juulabel/common/config/QuerydslConfig.java(1 hunks)src/main/java/com/juu/juulabel/common/converter/ProviderConverter.java(1 hunks)src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java(1 hunks)src/main/java/com/juu/juulabel/s3/S3Controller.java(2 hunks)src/main/resources/scripts/revoke_refresh_token_by_index_key.lua(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/java/com/juu/juulabel/common/config/QuerydslConfig.java
- src/main/java/com/juu/juulabel/s3/S3Controller.java
🔇 Additional comments (5)
src/main/java/com/juu/juulabel/common/converter/ProviderConverter.java (1)
16-16: 허용된 Provider 설정이 적절하다, 바보야!
KAKAO만 허용하는 것은 현재 지원하는 OAuth 제공자에 맞게 잘 설정되어 있다니! I pity the fool who tries to use unsupported providers!src/main/resources/scripts/revoke_refresh_token_by_index_key.lua (3)
3-10: SCAN 사용으로 성능 개선했다니, 잘했어 바보야!이전 리뷰에서 지적된
KEYS명령어 문제를SCAN으로 해결했다니! I pity the fool who still uses blocking KEYS in production! 배치 크기도 100으로 적절하게 설정했고, 논블로킹 방식으로 개선되었다.
15-31: 배치 처리로 Redis 호출 최적화했다니, 똑똑한 바보야!토큰 철회를 배치로 처리해서 Redis 호출 횟수를 줄였다니!
unpack(batch)를 사용해서 여러HSET명령어를 한 번에 실행하는 것도 효율적이다. I pity the fool who doesn't batch their Redis operations!
33-35: 인덱스 키 정리도 제대로 했다니, 바보야!토큰들을 철회한 후에 인덱스 키를
DEL로 삭제하는 것도 메모리 누수를 방지하는 좋은 방법이다니! 이전 리뷰의tokenKey변수 스코프 문제도 해결되었다.build.gradle (1)
36-37: Redis 의존성 추가가 적절하다, 바보야!인증 API 리팩터링에 필요한
spring-boot-starter-data-redis의존성을 제대로 추가했다니! I pity the fool who tries to manage tokens without Redis! 토큰 관리와 블랙리스트 기능을 위해 필수적이다.
| private static boolean isSpecialAddress(String ip) { | ||
| return ip.equals("127.0.0.1") || ip.equals("::1") || ip.equals("0.0.0.0"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
특수 주소 체크가 너무 대충이야, 바보야!
localhost만 체크하고 다른 중요한 특수 주소들을 놓치고 있어! Broadcast, multicast, 그리고 다른 reserved 주소들도 고려해야 해!
private static boolean isSpecialAddress(String ip) {
- return ip.equals("127.0.0.1") || ip.equals("::1") || ip.equals("0.0.0.0");
+ if (ip.equals("127.0.0.1") || ip.equals("::1") || ip.equals("0.0.0.0")) {
+ return true;
+ }
+
+ try {
+ InetAddress addr = InetAddress.getByName(ip);
+ return addr.isLoopbackAddress() || addr.isMulticastAddress() ||
+ addr.isAnyLocalAddress() || ip.startsWith("169.254."); // Link-local
+ } catch (UnknownHostException e) {
+ return true; // If we can't parse it, treat as special/invalid
+ }
}🤖 Prompt for AI Agents
In src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java at lines
166 to 168, the method isSpecialAddress only checks for localhost addresses and
ignores other important special IP ranges such as broadcast, multicast, and
reserved addresses. Expand the method to include additional checks for these
special address ranges, ensuring comprehensive detection of all relevant special
IP addresses.
- Updated RedisConfig to use properties for host, port, and SSL settings, allowing for more flexible configuration. - Improved connection factory setup to conditionally enable SSL based on configuration values.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/juu/juulabel/common/config/RedisConfig.java (1)
32-39: 🛠️ Refactor suggestionI pity the fool who ignores SSL security warnings!
SSL peer verification을 비활성화하는 것은 중간자 공격에 취약합니다. 개발 환경에서만 사용하고 프로덕션에서는 적절한 인증서 설정을 권장합니다.
또한 연결 풀링 설정과 타임아웃 구성이 누락되어 성능에 영향을 줄 수 있습니다.
다음과 같이 개선하세요:
if (sslEnabled) { clientConfig = LettuceClientConfiguration.builder() .useSsl() - .disablePeerVerification() // trust any certificate (disable hostname check) + .and() + .commandTimeout(Duration.ofSeconds(2)) + .poolConfig(LettucePoolingClientConfiguration.builder() + .poolConfig(GenericObjectPoolConfig.defaultConfig()) + .build()) .build(); } else { - clientConfig = LettuceClientConfiguration.builder().build(); + clientConfig = LettuceClientConfiguration.builder() + .commandTimeout(Duration.ofSeconds(2)) + .poolConfig(LettucePoolingClientConfiguration.builder() + .poolConfig(GenericObjectPoolConfig.defaultConfig()) + .build()) + .build(); }
🧹 Nitpick comments (3)
src/main/java/com/juu/juulabel/common/config/RedisConfig.java (3)
17-24: I pity the fool who forgot the password configuration!속성 주입이 호스트와 포트는 잘 구성되어 있지만, Redis 인증 정보가 누락되었습니다. 프로덕션 환경에서는 보안을 위해 패스워드 설정이 필요할 수 있습니다.
@Value("${spring.data.redis.ssl.enabled}") private boolean sslEnabled; +@Value("${spring.data.redis.password:}") +private String redisPassword;
28-28: I pity the fool who forgot authentication setup!Redis 연결 구성에서 패스워드 설정이 누락되었습니다. 보안이 필요한 환경에서는 인증 정보를 추가해야 합니다.
RedisStandaloneConfiguration config = new RedisStandaloneConfiguration(redisHost, redisPort); +if (redisPassword != null && !redisPassword.trim().isEmpty()) { + config.setPassword(redisPassword); +}
26-42: I pity the fool who doesn't handle connection failures!Redis 연결 팩토리 구성에서 연결 실패 시 예외 처리가 없습니다. 애플리케이션 시작 시 Redis 연결 문제를 조기에 발견할 수 있도록 연결 검증을 추가하는 것이 좋습니다.
@Bean LettuceConnectionFactory redisConnectionFactory() { RedisStandaloneConfiguration config = new RedisStandaloneConfiguration(redisHost, redisPort); // ... client config setup ... - return new LettuceConnectionFactory(config, clientConfig); + LettuceConnectionFactory factory = new LettuceConnectionFactory(config, clientConfig); + factory.setValidateConnection(true); + factory.afterPropertiesSet(); + return factory; }
| RedisTemplate<String, Object> template = new RedisTemplate<>(); | ||
| template.setConnectionFactory(factory); | ||
| template.setKeySerializer(new StringRedisSerializer()); | ||
| template.setValueSerializer(new Jackson2JsonRedisSerializer<>(Object.class)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
I pity the fool who uses deprecated serializers!
Jackson2JsonRedisSerializer의 Object.class 사용은 타입 안전성 문제와 잠재적인 보안 위험을 초래할 수 있습니다. GenericJackson2JsonRedisSerializer 사용을 권장합니다.
-template.setValueSerializer(new Jackson2JsonRedisSerializer<>(Object.class));
+template.setValueSerializer(new GenericJackson2JsonRedisSerializer());또한 hash key와 hash value serializer도 설정하는 것이 좋습니다:
template.setKeySerializer(new StringRedisSerializer());
+template.setHashKeySerializer(new StringRedisSerializer());
template.setValueSerializer(new GenericJackson2JsonRedisSerializer());
+template.setHashValueSerializer(new GenericJackson2JsonRedisSerializer());
+template.afterPropertiesSet();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| template.setValueSerializer(new Jackson2JsonRedisSerializer<>(Object.class)); | |
| template.setKeySerializer(new StringRedisSerializer()); | |
| template.setHashKeySerializer(new StringRedisSerializer()); | |
| template.setValueSerializer(new GenericJackson2JsonRedisSerializer()); | |
| template.setHashValueSerializer(new GenericJackson2JsonRedisSerializer()); | |
| template.afterPropertiesSet(); |
🤖 Prompt for AI Agents
In src/main/java/com/juu/juulabel/common/config/RedisConfig.java at line 49, the
code uses Jackson2JsonRedisSerializer with Object.class, which is deprecated and
can cause type safety and security issues. Replace it with
GenericJackson2JsonRedisSerializer for better safety. Additionally, set
serializers for hash keys and hash values to ensure consistent serialization
behavior across Redis operations.
youngjun-k
changed the title
- Replaced InvalidParamException with BaseException in ProviderConverter for improved error handling. - Added support for IPv6 private address validation in IpAddressExtractor, enhancing the method to check for private IP ranges.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java (2)
181-183: 특수 주소 체크가 여전히 부족해, 바보야!이전 리뷰에서 지적된 문제가 아직 해결되지 않았어! localhost만 체크하고 다른 중요한 특수 주소들을 놓치고 있어! Broadcast, multicast, 그리고 다른 reserved 주소들도 고려해야 해! I pity the fool who only checks localhost!
private static boolean isSpecialAddress(String ip) { - return ip.equals("127.0.0.1") || ip.equals("::1") || ip.equals("0.0.0.0"); + if (ip.equals("127.0.0.1") || ip.equals("::1") || ip.equals("0.0.0.0")) { + return true; + } + + try { + InetAddress addr = InetAddress.getByName(ip); + return addr.isLoopbackAddress() || addr.isMulticastAddress() || + addr.isAnyLocalAddress() || ip.startsWith("169.254."); // Link-local + } catch (UnknownHostException e) { + return true; // If we can't parse it, treat as special/invalid + } }
185-193: 헤더 신뢰도 로직의 보안 문제가 여전해, 바보야!이전 리뷰에서 지적된
X-Forwarded-For헤더 신뢰도 문제가 아직 해결되지 않았어! Public IP라는 이유만으로 MEDIUM 신뢰도를 주는 건 여전히 위험해! 이 헤더는 쉽게 스푸핑될 수 있거든! I pity the fool who trusts X-Forwarded-For too much!private static ReliabilityLevel getReliabilityLevel(String headerName, String ip) { // Rate headers by trustworthiness return switch (headerName) { case "CF-Connecting-IP", "True-Client-IP" -> ReliabilityLevel.HIGH; case "X-Real-IP", "X-Cluster-Client-IP" -> ReliabilityLevel.MEDIUM; - case "X-Forwarded-For" -> isPublicIpAddress(ip) ? ReliabilityLevel.MEDIUM : ReliabilityLevel.LOW; + case "X-Forwarded-For" -> { + // X-Forwarded-For is easily spoofed, be more conservative + if (isPublicIpAddress(ip) && !isSuspiciousIp(ip)) { + yield ReliabilityLevel.LOW; // Still low due to spoofing risk + } else { + yield ReliabilityLevel.LOW; + } + } default -> ReliabilityLevel.LOW; }; }
🧹 Nitpick comments (1)
src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java (1)
112-114: IP 검증 로직이 이중 체크하고 있어, 바보야!
InetAddress.getByName()로 검증한 다음에 또 정규식으로 검증하고 있어! 이건 약간 중복된 작업이야. InetAddress가 이미 유효성을 체크하니까 정규식은 불필요할 수도 있어. 하지만 정확한 형식 검증을 위해서라면 이해할 만해! I pity the fool who doesn't double-check IP validation!성능을 위해 다음과 같이 간소화할 수 있어:
private static boolean isValidIpAddress(String ip) { if (ip == null || ip.trim().isEmpty()) { return false; } try { InetAddress.getByName(ip); - return IPV4_PATTERN.matcher(ip).matches() || IPV6_PATTERN.matcher(ip).matches(); + return true; // InetAddress already validates the format } catch (UnknownHostException e) { return false; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/juu/juulabel/common/converter/ProviderConverter.java(1 hunks)src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java(1 hunks)
🔇 Additional comments (7)
src/main/java/com/juu/juulabel/common/converter/ProviderConverter.java (2)
12-15: 클래스 구조가 깔끔하다, fool!Spring
@Component애노테이션과Converter인터페이스 구현이 적절하고, 허용된 프로바이더들을 불변Set으로 정의한 것도 좋다. I pity the fool who doesn't appreciate this clean design!
20-26: 이전 리뷰 피드백 반영 잘했다, fool!예외 처리 일관성 문제를 해결했고, 불필요한 예외 타입들을 제거해서
BaseException으로 통일한 게 좋다.IllegalArgumentException만 잡아서 깔끔하게 처리하고 있다니, I ain't gonna pity this implementation!src/main/java/com/juu/juulabel/common/util/IpAddressExtractor.java (5)
34-40: IPv6 정규식 패턴이 훨씬 나아졌어, 바보야!이전 리뷰에서 지적된 IPv6 정규식 패턴 문제가 해결된 것 같아! 이제 다양한 IPv6 축약 표기법들을 훨씬 잘 처리할 수 있을 거야. I pity the fool who thought that simple pattern would work!
62-62: Public IP 우선 처리 로직이 똑똑하네, 바보야!Public IP를 우선적으로 필터링하는 로직이 보안상 좋은 접근이야! 하지만 만약 모든 요청이 private network에서 오는 환경이라면 fallback 로직도 잘 작동할 거야. I pity the fool who doesn't prioritize public IPs!
134-147: IPv6 private 주소 처리가 추가되었네, 바보야!이전 리뷰에서 지적된 IPv6 private 주소 범위 문제가 해결된 것 같아!
isPrivateIpv6메서드가 추가되어서 IPv6 private 범위들도 제대로 체크하고 있어. I pity the fool who forgot about IPv6 private ranges!
149-159: IPv6 private 범위 체크가 꼼꼼하네, 바보야!Site-local, link-local 주소와 ULA(fc00::/7, fd00::/7) 범위까지 모두 체크하고 있어! InetAddress API와 prefix 체크를 조합한 접근법이 좋아. I pity the fool who doesn't check all IPv6 private ranges!
198-220: IpAddressInfo 클래스가 깔끔하네, 바보야!IP 주소, 소스 헤더, 신뢰도 레벨을 모두 담는 immutable 데이터 클래스가 잘 설계되었어! 모니터링이나 로깅에 유용할 거야. I pity the fool who doesn't appreciate good data encapsulation!
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Summary by CodeRabbit
신규 기능
/auth경로로 분리되어 로그인, 회원가입, 토큰 재발급, 로그아웃, 회원 탈퇴 등 인증 관련 엔드포인트 제공버그 수정
문서화
리팩터링
스타일