From 8af93603d55924f4e036724aa0fc79caf1a275c4 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 30 Aug 2022 13:08:29 +0200 Subject: [PATCH] improve test code coverage and generic array return from native queries --- .../config/PostgreSQL95CustomDialect.java | 16 ---------- .../config/PostgresCustomDialect.java | 15 ++++++++++ .../hsadminng/context/Context.java | 24 ++++++++++----- src/main/resources/application.yml | 2 +- .../db/changelog/054-rbac-context.sql | 16 ++++++---- .../context/ContextIntegrationTests.java | 30 ++++++++++++------- .../RbacGrantControllerAcceptanceTest.java | 18 +++++------ .../RbacRoleRepositoryIntegrationTest.java | 2 +- src/test/resources/application.yml | 2 +- 9 files changed, 71 insertions(+), 54 deletions(-) delete mode 100644 src/main/java/net/hostsharing/hsadminng/config/PostgreSQL95CustomDialect.java create mode 100644 src/main/java/net/hostsharing/hsadminng/config/PostgresCustomDialect.java diff --git a/src/main/java/net/hostsharing/hsadminng/config/PostgreSQL95CustomDialect.java b/src/main/java/net/hostsharing/hsadminng/config/PostgreSQL95CustomDialect.java deleted file mode 100644 index 4f4d73aa..00000000 --- a/src/main/java/net/hostsharing/hsadminng/config/PostgreSQL95CustomDialect.java +++ /dev/null @@ -1,16 +0,0 @@ -package net.hostsharing.hsadminng.config; - -import com.vladmihalcea.hibernate.type.array.UUIDArrayType; -import org.hibernate.dialect.PostgreSQL95Dialect; - -import java.sql.Types; - -@SuppressWarnings("unused") // configured in application.yml -public class PostgreSQL95CustomDialect extends PostgreSQL95Dialect { - - public PostgreSQL95CustomDialect() { - this.registerHibernateType(Types.OTHER, "pg-uuid"); - this.registerHibernateType(Types.ARRAY, UUIDArrayType.class.getName()); - } - -} diff --git a/src/main/java/net/hostsharing/hsadminng/config/PostgresCustomDialect.java b/src/main/java/net/hostsharing/hsadminng/config/PostgresCustomDialect.java new file mode 100644 index 00000000..243de957 --- /dev/null +++ b/src/main/java/net/hostsharing/hsadminng/config/PostgresCustomDialect.java @@ -0,0 +1,15 @@ +package net.hostsharing.hsadminng.config; + +import org.hibernate.dialect.PostgreSQL95Dialect; + +import java.sql.Types; + +@SuppressWarnings("unused") // configured in application.yml +public class PostgresCustomDialect extends PostgreSQL95Dialect { + + public PostgresCustomDialect() { + this.registerHibernateType(Types.OTHER, "pg-uuid"); + this.registerHibernateType(Types.ARRAY, "array"); + } + +} diff --git a/src/main/java/net/hostsharing/hsadminng/context/Context.java b/src/main/java/net/hostsharing/hsadminng/context/Context.java index 374967dd..40ffada5 100644 --- a/src/main/java/net/hostsharing/hsadminng/context/Context.java +++ b/src/main/java/net/hostsharing/hsadminng/context/Context.java @@ -1,5 +1,7 @@ package net.hostsharing.hsadminng.context; +import com.vladmihalcea.hibernate.type.array.StringArrayType; +import com.vladmihalcea.hibernate.type.array.UUIDArrayType; import lombok.AllArgsConstructor; import lombok.SneakyThrows; import org.apache.commons.lang3.StringUtils; @@ -54,12 +56,12 @@ public class Context { final String assumedRoles) { final var query = em.createNativeQuery( """ - call defineContext( - cast(:currentTask as varchar), - cast(:currentRequest as varchar), - cast(:currentUser as varchar), - cast(:assumedRoles as varchar)); - """); + call defineContext( + cast(:currentTask as varchar), + cast(:currentRequest as varchar), + cast(:currentUser as varchar), + cast(:assumedRoles as varchar)); + """); query.setParameter("currentTask", shortenToMaxLength(currentTask, 96)); query.setParameter("currentRequest", shortenToMaxLength(currentRequest, 512)); // TODO.SPEC: length? query.setParameter("currentUser", currentUser); @@ -80,11 +82,17 @@ public class Context { } public String[] getAssumedRoles() { - return (String[]) em.createNativeQuery("select assumedRoles()").getSingleResult(); + return (String[]) em.createNativeQuery("select assumedRoles() as roles") + .unwrap(org.hibernate.query.NativeQuery.class) + .addScalar("roles", StringArrayType.INSTANCE) + .getSingleResult(); } public UUID[] currentSubjectsUuids() { - return (UUID[]) em.createNativeQuery("select currentSubjectsUuids()").getSingleResult(); + return (UUID[]) em.createNativeQuery("select currentSubjectsUuids() as uuids") + .unwrap(org.hibernate.query.NativeQuery.class) + .addScalar("uuids", UUIDArrayType.INSTANCE) // TODO.BLOG + .getSingleResult(); } private static String getCallerMethodNameFromStack() { diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 377d1cf2..120e98c3 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -16,7 +16,7 @@ spring: jpa: properties: hibernate: - dialect: net.hostsharing.hsadminng.config.PostgreSQL95CustomDialect + dialect: net.hostsharing.hsadminng.config.PostgresCustomDialect liquibase: contexts: dev diff --git a/src/main/resources/db/changelog/054-rbac-context.sql b/src/main/resources/db/changelog/054-rbac-context.sql index 0f3af3c5..37ff4a01 100644 --- a/src/main/resources/db/changelog/054-rbac-context.sql +++ b/src/main/resources/db/changelog/054-rbac-context.sql @@ -28,6 +28,7 @@ create or replace function determineCurrentSubjectsUuids(currentUserUuid uuid, a language plpgsql as $$ declare roleName varchar(63); + roleNameParts varchar(63); objectTableToAssume varchar(63); objectNameToAssume varchar(63); objectUuidToAssume uuid; @@ -48,10 +49,10 @@ begin foreach roleName in array string_to_array(assumedRoles, ';') loop - roleName = overlay(roleName placing '#' from length(roleName) + 1 - strpos(reverse(roleName), '.')); - objectTableToAssume = split_part(roleName, '#', 1); - objectNameToAssume = split_part(roleName, '#', 2); - roleTypeToAssume = split_part(roleName, '#', 3); + roleNameParts = overlay(roleName placing '#' from length(roleName) + 1 - strpos(reverse(roleName), '.')); + objectTableToAssume = split_part(roleNameParts, '#', 1); + objectNameToAssume = split_part(roleNameParts, '#', 2); + roleTypeToAssume = split_part(roleNameParts, '#', 3); objectUuidToAssume = findObjectUuidByIdName(objectTableToAssume, objectNameToAssume); @@ -60,7 +61,10 @@ begin where r.objectUuid = objectUuidToAssume and r.roleType = roleTypeToAssume into roleUuidToAssume; - if (not isGranted(currentUserUuid, roleUuidToAssume)) then + if roleUuidToAssume is null then + raise exception '[403] role % not accessible for user %', roleName, currentUser(); + end if; + if not isGranted(currentUserUuid, roleUuidToAssume) then raise exception '[403] user % has no permission to assume role %', currentUser(), roleName; end if; roleIdsToAssume := roleIdsToAssume || roleUuidToAssume; @@ -160,7 +164,7 @@ begin if (currentSubjectsUuids is null or length(currentSubjectsUuids) = 0 ) then currentUserName := currentUser(); if (length(currentUserName) > 0) then - raise exception '[401] currentUserUuid cannot be determined, unknown user name "%"', currentUserName; + raise exception '[401] currentSubjectsUuids (%) cannot be determined, unknown user name "%"', currentSubjectsUuids, currentUserName; else raise exception '[401] currentSubjectsUuids cannot be determined, please call `defineContext(...)` first;"'; end if; diff --git a/src/test/java/net/hostsharing/hsadminng/context/ContextIntegrationTests.java b/src/test/java/net/hostsharing/hsadminng/context/ContextIntegrationTests.java index c454feb3..25476ff4 100644 --- a/src/test/java/net/hostsharing/hsadminng/context/ContextIntegrationTests.java +++ b/src/test/java/net/hostsharing/hsadminng/context/ContextIntegrationTests.java @@ -1,5 +1,6 @@ package net.hostsharing.hsadminng.context; +import net.hostsharing.test.Array; import net.hostsharing.test.JpaAttempt; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -32,23 +33,26 @@ class ContextIntegrationTests { context.define("mike@hostsharing.net", null); - final var currentTask = context.getCurrentTask(); - assertThat(currentTask).isEqualTo("ContextIntegrationTests.defineWithoutHttpServletRequestUsesCallStack"); + assertThat(context.getCurrentTask()) + .isEqualTo("ContextIntegrationTests.defineWithoutHttpServletRequestUsesCallStack"); } @Test @Transactional void defineWithCurrentUserButWithoutAssumedRoles() { + // when context.define("mike@hostsharing.net"); - final var currentUser = context.getCurrentUser(); - assertThat(currentUser).isEqualTo("mike@hostsharing.net"); + // then + assertThat(context.getCurrentUser()). + isEqualTo("mike@hostsharing.net"); - final var currentUserUuid = context.getCurrentUserUUid(); - assertThat(currentUserUuid).isNotNull(); + assertThat(context.getCurrentUserUUid()).isNotNull(); - final var assumedRoleUuids = context.currentSubjectsUuids(); - assertThat(assumedRoleUuids).containsExactly(currentUserUuid); + assertThat(context.getAssumedRoles()).isEmpty(); + + assertThat(context.currentSubjectsUuids()) + .containsExactly(context.getCurrentUserUUid()); } @Test @@ -80,13 +84,17 @@ class ContextIntegrationTests { @Test @Transactional void defineWithCurrentUserAndAssumedRoles() { + // given context.define("mike@hostsharing.net", "customer#xxx.owner;customer#yyy.owner"); + // when final var currentUser = context.getCurrentUser(); assertThat(currentUser).isEqualTo("mike@hostsharing.net"); - final var assumedRoles = context.currentSubjectsUuids(); - assertThat(assumedRoles).hasSize(2); + // then + assertThat(context.getAssumedRoles()) + .isEqualTo(Array.of("customer#xxx.owner", "customer#yyy.owner")); + assertThat(context.currentSubjectsUuids()).hasSize(2); } @Test @@ -99,6 +107,6 @@ class ContextIntegrationTests { // then result.assertExceptionWithRootCauseMessage( javax.persistence.PersistenceException.class, - "ERROR: [403] user customer-admin@xxx.example.com has no permission to assume role package#yyy00#admin"); + "ERROR: [403] user customer-admin@xxx.example.com has no permission to assume role package#yyy00.admin"); } } 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 38b74b98..0726e249 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantControllerAcceptanceTest.java @@ -5,6 +5,7 @@ import io.restassured.http.ContentType; import io.restassured.response.ValidatableResponse; import net.hostsharing.hsadminng.Accepts; import net.hostsharing.hsadminng.HsadminNgApplication; +import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.context.ContextBasedTest; import net.hostsharing.hsadminng.rbac.rbacrole.RbacRoleEntity; import net.hostsharing.hsadminng.rbac.rbacrole.RbacRoleRepository; @@ -203,7 +204,7 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { @Test @Accepts({ "GRT:R(Read)", "GRT:X(Access Control)" }) - void packageAdmin_withAssumedPackageAdmin_canNotReadItsOwnGrantByIdAnymore() { + void packageAdmin_withAssumedPackageAdmin_canStillReadItsOwnGrantById() { // given final var givenCurrentUserAsPackageAdmin = new Subject( "pac-admin-xxx00@xxx.example.com", @@ -225,21 +226,20 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { @Test @Accepts({ "GRT:R(Read)", "GRT:X(Access Control)" }) - void packageAdmin_withAssumedUnixUserAdmin_canNotReadItsOwnGrantByIdAnymore() { + void packageAdmin_withAssumedPackageTenantRole_canNotReadItsOwnGrantByIdAnymore() { + // given final var givenCurrentUserAsPackageAdmin = new Subject( "pac-admin-xxx00@xxx.example.com", - "unixuser#xxx00-xxxa.admin"); + "package#xxx00.tenant"); final var givenGranteeUser = findRbacUserByName("pac-admin-xxx00@xxx.example.com"); final var givenGrantedRole = findRbacRoleByName("package#xxx00.admin"); - - // when final var grant = givenCurrentUserAsPackageAdmin.getGrantById() .forGrantedRole(givenGrantedRole).toGranteeUser(givenGranteeUser); // then grant.assertThat() - .statusCode(401); + .statusCode(404); } } @@ -453,8 +453,6 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { private Subject currentSubject = Subject.this; private RbacRoleEntity grantedRole; - private boolean assumed; - private RbacUserEntity granteeUser; GetGrantByIdFixture forGrantedRole(final RbacRoleEntity grantedRole) { this.grantedRole = grantedRole; @@ -462,7 +460,6 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { } ValidatableResponse toGranteeUser(final RbacUserEntity granteeUser) { - this.granteeUser = granteeUser; return RestAssured // @formatter:ff .given() @@ -473,7 +470,8 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { .get("http://localhost/api/rbac-grants/%s/%s".formatted( grantedRole.getUuid(), granteeUser.getUuid() )) - .then().log().all(); // @formatter:on + .then().log().all(); + // @formatter:on } } } 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 2f76dccb..6e423a57 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepositoryIntegrationTest.java @@ -147,7 +147,7 @@ class RbacRoleRepositoryIntegrationTest { result.assertExceptionWithRootCauseMessage( JpaSystemException.class, - "[401] currentUserUuid cannot be determined, unknown user name \"unknown@example.org\""); + "[401] currentSubjectsUuids () cannot be determined, unknown user name \"unknown@example.org\""); } } diff --git a/src/test/resources/application.yml b/src/test/resources/application.yml index 7f525a78..c6e28a1a 100644 --- a/src/test/resources/application.yml +++ b/src/test/resources/application.yml @@ -13,7 +13,7 @@ spring: properties: hibernate: default_schema: public - dialect: net.hostsharing.hsadminng.config.PostgreSQL95CustomDialect + dialect: net.hostsharing.hsadminng.config.PostgresCustomDialect format_sql: false hibernate: ddl-auto: none