Skip to content

Commit

Permalink
Fix logic to detect readable relations between user and group elements,
Browse files Browse the repository at this point in the history 
fixes #188. Uncommented and fixed assertions in feature tests, related to #188.
  • Loading branch information
@Syndesi
Syndesi committed Dec 8, 2023
1 parent 4bb28d9 commit 3527e43
Show file tree
Hide file tree
Showing 18 changed files with 292 additions and 74 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Upgrade dependencies, e.g. Symfony to 7.0 and intermediate Neo4j PHP dependencies as far as possible.
- Deactivate `composer mess` in CI due to upstream issue, tracked in #203.
- Upgrade to Alpine 3.19, closes #205.
### Fixed
- Fix logic to detect readable relations between user and group elements, fixes #188.
- Uncommented and fixed assertions in feature tests, related to #188.

## 0.0.37 - 2023-11-24
### Changed
Expand Down
120 changes: 118 additions & 2 deletions src/Security/AccessChecker.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ public function hasAccessToElement(UuidInterface $userUuid, UuidInterface $eleme
return $this->hasGeneralAccessToNode($userUuid, $elementUuid, $accessType);
}
} else {
return $this->hasAccessToRelation($userUuid, $elementUuid, $accessType);
if (AccessType::READ === $accessType) {
return $this->hasReadAccessToRelation($userUuid, $elementUuid);
} else {
return $this->hasGeneralAccessToRelation($userUuid, $elementUuid, $accessType);
}
}
}

Expand Down Expand Up @@ -172,7 +176,119 @@ public function hasGeneralAccessToNode(UuidInterface $userUuid, UuidInterface $e
return true;
}

public function hasAccessToRelation(UuidInterface $userUuid, UuidInterface $elementUuid, AccessType $accessType): bool
public function hasReadAccessToRelation(UuidInterface $userUuid, UuidInterface $elementUuid): bool
{
$res = $this->cypherEntityManager->getClient()->runStatement(Statement::create(
"MATCH (user:User {id: \$userId})\n".
"MATCH (start)-[element {id: \$elementId}]->(end)\n".
'OPTIONAL MATCH (user)-[:IS_IN_GROUP*0..]->()-[startRelations:OWNS|HAS_'.AccessType::READ->value."_ACCESS*0..]->(start)\n".
'OPTIONAL MATCH (user)-[:IS_IN_GROUP*0..]->()-[endRelations:OWNS|HAS_'.AccessType::READ->value."_ACCESS*0..]->(end)\n".
"WHERE\n".
" (\n".
" user.id = start.id\n".
" OR\n".
" ALL(startRelation in startRelations WHERE\n".
" type(startRelation) = \"OWNS\"\n".
" OR\n".
" (\n".
' type(startRelation) = "HAS_'.AccessType::READ->value."_ACCESS\"\n".
" AND\n".
" (\n".
" startRelation.onLabel IS NULL\n".
" OR\n".
" startRelation.onLabel IN labels(start)\n".
" )\n".
" AND\n".
" (\n".
" startRelation.onParentLabel IS NULL\n".
" OR\n".
" startRelation.onParentLabel IN labels(start)\n".
" )\n".
" AND\n".
" (\n".
" startRelation.onState IS NULL\n".
" OR\n".
" (start)<-[:OWNS*0..]-()-[:HAS_STATE]->(:State {id: startRelation.onState})\n".
" )\n".
" AND\n".
" (\n".
" startRelation.onCreatedByUser IS NULL\n".
" OR\n".
" (start)<-[:CREATED_BY*]-(user)\n".
" )\n".
" )\n".
" )\n".
" )\n".
" AND\n".
" (\n".
" user.id = end.id\n".
" OR \n".
" ALL(endRelation in endRelations WHERE\n".
" type(endRelation) = \"OWNS\"\n".
" OR\n".
" (\n".
" (\n".
" type(endRelation) = \"HAS_READ_ACCESS\"\n".
" OR\n".
' type(endRelation) = "'.AccessType::READ->value."\"\n".
" )\n".
" AND\n".
" (\n".
" endRelation.onLabel IS NULL\n".
" OR\n".
" endRelation.onLabel IN labels(end)\n".
" )\n".
" AND\n".
" (\n".
" endRelation.onParentLabel IS NULL\n".
" OR\n".
" endRelation.onParentLabel IN labels(end)\n".
" )\n".
" AND\n".
" (\n".
" endRelation.onState IS NULL\n".
" OR\n".
" (end)<-[:OWNS*0..]-()-[:HAS_STATE]->(:State {id: endRelation.onState})\n".
" )\n".
" AND\n".
" (\n".
" endRelation.onCreatedByUser IS NULL\n".
" OR\n".
" (end)<-[:CREATED_BY*]-(user)\n".
" )\n".
" )\n".
" )\n".
" )\n".
"WITH user, element, start, end, startRelations, endRelations\n".
"WHERE\n".
" (\n".
" user.id = start.id\n".
" OR\n".
" startRelations IS NOT NULL\n".
" )\n".
" AND\n".
" (\n".
" user.id = end.id\n".
" OR\n".
" endRelations IS NOT NULL\n".
" )\n".
'RETURN count(*) as count;',
[
'userId' => $userUuid->toString(),
'elementId' => $elementUuid->toString(),
]
));
if (1 !== $res->count()) {
return false;
}
if (0 === $res->first()->get('count')) {
return false;
}

return true;
}

public function hasGeneralAccessToRelation(UuidInterface $userUuid, UuidInterface $elementUuid, AccessType $accessType): bool
{
$res = $this->cypherEntityManager->getClient()->runStatement(Statement::create(
"MATCH (user:User {id: \$userId})\n".
Expand Down
4 changes: 2 additions & 2 deletions tests/FeatureTests/Security/_02_02_ImmediateRelationOwnershipTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ public function testGetIndex(): void
$this->assertIsCollectionResponse($response);
}

public function testGetAllowedNode(): void
public function testGetAllowedNodes(): void
{
$response = $this->runGetRequest(sprintf('/%s', self::DATA_1), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::DATA_2), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
}

public function testGetAllowedRelation(): void
public function testGetAllowedRelations(): void
{
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_1), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
Expand Down
2 changes: 1 addition & 1 deletion tests/FeatureTests/Security/_02_03_PathNodeOwnershipTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public function testGetIndex(): void
$this->assertIsCollectionResponse($response);
}

public function testGetAllowedNode(): void
public function testGetAllowedNodes(): void
{
$response = $this->runGetRequest(sprintf('/%s', self::DATA_1), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
Expand Down
2 changes: 1 addition & 1 deletion tests/FeatureTests/Security/_02_04_PathRelationOwnershipTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public function testGetIndex(): void
$this->assertIsCollectionResponse($response);
}

public function testGetAllowedNode(): void
public function testGetAllowedNodes(): void
{
$response = $this->runGetRequest(sprintf('/%s', self::DATA_1), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
Expand Down
19 changes: 16 additions & 3 deletions tests/FeatureTests/Security/_03_01_GroupImmediateNodeOwnershipTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,31 @@ class _03_01_GroupImmediateNodeOwnershipTest extends BaseRequestTestCase

public const GROUP = 'f95c84f8-c33a-410a-93be-da0d65c1f0de';
public const DATA = '56acb0c6-aaed-451f-b277-bceac61d7632';
public const IS_IN_GROUP = 'f656b91e-6c2f-4dda-a4ff-69717e0b4559';
public const OWNS_DATA = 'c76a7d30-2250-4bb3-bffd-e5f83a2ccd5b';
public const OWNS_TOKEN = '2541a4d1-73f1-4716-8de8-2caf3c943d15';

public function testGetIndex(): void
{
$response = $this->runGetRequest('/', self::TOKEN);
$this->assertIsCollectionResponse($response);
}

public function testGetAllowedNode(): void
public function testGetAllowedNodes(): void
{
// $response = $this->runGetRequest(sprintf('/%s', self::GROUP), self::TOKEN);
// $this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::GROUP), self::TOKEN);
$this->assertIsNodeResponse($response, 'Group');
$response = $this->runGetRequest(sprintf('/%s', self::DATA), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
}

public function testGetAllowedRelations(): void
{
$response = $this->runGetRequest(sprintf('/%s', self::IS_IN_GROUP), self::TOKEN);
$this->assertIsRelationResponse($response, 'IS_IN_GROUP');
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_DATA), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_TOKEN), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
}
}
25 changes: 22 additions & 3 deletions tests/FeatureTests/Security/_03_02_GroupImmediateRelationOwnershipTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,39 @@ class _03_02_GroupImmediateRelationOwnershipTest extends BaseRequestTestCase
public const GROUP = 'f935bed2-cf13-406b-8ad2-0a943b9329f1';
public const DATA_1 = 'bc69b38c-d087-4355-97a9-4e4ec48e4810';
public const DATA_2 = '04cc4124-8b39-41ad-979c-e389e7c73fc6';
public const IS_IN_GROUP = 'c2e419ef-998c-48c6-8af3-e9db2b9bff17';
public const OWNS_TOKEN = '2643d165-8e0f-4978-b344-c360ce63ae41';
public const OWNS_DATA_1 = 'a8805631-5a83-49cb-af8b-628ff727b6d4';
public const OWNS_DATA_2 = '0bfd0925-75e1-4574-8cab-0e97267e7052';
public const RELATION = '704eac29-cda1-441f-93b4-069df785b9c2';

public function testGetIndex(): void
{
$response = $this->runGetRequest('/', self::TOKEN);
$this->assertIsCollectionResponse($response);
}

public function testGetAllowedNode(): void
public function testGetAllowedNodes(): void
{
// $response = $this->runGetRequest(sprintf('/%s', self::GROUP), self::TOKEN);
// $this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::GROUP), self::TOKEN);
$this->assertIsNodeResponse($response, 'Group');
$response = $this->runGetRequest(sprintf('/%s', self::DATA_1), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::DATA_2), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
}

public function testGetAllowedRelations(): void
{
$response = $this->runGetRequest(sprintf('/%s', self::IS_IN_GROUP), self::TOKEN);
$this->assertIsRelationResponse($response, 'IS_IN_GROUP');
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_TOKEN), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_DATA_1), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_DATA_2), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
$response = $this->runGetRequest(sprintf('/%s', self::RELATION), self::TOKEN);
$this->assertIsRelationResponse($response, 'RELATION');
}
}
6 changes: 3 additions & 3 deletions tests/FeatureTests/Security/_03_03_GroupPathNodeOwnershipTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ public function testGetIndex(): void
$this->assertIsCollectionResponse($response);
}

public function testGetAllowedNode(): void
public function testGetAllowedNodes(): void
{
// $response = $this->runGetRequest(sprintf('/%s', self::GROUP), self::TOKEN);
// $this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::GROUP), self::TOKEN);
$this->assertIsNodeResponse($response, 'Group');
$response = $this->runGetRequest(sprintf('/%s', self::DATA_1), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::DATA_2), self::TOKEN);
Expand Down
6 changes: 3 additions & 3 deletions tests/FeatureTests/Security/_03_04_GroupPathRelationOwnershipTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ public function testGetIndex(): void
$this->assertIsCollectionResponse($response);
}

public function testGetAllowedNode(): void
public function testGetAllowedNodes(): void
{
// $response = $this->runGetRequest(sprintf('/%s', self::GROUP), self::TOKEN);
// $this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::GROUP), self::TOKEN);
$this->assertIsNodeResponse($response, 'Group');
$response = $this->runGetRequest(sprintf('/%s', self::DATA_1), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::DATA_2), self::TOKEN);
Expand Down
33 changes: 26 additions & 7 deletions tests/FeatureTests/Security/_04_01_GroupsImmediateNodeOwnershipTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,41 @@ class _04_01_GroupsImmediateNodeOwnershipTest extends BaseRequestTestCase
public const GROUP_2 = '99f40c22-1b11-48bf-b9f4-350cac7b1fdd';
public const GROUP_3 = 'f1c7a5c3-5135-458f-b5a3-940335b90d31';
public const DATA = '896b608e-0dba-4f5f-8b39-522bc3e079c1';
public const IS_IN_GROUP_1 = '8c9d0790-7faa-4310-abab-8800fd2c14d4';
public const IS_IN_GROUP_2 = '0b9cf8e7-6bf0-4697-adb5-65b7145b84c5';
public const IS_IN_GROUP_3 = '60f2a995-4c11-4c6c-a16b-2da6edbcaf44';
public const OWNS_DATA = '1ef6accb-650a-459e-87fb-3209f15baa51';
public const OWNS_TOKEN = '5bb067b2-3e28-4f2d-b0c9-0aa306f1d15f';

public function testGetIndex(): void
{
$response = $this->runGetRequest('/', self::TOKEN);
$this->assertIsCollectionResponse($response);
}

public function testGetAllowedNode(): void
public function testGetAllowedNodes(): void
{
// $response = $this->runGetRequest(sprintf('/%s', self::GROUP_1), self::TOKEN);
// $this->assertIsNodeResponse($response, 'Data');
// $response = $this->runGetRequest(sprintf('/%s', self::GROUP_2), self::TOKEN);
// $this->assertIsNodeResponse($response, 'Data');
// $response = $this->runGetRequest(sprintf('/%s', self::GROUP_3), self::TOKEN);
// $this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::GROUP_1), self::TOKEN);
$this->assertIsNodeResponse($response, 'Group');
$response = $this->runGetRequest(sprintf('/%s', self::GROUP_2), self::TOKEN);
$this->assertIsNodeResponse($response, 'Group');
$response = $this->runGetRequest(sprintf('/%s', self::GROUP_3), self::TOKEN);
$this->assertIsNodeResponse($response, 'Group');
$response = $this->runGetRequest(sprintf('/%s', self::DATA), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
}

public function testGetAllowedRelations(): void
{
$response = $this->runGetRequest(sprintf('/%s', self::IS_IN_GROUP_1), self::TOKEN);
$this->assertIsRelationResponse($response, 'IS_IN_GROUP');
$response = $this->runGetRequest(sprintf('/%s', self::IS_IN_GROUP_2), self::TOKEN);
$this->assertIsRelationResponse($response, 'IS_IN_GROUP');
$response = $this->runGetRequest(sprintf('/%s', self::IS_IN_GROUP_3), self::TOKEN);
$this->assertIsRelationResponse($response, 'IS_IN_GROUP');
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_DATA), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_TOKEN), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
}
}
39 changes: 32 additions & 7 deletions tests/FeatureTests/Security/_04_02_GroupsImmediateRelationOwnershipTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,49 @@ class _04_02_GroupsImmediateRelationOwnershipTest extends BaseRequestTestCase
public const GROUP_3 = '9133d23d-0a4c-4f7b-a1f4-d84587ae9b3c';
public const DATA_1 = 'd79d90f5-7929-4ebd-8d06-1fa9a15aa63e';
public const DATA_2 = '450b8457-641c-4685-a171-2c371ce7f3f0';
public const IS_IN_GROUP_1 = '5f3bdada-0ae1-442d-a4b7-1a1f725c0a6e';
public const IS_IN_GROUP_2 = 'c6eda8b4-bd6c-4df7-bf4a-41d2dc88e2cc';
public const IS_IN_GROUP_3 = 'b74d5ae5-c534-4cf7-97a2-5190f8be8ba8';
public const OWNS_DATA_1 = 'fd90f318-dc45-4432-a1a5-39b4e24d3b15';
public const OWNS_DATA_2 = '3481bf4d-e393-4fd3-b0e9-131155495dbe';
public const OWNS_TOKEN = '2bef8c63-b319-49a2-9431-dbe88b780e6a';
public const RELATION = '933d3e3b-da81-4f3e-9320-122c71ddcfd9';

public function testGetIndex(): void
{
$response = $this->runGetRequest('/', self::TOKEN);
$this->assertIsCollectionResponse($response);
}

public function testGetAllowedNode(): void
public function testGetAllowedNodes(): void
{
// $response = $this->runGetRequest(sprintf('/%s', self::GROUP_1), self::TOKEN);
// $this->assertIsNodeResponse($response, 'Data');
// $response = $this->runGetRequest(sprintf('/%s', self::GROUP_2), self::TOKEN);
// $this->assertIsNodeResponse($response, 'Data');
// $response = $this->runGetRequest(sprintf('/%s', self::GROUP_3), self::TOKEN);
// $this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::GROUP_1), self::TOKEN);
$this->assertIsNodeResponse($response, 'Group');
$response = $this->runGetRequest(sprintf('/%s', self::GROUP_2), self::TOKEN);
$this->assertIsNodeResponse($response, 'Group');
$response = $this->runGetRequest(sprintf('/%s', self::GROUP_3), self::TOKEN);
$this->assertIsNodeResponse($response, 'Group');
$response = $this->runGetRequest(sprintf('/%s', self::DATA_1), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
$response = $this->runGetRequest(sprintf('/%s', self::DATA_2), self::TOKEN);
$this->assertIsNodeResponse($response, 'Data');
}

public function testGetAllowedRelations(): void
{
$response = $this->runGetRequest(sprintf('/%s', self::IS_IN_GROUP_1), self::TOKEN);
$this->assertIsRelationResponse($response, 'IS_IN_GROUP');
$response = $this->runGetRequest(sprintf('/%s', self::IS_IN_GROUP_2), self::TOKEN);
$this->assertIsRelationResponse($response, 'IS_IN_GROUP');
$response = $this->runGetRequest(sprintf('/%s', self::IS_IN_GROUP_3), self::TOKEN);
$this->assertIsRelationResponse($response, 'IS_IN_GROUP');
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_TOKEN), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_DATA_1), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
$response = $this->runGetRequest(sprintf('/%s', self::OWNS_DATA_2), self::TOKEN);
$this->assertIsRelationResponse($response, 'OWNS');
$response = $this->runGetRequest(sprintf('/%s', self::RELATION), self::TOKEN);
$this->assertIsRelationResponse($response, 'RELATION');
}
}
Loading

0 comments on commit 3527e43

Please sign in to comment.