Merge pull request #86 from DocGrid/refactor/85

[Refactor] 인증/권한 기능 리펙토링

Author: kangcheolung

.claude/commands/update-design-doc.md

@@ -0,0 +1,21 @@
+# 설계 문서 동기화 커맨드
+
+이슈 번호($ARGUMENTS)를 받아, 코드 리팩토링/수정 이후 낡아진 `docs/design/` 설계 문서를 실제 코드에 맞게 갱신합니다. `$ARGUMENTS`는 `#` 없이 숫자만 전달한다(예: `21`) — 아래 패턴 자체에 `#`이 포함돼 있어 `#21`처럼 넘기면 `##21`이 되어 매칭에 실패한다.
+
+## 절차
+
+1. `docs/design/*-#{이슈번호}-*.md` 패턴으로 해당 이슈의 설계 문서를 찾는다. 없으면 사용자에게 알리고 중단한다(새 설계 문서 작성은 이 커맨드의 범위가 아님 — `docs-management.md` 규칙에 따라 기능 구현 PR과 함께 사람이 작성).
+2. 문서에 등장하는 코드 스니펫·파일 경로·메서드 시그니처를 실제 소스 코드와 하나씩 대조한다. 문서가 인용하는 파일들을 전부 읽는다.
+3. 다음을 갱신 대상으로 삼는다:
+   - 문서의 코드 스니펫이 실제 코드와 달라진 부분 (시그니처 변경, 로직 변경, 삭제된 메서드 등)
+   - "남은 이슈 / TODO" 섹션 중 이번 변경으로 실제로 해결된 항목 — 삭제하지 말고 `~~취소선~~` 처리 후 "→ 해결됨(어떻게 해결됐는지 한 줄)"로 남긴다 (기존 `#18` 문서의 Swagger 설명 수정 이력이 이 패턴을 이미 쓰고 있으니 그대로 따른다)
+   - 새로 추가된 메서드/엔드포인트가 있는데 문서에 없으면 "신규 파일" 또는 관련 섹션에 추가
+   - "설계 결정 요약"에 이번 변경으로 새로 생긴 결정(예: 오버로드 분리, status 필터링 방식)이 있으면 한 항목 추가
+4. **바꾸지 않는 것**: 문서 제목, `closes #{이슈번호}`, 배경(왜 만들었는지) 섹션의 원래 취지, "로컬 검증"에 기록된 과거 수동 테스트 기록(사실 기록이므로 보존). 테스트 결과 문서(`docs/test-results/`)는 별도 이슈로 관리되므로 건드리지 않는다.
+5. 수정은 Edit 도구로 최소 diff만 반영한다 — 문서 전체를 새로 쓰지 않는다.
+6. 완료 후 어떤 섹션을 왜 고쳤는지 3~5줄로 요약해서 보고한다. 이 커맨드는 git add/commit을 하지 않는다 — 커밋 여부는 `git-conventions.md`대로 사용자 확인 후 별도로 진행한다.
+
+## 주의
+
+- 이슈 하나에 문서가 여러 개 걸릴 수 있다(예: `#16`/`#29`처럼 같은 서비스를 다루는 후속 이슈). `$ARGUMENTS`로 지정한 이슈 번호의 문서만 갱신하고, 관련된 다른 이슈 문서가 있으면 "관련 문서 `#N`도 같은 이유로 낡았을 수 있음"이라고 언급만 하고 자동으로 같이 고치지 않는다 — 범위를 명시적으로 지정받는다.
+- 코드에서 확인이 안 되는 내용(의도인지 버그인지 불확실한 부분)은 문서에 임의로 단정해서 쓰지 않는다. 확인이 필요하면 사용자에게 먼저 묻는다.

docs/design/kangcheolung-#16-collection-crud.md

@@ -159,9 +159,10 @@ public CollectionResponse createCollection(Long userId, CreateCollectionRequest
 
 public CollectionDocumentResponse addDocument(Long collectionId, Long userId, AddDocumentRequest request) {
     DocumentCollection collection = collectionRepository.findById(collectionId)
+            .filter(c -> c.getStatus() != CollectionStatus.DELETED)
             .orElseThrow(() -> new DocGridException(ErrorCode.COLLECTION_NOT_FOUND));
 
-    if (!permissionQueryService.canWriteCollection(userId, collectionId)) {
+    if (!permissionQueryService.canWriteCollection(userId, collection)) {
         throw new DocGridException(ErrorCode.PERMISSION_DENIED);
     }
 
@@ -185,19 +186,23 @@ public CollectionDocumentResponse addDocument(Long collectionId, Long userId, Ad
 - `userRepository.getReferenceById(userId)`: `JpaRepository`가 기본 제공하는 프록시 조회 메서드. `SearchResultCommandService`(RAG 블록) 등이 쓰는 `entityManager.getReference()`와 동일한 목적(불필요한 SELECT 생략)을, Spring Data가 표준으로 제공하는 방식으로 구현한 것이다.
 - `addDocument()`는 `#21`에서 만든 `PermissionQueryService.canWriteCollection()`을 그대로 재사용한다 — 이 이슈에서 권한 판단 로직을 새로 만들지 않는다.
 - 존재 확인(컬렉션) → 권한 확인 → 존재 확인(문서) → 중복 확인 순서다. 컬렉션이 없는데 권한부터 확인하면 404 대신 엉뚱한 에러가 날 수 있어 존재 확인이 항상 먼저 온다.
-- **확인된 갭**: `collectionRepository.findById(collectionId)`는 `status`를 전혀 필터링하지 않는다(`CollectionRepository`에는 `findAllByOwnerIdAndStatus`만 있고, ID 단건 조회에 상태 조건을 건 메서드가 없다). 즉 `status=DELETED`(소프트 삭제된) 컬렉션에도 `addDocument()`로 문서를 계속 추가할 수 있다 — 아래 "남은 이슈/TODO"에 기록.
+- ~~**확인된 갭**: `collectionRepository.findById(collectionId)`는 `status`를 전혀 필터링하지 않는다(`CollectionRepository`에는 `findAllByOwnerIdAndStatus`만 있고, ID 단건 조회에 상태 조건을 건 메서드가 없다). 즉 `status=DELETED`(소프트 삭제된) 컬렉션에도 `addDocument()`로 문서를 계속 추가할 수 있다 — 아래 "남은 이슈/TODO"에 기록.~~ → 해결됨: `findById(...).filter(c -> c.getStatus() != CollectionStatus.DELETED)`로 soft-delete된 컬렉션을 걸러내도록 수정.
 
 ### 5. `domain/collection/service/query/CollectionQueryService.java` — `getCollection`
 
 ```java
-public CollectionResponse getCollection(Long collectionId) {
+public CollectionResponse getCollection(Long userId, Long collectionId) {
     DocumentCollection collection = collectionRepository.findById(collectionId)
+            .filter(c -> c.getStatus() != CollectionStatus.DELETED)
             .orElseThrow(() -> new DocGridException(ErrorCode.COLLECTION_NOT_FOUND));
+    if (!permissionQueryService.canReadCollection(userId, collection)) {
+        throw new DocGridException(ErrorCode.PERMISSION_DENIED);
+    }
     return collectionConverter.toResponse(collection);
 }
 ```
 
-**주의**: 컬렉션 단건 조회는 권한 체크를 하지 않는다. `visibility`/`status`와 무관하게 ID만 알면 누구나(인증된 사용자면) 조회 가능하다. 이건 명시적으로 의도된 설계인지, 이번 이슈 범위에서 권한 체크가 빠진 것인지 코드만으로는 확정하기 어렵다 — 컬렉션 메타데이터(이름/설명 정도)만 노출되고 소속 문서 내용은 노출되지 않아서 위험도가 낮다고 판단했을 가능성이 있다.
+~~**주의**: 컬렉션 단건 조회는 권한 체크를 하지 않는다. `visibility`/`status`와 무관하게 ID만 알면 누구나(인증된 사용자면) 조회 가능하다. 이건 명시적으로 의도된 설계인지, 이번 이슈 범위에서 권한 체크가 빠진 것인지 코드만으로는 확정하기 어렵다 — 컬렉션 메타데이터(이름/설명 정도)만 노출되고 소속 문서 내용은 노출되지 않아서 위험도가 낮다고 판단했을 가능성이 있다.~~ → 해결됨: `PermissionQueryService.canReadCollection()`(소유자/PUBLIC/USER·ROLE·DEPARTMENT 권한)을 추가해서 `getCollection()`에 명시적으로 연결했다. `#21` 문서에 `canReadCollection` 메서드 설명 추가 필요.
 
 ### 6. `domain/collection/repository/CollectionRepository.java`, `CollectionDocumentRepository.java`
 
@@ -277,7 +282,7 @@ POST /collections/1/documents
 $ ./gradlew test --tests "*CollectionCommandServiceTest*" --tests "*CollectionQueryServiceTest*"
 BUILD SUCCESSFUL
 ```
-`CollectionCommandServiceTest` 15개, `CollectionQueryServiceTest` 2개, 총 17개 모두 통과(현재 기준 재검증).
+`CollectionCommandServiceTest` 15개, `CollectionQueryServiceTest` 4개(`canReadCollection` 도입으로 권한없음/삭제된 컬렉션 케이스 추가), 총 19개 모두 통과(현재 기준 재검증).
 
 ---
 
@@ -301,14 +306,16 @@ BUILD SUCCESSFUL
 
 **컬렉션 트리(`parentCollection`)는 자기참조 FK만 준비하고 순회 API는 만들지 않음**: 나중에 "하위 컬렉션 전체 조회" 같은 기능이 필요해질 걸 대비해 스키마는 미리 잡아뒀지만, 지금 당장 필요하지 않은 API까지 만들지 않았다(Simplicity First).
 
+**(추가) `canReadCollection` 도입 + soft-delete 필터를 "엔티티 오버로드"로 통일**: `getCollection()`/`addDocument()`가 컬렉션을 조회한 뒤 `PermissionQueryService`를 ID로 다시 호출하면 같은 row를 두 번 SELECT하게 된다. `PermissionQueryService.canReadCollection`/`canWriteCollection`/`canAdminCollection`을 각각 "ID 버전(조회 후 위임) + 엔티티 버전(조회 없이 판단)"으로 나눠서, 이미 엔티티를 들고 있는 호출부는 엔티티 버전을 호출해 중복 조회를 없앴다. soft-delete 필터(`status != DELETED`)는 `AuthCommandService.signup()`의 부서 활성 필터와 동일하게 `findById(...).filter(...).orElseThrow(...)` 관용구로 통일했다.
+
 ---
 
 ## 남은 이슈 / TODO
 
-- `getCollection()`(단건 조회)에 권한 체크가 없다 — 컬렉션 메타데이터만 노출되어 위험도가 낮다고 판단했을 수 있으나, 명시적으로 재검토가 필요하다.
-- `getCollection()`과 `addDocument()` 둘 다 `collectionRepository.findById()`만 쓰고 `status`를 확인하지 않는다 — `#29`에서 소프트 삭제(`status=DELETED`)를 도입한 이후에도 이 두 경로는 여전히 삭제된 컬렉션에 접근/문서 추가가 가능하다(코드리뷰 지적사항, `CollectionRepository`에 ID+ACTIVE 조합 조회 메서드 추가가 필요).
+- ~~`getCollection()`(단건 조회)에 권한 체크가 없다~~ → 해결됨: `canReadCollection()` 추가 (아래 "설계 결정 요약" 참고).
+- ~~`getCollection()`과 `addDocument()` 둘 다 `collectionRepository.findById()`만 쓰고 `status`를 확인하지 않는다~~ → 해결됨: `.filter(c -> c.getStatus() != CollectionStatus.DELETED)`를 두 메서드 모두에 추가. `#29`의 `deleteCollection()`/`removeDocument()`에도 동일하게 적용됨(해당 문서 참고).
 - `CollectionStatus.ARCHIVED`는 정의만 되어 있고 전환 로직이 없다.
-- `CollectionPermission`/`DocumentPermission` 엔티티의 Javadoc에 이미 명시된 TODO: `target_type`별로 단일 FK만 채워져야 한다는 규칙이 DB CHECK 제약으로 강제되지 않고 애플리케이션 검증(`validateTargetType()`, `#18`)에만 의존한다.
+- ~~`CollectionPermission`/`DocumentPermission` 엔티티의 Javadoc에 이미 명시된 TODO: `target_type`별로 단일 FK만 채워져야 한다는 규칙이 DB CHECK 제약으로 강제되지 않고 애플리케이션 검증(`validateTargetType()`, `#18`)에만 의존한다.~~ → 확인 결과 이미 해결되어 있음: `V11__create_collection_permissions.sql`/`V12__create_document_permissions.sql`에 `CHECK` 제약이 반영되어 있다(엔티티 Javadoc만 갱신되지 않은 상태였음).
 
 ## 다음 단계
 

docs/design/kangcheolung-#18-permission-grant-revoke.md

@@ -137,9 +137,10 @@ public enum AccessSourceType {
 ```java
 public CollectionPermissionResponse grantPermission(Long collectionId, Long grantorId, GrantPermissionRequest request) {
     DocumentCollection collection = collectionRepository.findById(collectionId)
+            .filter(c -> c.getStatus() != CollectionStatus.DELETED)
             .orElseThrow(() -> new DocGridException(ErrorCode.COLLECTION_NOT_FOUND));
 
-    if (!permissionQueryService.canAdminCollection(grantorId, collectionId)) {
+    if (!permissionQueryService.canAdminCollection(grantorId, collection)) {
         throw new DocGridException(ErrorCode.PERMISSION_DENIED);
     }
 
@@ -329,11 +330,14 @@ BUILD SUCCESSFUL
 
 **권한 부여 자체에도 ADMIN 권한이 필요**: `#16`까지는 소유자만 문서를 추가할 수 있었는데, 이 이슈부터 "소유자가 다른 사람에게 ADMIN 권한을 위임하면, 그 사람도 권한을 나눠줄 수 있다"는 위임 구조가 생긴다. `canAdminCollection()`이 소유자와 ADMIN 위임자를 모두 포함해서 판단하므로 이 위임이 자연스럽게 성립한다.
 
+**(추가) `canAdminCollection`을 ID 버전 + 엔티티 버전으로 분리**: `grantPermission()`이 컬렉션을 조회한 뒤 `canAdminCollection(grantorId, collectionId)`을 ID로 다시 호출하면 같은 row를 두 번 SELECT하게 되고, soft-delete된 컬렉션에도 새 권한을 부여할 수 있는 구멍이 있었다. `canAdminCollection(userId, DocumentCollection)` 엔티티 오버로드를 추가해 이미 조회한 엔티티를 그대로 넘기도록 하고, ID 버전에는 `status != DELETED` 필터를 넣었다(`#16` 문서의 동일 리팩토링과 같은 패턴).
+
 ---
 
 ## 남은 이슈 / TODO
 
-- `target_type`별 단일 FK 제약이 DB 레벨(CHECK)이 아니라 애플리케이션 검증에만 있다(엔티티 Javadoc에 이미 기록됨).
+- ~~`target_type`별 단일 FK 제약이 DB 레벨(CHECK)이 아니라 애플리케이션 검증에만 있다(엔티티 Javadoc에 이미 기록됨).~~ → 확인 결과 이미 해결되어 있음: `V11__create_collection_permissions.sql`/`V12__create_document_permissions.sql`에 `CHECK` 제약이 반영되어 있다(엔티티 Javadoc만 갱신되지 않은 상태였음, `#16` 문서에서도 동일하게 확인).
+- `grantPermission()`/`canAdminCollection()`이 컬렉션을 두 번 조회하던 중복 쿼리 및 soft-delete된 컬렉션에도 권한을 부여할 수 있던 문제 → 해결됨(아래 "설계 결정 요약" 참고). `revokePermission()`은 자체 `findById` 호출이 없어 `canAdminCollection(revokerId, collectionId)`(ID 버전) 내부의 status 필터를 그대로 적용받는다 — 별도 수정 불필요.
 - `expiresAt`이 지난 권한을 정리(삭제 또는 자동 무효화)하는 배치가 없다 — live 조회 시 `expiresAt > CURRENT_TIMESTAMP` 조건으로 걸러지긴 하지만, 만료된 레코드 자체는 DB에 계속 쌓인다.
 - `AccessSourceType.OWNER`가 정의만 되어 있고 실제로 생성되지 않는다(위 "확인된 불일치" 참고) — enum에서 제거하거나, 실제로 OWNER 캐시를 생성하도록 코드를 맞추거나 둘 중 하나로 정리가 필요하다.
 - ~~`PermissionController`의 컬렉션/문서 권한 부여·회수 4개 엔드포인트 Swagger description이 "소유자(owner)만 가능"이라고 적혀 있던 문제~~ → 코드리뷰로 발견해 실제 인가 규칙(`canAdminCollection()`/`canAdminDocument()`, ADMIN 위임자도 허용)에 맞게 4곳 모두 "ADMIN 권한 보유자(소유자 포함)"로 수정 완료.