[Refactor] 인증/권한 기능 리펙토링 - #86
Merged
Merged
Conversation
AuthCommandService.login()에서 User.recordLogin()을 호출하지 않아 MeResponse.lastLoginAt이 항상 null로 응답되던 문제를 수정한다. UserRoleRepository에 role 코드만 조회하는 쿼리를 추가해 login()/getMe()에 중복돼 있던 role 조회 로직을 정리한다. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
기존에는 /auth/** 전체가 permitAll()이라 /auth/me의 인증 강제가 CurrentUserArgumentResolver의 수동 체크에만 의존하고 있었다. /auth/signup, /auth/login만 permitAll로 좁혀서 /auth/me는 anyRequest().authenticated()로 걸리도록 명시화한다. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AuthCommandService(signup/login), AuthQueryService(getMe)에 대한 단위 테스트가 전무했던 것을 추가한다. AuthFixture로 User/Department/ Role/UserRole 생성 헬퍼를 제공한다. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
getCollection()에 권한 체크가 전혀 없어 PRIVATE 컬렉션도 아무나 조회 가능하던 문제를 PermissionQueryService.canReadCollection() (OWNER→PUBLIC→USER→ROLE→DEPARTMENT) 추가로 해결한다. canReadCollection/canWriteCollection/canAdminCollection을 ID 버전과 엔티티 버전으로 분리해, 이미 컬렉션을 조회한 호출부(getCollection, addDocument, grantPermission)가 같은 row를 두 번 SELECT하던 중복을 없앤다. 동시에 ID 버전 내부에 status != DELETED 필터를 추가해 soft-delete된 컬렉션에 접근/권한부여가 가능했던 문제도 함께 막는다. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
canReadCollection 케이스와 삭제된 컬렉션 케이스를 추가하고, canWriteCollection/canAdminCollection 호출이 엔티티 버전으로 바뀐 서비스들의 stub을 그에 맞게 수정한다. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
이슈 번호를 받아 docs/design/의 해당 설계 문서를 실제 코드와 대조해 갱신하는 /update-design-doc 커맨드를 추가한다. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthrough인증 역할 조회와 로그인 기록을 정리했습니다. 컬렉션 읽기·쓰기·관리 권한과 soft-delete 검증을 강화했습니다. 관련 서비스, 저장소, 컨트롤러, 테스트, 설계 문서를 갱신했습니다. Changes인증 흐름
컬렉션 권한 흐름
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CollectionController
participant CollectionQueryService
participant PermissionQueryService
participant CollectionPermissionRepository
Client->>CollectionController: 컬렉션 조회 요청
CollectionController->>CollectionQueryService: userId와 collectionId 전달
CollectionQueryService->>PermissionQueryService: canReadCollection 호출
PermissionQueryService->>CollectionPermissionRepository: 직접·역할·부서 읽기 권한 조회
CollectionPermissionRepository-->>PermissionQueryService: 권한 결과 반환
PermissionQueryService-->>CollectionQueryService: 허용 또는 거부
CollectionQueryService-->>CollectionController: 컬렉션 응답 또는 예외
CollectionController-->>Client: 조회 결과 반환
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![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
🧹 Nitpick comments (4)
.claude/commands/update-design-doc.md (1)
12-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win새 파일과 새 메서드·엔드포인트의 문서 위치를 구분하세요.
"신규 파일" 또는 관련 섹션이라는 표현은 새 메서드나 엔드포인트를"신규 파일"섹션에 기록하게 만들 수 있습니다. 새 파일은"신규 파일"에 기록하고, 기존 파일의 새 메서드·엔드포인트는 해당 API 또는 흐름 섹션에 기록한다고 명시하세요.수정 예시
- - 새로 추가된 메서드/엔드포인트가 있는데 문서에 없으면 "신규 파일" 또는 관련 섹션에 추가 + - 새 파일은 "신규 파일" 섹션에 추가하고, 새 메서드/엔드포인트는 해당 API 또는 흐름 섹션에 추가🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/commands/update-design-doc.md around lines 12 - 13, Update the documentation guidance in the relevant command instructions to distinguish placement by change type: record newly added files under the “신규 파일” section, while documenting new methods or endpoints added to existing files in their corresponding API or flow section. Preserve the separate “설계 결정 요약” guidance for newly introduced design decisions.src/main/java/com/opensource/docgrid/domain/permission/service/query/PermissionQueryService.java (1)
249-287: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win순차 실행 흐름에 단계 번호 주석을 추가하세요.
src/main/java/com/opensource/docgrid/domain/permission/service/query/PermissionQueryService.java#L249-L287: OWNER, PUBLIC, USER, ROLE, DEPARTMENT 권한 판정 순서를1.,2.형식으로 표시하세요.src/main/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandService.java#L77-L80: 활성 컬렉션 조회와 쓰기 권한 확인 순서를 번호로 표시하세요.src/main/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandService.java#L108-L124: 소유자 확인, 캐시 무효화, 권한 삭제, soft delete 순서를 번호로 표시하세요.src/main/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandService.java#L127-L129: 활성 컬렉션 검증 이후의 제거 흐름을 번호로 표시하세요.src/main/java/com/opensource/docgrid/domain/permission/service/command/CollectionPermissionCommandService.java#L53-L57: 활성 컬렉션 검증과 관리자 권한 확인 순서를 번호로 표시하세요.As per coding guidelines, “For sequential execution flows, add numbered comments such as
1.,2.,3., and4.at the relevant steps.”Source: Coding guidelines
docs/design/kangcheolung-#18-permission-grant-revoke.md (1)
333-340: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
revokePermission()의 중복 조회 계약을 명확히 하세요.
CollectionPermissionCommandService.revokePermission()은 이미permission.getCollection()으로 컬렉션을 얻습니다. 그러나canAdminCollection(revokerId, collectionId)를 호출해 ID 오버로드의getActiveCollection(collectionId)를 다시 실행합니다. 이 경로는 추가 repository 조회를 수행하며, 연관관계가 초기화되지 않았다면 추가 SELECT가 발생할 수 있습니다.Line 340의 “별도 수정 불필요”는 soft-delete 검사는 설명하지만, 중복 조회 감소 범위는 설명하지 않습니다. 이미 조회한 엔티티의 상태를 확인한 뒤 엔티티 오버로드를 사용하거나, fresh status 조회가 의도된 보호 장치라면 그 이유와 SQL 검증 테스트를 문서화하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/kangcheolung-`#18-permission-grant-revoke.md around lines 333 - 340, Update the documented revokePermission() flow to explicitly address duplicate collection loading: use permission.getCollection() with the entity overload of canAdminCollection() after validating the entity status, or document the intentional fresh status lookup and add SQL-verification coverage. Replace the claim that no change is needed with the chosen behavior and preserve the soft-delete protection.src/main/java/com/opensource/docgrid/domain/collection/service/query/CollectionQueryService.java (1)
28-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win변경한 순차 흐름에 단계 주석을 추가하세요.
Line 28-35는 컬렉션 조회, soft-delete 필터, 읽기 권한 검사, DTO 변환의 순차 흐름을 추가합니다. 중요한 단계에
1.,2.,3.주석을 추가하세요. 주석은 soft-delete 및 권한 경계를 설명해야 하며 단순한 문법 반복은 피하세요.주석 예시
- // 컬렉션 단건 조회 — 소유자, PUBLIC, 또는 권한을 부여받은 사용자만 가능 + // 컬렉션 단건 조회 public CollectionResponse getCollection(Long userId, Long collectionId) { + // 1. 삭제된 컬렉션을 제외한다. DocumentCollection collection = collectionRepository.findById(collectionId) .filter(c -> c.getStatus() != CollectionStatus.DELETED) .orElseThrow(() -> new DocGridException(ErrorCode.COLLECTION_NOT_FOUND)); + // 2. 현재 사용자의 읽기 권한을 확인한다. if (!permissionQueryService.canReadCollection(userId, collection)) { throw new DocGridException(ErrorCode.PERMISSION_DENIED); } + // 3. 권한이 확인된 컬렉션을 응답 DTO로 변환한다. return collectionConverter.toResponse(collection); }As per coding guidelines: “For sequential execution flows, add numbered comments such as
1.,2.,3., and4.at the relevant steps.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/opensource/docgrid/domain/collection/service/query/CollectionQueryService.java` around lines 28 - 35, Update getCollection with numbered comments for the sequential flow: mark collection lookup with the soft-delete exclusion boundary, mark read-permission validation for the owner/PUBLIC/authorized-user boundary, and mark the final DTO conversion step. Keep comments descriptive rather than repeating the statements.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/commands/update-design-doc.md:
- Around line 3-7: Update the argument guidance in the update-design-doc command
to require issue numbers without a leading “#”, and change the design-document
lookup pattern to add the single expected “#” before the numeric argument so
filenames such as kangcheolung-#21-permission-query-service.md match correctly.
In `@docs/design/kangcheolung-`#21-permission-query-service.md:
- Line 37: Update the PermissionQueryService documentation at the referenced
overview and repeated summary to distinguish logical permission groups from Java
method signatures: describe the three document permission methods plus the three
collection permission groups, each exposing ID and DocumentCollection overloads,
for a total of nine public signatures rather than six.
In `@docs/design/kangcheolung-`#29-collection-management.md:
- Line 214: Update the documentation statement describing
CollectionQueryServiceTest so it matches the actual tests: either remove the
claim that getMyCollections cases exist, or add the corresponding
getMyCollections test cases before retaining that claim. Keep the documented
getCollection test count accurate.
In
`@src/main/java/com/opensource/docgrid/domain/collection/controller/CollectionController.java`:
- Around line 98-100: Update the CollectionController operation description for
adding documents to explicitly include ADMIN alongside WRITE and the owner as
permitted users, matching the canWrite policy used by canWriteCollection().
In
`@src/main/java/com/opensource/docgrid/domain/permission/service/query/PermissionQueryService.java`:
- Around line 254-287: Update the DocumentCollection overloads
canReadCollection(Long, DocumentCollection), canWriteCollection(Long,
DocumentCollection), and canAdminCollection(Long, DocumentCollection) to reject
collections whose status is CollectionStatus.DELETED by throwing the existing
COLLECTION_NOT_FOUND exception before any permission checks. Revise the
canReadCollection comment so it no longer relies on callers to guarantee
non-deleted entities.
---
Nitpick comments:
In @.claude/commands/update-design-doc.md:
- Around line 12-13: Update the documentation guidance in the relevant command
instructions to distinguish placement by change type: record newly added files
under the “신규 파일” section, while documenting new methods or endpoints added to
existing files in their corresponding API or flow section. Preserve the separate
“설계 결정 요약” guidance for newly introduced design decisions.
In `@docs/design/kangcheolung-`#18-permission-grant-revoke.md:
- Around line 333-340: Update the documented revokePermission() flow to
explicitly address duplicate collection loading: use permission.getCollection()
with the entity overload of canAdminCollection() after validating the entity
status, or document the intentional fresh status lookup and add SQL-verification
coverage. Replace the claim that no change is needed with the chosen behavior
and preserve the soft-delete protection.
In
`@src/main/java/com/opensource/docgrid/domain/collection/service/query/CollectionQueryService.java`:
- Around line 28-35: Update getCollection with numbered comments for the
sequential flow: mark collection lookup with the soft-delete exclusion boundary,
mark read-permission validation for the owner/PUBLIC/authorized-user boundary,
and mark the final DTO conversion step. Keep comments descriptive rather than
repeating the statements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 793b3b8f-aef6-4103-8835-ab661852c569
📒 Files selected for processing (22)
.claude/commands/update-design-doc.mddocs/design/kangcheolung-#16-collection-crud.mddocs/design/kangcheolung-#18-permission-grant-revoke.mddocs/design/kangcheolung-#21-permission-query-service.mddocs/design/kangcheolung-#29-collection-management.mdsrc/main/java/com/opensource/docgrid/domain/auth/service/command/AuthCommandService.javasrc/main/java/com/opensource/docgrid/domain/auth/service/query/AuthQueryService.javasrc/main/java/com/opensource/docgrid/domain/collection/controller/CollectionController.javasrc/main/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandService.javasrc/main/java/com/opensource/docgrid/domain/collection/service/query/CollectionQueryService.javasrc/main/java/com/opensource/docgrid/domain/permission/repository/CollectionPermissionRepository.javasrc/main/java/com/opensource/docgrid/domain/permission/service/command/CollectionPermissionCommandService.javasrc/main/java/com/opensource/docgrid/domain/permission/service/query/PermissionQueryService.javasrc/main/java/com/opensource/docgrid/domain/user/repository/UserRoleRepository.javasrc/main/java/com/opensource/docgrid/global/config/SecurityConfig.javasrc/test/java/com/opensource/docgrid/domain/auth/fixture/AuthFixture.javasrc/test/java/com/opensource/docgrid/domain/auth/service/command/AuthCommandServiceTest.javasrc/test/java/com/opensource/docgrid/domain/auth/service/query/AuthQueryServiceTest.javasrc/test/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandServiceTest.javasrc/test/java/com/opensource/docgrid/domain/collection/service/query/CollectionQueryServiceTest.javasrc/test/java/com/opensource/docgrid/domain/permission/service/command/CollectionPermissionCommandServiceTest.javasrc/test/java/com/opensource/docgrid/domain/permission/service/query/PermissionQueryServiceTest.java
canReadCollection/canWriteCollection/canAdminCollection의 엔티티 오버로드가 호출부의 soft-delete 필터링에만 의존하고 있어, 호출부가 필터링을 빠뜨린 엔티티를 넘기면 삭제된 컬렉션도 권한이 통과될 수 있었다. validateActiveCollection()을 엔티티 오버로드 진입 지점에 추가해 추가 쿼리 없이 자체 방어하도록 한다(CodeRabbit 리뷰 반영). addDocument()의 Swagger description도 canWriteCollection()이 WRITE뿐 아니라 ADMIN 권한자도 통과시킨다는 실제 동작에 맞게 정정한다. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
소유자여도 삭제된 컬렉션 엔티티를 canReadCollection에 직접 넘기면 COLLECTION_NOT_FOUND가 발생하는지 검증한다. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍️ 작업 내용
✨ 상세 설명
인증(Auth)
AuthCommandService.login()에서User.recordLogin()미호출로MeResponse.lastLoginAt이 항상null이던 버그 수정login()/getMe()에 중복돼 있던 role 코드 조회 로직을UserRoleRepository.findRoleCodesByUserId()로 정리/auth/**블랭킷permitAll()을/auth/signup,/auth/login으로 좁혀/auth/me인증 강제를SecurityConfig에서 명시화AuthCommandService/AuthQueryService단위 테스트 신규 추가 (기존엔 전무했음)권한/컬렉션(Permission/Collection)
CollectionQueryService.getCollection()에 권한 체크가 전혀 없어 PRIVATE 컬렉션도 아무나 조회 가능하던 문제를PermissionQueryService.canReadCollection()(OWNER→PUBLIC→USER→ROLE→DEPARTMENT) 추가로 해결canReadCollection/canWriteCollection/canAdminCollection을 ID 버전(조회+필터 후 위임) / 엔티티 버전(조회 없이 판단)으로 분리해, 이미 컬렉션을 조회한 호출부(getCollection,addDocument,grantPermission)의 중복 SELECT 제거status != DELETED필터를 추가해 soft-delete된 컬렉션에 접근/권한부여가 가능했던 문제 차단CollectionController.addDocument()의 Swagger description을 실제 인가 규칙(WRITE 권한 보유자, 소유자 포함)에 맞게 수정docs/design/#16,#18,#21,#29설계 문서를 위 변경사항에 맞게 동기화🛠️ 추후 리팩토링 및 고도화 계획
DocumentPermissionCommandService) 쪽에도 동일한 중복 쿼리/soft-delete 필터 누락 문제가 있어 별도 후속 작업 필요createCollection()의parentCollectionId조회 시 부모 컬렉션에 대한 권한 체크가 없음 (별도 이슈로 분리 필요)AccessSourceType.OWNER가 정의만 되고 실제 생성되지 않는 dead value로 남아있음expiresAt지난 것) 정리 배치 없음💬 리뷰 요구사항
canReadCollection()에서 컬렉션visibility=PUBLIC이면 소유자/권한 여부와 무관하게 읽기를 허용하도록 했습니다(canReadDocument()와 동일한 정책). 이 설계 방향이 맞는지 확인 부탁드립니다.Summary by CodeRabbit
새 기능
버그 수정
테스트