From 2cb9375d03e2a9d6cdaa430ef238ed34ca60c681 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 16 Aug 2022 17:51:51 +0200 Subject: [PATCH] implements revoking role from user at repository level --- .../rbac/rbacgrant/RbacGrantRepository.java | 1 + .../2022-07-28-007-rbac-user-grant.sql | 1 + .../changelog/2022-07-28-008-rbac-views.sql | 4 +- .../RbacGrantControllerAcceptanceTest.java | 67 ++++---- .../RbacGrantRepositoryIntegrationTest.java | 145 +++++++++++++++++- .../java/net/hostsharing/test/JpaAttempt.java | 11 ++ 6 files changed, 189 insertions(+), 40 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantRepository.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantRepository.java index 655f6216..ae589e7e 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantRepository.java @@ -10,4 +10,5 @@ public interface RbacGrantRepository extends Repository findAllGrantsOfUser(final String userName) { diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantRepositoryIntegrationTest.java index 4d83a990..e9ffd11c 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantRepositoryIntegrationTest.java @@ -17,11 +17,13 @@ import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import javax.persistence.EntityManager; +import javax.persistence.PersistenceException; import java.util.List; import java.util.UUID; import static net.hostsharing.test.JpaAttempt.attempt; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assumptions.assumeThat; @DataJpaTest @ComponentScan(basePackageClasses = { RbacGrantRepository.class, Context.class, JpaAttempt.class }) @@ -48,7 +50,7 @@ class RbacGrantRepositoryIntegrationTest { JpaAttempt jpaAttempt; @Nested - class FindAllRbacGrants { + class FindAllGrantsOfUser { @Test @Accepts({ "GRT:L(List)" }) @@ -101,10 +103,9 @@ class RbacGrantRepositoryIntegrationTest { } @Nested - class CreateRbacGrant { + class GrantRoleToUser { @Test - @Accepts({ "GRT:C(Create)" }) public void customerAdmin_canGrantOwnPackageAdminRole_toArbitraryUser() { // given currentUser("admin@aaa.example.com"); @@ -129,16 +130,17 @@ class RbacGrantRepositoryIntegrationTest { } @Test - @Accepts({ "GRT:C(Create)" }) @Transactional(propagation = Propagation.NEVER) public void packageAdmin_canNotGrantPackageOwnerRole() { // given - record Given(RbacUserEntity arbitraryUser, UUID packageOwnerRoleUuid) {} + record Given(RbacUserEntity arbitraryUser, UUID packageOwnerRoleUuid) { + + } final var given = jpaAttempt.transacted(() -> { // to find the uuids of we need to have access rights to these currentUser("admin@aaa.example.com"); return new Given( - createNewUser(), // eigene Transaktion? + createNewUser(), rbacRoleRepository.findByRoleName("package#aaa00.owner").getUuid() ); }).returnedValue(); @@ -162,15 +164,144 @@ class RbacGrantRepositoryIntegrationTest { "ERROR: [403] Access to granted role " + given.packageOwnerRoleUuid + " forbidden for {package#aaa00.admin}"); jpaAttempt.transacted(() -> { + // finally, we use the new user to make sure, no roles were granted currentUser(given.arbitraryUser.getName()); assertThat(rbacGrantRepository.findAll()) .extracting(RbacGrantEntity::toDisplay) .hasSize(0); - // "{ grant assumed role package#aaa00.admin to user aac00@aac.example.com by role customer#aaa.admin }"); }); } } + @Nested + class RevokeRoleFromUser { + + @Test + public void customerAdmin_canRevokeSelfGrantedPackageAdminRole() { + // given + final var grant = create(grant() + .byUser("admin@aaa.example.com").withAssumedRole("customer#aaa.admin") + .grantingRole("package#aaa00.admin").toUser("aac00@aac.example.com")); + + // when + currentUser("admin@aaa.example.com"); + assumedRoles("customer#aaa.admin"); + final var revokeAttempt = attempt(em, () -> { + rbacGrantRepository.delete(grant); + }); + + // then + currentUser("admin@aaa.example.com"); + assumedRoles("customer#aaa.admin"); + assertThat(revokeAttempt.caughtExceptionsRootCause()).isNull(); + assertThat(rbacGrantRepository.findAll()) + .extracting(RbacGrantEntity::getGranteeUserName) + .doesNotContain("aac00@aac.example.com"); + } + + @Test + public void packageAdmin_canRevokeOwnPackageAdminRoleGrantedByAnotherAdminOfThatPackage() { + // given + final var grant = create(grant() + .byUser("admin@aaa.example.com").withAssumedRole("package#aaa00.admin") + .grantingRole("package#aaa00.admin").toUser(createNewUser().getName())); + + // when + currentUser("aaa00@aaa.example.com"); + assumedRoles("package#aaa00.admin"); + final var revokeAttempt = attempt(em, () -> { + rbacGrantRepository.delete(grant); + }); + + // then + assertThat(revokeAttempt.caughtExceptionsRootCause()).isNull(); + currentUser("admin@aaa.example.com"); + assumedRoles("customer#aaa.admin"); + assertThat(rbacGrantRepository.findAll()) + .extracting(RbacGrantEntity::getGranteeUserName) + .doesNotContain("aac00@aac.example.com"); + } + + @Test + public void packageAdmin_canNotRevokeOwnPackageAdminRoleGrantedByOwnerRoleOfThatPackage() { + // given + final var grant = create(grant() + .byUser("admin@aaa.example.com").withAssumedRole("package#aaa00.owner") + .grantingRole("package#aaa00.admin").toUser("aac00@aac.example.com")); + final var grantedByRole = rbacRoleRepository.findByRoleName("package#aaa00.owner"); + + // when + currentUser("aaa00@aaa.example.com"); + assumedRoles("package#aaa00.admin"); + final var revokeAttempt = attempt(em, () -> { + rbacGrantRepository.delete(grant); + }); + + // then + revokeAttempt.assertExceptionWithRootCauseMessage( + PersistenceException.class, + "ERROR: [403] Revoking role created by %s is forbidden for {package#aaa00.admin}." .formatted( + grantedByRole.getUuid() + )); + } + + private RbacGrantEntity create(GrantBuilder with) { + currentUser(with.byUserName); + assumedRoles(with.assumedRole); + final var givenArbitraryUserUuid = rbacUserRepository.findUuidByName(with.granteeUserName); + final var givenOwnPackageRoleUuid = rbacRoleRepository.findByRoleName(with.grantedRole).getUuid(); + + final var grant = RbacGrantEntity.builder() + .granteeUserUuid(givenArbitraryUserUuid).grantedRoleUuid(givenOwnPackageRoleUuid) + .assumed(true) + .build(); + final var grantAttempt = attempt(em, () -> + rbacGrantRepository.save(grant) + ); + + assumeThat(grantAttempt.caughtException()).isNull(); + assumeThat(rbacGrantRepository.findAll()) + .extracting(RbacGrantEntity::toDisplay) + .contains("{ grant assumed role %s to user %s by role %s }" .formatted( + with.grantedRole, with.granteeUserName, with.assumedRole + )); + + return grant; + } + + private GrantBuilder grant() { + return new GrantBuilder(); + } + + static class GrantBuilder { + + String byUserName; + String assumedRole = ""; + String grantedRole; + String granteeUserName; + + GrantBuilder byUser(final String userName) { + byUserName = userName; + return this; + } + + GrantBuilder withAssumedRole(final String assumedRole) { + this.assumedRole = assumedRole != null ? assumedRole : ""; + return this; + } + + GrantBuilder grantingRole(final String grantingRole) { + this.grantedRole = grantingRole; + return this; + } + + GrantBuilder toUser(final String toUser) { + this.granteeUserName = toUser; + return this; + } + } + } + private RbacUserEntity createNewUser() { return rbacUserRepository.create( new RbacUserEntity(null, "test-user-" + System.currentTimeMillis() + "@example.com")); diff --git a/src/test/java/net/hostsharing/test/JpaAttempt.java b/src/test/java/net/hostsharing/test/JpaAttempt.java index 4d4499d7..eafa8fc1 100644 --- a/src/test/java/net/hostsharing/test/JpaAttempt.java +++ b/src/test/java/net/hostsharing/test/JpaAttempt.java @@ -4,6 +4,7 @@ import junit.framework.AssertionFailedError; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.NestedExceptionUtils; import org.springframework.stereotype.Service; +import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.support.TransactionTemplate; import javax.persistence.EntityManager; @@ -11,6 +12,7 @@ import java.util.Optional; import java.util.function.Supplier; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; /** * Wraps the 'when' part of a DataJpaTest to improve readability of tests. @@ -62,6 +64,7 @@ public class JpaAttempt { public JpaResult transacted(final Runnable code) { try { + transactionTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); transactionTemplate.execute(transactionStatus -> { code.run(); return null; @@ -115,6 +118,10 @@ public class JpaAttempt { throw new AssertionFailedError("expected " + expectedExceptionClass + " but got " + exception); } + public Throwable caughtExceptionsRootCause() { + return exception == null ? null : NestedExceptionUtils.getRootCause(exception); + } + public void assertExceptionWithRootCauseMessage( final Class expectedExceptionClass, final String... expectedRootCauseMessages) { @@ -125,6 +132,10 @@ public class JpaAttempt { } } + public void assertSuccessful() { + assertThat(exception).isNull();; + } + private String firstRootCauseMessageLineOf(final RuntimeException exception) { final var rootCause = NestedExceptionUtils.getRootCause(exception); return Optional.ofNullable(rootCause)