From a1c3e95032895bc0b3142856d8c0f0b8a44bf19b Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 30 Aug 2022 09:35:59 +0200 Subject: [PATCH] introduce defineContext replacing explicit "set local current..." --- .../config/PostgreSQL95CustomDialect.java | 8 +- .../hsadminng/context/Context.java | 20 ++- .../resources/db/changelog/010-context.sql | 41 +++-- .../db/changelog/051-rbac-user-grant.sql | 6 +- .../db/changelog/054-rbac-context.sql | 154 ++++++++++++++---- .../resources/db/changelog/100-hs-base.sql | 30 ++-- .../db/changelog/113-hs-customer-rbac.sql | 6 +- .../changelog/118-hs-customer-test-data.sql | 5 +- .../db/changelog/128-hs-package-test-data.sql | 6 +- .../changelog/138-hs-unixuser-test-data.sql | 8 +- .../context/ContextIntegrationTests.java | 74 +++++++-- .../hsadminng/context/ContextUnitTest.java | 24 +-- .../CustomerRepositoryIntegrationTest.java | 51 +----- .../PackageRepositoryIntegrationTest.java | 52 +----- .../RbacGrantControllerAcceptanceTest.java | 28 +++- .../RbacGrantRepositoryIntegrationTest.java | 5 + .../RbacRoleRepositoryIntegrationTest.java | 36 +--- .../RbacUserRepositoryIntegrationTest.java | 5 + .../java/net/hostsharing/test/JpaAttempt.java | 17 +- 19 files changed, 328 insertions(+), 248 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/config/PostgreSQL95CustomDialect.java b/src/main/java/net/hostsharing/hsadminng/config/PostgreSQL95CustomDialect.java index 5b86aba9..4f4d73aa 100644 --- a/src/main/java/net/hostsharing/hsadminng/config/PostgreSQL95CustomDialect.java +++ b/src/main/java/net/hostsharing/hsadminng/config/PostgreSQL95CustomDialect.java @@ -1,14 +1,16 @@ package net.hostsharing.hsadminng.config; -import com.vladmihalcea.hibernate.type.array.StringArrayType; +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(2003, StringArrayType.class.getName()); - this.registerHibernateType(1111, "pg-uuid"); + this.registerHibernateType(Types.OTHER, "pg-uuid"); + this.registerHibernateType(Types.ARRAY, UUIDArrayType.class.getName()); } } diff --git a/src/main/java/net/hostsharing/hsadminng/context/Context.java b/src/main/java/net/hostsharing/hsadminng/context/Context.java index 0e461604..374967dd 100644 --- a/src/main/java/net/hostsharing/hsadminng/context/Context.java +++ b/src/main/java/net/hostsharing/hsadminng/context/Context.java @@ -1,5 +1,6 @@ package net.hostsharing.hsadminng.context; +import lombok.AllArgsConstructor; import lombok.SneakyThrows; import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Autowired; @@ -10,16 +11,17 @@ import org.springframework.web.context.request.RequestContextHolder; import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; import javax.servlet.http.HttpServletRequest; -import java.io.IOException; import java.util.Collections; import java.util.Optional; import java.util.Set; +import java.util.UUID; import java.util.stream.Collectors; import static java.util.function.Predicate.not; import static org.springframework.transaction.annotation.Propagation.MANDATORY; @Service +@AllArgsConstructor public class Context { private static final Set HEADERS_TO_IGNORE = Set.of( @@ -51,7 +53,13 @@ public class Context { final String currentUser, final String assumedRoles) { final var query = em.createNativeQuery( - "call defineContext(:currentTask, :currentRequest, :currentUser, :assumedRoles);"); + """ + 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); @@ -67,10 +75,18 @@ public class Context { return String.valueOf(em.createNativeQuery("select currentUser()").getSingleResult()); } + public UUID getCurrentUserUUid() { + return (UUID) em.createNativeQuery("select currentUserUUid()").getSingleResult(); + } + public String[] getAssumedRoles() { return (String[]) em.createNativeQuery("select assumedRoles()").getSingleResult(); } + public UUID[] currentSubjectsUuids() { + return (UUID[]) em.createNativeQuery("select currentSubjectsUuids()").getSingleResult(); + } + private static String getCallerMethodNameFromStack() { final Optional caller = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE) diff --git a/src/main/resources/db/changelog/010-context.sql b/src/main/resources/db/changelog/010-context.sql index f42efd99..2d3486df 100644 --- a/src/main/resources/db/changelog/010-context.sql +++ b/src/main/resources/db/changelog/010-context.sql @@ -4,10 +4,24 @@ -- ============================================================================ --changeset context-DEFINE:1 endDelimiter:--// -- ---------------------------------------------------------------------------- + +/* + Callback which is called after the context has been (re-) defined. + This function will be overwritten by later changesets. + */ +create procedure contextDefined( + currentTask varchar, + currentRequest varchar, + currentUser varchar, + assumedRoles varchar +) + language plpgsql as $$ +begin +end; $$; + /* Defines the transaction context. */ - create or replace procedure defineContext( currentTask varchar, currentRequest varchar, @@ -16,14 +30,18 @@ create or replace procedure defineContext( ) language plpgsql as $$ begin - raise notice 'currentRequest: %', defineContext.currentRequest; execute format('set local hsadminng.currentTask to %L', currentTask); + + currentRequest := coalesce(currentRequest, ''); + execute format('set local hsadminng.currentRequest to %L', currentRequest); + + currentUser := coalesce(currentUser, ''); execute format('set local hsadminng.currentUser to %L', currentUser); - if length(assumedRoles) > 0 then - execute format('set local hsadminng.assumedRoles to %L', assumedRoles); - else - execute format('set local hsadminng.assumedRoles to %L', ''); - end if; + + assumedRoles := coalesce(assumedRoles, ''); + execute format('set local hsadminng.assumedRoles to %L', assumedRoles); + + call contextDefined(currentTask, currentRequest, currentUser, assumedRoles); end; $$; --// @@ -49,7 +67,7 @@ begin currentTask := null; end; if (currentTask is null or currentTask = '') then - raise exception '[401] hsadminng.currentTask must be defined, please use "SET LOCAL ...;"'; + raise exception '[401] currentTask must be defined, please call `defineContext(...)`'; end if; raise debug 'currentTask: %', currentTask; return currentTask; @@ -61,8 +79,7 @@ end; $$; --changeset context-CURRENT-USER:1 endDelimiter:--// -- ---------------------------------------------------------------------------- /* - Returns the current user as set by `hsadminng.currentUser`. - Raises exception if not set. + Returns the current user as defined by `defineContext(...)`. */ create or replace function currentUser() returns varchar(63) @@ -77,10 +94,6 @@ begin when others then currentUser := null; end; - if (currentUser is null or currentUser = '') then - raise exception '[401] hsadminng.currentUser must be defined, please use "SET LOCAL ...;"'; - end if; - raise debug 'currentUser: %', currentUser; return currentUser; end; $$; --// diff --git a/src/main/resources/db/changelog/051-rbac-user-grant.sql b/src/main/resources/db/changelog/051-rbac-user-grant.sql index d5698d6e..d36c9f24 100644 --- a/src/main/resources/db/changelog/051-rbac-user-grant.sql +++ b/src/main/resources/db/changelog/051-rbac-user-grant.sql @@ -9,15 +9,15 @@ create or replace function assumedRoleUuid() stable leakproof language plpgsql as $$ declare - currentSubjectUuids uuid[]; + currentSubjectsUuids uuid[]; begin -- exactly one role must be assumed, not none not more than one if cardinality(assumedRoles()) <> 1 then raise exception '[400] Granting roles to user is only possible if exactly one role is assumed, given: %', assumedRoles(); end if; - currentSubjectUuids := currentSubjectsUuids(); - return currentSubjectUuids[1]; + currentSubjectsUuids := currentSubjectsUuids(); + return currentSubjectsUuids[1]; end; $$; create or replace procedure grantRoleToUserUnchecked(grantedByRoleUuid uuid, roleUuid uuid, userUuid uuid, doAssume boolean = true) diff --git a/src/main/resources/db/changelog/054-rbac-context.sql b/src/main/resources/db/changelog/054-rbac-context.sql index a39f2bca..0f3af3c5 100644 --- a/src/main/resources/db/changelog/054-rbac-context.sql +++ b/src/main/resources/db/changelog/054-rbac-context.sql @@ -1,45 +1,32 @@ --liquibase formatted sql --- ============================================================================ ---changeset rbac-context-CURRENT-USER-ID:1 endDelimiter:--// --- ---------------------------------------------------------------------------- -/* - Returns the id of the current user as set by `hsadminng.currentUser`. - Raises exception if not set. - */ -create or replace function currentUserUuid() +-- ============================================================================ +--changeset rbac-context-DETERMINE:1 endDelimiter:--// +-- ---------------------------------------------------------------------------- + +create or replace function determineCurrentUserUuid(currentUser varchar) returns uuid stable leakproof language plpgsql as $$ declare - currentUser varchar(63); currentUserUuid uuid; begin - currentUser := currentUser(); - currentUserUuid = (select uuid from RbacUser where name = currentUser); - if currentUserUuid is null then - raise exception '[401] hsadminng.currentUser defined as %, but does not exists', currentUser; + if currentUser = '' then + return null; end if; + + select uuid from RbacUser where name = currentUser into currentUserUuid; + -- TODO: maybe this should be changed, and in this case no user name defined in context? + -- no exception if user does not exist because users can register themselves return currentUserUuid; end; $$; ---// --- ============================================================================ ---changeset rbac-context-CURRENT-SUBJECT-IDS:1 endDelimiter:--// --- ---------------------------------------------------------------------------- -/* - Returns id of current user as set in `hsadminng.currentUser` - or, if any, ids of assumed role names as set in `hsadminng.assumedRoles` - or empty array, if not set. - */ -create or replace function currentSubjectsUuids() +create or replace function determineCurrentSubjectsUuids(currentUserUuid uuid, assumedRoles varchar) returns uuid[] stable leakproof language plpgsql as $$ declare - currentUserUuid uuid; - roleNames varchar(63)[]; roleName varchar(63); objectTableToAssume varchar(63); objectNameToAssume varchar(63); @@ -48,19 +35,18 @@ declare roleIdsToAssume uuid[]; roleUuidToAssume uuid; begin - currentUserUuid := currentUserUuid(); if currentUserUuid is null then - raise exception '[401] user % does not exist', currentUser(); + if length(coalesce(assumedRoles, '')) > 0 then + raise exception '[403] undefined has no permission to assume role %', assumedRoles; + else + return array[]::uuid[]; + end if; end if; - - roleNames := assumedRoles(); - if cardinality(roleNames) = 0 then + if length(coalesce(assumedRoles, '')) = 0 then return array [currentUserUuid]; end if; - raise notice 'assuming roles: %', roleNames; - - foreach roleName in array roleNames + 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); @@ -69,19 +55,117 @@ begin objectUuidToAssume = findObjectUuidByIdName(objectTableToAssume, objectNameToAssume); - -- TODO: either the result needs to be cached at least per transaction or we need to get rid of SELCT in a loop select uuid as roleuuidToAssume from RbacRole r where r.objectUuid = objectUuidToAssume and r.roleType = roleTypeToAssume into roleUuidToAssume; if (not isGranted(currentUserUuid, roleUuidToAssume)) then - raise exception '[403] user % (%) has no permission to assume role % (%)', currentUser(), currentUserUuid, roleName, roleUuidToAssume; + raise exception '[403] user % has no permission to assume role %', currentUser(), roleName; end if; roleIdsToAssume := roleIdsToAssume || roleUuidToAssume; end loop; return roleIdsToAssume; end; $$; + +-- ============================================================================ +--changeset rbac-context-CONTEXT-DEFINED:1 endDelimiter:--// +-- ---------------------------------------------------------------------------- +/* + Callback which is called after the context has been (re-) defined. + This function will be overwritten by later changesets. + */ +create or replace procedure contextDefined( + currentTask varchar, + currentRequest varchar, + currentUser varchar, + assumedRoles varchar +) + language plpgsql as $$ +declare + currentUserUuid uuid; +begin + execute format('set local hsadminng.currentTask to %L', currentTask); + + execute format('set local hsadminng.currentRequest to %L', currentRequest); + + execute format('set local hsadminng.currentUser to %L', currentUser); + select determineCurrentUserUuid(currentUser) into currentUserUuid; + execute format('set local hsadminng.currentUserUuid to %L', coalesce(currentUserUuid::text, '')); + + execute format('set local hsadminng.assumedRoles to %L', assumedRoles); + execute format('set local hsadminng.currentSubjectsUuids to %L', + (select array_to_string(determinecurrentSubjectsUuids(currentUserUuid, assumedRoles), ';'))); + + raise notice 'Context defined as: %, %, %, [%]', currentTask, currentRequest, currentUser, assumedRoles; +end; $$; + + +-- ============================================================================ +--changeset rbac-context-CURRENT-USER-ID:1 endDelimiter:--// +-- ---------------------------------------------------------------------------- +/* + Returns the uuid of the current user as set via `defineContext(...)`. + */ + +create or replace function currentUserUuid() + returns uuid + stable leakproof + language plpgsql as $$ +declare + currentUserUuid text; + currentUserName text; +begin + begin + currentUserUuid := current_setting('hsadminng.currentUserUuid'); + exception + when others then + currentUserUuid := null; + end; + if (currentUserUuid is null or currentUserUuid = '') then + currentUserName := currentUser(); + if (length(currentUserName) > 0) then + raise exception '[401] currentUserUuid cannot be determined, unknown user name "%"', currentUserName; + else + raise exception '[401] currentUserUuid cannot be determined, please call `defineContext(...)` first;"'; + end if; + end if; + return currentUserUuid::uuid; +end; $$; +--// + +-- ============================================================================ +--changeset rbac-context-CURRENT-SUBJECT-UUIDS:1 endDelimiter:--// +-- ---------------------------------------------------------------------------- +/* + Returns the uuid of the current user as set via `defineContext(...)`, + or, if any, the uuids of all assumed roles as set via `defineContext(...)` + or empty array, if context is not defined. + */ +create or replace function currentSubjectsUuids() + returns uuid[] + stable leakproof + language plpgsql as $$ +declare + currentSubjectsUuids text; + currentUserName text; +begin + begin + currentSubjectsUuids := current_setting('hsadminng.currentSubjectsUuids'); + exception + when others then + currentSubjectsUuids := null; + end; + 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; + else + raise exception '[401] currentSubjectsUuids cannot be determined, please call `defineContext(...)` first;"'; + end if; + end if; + return string_to_array(currentSubjectsUuids, ';'); +end; $$; --// diff --git a/src/main/resources/db/changelog/100-hs-base.sql b/src/main/resources/db/changelog/100-hs-base.sql index f01dc084..be486aed 100644 --- a/src/main/resources/db/changelog/100-hs-base.sql +++ b/src/main/resources/db/changelog/100-hs-base.sql @@ -21,12 +21,13 @@ grant select on global to restricted; /** A single row to be referenced as a global object. */ -set local hsadminng.currentUser to 'init'; -set local hsadminng.currentTask to 'initializing table "global"'; -insert - into RbacObject (objecttable) values ('global'); -insert - into Global (uuid, name) values ((select uuid from RbacObject where objectTable = 'global'), 'hostsharing'); +begin transaction; + call defineContext('initializing table "global"', null, null, null); + insert + into RbacObject (objecttable) values ('global'); + insert + into Global (uuid, name) values ((select uuid from RbacObject where objectTable = 'global'), 'hostsharing'); +commit; --// @@ -40,8 +41,7 @@ create or replace function hasGlobalPermission(op RbacOp) $$ -- TODO: this could to be optimized select (select uuid from global) in - (select queryAccessibleObjectUuidsOfSubjectIds( - op, 'global', currentSubjectsUuids())); + (select queryAccessibleObjectUuidsOfSubjectIds(op, 'global', currentSubjectsUuids())); $$; --// @@ -94,9 +94,10 @@ create or replace function hostsharingAdmin() select 'global', (select uuid from RbacObject where objectTable = 'global'), 'admin'::RbacRoleType; $$; -set local hsadminng.currentUser to 'init'; -set local hsadminng.currentTask to 'creating Hostsharing admin role'; -select createRole(hostsharingAdmin()); +begin transaction; + call defineContext('creating Hostsharing admin role', null, null, null); + select createRole(hostsharingAdmin()); +commit; -- ============================================================================ --changeset hs-base-ADMIN-USERS:1 context:dev,tc endDelimiter:--// @@ -108,8 +109,7 @@ do language plpgsql $$ declare admins uuid ; begin - set local hsadminng.currentUser to 'init'; - set local hsadminng.currentTask to 'creating fake Hostsharing admin users'; + call defineContext('creating fake Hostsharing admin users', null, null, null); admins = findRoleId(hostsharingAdmin()); call grantRoleToUserUnchecked(admins, admins, createRbacUser('mike@hostsharing.net')); @@ -131,13 +131,13 @@ do language plpgsql $$ declare userName varchar; begin - set local hsadminng.currentUser = 'sven@hostsharing.net'; + call defineContext('testing currentUserUuid', null, 'sven@hostsharing.net', null); select userName from RbacUser where uuid = currentUserUuid() into userName; if userName <> 'sven@hostsharing.net' then raise exception 'setting or fetching initial currentUser failed, got: %', userName; end if; - set local hsadminng.currentUser = 'mike@hostsharing.net'; + call defineContext('testing currentUserUuid', null, 'mike@hostsharing.net', null); select userName from RbacUser where uuid = currentUserUuid() into userName; if userName = 'mike@hostsharing.net' then raise exception 'currentUser should not change in one transaction, but did change, got: %', userName; diff --git a/src/main/resources/db/changelog/113-hs-customer-rbac.sql b/src/main/resources/db/changelog/113-hs-customer-rbac.sql index 9ce1b838..63cbef1c 100644 --- a/src/main/resources/db/changelog/113-hs-customer-rbac.sql +++ b/src/main/resources/db/changelog/113-hs-customer-rbac.sql @@ -206,8 +206,7 @@ do language plpgsql $$ hostsharingObjectUuid uuid; hsAdminRoleUuid uuid ; begin - set local hsadminng.currentUser to 'init'; - set local hsadminng.currentTask to 'granting global add-customer permission to Hostsharing admin role'; + call defineContext('granting global add-customer permission to Hostsharing admin role', null, null, null); hsAdminRoleUuid := findRoleId(hostsharingAdmin()); hostsharingObjectUuid := (select uuid from global); @@ -224,7 +223,8 @@ create or replace function addCustomerNotAllowedForCurrentSubjects() language PLPGSQL as $$ begin - raise exception '[403] add-customer not permitted for %', array_to_string(currentSubjects(), ';', 'null'); + raise exception '[403] add-customer not permitted for %', + array_to_string(currentSubjects(), ';', 'null'); end; $$; /** diff --git a/src/main/resources/db/changelog/118-hs-customer-test-data.sql b/src/main/resources/db/changelog/118-hs-customer-test-data.sql index b7b8c102..172b0d40 100644 --- a/src/main/resources/db/changelog/118-hs-customer-test-data.sql +++ b/src/main/resources/db/changelog/118-hs-customer-test-data.sql @@ -30,8 +30,7 @@ declare custAdminName varchar; begin currentTask = 'creating RBAC test customer #' || custReference || '/' || custPrefix; - set local hsadminng.currentUser to 'mike@hostsharing.net'; - set local hsadminng.assumedRoles to 'global#hostsharing.admin'; + call defineContext(currentTask, null, 'mike@hostsharing.net', 'global#hostsharing.admin'); execute format('set local hsadminng.currentTask to %L', currentTask); custRowId = uuid_generate_v4(); @@ -53,8 +52,6 @@ create or replace procedure createCustomerTestData( ) language plpgsql as $$ begin - set hsadminng.currentUser to ''; - for t in startCount..endCount loop call createCustomerTestData(testCustomerReference(t), intToVarChar(t, 3)); diff --git a/src/main/resources/db/changelog/128-hs-package-test-data.sql b/src/main/resources/db/changelog/128-hs-package-test-data.sql index b8263b21..35f098de 100644 --- a/src/main/resources/db/changelog/128-hs-package-test-data.sql +++ b/src/main/resources/db/changelog/128-hs-package-test-data.sql @@ -26,9 +26,7 @@ begin custAdminUser = 'customer-admin@' || cust.prefix || '.example.com'; custAdminRole = 'customer#' || cust.prefix || '.admin'; - execute format('set local hsadminng.currentUser to %L', custAdminUser); - execute format('set local hsadminng.assumedRoles to %L', custAdminRole); - execute format('set local hsadminng.currentTask to %L', currentTask); + call defineContext(currentTask, null, custAdminUser, custAdminRole); raise notice 'task: % by % as %', currentTask, custAdminUser, custAdminRole; insert @@ -53,8 +51,6 @@ create or replace procedure createPackageTestData() declare cust customer; begin - set hsadminng.currentUser to ''; - for cust in (select * from customer) loop continue when cust.reference >= 90000; -- reserved for functional testing diff --git a/src/main/resources/db/changelog/138-hs-unixuser-test-data.sql b/src/main/resources/db/changelog/138-hs-unixuser-test-data.sql index 2d6607e9..0636d63a 100644 --- a/src/main/resources/db/changelog/138-hs-unixuser-test-data.sql +++ b/src/main/resources/db/changelog/138-hs-unixuser-test-data.sql @@ -13,8 +13,6 @@ declare pacAdmin varchar; currentTask varchar; begin - set hsadminng.currentUser to ''; - select p.uuid, p.name, c.prefix as custPrefix from package p join customer c on p.customeruuid = c.uuid @@ -26,9 +24,7 @@ begin currentTask = 'creating RBAC test unixuser #' || t || ' for package ' || pac.name || ' #' || pac.uuid; raise notice 'task: %', currentTask; pacAdmin = 'pac-admin-' || pac.name || '@' || pac.custPrefix || '.example.com'; - execute format('set local hsadminng.currentTask to %L', currentTask); - execute format('set local hsadminng.currentUser to %L', pacAdmin); - set local hsadminng.assumedRoles = ''; + call defineContext(currentTask, null, pacAdmin, null); insert into unixuser (name, packageUuid) @@ -46,8 +42,6 @@ declare pacAdmin varchar; currentTask varchar; begin - set hsadminng.currentUser to ''; - for pac in (select p.uuid, p.name from package p diff --git a/src/test/java/net/hostsharing/hsadminng/context/ContextIntegrationTests.java b/src/test/java/net/hostsharing/hsadminng/context/ContextIntegrationTests.java index 1214c5b9..c454feb3 100644 --- a/src/test/java/net/hostsharing/hsadminng/context/ContextIntegrationTests.java +++ b/src/test/java/net/hostsharing/hsadminng/context/ContextIntegrationTests.java @@ -1,52 +1,104 @@ package net.hostsharing.hsadminng.context; +import net.hostsharing.test.JpaAttempt; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.ComponentScan; import org.springframework.test.annotation.DirtiesContext; import org.springframework.transaction.annotation.Transactional; +import javax.servlet.http.HttpServletRequest; + import static org.assertj.core.api.Assertions.assertThat; @DataJpaTest -@ComponentScan(basePackageClasses = Context.class) +@ComponentScan(basePackageClasses = { Context.class, JpaAttempt.class }) @DirtiesContext class ContextIntegrationTests { @Autowired private Context context; - @Test - void registerWithoutHttpServletRequestUsesCallStack() { + @MockBean + private HttpServletRequest request; - context.define("current-user", null); + @Autowired + private JpaAttempt jpaAttempt; + + @Test + void defineWithoutHttpServletRequestUsesCallStack() { + + context.define("mike@hostsharing.net", null); final var currentTask = context.getCurrentTask(); - assertThat(currentTask).isEqualTo("ContextIntegrationTests.registerWithoutHttpServletRequestUsesCallStack"); + assertThat(currentTask).isEqualTo("ContextIntegrationTests.defineWithoutHttpServletRequestUsesCallStack"); } @Test @Transactional - void setCurrentUser() { + void defineWithCurrentUserButWithoutAssumedRoles() { context.define("mike@hostsharing.net"); final var currentUser = context.getCurrentUser(); assertThat(currentUser).isEqualTo("mike@hostsharing.net"); - final var assumedRoles = context.getAssumedRoles(); - assertThat(assumedRoles).isEmpty(); + final var currentUserUuid = context.getCurrentUserUUid(); + assertThat(currentUserUuid).isNotNull(); + + final var assumedRoleUuids = context.currentSubjectsUuids(); + assertThat(assumedRoleUuids).containsExactly(currentUserUuid); + } + + @Test + void defineWithoutCurrentUserButWithAssumedRoles() { + // when + final var result = jpaAttempt.transacted(() -> + context.define(null, "package#yyy00.admin") + ); + + // then + result.assertExceptionWithRootCauseMessage( + javax.persistence.PersistenceException.class, + "ERROR: [403] undefined has no permission to assume role package#yyy00.admin"); + } + + @Test + void defineWithUnknownCurrentUserButWithAssumedRoles() { + // when + final var result = jpaAttempt.transacted(() -> + context.define("unknown@example.org", "package#yyy00.admin") + ); + + // then + result.assertExceptionWithRootCauseMessage( + javax.persistence.PersistenceException.class, + "ERROR: [403] undefined has no permission to assume role package#yyy00.admin"); } @Test @Transactional - void assumeRoles() { + void defineWithCurrentUserAndAssumedRoles() { context.define("mike@hostsharing.net", "customer#xxx.owner;customer#yyy.owner"); final var currentUser = context.getCurrentUser(); assertThat(currentUser).isEqualTo("mike@hostsharing.net"); - final var assumedRoles = context.getAssumedRoles(); - assertThat(assumedRoles).containsExactlyInAnyOrder("customer#xxx.owner", "customer#yyy.owner"); + final var assumedRoles = context.currentSubjectsUuids(); + assertThat(assumedRoles).hasSize(2); + } + + @Test + public void defineContextWithCurrentUserAndAssumeInaccessibleRole() { + // when + final var result = jpaAttempt.transacted(() -> + context.define("customer-admin@xxx.example.com", "package#yyy00.admin") + ); + + // then + result.assertExceptionWithRootCauseMessage( + javax.persistence.PersistenceException.class, + "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/context/ContextUnitTest.java b/src/test/java/net/hostsharing/hsadminng/context/ContextUnitTest.java index fec12cd9..6cb0368b 100644 --- a/src/test/java/net/hostsharing/hsadminng/context/ContextUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/context/ContextUnitTest.java @@ -26,11 +26,13 @@ import static org.mockito.Mockito.verify; @ExtendWith(MockitoExtension.class) class ContextUnitTest { - @Mock - EntityManager em; - - @Mock - Query nativeQuery; + private String defineContextQueryString = """ + call defineContext( + cast(:currentTask as varchar), + cast(:currentRequest as varchar), + cast(:currentUser as varchar), + cast(:assumedRoles as varchar)); + """; @Nested class WithoutHttpRequest { @@ -56,7 +58,7 @@ class ContextUnitTest { context.define("current-user"); - verify(em).createNativeQuery("call defineContext(:currentTask, :currentRequest, :currentUser, :assumedRoles);"); + verify(em).createNativeQuery(defineContextQueryString); verify(nativeQuery).setParameter( "currentTask", "WithoutHttpRequest.registerWithoutHttpServletRequestUsesCallStackForTask"); @@ -68,7 +70,7 @@ class ContextUnitTest { context.define("current-user"); - verify(em).createNativeQuery("call defineContext(:currentTask, :currentRequest, :currentUser, :assumedRoles);"); + verify(em).createNativeQuery(defineContextQueryString); verify(nativeQuery).setParameter("currentRequest", ""); } } @@ -113,7 +115,7 @@ class ContextUnitTest { context.define("current-user"); - verify(em).createNativeQuery("call defineContext(:currentTask, :currentRequest, :currentUser, :assumedRoles);"); + verify(em).createNativeQuery(defineContextQueryString); verify(nativeQuery).setParameter("currentTask", "POST http://localhost:9999/api/endpoint"); } @@ -127,7 +129,7 @@ class ContextUnitTest { context.define("current-user"); - verify(em).createNativeQuery("call defineContext(:currentTask, :currentRequest, :currentUser, :assumedRoles);"); + verify(em).createNativeQuery(defineContextQueryString); verify(nativeQuery).setParameter("currentRequest", """ curl -0 -v -X POST http://localhost:9999/api/endpoint \\ -H 'current-user:given-user' \\ @@ -150,7 +152,7 @@ class ContextUnitTest { context.define("current-user"); - verify(em).createNativeQuery("call defineContext(:currentTask, :currentRequest, :currentUser, :assumedRoles);"); + verify(em).createNativeQuery(defineContextQueryString); verify(nativeQuery).setParameter(eq("currentTask"), argThat((String t) -> t.length() == 96)); } @@ -169,7 +171,7 @@ class ContextUnitTest { context.define("current-user"); - verify(em).createNativeQuery("call defineContext(:currentTask, :currentRequest, :currentUser, :assumedRoles);"); + verify(em).createNativeQuery(defineContextQueryString); verify(nativeQuery).setParameter(eq("currentRequest"), argThat((String t) -> t.length() == 512)); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerRepositoryIntegrationTest.java index fd7a9889..ab84986d 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hscustomer/CustomerRepositoryIntegrationTest.java @@ -6,13 +6,13 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.ComponentScan; -import org.springframework.orm.jpa.JpaSystemException; import org.springframework.test.annotation.DirtiesContext; -import org.springframework.transaction.annotation.Transactional; import javax.persistence.EntityManager; import javax.persistence.PersistenceException; +import javax.servlet.http.HttpServletRequest; import java.util.List; import java.util.UUID; @@ -30,6 +30,9 @@ class CustomerRepositoryIntegrationTest extends ContextBasedTest { @Autowired EntityManager em; + @MockBean + HttpServletRequest request; + @Nested class CreateCustomer { @@ -144,50 +147,6 @@ class CustomerRepositoryIntegrationTest extends ContextBasedTest { exactlyTheseCustomersAreReturned(result, "xxx"); } - - @Test - public void customerAdmin_withAssumedAlienPackageAdminRole_cannotViewAnyCustomer() { - // given: - context("customer-admin@xxx.example.com", "package#yyy00.admin"); - - // when - final var result = attempt( - em, - () -> customerRepository.findCustomerByOptionalPrefixLike(null)); - - // then - result.assertExceptionWithRootCauseMessage( - JpaSystemException.class, - "[403] user customer-admin@xxx.example.com", "has no permission to assume role package#yyy00#admin"); - } - - @Test - void unknownUser_withoutAssumedRole_cannotViewAnyCustomers() { - context("unknown@example.org", null); - - final var result = attempt( - em, - () -> customerRepository.findCustomerByOptionalPrefixLike(null)); - - result.assertExceptionWithRootCauseMessage( - JpaSystemException.class, - "hsadminng.currentUser defined as unknown@example.org, but does not exists"); - } - - @Test - @Transactional - void unknownUser_withAssumedCustomerRole_cannotViewAnyCustomers() { - context("unknown@example.org", "customer#xxx.admin"); - - final var result = attempt( - em, - () -> customerRepository.findCustomerByOptionalPrefixLike(null)); - - result.assertExceptionWithRootCauseMessage( - JpaSystemException.class, - "hsadminng.currentUser defined as unknown@example.org, but does not exists"); - } - } @Nested diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hspackage/PackageRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hspackage/PackageRepositoryIntegrationTest.java index fa326ed4..036750ca 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hspackage/PackageRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hspackage/PackageRepositoryIntegrationTest.java @@ -7,16 +7,15 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.ComponentScan; import org.springframework.orm.ObjectOptimisticLockingFailureException; -import org.springframework.orm.jpa.JpaSystemException; import org.springframework.test.annotation.DirtiesContext; -import org.springframework.transaction.annotation.Transactional; import javax.persistence.EntityManager; +import javax.servlet.http.HttpServletRequest; import java.util.List; -import static net.hostsharing.test.JpaAttempt.attempt; import static org.assertj.core.api.Assertions.assertThat; @DataJpaTest @@ -36,6 +35,9 @@ class PackageRepositoryIntegrationTest { @Autowired JpaAttempt jpaAttempt; + @MockBean + HttpServletRequest request; + @Nested class FindAllByOptionalNameLike { @@ -83,50 +85,6 @@ class PackageRepositoryIntegrationTest { exactlyThesePackagesAreReturned(result, "xxx00"); } - - @Test - public void customerAdmin_withAssumedAlienPackageAdminRole_cannotViewAnyPackages() { - // given: - context.define("customer-admin@xxx.example.com", "package#yyy00.admin"); - - // when - final var result = attempt( - em, - () -> packageRepository.findAllByOptionalNameLike(null)); - - // then - result.assertExceptionWithRootCauseMessage( - JpaSystemException.class, - "[403] user customer-admin@xxx.example.com", "has no permission to assume role package#yyy00#admin"); - } - - @Test - void unknownUser_withoutAssumedRole_cannotViewAnyPackages() { - context.define("unknown@example.org"); - - final var result = attempt( - em, - () -> packageRepository.findAllByOptionalNameLike(null)); - - result.assertExceptionWithRootCauseMessage( - JpaSystemException.class, - "hsadminng.currentUser defined as unknown@example.org, but does not exists"); - } - - @Test - @Transactional - void unknownUser_withAssumedCustomerRole_cannotViewAnyPackages() { - context.define("unknown@example.org", "customer#xxx.admin"); - - final var result = attempt( - em, - () -> packageRepository.findAllByOptionalNameLike(null)); - - result.assertExceptionWithRootCauseMessage( - JpaSystemException.class, - "hsadminng.currentUser defined as unknown@example.org, but does not exists"); - } - } @Nested 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 b73f1d4b..38b74b98 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantControllerAcceptanceTest.java @@ -202,8 +202,30 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { } @Test - @Accepts({ "GRT:R(Read)" }) - void packageAdmin_withAssumedUnixUserAdmin_canNotReadItsOwnGrantById() { + @Accepts({ "GRT:R(Read)", "GRT:X(Access Control)" }) + void packageAdmin_withAssumedPackageAdmin_canNotReadItsOwnGrantByIdAnymore() { + // given + final var givenCurrentUserAsPackageAdmin = new Subject( + "pac-admin-xxx00@xxx.example.com", + "package#xxx00.admin"); + 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(200) + .body("grantedByRoleIdName", is("customer#xxx.admin")) + .body("grantedRoleIdName", is("package#xxx00.admin")) + .body("granteeUserName", is("pac-admin-xxx00@xxx.example.com")); + } + + @Test + @Accepts({ "GRT:R(Read)", "GRT:X(Access Control)" }) + void packageAdmin_withAssumedUnixUserAdmin_canNotReadItsOwnGrantByIdAnymore() { // given final var givenCurrentUserAsPackageAdmin = new Subject( "pac-admin-xxx00@xxx.example.com", @@ -217,7 +239,7 @@ class RbacGrantControllerAcceptanceTest extends ContextBasedTest { // then grant.assertThat() - .statusCode(404); + .statusCode(401); } } 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 3dfb033a..d0c12e11 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacgrant/RbacGrantRepositoryIntegrationTest.java @@ -11,6 +11,7 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.ComponentScan; import org.springframework.orm.jpa.JpaSystemException; import org.springframework.test.annotation.DirtiesContext; @@ -18,6 +19,7 @@ import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import javax.persistence.EntityManager; +import javax.servlet.http.HttpServletRequest; import java.util.List; import java.util.UUID; @@ -33,6 +35,9 @@ class RbacGrantRepositoryIntegrationTest extends ContextBasedTest { @Autowired Context context; + @MockBean + HttpServletRequest request; + @Autowired RbacGrantRepository rbacGrantRepository; 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 7bc85bd6..2f76dccb 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleRepositoryIntegrationTest.java @@ -6,11 +6,13 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.ComponentScan; import org.springframework.orm.jpa.JpaSystemException; import org.springframework.test.annotation.DirtiesContext; import javax.persistence.EntityManager; +import javax.servlet.http.HttpServletRequest; import java.util.List; import static net.hostsharing.test.JpaAttempt.attempt; @@ -30,6 +32,9 @@ class RbacRoleRepositoryIntegrationTest { @Autowired EntityManager em; + @MockBean + HttpServletRequest request; + @Nested class FindAllRbacRoles { @@ -132,22 +137,6 @@ class RbacRoleRepositoryIntegrationTest { "unixuser#xxx00-aaab.owner"); } - @Test - public void customerAdmin_withAssumedAlienPackageAdminRole_cannotViewAnyRbacRole() { - // given: - context.define("customer-admin@xxx.example.com", "package#yyy00.admin"); - - // when - final var result = attempt( - em, - () -> rbacRoleRepository.findAll()); - - // then - result.assertExceptionWithRootCauseMessage( - JpaSystemException.class, - "[403] user customer-admin@xxx.example.com", "has no permission to assume role package#yyy00#admin"); - } - @Test void unknownUser_withoutAssumedRole_cannotViewAnyRbacRoles() { context.define("unknown@example.org"); @@ -158,20 +147,7 @@ class RbacRoleRepositoryIntegrationTest { result.assertExceptionWithRootCauseMessage( JpaSystemException.class, - "hsadminng.currentUser defined as unknown@example.org, but does not exists"); - } - - @Test - void unknownUser_withAssumedRbacRoleRole_cannotViewAnyRbacRoles() { - context.define("unknown@example.org", "RbacRole#xxx.admin"); - - final var result = attempt( - em, - () -> rbacRoleRepository.findAll()); - - result.assertExceptionWithRootCauseMessage( - JpaSystemException.class, - "hsadminng.currentUser defined as unknown@example.org, but does not exists"); + "[401] currentUserUuid cannot be determined, unknown user name \"unknown@example.org\""); } } 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 edcc3184..e34776bc 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserRepositoryIntegrationTest.java @@ -8,6 +8,7 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.ComponentScan; import org.springframework.orm.jpa.JpaSystemException; import org.springframework.test.annotation.DirtiesContext; @@ -15,6 +16,7 @@ import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import javax.persistence.EntityManager; +import javax.servlet.http.HttpServletRequest; import java.util.List; import java.util.UUID; @@ -35,6 +37,9 @@ class RbacUserRepositoryIntegrationTest extends ContextBasedTest { @Autowired EntityManager em; + @MockBean + HttpServletRequest request; + @Nested class CreateUser { diff --git a/src/test/java/net/hostsharing/test/JpaAttempt.java b/src/test/java/net/hostsharing/test/JpaAttempt.java index 4c66d7b3..28dc7130 100644 --- a/src/test/java/net/hostsharing/test/JpaAttempt.java +++ b/src/test/java/net/hostsharing/test/JpaAttempt.java @@ -11,7 +11,6 @@ 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. @@ -55,7 +54,7 @@ public class JpaAttempt { public JpaResult transacted(final Supplier code) { try { return JpaResult.forValue( - transactionTemplate.execute(transactionStatus -> code.get())); + transactionTemplate.execute(transactionStatus -> code.get())); } catch (final RuntimeException exc) { return JpaResult.forException(exc); } @@ -114,7 +113,6 @@ public class JpaAttempt { if (expectedExceptionClass.isAssignableFrom(exception.getClass())) { return (E) exception; } - fail(""); throw new AssertionError("expected " + expectedExceptionClass + " but got " + exception); } @@ -123,8 +121,8 @@ public class JpaAttempt { } public void assertExceptionWithRootCauseMessage( - final Class expectedExceptionClass, - final String... expectedRootCauseMessages) { + final Class expectedExceptionClass, + final String... expectedRootCauseMessages) { assertThat(wasSuccessful()).isFalse(); final String firstRootCauseMessageLine = firstRootCauseMessageLineOf(caughtException(expectedExceptionClass)); for (String expectedRootCauseMessage : expectedRootCauseMessages) { @@ -133,16 +131,17 @@ public class JpaAttempt { } public JpaResult assumeSuccessful() { - assertThat(exception).isNull();; + assertThat(exception).isNull(); + ; return this; } private String firstRootCauseMessageLineOf(final RuntimeException exception) { final var rootCause = NestedExceptionUtils.getRootCause(exception); return Optional.ofNullable(rootCause) - .map(Throwable::getMessage) - .map(message -> message.split("\\r|\\n|\\r\\n", 0)[0]) - .orElse(null); + .map(Throwable::getMessage) + .map(message -> message.split("\\r|\\n|\\r\\n", 0)[0]) + .orElse(null); } }