From de4e048e92b7d779c4a9eedc458cf1933dab5f9b Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 2 Apr 2024 11:41:33 +0200 Subject: [PATCH] fixing issues from code-review --- README.md | 2 +- doc/rbac.md | 22 +++++++++---------- .../HsOfficeCoopAssetsTransactionEntity.java | 1 - .../HsOfficeCoopSharesTransactionEntity.java | 1 - .../rbac/rbacrole/RbacRoleController.java | 2 +- ...cRoleRvEntity.java => RbacRoleEntity.java} | 2 +- .../rbac/rbacrole/RbacRoleRepository.java | 6 ++--- .../db/changelog/080-rbac-global.sql | 2 +- .../test/ContextBasedTestWithCleanup.java | 4 ++-- .../RbacGrantControllerAcceptanceTest.java | 20 ++++++++--------- .../RbacRoleRepositoryIntegrationTest.java | 12 +++++----- .../hsadminng/rbac/rbacrole/TestRbacRole.java | 10 ++++----- 12 files changed, 41 insertions(+), 43 deletions(-) rename src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/{RbacRoleRvEntity.java => RbacRoleEntity.java} (96%) diff --git a/README.md b/README.md index 4301d8fe..23209dd2 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ If you have at least Docker and the Java JDK installed in appropriate versions a # the following command should return a JSON array with just all packages visible for the admin of the customer yyy: curl \ - -H 'current-user: superuser-alex@hostsharing.net' -H 'assumed-roles: test_customer#yyy:admin' \ + -H 'current-user: superuser-alex@hostsharing.net' -H 'assumed-roles: test_customer#yyy:ADMIN' \ http://localhost:8080/api/test/packages # add a new customer diff --git a/doc/rbac.md b/doc/rbac.md index 3cf58e25..9e562148 100644 --- a/doc/rbac.md +++ b/doc/rbac.md @@ -206,7 +206,7 @@ and the *role-stereotype* describes a role relative to a referenced business-obj #### owner The owner-role is granted to the subject which created the business object. -E.g. for a new *customer* it would be granted to 'administrators' and for a new *package* to the 'customer#...:admin'. +E.g. for a new *customer* it would be granted to 'administrators' and for a new *package* to the 'customer#...:ADMIN'. Whoever has the owner-role assigned can do everything with the related business-object, including deleting (or deactivating) it. @@ -470,14 +470,14 @@ together { permCustomerXyzSELECT--> boCustXyz } -entity "Role customer#xyz:tenant" as roleCustXyzTenant +entity "Role customer#xyz:TENANT" as roleCustXyzTenant roleCustXyzTenant --> permCustomerXyzSELECT -entity "Role customer#xyz:admin" as roleCustXyzAdmin +entity "Role customer#xyz:ADMIN" as roleCustXyzAdmin roleCustXyzAdmin --> roleCustXyzTenant roleCustXyzAdmin --> permCustomerXyzINSERT:package -entity "Role customer#xyz:owner" as roleCustXyzOwner +entity "Role customer#xyz:OWNER" as roleCustXyzOwner roleCustXyzOwner ..> roleCustXyzAdmin roleCustXyzOwner --> permCustomerXyzDELETE @@ -493,7 +493,7 @@ actorHostmaster --> roleAdmins ``` As you can see, there something special: -From the 'Role customer#xyz:owner' to the 'Role customer#xyz:admin' there is a dashed line, whereas all other lines are solid lines. +From the 'Role customer#xyz:OWNER' to the 'Role customer#xyz:admin' there is a dashed line, whereas all other lines are solid lines. Solid lines means, that one role is granted to another and automatically assumed in all queries to the restricted views. The dashed line means that one role is granted to another but not automatically assumed in queries to the restricted views. @@ -541,15 +541,15 @@ together { } package { - entity "Role customer#xyz:tenant" as roleCustXyzTenant - entity "Role customer#xyz:admin" as roleCustXyzAdmin - entity "Role customer#xyz:owner" as roleCustXyzOwner + entity "Role customer#xyz:TENANT" as roleCustXyzTenant + entity "Role customer#xyz:ADMIN" as roleCustXyzAdmin + entity "Role customer#xyz:OWNER" as roleCustXyzOwner } package { - entity "Role package#xyz00:owner" as rolePacXyz00Owner - entity "Role package#xyz00:admin" as rolePacXyz00Admin - entity "Role package#xyz00:tenant" as rolePacXyz00Tenant + entity "Role package#xyz00:OWNER" as rolePacXyz00Owner + entity "Role package#xyz00:ADMIN" as rolePacXyz00Admin + entity "Role package#xyz00:TENANT" as rolePacXyz00Tenant } rolePacXyz00Tenant --> permPacXyz00SELECT diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java index 7b08065e..03d3ae49 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java @@ -24,7 +24,6 @@ import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; import jakarta.persistence.Table; import java.io.IOException; -import java.io.IOException; import java.math.BigDecimal; import java.time.LocalDate; import java.util.Optional; diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java index 64d193f6..52222582 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java @@ -23,7 +23,6 @@ import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; import jakarta.persistence.Table; import java.io.IOException; -import java.io.IOException; import java.time.LocalDate; import java.util.UUID; diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleController.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleController.java index cf528361..0405fee2 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleController.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleController.java @@ -31,7 +31,7 @@ public class RbacRoleController implements RbacRolesApi { context.define(currentUser, assumedRoles); - final List result = rbacRoleRepository.findAll(); + final List result = rbacRoleRepository.findAll(); return ResponseEntity.ok(mapper.mapList(result, RbacRoleResource.class)); } diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRvEntity.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleEntity.java similarity index 96% rename from src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRvEntity.java rename to src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleEntity.java index d174182a..fa21785a 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRvEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleEntity.java @@ -15,7 +15,7 @@ import java.util.UUID; @Immutable @NoArgsConstructor @AllArgsConstructor -public class RbacRoleRvEntity { +public class RbacRoleEntity { @Id @GeneratedValue diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepository.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepository.java index 2d9d005b..94633d7c 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepository.java @@ -5,7 +5,7 @@ import org.springframework.data.repository.Repository; import java.util.List; import java.util.UUID; -public interface RbacRoleRepository extends Repository { +public interface RbacRoleRepository extends Repository { /** * @return the number of persistent RbacRoleEntity instances, mostly for testing purposes. @@ -15,7 +15,7 @@ public interface RbacRoleRepository extends Repository { /** * @return all persistent RbacRoleEntity instances, assigned to the current subject (user or assumed roles) */ - List findAll(); + List findAll(); - RbacRoleRvEntity findByRoleName(String roleName); + RbacRoleEntity findByRoleName(String roleName); } diff --git a/src/main/resources/db/changelog/080-rbac-global.sql b/src/main/resources/db/changelog/080-rbac-global.sql index 7577e88c..3078922f 100644 --- a/src/main/resources/db/changelog/080-rbac-global.sql +++ b/src/main/resources/db/changelog/080-rbac-global.sql @@ -139,7 +139,7 @@ select 'global', (select uuid from RbacObject where objectTable = 'global'), 'GU $$; begin transaction; - call defineContext('creating role:global#loba:guest', null, null, null); + call defineContext('creating role:global#globa:guest', null, null, null); select createRole(globalGuest()); commit; --// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/test/ContextBasedTestWithCleanup.java b/src/test/java/net/hostsharing/hsadminng/hs/office/test/ContextBasedTestWithCleanup.java index 1866f483..722fd87e 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/test/ContextBasedTestWithCleanup.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/test/ContextBasedTestWithCleanup.java @@ -6,7 +6,7 @@ import net.hostsharing.hsadminng.rbac.rbacgrant.RbacGrantEntity; import net.hostsharing.hsadminng.rbac.rbacgrant.RbacGrantRepository; import net.hostsharing.hsadminng.rbac.rbacgrant.RbacGrantsDiagramService; import net.hostsharing.hsadminng.rbac.rbacobject.RbacObject; -import net.hostsharing.hsadminng.rbac.rbacrole.RbacRoleRvEntity; +import net.hostsharing.hsadminng.rbac.rbacrole.RbacRoleEntity; import net.hostsharing.hsadminng.rbac.rbacrole.RbacRoleRepository; import net.hostsharing.test.JpaAttempt; import org.jetbrains.annotations.NotNull; @@ -255,7 +255,7 @@ public abstract class ContextBasedTestWithCleanup extends ContextBasedTest { return jpaAttempt.transacted(() -> { context.define("superuser-alex@hostsharing.net", null); return rbacRoleRepo.findAll().stream() - .map(RbacRoleRvEntity::getRoleName) + .map(RbacRoleEntity::getRoleName) .collect(toSet()); }).assertSuccessful().returnedValue(); } 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 8bcfd28e..15738504 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantControllerAcceptanceTest.java @@ -5,7 +5,7 @@ import io.restassured.http.ContentType; import io.restassured.response.ValidatableResponse; import net.hostsharing.hsadminng.HsadminNgApplication; import net.hostsharing.hsadminng.context.ContextBasedTest; -import net.hostsharing.hsadminng.rbac.rbacrole.RbacRoleRvEntity; +import net.hostsharing.hsadminng.rbac.rbacrole.RbacRoleEntity; import net.hostsharing.hsadminng.rbac.rbacrole.RbacRoleRepository; import net.hostsharing.hsadminng.rbac.rbacuser.RbacUserEntity; import net.hostsharing.hsadminng.rbac.rbacuser.RbacUserRepository; @@ -361,11 +361,11 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { this(currentUser, ""); } - GrantFixture grantsRole(final RbacRoleRvEntity givenOwnPackageAdminRole) { + GrantFixture grantsRole(final RbacRoleEntity givenOwnPackageAdminRole) { return new GrantFixture(givenOwnPackageAdminRole); } - RevokeFixture revokesRole(final RbacRoleRvEntity givenOwnPackageAdminRole) { + RevokeFixture revokesRole(final RbacRoleEntity givenOwnPackageAdminRole) { return new RevokeFixture(givenOwnPackageAdminRole); } @@ -376,11 +376,11 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { class GrantFixture { private Subject grantingSubject = Subject.this; - private final RbacRoleRvEntity grantedRole; + private final RbacRoleEntity grantedRole; private boolean assumed; private RbacUserEntity granteeUser; - public GrantFixture(final RbacRoleRvEntity roleToGrant) { + public GrantFixture(final RbacRoleEntity roleToGrant) { this.grantedRole = roleToGrant; } @@ -417,11 +417,11 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { class RevokeFixture { private Subject currentSubject = Subject.this; - private final RbacRoleRvEntity grantedRole; + private final RbacRoleEntity grantedRole; private boolean assumed; private RbacUserEntity granteeUser; - public RevokeFixture(final RbacRoleRvEntity roleToGrant) { + public RevokeFixture(final RbacRoleEntity roleToGrant) { this.grantedRole = roleToGrant; } @@ -455,9 +455,9 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { private class GetGrantByIdFixture { private Subject currentSubject = Subject.this; - private RbacRoleRvEntity grantedRole; + private RbacRoleEntity grantedRole; - GetGrantByIdFixture forGrantedRole(final RbacRoleRvEntity grantedRole) { + GetGrantByIdFixture forGrantedRole(final RbacRoleEntity grantedRole) { this.grantedRole = grantedRole; return this; } @@ -507,7 +507,7 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { }).assertNotNull().returnedValue(); } - RbacRoleRvEntity getRbacRoleByName(final String roleName) { + RbacRoleEntity getRbacRoleByName(final String roleName) { return jpaAttempt.transacted(() -> { context("superuser-alex@hostsharing.net", null); return rbacRoleRepository.findByRoleName(roleName); diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepositoryIntegrationTest.java index d94382d7..4d873fa6 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepositoryIntegrationTest.java @@ -175,21 +175,21 @@ class RbacRoleRepositoryIntegrationTest { } } - void exactlyTheseRbacRolesAreReturned(final List actualResult, final String... expectedRoleNames) { + void exactlyTheseRbacRolesAreReturned(final List actualResult, final String... expectedRoleNames) { assertThat(actualResult) - .extracting(RbacRoleRvEntity::getRoleName) + .extracting(RbacRoleEntity::getRoleName) .containsExactlyInAnyOrder(expectedRoleNames); } - void allTheseRbacRolesAreReturned(final List actualResult, final String... expectedRoleNames) { + void allTheseRbacRolesAreReturned(final List actualResult, final String... expectedRoleNames) { assertThat(actualResult) - .extracting(RbacRoleRvEntity::getRoleName) + .extracting(RbacRoleEntity::getRoleName) .contains(expectedRoleNames); } - void noneOfTheseRbacRolesIsReturned(final List actualResult, final String... unexpectedRoleNames) { + void noneOfTheseRbacRolesIsReturned(final List actualResult, final String... unexpectedRoleNames) { assertThat(actualResult) - .extracting(RbacRoleRvEntity::getRoleName) + .extracting(RbacRoleEntity::getRoleName) .doesNotContain(unexpectedRoleNames); } diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/TestRbacRole.java b/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/TestRbacRole.java index 9eb0f3c7..73e30a1b 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/TestRbacRole.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/TestRbacRole.java @@ -4,11 +4,11 @@ import static java.util.UUID.randomUUID; public class TestRbacRole { - public static final RbacRoleRvEntity hostmasterRole = rbacRole("global", "global", RbacRoleType.ADMIN); - static final RbacRoleRvEntity customerXxxOwner = rbacRole("test_customer", "xxx", RbacRoleType.OWNER); - static final RbacRoleRvEntity customerXxxAdmin = rbacRole("test_customer", "xxx", RbacRoleType.ADMIN); + public static final RbacRoleEntity hostmasterRole = rbacRole("global", "global", RbacRoleType.ADMIN); + static final RbacRoleEntity customerXxxOwner = rbacRole("test_customer", "xxx", RbacRoleType.OWNER); + static final RbacRoleEntity customerXxxAdmin = rbacRole("test_customer", "xxx", RbacRoleType.ADMIN); - static public RbacRoleRvEntity rbacRole(final String objectTable, final String objectIdName, final RbacRoleType roleType) { - return new RbacRoleRvEntity(randomUUID(), randomUUID(), objectTable, objectIdName, roleType, objectTable+'#'+objectIdName+':'+roleType); + static public RbacRoleEntity rbacRole(final String objectTable, final String objectIdName, final RbacRoleType roleType) { + return new RbacRoleEntity(randomUUID(), randomUUID(), objectTable, objectIdName, roleType, objectTable+'#'+objectIdName+':'+roleType); } }