From e750f3f33b97a0a5b3fdb930d07898791c888756 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 15 Oct 2024 12:09:38 +0200 Subject: [PATCH 1/8] tests(Behat): add CRUD tests on table issued via context Signed-off-by: Arthur Schiwon --- tests/integration/features/APIv2.feature | 168 ++++++++++++++++++ .../features/bootstrap/FeatureContext.php | 127 +++++++++++++ 2 files changed, 295 insertions(+) diff --git a/tests/integration/features/APIv2.feature b/tests/integration/features/APIv2.feature index bc43fc76b..4a0eabc29 100644 --- a/tests/integration/features/APIv2.feature +++ b/tests/integration/features/APIv2.feature @@ -927,3 +927,171 @@ Feature: APIv2 | four | 2023-12-24 | | five | [{"id": "admin", "type": 0}] | Then the reported status is 404 + + @api2 @contexts @contexts-content + Scenario: Execute CRUD permissions on a table available through a context + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | t1 | table | read,create,update,delete | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using table "t1" + And user "participant1-v2" creates row "r-updatable" with following values: + | statement | testing update | + And user "participant1-v2" creates row "r-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-inserted" with following values: + | statement | testing insert | + Then the reported status is "200" + When user "participant2-v2" updates row "r-updatable" with following values: + | statement | update accomplished | + Then the reported status is "200" + When user "participant2-v2" deletes row "r-deletable" + Then the reported status is "200" + And the user "participant1-v2" fetches table "t1", it has exactly these rows "r-updatable,r-inserted" + And the column "statement" of row "r-updatable" has the value "update accomplished" + + @api2 @contexts @contexts-content + Scenario: Execute CRU permissions on a table available through a context, ensure D is not possible + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | t1 | table | read,create,update | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using table "t1" + And user "participant1-v2" creates row "r-updatable" with following values: + | statement | testing update | + And user "participant1-v2" creates row "r-not-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-inserted" with following values: + | statement | testing insert | + Then the reported status is "200" + When user "participant2-v2" updates row "r-updatable" with following values: + | statement | update accomplished | + Then the reported status is "200" + When user "participant2-v2" deletes row "r-not-deletable" + Then the reported status is "403" + And the user "participant1-v2" fetches table "t1", it has exactly these rows "r-updatable,r-inserted,r-not-deletable" + And the column "statement" of row "r-updatable" has the value "update accomplished" + + @api2 @contexts @contexts-content + Scenario: Execute CRD permissions on a table available through a context, ensure U is not possible + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | t1 | table | read,create,delete | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using table "t1" + And user "participant1-v2" creates row "r-not-updatable" with following values: + | statement | testing not updated | + And user "participant1-v2" creates row "r-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-inserted" with following values: + | statement | testing insert | + Then the reported status is "200" + When user "participant2-v2" updates row "r-not-updatable" with following values: + | statement | update accomplished | + Then the reported status is "403" + When user "participant2-v2" deletes row "r-deletable" + Then the reported status is "200" + And the user "participant1-v2" fetches table "t1", it has exactly these rows "r-not-updatable,r-inserted" + And the column "statement" of row "r-not-updatable" has the value "testing not updated" + + @api2 @contexts @contexts-content + Scenario: Execute RUD permissions on a table available through a context, ensure C is not possible + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | t1 | table | read,update,delete | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using table "t1" + And user "participant1-v2" creates row "r-updatable" with following values: + | statement | testing update | + And user "participant1-v2" creates row "r-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-not-inserted" with following values: + | statement | testing insert | + Then the reported status is "403" + When user "participant2-v2" updates row "r-updatable" with following values: + | statement | update accomplished | + Then the reported status is "200" + When user "participant2-v2" deletes row "r-deletable" + Then the reported status is "200" + And the user "participant1-v2" fetches table "t1", it has exactly these rows "r-updatable" + And the column "statement" of row "r-updatable" has the value "update accomplished" + + @api2 @contexts @contexts-content + Scenario: Execute CR permissions on a table available through a context, ensure UD is not possible + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | t1 | table | read,create | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using table "t1" + And user "participant1-v2" creates row "r-not-updatable" with following values: + | statement | testing not updated | + And user "participant1-v2" creates row "r-not-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-inserted" with following values: + | statement | testing insert | + Then the reported status is "200" + When user "participant2-v2" updates row "r-not-updatable" with following values: + | statement | update accomplished | + Then the reported status is "403" + When user "participant2-v2" deletes row "r-not-deletable" + Then the reported status is "403" + And the user "participant1-v2" fetches table "t1", it has exactly these rows "r-not-updatable,r-inserted,r-not-deletable" + And the column "statement" of row "r-not-updatable" has the value "testing not updated" + + @api2 @contexts @contexts-content + Scenario: Execute R permissions on a table available through a context, ensure CUD is not possible + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | t1 | table | read | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using table "t1" + And user "participant1-v2" creates row "r-not-updatable" with following values: + | statement | testing not updated | + And user "participant1-v2" creates row "r-not-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-not-inserted" with following values: + | statement | testing insert | + Then the reported status is "403" + When user "participant2-v2" updates row "r-not-updatable" with following values: + | statement | update accomplished | + Then the reported status is "403" + When user "participant2-v2" deletes row "r-not-deletable" + Then the reported status is "403" + And the user "participant1-v2" fetches table "t1", it has exactly these rows "r-not-updatable,r-not-deletable" + And the column "statement" of row "r-not-updatable" has the value "testing not updated" diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 5d7891d79..9642f44b0 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -117,6 +117,9 @@ public function createTableV2(string $user, string $title, string $tableName, ?s $newTable = $this->getDataFromResponse($this->response)['ocs']['data']; $this->tableIds[$tableName] = $newTable['id']; + $this->collectionManager->register($newTable, 'table', $newTable['id'], $tableName, function () use ($user, $tableName) { + $this->deleteTableV2($user, $tableName); + }); Assert::assertEquals(200, $this->response->getStatusCode()); Assert::assertEquals($newTable['title'], $title); @@ -2420,4 +2423,128 @@ public function theInsertedRowHasTheFollowingValues(TableNode $columnValues) { throw new \Exception(sprintf('Some expected columns were not returned: %s ', print_r($expected, true))); } } + + /** + * @When using table :tableAlias + */ + public function usingTable(string $tableAlias): void { + $table = $this->collectionManager->getByAlias('table', $tableAlias); + $this->tableId = $table['id']; + } + + /** + * @Given user :user creates row :rowAlias with following values: + */ + public function userCreatesRowWithFollowingValues(string $user, string $rowAlias, TableNode $properties): void { + $this->setCurrentUser($user); + + $props = []; + foreach ($properties->getRows() as $row) { + $columnId = $this->tableColumns[$row[0]]; + $props[$columnId] = $row[1]; + } + + $this->sendOcsRequest( + 'POST', + '/apps/tables/api/2/tables/' . $this->tableId . '/rows', + ['data' => $props] + ); + + $newRow = $this->getDataFromResponse($this->response)['ocs']['data']; + if ($this->response->getStatusCode() === 200) { + $this->collectionManager->register($newRow, 'row', $newRow['id'], $rowAlias); + } + } + + /** + * @When user :user updates row :rowAlias with following values: + */ + public function userUpdatesRowWithFollowingValues(string $user, string $rowAlias, TableNode $properties): void { + $this->setCurrentUser($user); + + $props = []; + foreach ($properties->getRows() as $row) { + $columnId = $this->tableColumns[$row[0]]; + $props[$columnId] = $row[1]; + } + + $row = $this->collectionManager->getByAlias('row', $rowAlias); + $this->sendRequest( + 'PUT', + '/apps/tables/api/1/rows/' . $row['id'], + ['data' => $props] + ); + + if ($this->response->getStatusCode() === 200) { + $row = $this->getDataFromResponse($this->response); + $this->collectionManager->update($row, 'row', $row['id']); + } + } + + /** + * @When user :user deletes row :rowAlias + */ + public function userDeletesRow(string $user, string $rowAlias): void { + $this->setCurrentUser($user); + + $row = $this->collectionManager->getByAlias('row', $rowAlias); + $this->sendRequest( + 'DELETE', + '/apps/tables/api/1/rows/' . $row['id'], + ); + + $row = $this->getDataFromResponse($this->response); + if ($this->response->getStatusCode() === 200) { + $this->collectionManager->forget('row', $row['id']); + } + } + + /** + * @When the user :user fetches table :tableAlias, it has exactly these rows :rowAliasList + */ + public function tableHasExactlyThoseRows(string $user, string $tableAlias, string $rowAliasList): void { + $table = $this->collectionManager->getByAlias('table', $tableAlias); + + $this->sendRequest( + 'GET', + '/apps/tables/api/1/tables/' . $table['id'] . '/rows', + ); + + $allRows = $this->getDataFromResponse($this->response); + Assert::assertEquals(200, $this->response->getStatusCode()); + + $rowAliases = explode(',', $rowAliasList); + $expectedIds = array_map(function ($rowAlias): int { + $row = $this->collectionManager->getByAlias('row', $rowAlias); + return $row['id']; + }, $rowAliases); + sort($expectedIds); + + $actualRowIds = array_map(function (array $actualRow): int { + if ($this->collectionManager->getById('row', (int)$actualRow['id'])) { + $this->collectionManager->update($actualRow, 'row', (int)$actualRow['id']); + } else { + $this->collectionManager->register($actualRow, 'row', (int)$actualRow['id']); + } + return (int)$actualRow['id']; + }, $allRows); + sort($actualRowIds); + + Assert::assertSame($expectedIds, $actualRowIds); + } + + /** + * @When the column :columnName of row :rowAlias has the value :value + */ + public function columnOfRowIs(string $columnName, string $rowAlias, string $value): void { + $row = $this->collectionManager->getByAlias('row', $rowAlias); + $column = $this->collectionManager->getByAlias('column', $columnName); + + $expected = [ + 'columnId' => $column['id'], + 'value' => $value, + ]; + + Assert::assertContains($expected, $row['data']); + } } From 077e08b85e3332a3ca0fea8b878738f494ff9dc2 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 15 Oct 2024 12:10:15 +0200 Subject: [PATCH 2/8] fix(Rows): return correct status code when row update lacks permissions Signed-off-by: Arthur Schiwon --- lib/Service/RowService.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 18eeafeef..d903155be 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -344,6 +344,7 @@ private function getRowById(int $rowId): Row2 { * * @throws InternalError * @throws NotFoundError + * @throws PermissionError * @noinspection DuplicatedCode */ public function updateSet( @@ -367,7 +368,7 @@ public function updateSet( if (!$this->permissionsService->canUpdateRowsByViewId($viewId)) { $e = new \Exception('Update row is not allowed.'); $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); + throw new PermissionError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); } try { @@ -402,7 +403,7 @@ public function updateSet( if (!$this->permissionsService->canUpdateRowsByTableId($tableId)) { $e = new \Exception('Update row is not allowed.'); $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); + throw new PermissionError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); } try { $columns = $this->columnMapper->findAllByTable($tableId); From 91854cdeb93d0b32290a14f7297d841ff8a69838 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 15 Oct 2024 13:13:45 +0200 Subject: [PATCH 3/8] tests(Behat): add CRUD tests on table issued via context Signed-off-by: Arthur Schiwon --- tests/integration/features/APIv2.feature | 180 ++++++++++++++++++ .../features/bootstrap/FeatureContext.php | 51 +++-- 2 files changed, 217 insertions(+), 14 deletions(-) diff --git a/tests/integration/features/APIv2.feature b/tests/integration/features/APIv2.feature index 4a0eabc29..435ad747e 100644 --- a/tests/integration/features/APIv2.feature +++ b/tests/integration/features/APIv2.feature @@ -1095,3 +1095,183 @@ Feature: APIv2 Then the reported status is "403" And the user "participant1-v2" fetches table "t1", it has exactly these rows "r-not-updatable,r-not-deletable" And the column "statement" of row "r-not-updatable" has the value "testing not updated" + + @api2 @contexts @contexts-content + Scenario: Execute CRUD permissions on a table available through a context + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" create view "v1" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" sets columns "statement" to view "v1" + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | v1 | view | read,create,update,delete | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using view "v1" + And user "participant1-v2" creates row "r-updatable" with following values: + | statement | testing update | + And user "participant1-v2" creates row "r-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-inserted" with following values: + | statement | testing insert | + Then the reported status is "200" + When user "participant2-v2" updates row "r-updatable" with following values: + | statement | update accomplished | + Then the reported status is "200" + When user "participant2-v2" deletes row "r-deletable" + Then the reported status is "200" + And the user "participant1-v2" fetches view "v1", it has exactly these rows "r-updatable,r-inserted" + And the column "statement" of row "r-updatable" has the value "update accomplished" + + @api2 @contexts @contexts-content + Scenario: Execute CRU permissions on a table available through a context, ensure D is not possible + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" create view "v1" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" sets columns "statement" to view "v1" + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | v1 | view | read,create,update | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using view "v1" + And user "participant1-v2" creates row "r-updatable" with following values: + | statement | testing update | + And user "participant1-v2" creates row "r-not-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-inserted" with following values: + | statement | testing insert | + Then the reported status is "200" + When user "participant2-v2" updates row "r-updatable" with following values: + | statement | update accomplished | + Then the reported status is "200" + When user "participant2-v2" deletes row "r-not-deletable" + Then the reported status is "403" + And the user "participant1-v2" fetches view "v1", it has exactly these rows "r-updatable,r-inserted,r-not-deletable" + And the column "statement" of row "r-updatable" has the value "update accomplished" + + @api2 @contexts @contexts-content + Scenario: Execute CRD permissions on a table available through a context, ensure U is not possible + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" create view "v1" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" sets columns "statement" to view "v1" + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | v1 | view | read,create,delete | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using view "v1" + And user "participant1-v2" creates row "r-not-updatable" with following values: + | statement | testing not updated | + And user "participant1-v2" creates row "r-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-inserted" with following values: + | statement | testing insert | + Then the reported status is "200" + When user "participant2-v2" updates row "r-not-updatable" with following values: + | statement | update accomplished | + Then the reported status is "403" + When user "participant2-v2" deletes row "r-deletable" + Then the reported status is "200" + And the user "participant1-v2" fetches view "v1", it has exactly these rows "r-not-updatable,r-inserted" + And the column "statement" of row "r-not-updatable" has the value "testing not updated" + + @api2 @contexts @contexts-content + Scenario: Execute RUD permissions on a table available through a context, ensure C is not possible + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" create view "v1" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" sets columns "statement" to view "v1" + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | v1 | view | read,update,delete | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using view "v1" + And user "participant1-v2" creates row "r-updatable" with following values: + | statement | testing update | + And user "participant1-v2" creates row "r-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-not-inserted" with following values: + | statement | testing insert | + Then the reported status is "403" + When user "participant2-v2" updates row "r-updatable" with following values: + | statement | update accomplished | + Then the reported status is "200" + When user "participant2-v2" deletes row "r-deletable" + Then the reported status is "200" + And the user "participant1-v2" fetches view "v1", it has exactly these rows "r-updatable" + And the column "statement" of row "r-updatable" has the value "update accomplished" + + @api2 @contexts @contexts-content + Scenario: Execute CR permissions on a table available through a context, ensure UD is not possible + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" create view "v1" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" sets columns "statement" to view "v1" + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | v1 | view | read,create | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using view "v1" + And user "participant1-v2" creates row "r-not-updatable" with following values: + | statement | testing not updated | + And user "participant1-v2" creates row "r-not-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-inserted" with following values: + | statement | testing insert | + Then the reported status is "200" + When user "participant2-v2" updates row "r-not-updatable" with following values: + | statement | update accomplished | + Then the reported status is "403" + When user "participant2-v2" deletes row "r-not-deletable" + Then the reported status is "403" + And the user "participant1-v2" fetches view "v1", it has exactly these rows "r-not-updatable,r-inserted,r-not-deletable" + And the column "statement" of row "r-not-updatable" has the value "testing not updated" + + @api2 @contexts @contexts-content + Scenario: Execute R permissions on a table available through a context, ensure CUD is not possible + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" create view "v1" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" sets columns "statement" to view "v1" + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | v1 | view | read | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using view "v1" + And user "participant1-v2" creates row "r-not-updatable" with following values: + | statement | testing not updated | + And user "participant1-v2" creates row "r-not-deletable" with following values: + | statement | testing delete | + When user "participant2-v2" creates row "r-not-inserted" with following values: + | statement | testing insert | + Then the reported status is "403" + When user "participant2-v2" updates row "r-not-updatable" with following values: + | statement | update accomplished | + Then the reported status is "403" + When user "participant2-v2" deletes row "r-not-deletable" + Then the reported status is "403" + And the user "participant1-v2" fetches view "v1", it has exactly these rows "r-not-updatable,r-not-deletable" + And the column "statement" of row "r-not-updatable" has the value "testing not updated" diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 9642f44b0..aee4aae27 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -49,6 +49,7 @@ class FeatureContext implements Context { private ?int $rowId = null; private ?array $tableColumns = []; private ?array $importResult = null; + private ?array $activeNode = null; // we need some ids to reuse it in some contexts, // but we can not return them and reuse it in the scenarios, @@ -81,6 +82,7 @@ public function __construct() { public function setUp() { $this->createdUsers = []; $this->createdGroups = []; + $this->activeNode = null; } /** @@ -2425,11 +2427,19 @@ public function theInsertedRowHasTheFollowingValues(TableNode $columnValues) { } /** - * @When using table :tableAlias + * @When using :nodeType :alias */ - public function usingTable(string $tableAlias): void { - $table = $this->collectionManager->getByAlias('table', $tableAlias); - $this->tableId = $table['id']; + public function using(string $nodeType, string $alias): void { + $node = $this->collectionManager->getByAlias($nodeType, $alias); + $this->activeNode = [ + 'type' => $nodeType, + 'alias' => $alias, + 'id' => $node['id'], + ]; + if ($nodeType === 'table') { + //FIXME: remove $this->tableId everywhere + $this->tableId = $node['id']; + } } /** @@ -2444,9 +2454,13 @@ public function userCreatesRowWithFollowingValues(string $user, string $rowAlias $props[$columnId] = $row[1]; } + if (!$this->activeNode) { + throw new \LogicException('Set an active node via "@And using table|view nodeAlias"'); + } + $this->sendOcsRequest( 'POST', - '/apps/tables/api/2/tables/' . $this->tableId . '/rows', + sprintf('/apps/tables/api/2/%ss/%s/rows', $this->activeNode['type'], $this->activeNode['id']), ['data' => $props] ); @@ -2469,10 +2483,14 @@ public function userUpdatesRowWithFollowingValues(string $user, string $rowAlias } $row = $this->collectionManager->getByAlias('row', $rowAlias); + $payload = ['data' => $props]; + if ($this->activeNode['type'] === 'view') { + $payload['viewId'] = $this->activeNode['id']; + } $this->sendRequest( 'PUT', '/apps/tables/api/1/rows/' . $row['id'], - ['data' => $props] + $payload, ); if ($this->response->getStatusCode() === 200) { @@ -2488,10 +2506,14 @@ public function userDeletesRow(string $user, string $rowAlias): void { $this->setCurrentUser($user); $row = $this->collectionManager->getByAlias('row', $rowAlias); - $this->sendRequest( - 'DELETE', - '/apps/tables/api/1/rows/' . $row['id'], - ); + + if ($this->activeNode['type'] === 'view') { + $endpoint = sprintf('/apps/tables/api/1/views/%s/rows/%s',$this->activeNode['id'], $row['id']); + } else { + $endpoint = sprintf('/apps/tables/api/1/rows/%s', $row['id']); + } + + $this->sendRequest('DELETE', $endpoint); $row = $this->getDataFromResponse($this->response); if ($this->response->getStatusCode() === 200) { @@ -2500,14 +2522,15 @@ public function userDeletesRow(string $user, string $rowAlias): void { } /** - * @When the user :user fetches table :tableAlias, it has exactly these rows :rowAliasList + * @When the user :user fetches :nodeType :nodeAlias, it has exactly these rows :rowAliasList */ - public function tableHasExactlyThoseRows(string $user, string $tableAlias, string $rowAliasList): void { - $table = $this->collectionManager->getByAlias('table', $tableAlias); + public function nodeHasExactlyThoseRows(string $user, string $nodeType, string $nodeAlias, string $rowAliasList): void { + $this->setCurrentUser($user); + $nodeItem = $this->collectionManager->getByAlias($nodeType, $nodeAlias); $this->sendRequest( 'GET', - '/apps/tables/api/1/tables/' . $table['id'] . '/rows', + sprintf('/apps/tables/api/1/%ss/%s/rows', $nodeType, $nodeItem['id']), ); $allRows = $this->getDataFromResponse($this->response); From c748709809e812e66c99166c1b44e171b4dec434 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 15 Oct 2024 13:14:19 +0200 Subject: [PATCH 4/8] fix(Api1): return correct status code when row delete lacks permissions Signed-off-by: Arthur Schiwon --- lib/Service/RowService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index d903155be..684b6e043 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -460,7 +460,7 @@ public function delete(int $id, ?int $viewId, string $userId): Row2 { if (!$this->permissionsService->canDeleteRowsByViewId($viewId)) { $e = new \Exception('Update row is not allowed.'); $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); + throw new PermissionError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); } try { $view = $this->viewMapper->find($viewId); From ba1bc4e9762867372573130fa8e130e3bd291039 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 15 Oct 2024 13:34:28 +0200 Subject: [PATCH 5/8] tests(Behat): ensure folks without access cannot modify res via context Signed-off-by: Arthur Schiwon --- tests/integration/features/APIv2.feature | 42 +++++++++++++++++++ .../features/bootstrap/FeatureContext.php | 2 +- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/tests/integration/features/APIv2.feature b/tests/integration/features/APIv2.feature index 435ad747e..4dd51c2d3 100644 --- a/tests/integration/features/APIv2.feature +++ b/tests/integration/features/APIv2.feature @@ -1275,3 +1275,45 @@ Feature: APIv2 Then the reported status is "403" And the user "participant1-v2" fetches view "v1", it has exactly these rows "r-not-updatable,r-not-deletable" And the column "statement" of row "r-not-updatable" has the value "testing not updated" + + @api2 @contexts @contexts-content + Scenario: Ensure elements cannot be modified in an unavailable table or view, otherwise shared through a context + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And user "participant1-v2" create view "v1" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" sets columns "statement" to view "v1" + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | t1 | table | read,create,update,delete | + | v1 | view | read,create,update,delete | + And user "participant1-v2" shares the Context "c1" to "user" "participant2-v2" + And using table "t1" + And user "participant1-v2" creates row "r-not-updatable" with following values: + | statement | testing not updated | + And user "participant1-v2" creates row "r-not-deletable" with following values: + | statement | testing delete | + When user "participant3-v2" creates row "r-not-inserted" with following values: + | statement | testing insert | + Then the reported status is "404" + When user "participant3-v2" updates row "r-not-updatable" with following values: + | statement | update accomplished | + Then the reported status is "404" + When user "participant3-v2" deletes row "r-not-deletable" + Then the reported status is "404" + And the user "participant1-v2" fetches table "t1", it has exactly these rows "r-not-updatable,r-not-deletable" + And the column "statement" of row "r-not-updatable" has the value "testing not updated" + And using view "v1" + When user "participant3-v2" creates row "r-not-inserted" with following values: + | statement | testing insert | + Then the reported status is "404" + When user "participant3-v2" updates row "r-not-updatable" with following values: + | statement | update accomplished | + Then the reported status is "404" + When user "participant3-v2" deletes row "r-not-deletable" + Then the reported status is "404" + And the user "participant1-v2" fetches table "t1", it has exactly these rows "r-not-updatable,r-not-deletable" + And the column "statement" of row "r-not-updatable" has the value "testing not updated" diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index aee4aae27..f63c78529 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -2464,8 +2464,8 @@ public function userCreatesRowWithFollowingValues(string $user, string $rowAlias ['data' => $props] ); - $newRow = $this->getDataFromResponse($this->response)['ocs']['data']; if ($this->response->getStatusCode() === 200) { + $newRow = $this->getDataFromResponse($this->response)['ocs']['data']; $this->collectionManager->register($newRow, 'row', $newRow['id'], $rowAlias); } } From f197e543c0a7461803d5f8eedefaa586b6fecaf4 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 15 Oct 2024 13:35:07 +0200 Subject: [PATCH 6/8] fix(Rows): return correct status code when row update/delete lacks access Signed-off-by: Arthur Schiwon --- lib/Service/RowService.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 684b6e043..0f0398a99 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -365,6 +365,11 @@ public function updateSet( if ($viewId) { // security + if (!$this->permissionsService->canReadRowsByElementId($viewId, 'view', $userId)) { + $e = new \Exception('Row not found.'); + $this->logger->error($e->getMessage(), ['exception' => $e]); + throw new NotFoundError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); + } if (!$this->permissionsService->canUpdateRowsByViewId($viewId)) { $e = new \Exception('Update row is not allowed.'); $this->logger->error($e->getMessage(), ['exception' => $e]); @@ -400,6 +405,11 @@ public function updateSet( $tableId = $item->getTableId(); // security + if (!$this->permissionsService->canReadRowsByElementId($item->getTableId(), 'table', $userId)) { + $e = new \Exception('Row not found.'); + $this->logger->error($e->getMessage(), ['exception' => $e]); + throw new NotFoundError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); + } if (!$this->permissionsService->canUpdateRowsByTableId($tableId)) { $e = new \Exception('Update row is not allowed.'); $this->logger->error($e->getMessage(), ['exception' => $e]); @@ -457,6 +467,11 @@ public function delete(int $id, ?int $viewId, string $userId): Row2 { if ($viewId) { // security + if (!$this->permissionsService->canReadRowsByElementId($viewId, 'view', $userId)) { + $e = new \Exception('Row not found.'); + $this->logger->error($e->getMessage(), ['exception' => $e]); + throw new NotFoundError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); + } if (!$this->permissionsService->canDeleteRowsByViewId($viewId)) { $e = new \Exception('Update row is not allowed.'); $this->logger->error($e->getMessage(), ['exception' => $e]); @@ -475,6 +490,11 @@ public function delete(int $id, ?int $viewId, string $userId): Row2 { } } else { // security + if (!$this->permissionsService->canReadRowsByElementId($item->getTableId(), 'table', $userId)) { + $e = new \Exception('Row not found.'); + $this->logger->error($e->getMessage(), ['exception' => $e]); + throw new NotFoundError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); + } if (!$this->permissionsService->canDeleteRowsByTableId($item->getTableId())) { $e = new \Exception('Update row is not allowed.'); $this->logger->error($e->getMessage(), ['exception' => $e]); From e0c7614c3312cd84ca2f329a640fadea04ca44e9 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 15 Oct 2024 13:42:06 +0200 Subject: [PATCH 7/8] style(PHP): satisfy code style linter Signed-off-by: Arthur Schiwon --- tests/integration/features/bootstrap/FeatureContext.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index f63c78529..fe0843d78 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -2508,7 +2508,7 @@ public function userDeletesRow(string $user, string $rowAlias): void { $row = $this->collectionManager->getByAlias('row', $rowAlias); if ($this->activeNode['type'] === 'view') { - $endpoint = sprintf('/apps/tables/api/1/views/%s/rows/%s',$this->activeNode['id'], $row['id']); + $endpoint = sprintf('/apps/tables/api/1/views/%s/rows/%s', $this->activeNode['id'], $row['id']); } else { $endpoint = sprintf('/apps/tables/api/1/rows/%s', $row['id']); } From ad3af8fb2bc282861469045f75ff79febc8220ad Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 15 Oct 2024 16:05:13 +0200 Subject: [PATCH 8/8] test(Behat): remove table from collection manager upon delete Signed-off-by: Arthur Schiwon --- .../features/bootstrap/CollectionManager.php | 3 ++- .../integration/features/bootstrap/FeatureContext.php | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/integration/features/bootstrap/CollectionManager.php b/tests/integration/features/bootstrap/CollectionManager.php index 0fd53a581..4e1bf3111 100644 --- a/tests/integration/features/bootstrap/CollectionManager.php +++ b/tests/integration/features/bootstrap/CollectionManager.php @@ -44,7 +44,8 @@ public function forget(string $type, int $id, ?string $alias = null): void { if ($alias) { unset($this->mapByAlias[$this->makeKey($type, $alias)]); } - unset($this->itemsById[$this->makeKey($type, $id)]); + $idKey = $this->makeKey($type, $id); + unset($this->cleanUp[$idKey], $this->itemsById[$idKey]); } public function cleanUp(): void { diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index fe0843d78..e3b2f1128 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -318,6 +318,9 @@ public function transferTableV2(string $user, string $newUser, string $tableName Assert::assertEquals(200, $this->response->getStatusCode()); Assert::assertEquals($updatedTable['ownership'], $newUser); + $this->collectionManager->update($updatedTable, 'table', $updatedTable['id'], function () use ($newUser, $tableName): void { + $this->deleteTableV2($newUser, $tableName); + }); } /** @@ -367,6 +370,9 @@ public function deleteTableV2(string $user, string $tableName): void { Assert::assertEquals(404, $this->response->getStatusCode()); unset($this->tableIds[$tableName]); + if ($table = $this->collectionManager->getByAlias('table', $tableName)) { + $this->collectionManager->forget('table', $table['id']); + } } /** @@ -915,6 +921,10 @@ public function deleteTable(string $user, string $keyword): void { Assert::assertEquals($deletedTable['title'], $table['title']); Assert::assertEquals($deletedTable['id'], $table['id']); + if ($tableItem = $this->collectionManager->getById('table', $table['id'])) { + $this->collectionManager->forget('table', $tableItem['id']); + } + $this->sendRequest( 'GET', '/apps/tables/api/1/tables/'.$deletedTable['id'],