From c5b00ff62314942f6b2cee09993d9ddd8648e8e2 Mon Sep 17 00:00:00 2001 From: "Zimin A.N." Date: Wed, 13 May 2026 12:44:23 +0300 Subject: [PATCH] refactor(auth): domain-specific role names (consistent convention) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User feedback: универсальный 'ordinis:approver' отступал от convention 'ordinis::'. Меняю на domain-specific: WorkflowRoles constants: - APPROVER → SCHEMA_REVIEWER ('ordinis:schema:reviewer') - PUBLISHER → SCHEMA_PUBLISHER ('ordinis:schema:publisher') - Новая RECORD_REVIEWER ('ordinis:record:reviewer') Applied: - SchemaDraftService.decide() → require SCHEMA_REVIEWER - SchemaDraftService.publish() → require SCHEMA_PUBLISHER - DraftService.approve() / .reject() → require RECORD_REVIEWER Frontend usePermissions.ts: - ROLE_APPROVER + ROLE_PUBLISHER → ROLE_SCHEMA_REVIEWER + ROLE_SCHEMA_PUBLISHER + ROLE_RECORD_REVIEWER - Новый hook useCanReviewRecord() для record draft actions Keycloak setup (updated): - ordinis:schema:reviewer - ordinis:schema:publisher - ordinis:record:reviewer (Composite role 'ordinis:admin' = all три + scope roles — для удобства.) --- .gitignore | 1 + ordinis-admin-ui/src/auth/usePermissions.ts | 36 ++++++++++------- .../ordinis/restapi/auth/WorkflowRoles.java | 40 +++++++++++-------- .../restapi/service/draft/DraftService.java | 6 +-- .../schemaworkflow/SchemaDraftService.java | 13 +++--- .../restapi/auth/WorkflowRolesTest.java | 26 ++++++------ 6 files changed, 68 insertions(+), 54 deletions(-) diff --git a/.gitignore b/.gitignore index 2bdc03d..ec6bf16 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,4 @@ HELP.md # Sensitive *.pem *.key +.gstack/ diff --git a/ordinis-admin-ui/src/auth/usePermissions.ts b/ordinis-admin-ui/src/auth/usePermissions.ts index 10ecc4b..49c7117 100644 --- a/ordinis-admin-ui/src/auth/usePermissions.ts +++ b/ordinis-admin-ui/src/auth/usePermissions.ts @@ -4,10 +4,9 @@ import { useAuth } from 'react-oidc-context' * Permission helpers для schema + record workflow. Phase 1d — **ACTIVE**: * читает {@code realm_access.roles} из JWT и гейтит actions. * - *

Naming convention: colon-separated namespace - * {@code ordinis::} consistent с scope ACL ролями - * ({@code ordinis:client:public}). Backend parser в - * {@code ScopeContext.java} / {@code WorkflowRoles.java}. + *

Naming convention: colon-separated {@code ordinis::} + * pattern (consistent с scope ACL {@code ordinis:client:public}). Backend + * parser в {@code WorkflowRoles.java}. * *

Backend enforcement (см. {@code WorkflowRoles}): *

    @@ -21,13 +20,19 @@ import { useAuth } from 'react-oidc-context' * backend всё равно 403'нет на прямой API call. * *

    Migration note (Phase 1d enforcement): Keycloak realm `nstart` - * должен иметь созданные роли {@code ordinis:approver} и - * {@code ordinis:publisher}, и actual reviewer-users должны их получить. - * До этого все decide / publish операции вернут {@code 403 forbidden_role}. + * должен иметь созданные роли: + *

      + *
    • {@code ordinis:schema:reviewer} — schema decide
      • + *
    • {@code ordinis:schema:publisher} — schema publish
      • + *
    • {@code ordinis:record:reviewer} — record approve/reject
      • + *
    + * и actual reviewer-users должны их получить. До этого все decide / + * publish операции вернут {@code 403 forbidden_role}. */ -export const ROLE_APPROVER = 'ordinis:approver' -export const ROLE_PUBLISHER = 'ordinis:publisher' +export const ROLE_SCHEMA_REVIEWER = 'ordinis:schema:reviewer' +export const ROLE_SCHEMA_PUBLISHER = 'ordinis:schema:publisher' +export const ROLE_RECORD_REVIEWER = 'ordinis:record:reviewer' /** Достаёт realm roles из JWT (Keycloak claim path: realm_access.roles). */ function realmRoles(profile: unknown): string[] { @@ -43,19 +48,22 @@ function hasRole(profile: unknown, role: string): boolean { return realmRoles(profile).includes(role) } -/** Может ли актор approve / request_changes / reject schema OR record draft. */ +/** Может ли актор approve / request_changes / reject schema draft. */ export function useCanReviewSchema(): boolean { const auth = useAuth() - return auth.isAuthenticated && hasRole(auth.user?.profile, ROLE_APPROVER) + return auth.isAuthenticated && hasRole(auth.user?.profile, ROLE_SCHEMA_REVIEWER) } -/** Universal alias — то же что {@link useCanReviewSchema}. */ -export const useCanReviewDraft = useCanReviewSchema +/** Может ли актор approve / reject record draft. */ +export function useCanReviewRecord(): boolean { + const auth = useAuth() + return auth.isAuthenticated && hasRole(auth.user?.profile, ROLE_RECORD_REVIEWER) +} /** Может ли актор publish approved schema draft (bump live version). */ export function useCanPublishSchema(): boolean { const auth = useAuth() - return auth.isAuthenticated && hasRole(auth.user?.profile, ROLE_PUBLISHER) + return auth.isAuthenticated && hasRole(auth.user?.profile, ROLE_SCHEMA_PUBLISHER) } /** diff --git a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/auth/WorkflowRoles.java b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/auth/WorkflowRoles.java index 7cf33ea..b67ab06 100644 --- a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/auth/WorkflowRoles.java +++ b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/auth/WorkflowRoles.java @@ -17,16 +17,17 @@ import java.util.Set; * Realm-role enforcement для workflow actions (approve/reject/publish * как record draft, так и schema draft). * - *

    Naming convention: colon-separated namespace, тот же что у scope ACL - * ({@code ordinis:client:public}). Constants: + *

    Naming convention: colon-separated {@code ordinis::} + * pattern (consistent с scope ACL {@code ordinis:client:public}). + * Domain-specific роли для clear separation между record и schema workflows: *

      - *
    • {@link #APPROVER} {@value APPROVER} — gate на approve / reject / - * request_changes для record AND schema drafts. Single role для - * всего decide layer'а — easier to assign в Keycloak (one role - * grants both review workflows).
      • - *
    • {@link #PUBLISHER} {@value PUBLISHER} — gate на publish approved - * schema draft. У records publish auto-выполняется при approve, - * отдельной publish step нет.
      • + *
    • {@link #SCHEMA_REVIEWER} {@value SCHEMA_REVIEWER} — gate на + * schema draft decide (approve / request_changes / reject).
      • + *
    • {@link #SCHEMA_PUBLISHER} {@value SCHEMA_PUBLISHER} — gate на + * publish approved schema draft → live. У records publish + * auto-выполняется при approve, отдельной step'и нет.
      • + *
    • {@link #RECORD_REVIEWER} {@value RECORD_REVIEWER} — gate на + * record draft approve / reject (Approval Workflow v2).
      • *
    * *

    Maker side (create / submit / withdraw) пока не role-gated — любой @@ -37,21 +38,26 @@ import java.util.Set; *

    JWT источник: {@code realm_access.roles} claim (Keycloak realm roles). * Совместимо с {@code ScopeContext} JWT parsing pattern. * - *

    Migration note (Phase 1d enforcement): до deploy этой версии - * Keycloak realm должен иметь созданные роли {@code ordinis:approver} и - * {@code ordinis:publisher}, и actual reviewer-users должны их получить. - * Иначе все decide / publish операции вернут {@code 403 forbidden_role}. + *

    Migration note (Phase 1d enforcement): Keycloak realm должен + * иметь созданные роли, и users должны их получить. Иначе все decide / + * publish операции вернут {@code 403 forbidden_role}. + * + *

    Composite role idea: {@code ordinis:admin} = combined из всех трёх + * + scope role'ов — single assignment grants full workflow access. */ @Component public class WorkflowRoles { private static final Logger log = LoggerFactory.getLogger(WorkflowRoles.class); - /** Universal approver роль — gate на decide / approve / reject. */ - public static final String APPROVER = "ordinis:approver"; + /** Approve / reject / request_changes schema draft (Phase 1c workflow). */ + public static final String SCHEMA_REVIEWER = "ordinis:schema:reviewer"; - /** Publish approved schema draft → live (bump dictionary live version). */ - public static final String PUBLISHER = "ordinis:publisher"; + /** Publish approved schema draft → live (bump dictionary schema version). */ + public static final String SCHEMA_PUBLISHER = "ordinis:schema:publisher"; + + /** Approve / reject record draft (Approval Workflow v2). */ + public static final String RECORD_REVIEWER = "ordinis:record:reviewer"; /** Read realm_access.roles из current request JWT. Anonymous → empty. */ public Set currentRoles() { diff --git a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/draft/DraftService.java b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/draft/DraftService.java index bb7bbf8..5e65ad1 100644 --- a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/draft/DraftService.java +++ b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/draft/DraftService.java @@ -223,8 +223,8 @@ public class DraftService { if (recordService == null) { throw new IllegalStateException("DictionaryRecordService not wired (test ctor used)."); } - // Phase 1d: realm role gate (universal 'ordinis:approver'). - requireRoleIfEnforced(cloud.nstart.terravault.ordinis.restapi.auth.WorkflowRoles.APPROVER); + // Phase 1d: realm role gate (record-specific 'ordinis:record:reviewer'). + requireRoleIfEnforced(cloud.nstart.terravault.ordinis.restapi.auth.WorkflowRoles.RECORD_REVIEWER); String reviewerId = currentUserId(); RecordDraft draft = findById(draftId); if (draft.getStatus() != DraftStatus.PENDING) { @@ -273,7 +273,7 @@ public class DraftService { @Transactional public RecordDraft reject(UUID draftId, String reviewComment) { // Phase 1d: same role как approve — reviewer'ы могут оба действия. - requireRoleIfEnforced(cloud.nstart.terravault.ordinis.restapi.auth.WorkflowRoles.APPROVER); + requireRoleIfEnforced(cloud.nstart.terravault.ordinis.restapi.auth.WorkflowRoles.RECORD_REVIEWER); if (reviewComment == null || reviewComment.isBlank()) { throw OrdinisException.badRequest( "reject_reason_required", diff --git a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/schemaworkflow/SchemaDraftService.java b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/schemaworkflow/SchemaDraftService.java index 1700f26..deb3602 100644 --- a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/schemaworkflow/SchemaDraftService.java +++ b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/schemaworkflow/SchemaDraftService.java @@ -244,9 +244,9 @@ public class SchemaDraftService { */ @Transactional public DictionarySchemaDraft decide(UUID draftId, DecisionRequest req) { - // Phase 1d: realm role gate. Без 'ordinis:approver' actor получит - // 403 forbidden_role до даже status checks — fail-fast. - requireRoleIfEnforced(WorkflowRoles.APPROVER); + // Phase 1d: realm role gate. Без 'ordinis:schema:reviewer' actor + // получит 403 forbidden_role до даже status checks — fail-fast. + requireRoleIfEnforced(WorkflowRoles.SCHEMA_REVIEWER); DictionarySchemaDraft draft = findById(draftId); String reviewerId = currentUserId(); @@ -308,10 +308,9 @@ public class SchemaDraftService { */ @Transactional(isolation = Isolation.SERIALIZABLE) public DictionarySchemaDraft publish(UUID draftId, PublishDraftRequest req) { - // Phase 1d: publish — отдельная role от approver. У publisher могут - // быть более узкие пользователи (production gatekeepers), тогда как - // approvers более wide (любой senior reviewer). - requireRoleIfEnforced(WorkflowRoles.PUBLISHER); + // Phase 1d: publish — отдельная role от reviewer'а. Publishers + // обычно более узкая группа (production gatekeepers) чем reviewers. + requireRoleIfEnforced(WorkflowRoles.SCHEMA_PUBLISHER); DictionarySchemaDraft draft = findById(draftId); DictionaryDefinition def = definitionService.findById(draft.getDictionaryId()); diff --git a/ordinis-rest-api/src/test/java/cloud/nstart/terravault/ordinis/restapi/auth/WorkflowRolesTest.java b/ordinis-rest-api/src/test/java/cloud/nstart/terravault/ordinis/restapi/auth/WorkflowRolesTest.java index e24718f..d2ae2f8 100644 --- a/ordinis-rest-api/src/test/java/cloud/nstart/terravault/ordinis/restapi/auth/WorkflowRolesTest.java +++ b/ordinis-rest-api/src/test/java/cloud/nstart/terravault/ordinis/restapi/auth/WorkflowRolesTest.java @@ -36,50 +36,50 @@ class WorkflowRolesTest { @Test void anonymous_hasRole_false() { SecurityContextHolder.clearContext(); - assertThat(roles.hasRole(WorkflowRoles.APPROVER)).isFalse(); + assertThat(roles.hasRole(WorkflowRoles.SCHEMA_REVIEWER)).isFalse(); } @Test void jwtWithApprover_hasRole_true() { - setJwtWithRealmRoles(List.of(WorkflowRoles.APPROVER)); - assertThat(roles.hasRole(WorkflowRoles.APPROVER)).isTrue(); - assertThat(roles.hasRole(WorkflowRoles.PUBLISHER)).isFalse(); + setJwtWithRealmRoles(List.of(WorkflowRoles.SCHEMA_REVIEWER)); + assertThat(roles.hasRole(WorkflowRoles.SCHEMA_REVIEWER)).isTrue(); + assertThat(roles.hasRole(WorkflowRoles.SCHEMA_PUBLISHER)).isFalse(); } @Test void jwtWithMultipleRoles_currentRoles_contains() { setJwtWithRealmRoles(List.of( - WorkflowRoles.APPROVER, - WorkflowRoles.PUBLISHER, + WorkflowRoles.SCHEMA_REVIEWER, + WorkflowRoles.SCHEMA_PUBLISHER, "ordinis:client:restricted", "some-other-role")); assertThat(roles.currentRoles()) .containsExactlyInAnyOrder( - WorkflowRoles.APPROVER, - WorkflowRoles.PUBLISHER, + WorkflowRoles.SCHEMA_REVIEWER, + WorkflowRoles.SCHEMA_PUBLISHER, "ordinis:client:restricted", "some-other-role"); } @Test void requireRole_present_noThrow() { - setJwtWithRealmRoles(List.of(WorkflowRoles.APPROVER)); - roles.requireRole(WorkflowRoles.APPROVER); + setJwtWithRealmRoles(List.of(WorkflowRoles.SCHEMA_REVIEWER)); + roles.requireRole(WorkflowRoles.SCHEMA_REVIEWER); // no exception — passes } @Test void requireRole_missing_throws403() { setJwtWithRealmRoles(List.of("some-other-role")); - assertThatThrownBy(() -> roles.requireRole(WorkflowRoles.PUBLISHER)) + assertThatThrownBy(() -> roles.requireRole(WorkflowRoles.SCHEMA_PUBLISHER)) .isInstanceOf(OrdinisException.class) - .hasMessageContaining(WorkflowRoles.PUBLISHER); + .hasMessageContaining(WorkflowRoles.SCHEMA_PUBLISHER); } @Test void requireRole_anonymous_throws() { SecurityContextHolder.clearContext(); - assertThatThrownBy(() -> roles.requireRole(WorkflowRoles.APPROVER)) + assertThatThrownBy(() -> roles.requireRole(WorkflowRoles.SCHEMA_REVIEWER)) .isInstanceOf(OrdinisException.class); }