diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserController.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserController.java index b2f74134..0a1dc775 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserController.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserController.java @@ -45,6 +45,20 @@ public class RbacUserController implements RbacusersApi { return ResponseEntity.created(uri).body(map(saved, RbacUserResource.class)); } + @Override + @Transactional + public ResponseEntity deleteUserByUuid( + final String currentUser, + final String assumedRoles, + final UUID userUuid + ) { + context.define(currentUser, assumedRoles); + + rbacUserRepository.deleteByUuid(userUuid); + + return ResponseEntity.noContent().build(); + } + @Override @Transactional(readOnly = true) public ResponseEntity getUserById( diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepository.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepository.java index 9c107f8c..bfe11a19 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepository.java @@ -41,4 +41,6 @@ public interface RbacUserRepository extends Repository { insert(rbacUserEntity); return rbacUserEntity; } + + void deleteByUuid(UUID userUuid); } diff --git a/src/main/resources/api-definition.yaml b/src/main/resources/api-definition.yaml index 90373ea7..46988e83 100644 --- a/src/main/resources/api-definition.yaml +++ b/src/main/resources/api-definition.yaml @@ -17,7 +17,7 @@ paths: $ref: "./api-definition/rbac-users-with-id-permissions.yaml" /api/rbac-users/{userUuid}: - $ref: "./api-definition/rbac-users-with-id.yaml" + $ref: "./api-definition/rbac-users-with-uuid.yaml" /api/rbac-roles: $ref: "./api-definition/rbac-roles.yaml" diff --git a/src/main/resources/api-definition/rbac-users-with-id.yaml b/src/main/resources/api-definition/rbac-users-with-uuid.yaml similarity index 52% rename from src/main/resources/api-definition/rbac-users-with-id.yaml rename to src/main/resources/api-definition/rbac-users-with-uuid.yaml index 39f8ec27..da1a02a1 100644 --- a/src/main/resources/api-definition/rbac-users-with-id.yaml +++ b/src/main/resources/api-definition/rbac-users-with-uuid.yaml @@ -24,3 +24,28 @@ get: $ref: './api-definition/error-responses.yaml#/components/responses/Unauthorized' "403": $ref: './api-definition/error-responses.yaml#/components/responses/Forbidden' + + +delete: + tags: + - rbacusers + operationId: deleteUserByUuid + parameters: + - $ref: './api-definition/auth.yaml#/components/parameters/currentUser' + - $ref: './api-definition/auth.yaml#/components/parameters/assumedRoles' + - name: userUuid + in: path + required: true + schema: + type: string + format: uuid + description: UUID of the user to delete. + responses: + "204": + description: No Content + "401": + $ref: './api-definition/error-responses.yaml#/components/responses/Unauthorized' + "403": + $ref: './api-definition/error-responses.yaml#/components/responses/Forbidden' + "404": + $ref: './api-definition/error-responses.yaml#/components/responses/NotFound' diff --git a/src/main/resources/db/changelog/055-rbac-views.sql b/src/main/resources/db/changelog/055-rbac-views.sql index 5eb1aa26..bd309b67 100644 --- a/src/main/resources/db/changelog/055-rbac-views.sql +++ b/src/main/resources/db/changelog/055-rbac-views.sql @@ -209,7 +209,9 @@ create or replace view RbacUser_rv as union select users.* from RbacUser as users - where cardinality(assumedRoles()) = 0 and currentUserUuid() = users.uuid + where cardinality(assumedRoles()) = 0 and + (currentUserUuid() = users.uuid or hasGlobalRoleGranted(currentUserUuid())) + ) as unordered -- @formatter:on order by unordered.name; @@ -250,7 +252,35 @@ create trigger insertRbacUser_Trigger on RbacUser_rv for each row execute function insertRbacUser(); +--// +-- ============================================================================ +--changeset rbac-views-USER-RV-DELETE-TRIGGER:1 endDelimiter:--// +-- ---------------------------------------------------------------------------- + +/** + Instead of delete trigger function for RbacUser_RV. + */ +create or replace function deleteRbacUser() + returns trigger + language plpgsql as $$ +begin + if currentUserUuid() = old.uuid or hasGlobalRoleGranted(currentUserUuid()) then + delete from RbacUser where uuid = old.uuid; + return old; + end if; + raise exception '[403] User % not allowed to delete user uuid %', currentUser(), old.uuid; +end; $$; + +/* + Creates an instead of delete trigger for the RbacUser_rv view. + */ +create trigger deleteRbacUser_Trigger + instead of delete + on RbacUser_rv + for each row +execute function deleteRbacUser(); +--/ -- ============================================================================ --changeset rbac-views-OWN-GRANTED-PERMISSIONS-VIEW:1 endDelimiter:--// diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserControllerAcceptanceTest.java index c1a6d28d..13e4edf6 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserControllerAcceptanceTest.java @@ -397,10 +397,92 @@ class RbacUserControllerAcceptanceTest { } } + @Nested + class DeleteRbacUser { + + @Test + @Accepts({ "USR:D(Create)" }) + void anybody_canDeleteTheirOwnUser() { + + // given + final var givenUser = givenANewUser(); + + // @formatter:off + final var location = RestAssured + .given() + .header("current-user", givenUser.getName()) + .port(port) + .when() + .delete("http://localhost/api/rbac-users/" + givenUser.getUuid()) + .then().log().all().assertThat() + .statusCode(204); + // @formatter:on + + // finally, the user is actually deleted + assertThat(rbacUserRepository.findByName(givenUser.getName())).isNull(); + } + + @Test + @Accepts({ "USR:D(Create)", "USR:X(Access Control)" }) + void customerAdmin_canNotDeleteOtherUser() { + + // given + final var givenUser = givenANewUser(); + + // @formatter:off + final var location = RestAssured + .given() + .header("current-user", "customer-admin@xxx.example.com") + .port(port) + .when() + .delete("http://localhost/api/rbac-users/" + givenUser.getUuid()) + .then().log().all().assertThat() + // that user cannot even see other users, thus the system won't even try to delete + .statusCode(204); + // @formatter:on + + // finally, the user is still there + assertThat(rbacUserRepository.findByName(givenUser.getName())).isNotNull(); + } + + @Test + @Accepts({ "USR:D(Create)", "USR:X(Access Control)" }) + void globalAdmin_canDeleteArbitraryUser() { + + // given + final var givenUser = givenANewUser(); + + // @formatter:off + final var location = RestAssured + .given() + .header("current-user", "mike@example.org") + .port(port) + .when() + .delete("http://localhost/api/rbac-users/" + givenUser.getUuid()) + .then().log().all().assertThat() + .statusCode(204); + // @formatter:on + + // finally, the user is actually deleted + assertThat(rbacUserRepository.findByName(givenUser.getName())).isNull(); + } + } + RbacUserEntity findRbacUserByName(final String userName) { return jpaAttempt.transacted(() -> { context.define("mike@example.org"); return rbacUserRepository.findByName(userName); }).returnedValue(); } + + RbacUserEntity givenANewUser() { + final var givenUserName = "test-user-" + System.currentTimeMillis() + "@example.com"; + final var givenUser = jpaAttempt.transacted(() -> { + context.define(null); + return rbacUserRepository.create(new RbacUserEntity(UUID.randomUUID(), givenUserName)); + }).assumeSuccessful().returnedValue(); + assertThat(rbacUserRepository.findByName(givenUser.getName())).isNotNull(); + return givenUser; + } + } diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepositoryIntegrationTest.java index ac912f77..4d701240 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepositoryIntegrationTest.java @@ -43,28 +43,9 @@ class RbacUserRepositoryIntegrationTest extends ContextBasedTest { @Nested class CreateUser { - @Test - public void anyoneCanCreateTheirOwnUser() { - // given - final var givenNewUserName = "test-user-" + System.currentTimeMillis() + "@example.com"; - context(null); - - // when - final var result = rbacUserRepository.create( - new RbacUserEntity(null, givenNewUserName)); - - // then the persisted user is returned - assertThat(result).isNotNull().extracting(RbacUserEntity::getName).isEqualTo(givenNewUserName); - - // and the new user entity can be fetched by the user itself - context(givenNewUserName); - assertThat(em.find(RbacUserEntity.class, result.getUuid())) - .isNotNull().extracting(RbacUserEntity::getName).isEqualTo(givenNewUserName); - } - @Test @Transactional(propagation = Propagation.NEVER) - void anyoneCanCreateTheirOwnUser_committed() { + void anyoneCanCreateTheirOwnUser() { // given: final var givenUuid = UUID.randomUUID(); @@ -72,7 +53,7 @@ class RbacUserRepositoryIntegrationTest extends ContextBasedTest { // when: final var result = jpaAttempt.transacted(() -> { - context("customer-admin@xxx.example.com"); + context(null); return rbacUserRepository.create(new RbacUserEntity(givenUuid, newUserName)); }); @@ -80,11 +61,36 @@ class RbacUserRepositoryIntegrationTest extends ContextBasedTest { assertThat(result.wasSuccessful()).isTrue(); assertThat(result.returnedValue()).isNotNull() .extracting(RbacUserEntity::getUuid).isEqualTo(givenUuid); - jpaAttempt.transacted(() -> { - context(newUserName); - assertThat(em.find(RbacUserEntity.class, givenUuid)) - .isNotNull().extracting(RbacUserEntity::getName).isEqualTo(newUserName); + assertThat(rbacUserRepository.findByName(result.returnedValue().getName())).isNotNull(); + // jpaAttempt.transacted(() -> { + // context(givenUser.getName()); + // assertThat(em.find(RbacUserEntity.class, givenUser.getUuid())) + // .isNotNull().extracting(RbacUserEntity::getName).isEqualTo(givenUser.getName()); + // }).assertSuccessful(); + } + } + + @Nested + class DeleteUser { + + @Test + @Transactional(propagation = Propagation.NEVER) + public void anyoneCanDeleteTheirOwnUser() { + // given + final RbacUserEntity givenUser = givenANewUser(); + + // when + final var result = jpaAttempt.transacted(() -> { + context(givenUser.getName()); + rbacUserRepository.deleteByUuid(givenUser.getUuid()); }); + + // then the user is deleted + result.assertSuccessful(); + assertThat(rbacUserRepository.findByName(givenUser.getName())).isNull(); + // jpaAttempt.transacted(() -> { + // assertThat(rbacUserRepository.findByName(givenUser.getName())).isNull(); + // }).assertSuccessful(); } } @@ -392,6 +398,16 @@ class RbacUserRepositoryIntegrationTest extends ContextBasedTest { return rbacUserRepository.findByName(userName).getUuid(); } + RbacUserEntity givenANewUser() { + final var givenUserName = "test-user-" + System.currentTimeMillis() + "@example.com"; + final var givenUser = jpaAttempt.transacted(() -> { + context(null); + return rbacUserRepository.create(new RbacUserEntity(UUID.randomUUID(), givenUserName)); + }).assumeSuccessful().returnedValue(); + assertThat(rbacUserRepository.findByName(givenUser.getName())).isNotNull(); + return givenUser; + } + void exactlyTheseRbacUsersAreReturned(final List actualResult, final String... expectedUserNames) { assertThat(actualResult) .extracting(RbacUserEntity::getName) diff --git a/src/test/java/net/hostsharing/test/JpaAttempt.java b/src/test/java/net/hostsharing/test/JpaAttempt.java index 28dc7130..85202a4b 100644 --- a/src/test/java/net/hostsharing/test/JpaAttempt.java +++ b/src/test/java/net/hostsharing/test/JpaAttempt.java @@ -11,6 +11,7 @@ import java.util.Optional; import java.util.function.Supplier; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assumptions.assumeThat; /** * Wraps the 'when' part of a DataJpaTest to improve readability of tests. @@ -53,6 +54,7 @@ public class JpaAttempt { public JpaResult transacted(final Supplier code) { try { + transactionTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); return JpaResult.forValue( transactionTemplate.execute(transactionStatus -> code.get())); } catch (final RuntimeException exc) { @@ -131,11 +133,19 @@ public class JpaAttempt { } public JpaResult assumeSuccessful() { - assertThat(exception).isNull(); - ; + assumeThat(exception).as(getSensibleMessage(exception)).isNull(); return this; } + public JpaResult assertSuccessful() { + assertThat(exception).as(getSensibleMessage(exception)).isNull(); + return this; + } + + private String getSensibleMessage(final RuntimeException exception) { + return exception != null ? NestedExceptionUtils.getRootCause(exception).getMessage() : null; + } + private String firstRootCauseMessageLineOf(final RuntimeException exception) { final var rootCause = NestedExceptionUtils.getRootCause(exception); return Optional.ofNullable(rootCause)