From 511b467ad518d2a12101ea94eee6a0072844fa69 Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Thu, 21 Nov 2024 11:22:24 +0100 Subject: [PATCH 1/7] Issue 52: Fix sniffer errors. --- collabora_online.views.inc | 5 ++ .../collabora_online_group.install | 8 +++- .../src/Functional/GroupMediaViewsTest.php | 8 ++-- .../tests/src/Kernel/AccessTest.php | 47 ++++++++++--------- .../tests/src/Kernel/PermissionTest.php | 7 +-- .../tests/src/Traits/GroupRelationTrait.php | 2 +- src/Plugin/views/field/CollaboraEdit.php | 1 + src/Plugin/views/field/CollaboraPreview.php | 1 + tests/src/Kernel/ViewsLinkFieldsTest.php | 21 +++++---- 9 files changed, 58 insertions(+), 42 deletions(-) diff --git a/collabora_online.views.inc b/collabora_online.views.inc index 1e3d8302..275511b5 100644 --- a/collabora_online.views.inc +++ b/collabora_online.views.inc @@ -1,5 +1,10 @@ t('Edit in Collabora Online'), ], ]; - // Add new fields as options for the dropbutton and move the element to the end. + // Add new fields as options for the dropbutton, and move the dropbutton to + // the end of the array. $dropbutton = $display['display_options']['fields']['dropbutton']; $dropbutton['fields'] += [ 'collabora_preview' => 'collabora_preview', diff --git a/modules/collabora_online_group/tests/src/Functional/GroupMediaViewsTest.php b/modules/collabora_online_group/tests/src/Functional/GroupMediaViewsTest.php index b574ba74..a95267d3 100644 --- a/modules/collabora_online_group/tests/src/Functional/GroupMediaViewsTest.php +++ b/modules/collabora_online_group/tests/src/Functional/GroupMediaViewsTest.php @@ -114,12 +114,12 @@ public function testViewLinks(): void { Url::fromRoute( 'collabora-online.view', [ - 'media' => $media->id() + 'media' => $media->id(), ], [ 'query' => [ 'destination' => "/group/{$group->id()}/media", - ] + ], ] )->toString(), $operation_links[0]->getAttribute('href') @@ -129,12 +129,12 @@ public function testViewLinks(): void { Url::fromRoute( 'collabora-online.edit', [ - 'media' => $media->id() + 'media' => $media->id(), ], [ 'query' => [ 'destination' => "/group/{$group->id()}/media", - ] + ], ] )->toString(), $operation_links[1]->getAttribute('href') diff --git a/modules/collabora_online_group/tests/src/Kernel/AccessTest.php b/modules/collabora_online_group/tests/src/Kernel/AccessTest.php index 866e9b51..d73f6c5c 100644 --- a/modules/collabora_online_group/tests/src/Kernel/AccessTest.php +++ b/modules/collabora_online_group/tests/src/Kernel/AccessTest.php @@ -33,20 +33,20 @@ class AccessTest extends GroupKernelTestBase { 'groupmedia', 'collabora_online', 'collabora_online_group', - 'user' + 'user', ]; /** - * {@inheritdoc} - */ - protected function setUp(): void { - parent::setUp(); + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); - $this->installEntitySchema('file'); - $this->installEntitySchema('media'); - $this->installEntitySchema('group_role'); - $this->installSchema('file', ['file_usage']); - } + $this->installEntitySchema('file'); + $this->installEntitySchema('media'); + $this->installEntitySchema('group_role'); + $this->installSchema('file', ['file_usage']); + } /** * Tests that access to Collabora group permissions is handled. @@ -76,7 +76,8 @@ public function testCollaboraAccess(): void { foreach ($this->getTestScenarios() as $scenario_name => $scenario) { // Set the current permissions for the existing role. $group_role->set('permissions', $scenario['group_permissions'])->save(); - // Create the user with the given permissions and as member of the group. + // Create the user with the given permissions and as member of the + // group. $user = $this->createUser($scenario['permissions']); $group->addMember($user); // Set user as owner if the scope is 'own'. @@ -90,7 +91,7 @@ public function testCollaboraAccess(): void { sprintf('Access check failed for scenario: %s', $scenario_name) ); } - } + } /** * Retrieves the scenarios to be tested. @@ -105,65 +106,65 @@ protected function getTestScenarios(): array { 'permissions' => [], 'group_permissions' => [], 'operation' => 'preview in collabora', - 'scope' => 'any' + 'scope' => 'any', ], 'preview_global_permisions' => [ 'result' => FALSE, 'permissions' => ['preview document in collabora'], 'group_permissions' => [], 'operation' => 'preview in collabora', - 'scope' => 'any' + 'scope' => 'any', ], - 'preview_group_permisions' =>[ + 'preview_group_permisions' => [ 'result' => TRUE, 'permissions' => [], 'group_permissions' => ['preview group_media:document in collabora'], 'operation' => 'preview in collabora', - 'scope' => 'any' + 'scope' => 'any', ], 'edit_any_no_permisions' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => [], 'operation' => 'edit in collabora', - 'scope' => 'any' + 'scope' => 'any', ], 'edit_any_global_permisions' => [ 'result' => FALSE, 'permissions' => ['edit any document in collabora'], 'group_permissions' => [], 'operation' => 'edit in collabora', - 'scope' => 'any' + 'scope' => 'any', ], 'edit_any_group_permisions' => [ 'result' => TRUE, 'permissions' => [], 'group_permissions' => ['edit any group_media:document in collabora'], 'operation' => 'edit in collabora', - 'scope' => 'any' + 'scope' => 'any', ], 'edit_own_no_permisions' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => [], 'operation' => 'edit in collabora', - 'scope' => 'own' + 'scope' => 'own', ], 'edit_own_global_permisions' => [ 'result' => FALSE, 'permissions' => ['edit own document in collabora'], 'group_permissions' => [], 'operation' => 'edit in collabora', - 'scope' => 'own' + 'scope' => 'own', ], 'edit_own_group_permisions' => [ 'result' => TRUE, 'permissions' => [], 'group_permissions' => ['edit own group_media:document in collabora'], 'operation' => 'edit in collabora', - 'scope' => 'own' + 'scope' => 'own', ], ]; - } + } } diff --git a/modules/collabora_online_group/tests/src/Kernel/PermissionTest.php b/modules/collabora_online_group/tests/src/Kernel/PermissionTest.php index 362c9c7a..9baa06ce 100644 --- a/modules/collabora_online_group/tests/src/Kernel/PermissionTest.php +++ b/modules/collabora_online_group/tests/src/Kernel/PermissionTest.php @@ -45,7 +45,7 @@ public function testGroupPermissions(): void { 'group_cardinality' => 0, 'entity_cardinality' => 1, 'use_creation_wizard' => FALSE, - ]); + ]); $this->createPluginRelation( $group_type_2, 'group_media:document', @@ -53,7 +53,7 @@ public function testGroupPermissions(): void { 'group_cardinality' => 0, 'entity_cardinality' => 1, 'use_creation_wizard' => FALSE, - ]); + ]); $this->createPluginRelation( $group_type_2, 'group_media:spreadsheet', @@ -61,7 +61,7 @@ public function testGroupPermissions(): void { 'group_cardinality' => 0, 'entity_cardinality' => 1, 'use_creation_wizard' => FALSE, - ]); + ]); // Check that permissions are generated for the groups. // Save current permissions. @@ -117,4 +117,5 @@ public function testGroupPermissions(): void { $new_permissions_3, )); } + } diff --git a/modules/collabora_online_group/tests/src/Traits/GroupRelationTrait.php b/modules/collabora_online_group/tests/src/Traits/GroupRelationTrait.php index b66dc3d2..fb6c508d 100644 --- a/modules/collabora_online_group/tests/src/Traits/GroupRelationTrait.php +++ b/modules/collabora_online_group/tests/src/Traits/GroupRelationTrait.php @@ -18,7 +18,7 @@ trait GroupRelationTrait { * Wrapper to support group 2.x and 3.x. * * @param \Drupal\group\Entity\GroupTypeInterface $group_type - * The group type + * The group type. * @param string $plugin_id * The plugin. * @param array $values diff --git a/src/Plugin/views/field/CollaboraEdit.php b/src/Plugin/views/field/CollaboraEdit.php index 92bc2bd7..328ebc76 100644 --- a/src/Plugin/views/field/CollaboraEdit.php +++ b/src/Plugin/views/field/CollaboraEdit.php @@ -39,4 +39,5 @@ protected function getUrlInfo(ResultRow $row): Url|null { protected function getDefaultLabel(): TranslatableMarkup { return $this->t('Edit in Collabora Online'); } + } diff --git a/src/Plugin/views/field/CollaboraPreview.php b/src/Plugin/views/field/CollaboraPreview.php index e93817f9..f636b359 100644 --- a/src/Plugin/views/field/CollaboraPreview.php +++ b/src/Plugin/views/field/CollaboraPreview.php @@ -39,4 +39,5 @@ protected function getUrlInfo(ResultRow $row): Url|null { protected function getDefaultLabel(): TranslatableMarkup { return $this->t('View in Collabora Online'); } + } diff --git a/tests/src/Kernel/ViewsLinkFieldsTest.php b/tests/src/Kernel/ViewsLinkFieldsTest.php index 2aa9e7ef..e7dc336d 100644 --- a/tests/src/Kernel/ViewsLinkFieldsTest.php +++ b/tests/src/Kernel/ViewsLinkFieldsTest.php @@ -25,7 +25,7 @@ class ViewsLinkFieldsTest extends KernelTestBase { /** * Media owned by current user. * - * @var \Drupal\media\MediaInterface; + * @var \Drupal\media\MediaInterface */ protected $ownMedia = NULL; @@ -59,7 +59,7 @@ protected function setUp(): void { \Drupal::moduleHandler()->loadInclude('user', 'install'); user_install(); - // Create two medias to check access with different scopes, 'any' and 'own'. + // Create two medias to check access with different scopes: any and own. $this->createMediaEntity('document'); $this->ownMedia = $this->createMediaEntity('document'); } @@ -83,7 +83,7 @@ public function testLinks(): void { 'edit' => [FALSE, FALSE], ], $this->createUser([ - 'preview document in collabora' + 'preview document in collabora', ]) ); // User with 'Edit any' permission can see edit link. @@ -93,17 +93,18 @@ public function testLinks(): void { 'edit' => [TRUE, TRUE], ], $this->createUser([ - 'edit any document in collabora' + 'edit any document in collabora', ]) ); - // User with 'Edit own' permission can see edit link for entities they own. + // User with 'Edit own' permission can see edit link for entities they + // own. $this->doTestLinks( [ 'preview' => [FALSE, FALSE], 'edit' => [FALSE, TRUE], ], $this->createUser([ - 'edit own document in collabora' + 'edit own document in collabora', ]) ); } @@ -127,12 +128,12 @@ protected function doTestLinks(array $expected_results, AccountInterface $accoun 'preview' => [ 'label' => 'View in Collabora Online', 'field_id' => 'collabora_preview', - 'route' => 'collabora-online.view' + 'route' => 'collabora-online.view', ], 'edit' => [ 'label' => 'Edit in Collabora Online', 'field_id' => 'collabora_edit', - 'route' => 'collabora-online.edit' + 'route' => 'collabora-online.edit', ], ]; @@ -141,12 +142,12 @@ protected function doTestLinks(array $expected_results, AccountInterface $accoun foreach (Media::loadMultiple() as $media) { foreach ($expected_results as $operation => $expected_result) { $expected_link = ''; - // The operation array contains results for each of the entities. + // The operations array contains results for each entity. if ($expected_result[$i]) { $path = Url::fromRoute($info[$operation]['route'], ['media' => $media->id()])->toString(); $expected_link = '' . $info[$operation]['label'] . ''; } - // We check the output, whether it is a link or is empty (access denied). + // We check the output: link HTML or empty (access denied). $link = $view->style_plugin->getField($i, $info[$operation]['field_id']); $this->assertEquals($expected_link, (string) $link); } From 39367fbe36c1da4f94f281a6b46abaf69d7f5834 Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Thu, 21 Nov 2024 11:33:18 +0100 Subject: [PATCH 2/7] Issue 52: Use media creation trait. --- tests/src/Kernel/CollaboraMediaAccessTest.php | 31 ++----------------- 1 file changed, 2 insertions(+), 29 deletions(-) diff --git a/tests/src/Kernel/CollaboraMediaAccessTest.php b/tests/src/Kernel/CollaboraMediaAccessTest.php index fd9a9c42..b9b6d814 100644 --- a/tests/src/Kernel/CollaboraMediaAccessTest.php +++ b/tests/src/Kernel/CollaboraMediaAccessTest.php @@ -8,10 +8,8 @@ use Drupal\Core\Session\AccountInterface; use Drupal\Core\Session\AnonymousUserSession; use Drupal\Core\Url; -use Drupal\file\Entity\File; use Drupal\KernelTests\KernelTestBase; -use Drupal\media\Entity\Media; -use Drupal\media\MediaInterface; +use Drupal\Tests\collabora_online\Traits\MediaCreationTrait; use Drupal\Tests\media\Traits\MediaTypeCreationTrait; use Drupal\Tests\user\Traits\UserCreationTrait; use Drupal\user\RoleInterface; @@ -23,6 +21,7 @@ class CollaboraMediaAccessTest extends KernelTestBase { use MediaTypeCreationTrait; use UserCreationTrait; + use MediaCreationTrait; /** * {@inheritdoc} @@ -272,32 +271,6 @@ protected function assertCollaboraMediaAccess(array $expected, AccountInterface ); } - /** - * Creates a media entity with attached file. - * - * @param string $type - * Media type. - * @param array $values - * Values for the media entity. - * - * @return \Drupal\media\MediaInterface - * New media entity. - */ - protected function createMediaEntity(string $type, array $values = []): MediaInterface { - file_put_contents('public://test.txt', 'Hello test'); - $file = File::create([ - 'uri' => 'public://test.txt', - ]); - $file->save(); - $values += [ - 'bundle' => $type, - 'field_media_file' => $file->id(), - ]; - $media = Media::create($values); - $media->save(); - return $media; - } - /** * Asserts that two values are the same when exported to yaml. * From e41b01ce7ac030b67afdd2b72d358b07101f9761 Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Fri, 22 Nov 2024 12:21:05 +0100 Subject: [PATCH 3/7] Issue 52: Add new permission. --- .../CollaboraPermissionProvider.php | 24 +++++++++++-------- .../tests/src/Kernel/PermissionTest.php | 21 +++++++++------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraPermissionProvider.php b/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraPermissionProvider.php index f523bdbe..319d27af 100644 --- a/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraPermissionProvider.php +++ b/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraPermissionProvider.php @@ -23,7 +23,10 @@ public function buildPermissions(): array { // Add Collabora permissions. $prefix = 'Entity:'; if ($name = $provider_chain->getPermission('preview in collabora', 'entity')) { - $permissions[$name] = $this->buildPermission("$prefix Preview %entity_type in collabora"); + $permissions[$name] = $this->buildPermission("$prefix Preview published %entity_type in collabora"); + } + if ($name = $provider_chain->getPermission('preview in collabora', 'entity', 'own')) { + $permissions[$name] = $this->buildPermission("$prefix Preview own unpublished %entity_type in collabora"); } if ($name = $provider_chain->getPermission('edit in collabora', 'entity')) { $permissions[$name] = $this->buildPermission("$prefix Edit any %entity_type in collabora"); @@ -39,20 +42,21 @@ public function buildPermissions(): array { * {@inheritdoc} */ public function getPermission($operation, $target, $scope = 'any'): bool|string { - if ($target === 'entity') { + if ( + $target === 'entity' && + $this->definesEntityPermissions && + ($this->implementsOwnerInterface || $scope === 'any') + ) { switch ($operation) { case 'preview in collabora': + if ($scope === 'own') { + return "preview $scope unpublished $this->pluginId in collabora"; + } + return "preview $this->pluginId in collabora"; case 'edit in collabora': - if ( - $this->definesEntityPermissions && - ($this->implementsOwnerInterface || $scope === 'any') - ) { - return "edit $scope $this->pluginId in collabora"; - } - - return FALSE; + return "edit $scope $this->pluginId in collabora"; } } diff --git a/modules/collabora_online_group/tests/src/Kernel/PermissionTest.php b/modules/collabora_online_group/tests/src/Kernel/PermissionTest.php index 9baa06ce..4938b508 100644 --- a/modules/collabora_online_group/tests/src/Kernel/PermissionTest.php +++ b/modules/collabora_online_group/tests/src/Kernel/PermissionTest.php @@ -89,12 +89,13 @@ public function testGroupPermissions(): void { [ 'edit any group_media:document in collabora' => 'Entity: Edit any media item in collabora', 'edit own group_media:document in collabora' => 'Entity: Edit own media item in collabora', - 'preview group_media:document in collabora' => 'Entity: Preview media item in collabora', + 'preview group_media:document in collabora' => 'Entity: Preview published media item in collabora', + 'preview own unpublished group_media:document in collabora' => 'Entity: Preview own unpublished media item in collabora', ], array_map( - fn ($permission) => (string) $permission['title'], + fn($permission) => (string) $permission['title'], $new_permissions_1, - )); + )); // The 'group_2' has 'document' and 'spreadsheet' permissions. $this->assertSame( [ @@ -102,20 +103,22 @@ public function testGroupPermissions(): void { 'edit any group_media:spreadsheet in collabora' => 'Entity: Edit any media item in collabora', 'edit own group_media:document in collabora' => 'Entity: Edit own media item in collabora', 'edit own group_media:spreadsheet in collabora' => 'Entity: Edit own media item in collabora', - 'preview group_media:document in collabora' => 'Entity: Preview media item in collabora', - 'preview group_media:spreadsheet in collabora' => 'Entity: Preview media item in collabora', + 'preview group_media:document in collabora' => 'Entity: Preview published media item in collabora', + 'preview group_media:spreadsheet in collabora' => 'Entity: Preview published media item in collabora', + 'preview own unpublished group_media:document in collabora' => 'Entity: Preview own unpublished media item in collabora', + 'preview own unpublished group_media:spreadsheet in collabora' => 'Entity: Preview own unpublished media item in collabora', ], array_map( - fn ($permission) => (string) $permission['title'], + fn($permission) => (string) $permission['title'], $new_permissions_2, - )); + )); // The 'group_3' doesn't have any new permissions. $this->assertSame( [], array_map( - fn ($permission) => (string) $permission['title'], + fn($permission) => (string) $permission['title'], $new_permissions_3, - )); + )); } } From 38a976e645857d887d03dc592665fbd385965bac Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Thu, 21 Nov 2024 14:02:29 +0100 Subject: [PATCH 4/7] Issue 52: Add new cases to group access test. --- .../tests/src/Kernel/AccessTest.php | 97 +++++++++++++++++-- 1 file changed, 87 insertions(+), 10 deletions(-) diff --git a/modules/collabora_online_group/tests/src/Kernel/AccessTest.php b/modules/collabora_online_group/tests/src/Kernel/AccessTest.php index d73f6c5c..ee3fc3e1 100644 --- a/modules/collabora_online_group/tests/src/Kernel/AccessTest.php +++ b/modules/collabora_online_group/tests/src/Kernel/AccessTest.php @@ -74,6 +74,8 @@ public function testCollaboraAccess(): void { // Iterate over each scenario. foreach ($this->getTestScenarios() as $scenario_name => $scenario) { + // Apply status to media. + $media->set('status', $scenario['status'])->save(); // Set the current permissions for the existing role. $group_role->set('permissions', $scenario['group_permissions'])->save(); // Create the user with the given permissions and as member of the @@ -88,7 +90,7 @@ public function testCollaboraAccess(): void { $this->assertEquals( $scenario['result'], $media->access($scenario['operation'], $user), - sprintf('Access check failed for scenario: %s', $scenario_name) + sprintf('Access check failed for scenario: "%s"', $scenario_name) ); } } @@ -100,70 +102,145 @@ public function testCollaboraAccess(): void { * An array of test scenarios. */ protected function getTestScenarios(): array { + // The scenario keys contains values used for each scenario: + // 'operation:status:scope:global_permission:group_permission'. return [ - 'preview_no_permisions' => [ + 'preview:published:any:::' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => [], 'operation' => 'preview in collabora', + 'status' => 1, 'scope' => 'any', ], - 'preview_global_permisions' => [ + 'preview:published:any:preview::' => [ 'result' => FALSE, 'permissions' => ['preview document in collabora'], 'group_permissions' => [], 'operation' => 'preview in collabora', + 'status' => 1, 'scope' => 'any', ], - 'preview_group_permisions' => [ + 'preview:published:any::preview' => [ 'result' => TRUE, 'permissions' => [], 'group_permissions' => ['preview group_media:document in collabora'], 'operation' => 'preview in collabora', + 'status' => 1, 'scope' => 'any', ], - 'edit_any_no_permisions' => [ + 'preview:published:own::preview' => [ + 'result' => TRUE, + 'permissions' => [], + 'group_permissions' => ['preview group_media:document in collabora'], + 'operation' => 'preview in collabora', + 'status' => 1, + 'scope' => 'own', + ], + 'FAIL preview:unpublished:any::preview' => [ + 'result' => FALSE, + 'permissions' => [], + 'group_permissions' => ['preview group_media:document in collabora'], + 'operation' => 'preview in collabora', + 'status' => 0, + 'scope' => 'any', + ], + 'FAIL preview:unpublished:own::preview' => [ + 'result' => FALSE, + 'permissions' => [], + 'group_permissions' => ['preview group_media:document in collabora'], + 'operation' => 'preview in collabora', + 'status' => 0, + 'scope' => 'own', + ], + 'preview:unpublished:own:preview_own::' => [ + 'result' => FALSE, + 'permissions' => ['preview own unpublished document in collabora'], + 'group_permissions' => [], + 'operation' => 'preview in collabora', + 'status' => 0, + 'scope' => 'own', + ], + 'preview:unpublished:own::preview_own' => [ + 'result' => TRUE, + 'permissions' => [], + 'group_permissions' => ['preview own unpublished group_media:document in collabora'], + 'operation' => 'preview in collabora', + 'status' => 0, + 'scope' => 'own', + ], + 'FAIL preview:published:own::preview_own' => [ + 'result' => FALSE, + 'permissions' => [], + 'group_permissions' => ['preview own unpublished group_media:document in collabora'], + 'operation' => 'preview in collabora', + 'status' => 1, + 'scope' => 'own', + ], + 'edit:published:any:::' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => [], 'operation' => 'edit in collabora', + 'status' => 1, 'scope' => 'any', ], - 'edit_any_global_permisions' => [ + 'edit:published:any:edit_any::' => [ 'result' => FALSE, 'permissions' => ['edit any document in collabora'], 'group_permissions' => [], 'operation' => 'edit in collabora', + 'status' => 1, 'scope' => 'any', ], - 'edit_any_group_permisions' => [ + 'edit:published:any::edit_any' => [ 'result' => TRUE, 'permissions' => [], 'group_permissions' => ['edit any group_media:document in collabora'], 'operation' => 'edit in collabora', + 'status' => 1, 'scope' => 'any', ], - 'edit_own_no_permisions' => [ + 'edit:published:own::edit_any' => [ + 'result' => TRUE, + 'permissions' => [], + 'group_permissions' => ['edit any group_media:document in collabora'], + 'operation' => 'edit in collabora', + 'status' => 1, + 'scope' => 'own', + ], + 'edit:published:own::' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => [], 'operation' => 'edit in collabora', + 'status' => 1, 'scope' => 'own', ], - 'edit_own_global_permisions' => [ + 'edit:published:own:edit_own:' => [ 'result' => FALSE, 'permissions' => ['edit own document in collabora'], 'group_permissions' => [], 'operation' => 'edit in collabora', + 'status' => 1, 'scope' => 'own', ], - 'edit_own_group_permisions' => [ + 'edit:published:own::edit_own' => [ 'result' => TRUE, 'permissions' => [], 'group_permissions' => ['edit own group_media:document in collabora'], 'operation' => 'edit in collabora', + 'status' => 1, 'scope' => 'own', ], + 'edit:published:any::edit_own' => [ + 'result' => FALSE, + 'permissions' => [], + 'group_permissions' => ['edit own group_media:document in collabora'], + 'operation' => 'edit in collabora', + 'status' => 1, + 'scope' => 'any', + ], ]; } From 433b65519bbc90736c3634c085de076838e5143d Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Thu, 21 Nov 2024 18:31:54 +0100 Subject: [PATCH 5/7] Issue 52: Add access control to set preview operation for unpublished entities. --- .../collabora_online_group.services.yml | 6 ++- .../CollaboraAccessControl.php | 42 +++++++++++++++++++ .../CollaboraPermissionProvider.php | 13 ++++-- .../tests/src/Kernel/AccessTest.php | 6 +-- 4 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraAccessControl.php diff --git a/modules/collabora_online_group/collabora_online_group.services.yml b/modules/collabora_online_group/collabora_online_group.services.yml index 06b6737a..25f25af2 100644 --- a/modules/collabora_online_group/collabora_online_group.services.yml +++ b/modules/collabora_online_group/collabora_online_group.services.yml @@ -2,4 +2,8 @@ services: group.relation_handler.permission_provider.collabora_group_media: class: 'Drupal\collabora_online_group\Plugin\Group\RelationHandler\CollaboraPermissionProvider' decorates: group.relation_handler.permission_provider.group_media - arguments: [ '@group.relation_handler.permission_provider.collabora_group_media.inner' ] + arguments: ["@group.relation_handler.permission_provider.collabora_group_media.inner"] + + group.relation_handler.access_control.group_media: + class: 'Drupal\collabora_online_group\Plugin\Group\RelationHandler\CollaboraAccessControl' + arguments: ["@group.relation_handler.access_control"] diff --git a/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraAccessControl.php b/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraAccessControl.php new file mode 100644 index 00000000..e2574362 --- /dev/null +++ b/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraAccessControl.php @@ -0,0 +1,42 @@ +parent = $parent; + } + + /** + * {@inheritdoc} + */ + public function entityAccess(EntityInterface $entity, $operation, AccountInterface $account, $return_as_object = FALSE) { + // Add support for unpublished vs published for "preview in collabora". + $check_published = $operation === 'preview in collabora' && $this->implementsPublishedInterface; + + if ($check_published && !$entity->isPublished()) { + $operation .= ' unpublished'; + } + + return $this->parent->entityAccess($entity, $operation, $account, $return_as_object); + } + +} diff --git a/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraPermissionProvider.php b/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraPermissionProvider.php index 319d27af..8d754ba6 100644 --- a/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraPermissionProvider.php +++ b/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraPermissionProvider.php @@ -25,7 +25,7 @@ public function buildPermissions(): array { if ($name = $provider_chain->getPermission('preview in collabora', 'entity')) { $permissions[$name] = $this->buildPermission("$prefix Preview published %entity_type in collabora"); } - if ($name = $provider_chain->getPermission('preview in collabora', 'entity', 'own')) { + if ($name = $provider_chain->getPermission('preview in collabora unpublished', 'entity', 'own')) { $permissions[$name] = $this->buildPermission("$prefix Preview own unpublished %entity_type in collabora"); } if ($name = $provider_chain->getPermission('edit in collabora', 'entity')) { @@ -49,11 +49,18 @@ public function getPermission($operation, $target, $scope = 'any'): bool|string ) { switch ($operation) { case 'preview in collabora': - if ($scope === 'own') { + if ($scope === 'any') { + return "preview $this->pluginId in collabora"; + } + + return FALSE; + + case 'preview in collabora unpublished': + if ($this->implementsPublishedInterface && $scope === 'own') { return "preview $scope unpublished $this->pluginId in collabora"; } - return "preview $this->pluginId in collabora"; + return FALSE; case 'edit in collabora': return "edit $scope $this->pluginId in collabora"; diff --git a/modules/collabora_online_group/tests/src/Kernel/AccessTest.php b/modules/collabora_online_group/tests/src/Kernel/AccessTest.php index ee3fc3e1..c2d9e6b2 100644 --- a/modules/collabora_online_group/tests/src/Kernel/AccessTest.php +++ b/modules/collabora_online_group/tests/src/Kernel/AccessTest.php @@ -137,7 +137,7 @@ protected function getTestScenarios(): array { 'status' => 1, 'scope' => 'own', ], - 'FAIL preview:unpublished:any::preview' => [ + 'preview:unpublished:any::preview' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => ['preview group_media:document in collabora'], @@ -145,7 +145,7 @@ protected function getTestScenarios(): array { 'status' => 0, 'scope' => 'any', ], - 'FAIL preview:unpublished:own::preview' => [ + 'preview:unpublished:own::preview' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => ['preview group_media:document in collabora'], @@ -169,7 +169,7 @@ protected function getTestScenarios(): array { 'status' => 0, 'scope' => 'own', ], - 'FAIL preview:published:own::preview_own' => [ + 'preview:published:own::preview_own' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => ['preview own unpublished group_media:document in collabora'], From a06d92ae1f261d897923ab214bf43b67ff334677 Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Fri, 22 Nov 2024 09:32:13 +0100 Subject: [PATCH 6/7] Issue 52: Add access check for links in view. --- .../views.view.test_collabora_links.yml | 31 +++++----- tests/src/Kernel/ViewsLinkFieldsTest.php | 58 ++++++++++++------- 2 files changed, 54 insertions(+), 35 deletions(-) diff --git a/tests/modules/collabora_online_test/config/optional/views.view.test_collabora_links.yml b/tests/modules/collabora_online_test/config/optional/views.view.test_collabora_links.yml index 7f79189b..bb423faf 100644 --- a/tests/modules/collabora_online_test/config/optional/views.view.test_collabora_links.yml +++ b/tests/modules/collabora_online_test/config/optional/views.view.test_collabora_links.yml @@ -199,27 +199,30 @@ display: sort_asc_label: Asc sort_desc_label: Desc access: - type: perm - options: - perm: "view media" + type: none + options: { } cache: type: tag options: {} empty: {} - sorts: {} - arguments: {} - filters: - status: - id: status + sorts: + mid: + id: mid table: media_field_data - field: status + field: mid + relationship: none + group_type: group + admin_label: '' entity_type: media - entity_field: status - plugin_id: boolean - value: "1" - group: 1 + entity_field: mid + plugin_id: standard + order: ASC expose: - operator: "" + label: '' + field_identifier: '' + exposed: false + arguments: {} + filters: {} style: type: default options: diff --git a/tests/src/Kernel/ViewsLinkFieldsTest.php b/tests/src/Kernel/ViewsLinkFieldsTest.php index e7dc336d..10c4cbc2 100644 --- a/tests/src/Kernel/ViewsLinkFieldsTest.php +++ b/tests/src/Kernel/ViewsLinkFieldsTest.php @@ -22,13 +22,6 @@ class ViewsLinkFieldsTest extends KernelTestBase { use MediaCreationTrait; use MediaTypeCreationTrait; - /** - * Media owned by current user. - * - * @var \Drupal\media\MediaInterface - */ - protected $ownMedia = NULL; - /** * {@inheritdoc} */ @@ -58,10 +51,6 @@ protected function setUp(): void { // Install user module to avoid user 1 permissions bypass. \Drupal::moduleHandler()->loadInclude('user', 'install'); user_install(); - - // Create two medias to check access with different scopes: any and own. - $this->createMediaEntity('document'); - $this->ownMedia = $this->createMediaEntity('document'); } /** @@ -71,26 +60,37 @@ public function testLinks(): void { // User without permissions can't see links. $this->doTestLinks( [ - 'preview' => [FALSE, FALSE], - 'edit' => [FALSE, FALSE], + 'preview' => [FALSE, FALSE, FALSE, FALSE], + 'edit' => [FALSE, FALSE, FALSE, FALSE], ], $this->createUser([]) ); // User with 'Preview' permission can see preview link. $this->doTestLinks( [ - 'preview' => [TRUE, TRUE], - 'edit' => [FALSE, FALSE], + 'preview' => [TRUE, FALSE, TRUE, FALSE], + 'edit' => [FALSE, FALSE, FALSE, FALSE], ], $this->createUser([ 'preview document in collabora', ]) ); + // User with 'Preview own unpublished' permission can see preview link + // for unpublished entity they own. + $this->doTestLinks( + [ + 'preview' => [FALSE, FALSE, FALSE, TRUE], + 'edit' => [FALSE, FALSE, FALSE, FALSE], + ], + $this->createUser([ + 'preview own unpublished document in collabora', + ]) + ); // User with 'Edit any' permission can see edit link. $this->doTestLinks( [ - 'preview' => [FALSE, FALSE], - 'edit' => [TRUE, TRUE], + 'preview' => [FALSE, FALSE, FALSE, FALSE], + 'edit' => [TRUE, TRUE, TRUE, TRUE], ], $this->createUser([ 'edit any document in collabora', @@ -100,8 +100,8 @@ public function testLinks(): void { // own. $this->doTestLinks( [ - 'preview' => [FALSE, FALSE], - 'edit' => [FALSE, TRUE], + 'preview' => [FALSE, FALSE, FALSE, FALSE], + 'edit' => [FALSE, FALSE, TRUE, TRUE], ], $this->createUser([ 'edit own document in collabora', @@ -119,8 +119,22 @@ public function testLinks(): void { */ protected function doTestLinks(array $expected_results, AccountInterface $account): void { $this->setCurrentUser($account); - // Set the current user as the owner to check 'edit own' access. - $this->ownMedia->setOwnerId($account->id())->save(); + // Create medias: cover all combinations of status and ownership. + $this->createMediaEntity('document', [ + 'uid' => $this->createUser(), + ]); + $this->createMediaEntity('document', [ + 'uid' => $this->createUser(), + 'status' => 0, + ]); + $this->createMediaEntity('document', [ + 'uid' => $account->id(), + ]); + $this->createMediaEntity('document', [ + 'uid' => $account->id(), + 'status' => 0, + ]); + $view = Views::getView('test_collabora_links'); $view->preview(); @@ -152,6 +166,8 @@ protected function doTestLinks(array $expected_results, AccountInterface $accoun $this->assertEquals($expected_link, (string) $link); } $i++; + // Clean medias as we check results. + $media->delete(); } } From 23a00ae7f80380e82e77b7faa9fd1ecd48fe6c6f Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Fri, 22 Nov 2024 10:46:20 +0100 Subject: [PATCH 7/7] Issue 52: Add more cases to access check and improve docs. --- .../CollaboraAccessControl.php | 9 +- .../tests/src/Kernel/AccessTest.php | 123 +++++++++++++++--- 2 files changed, 110 insertions(+), 22 deletions(-) diff --git a/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraAccessControl.php b/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraAccessControl.php index e2574362..3694007f 100644 --- a/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraAccessControl.php +++ b/modules/collabora_online_group/src/Plugin/Group/RelationHandler/CollaboraAccessControl.php @@ -1,7 +1,10 @@ implementsPublishedInterface; if ($check_published && !$entity->isPublished()) { diff --git a/modules/collabora_online_group/tests/src/Kernel/AccessTest.php b/modules/collabora_online_group/tests/src/Kernel/AccessTest.php index c2d9e6b2..b4d6e231 100644 --- a/modules/collabora_online_group/tests/src/Kernel/AccessTest.php +++ b/modules/collabora_online_group/tests/src/Kernel/AccessTest.php @@ -105,7 +105,8 @@ protected function getTestScenarios(): array { // The scenario keys contains values used for each scenario: // 'operation:status:scope:global_permission:group_permission'. return [ - 'preview:published:any:::' => [ + // Preview no permissions cases. + 'preview:published:any::' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => [], @@ -113,7 +114,17 @@ protected function getTestScenarios(): array { 'status' => 1, 'scope' => 'any', ], - 'preview:published:any:preview::' => [ + 'preview:published:own::' => [ + 'result' => FALSE, + 'permissions' => [], + 'group_permissions' => [], + 'operation' => 'preview in collabora', + 'status' => 1, + 'scope' => 'own', + ], + // The global permissions that would allow to preview, doesn't work + // in a media related to a group. + 'preview:published:any:preview:' => [ 'result' => FALSE, 'permissions' => ['preview document in collabora'], 'group_permissions' => [], @@ -121,6 +132,16 @@ protected function getTestScenarios(): array { 'status' => 1, 'scope' => 'any', ], + 'preview:published:own:preview:' => [ + 'result' => FALSE, + 'permissions' => ['preview document in collabora'], + 'group_permissions' => [], + 'operation' => 'preview in collabora', + 'status' => 1, + 'scope' => 'own', + ], + // User can only see published entities with the group preview + // permission. 'preview:published:any::preview' => [ 'result' => TRUE, 'permissions' => [], @@ -153,7 +174,9 @@ protected function getTestScenarios(): array { 'status' => 0, 'scope' => 'own', ], - 'preview:unpublished:own:preview_own::' => [ + // The global preview unpublished doesn't affect to medias related + // to a group. + 'preview:unpublished:own:preview_own_unpublished:' => [ 'result' => FALSE, 'permissions' => ['preview own unpublished document in collabora'], 'group_permissions' => [], @@ -161,7 +184,25 @@ protected function getTestScenarios(): array { 'status' => 0, 'scope' => 'own', ], - 'preview:unpublished:own::preview_own' => [ + // The group permission to preview own unpublished permission allows + // to see only entities with such properties. + 'preview:published:any::preview_own_unpublished' => [ + 'result' => FALSE, + 'permissions' => [], + 'group_permissions' => ['preview own unpublished group_media:document in collabora'], + 'operation' => 'preview in collabora', + 'status' => 1, + 'scope' => 'any', + ], + 'preview:published:own::preview_own_unpublished' => [ + 'result' => FALSE, + 'permissions' => [], + 'group_permissions' => ['preview own unpublished group_media:document in collabora'], + 'operation' => 'preview in collabora', + 'status' => 1, + 'scope' => 'own', + ], + 'preview:unpublished:own::preview_own_unpublished' => [ 'result' => TRUE, 'permissions' => [], 'group_permissions' => ['preview own unpublished group_media:document in collabora'], @@ -169,15 +210,16 @@ protected function getTestScenarios(): array { 'status' => 0, 'scope' => 'own', ], - 'preview:published:own::preview_own' => [ + 'preview:unpublished:any::preview_own_unpublished' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => ['preview own unpublished group_media:document in collabora'], 'operation' => 'preview in collabora', - 'status' => 1, - 'scope' => 'own', + 'status' => 0, + 'scope' => 'any', ], - 'edit:published:any:::' => [ + // Edit no permissions cases. + 'edit:published:any::' => [ 'result' => FALSE, 'permissions' => [], 'group_permissions' => [], @@ -185,7 +227,16 @@ protected function getTestScenarios(): array { 'status' => 1, 'scope' => 'any', ], - 'edit:published:any:edit_any::' => [ + 'edit:published:own::' => [ + 'result' => FALSE, + 'permissions' => [], + 'group_permissions' => [], + 'operation' => 'edit in collabora', + 'status' => 1, + 'scope' => 'own', + ], + // The global permission doesn't grant access to edit in a group. + 'edit:published:any:edit_any:' => [ 'result' => FALSE, 'permissions' => ['edit any document in collabora'], 'group_permissions' => [], @@ -193,6 +244,23 @@ protected function getTestScenarios(): array { 'status' => 1, 'scope' => 'any', ], + 'edit:published:own:edit_any:' => [ + 'result' => FALSE, + 'permissions' => ['edit any document in collabora'], + 'group_permissions' => [], + 'operation' => 'edit in collabora', + 'status' => 1, + 'scope' => 'own', + ], + 'edit:published:own:edit_own:' => [ + 'result' => FALSE, + 'permissions' => ['edit own document in collabora'], + 'group_permissions' => [], + 'operation' => 'edit in collabora', + 'status' => 1, + 'scope' => 'own', + ], + // Only users with edit any permission in a group can edit all. 'edit:published:any::edit_any' => [ 'result' => TRUE, 'permissions' => [], @@ -209,22 +277,23 @@ protected function getTestScenarios(): array { 'status' => 1, 'scope' => 'own', ], - 'edit:published:own::' => [ - 'result' => FALSE, + 'edit:unpublished:any::edit_any' => [ + 'result' => TRUE, 'permissions' => [], - 'group_permissions' => [], + 'group_permissions' => ['edit any group_media:document in collabora'], 'operation' => 'edit in collabora', - 'status' => 1, - 'scope' => 'own', + 'status' => 0, + 'scope' => 'any', ], - 'edit:published:own:edit_own:' => [ - 'result' => FALSE, - 'permissions' => ['edit own document in collabora'], - 'group_permissions' => [], + 'edit:unpublished:own::edit_any' => [ + 'result' => TRUE, + 'permissions' => [], + 'group_permissions' => ['edit any group_media:document in collabora'], 'operation' => 'edit in collabora', - 'status' => 1, + 'status' => 0, 'scope' => 'own', ], + // Or edit own permission for the entities the user owns. 'edit:published:own::edit_own' => [ 'result' => TRUE, 'permissions' => [], @@ -233,6 +302,14 @@ protected function getTestScenarios(): array { 'status' => 1, 'scope' => 'own', ], + 'edit:unpublished:own::edit_own' => [ + 'result' => TRUE, + 'permissions' => [], + 'group_permissions' => ['edit own group_media:document in collabora'], + 'operation' => 'edit in collabora', + 'status' => 0, + 'scope' => 'own', + ], 'edit:published:any::edit_own' => [ 'result' => FALSE, 'permissions' => [], @@ -241,6 +318,14 @@ protected function getTestScenarios(): array { 'status' => 1, 'scope' => 'any', ], + 'edit:unpublished:any::edit_own' => [ + 'result' => FALSE, + 'permissions' => [], + 'group_permissions' => ['edit own group_media:document in collabora'], + 'operation' => 'edit in collabora', + 'status' => 0, + 'scope' => 'any', + ], ]; }