From 7aa158e4066825d19f55505aea8cefbb164fbf44 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Mon, 11 Mar 2024 12:01:23 +0100 Subject: [PATCH] amendments according to code review --- .../hsadminng/rbac/rbacdef/RbacView.java | 2 +- ...=> RbacViewMermaidFlowchartGenerator.java} | 8 +- .../resources/db/changelog/010-context.sql | 13 +-- .../resources/db/changelog/020-audit-log.sql | 4 +- .../db/changelog/051-rbac-user-grant.sql | 2 - .../db/changelog/113-test-customer-rbac.md | 4 +- .../db/changelog/113-test-customer-rbac.sql | 2 +- .../db/changelog/123-test-package-rbac.md | 4 +- .../db/changelog/123-test-package-rbac.sql | 2 +- .../db/changelog/133-test-domain-rbac.md | 88 +++++++++++++++++++ .../db/changelog/133-test-domain-rbac.sql | 2 +- ...fficeDebitorRepositoryIntegrationTest.java | 1 - .../RbacGrantControllerAcceptanceTest.java | 2 + .../test/cust/TestCustomerEntityUnitTest.java | 4 +- .../test/pac/TestPackageEntityUnitTest.java | 4 +- 15 files changed, 119 insertions(+), 23 deletions(-) rename src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/{RbacViewMermaidFlowchart.java => RbacViewMermaidFlowchartGenerator.java} (95%) create mode 100644 src/main/resources/db/changelog/133-test-domain-rbac.md diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RbacView.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RbacView.java index caa2405d..28d29365 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RbacView.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RbacView.java @@ -264,7 +264,7 @@ public class RbacView { } public void generateWithBaseFileName(final String baseFileName) { - new RbacViewMermaidFlowchart(this).generateToMarkdownFile(Path.of(OUTPUT_BASEDIR, baseFileName + ".md")); + new RbacViewMermaidFlowchartGenerator(this).generateToMarkdownFile(Path.of(OUTPUT_BASEDIR, baseFileName + ".md")); new RbacViewPostgresGenerator(this).generateToChangeLog(Path.of(OUTPUT_BASEDIR, baseFileName + ".sql")); } diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RbacViewMermaidFlowchart.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RbacViewMermaidFlowchartGenerator.java similarity index 95% rename from src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RbacViewMermaidFlowchart.java rename to src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RbacViewMermaidFlowchartGenerator.java index 9a806cc4..ccef566d 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RbacViewMermaidFlowchart.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RbacViewMermaidFlowchartGenerator.java @@ -9,7 +9,7 @@ import java.time.LocalDateTime; import static java.util.stream.Collectors.joining; import static net.hostsharing.hsadminng.rbac.rbacdef.RbacView.RbacGrantDefinition.GrantType.*; -public class RbacViewMermaidFlowchart { +public class RbacViewMermaidFlowchartGenerator { public static final String HOSTSHARING_DARK_ORANGE = "#dd4901"; public static final String HOSTSHARING_LIGHT_ORANGE = "#feb28c"; @@ -18,7 +18,7 @@ public class RbacViewMermaidFlowchart { private final RbacView rbacDef; private final StringWriter flowchart = new StringWriter(); - public RbacViewMermaidFlowchart(final RbacView rbacDef) { + public RbacViewMermaidFlowchartGenerator(final RbacView rbacDef) { this.rbacDef = rbacDef; flowchart.writeLn(""" %%{init:{'flowchart':{'htmlLabels':false}}}%% @@ -147,7 +147,9 @@ public class RbacViewMermaidFlowchart { Files.writeString( path, """ - ### rbac %{entityAlias} %{timestamp} + ### rbac %{entityAlias} + + This code generated was by RbacViewMermaidFlowchartGenerator at %{timestamp}. ```mermaid %{flowchart} diff --git a/src/main/resources/db/changelog/010-context.sql b/src/main/resources/db/changelog/010-context.sql index d1129184..8de41891 100644 --- a/src/main/resources/db/changelog/010-context.sql +++ b/src/main/resources/db/changelog/010-context.sql @@ -24,23 +24,26 @@ end; $$; */ create or replace procedure defineContext( currentTask varchar(96), - currentRequest varchar(512) = null, - currentUser varchar = null, - assumedRoles varchar = null + currentRequest text = null, + currentUser varchar(63) = null, + assumedRoles varchar(256) = null ) language plpgsql as $$ begin - assert length(currentTask) <= 96, 'currentTask must not be longer than 96 characters'; - assert length(currentTask) > 8, 'currentTask must be at least 8 characters long'; + currentTask := coalesce(currentTask, ''); + assert length(currentTask) <= 96, FORMAT('currentTask must not be longer than 96 characters: "%s"', currentTask); + assert length(currentTask) > 8, FORMAT('currentTask must be at least 8 characters long: "%s""', currentTask); execute format('set local hsadminng.currentTask to %L', currentTask); currentRequest := coalesce(currentRequest, ''); execute format('set local hsadminng.currentRequest to %L', currentRequest); currentUser := coalesce(currentUser, ''); + assert length(currentUser) <= 63, FORMAT('currentUser must not be longer than 63 characters: "%s"', currentUser); execute format('set local hsadminng.currentUser to %L', currentUser); assumedRoles := coalesce(assumedRoles, ''); + assert length(assumedRoles) <= 256, FORMAT('assumedRoles must not be longer than 256 characters: "%s"', assumedRoles); execute format('set local hsadminng.assumedRoles to %L', assumedRoles); call contextDefined(currentTask, currentRequest, currentUser, assumedRoles); diff --git a/src/main/resources/db/changelog/020-audit-log.sql b/src/main/resources/db/changelog/020-audit-log.sql index 173e5741..ec14ad0d 100644 --- a/src/main/resources/db/changelog/020-audit-log.sql +++ b/src/main/resources/db/changelog/020-audit-log.sql @@ -27,9 +27,9 @@ create table tx_context txId bigint not null, txTimestamp timestamp not null, currentUser varchar(63) not null, -- not the uuid, because users can be deleted - assumedRoles varchar not null, -- not the uuids, because roles can be deleted + assumedRoles varchar(256) not null, -- not the uuids, because roles can be deleted currentTask varchar(96) not null, - currentRequest varchar(512) not null + currentRequest text not null ); create index on tx_context using brin (txTimestamp); diff --git a/src/main/resources/db/changelog/051-rbac-user-grant.sql b/src/main/resources/db/changelog/051-rbac-user-grant.sql index beeeb7d2..a82865c8 100644 --- a/src/main/resources/db/changelog/051-rbac-user-grant.sql +++ b/src/main/resources/db/changelog/051-rbac-user-grant.sql @@ -119,8 +119,6 @@ end; $$; create or replace procedure revokePermissionFromRole(permissionUuid uuid, superRoleUuid uuid) language plpgsql as $$ begin - -- TODO: call checkRevokeRoleFromUserPreconditions(grantedByRoleUuid, grantedRoleUuid, userUuid); - raise INFO 'delete from RbacGrants where ascendantUuid = % and descendantUuid = %', superRoleUuid, permissionUuid; delete from RbacGrants as g where g.ascendantUuid = superRoleUuid and g.descendantUuid = permissionUuid; diff --git a/src/main/resources/db/changelog/113-test-customer-rbac.md b/src/main/resources/db/changelog/113-test-customer-rbac.md index f126a216..7770e470 100644 --- a/src/main/resources/db/changelog/113-test-customer-rbac.md +++ b/src/main/resources/db/changelog/113-test-customer-rbac.md @@ -1,4 +1,6 @@ -### rbac customer 2024-03-11T09:06:04.484587070 +### rbac customer + +This code generated was by RbacViewMermaidFlowchartGenerator at 2024-03-11T11:29:11.571772062. ```mermaid %%{init:{'flowchart':{'htmlLabels':false}}}%% diff --git a/src/main/resources/db/changelog/113-test-customer-rbac.sql b/src/main/resources/db/changelog/113-test-customer-rbac.sql index da758029..6ae19710 100644 --- a/src/main/resources/db/changelog/113-test-customer-rbac.sql +++ b/src/main/resources/db/changelog/113-test-customer-rbac.sql @@ -1,5 +1,5 @@ --liquibase formatted sql --- This code generated was by RbacViewPostgresGenerator at 2024-03-11T09:06:04.497071201. +-- This code generated was by RbacViewPostgresGenerator at 2024-03-11T11:29:11.584886824. -- ============================================================================ --changeset test-customer-rbac-OBJECT:1 endDelimiter:--// diff --git a/src/main/resources/db/changelog/123-test-package-rbac.md b/src/main/resources/db/changelog/123-test-package-rbac.md index 4b56651a..78da4439 100644 --- a/src/main/resources/db/changelog/123-test-package-rbac.md +++ b/src/main/resources/db/changelog/123-test-package-rbac.md @@ -1,4 +1,6 @@ -### rbac package 2024-03-11T09:06:04.536081351 +### rbac package + +This code generated was by RbacViewMermaidFlowchartGenerator at 2024-03-11T11:29:11.624847792. ```mermaid %%{init:{'flowchart':{'htmlLabels':false}}}%% diff --git a/src/main/resources/db/changelog/123-test-package-rbac.sql b/src/main/resources/db/changelog/123-test-package-rbac.sql index 371a28ff..20562642 100644 --- a/src/main/resources/db/changelog/123-test-package-rbac.sql +++ b/src/main/resources/db/changelog/123-test-package-rbac.sql @@ -1,5 +1,5 @@ --liquibase formatted sql --- This code generated was by RbacViewPostgresGenerator at 2024-03-11T09:06:04.536525766. +-- This code generated was by RbacViewPostgresGenerator at 2024-03-11T11:29:11.625353859. -- ============================================================================ --changeset test-package-rbac-OBJECT:1 endDelimiter:--// diff --git a/src/main/resources/db/changelog/133-test-domain-rbac.md b/src/main/resources/db/changelog/133-test-domain-rbac.md new file mode 100644 index 00000000..bd5cf706 --- /dev/null +++ b/src/main/resources/db/changelog/133-test-domain-rbac.md @@ -0,0 +1,88 @@ +### rbac domain + +This code generated was by RbacViewMermaidFlowchartGenerator at 2024-03-11T11:29:11.644658132. + +```mermaid +%%{init:{'flowchart':{'htmlLabels':false}}}%% +flowchart TB + +subgraph package.customer["`**package.customer**`"] + direction TB + style package.customer fill:#99bcdb,stroke:#274d6e,stroke-width:8px + + subgraph package.customer:roles[ ] + style package.customer:roles fill:#99bcdb,stroke:white + + role:package.customer:owner[[package.customer:owner]] + role:package.customer:admin[[package.customer:admin]] + role:package.customer:tenant[[package.customer:tenant]] + end +end + +subgraph package["`**package**`"] + direction TB + style package fill:#99bcdb,stroke:#274d6e,stroke-width:8px + + subgraph package.customer["`**package.customer**`"] + direction TB + style package.customer fill:#99bcdb,stroke:#274d6e,stroke-width:8px + + subgraph package.customer:roles[ ] + style package.customer:roles fill:#99bcdb,stroke:white + + role:package.customer:owner[[package.customer:owner]] + role:package.customer:admin[[package.customer:admin]] + role:package.customer:tenant[[package.customer:tenant]] + end + end + + subgraph package:roles[ ] + style package:roles fill:#99bcdb,stroke:white + + role:package:owner[[package:owner]] + role:package:admin[[package:admin]] + role:package:tenant[[package:tenant]] + end +end + +subgraph domain["`**domain**`"] + direction TB + style domain fill:#dd4901,stroke:#274d6e,stroke-width:8px + + subgraph domain:roles[ ] + style domain:roles fill:#dd4901,stroke:white + + role:domain:owner[[domain:owner]] + role:domain:admin[[domain:admin]] + end + + subgraph domain:permissions[ ] + style domain:permissions fill:#dd4901,stroke:white + + perm:domain:INSERT{{domain:INSERT}} + perm:domain:DELETE{{domain:DELETE}} + perm:domain:UPDATE{{domain:UPDATE}} + perm:domain:SELECT{{domain:SELECT}} + end +end + +%% granting roles to roles +role:global:admin -.->|XX| role:package.customer:owner +role:package.customer:owner -.-> role:package.customer:admin +role:package.customer:admin -.-> role:package.customer:tenant +role:package.customer:admin -.-> role:package:owner +role:package:owner -.-> role:package:admin +role:package:admin -.-> role:package:tenant +role:package:tenant -.-> role:package.customer:tenant +role:package:admin ==> role:domain:owner +role:domain:owner ==> role:package:tenant +role:domain:owner ==> role:domain:admin +role:domain:admin ==> role:package:tenant + +%% granting permissions to roles +role:package:admin ==> perm:domain:INSERT +role:domain:owner ==> perm:domain:DELETE +role:domain:owner ==> perm:domain:UPDATE +role:domain:admin ==> perm:domain:SELECT + +``` diff --git a/src/main/resources/db/changelog/133-test-domain-rbac.sql b/src/main/resources/db/changelog/133-test-domain-rbac.sql index c230bcde..e686dada 100644 --- a/src/main/resources/db/changelog/133-test-domain-rbac.sql +++ b/src/main/resources/db/changelog/133-test-domain-rbac.sql @@ -1,5 +1,5 @@ --liquibase formatted sql --- This code generated was by RbacViewPostgresGenerator at 2024-03-11T09:06:04.558752062. +-- This code generated was by RbacViewPostgresGenerator at 2024-03-11T11:29:11.645391647. -- ============================================================================ --changeset test-domain-rbac-OBJECT:1 endDelimiter:--// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java index 7c2420ee..46d0878f 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java @@ -118,7 +118,6 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean }); // then - System.out.println("ok"); result.assertExceptionWithRootCauseMessage(org.hibernate.exception.ConstraintViolationException.class); } diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantControllerAcceptanceTest.java index fdf7e693..f56baf34 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantControllerAcceptanceTest.java @@ -73,6 +73,7 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { .contentType("application/json") .body("", hasItem( allOf( + // TODO: should there be a grantedByRole or just a grantedByTrigger? hasEntry("grantedByRoleIdName", "test_customer#xxx.owner"), hasEntry("grantedRoleIdName", "test_customer#xxx.admin"), hasEntry("granteeUserName", "customer-admin@xxx.example.com") @@ -80,6 +81,7 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { )) .body("", hasItem( allOf( + // TODO: should there be a grantedByRole or just a grantedByTrigger? hasEntry("grantedByRoleIdName", "test_customer#yyy.owner"), hasEntry("grantedRoleIdName", "test_customer#yyy.admin"), hasEntry("granteeUserName", "customer-admin@yyy.example.com") diff --git a/src/test/java/net/hostsharing/hsadminng/test/cust/TestCustomerEntityUnitTest.java b/src/test/java/net/hostsharing/hsadminng/test/cust/TestCustomerEntityUnitTest.java index 4dfcc183..eca0aec1 100644 --- a/src/test/java/net/hostsharing/hsadminng/test/cust/TestCustomerEntityUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/test/cust/TestCustomerEntityUnitTest.java @@ -1,6 +1,6 @@ package net.hostsharing.hsadminng.test.cust; -import net.hostsharing.hsadminng.rbac.rbacdef.RbacViewMermaidFlowchart; +import net.hostsharing.hsadminng.rbac.rbacdef.RbacViewMermaidFlowchartGenerator; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -9,7 +9,7 @@ class TestCustomerEntityUnitTest { @Test void definesRbac() { - final var rbacFlowchart = new RbacViewMermaidFlowchart(TestCustomerEntity.rbac()).toString(); + final var rbacFlowchart = new RbacViewMermaidFlowchartGenerator(TestCustomerEntity.rbac()).toString(); assertThat(rbacFlowchart).isEqualTo(""" %%{init:{'flowchart':{'htmlLabels':false}}}%% flowchart TB diff --git a/src/test/java/net/hostsharing/hsadminng/test/pac/TestPackageEntityUnitTest.java b/src/test/java/net/hostsharing/hsadminng/test/pac/TestPackageEntityUnitTest.java index 2546a8e8..c5dccfd3 100644 --- a/src/test/java/net/hostsharing/hsadminng/test/pac/TestPackageEntityUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/test/pac/TestPackageEntityUnitTest.java @@ -1,6 +1,6 @@ package net.hostsharing.hsadminng.test.pac; -import net.hostsharing.hsadminng.rbac.rbacdef.RbacViewMermaidFlowchart; +import net.hostsharing.hsadminng.rbac.rbacdef.RbacViewMermaidFlowchartGenerator; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -9,7 +9,7 @@ class TestPackageEntityUnitTest { @Test void definesRbac() { - final var rbacFlowchart = new RbacViewMermaidFlowchart(TestPackageEntity.rbac()).toString(); + final var rbacFlowchart = new RbacViewMermaidFlowchartGenerator(TestPackageEntity.rbac()).toString(); assertThat(rbacFlowchart).isEqualTo(""" %%{init:{'flowchart':{'htmlLabels':false}}}%% flowchart TB