From 6346dff3ef05de66bef7c67c23056d48bde8376a Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 2 Apr 2024 09:32:16 +0200 Subject: [PATCH] remove superfluous grant to TENANT which was also granted to AGENT --- .../relation/HsOfficeRelationEntity.java | 1 - .../RolesGrantsAndPermissionsGenerator.java | 67 +++++++++++-------- .../5033-hs-office-relation-rbac.md | 1 - .../5033-hs-office-relation-rbac.sql | 1 - ...ficeRelationRepositoryIntegrationTest.java | 1 - 5 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java index 8d6c6fe8..21c82852 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java @@ -118,7 +118,6 @@ public class HsOfficeRelationEntity implements RbacObject, Stringifyable { with.incomingSuperRole("holderPerson", ADMIN); }) .createSubRole(TENANT, (with) -> { - with.incomingSuperRole("holderPerson", ADMIN); with.incomingSuperRole("contact", ADMIN); with.outgoingSubRole("anchorPerson", REFERRER); with.outgoingSubRole("holderPerson", REFERRER); diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RolesGrantsAndPermissionsGenerator.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RolesGrantsAndPermissionsGenerator.java index 484415f2..98f29973 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RolesGrantsAndPermissionsGenerator.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacdef/RolesGrantsAndPermissionsGenerator.java @@ -1,5 +1,6 @@ package net.hostsharing.hsadminng.rbac.rbacdef; +import net.hostsharing.hsadminng.rbac.rbacdef.RbacView.RbacGrantDefinition; import net.hostsharing.hsadminng.rbac.rbacdef.RbacView.RbacPermissionDefinition; import java.util.HashSet; @@ -22,7 +23,7 @@ import static org.apache.commons.lang3.StringUtils.uncapitalize; class RolesGrantsAndPermissionsGenerator { private final RbacView rbacDef; - private final Set rbacGrants = new HashSet<>(); + private final Set rbacGrants = new HashSet<>(); private final String liquibaseTagPrefix; private final String simpleEntityName; private final String simpleEntityVarName; @@ -31,7 +32,7 @@ class RolesGrantsAndPermissionsGenerator { RolesGrantsAndPermissionsGenerator(final RbacView rbacDef, final String liquibaseTagPrefix) { this.rbacDef = rbacDef; this.rbacGrants.addAll(rbacDef.getGrantDefs().stream() - .filter(RbacView.RbacGrantDefinition::isToCreate) + .filter(RbacGrantDefinition::isToCreate) .collect(toSet())); this.liquibaseTagPrefix = liquibaseTagPrefix; @@ -67,13 +68,11 @@ class RolesGrantsAndPermissionsGenerator { NEW ${rawTableName} ) language plpgsql as $$ - - declare """ .replace("${simpleEntityName}", simpleEntityName) .replace("${rawTableName}", rawTableName)); - plPgSql.chopEmptyLines(); + plPgSql.writeLn("declare"); plPgSql.indented(() -> { referencedEntityAliases() .forEach((ea) -> plPgSql.writeLn(entityRefVar(NEW, ea) + " " + ea.getRawTableName() + ";")); @@ -172,6 +171,10 @@ class RolesGrantsAndPermissionsGenerator { .anyMatch(e -> true); } + private boolean hasAnyConditionalGrants() { + return rbacDef.getGrantDefs().stream().anyMatch(RbacGrantDefinition::isConditional); + } + private void generateCreateRolesAndGrantsAfterInsert(final StringWriter plPgSql) { referencedEntityAliases() .forEach((ea) -> { @@ -248,7 +251,7 @@ class RolesGrantsAndPermissionsGenerator { private void updateGrantsDependingOn(final StringWriter plPgSql, final String columnName) { rbacDef.getGrantDefs().stream() - .filter(RbacView.RbacGrantDefinition::isToCreate) + .filter(RbacGrantDefinition::isToCreate) .filter(g -> g.dependsOnColumn(columnName)) .filter(g -> !isInsertPermissionGrant(g)) .forEach(g -> { @@ -259,21 +262,21 @@ class RolesGrantsAndPermissionsGenerator { }); } - private static Boolean isInsertPermissionGrant(final RbacView.RbacGrantDefinition g) { + private static Boolean isInsertPermissionGrant(final RbacGrantDefinition g) { final var isInsertPermissionGrant = ofNullable(g.getPermDef()).map(RbacPermissionDefinition::getPermission).map(p -> p == INSERT).orElse(false); return isInsertPermissionGrant; } - private void generateGrants(final StringWriter plPgSql, final RbacView.RbacGrantDefinition.GrantType grantType) { + private void generateGrants(final StringWriter plPgSql, final RbacGrantDefinition.GrantType grantType) { plPgSql.ensureSingleEmptyLine(); rbacGrants.stream() .filter(g -> g.grantType() == grantType) .map(this::generateGrant) .sorted() - .forEach(text -> plPgSql.writeLn(text)); + .forEach(text -> plPgSql.writeLn(text, with("ref", NEW.name()))); } - private String generateRevoke(RbacView.RbacGrantDefinition grantDef) { + private String generateRevoke(RbacGrantDefinition grantDef) { return switch (grantDef.grantType()) { case ROLE_TO_USER -> throw new IllegalArgumentException("unexpected grant"); case ROLE_TO_ROLE -> "call revokeRoleFromRole(${subRoleRef}, ${superRoleRef});" @@ -285,8 +288,8 @@ class RolesGrantsAndPermissionsGenerator { }; } - private String generateGrant(RbacView.RbacGrantDefinition grantDef) { - return switch (grantDef.grantType()) { + private String generateGrant(RbacGrantDefinition grantDef) { + final var grantSql = switch (grantDef.grantType()) { case ROLE_TO_USER -> throw new IllegalArgumentException("unexpected grant"); case ROLE_TO_ROLE -> "call grantRoleToRole(${subRoleRef}, ${superRoleRef}${assumed});" .replace("${assumed}", grantDef.isAssumed() ? "" : ", unassumed()") @@ -298,6 +301,12 @@ class RolesGrantsAndPermissionsGenerator { .replace("${permRef}", createPerm(NEW, grantDef.getPermDef())) .replace("${superRoleRef}", roleRef(NEW, grantDef.getSuperRoleDef())); }; + if (grantDef.isConditional()) { + return "if " + grantDef.getSqlWhere() + " then\n" + + " " + grantSql + "\n" + + "end if;"; + } + return grantSql; } private String findPerm(final PostgresTriggerReference ref, final RbacPermissionDefinition permDef) { @@ -380,7 +389,7 @@ class RolesGrantsAndPermissionsGenerator { final var grantsToUsers = findGrantsToUserForRole(rbacDef.getRootEntityAlias(), role); if (!grantsToUsers.isEmpty()) { final var arrayElements = grantsToUsers.stream() - .map(RbacView.RbacGrantDefinition::getUserDef) + .map(RbacGrantDefinition::getUserDef) .map(this::toPlPgSqlReference) .toList(); plPgSql.indented(() -> @@ -393,7 +402,7 @@ class RolesGrantsAndPermissionsGenerator { final var permissionGrantsForRole = findPermissionsGrantsForRole(rbacDef.getRootEntityAlias(), role); if (!permissionGrantsForRole.isEmpty()) { final var arrayElements = permissionGrantsForRole.stream() - .map(RbacView.RbacGrantDefinition::getPermDef) + .map(RbacGrantDefinition::getPermDef) .map(RbacPermissionDefinition::getPermission) .map(RbacView.Permission::name) .map(p -> "'" + p + "'") @@ -406,26 +415,30 @@ class RolesGrantsAndPermissionsGenerator { } private void generateIncomingSuperRolesForRole(final StringWriter plPgSql, final RbacView.Role role) { - final var incomingGrants = findIncomingSuperRolesForRole(rbacDef.getRootEntityAlias(), role); - if (!incomingGrants.isEmpty()) { - final var arrayElements = incomingGrants.stream() + final var unconditionalIncomingGrants = findIncomingSuperRolesForRole(rbacDef.getRootEntityAlias(), role).stream() + .filter(g -> !g.isConditional()) + .toList(); + if (!unconditionalIncomingGrants.isEmpty()) { + final var arrayElements = unconditionalIncomingGrants.stream() .map(g -> toPlPgSqlReference(NEW, g.getSuperRoleDef(), g.isAssumed())) .sorted().toList(); plPgSql.indented(() -> plPgSql.writeLn("incomingSuperRoles => array[" + joinArrayElements(arrayElements, 1) + "],\n")); - rbacGrants.removeAll(incomingGrants); + rbacGrants.removeAll(unconditionalIncomingGrants); } } private void generateOutgoingSubRolesForRole(final StringWriter plPgSql, final RbacView.Role role) { - final var outgoingGrants = findOutgoingSuperRolesForRole(rbacDef.getRootEntityAlias(), role); - if (!outgoingGrants.isEmpty()) { - final var arrayElements = outgoingGrants.stream() + final var unconditionalOutgoingGrants = findOutgoingSuperRolesForRole(rbacDef.getRootEntityAlias(), role).stream() + .filter(g -> !g.isConditional()) + .toList(); + if (!unconditionalOutgoingGrants.isEmpty()) { + final var arrayElements = unconditionalOutgoingGrants.stream() .map(g -> toPlPgSqlReference(NEW, g.getSubRoleDef(), g.isAssumed())) .sorted().toList(); plPgSql.indented(() -> plPgSql.writeLn("outgoingSubRoles => array[" + joinArrayElements(arrayElements, 1) + "],\n")); - rbacGrants.removeAll(outgoingGrants); + rbacGrants.removeAll(unconditionalOutgoingGrants); } } @@ -435,7 +448,7 @@ class RolesGrantsAndPermissionsGenerator { : arrayElements.stream().collect(joining(",\n\t", "\n\t", "")); } - private Set findPermissionsGrantsForRole( + private Set findPermissionsGrantsForRole( final RbacView.EntityAlias entityAlias, final RbacView.Role role) { final var roleDef = rbacDef.findRbacRole(entityAlias, role); @@ -444,7 +457,7 @@ class RolesGrantsAndPermissionsGenerator { .collect(toSet()); } - private Set findGrantsToUserForRole( + private Set findGrantsToUserForRole( final RbacView.EntityAlias entityAlias, final RbacView.Role role) { final var roleDef = rbacDef.findRbacRole(entityAlias, role); @@ -453,7 +466,7 @@ class RolesGrantsAndPermissionsGenerator { .collect(toSet()); } - private Set findIncomingSuperRolesForRole( + private Set findIncomingSuperRolesForRole( final RbacView.EntityAlias entityAlias, final RbacView.Role role) { final var roleDef = rbacDef.findRbacRole(entityAlias, role); @@ -462,7 +475,7 @@ class RolesGrantsAndPermissionsGenerator { .collect(toSet()); } - private Set findOutgoingSuperRolesForRole( + private Set findOutgoingSuperRolesForRole( final RbacView.EntityAlias entityAlias, final RbacView.Role role) { final var roleDef = rbacDef.findRbacRole(entityAlias, role); @@ -506,7 +519,7 @@ class RolesGrantsAndPermissionsGenerator { private void generateUpdateTrigger(final StringWriter plPgSql) { generateHeader(plPgSql, "update"); - if ( hasAnyUpdatableAndNullableEntityAliases() ) { + if ( hasAnyUpdatableAndNullableEntityAliases() || hasAnyConditionalGrants() ) { generateSimplifiedUpdateTriggerFunction(plPgSql); } else { generateUpdateTriggerFunction(plPgSql); diff --git a/src/main/resources/db/changelog/5-hs-office/503-relation/5033-hs-office-relation-rbac.md b/src/main/resources/db/changelog/5-hs-office/503-relation/5033-hs-office-relation-rbac.md index 8014cdaf..3f45379b 100644 --- a/src/main/resources/db/changelog/5-hs-office/503-relation/5033-hs-office-relation-rbac.md +++ b/src/main/resources/db/changelog/5-hs-office/503-relation/5033-hs-office-relation-rbac.md @@ -87,7 +87,6 @@ role:anchorPerson:ADMIN ==> role:relation:ADMIN role:relation:ADMIN ==> role:relation:AGENT role:holderPerson:ADMIN ==> role:relation:AGENT role:relation:AGENT ==> role:relation:TENANT -role:holderPerson:ADMIN ==> role:relation:TENANT role:contact:ADMIN ==> role:relation:TENANT role:relation:TENANT ==> role:anchorPerson:REFERRER role:relation:TENANT ==> role:holderPerson:REFERRER diff --git a/src/main/resources/db/changelog/5-hs-office/503-relation/5033-hs-office-relation-rbac.sql b/src/main/resources/db/changelog/5-hs-office/503-relation/5033-hs-office-relation-rbac.sql index ff890a59..ae783ff6 100644 --- a/src/main/resources/db/changelog/5-hs-office/503-relation/5033-hs-office-relation-rbac.sql +++ b/src/main/resources/db/changelog/5-hs-office/503-relation/5033-hs-office-relation-rbac.sql @@ -74,7 +74,6 @@ begin permissions => array['SELECT'], incomingSuperRoles => array[ hsOfficeContactADMIN(newContact), - hsOfficePersonADMIN(newHolderPerson), hsOfficeRelationAGENT(NEW)], outgoingSubRoles => array[ hsOfficeContactREFERRER(newContact), diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java index f474de0c..91a7965e 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java @@ -153,7 +153,6 @@ class HsOfficeRelationRepositoryIntegrationTest extends ContextBasedTestWithClea // REPRESENTATIVE holder person -> (represented) anchor person "{ grant role:hs_office_relation#ErbenBesslerMelBessler-with-REPRESENTATIVE-BesslerBert:TENANT to role:hs_office_contact#fourthcontact:ADMIN by system and assume }", - "{ grant role:hs_office_relation#ErbenBesslerMelBessler-with-REPRESENTATIVE-BesslerBert:TENANT to role:hs_office_person#BesslerBert:ADMIN by system and assume }", null) );