You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
docs: 권한/컬렉션 설계 문서를 canReadCollection 리팩토링에 맞게 동기화
#16, #18, #21, #29 문서의 코드 스니펫을 실제 코드(엔티티 오버로드,
status 필터)에 맞게 갱신하고, 해결된 TODO는 취소선 처리 후
해결 내역을 남긴다. target_type CHECK 제약 TODO는 이미 마이그레이션에
반영되어 있던 것으로 확인되어 함께 정리한다.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@@ -185,19 +186,23 @@ public CollectionDocumentResponse addDocument(Long collectionId, Long userId, Ad
185
186
-`userRepository.getReferenceById(userId)`: `JpaRepository`가 기본 제공하는 프록시 조회 메서드. `SearchResultCommandService`(RAG 블록) 등이 쓰는 `entityManager.getReference()`와 동일한 목적(불필요한 SELECT 생략)을, Spring Data가 표준으로 제공하는 방식으로 구현한 것이다.
186
187
-`addDocument()`는 `#21`에서 만든 `PermissionQueryService.canWriteCollection()`을 그대로 재사용한다 — 이 이슈에서 권한 판단 로직을 새로 만들지 않는다.
187
188
- 존재 확인(컬렉션) → 권한 확인 → 존재 확인(문서) → 중복 확인 순서다. 컬렉션이 없는데 권한부터 확인하면 404 대신 엉뚱한 에러가 날 수 있어 존재 확인이 항상 먼저 온다.
188
-
-**확인된 갭**: `collectionRepository.findById(collectionId)`는 `status`를 전혀 필터링하지 않는다(`CollectionRepository`에는 `findAllByOwnerIdAndStatus`만 있고, ID 단건 조회에 상태 조건을 건 메서드가 없다). 즉 `status=DELETED`(소프트 삭제된) 컬렉션에도 `addDocument()`로 문서를 계속 추가할 수 있다 — 아래 "남은 이슈/TODO"에 기록.
189
+
-~~**확인된 갭**: `collectionRepository.findById(collectionId)`는 `status`를 전혀 필터링하지 않는다(`CollectionRepository`에는 `findAllByOwnerIdAndStatus`만 있고, ID 단건 조회에 상태 조건을 건 메서드가 없다). 즉 `status=DELETED`(소프트 삭제된) 컬렉션에도 `addDocument()`로 문서를 계속 추가할 수 있다 — 아래 "남은 이슈/TODO"에 기록.~~ → 해결됨: `findById(...).filter(c -> c.getStatus() != CollectionStatus.DELETED)`로 soft-delete된 컬렉션을 걸러내도록 수정.
**주의**: 컬렉션 단건 조회는 권한 체크를 하지 않는다. `visibility`/`status`와 무관하게 ID만 알면 누구나(인증된 사용자면) 조회 가능하다. 이건 명시적으로 의도된 설계인지, 이번 이슈 범위에서 권한 체크가 빠진 것인지 코드만으로는 확정하기 어렵다 — 컬렉션 메타데이터(이름/설명 정도)만 노출되고 소속 문서 내용은 노출되지 않아서 위험도가 낮다고 판단했을 가능성이 있다.
205
+
~~**주의**: 컬렉션 단건 조회는 권한 체크를 하지 않는다. `visibility`/`status`와 무관하게 ID만 알면 누구나(인증된 사용자면) 조회 가능하다. 이건 명시적으로 의도된 설계인지, 이번 이슈 범위에서 권한 체크가 빠진 것인지 코드만으로는 확정하기 어렵다 — 컬렉션 메타데이터(이름/설명 정도)만 노출되고 소속 문서 내용은 노출되지 않아서 위험도가 낮다고 판단했을 가능성이 있다.~~ → 해결됨: `PermissionQueryService.canReadCollection()`(소유자/PUBLIC/USER·ROLE·DEPARTMENT 권한)을 추가해서 `getCollection()`에 명시적으로 연결했다. `#21` 문서에 `canReadCollection` 메서드 설명 추가 필요.
$ ./gradlew test --tests "*CollectionCommandServiceTest*" --tests "*CollectionQueryServiceTest*"
278
283
BUILD SUCCESSFUL
279
284
```
280
-
`CollectionCommandServiceTest` 15개, `CollectionQueryServiceTest`2개, 총 17개 모두 통과(현재 기준 재검증).
285
+
`CollectionCommandServiceTest` 15개, `CollectionQueryServiceTest`4개(`canReadCollection` 도입으로 권한없음/삭제된 컬렉션 케이스 추가), 총 19개 모두 통과(현재 기준 재검증).
281
286
282
287
---
283
288
@@ -301,14 +306,16 @@ BUILD SUCCESSFUL
301
306
302
307
**컬렉션 트리(`parentCollection`)는 자기참조 FK만 준비하고 순회 API는 만들지 않음**: 나중에 "하위 컬렉션 전체 조회" 같은 기능이 필요해질 걸 대비해 스키마는 미리 잡아뒀지만, 지금 당장 필요하지 않은 API까지 만들지 않았다(Simplicity First).
303
308
309
+
**(추가) `canReadCollection` 도입 + soft-delete 필터를 "엔티티 오버로드"로 통일**: `getCollection()`/`addDocument()`가 컬렉션을 조회한 뒤 `PermissionQueryService`를 ID로 다시 호출하면 같은 row를 두 번 SELECT하게 된다. `PermissionQueryService.canReadCollection`/`canWriteCollection`/`canAdminCollection`을 각각 "ID 버전(조회 후 위임) + 엔티티 버전(조회 없이 판단)"으로 나눠서, 이미 엔티티를 들고 있는 호출부는 엔티티 버전을 호출해 중복 조회를 없앴다. soft-delete 필터(`status != DELETED`)는 `AuthCommandService.signup()`의 부서 활성 필터와 동일하게 `findById(...).filter(...).orElseThrow(...)` 관용구로 통일했다.
310
+
304
311
---
305
312
306
313
## 남은 이슈 / TODO
307
314
308
-
-`getCollection()`(단건 조회)에 권한 체크가 없다 — 컬렉션 메타데이터만 노출되어 위험도가 낮다고 판단했을 수 있으나, 명시적으로 재검토가 필요하다.
309
-
-`getCollection()`과 `addDocument()` 둘 다 `collectionRepository.findById()`만 쓰고 `status`를 확인하지 않는다 — `#29`에서 소프트 삭제(`status=DELETED`)를 도입한 이후에도 이 두 경로는 여전히 삭제된 컬렉션에 접근/문서 추가가 가능하다(코드리뷰 지적사항, `CollectionRepository`에 ID+ACTIVE 조합 조회 메서드 추가가 필요).
315
+
-~~`getCollection()`(단건 조회)에 권한 체크가 없다~~ → 해결됨: `canReadCollection()` 추가 (아래 "설계 결정 요약" 참고).
316
+
-~~`getCollection()`과 `addDocument()` 둘 다 `collectionRepository.findById()`만 쓰고 `status`를 확인하지 않는다~~ → 해결됨: `.filter(c -> c.getStatus() != CollectionStatus.DELETED)`를 두 메서드 모두에 추가. `#29`의 `deleteCollection()`/`removeDocument()`에도 동일하게 적용됨(해당 문서 참고).
310
317
-`CollectionStatus.ARCHIVED`는 정의만 되어 있고 전환 로직이 없다.
311
-
-`CollectionPermission`/`DocumentPermission` 엔티티의 Javadoc에 이미 명시된 TODO: `target_type`별로 단일 FK만 채워져야 한다는 규칙이 DB CHECK 제약으로 강제되지 않고 애플리케이션 검증(`validateTargetType()`, `#18`)에만 의존한다.
318
+
-~~`CollectionPermission`/`DocumentPermission` 엔티티의 Javadoc에 이미 명시된 TODO: `target_type`별로 단일 FK만 채워져야 한다는 규칙이 DB CHECK 제약으로 강제되지 않고 애플리케이션 검증(`validateTargetType()`, `#18`)에만 의존한다.~~ → 확인 결과 이미 해결되어 있음: `V11__create_collection_permissions.sql`/`V12__create_document_permissions.sql`에 `CHECK` 제약이 반영되어 있다(엔티티 Javadoc만 갱신되지 않은 상태였음).
**권한 부여 자체에도 ADMIN 권한이 필요**: `#16`까지는 소유자만 문서를 추가할 수 있었는데, 이 이슈부터 "소유자가 다른 사람에게 ADMIN 권한을 위임하면, 그 사람도 권한을 나눠줄 수 있다"는 위임 구조가 생긴다. `canAdminCollection()`이 소유자와 ADMIN 위임자를 모두 포함해서 판단하므로 이 위임이 자연스럽게 성립한다.
331
332
333
+
**(추가) `canAdminCollection`을 ID 버전 + 엔티티 버전으로 분리**: `grantPermission()`이 컬렉션을 조회한 뒤 `canAdminCollection(grantorId, collectionId)`을 ID로 다시 호출하면 같은 row를 두 번 SELECT하게 되고, soft-delete된 컬렉션에도 새 권한을 부여할 수 있는 구멍이 있었다. `canAdminCollection(userId, DocumentCollection)` 엔티티 오버로드를 추가해 이미 조회한 엔티티를 그대로 넘기도록 하고, ID 버전에는 `status != DELETED` 필터를 넣었다(`#16` 문서의 동일 리팩토링과 같은 패턴).
334
+
332
335
---
333
336
334
337
## 남은 이슈 / TODO
335
338
336
-
-`target_type`별 단일 FK 제약이 DB 레벨(CHECK)이 아니라 애플리케이션 검증에만 있다(엔티티 Javadoc에 이미 기록됨).
339
+
-~~`target_type`별 단일 FK 제약이 DB 레벨(CHECK)이 아니라 애플리케이션 검증에만 있다(엔티티 Javadoc에 이미 기록됨).~~ → 확인 결과 이미 해결되어 있음: `V11__create_collection_permissions.sql`/`V12__create_document_permissions.sql`에 `CHECK` 제약이 반영되어 있다(엔티티 Javadoc만 갱신되지 않은 상태였음, `#16` 문서에서도 동일하게 확인).
340
+
-`grantPermission()`/`canAdminCollection()`이 컬렉션을 두 번 조회하던 중복 쿼리 및 soft-delete된 컬렉션에도 권한을 부여할 수 있던 문제 → 해결됨(아래 "설계 결정 요약" 참고). `revokePermission()`은 자체 `findById` 호출이 없어 `canAdminCollection(revokerId, collectionId)`(ID 버전) 내부의 status 필터를 그대로 적용받는다 — 별도 수정 불필요.
337
341
-`expiresAt`이 지난 권한을 정리(삭제 또는 자동 무효화)하는 배치가 없다 — live 조회 시 `expiresAt > CURRENT_TIMESTAMP` 조건으로 걸러지긴 하지만, 만료된 레코드 자체는 DB에 계속 쌓인다.
338
342
-`AccessSourceType.OWNER`가 정의만 되어 있고 실제로 생성되지 않는다(위 "확인된 불일치" 참고) — enum에서 제거하거나, 실제로 OWNER 캐시를 생성하도록 코드를 맞추거나 둘 중 하나로 정리가 필요하다.
339
343
-~~`PermissionController`의 컬렉션/문서 권한 부여·회수 4개 엔드포인트 Swagger description이 "소유자(owner)만 가능"이라고 적혀 있던 문제~~ → 코드리뷰로 발견해 실제 인가 규칙(`canAdminCollection()`/`canAdminDocument()`, ADMIN 위임자도 허용)에 맞게 4곳 모두 "ADMIN 권한 보유자(소유자 포함)"로 수정 완료.
이 이슈의 핵심 파일이며, 6개의 public 메서드를 제공한다(`canReadCollection`은 이후 리팩토링에서 추가됨 — 아래 참고). 컬렉션 대상 3종(`canReadCollection`/`canWriteCollection`/`canAdminCollection`)은 각각 ID로 조회하는 버전과, 이미 조회된 `DocumentCollection` 엔티티를 받는 버전 2개씩 오버로드로 제공한다 — 호출부가 이미 엔티티를 들고 있으면 중복 조회 없이 엔티티 버전을 바로 쓸 수 있다.
38
38
39
39
#### `canReadDocument` (5단계)
40
40
@@ -76,19 +76,32 @@ public boolean canReadDocument(Long userId, Long documentId) {
76
76
77
77
`canReadDocument`와 거의 같은 구조이지만 **PUBLIC 단계가 없다** — PUBLIC은 읽기만 허용하는 개념이라 쓰기/관리 권한 판단에는 끼어들 자리가 없다. 그래서 1단계(소유자) → 2단계(USER 캐시) → 3단계(ROLE) → 4단계(DEPT), 총 4단계다.
컬렉션 판단에는 **캐시가 없다** — `user_document_access_cache`는 문서 단위 캐시라 컬렉션 자체에 대한 캐시 개념이 없다. 그래서 OWNER, USER 직접 권한, ROLE, DEPT를 매번 순서대로 live 조회한다. `#16`의 `addDocument()`, `#18`의 `grantPermission()`이 이 메서드들을 그대로 호출한다.
102
+
`canReadCollection`은 위와 동일한 구조에 **PUBLIC 단계**만 추가된다(`canReadDocument`의 2단계와 동일하게, `collection.getVisibility() == VisibilityType.PUBLIC`이면 소유자/권한 여부와 무관하게 허용). `canAdminCollection`도 같은 뼈대(대상 권한 종류만 다름)다.
103
+
104
+
컬렉션 판단에는 **캐시가 없다** — `user_document_access_cache`는 문서 단위 캐시라 컬렉션 자체에 대한 캐시 개념이 없다. 그래서 OWNER, (READ의 경우 PUBLIC), USER 직접 권한, ROLE, DEPT를 매번 순서대로 live 조회한다. `#16`의 `addDocument()`/`getCollection()`, `#18`의 `grantPermission()`이 엔티티 버전을 호출해 중복 조회 없이 이 메서드들을 재사용한다.
@@ -159,7 +172,7 @@ if (cacheRepository.existsValidReadCache(...)) { ... }
159
172
$ ./gradlew test --tests "*PermissionQueryServiceTest*"
160
173
BUILD SUCCESSFUL
161
174
```
162
-
`PermissionQueryServiceTest`35개 모두 통과(현재 기준 재검증) — 이 서비스의 메서드 5개(`canReadDocument`, `canWriteDocument`, `canAdminDocument`, `canWriteCollection`, `canAdminCollection`) 각각의 단계별 분기를 검증하는 테스트가 다수 포함되어 있다.
175
+
`PermissionQueryServiceTest`43개 모두 통과(현재 기준 재검증) — 이 서비스의 메서드 6개(`canReadDocument`, `canWriteDocument`, `canAdminDocument`, `canReadCollection`, `canWriteCollection`, `canAdminCollection`) 각각의 단계별 분기를 검증하는 테스트가 다수 포함되어 있다.
163
176
164
177
---
165
178
@@ -182,12 +195,14 @@ BUILD SUCCESSFUL
182
195
183
196
**컬렉션 판단에는 캐시를 두지 않음**: `user_document_access_cache`가 애초에 문서 단위로 설계되어 있어서, 컬렉션 자체에 대한 접근 여부는 캐시할 방법이 없다(캐시하려면 별도 테이블이 필요했을 것). 컬렉션 판단은 상대적으로 호출 빈도가 낮다고 보고(문서 접근이 압도적으로 잦음) live 조회만으로 충분하다고 판단한 것으로 보인다.
184
197
198
+
**(추가) `canReadCollection` 도입 + ID/엔티티 오버로드 분리**: `getCollection()`(`#16`) 단건 조회에 권한 체크가 전혀 없던 게 확인되어 `canReadCollection`(OWNER→PUBLIC→USER→ROLE→DEPT)을 추가했다. 동시에 `getCollection()`/`addDocument()`/`grantPermission()`이 컬렉션을 조회한 뒤 권한 메서드를 ID로 다시 호출해 같은 row를 두 번 SELECT하던 문제와, `findById()`가 soft-delete(`status=DELETED`)를 걸러내지 않던 문제가 함께 발견되어, 컬렉션 대상 3개 메서드를 "ID 버전(내부에서 조회+status 필터 후 위임) + 엔티티 버전(조회 없이 판단)"으로 나눠 한 번에 해결했다.
199
+
185
200
---
186
201
187
202
## 남은 이슈 / TODO
188
203
189
204
- 나노초 기반 성능 로그가 프로덕션에서도 항상 `log.info()`로 남는다 — 트래픽이 많아지면 로그량 자체가 부담일 수 있어 로그 레벨 조정이나 샘플링이 필요할 수 있다.
190
-
-`canReadDocument`류 메서드들 사이에 문서 조회(`documentRepository.findById`)가 메서드마다 중복된다 — 셋 다 필요하면(예: `checkDocumentPermission`처럼) 문서 조회를 한 번만 하고 넘기는 내부 메서드로 리팩터링할 여지가 있다.
205
+
-`canReadDocument`류 메서드들 사이에 문서 조회(`documentRepository.findById`)가 메서드마다 중복된다 — 셋 다 필요하면(예: `checkDocumentPermission`처럼) 문서 조회를 한 번만 하고 넘기는 내부 메서드로 리팩터링할 여지가 있다.**(참고)** 컬렉션 쪽(`canReadCollection`/`canWriteCollection`/`canAdminCollection`)은 이미 "ID 버전 + 엔티티 버전" 오버로드로 이 문제를 해결했다 — 문서 쪽도 같은 패턴을 그대로 적용하면 된다(별도 후속 작업으로 분리, 이번 라운드는 컬렉션만 처리).
0 commit comments