Merge pull request 'fix booking item identity view and some other minor issues' (#47) from fix-booking-item-identity-view into master

Reviewed-on: #47
Reviewed-by: Marc Sandlus <marc.sandlus@hostsharing.net>

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);

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

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
 
 ```

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$);

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; $$;
 --//
 

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
                              }
                          }

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 })");
         }
     }
 

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

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 }",