Refactor public access policy and improve unauthenticated user handling #142
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.
- Added links to PR #139 in relevant documentation files for better traceability. - Cleaned up .gitignore by removing duplicate entries and ensuring proper formatting.
- 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.
- 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.
- 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.
…nto refactor/auth
- Updated method signatures for better alignment and clarity. - Enhanced query construction by utilizing Projections for cleaner code. - Replaced deprecated Objects utility with java.util.Objects for null checks. - Improved privacy condition handling in DailyLifeQueryRepository and TastingNoteQueryRepository. - Streamlined the isLikedSubQuery method to handle null member cases more effectively. - Adjusted formatting and indentation for better code readability across multiple files.
|
Caution Review failedThe pull request is closed. """ Walkthrough이 변경은 비회원(미인증 사용자)의 공개 접근 정책을 리팩토링하고, 여러 리포지토리 쿼리에서 인증되지 않은 사용자에 대한 null 처리와 코드 포맷을 개선했어. 또한 보안 설정이 재정비되어 GET 엔드포인트의 접근 권한이 세분화되고, 코드 전반에 걸쳐 불필요한 쿼리 실행을 방지하는 로직이 추가됐지. Changes
Sequence Diagram(s)sequenceDiagram
participant NonMember as 비회원 사용자
participant API as REST API
participant Security as SecurityConfig
participant Repo as Repository
NonMember->>API: GET /public-endpoint
API->>Security: 인증 체크 및 권한 확인
Security-->>API: 비회원 허용된 GET 엔드포인트면 통과
API->>Repo: 데이터 조회 쿼리 (member=null)
Repo->>Repo: 프라이버시 조건 분기 (비회원은 공개만)
Repo-->>API: 결과 반환
API-->>NonMember: 데이터 응답
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ 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 (
|
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
src/main/java/com/juu/juulabel/common/config/SecurityConfig.java (1)
121-121: 🛠️ Refactor suggestionCORS 설정에서 모든 메소드 허용, 조심해야 해!
addAllowedMethod("*")로 모든 HTTP 메소드를 허용하는 건 위험할 수 있어, fool! 실제로 필요한 메소드만 명시적으로 허용하는 게 더 안전해.- config.addAllowedMethod("*"); + config.addAllowedMethod("GET"); + config.addAllowedMethod("POST"); + config.addAllowedMethod("PUT"); + config.addAllowedMethod("DELETE"); + config.addAllowedMethod("OPTIONS");
🧹 Nitpick comments (6)
src/main/java/com/juu/juulabel/dailylife/repository/query/DailyLifeQueryRepository.java (1)
265-268: 불필요한 서브쿼리 방지 로직이 멋지다!비인증 사용자(member가 null)일 때
Expressions.FALSE를 반환하여 좋아요 여부 체크를 위한 불필요한 DB 쿼리를 방지합니다. 이는 성능 최적화에 큰 도움이 될 것입니다. I pity the fool who runs unnecessary queries!docs/pr/pr-140-refactor--public-access-control.md.md (1)
1-109: 완벽한 문서화 작업이다!이 문서는 비회원 접근 정책 리팩터링을 매우 명확하게 설명하고 있습니다:
- 변경 전후 비교가 명확함
- 새롭게 허용된 엔드포인트들이 체계적으로 정리됨
- 보안 설정 코드 예시가 포함됨
- 향후 고려사항까지 포함한 포괄적인 내용
특히 쓰기 작업(POST, PUT, DELETE)은 여전히 인증이 필요하도록 유지하면서 읽기 작업만 선별적으로 허용한 점이 훌륭합니다. I pity the fool who doesn't document their changes this well!
src/main/java/com/juu/juulabel/alcohol/repository/query/TastingNoteQueryRepository.java (2)
51-51: 하드코딩된 문자열을 상수로 교체한 것이 멋지다!
THUMBNAIL_PATH상수를 도입하여 코드 전체에서 사용되던"thumbnailPath"문자열 리터럴을 제거했습니다. 이는 유지보수성과 일관성을 크게 향상시킵니다. I pity the fool who hardcodes their strings!
422-425: 불필요한 좋아요 체크 쿼리 방지 로직이 완벽하다!DailyLife 리포지토리와 동일하게 비인증 사용자에 대해
Expressions.FALSE를 반환하여 불필요한 서브쿼리 실행을 방지합니다. 이는 성능 최적화와 일관성 측면에서 매우 중요한 개선사항입니다. I pity the fool who doesn't optimize their queries!src/main/java/com/juu/juulabel/common/config/SecurityConfig.java (1)
43-49: 보호된 GET 엔드포인트 추가, 똑똑하군!사용자 개인 정보와 관련된 엔드포인트들을 별도로 보호하는 건 좋은 접근이야. 하지만 이런 중요한 보안 설정은 문서화도 함께 해야 해, fool!
이 보안 정책 변경사항을 README나 API 문서에 추가하는 걸 도와줄까?
src/main/java/com/juu/juulabel/follow/repository/query/FollowQueryRepository.java (1)
292-315: 랜덤 추천 사용자 목록 메소드, 좋아 보여!RAND() 함수를 사용한 랜덤 정렬은 작은 데이터셋에는 괜찮지만, 사용자가 많아지면 성능 문제가 생길 수 있어. 나중에 사용자가 증가하면 다른 방법을 고려해봐!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/pr/pr-140-refactor--public-access-control.md.md(1 hunks)src/main/java/com/juu/juulabel/alcohol/repository/query/TastingNoteCommentQueryRepository.java(4 hunks)src/main/java/com/juu/juulabel/alcohol/repository/query/TastingNoteQueryRepository.java(12 hunks)src/main/java/com/juu/juulabel/common/config/SecurityConfig.java(3 hunks)src/main/java/com/juu/juulabel/dailylife/repository/query/DailyLifeCommentQueryRepository.java(4 hunks)src/main/java/com/juu/juulabel/dailylife/repository/query/DailyLifeQueryRepository.java(7 hunks)src/main/java/com/juu/juulabel/follow/repository/FollowReader.java(2 hunks)src/main/java/com/juu/juulabel/follow/repository/query/FollowQueryRepository.java(1 hunks)
🔇 Additional comments (15)
src/main/java/com/juu/juulabel/follow/repository/FollowReader.java (2)
12-12: 적절한 null 처리를 위한 import 추가, 멋지군!
Objects클래스 import가 추가되어 안전한 null 체크를 지원합니다. I pity the fool who doesn't handle nulls properly!
56-58:Details
✅ Verification successful
비인증 사용자를 위한 추천 로직 개선이 훌륭하다!
tastingLastUserId가 null일 때 랜덤 추천 목록을 반환하는 로직이 잘 구현되어 있습니다. 이는 비회원 사용자에게 개인화되지 않은 추천을 제공하는 좋은 접근법입니다. I pity the fool who doesn't give non-members a taste of the action!새로 추가된
findRandomRecommendUserList메소드가 올바르게 구현되었는지 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash # Description: Verify that findRandomRecommendUserList method exists in FollowQueryRepository # Expected: Method should be implemented and accessible ast-grep --pattern $'findRandomRecommendUserList($$$) { $$$ }'Length of output: 65
메서드 선언이 아직 확인되지 않았습니다. 프로젝트 전체에서
findRandomRecommendUserList와FollowQueryRepository를 검색해보겠습니다. I pity the fool who doesn't!
🏁 Script executed:
#!/bin/bash # Description: 프로젝트 전체에서 메서드 선언 위치 확인 rg -n "findRandomRecommendUserList" . rg -n "FollowQueryRepository" .Length of output: 770
findRandomRecommendUserList 메소드 구현 확인 완료
FollowQueryRepository(src/main/java/com/juu/juulabel/follow/repository/query/FollowQueryRepository.java:292)에
public Slice<FollowUser> findRandomRecommendUserList(…)가 정상적으로 정의되어 있습니다.
I pity the fool who doesn't!src/main/java/com/juu/juulabel/dailylife/repository/query/DailyLifeQueryRepository.java (2)
16-16: 새로운 import들이 제대로 추가됐군!
Expressions와Objectsimport가 추가되어 비인증 사용자 지원과 null 안전성을 위한 기반이 마련되었습니다. I pity the fool who doesn't import what they need!Also applies to: 27-27
248-250: 새로운 헬퍼 메소드 추가가 적절하다!
isNotPrivate메소드가 추가되어 비인증 사용자를 위한 프라이버시 체크를 간단하게 처리할 수 있게 되었습니다. I pity the fool who doesn't keep their helper methods clean!src/main/java/com/juu/juulabel/common/config/SecurityConfig.java (2)
31-35: 멋진 보안 강화다, fool!공개 엔드포인트를 인증 관련 경로로만 제한한 건 훌륭한 결정이야. 더 이상 불필요한 경로들이 공개되지 않도록 했구만!
91-113: 우선순위 기반 인가 규칙, 완벽해!Spring Security의 규칙 처리 순서를 명확히 이해하고 있군! 주석으로 각 단계를 표시한 것도 훌륭해. 다른 개발자들이 이해하기 쉬울 거야!
src/main/java/com/juu/juulabel/follow/repository/query/FollowQueryRepository.java (3)
142-153: 서브쿼리 추출, 훌륭한 리팩토링이야!중복된 서브쿼리 로직을 별도 메소드로 추출한 건 DRY 원칙을 잘 따른 거야. 특히 로그인하지 않은 사용자에 대한 null 체크로 불필요한 쿼리를 방지한 점이 마음에 들어!
154-168: isFollowing 메소드의 null 체크, 똑똑해!로그인하지 않은 사용자에 대해 즉시 false를 반환하는 건 성능상 좋은 선택이야. 데이터베이스 쿼리를 아예 실행하지 않으니까!
321-327: Objects.isEmpty에서 Objects.isNull로 수정, 잘했어!Long 타입에 대해 isEmpty를 사용하는 건 컴파일 에러야! isNull로 수정한 건 올바른 판단이야, fool!
src/main/java/com/juu/juulabel/alcohol/repository/query/TastingNoteCommentQueryRepository.java (2)
119-125: 비인증 사용자 처리, 완벽해!로그인하지 않은 사용자에 대해 FALSE를 반환하는 건 훌륭한 방어적 프로그래밍이야! 불필요한 서브쿼리 실행을 막고 성능도 향상시켰어!
138-140: null 체크 수정, 잘했어!Long 타입에 대한 올바른 null 체크로 수정한 건 좋은 판단이야, fool!
src/main/java/com/juu/juulabel/dailylife/repository/query/DailyLifeCommentQueryRepository.java (4)
12-12: 올바른 import 추가, fool!
Expressions와Objects클래스 추가가 적절하다. 특히 비인증 사용자 처리를 위한Expressions.FALSE사용과 올바른 null 체크를 위한java.util.Objects사용이 정확하다. I pity the fool who uses wrong imports!Also applies to: 22-22
33-67: 쿼리 포맷팅 개선, 멋지다 fool!
getAllByDailyLifeId메서드의 쿼리 구조가 훨씬 읽기 쉽게 개선되었다. 인덴테이션과 줄 바꿈이 일관성 있게 적용되어 코드 가독성이 크게 향상되었다. I pity the fool who writes messy queries!
77-105: 또 다른 훌륭한 포맷팅, fool!
getAllRepliesByParentId메서드도 동일하게 깔끔하게 포맷팅되었다. 일관된 스타일로 유지보수성이 크게 개선되었다.
115-128: 비인증 사용자 처리 로직 추가, 이거야 fool!
isLikedSubQuery메서드에 member가 null인 경우Expressions.FALSE를 반환하는 로직이 추가되었다. 이는 비인증 사용자에 대해 불필요한 서브쿼리 실행을 방지하고 성능을 개선하는 중요한 변경이다. I pity the fool who runs unnecessary queries for unauthenticated users!
| private BooleanExpression getPrivacyCondition(QDailyLife dailyLife, Member member) { | ||
| return Objects.isNull(member) ? isNotPrivate(dailyLife) : isNotPrivateOrAuthor(dailyLife, member); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
프라이버시 조건 로직의 중앙화, 훌륭한 리팩터링이다!
새로운 getPrivacyCondition 메소드는 비인증 사용자(member가 null)와 인증 사용자를 구분하여 적절한 프라이버시 조건을 반환합니다. 이는 코드의 일관성과 유지보수성을 크게 향상시킵니다. I pity the fool who doesn't centralize their logic!
🤖 Prompt for AI Agents
In
src/main/java/com/juu/juulabel/dailylife/repository/query/DailyLifeQueryRepository.java
around lines 236 to 238, the getPrivacyCondition method correctly centralizes
privacy logic by returning different conditions based on whether the member is
null or not. No changes are needed as this refactoring improves code consistency
and maintainability.
| return Objects.isNull(lastDailyLifeId) ? null : dailyLife.id.lt(lastDailyLifeId); | ||
| } |
There was a problem hiding this comment.
중요한 버그 수정이 이루어졌다!
기존의 잘못된 Objects.isEmpty 호출을 올바른 Objects.isNull로 수정했습니다. isEmpty는 Objects 클래스에 존재하지 않는 메소드였습니다. I pity the fool who calls methods that don't exist!
🤖 Prompt for AI Agents
In
src/main/java/com/juu/juulabel/dailylife/repository/query/DailyLifeQueryRepository.java
at lines 233-234, replace the incorrect call to Objects.isEmpty with the correct
Objects.isNull method to properly check if lastDailyLifeId is null, ensuring the
code compiles and functions as intended.
| private BooleanExpression getPrivacyCondition(QTastingNote tastingNote, Member member) { | ||
| return Objects.isNull(member) ? isNotPrivate(tastingNote) : isNotPrivateOrAuthor(tastingNote, member); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
DailyLife 리포지토리와 일관된 프라이버시 로직 리팩터링!
getPrivacyCondition 메소드가 다른 리포지토리와 동일한 패턴으로 구현되어 코드베이스 전체의 일관성을 유지하고 있습니다. 비인증 사용자와 인증 사용자를 구분하여 적절한 프라이버시 조건을 적용하는 접근법이 훌륭합니다. I pity the fool who doesn't keep their repositories consistent!
🤖 Prompt for AI Agents
In
src/main/java/com/juu/juulabel/alcohol/repository/query/TastingNoteQueryRepository.java
around lines 308 to 310, ensure the getPrivacyCondition method maintains the
consistent privacy logic pattern used in the DailyLife repository by returning
isNotPrivate when the member is null and isNotPrivateOrAuthor when the member is
present, properly distinguishing between unauthenticated and authenticated
users.
| System.out.println("preferredAlcoholTypes = " + preferredAlcoholTypes.stream() | ||
| .map(AlcoholType::getId) | ||
| .toList()); |
There was a problem hiding this comment.
프로덕션 코드에 System.out.println은 안 돼, fool!
디버깅용 출력문이 남아있네! 이건 로그 프레임워크를 사용해야 해!
- System.out.println("preferredAlcoholTypes = " + preferredAlcoholTypes.stream()
- .map(AlcoholType::getId)
- .toList());
+ // 필요시 로깅 프레임워크 사용
+ // log.debug("preferredAlcoholTypes = {}", preferredAlcoholTypes.stream()
+ // .map(AlcoholType::getId)
+ // .toList());📝 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.
| System.out.println("preferredAlcoholTypes = " + preferredAlcoholTypes.stream() | |
| .map(AlcoholType::getId) | |
| .toList()); | |
| // 필요시 로깅 프레임워크 사용 | |
| // log.debug("preferredAlcoholTypes = {}", preferredAlcoholTypes.stream() | |
| // .map(AlcoholType::getId) | |
| // .toList()); |
🤖 Prompt for AI Agents
In
src/main/java/com/juu/juulabel/follow/repository/query/FollowQueryRepository.java
around lines 206 to 208, replace the System.out.println debugging statement with
a proper logging call using the project's logging framework. Use a logger
instance to log the preferredAlcoholTypes list at an appropriate log level
instead of printing directly to the console.
|
|
||
| private BooleanExpression noOffsetByCommentId(QDailyLifeComment dailyLifeComment, Long lastCommentId) { | ||
| return Objects.isEmpty(lastCommentId) ? null : dailyLifeComment.id.lt(lastCommentId); | ||
| return Objects.isNull(lastCommentId) ? null : dailyLifeComment.id.lt(lastCommentId); |
There was a problem hiding this comment.
중요한 버그 수정, fool!
Objects.isEmpty에서 Objects.isNull로 변경한 것이 정확하다. Objects.isEmpty는 존재하지 않는 메서드였고, 이로 인해 컴파일 에러가 발생했을 것이다. I pity the fool who uses non-existent methods!
이 변경으로 올바른 null 체크가 가능해졌다:
-return Objects.isEmpty(lastCommentId) ? null : dailyLifeComment.id.lt(lastCommentId);
+return Objects.isNull(lastCommentId) ? null : dailyLifeComment.id.lt(lastCommentId);📝 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.
| return Objects.isNull(lastCommentId) ? null : dailyLifeComment.id.lt(lastCommentId); | |
| return Objects.isNull(lastCommentId) ? null : dailyLifeComment.id.lt(lastCommentId); |
🤖 Prompt for AI Agents
In
src/main/java/com/juu/juulabel/dailylife/repository/query/DailyLifeCommentQueryRepository.java
at line 131, replace the incorrect method call Objects.isEmpty with
Objects.isNull to correctly check for null values. This fixes the compilation
error caused by using a non-existent method and ensures proper null checking
before returning the condition.
…ode and improve readability.
Summary by CodeRabbit
문서화
버그 수정
리팩터
신규 기능