Taiga459: make projects visible to debitor despite unassumed grant (#221)
Co-authored-by: Michael Hoennig <michael@hoennig.de> Reviewed-on: http://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/221 Reviewed-by: Stefan Begerad <stefan.begerad@hostsharing.net>
This commit is contained in:
@@ -0,0 +1,78 @@
|
|||||||
|
# PR#221: Make projects visible to debitors despite unassumed grant
|
||||||
|
|
||||||
|
See also [Taiga#459: Project per Backend API nicht sichtbar](https://plan.hostsharing.net/project/admin-hsadmin/us/459) - just for internal tracking.
|
||||||
|
|
||||||
|
## The Problem
|
||||||
|
|
||||||
|
Even though, according to the database, there is at least one project for a given debitor,
|
||||||
|
it cannot be fetched via API:
|
||||||
|
|
||||||
|
```
|
||||||
|
curl --no-progress-meter -X 'GET' \
|
||||||
|
'http://127.0.0.1:<port>/api/hs/booking/projects?debitorUuid=<some-existing-debitorUuid>' \
|
||||||
|
-H "Authorization: Bearer $BEARER" \
|
||||||
|
|jq|less
|
||||||
|
```
|
||||||
|
|
||||||
|
See that the response is an empty JSON array.
|
||||||
|
|
||||||
|
|
||||||
|
## The Cause
|
||||||
|
|
||||||
|
To avoid that too many objects are visible at once, which might confuse the user and slows the ReBAC-system down,
|
||||||
|
the grant from a Debitor:AGENT to Project:OWNER is unassumed.
|
||||||
|
Unassumed means that the grant is only effective if the specific role (here Project:OWNER) is explicitly assumed.
|
||||||
|
|
||||||
|
Unfortunately, here we have the problem that it's hard to assume the role of an object which cannot even be fetched,
|
||||||
|
as we need the UUID of the object to assume the role.
|
||||||
|
|
||||||
|
|
||||||
|
## The Solution
|
||||||
|
|
||||||
|
The solution is to add a role Project:REFERRER, which is getting granted to the Debitor:AGENT and auto-assumed.
|
||||||
|
By this grant, the project becomes visible for debitors, not yet anything below the project.
|
||||||
|
Usually there are not that many projects, just on the next level there might be very many objects;
|
||||||
|
thus such a grant is not a problem.
|
||||||
|
Just for anything below, the owner role still needs to be assumed, which now is possible,
|
||||||
|
as the UUID of the debitor's project(s) is now known.
|
||||||
|
|
||||||
|
This change is to be applied in the ReBAC-DSL of the HsBookingProjectRbacEntity:
|
||||||
|
|
||||||
|
```
|
||||||
|
// existing role
|
||||||
|
.createSubRole(TENANT, (with) -> {
|
||||||
|
with.outgoingSubRole("debitorRel", TENANT);
|
||||||
|
// with.permission(SELECT); moved to the REFERRER role
|
||||||
|
})
|
||||||
|
// new role
|
||||||
|
.createSubRole(REFERRER, (with) -> {
|
||||||
|
// make the project visible for debitors, but for anything below, the owner role needs to be assumed
|
||||||
|
with.incomingSuperRole("debitorRel", AGENT);
|
||||||
|
with.permission(SELECT);
|
||||||
|
})
|
||||||
|
```
|
||||||
|
|
||||||
|
Also have a look at the [updated diagram](../../src/main/resources/db/changelog/6-hs-booking/620-booking-project/6203-hs-booking-project-rbac.md)
|
||||||
|
|
||||||
|
The new test-cases:
|
||||||
|
|
||||||
|
```
|
||||||
|
@ValueSource(strings = {
|
||||||
|
"hs_office.relation#FirstGmbH-with-DEBITOR-FirstGmbH:ADMIN", // the debitor:ADMIN, failed before
|
||||||
|
"hs_booking.project#D-1000111-D-1000111defaultproject:OWNER", // the project:OWNER, worked before
|
||||||
|
"" // without any assumed-roles - failed before as well
|
||||||
|
})
|
||||||
|
void debitorAdminUser_canGetRelatedBookingProjectEvenWithoutAssumingTheProjectRole(final String assumedRoles)
|
||||||
|
```
|
||||||
|
|
||||||
|
### **Attention:** This change does not directly apply to existing data.
|
||||||
|
|
||||||
|
First, we needed to re-generate the PostgreSQL code by running
|
||||||
|
`net.hostsharing.hsadminng.hs.booking.project.HsBookingProjectRbacEntity.main`.
|
||||||
|
The generated PostgreSQL is part of the PR and applied directly to automated tests which use the Testcontainers PostgreSQL.
|
||||||
|
|
||||||
|
But furthermore, we either need to-regenerate the roles+grants for all projects or,
|
||||||
|
as there is no production data for booking+hosting yet, simply re-generate the whole test-data.
|
||||||
|
|
||||||
|
Now use the role-ID "hs_booking.project#<uuid-of-the-project>:OWNER" in the "assumed-roles" header.
|
||||||
|
This header should probably get renamed to "X-assumed-roles" or "X-hsadmin-ng-rbac-assumed-roles".
|
||||||
+6
-1
@@ -26,6 +26,7 @@ import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.Permission.UPDAT
|
|||||||
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.Role.ADMIN;
|
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.Role.ADMIN;
|
||||||
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.Role.AGENT;
|
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.Role.AGENT;
|
||||||
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.Role.OWNER;
|
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.Role.OWNER;
|
||||||
|
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.Role.REFERRER;
|
||||||
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.Role.TENANT;
|
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.Role.TENANT;
|
||||||
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.SQL.directlyFetchedByDependsOnColumn;
|
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.SQL.directlyFetchedByDependsOnColumn;
|
||||||
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.SQL.fetchedBySql;
|
import static net.hostsharing.hsadminng.rbac.generator.RbacSpec.SQL.fetchedBySql;
|
||||||
@@ -72,9 +73,13 @@ public class HsBookingProjectRbacEntity extends HsBookingProject {
|
|||||||
.createSubRole(ADMIN, (with) -> {
|
.createSubRole(ADMIN, (with) -> {
|
||||||
with.permission(UPDATE);
|
with.permission(UPDATE);
|
||||||
})
|
})
|
||||||
.createSubRole(AGENT)
|
.createSubRole(AGENT) // just for manual grants
|
||||||
.createSubRole(TENANT, (with) -> {
|
.createSubRole(TENANT, (with) -> {
|
||||||
with.outgoingSubRole("debitorRel", TENANT);
|
with.outgoingSubRole("debitorRel", TENANT);
|
||||||
|
})
|
||||||
|
.createSubRole(REFERRER, (with) -> {
|
||||||
|
// make the project visible for debitors, but for anything below, the owner role needs to be assumed
|
||||||
|
with.incomingSuperRole("debitorRel", AGENT);
|
||||||
with.permission(SELECT);
|
with.permission(SELECT);
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
+4
-1
@@ -31,6 +31,7 @@ subgraph project["`**project**`"]
|
|||||||
role:project:ADMIN[[project:ADMIN]]
|
role:project:ADMIN[[project:ADMIN]]
|
||||||
role:project:AGENT[[project:AGENT]]
|
role:project:AGENT[[project:AGENT]]
|
||||||
role:project:TENANT[[project:TENANT]]
|
role:project:TENANT[[project:TENANT]]
|
||||||
|
role:project:REFERRER[[project:REFERRER]]
|
||||||
end
|
end
|
||||||
|
|
||||||
subgraph project:permissions[ ]
|
subgraph project:permissions[ ]
|
||||||
@@ -53,11 +54,13 @@ role:project:OWNER ==> role:project:ADMIN
|
|||||||
role:project:ADMIN ==> role:project:AGENT
|
role:project:ADMIN ==> role:project:AGENT
|
||||||
role:project:AGENT ==> role:project:TENANT
|
role:project:AGENT ==> role:project:TENANT
|
||||||
role:project:TENANT ==> role:debitorRel:TENANT
|
role:project:TENANT ==> role:debitorRel:TENANT
|
||||||
|
role:project:TENANT ==> role:project:REFERRER
|
||||||
|
role:debitorRel:AGENT ==> role:project:REFERRER
|
||||||
|
|
||||||
%% granting permissions to roles
|
%% granting permissions to roles
|
||||||
role:debitorRel:ADMIN ==> perm:project:INSERT
|
role:debitorRel:ADMIN ==> perm:project:INSERT
|
||||||
role:rbac.global:ADMIN ==> perm:project:DELETE
|
role:rbac.global:ADMIN ==> perm:project:DELETE
|
||||||
role:project:ADMIN ==> perm:project:UPDATE
|
role:project:ADMIN ==> perm:project:UPDATE
|
||||||
role:project:TENANT ==> perm:project:SELECT
|
role:project:REFERRER ==> perm:project:SELECT
|
||||||
|
|
||||||
```
|
```
|
||||||
|
|||||||
+10
-3
@@ -65,11 +65,18 @@ begin
|
|||||||
|
|
||||||
perform rbac.defineRoleWithGrants(
|
perform rbac.defineRoleWithGrants(
|
||||||
hs_booking.project_TENANT(NEW),
|
hs_booking.project_TENANT(NEW),
|
||||||
permissions => array['SELECT'],
|
|
||||||
incomingSuperRoles => array[hs_booking.project_AGENT(NEW)],
|
incomingSuperRoles => array[hs_booking.project_AGENT(NEW)],
|
||||||
outgoingSubRoles => array[hs_office.relation_TENANT(newDebitorRel)]
|
outgoingSubRoles => array[hs_office.relation_TENANT(newDebitorRel)]
|
||||||
);
|
);
|
||||||
|
|
||||||
|
perform rbac.defineRoleWithGrants(
|
||||||
|
hs_booking.project_REFERRER(NEW),
|
||||||
|
permissions => array['SELECT'],
|
||||||
|
incomingSuperRoles => array[
|
||||||
|
hs_booking.project_TENANT(NEW),
|
||||||
|
hs_office.relation_AGENT(newDebitorRel)]
|
||||||
|
);
|
||||||
|
|
||||||
call rbac.grantPermissionToRole(rbac.createPermission(NEW.uuid, 'DELETE'), rbac.global_ADMIN());
|
call rbac.grantPermissionToRole(rbac.createPermission(NEW.uuid, 'DELETE'), rbac.global_ADMIN());
|
||||||
|
|
||||||
call rbac.leaveTriggerForObjectUuid(NEW.uuid);
|
call rbac.leaveTriggerForObjectUuid(NEW.uuid);
|
||||||
@@ -194,7 +201,6 @@ call rbac.generateRbacIdentityViewFromQuery('hs_booking.project',
|
|||||||
-- ============================================================================
|
-- ============================================================================
|
||||||
--changeset RbacRestrictedViewGenerator:hs-booking-project-rbac-RESTRICTED-VIEW runOnChange:true validCheckSum:ANY endDelimiter:--//
|
--changeset RbacRestrictedViewGenerator:hs-booking-project-rbac-RESTRICTED-VIEW runOnChange:true validCheckSum:ANY endDelimiter:--//
|
||||||
-- ----------------------------------------------------------------------------
|
-- ----------------------------------------------------------------------------
|
||||||
-- trigger change of change in generateRbacRestrictedView regarding #453 optimization for global:ADMIN
|
|
||||||
call rbac.generateRbacRestrictedView('hs_booking.project',
|
call rbac.generateRbacRestrictedView('hs_booking.project',
|
||||||
$orderBy$
|
$orderBy$
|
||||||
caption
|
caption
|
||||||
@@ -202,7 +208,8 @@ call rbac.generateRbacRestrictedView('hs_booking.project',
|
|||||||
$updates$
|
$updates$
|
||||||
version = new.version,
|
version = new.version,
|
||||||
caption = new.caption
|
caption = new.caption
|
||||||
$updates$);
|
$updates$
|
||||||
|
);
|
||||||
--//
|
--//
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+32
@@ -9,6 +9,8 @@ import net.hostsharing.hsadminng.rbac.test.JpaAttempt;
|
|||||||
import org.junit.jupiter.api.Nested;
|
import org.junit.jupiter.api.Nested;
|
||||||
import org.junit.jupiter.api.Tag;
|
import org.junit.jupiter.api.Tag;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
|
import org.junit.jupiter.params.ParameterizedTest;
|
||||||
|
import org.junit.jupiter.params.provider.ValueSource;
|
||||||
import org.springframework.beans.factory.annotation.Autowired;
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
import org.springframework.boot.test.context.SpringBootTest;
|
import org.springframework.boot.test.context.SpringBootTest;
|
||||||
import org.springframework.boot.test.web.server.LocalServerPort;
|
import org.springframework.boot.test.web.server.LocalServerPort;
|
||||||
@@ -185,6 +187,36 @@ class HsBookingProjectControllerAcceptanceTest extends ContextBasedTestWithClean
|
|||||||
}
|
}
|
||||||
""")); // @formatter:on
|
""")); // @formatter:on
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ParameterizedTest
|
||||||
|
@ValueSource(strings = {
|
||||||
|
"hs_office.relation#FirstGmbH-with-DEBITOR-FirstGmbH:ADMIN",
|
||||||
|
"hs_booking.project#D-1000111-D-1000111defaultproject:OWNER",
|
||||||
|
"" // without any assumed-roles
|
||||||
|
})
|
||||||
|
void debitorAdminUser_canGetRelatedBookingProjectEvenWithoutAssumingTheProjectRole(final String assumedRoles) {
|
||||||
|
context.define("superuser-alex@hostsharing.net");
|
||||||
|
final var debitorUuid = debitorRepo.findByDebitorNumber(1000111).stream()
|
||||||
|
.findAny().orElseThrow().getUuid();
|
||||||
|
|
||||||
|
RestAssured // @formatter:off
|
||||||
|
.given()
|
||||||
|
.header("Authorization", bearer("person-FirstGmbH@example.com"))
|
||||||
|
.header("assumed-roles", assumedRoles)
|
||||||
|
.port(port)
|
||||||
|
.when()
|
||||||
|
.get("http://localhost/api/hs/booking/projects?debitorUuid=" + debitorUuid)
|
||||||
|
.then().log().all().assertThat()
|
||||||
|
.statusCode(200)
|
||||||
|
.contentType("application/json")
|
||||||
|
.body("", lenientlyEquals("""
|
||||||
|
[
|
||||||
|
{
|
||||||
|
"caption": "D-1000111 default project"
|
||||||
|
}
|
||||||
|
]
|
||||||
|
""")); // @formatter:on
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Nested
|
@Nested
|
||||||
|
|||||||
Reference in New Issue
Block a user