From e347c1bc2f847b2cc2e1e969599c462049537a4d Mon Sep 17 00:00:00 2001 From: devmrko Date: Thu, 25 Jun 2026 21:28:34 +0900 Subject: [PATCH] fix #457: align permission rule UI with VPD filters --- .../README.md | 82 +++++++++++++++++++ .../service/PermissionService.java | 10 ++- .../resources/mapper/PermissionMapper.xml | 8 +- src/main/resources/static/js/app.js | 42 +++++++++- src/main/resources/templates/permissions.html | 10 ++- .../service/PermissionServiceTest.java | 61 ++++++++++++++ 6 files changed, 201 insertions(+), 12 deletions(-) create mode 100644 docs/design/457-permission-rule-ui-alignment/README.md diff --git a/docs/design/457-permission-rule-ui-alignment/README.md b/docs/design/457-permission-rule-ui-alignment/README.md new file mode 100644 index 0000000..cec0061 --- /dev/null +++ b/docs/design/457-permission-rule-ui-alignment/README.md @@ -0,0 +1,82 @@ +# 설계서: 권한 UI와 VPD filter function의 rule type 의미 정렬 (#457) + +> **상태**: Approved +> **작성**: [AI] Architect · **최종수정**: 2026-06-25 +> **추적성** — Redmine: #457 · 관련 ADR: 없음 +> · 구현 파일: `PermissionService`, `PermissionMapper.xml`, `permissions.html`, `app.js` · 테스트: `PermissionServiceTest`, `mvn test` + +## 1. 목적 (Why) + +권한 화면에서 저장하는 행규칙의 의미가 실제 `cb_agent_doc_vpd_filter`가 해석하는 predicate와 일치하도록 한다. + +## 2. 범위 (Scope) + +- **포함**: rule type enum 정렬, `MY_DEPT/SELF` 기본 컬럼 허용, `DEPT/EMP_NO` rule type 추가, 적용 필터 preview 개선, 권한 저장 검증 테스트 보강. +- **제외**: DB VPD 함수 자체 재설계, ORDS handler 변경, 권한 수정 전용 화면 신설. + +## 3. 인수조건 (Acceptance Criteria) + +- [ ] UI rule type 목록이 VPD 함수 지원 타입과 일치한다: `ALL`, `MY_DEPT`, `SELF`, `DEPT`, `EMP_NO`, `=`, `!=`. +- [ ] `MY_DEPT`는 컬럼을 비우면 `DEPT_CODE`, `SELF`는 컬럼을 비우면 `OWNER_EMP_NO`로 저장 가능하다. +- [ ] `DEPT`, `EMP_NO`, `=`, `!=`는 비교 값이 없으면 저장을 거부한다. +- [ ] 권한 목록의 적용 필터 preview가 실제 함수 의미와 같은 기본 컬럼/컨텍스트를 보여준다. +- [ ] `mvn test`가 통과한다. + +## 4. 컨텍스트 & 제약 + +- `cb_agent_doc_vpd_filter`는 `DEPT`와 `EMP_NO`를 이미 지원한다. +- Java 서비스 검증은 저장 전 사용자 입력을 막는 1차 방어선이다. +- 실제 predicate 보안은 DB 함수에서 다시 fail-closed로 처리한다. + +## 5. 아키텍처 개요 + +``` +permissions.html rule input + -> PermissionController.buildRules + -> PermissionService.validateRules + -> cb_permission_rule 저장 + -> PermissionMapper filter_preview + -> cb_agent_doc_vpd_filter runtime predicate +``` + +## 6. 데이터 모델 + +- `rule_type`: `ALL`, `MY_DEPT`, `SELF`, `DEPT`, `EMP_NO`, `=`, `!=`. +- `rule_column`: 선택값. `MY_DEPT/SELF/DEPT/EMP_NO`는 비어 있으면 VPD 함수의 기본 컬럼을 사용한다. +- `rule_value`: `DEPT`, `EMP_NO`, `=`, `!=`에서 필수. + +## 7. 함수 명세 (Function Specs) + +| 함수 | 책임(1줄) | 시그니처(잠정) | 입력 | 출력 | 에러/실패 | 복잡? | +|------|-----------|----------------|------|------|-----------|-------| +| `validateRules` | 저장 전 rule type/column/value 조합 검증 | `void validateRules(long, List)` | objectId, rules | 없음 | AppException | **복잡** | +| `filter_preview SQL` | 저장된 rule을 사람이 읽는 predicate preview로 변환 | MyBatis select fragment | rule rows | text | 없음 | 단순 | +| `syncRuleTypeHints` | rule type별 UI placeholder/상태를 갱신 | JS DOM handler | rule row | DOM update | 없음 | 단순 | + +## 8. 흐름 / 알고리즘 + +1. 사용자가 보호 객체를 선택하면 컬럼 목록을 로드한다. +2. rule type을 선택하면 UI가 기본 컬럼/비교값 필요 여부를 표시한다. +3. 저장 시 서비스가 rule type과 컬럼/값 조합을 검증한다. +4. 저장된 권한 목록에서 실제 VPD 함수 해석과 같은 preview를 보여준다. + +## 9. 엣지케이스 & 에러 처리 + +- `ALL`은 단독만 허용한다. +- `MY_DEPT/SELF`의 컬럼 생략은 허용한다. +- 컬럼이 제공되면 보호 객체 컬럼 목록에 있어야 한다. +- 값이 필요한 타입에서 값이 없으면 저장 거부한다. + +## 10. 테스트 계획 + +- `PermissionServiceTest`에 기본 컬럼 허용과 값 필수 검증 추가. +- `mvn test`. + +## 11. 리스크 & 대안 검토 + +- 선택: Java 검증과 UI hint를 동시에 맞춘다. DB 함수 fail-closed만 의존하면 운영자가 저장 성공 후 조회가 안 되는 원인을 알기 어렵다. +- 대안: UI만 수정. 서버 검증과 불일치가 남아 회귀 위험이 크다. + +## 12. 미해결 질문 (Open Questions) + +- 권한 수정 전용 화면에서 기존 rule rows를 편집하는 UX는 별도 이슈로 분리할 수 있다. diff --git a/src/main/java/com/cloudhandson/vpdbackoffice/service/PermissionService.java b/src/main/java/com/cloudhandson/vpdbackoffice/service/PermissionService.java index ba3f37d..7ffda54 100644 --- a/src/main/java/com/cloudhandson/vpdbackoffice/service/PermissionService.java +++ b/src/main/java/com/cloudhandson/vpdbackoffice/service/PermissionService.java @@ -19,7 +19,9 @@ import org.springframework.transaction.annotation.Transactional; @Service public class PermissionService { - private static final Set RULE_TYPES = Set.of("ALL", "=", "!=", "MY_DEPT", "SELF"); + private static final Set RULE_TYPES = Set.of("ALL", "=", "!=", "MY_DEPT", "SELF", "DEPT", "EMP_NO"); + private static final Set VALUE_REQUIRED_RULE_TYPES = Set.of("=", "!=", "DEPT", "EMP_NO"); + private static final Set DEFAULT_COLUMN_RULE_TYPES = Set.of("MY_DEPT", "SELF", "DEPT", "EMP_NO"); private final PermissionMapper permissionMapper; private final ProtectedObjectService protectedObjectService; @@ -137,17 +139,17 @@ public class PermissionService { throw new AppException("허용되지 않은 행 규칙입니다: " + type); } String column = normalizeNullable(rule.ruleColumn()); - if (!"ALL".equals(type) && !allowedColumns.contains(column)) { + if (column != null && !allowedColumns.contains(column)) { throw new AppException("행 규칙 컬럼은 보호 객체 컬럼이어야 합니다: " + column); } if (!seen.add(column + ":" + type + ":" + clean(rule.ruleValue()))) { throw new AppException("중복된 행 규칙이 있습니다."); } hasAll = hasAll || "ALL".equals(type); - if (!"ALL".equals(type) && column.isBlank()) { + if (!"ALL".equals(type) && column == null && !DEFAULT_COLUMN_RULE_TYPES.contains(type)) { throw new AppException(type + " 규칙에는 컬럼이 필요합니다."); } - if (("=".equals(type) || "!=".equals(type)) && clean(rule.ruleValue()).isBlank()) { + if (VALUE_REQUIRED_RULE_TYPES.contains(type) && clean(rule.ruleValue()).isBlank()) { throw new AppException(type + " 규칙에는 값이 필요합니다."); } } diff --git a/src/main/resources/mapper/PermissionMapper.xml b/src/main/resources/mapper/PermissionMapper.xml index 955219b..2ce05d6 100644 --- a/src/main/resources/mapper/PermissionMapper.xml +++ b/src/main/resources/mapper/PermissionMapper.xml @@ -49,10 +49,12 @@ ) AS visible_columns, ( SELECT LISTAGG( - CASE + CASE WHEN pr2.rule_type = 'ALL' THEN 'ALL ROWS' - WHEN pr2.rule_type = 'MY_DEPT' THEN pr2.rule_column || ' = SYS_CONTEXT(CB_AGENT_CTX.DEPT_CODE)' - WHEN pr2.rule_type = 'SELF' THEN pr2.rule_column || ' = SYS_CONTEXT(CB_AGENT_CTX.EMP_NO)' + WHEN pr2.rule_type = 'MY_DEPT' THEN NVL(pr2.rule_column, 'DEPT_CODE') || ' = SYS_CONTEXT(CB_AGENT_CTX.DEPT_CODE)' + WHEN pr2.rule_type = 'SELF' THEN NVL(pr2.rule_column, 'OWNER_EMP_NO') || ' = SYS_CONTEXT(CB_AGENT_CTX.EMP_NO)' + WHEN pr2.rule_type = 'DEPT' THEN NVL(pr2.rule_column, 'DEPT_CODE') || ' = ' || pr2.rule_value + WHEN pr2.rule_type = 'EMP_NO' THEN NVL(pr2.rule_column, 'OWNER_EMP_NO') || ' = ' || pr2.rule_value ELSE pr2.rule_column || ' ' || pr2.rule_type || ' ' || pr2.rule_value END, CHR(10) || 'AND ' diff --git a/src/main/resources/static/js/app.js b/src/main/resources/static/js/app.js index 921645d..0962d37 100644 --- a/src/main/resources/static/js/app.js +++ b/src/main/resources/static/js/app.js @@ -150,7 +150,7 @@ function filterUserRoleDetail() { function renderRuleColumnOptions(columns) { document.querySelectorAll('.rule-column-select').forEach((select) => { const current = select.value; - select.innerHTML = '' + columns + select.innerHTML = '' + columns .map((column) => ``) .join(''); if (columns.includes(current)) { @@ -159,6 +159,38 @@ function renderRuleColumnOptions(columns) { }); } +function syncRuleTypeHints(root = document) { + root.querySelectorAll('.rule-row').forEach((row) => { + const typeSelect = row.querySelector('.rule-type-select'); + const columnSelect = row.querySelector('.rule-column-select'); + const valueInput = row.querySelector('input[name="ruleValue"]'); + if (!typeSelect || !columnSelect || !valueInput) { + return; + } + + const type = typeSelect.value; + const valueRequired = ['=', '!=', 'DEPT', 'EMP_NO'].includes(type); + const columnOptional = ['ALL', 'MY_DEPT', 'SELF', 'DEPT', 'EMP_NO'].includes(type); + const placeholderByType = { + ALL: '값 불필요', + MY_DEPT: '값 불필요', + SELF: '값 불필요', + DEPT: '예: HR', + EMP_NO: '예: E2001', + '=': '비교 값', + '!=': '비교 값' + }; + + columnSelect.required = !columnOptional; + valueInput.required = valueRequired; + valueInput.disabled = ['ALL', 'MY_DEPT', 'SELF'].includes(type); + valueInput.placeholder = placeholderByType[type] || '비교 값'; + if (valueInput.disabled) { + valueInput.value = ''; + } + }); +} + async function syncRuleColumnOptions() { const objectSelect = document.querySelector('select[name="objectRef"]'); if (!objectSelect) { @@ -213,6 +245,10 @@ document.addEventListener('DOMContentLoaded', () => { objectSelect.addEventListener('change', syncRuleColumnOptions); syncRuleColumnOptions(); } + document.querySelectorAll('.rule-type-select').forEach((select) => { + select.addEventListener('change', () => syncRuleTypeHints()); + }); + syncRuleTypeHints(); const catalog = document.getElementById('objectCatalogSelect'); if (catalog) { catalog.addEventListener('change', syncObjectCatalogSelection); @@ -229,8 +265,12 @@ document.addEventListener('DOMContentLoaded', () => { const cloneButton = clone.querySelector('[data-rule-add]'); cloneButton.textContent = '삭제'; cloneButton.addEventListener('click', () => clone.remove()); + clone.querySelectorAll('.rule-type-select').forEach((select) => { + select.addEventListener('change', () => syncRuleTypeHints()); + }); list.appendChild(clone); syncRuleColumnOptions(); + syncRuleTypeHints(clone); }); }); }); diff --git a/src/main/resources/templates/permissions.html b/src/main/resources/templates/permissions.html index 0b28077..cd048e7 100644 --- a/src/main/resources/templates/permissions.html +++ b/src/main/resources/templates/permissions.html @@ -42,18 +42,20 @@
- - - + + + + diff --git a/src/test/java/com/cloudhandson/vpdbackoffice/service/PermissionServiceTest.java b/src/test/java/com/cloudhandson/vpdbackoffice/service/PermissionServiceTest.java index 6012f29..3249a24 100644 --- a/src/test/java/com/cloudhandson/vpdbackoffice/service/PermissionServiceTest.java +++ b/src/test/java/com/cloudhandson/vpdbackoffice/service/PermissionServiceTest.java @@ -14,6 +14,7 @@ import com.cloudhandson.vpdbackoffice.domain.protectedobject.ProtectedColumn; import com.cloudhandson.vpdbackoffice.domain.protectedobject.ProtectedObject; import com.cloudhandson.vpdbackoffice.mapper.AuditMapper; import com.cloudhandson.vpdbackoffice.mapper.PermissionMapper; +import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -75,6 +76,63 @@ class PermissionServiceTest { .hasMessageContaining("허용되지 않은"); } + @Test + void allowsDefaultColumnsForContextRules() { + var command = new PermissionSetCommand( + 10L, + 1L, + "SELECT", + List.of(new RuleCommand(null, "MY_DEPT", null), new RuleCommand(null, "SELF", null)), + List.of() + ); + + permissionService.savePermissionSet(command); + + FakePermissionMapper mapper = (FakePermissionMapper) permissionMapper; + assertThat(mapper.insertedRules) + .extracting(PermissionRule::ruleType) + .containsExactly("MY_DEPT", "SELF"); + assertThat(mapper.insertedRules) + .extracting(PermissionRule::ruleColumn) + .containsExactly(null, null); + } + + @Test + void acceptsDeptAndEmpNoRulesWithDefaultColumnsAndValues() { + var command = new PermissionSetCommand( + 10L, + 1L, + "SELECT", + List.of(new RuleCommand(null, "DEPT", "HR"), new RuleCommand(null, "EMP_NO", "E2001")), + List.of() + ); + + permissionService.savePermissionSet(command); + + FakePermissionMapper mapper = (FakePermissionMapper) permissionMapper; + assertThat(mapper.insertedRules) + .extracting(PermissionRule::ruleType) + .containsExactly("DEPT", "EMP_NO"); + assertThat(mapper.insertedRules) + .extracting(PermissionRule::ruleValue) + .containsExactly("HR", "E2001"); + } + + @Test + void rejectsDeptRuleWithoutValue() { + var command = new PermissionSetCommand( + 10L, + 1L, + "SELECT", + List.of(new RuleCommand(null, "DEPT", "")), + List.of() + ); + + assertThatThrownBy(() -> permissionService.savePermissionSet(command)) + .isInstanceOf(AppException.class) + .hasMessageContaining("값이 필요"); + } + @Test void disablesProtectedObjectWhenLastPermissionIsDeleted() { var mapper = new FakePermissionMapper(); @@ -100,6 +158,8 @@ class PermissionServiceTest { } private static class FakePermissionMapper implements PermissionMapper { + private final List insertedRules = new ArrayList<>(); + @Override public List findRoles() { return List.of(new AppRole(10L, "HR_DEPT_ROLE", null)); @@ -163,6 +223,7 @@ class PermissionServiceTest { @Override public void insertRule(PermissionRule rule) { + insertedRules.add(rule); } @Override