From 142810442d31571ddce8afe519557f28f2504ea1 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Fri, 5 Jul 2024 11:47:16 +0200 Subject: [PATCH] amendments according to code-review --- .../hosting/asset/HsHostingAssetEntity.java | 4 +- .../7013-hs-hosting-asset-rbac.md | 2 - .../7013-hs-hosting-asset-rbac.sql | 2 - ...HostingAssetRepositoryIntegrationTest.java | 6 +- ...DnsSetupHostingAssetValidatorUnitTest.java | 105 +++++++++++------- 5 files changed, 69 insertions(+), 50 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java index efd768e0..80f9294c 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java @@ -204,7 +204,7 @@ public class HsHostingAssetEntity implements Stringifyable, RbacObject, Properti .switchOnColumn("type", inCaseOf("DOMAIN_SETUP", then -> { then.toRole(GLOBAL, GUEST).grantPermission(INSERT); - then.toRole(GLOBAL, ADMIN).grantPermission(SELECT); // FIXME: remove + then.toRole(GLOBAL, ADMIN).grantPermission(SELECT); // TODO.spec: replace by a proper solution }) ) @@ -226,8 +226,6 @@ public class HsHostingAssetEntity implements Stringifyable, RbacObject, Properti with.outgoingSubRole("bookingItem", TENANT); with.outgoingSubRole("parentAsset", TENANT); with.incomingSuperRole("alarmContact", ADMIN); - with.incomingSuperRole(GLOBAL, GUEST); // FIXME: remove - with.incomingSuperRole(GLOBAL, ADMIN); // FIXME: remove with.permission(SELECT); }) diff --git a/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7013-hs-hosting-asset-rbac.md b/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7013-hs-hosting-asset-rbac.md index 5dbee08b..37b47e15 100644 --- a/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7013-hs-hosting-asset-rbac.md +++ b/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7013-hs-hosting-asset-rbac.md @@ -99,8 +99,6 @@ role:asset:AGENT ==> role:asset:TENANT role:asset:TENANT ==> role:bookingItem:TENANT role:asset:TENANT ==> role:parentAsset:TENANT role:alarmContact:ADMIN ==> role:asset:TENANT -role:global:GUEST ==> role:asset:TENANT -role:global:ADMIN ==> role:asset:TENANT %% granting permissions to roles role:global:ADMIN ==> perm:asset:INSERT diff --git a/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7013-hs-hosting-asset-rbac.sql b/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7013-hs-hosting-asset-rbac.sql index 9c5cb70d..5b740226 100644 --- a/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7013-hs-hosting-asset-rbac.sql +++ b/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7013-hs-hosting-asset-rbac.sql @@ -75,8 +75,6 @@ begin hsHostingAssetTENANT(NEW), permissions => array['SELECT'], incomingSuperRoles => array[ - globalADMIN(), - globalGUEST(), hsHostingAssetAGENT(NEW), hsOfficeContactADMIN(newAlarmContact)], outgoingSubRoles => array[ diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java index 100d5e95..579257a0 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java @@ -130,6 +130,9 @@ class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanu .containsExactlyInAnyOrder(fromFormatted( initialGrantNames, + // global-admin + "{ grant perm:hs_hosting_asset#fir00:SELECT to role:global#global:ADMIN by system and assume }", // workaround + // owner "{ grant role:hs_hosting_asset#fir00:OWNER to role:hs_booking_item#fir01:ADMIN by system and assume }", "{ grant role:hs_hosting_asset#fir00:OWNER to role:hs_hosting_asset#vm1011:ADMIN by system and assume }", @@ -138,7 +141,6 @@ class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanu // admin "{ grant role:hs_hosting_asset#fir00:ADMIN to role:hs_hosting_asset#fir00:OWNER by system and assume }", "{ grant role:hs_hosting_asset#fir00:ADMIN to role:hs_booking_item#fir01:AGENT by system and assume }", - "{ grant perm:hs_hosting_asset#fir00:INSERT>hs_hosting_asset to role:hs_hosting_asset#fir00:ADMIN by system and assume }", "{ grant perm:hs_hosting_asset#fir00:UPDATE to role:hs_hosting_asset#fir00:ADMIN by system and assume }", // agent @@ -149,7 +151,7 @@ class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanu "{ grant role:hs_booking_item#fir01:TENANT to role:hs_hosting_asset#fir00:TENANT by system and assume }", "{ grant role:hs_hosting_asset#fir00:TENANT to role:hs_hosting_asset#fir00:AGENT by system and assume }", "{ grant role:hs_hosting_asset#vm1011:TENANT to role:hs_hosting_asset#fir00:TENANT by system and assume }", - "{ grant perm:hs_hosting_asset#fir00:SELECT to role:hs_hosting_asset#fir00:TENANT by system and assume }", + "{ grant perm:hs_hosting_asset#fir00:SELECT to role:hs_hosting_asset#fir00:TENANT by system and assume }", // workaround null)); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsDomainDnsSetupHostingAssetValidatorUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsDomainDnsSetupHostingAssetValidatorUnitTest.java index c2ec5c52..671b9452 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsDomainDnsSetupHostingAssetValidatorUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/validators/HsDomainDnsSetupHostingAssetValidatorUnitTest.java @@ -43,6 +43,33 @@ class HsDomainDnsSetupHostingAssetValidatorUnitTest { )); } + @Test + void containsExpectedProperties() { + // when + final var validator = HsHostingAssetEntityValidatorRegistry.forType(DOMAIN_DNS_SETUP); + + // then + assertThat(validator.properties()).map(Map::toString).containsExactlyInAnyOrder( + "{type=integer, propertyName=TTL, min=0, defaultValue=21600}", + "{type=boolean, propertyName=auto-SOA-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-NS-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-MX-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-A-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-AAAA-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-MAILSERVICES-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-AUTOCONFIG-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-AUTODISCOVER-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-DKIM-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-SPF-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-WILDCARD-MX-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-WILDCARD-A-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-WILDCARD-AAAA-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-WILDCARD-DKIM-RR, defaultValue=true}", + "{type=boolean, propertyName=auto-WILDCARD-SPF-RR, defaultValue=true}", + "{type=string[], propertyName=user-RR, elementsOf={type=string, propertyName=user-RR, matchesRegEx=[([a-z0-9\\.-]+|@)\\s+(([1-9][0-9]*[mMhHdDwW]{0,1})+\\s+)*IN\\s+[A-Z]+\\s+[^;].*(;.*)*, ([a-z0-9\\.-]+|@)\\s+IN\\s+(([1-9][0-9]*[mMhHdDwW]{0,1})+\\s+)*[A-Z]+\\s+[^;].*(;.*)*], required=true}}" + ); + } + @Test void preprocessesTakesIdentifierFromParent() { // given @@ -84,33 +111,6 @@ class HsDomainDnsSetupHostingAssetValidatorUnitTest { assertThat(result).isEmpty(); } - @Test - void containsExpectedProperties() { - // when - final var validator = HsHostingAssetEntityValidatorRegistry.forType(DOMAIN_DNS_SETUP); - - // then - assertThat(validator.properties()).map(Map::toString).containsExactlyInAnyOrder( - "{type=integer, propertyName=TTL, min=0, defaultValue=21600}", - "{type=boolean, propertyName=auto-SOA-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-NS-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-MX-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-A-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-AAAA-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-MAILSERVICES-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-AUTOCONFIG-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-AUTODISCOVER-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-DKIM-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-SPF-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-WILDCARD-MX-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-WILDCARD-A-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-WILDCARD-AAAA-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-WILDCARD-DKIM-RR, defaultValue=true}", - "{type=boolean, propertyName=auto-WILDCARD-SPF-RR, defaultValue=true}", - "{type=string[], propertyName=user-RR, elementsOf={type=string, propertyName=user-RR, matchesRegEx=[([a-z0-9\\.-]+|@)\\s+(([1-9][0-9]*[mMhHdDwW]{0,1})+\\s+)*IN\\s+[A-Z]+\\s+[^;].*(;.*)*, ([a-z0-9\\.-]+|@)\\s+IN\\s+(([1-9][0-9]*[mMhHdDwW]{0,1})+\\s+)*[A-Z]+\\s+[^;].*(;.*)*], required=true}}" - ); - } - @Test void validatesReferencedEntities() { // given @@ -131,6 +131,42 @@ class HsDomainDnsSetupHostingAssetValidatorUnitTest { "'DOMAIN_DNS_SETUP:example.org.assignedToAsset' must be null but is set to D-???????-?:null"); } + @Test + void acceptsValidEntity() { + // given + final var givenEntity = validEntityBuilder().build(); + final var validator = HsHostingAssetEntityValidatorRegistry.forType(givenEntity.getType()); + + // when + final var errors = validator.validateEntity(givenEntity); + + // then + assertThat(errors).isEmpty(); + } + + @Test + void recectsInvalidProperties() { + // given + final var mangedServerHostingAssetEntity = validEntityBuilder() + .config(Map.ofEntries( + entry("TTL", "1d30m"), // currently only an integer for seconds is implemented here + entry("user-RR", Array.of( + "@ 1814400 IN 1814400 BAD1 TTL only allowed once", + "www BAD1 Record-Class missing / not enough columns")) + )) + .build(); + final var validator = HsHostingAssetEntityValidatorRegistry.forType(mangedServerHostingAssetEntity.getType()); + + // when + final var result = validator.validateEntity(mangedServerHostingAssetEntity); + + // then + assertThat(result).containsExactlyInAnyOrder( + "'DOMAIN_DNS_SETUP:example.org.config.TTL' is expected to be of type class java.lang.Integer, but is of type 'String'", + "'DOMAIN_DNS_SETUP:example.org.config.user-RR' is expected to match any of [([a-z0-9\\.-]+|@)\\s+(([1-9][0-9]*[mMhHdDwW]{0,1})+\\s+)*IN\\s+[A-Z]+\\s+[^;].*(;.*)*, ([a-z0-9\\.-]+|@)\\s+IN\\s+(([1-9][0-9]*[mMhHdDwW]{0,1})+\\s+)*[A-Z]+\\s+[^;].*(;.*)*] but '@ 1814400 IN 1814400 BAD1 TTL only allowed once' does not match any", + "'DOMAIN_DNS_SETUP:example.org.config.user-RR' is expected to match any of [([a-z0-9\\.-]+|@)\\s+(([1-9][0-9]*[mMhHdDwW]{0,1})+\\s+)*IN\\s+[A-Z]+\\s+[^;].*(;.*)*, ([a-z0-9\\.-]+|@)\\s+IN\\s+(([1-9][0-9]*[mMhHdDwW]{0,1})+\\s+)*[A-Z]+\\s+[^;].*(;.*)*] but 'www BAD1 Record-Class missing / not enough columns' does not match any"); + } + @Test void validStringMatchesRegEx() { assertThat("@ ").matches(RR_REGEX_NAME); @@ -186,20 +222,7 @@ class HsDomainDnsSetupHostingAssetValidatorUnitTest { } @Test - void acceptsValidEntity() { - // given - final var givenEntity = validEntityBuilder().build(); - final var validator = HsHostingAssetEntityValidatorRegistry.forType(givenEntity.getType()); - - // when - final var errors = validator.validateEntity(givenEntity); - - // then - assertThat(errors).isEmpty(); - } - - @Test - void rejectsInvalidEntity() { + void rejectsInvalidZonefile() { // given final var givenEntity = validEntityBuilder().config(Map.ofEntries( entry("user-RR", Array.of(