From d8b1d18952ee946741ac3c6351cd82a0c158b37b Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Fri, 19 Apr 2024 10:06:57 +0200 Subject: [PATCH] fix booking item identity view and some other minor issues --- .../hs/booking/item/HsBookingItemEntity.java | 13 +++++--- .../hs-booking/hs-booking-item-schemas.yaml | 7 +---- .../6013-hs-booking-item-rbac.md | 7 +++-- .../6013-hs-booking-item-rbac.sql | 20 ++++++++---- .../6018-hs-booking-item-test-data.sql | 2 +- ...HsBookingItemControllerAcceptanceTest.java | 8 ++--- ...sBookingItemRepositoryIntegrationTest.java | 31 ++++++++++++------- ...fficePartnerRepositoryIntegrationTest.java | 1 + ...ficeRelationRepositoryIntegrationTest.java | 2 +- 9 files changed, 54 insertions(+), 37 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java index 7a846f46..aad7d836 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java @@ -141,12 +141,12 @@ public class HsBookingItemEntity implements Stringifyable, RbacObject { public static RbacView rbac() { return rbacViewFor("bookingItem", HsBookingItemEntity.class) .withIdentityView(SQL.query(""" - SELECT i.uuid as uuid, d.idName || ':' || i.caption as idName - FROM hs_booking_item i - JOIN hs_office_debitor_iv d ON d.uuid = i.debitorUuid + SELECT bookingItem.uuid as uuid, debitorIV.idName || '-' || cleanIdentifier(bookingItem.caption) as idName + FROM hs_booking_item bookingItem + JOIN hs_office_debitor_iv debitorIV ON debitorIV.uuid = bookingItem.debitorUuid """)) .withRestrictedViewOrderBy(SQL.expression("validity")) - .withUpdatableColumns("version", "validity", "resources") + .withUpdatableColumns("version", "caption", "validity", "resources") .importEntityAlias("debitor", HsOfficeDebitorEntity.class, dependsOnColumn("debitorUuid"), @@ -167,9 +167,12 @@ public class HsBookingItemEntity implements Stringifyable, RbacObject { .createRole(OWNER, (with) -> { with.incomingSuperRole("debitorRel", AGENT); + }) + .createSubRole(ADMIN, (with) -> { + with.incomingSuperRole("debitorRel", AGENT); with.permission(UPDATE); }) - .createSubRole(ADMIN) + .createSubRole(AGENT) .createSubRole(TENANT, (with) -> { with.outgoingSubRole("debitorRel", TENANT); with.permission(SELECT); diff --git a/src/main/resources/api-definition/hs-booking/hs-booking-item-schemas.yaml b/src/main/resources/api-definition/hs-booking/hs-booking-item-schemas.yaml index 06f8b921..4d146683 100644 --- a/src/main/resources/api-definition/hs-booking/hs-booking-item-schemas.yaml +++ b/src/main/resources/api-definition/hs-booking/hs-booking-item-schemas.yaml @@ -48,7 +48,7 @@ components: caption: type: string minLength: 3 - maxLength: + maxLength: 80 nullable: false validFrom: type: string @@ -75,11 +75,6 @@ components: ManagedServerBookingResources: type: object properties: - caption: - type: string - minLength: 3 - maxLength: - nullable: false CPU: type: integer minimum: 1 diff --git a/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6013-hs-booking-item-rbac.md b/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6013-hs-booking-item-rbac.md index 1acb787d..5cc8616f 100644 --- a/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6013-hs-booking-item-rbac.md +++ b/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6013-hs-booking-item-rbac.md @@ -95,6 +95,7 @@ subgraph bookingItem["`**bookingItem**`"] role:bookingItem:OWNER[[bookingItem:OWNER]] role:bookingItem:ADMIN[[bookingItem:ADMIN]] + role:bookingItem:AGENT[[bookingItem:AGENT]] role:bookingItem:TENANT[[bookingItem:TENANT]] end @@ -273,13 +274,15 @@ role:debitorRel.anchorPerson:ADMIN -.-> role:debitorRel:OWNER role:debitorRel.holderPerson:ADMIN -.-> role:debitorRel:AGENT role:debitorRel:AGENT ==> role:bookingItem:OWNER role:bookingItem:OWNER ==> role:bookingItem:ADMIN -role:bookingItem:ADMIN ==> role:bookingItem:TENANT +role:debitorRel:AGENT ==> role:bookingItem:ADMIN +role:bookingItem:ADMIN ==> role:bookingItem:AGENT +role:bookingItem:AGENT ==> role:bookingItem:TENANT role:bookingItem:TENANT ==> role:debitorRel:TENANT %% granting permissions to roles role:debitorRel:ADMIN ==> perm:bookingItem:INSERT role:global:ADMIN ==> perm:bookingItem:DELETE -role:bookingItem:OWNER ==> perm:bookingItem:UPDATE +role:bookingItem:ADMIN ==> perm:bookingItem:UPDATE role:bookingItem:TENANT ==> perm:bookingItem:SELECT ``` diff --git a/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6013-hs-booking-item-rbac.sql b/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6013-hs-booking-item-rbac.sql index 590fef50..b2add620 100644 --- a/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6013-hs-booking-item-rbac.sql +++ b/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6013-hs-booking-item-rbac.sql @@ -49,19 +49,26 @@ begin perform createRoleWithGrants( hsBookingItemOWNER(NEW), - permissions => array['UPDATE'], incomingSuperRoles => array[hsOfficeRelationAGENT(newDebitorRel)] ); perform createRoleWithGrants( hsBookingItemADMIN(NEW), - incomingSuperRoles => array[hsBookingItemOWNER(NEW)] + permissions => array['UPDATE'], + incomingSuperRoles => array[ + hsBookingItemOWNER(NEW), + hsOfficeRelationAGENT(newDebitorRel)] + ); + + perform createRoleWithGrants( + hsBookingItemAGENT(NEW), + incomingSuperRoles => array[hsBookingItemADMIN(NEW)] ); perform createRoleWithGrants( hsBookingItemTENANT(NEW), permissions => array['SELECT'], - incomingSuperRoles => array[hsBookingItemADMIN(NEW)], + incomingSuperRoles => array[hsBookingItemAGENT(NEW)], outgoingSubRoles => array[hsOfficeRelationTENANT(newDebitorRel)] ); @@ -177,9 +184,9 @@ create trigger hs_booking_item_insert_permission_check_tg call generateRbacIdentityViewFromQuery('hs_booking_item', $idName$ - SELECT i.uuid as uuid, d.idName || ':' || i.caption as idName - FROM hs_booking_item i - JOIN hs_office_debitor_iv d ON d.uuid = i.debitorUuid + SELECT bookingItem.uuid as uuid, debitorIV.idName || '-' || cleanIdentifier(bookingItem.caption) as idName + FROM hs_booking_item bookingItem + JOIN hs_office_debitor_iv debitorIV ON debitorIV.uuid = bookingItem.debitorUuid $idName$); --// @@ -192,6 +199,7 @@ call generateRbacRestrictedView('hs_booking_item', $orderBy$, $updates$ version = new.version, + caption = new.caption, validity = new.validity, resources = new.resources $updates$); diff --git a/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6018-hs-booking-item-test-data.sql b/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6018-hs-booking-item-test-data.sql index 6326ce7f..38b80d6b 100644 --- a/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6018-hs-booking-item-test-data.sql +++ b/src/main/resources/db/changelog/6-hs-booking/601-booking-item/6018-hs-booking-item-test-data.sql @@ -34,7 +34,7 @@ begin into hs_booking_item (uuid, debitoruuid, caption, validity, resources) values (uuid_generate_v4(), relatedDebitor.uuid, 'some ManagedServer', daterange('20221001', null, '[]'), '{ "CPU": 2, "SDD": 512, "extra": 42 }'::jsonb), (uuid_generate_v4(), relatedDebitor.uuid, 'some CloudServer', daterange('20230115', '20240415', '[)'), '{ "CPU": 2, "HDD": 1024, "extra": 42 }'::jsonb), - (uuid_generate_v4(), relatedDebitor.uuid, 'some Whatever', daterange('20240401', null, '[]'), '{ "CPU": 1, "SDD": 512, "HDD": 2048, "extra": 42 }'::jsonb); + (uuid_generate_v4(), relatedDebitor.uuid, 'some PrivateCloud', daterange('20240401', null, '[]'), '{ "CPU": 10, "SDD": 10240, "HDD": 10240, "extra": 42 }'::jsonb); end; $$; --// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemControllerAcceptanceTest.java index 61e533b4..126aa966 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemControllerAcceptanceTest.java @@ -89,13 +89,13 @@ class HsBookingItemControllerAcceptanceTest extends ContextBasedTestWithCleanup } }, { - "caption": "some Whatever", + "caption": "some PrivateCloud", "validFrom": "2024-04-01", "validTo": null, "resources": { - "CPU": 1, - "HDD": 2048, - "SDD": 512, + "CPU": 10, + "HDD": 10240, + "SDD": 10240, "extra": 42 } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemRepositoryIntegrationTest.java index 9a5eaf00..7dc92ebd 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemRepositoryIntegrationTest.java @@ -109,28 +109,35 @@ class HsBookingItemRepositoryIntegrationTest extends ContextBasedTestWithCleanup final var all = rawRoleRepo.findAll(); assertThat(distinctRoleNamesOf(all)).containsExactlyInAnyOrder(Array.from( initialRoleNames, - "hs_booking_item#D-1000111:some new booking item:ADMIN", - "hs_booking_item#D-1000111:some new booking item:OWNER", - "hs_booking_item#D-1000111:some new booking item:TENANT")); + "hs_booking_item#D-1000111-somenewbookingitem:ADMIN", + "hs_booking_item#D-1000111-somenewbookingitem:AGENT", + "hs_booking_item#D-1000111-somenewbookingitem:OWNER", + "hs_booking_item#D-1000111-somenewbookingitem:TENANT")); assertThat(distinctGrantDisplaysOf(rawGrantRepo.findAll())) .map(s -> s.replace("hs_office_", "")) .containsExactlyInAnyOrder(fromFormatted( initialGrantNames, // insert+delete - "{ grant perm:hs_booking_item#D-1000111:some new booking item:DELETE to role:global#global:ADMIN by system and assume }", + "{ grant perm:hs_booking_item#D-1000111-somenewbookingitem:DELETE to role:global#global:ADMIN by system and assume }", // owner - "{ grant perm:hs_booking_item#D-1000111:some new booking item:UPDATE to role:hs_booking_item#D-1000111:some new booking item:OWNER by system and assume }", - "{ grant role:hs_booking_item#D-1000111:some new booking item:OWNER to role:relation#FirstGmbH-with-DEBITOR-FirstGmbH:AGENT by system and assume }", + //"{ grant perm:hs_booking_item#D-1000111-somenewbookingitem:UPDATE to role:hs_booking_item#D-1000111-somenewbookingitem:OWNER by system and assume }", + "{ grant role:hs_booking_item#D-1000111-somenewbookingitem:OWNER to role:relation#FirstGmbH-with-DEBITOR-FirstGmbH:AGENT by system and assume }", // admin - "{ grant role:hs_booking_item#D-1000111:some new booking item:ADMIN to role:hs_booking_item#D-1000111:some new booking item:OWNER by system and assume }", + "{ grant perm:hs_booking_item#D-1000111-somenewbookingitem:UPDATE to role:hs_booking_item#D-1000111-somenewbookingitem:ADMIN by system and assume }", + "{ grant role:hs_booking_item#D-1000111-somenewbookingitem:ADMIN to role:hs_booking_item#D-1000111-somenewbookingitem:OWNER by system and assume }", + //"{ grant role:hs_booking_item#D-1000111-somenewbookingitem:TENANT to role:hs_booking_item#D-1000111-somenewbookingitem:ADMIN by system and assume }", + + // agent + "{ grant role:hs_booking_item#D-1000111-somenewbookingitem:ADMIN to role:relation#FirstGmbH-with-DEBITOR-FirstGmbH:AGENT by system and assume }", + "{ grant role:hs_booking_item#D-1000111-somenewbookingitem:AGENT to role:hs_booking_item#D-1000111-somenewbookingitem:ADMIN by system and assume }", // tenant - "{ grant role:hs_booking_item#D-1000111:some new booking item:TENANT to role:hs_booking_item#D-1000111:some new booking item:ADMIN by system and assume }", - "{ grant perm:hs_booking_item#D-1000111:some new booking item:SELECT to role:hs_booking_item#D-1000111:some new booking item:TENANT by system and assume }", - "{ grant role:relation#FirstGmbH-with-DEBITOR-FirstGmbH:TENANT to role:hs_booking_item#D-1000111:some new booking item:TENANT by system and assume }", + "{ grant role:hs_booking_item#D-1000111-somenewbookingitem:TENANT to role:hs_booking_item#D-1000111-somenewbookingitem:AGENT by system and assume }", + "{ grant perm:hs_booking_item#D-1000111-somenewbookingitem:SELECT to role:hs_booking_item#D-1000111-somenewbookingitem:TENANT by system and assume }", + "{ grant role:relation#FirstGmbH-with-DEBITOR-FirstGmbH:TENANT to role:hs_booking_item#D-1000111-somenewbookingitem:TENANT by system and assume }", null)); } @@ -159,7 +166,7 @@ class HsBookingItemRepositoryIntegrationTest extends ContextBasedTestWithCleanup result, "HsBookingItemEntity(D-1000212, [2022-10-01,), some ManagedServer, { CPU: 2, SDD: 512, extra: 42 })", "HsBookingItemEntity(D-1000212, [2023-01-15,2024-04-15), some CloudServer, { CPU: 2, HDD: 1024, extra: 42 })", - "HsBookingItemEntity(D-1000212, [2024-04-01,), some Whatever, { CPU: 1, HDD: 2048, SDD: 512, extra: 42 })"); + "HsBookingItemEntity(D-1000212, [2024-04-01,), some PrivateCloud, { CPU: 10, HDD: 10240, SDD: 10240, extra: 42 })"); } @Test @@ -176,7 +183,7 @@ class HsBookingItemRepositoryIntegrationTest extends ContextBasedTestWithCleanup result, "HsBookingItemEntity(D-1000111, [2022-10-01,), some ManagedServer, { CPU: 2, SDD: 512, extra: 42 })", "HsBookingItemEntity(D-1000111, [2023-01-15,2024-04-15), some CloudServer, { CPU: 2, HDD: 1024, extra: 42 })", - "HsBookingItemEntity(D-1000111, [2024-04-01,), some Whatever, { CPU: 1, HDD: 2048, SDD: 512, extra: 42 })"); + "HsBookingItemEntity(D-1000111, [2024-04-01,), some PrivateCloud, { CPU: 10, HDD: 10240, SDD: 10240, extra: 42 })"); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java index c436e34a..7e09519c 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java @@ -141,6 +141,7 @@ class HsOfficePartnerRepositoryIntegrationTest extends ContextBasedTestWithClean .map(s -> s.replace("hs_office_", "")) .containsExactlyInAnyOrder(distinct(from( initialGrantNames, + // TODO.rbac: this grant should only be created for DEBITOR-Relationships, thus the RBAC DSL needs to support conditional grants "{ grant perm:relation#HostsharingeG-with-PARTNER-EBess:INSERT>sepamandate to role:relation#HostsharingeG-with-PARTNER-EBess:ADMIN by system and assume }", // permissions on partner diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java index 1c745bb8..8e632c21 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java @@ -131,7 +131,7 @@ class HsOfficeRelationRepositoryIntegrationTest extends ContextBasedTestWithClea "hs_office_relation#ErbenBesslerMelBessler-with-REPRESENTATIVE-BesslerBert:TENANT")); assertThat(distinctGrantDisplaysOf(rawGrantRepo.findAll())).containsExactlyInAnyOrder(Array.fromFormatted( initialGrantNames, - // TODO: this grant should only be created for DEBITOR-Relationships, thus the RBAC DSL needs to support conditional grants + // TODO.rbac: this grant should only be created for DEBITOR-Relationships, thus the RBAC DSL needs to support conditional grants "{ grant perm:hs_office_relation#ErbenBesslerMelBessler-with-REPRESENTATIVE-BesslerBert:INSERT>hs_office_sepamandate to role:hs_office_relation#ErbenBesslerMelBessler-with-REPRESENTATIVE-BesslerBert:ADMIN by system and assume }", "{ grant perm:hs_office_relation#ErbenBesslerMelBessler-with-REPRESENTATIVE-BesslerBert:DELETE to role:hs_office_relation#ErbenBesslerMelBessler-with-REPRESENTATIVE-BesslerBert:OWNER by system and assume }",