From 53d63b0f947322830babbad7b814824ad9353fc9 Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Thu, 7 Nov 2024 13:30:38 +0100 Subject: [PATCH 1/8] Issue #49: Create kernel tests for permissions and media access. --- tests/src/Functional/AccessTest.php | 8 + tests/src/Kernel/CollaboraMediaAccessTest.php | 312 ++++++++++++++++++ tests/src/Kernel/PermissionTest.php | 106 ++++++ 3 files changed, 426 insertions(+) create mode 100644 tests/src/Kernel/CollaboraMediaAccessTest.php create mode 100644 tests/src/Kernel/PermissionTest.php diff --git a/tests/src/Functional/AccessTest.php b/tests/src/Functional/AccessTest.php index 816e8f99..1e5d1b93 100644 --- a/tests/src/Functional/AccessTest.php +++ b/tests/src/Functional/AccessTest.php @@ -28,6 +28,14 @@ /** * Tests access to collabora routes. + * + * There is a kernel test with a similar purpose. + * The kernel test is (obviously) a lot faster, while this functional test might + * be more realistic. + * For now, both of these tests are kept, but most new development will happen + * in the kernel test. + * + * @see \Drupal\Tests\collabora_online\Kernel\CollaboraMediaAccessTest */ class AccessTest extends BrowserTestBase { diff --git a/tests/src/Kernel/CollaboraMediaAccessTest.php b/tests/src/Kernel/CollaboraMediaAccessTest.php new file mode 100644 index 00000000..f004f772 --- /dev/null +++ b/tests/src/Kernel/CollaboraMediaAccessTest.php @@ -0,0 +1,312 @@ +installEntitySchema('user'); + $this->installEntitySchema('file'); + $this->installSchema('file', 'file_usage'); + $this->installEntitySchema('media'); + $this->installConfig([ + 'field', + 'system', + 'user', + 'image', + 'file', + 'media', + ]); + + $this->createMediaType('file', ['id' => 'document']); + $this->createMediaType('file', ['id' => 'book']); + + // Consume the user id 1. + $this->createUser(); + } + + /** + * Tests media access for Collabora routes and operations. + */ + public function testCollaboraMediaAccess(): void { + $this->assertBasicAssumptions(); + + /** @var \Drupal\Core\Session\AccountInterface[] $accounts */ + $accounts = [ + // The default roles for anonymous and authenticated are not created + // in a kernel test. Therefore these users don't get any kind of + // access. + 'anonymous' => new AnonymousUserSession(), + 'authenticated' => $this->createUser(), + // Media permission do not grant access to Collabora operations. + 'media permissions only' => $this->createUser([ + 'administer media types', + 'view media', + 'update any media', + 'view own unpublished media', + ]), + // Permissions for 'book' do not grant access to 'document'. + 'Bookworm' => $this->createUser([ + 'preview book in collabora', + 'edit any book in collabora', + 'edit own book in collabora', + ]), + 'Previewer' => $this->createUser([ + 'preview document in collabora', + ]), + 'Editor' => $this->createUser([ + 'edit any document in collabora', + ]), + 'Kelly (edit own)' => $this->createUser([ + 'edit own document in collabora', + ]), + 'Media admin' => $this->createUser([ + 'administer media', + ]), + ]; + + /** @var \Drupal\media\MediaInterface[] $media_entities */ + $media_entities = [ + 'published document' => $this->createMediaEntity('document'), + 'unpublished document' => $this->createMediaEntity('document', [ + 'status' => 0, + ]), + "Kelly's published document" => $this->createMediaEntity('document', [ + 'uid' => $accounts['Kelly (edit own)']->id(), + ]), + "Kelly's unpublished document" => $this->createMediaEntity('document', [ + 'uid' => $accounts['Kelly (edit own)']->id(), + 'status' => 0, + ]), + ]; + + $this->assertEntityAccess( + [ + 'published document' => [ + 'preview in collabora' => ['Previewer', 'Media admin'], + 'edit in collabora' => ['Editor', 'Media admin'], + ], + 'unpublished document' => [ + 'preview in collabora' => ['Previewer', 'Media admin'], + 'edit in collabora' => ['Editor', 'Media admin'], + ], + "Kelly's published document" => [ + 'preview in collabora' => ['Previewer', 'Media admin'], + 'edit in collabora' => ['Editor', 'Kelly (edit own)', 'Media admin'], + ], + "Kelly's unpublished document" => [ + 'preview in collabora' => ['Previewer', 'Media admin'], + 'edit in collabora' => ['Editor', 'Kelly (edit own)', 'Media admin'], + ], + ], + $accounts, + $media_entities, + [ + 'preview in collabora', + 'edit in collabora', + ], + ); + + $this->assertEntityPathsAccess( + [ + "/cool/view/" => ['Previewer', 'Media admin'], + "/cool/edit/" => ['Editor', 'Media admin'], + "/cool/view/" => ['Previewer', 'Media admin'], + "/cool/edit/" => ['Editor', 'Media admin'], + "/cool/view/" => ['Previewer', 'Media admin'], + "/cool/edit/" => ['Editor', 'Kelly (edit own)', 'Media admin'], + "/cool/view/" => ['Previewer', 'Media admin'], + "/cool/edit/" => ['Editor', 'Kelly (edit own)', 'Media admin'], + ], + $accounts, + $media_entities, + [ + '/cool/view/%s', + '/cool/edit/%s', + ], + ); + } + + /** + * Asserts which users can access which entity operations. + * + * @param array>> $expected + * Expected outcome. + * The array is keyed by entity key and operation. + * The values are lists of keys from the $accounts parameter. + * @param array $accounts + * Entities to check. + * @param array $entities + * Operations to check. + * @param list $operations + * User accounts to check. + */ + protected function assertEntityAccess(array $expected, array $accounts, array $entities, array $operations): void { + $actual = []; + foreach ($entities as $media_key => $media) { + foreach ($operations as $operation) { + foreach ($accounts as $account_key => $account) { + $has_access = $media->access($operation, $account); + if ($has_access) { + $actual[$media_key][$operation][] = $account_key; + } + } + } + } + // Use yaml to avoid integer keys in list output. + $this->assertSame( + "\n" . Yaml::encode($expected), + "\n" . Yaml::encode($actual), + 'Users with access to given entities', + ); + } + + /** + * Asserts which users have access to which entity paths. + * + * @param array> $expected + * Array indicating which url should be accessible by which user. + * The array keys are either string keys from the $paths array. + * The array values are lists of keys from the $accounts array with access + * to that path. + * @param array $accounts + * Accounts to test access with, keyed by a distinguishable name. + * @param array $entities + * Entities for which to build paths. + * @param array $sprintf_path_patterns + * Path patterns with '%s' placeholder for the entity id. + */ + protected function assertEntityPathsAccess(array $expected, array $accounts, array $entities, array $sprintf_path_patterns) { + $paths = []; + // Build Collabora media paths for all media entities. + foreach ($entities as $entity_key => $entity) { + foreach ($sprintf_path_patterns as $pattern) { + $paths[sprintf($pattern, "<$entity_key>")] = sprintf($pattern, $entity->id()); + } + } + $this->assertPathsAccessByUsers($expected, $accounts, $paths); + } + + /** + * Asserts which users have access to which paths. + * + * @param array> $expected + * Array indicating which url should be accessible by which user. + * The array keys are either paths or string keys from the $paths array. + * The array values are lists of keys from the $accounts array with access + * to that path. + * @param array $accounts + * Accounts to test access with, keyed by a distinguishable name. + * @param array|null $paths + * An array of paths, or NULL to just use the array keys from $expected. + * This parameter is useful if the paths all look very similar. + */ + protected function assertPathsAccessByUsers(array $expected, array $accounts, ?array $paths = NULL): void { + if ($paths === NULL) { + $paths = array_keys($expected); + $paths = array_combine($paths, $paths); + } + // Build a report and assert it all at once, to have a more complete + // overview on failure. + $actual = []; + foreach ($paths as $path_key => $path) { + $url = Url::fromUserInput($path); + // Filter the user list by access to the url. + foreach ($accounts as $account_key => $account) { + $has_access = $url->access($account); + if ($has_access) { + $actual[$path_key][] = $account_key; + } + } + } + // Use yaml to avoid integer keys in list output. + $this->assertSame( + "\n" . Yaml::encode($expected), + "\n" . Yaml::encode($actual), + 'Users with access to given paths', + ); + } + + /** + * Verifies basic assumptions for this test. + * + * It is enough to call this from one test method. + */ + protected function assertBasicAssumptions(): void { + // Normally, when media module is installed, media_install() will grant + // the 'view media' permission to anonymous and authenticated roles. + // In a kernel test this does not happen. + $this->assertSame([], Role::load(RoleInterface::ANONYMOUS_ID)->getPermissions()); + $this->assertSame([], Role::load(RoleInterface::AUTHENTICATED_ID)->getPermissions()); + + // The first user created in a test method is not user 1. + $user = $this->createUser(); + $this->assertSame(2, (int) $user->id()); + $this->assertFalse($user->hasPermission('view media')); + } + + /** + * 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; + } + +} diff --git a/tests/src/Kernel/PermissionTest.php b/tests/src/Kernel/PermissionTest.php new file mode 100644 index 00000000..b8004b1e --- /dev/null +++ b/tests/src/Kernel/PermissionTest.php @@ -0,0 +1,106 @@ +installEntitySchema('user'); + $this->installEntitySchema('file'); + $this->installSchema('file', 'file_usage'); + $this->installEntitySchema('media'); + $this->installConfig(['field', 'system', 'user', 'file', 'media']); + } + + /** + * Tests that dynamic permissions are properly created. + */ + public function testDynamicPermissions(): void { + $this->createMediaType('file', [ + 'id' => 'public_wiki', + 'label' => 'Public wiki', + ]); + /** @var \Drupal\user\PermissionHandlerInterface $permission_handler */ + $permission_handler = \Drupal::service(PermissionHandlerInterface::class); + $permissions = $permission_handler->getPermissions(); + $permissions = array_filter( + $permissions, + fn (array $permission) => $permission['provider'] === 'collabora_online', + ); + // Remove noise that is hard to diff. + $permissions = array_map( + static function (array $permission) { + $permission['title'] = (string) $permission['title']; + if ($permission['description'] === NULL) { + unset($permission['description']); + } + if ($permission['provider'] === 'collabora_online') { + unset($permission['provider']); + } + return $permission; + }, + $permissions, + ); + ksort($permissions); + $this->assertSame([ + 'administer collabora instance' => [ + 'title' => 'Administer the Collabora instance', + 'restrict access' => TRUE, + ], + 'edit any public_wiki in collabora' => [ + 'title' => 'Public wiki: Edit any media file in Collabora', + 'dependencies' => ['config' => ['media.type.public_wiki']], + ], + 'edit own public_wiki in collabora' => [ + 'title' => 'Public wiki: Edit own media file in Collabora', + 'dependencies' => ['config' => ['media.type.public_wiki']], + ], + 'preview public_wiki in collabora' => [ + 'title' => 'Public wiki: Preview media file in Collabora', + 'dependencies' => ['config' => ['media.type.public_wiki']], + ], + ], $permissions); + } + +} From fe54f3b8307d57c94aeb82d4d45d045b7feb9adc Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Tue, 12 Nov 2024 15:32:04 +0100 Subject: [PATCH 2/8] Issue #49: Transform array structure in the test. --- tests/src/Kernel/CollaboraMediaAccessTest.php | 94 +++++++++++++------ 1 file changed, 64 insertions(+), 30 deletions(-) diff --git a/tests/src/Kernel/CollaboraMediaAccessTest.php b/tests/src/Kernel/CollaboraMediaAccessTest.php index f004f772..053f5c50 100644 --- a/tests/src/Kernel/CollaboraMediaAccessTest.php +++ b/tests/src/Kernel/CollaboraMediaAccessTest.php @@ -120,21 +120,31 @@ public function testCollaboraMediaAccess(): void { $this->assertEntityAccess( [ - 'published document' => [ - 'preview in collabora' => ['Previewer', 'Media admin'], - 'edit in collabora' => ['Editor', 'Media admin'], + 'anonymous' => [], + 'authenticated' => [], + 'media permissions only' => [], + 'Bookworm' => [], + 'Previewer' => [ + "published document" => ['preview in collabora'], + "unpublished document" => ['preview in collabora'], + "Kelly's published document" => ['preview in collabora'], + "Kelly's unpublished document" => ['preview in collabora'], ], - 'unpublished document' => [ - 'preview in collabora' => ['Previewer', 'Media admin'], - 'edit in collabora' => ['Editor', 'Media admin'], + 'Editor' => [ + "published document" => ['edit in collabora'], + "unpublished document" => ['edit in collabora'], + "Kelly's published document" => ['edit in collabora'], + "Kelly's unpublished document" => ['edit in collabora'], ], - "Kelly's published document" => [ - 'preview in collabora' => ['Previewer', 'Media admin'], - 'edit in collabora' => ['Editor', 'Kelly (edit own)', 'Media admin'], + 'Kelly (edit own)' => [ + "Kelly's published document" => ['edit in collabora'], + "Kelly's unpublished document" => ['edit in collabora'], ], - "Kelly's unpublished document" => [ - 'preview in collabora' => ['Previewer', 'Media admin'], - 'edit in collabora' => ['Editor', 'Kelly (edit own)', 'Media admin'], + 'Media admin' => [ + "published document" => ['preview in collabora', 'edit in collabora'], + "unpublished document" => ['preview in collabora', 'edit in collabora'], + "Kelly's published document" => ['preview in collabora', 'edit in collabora'], + "Kelly's unpublished document" => ['preview in collabora', 'edit in collabora'], ], ], $accounts, @@ -147,14 +157,36 @@ public function testCollaboraMediaAccess(): void { $this->assertEntityPathsAccess( [ - "/cool/view/" => ['Previewer', 'Media admin'], - "/cool/edit/" => ['Editor', 'Media admin'], - "/cool/view/" => ['Previewer', 'Media admin'], - "/cool/edit/" => ['Editor', 'Media admin'], - "/cool/view/" => ['Previewer', 'Media admin'], - "/cool/edit/" => ['Editor', 'Kelly (edit own)', 'Media admin'], - "/cool/view/" => ['Previewer', 'Media admin'], - "/cool/edit/" => ['Editor', 'Kelly (edit own)', 'Media admin'], + 'anonymous' => [], + 'authenticated' => [], + 'media permissions only' => [], + 'Bookworm' => [], + 'Previewer' => [ + "/cool/view/", + "/cool/view/", + "/cool/view/", + "/cool/view/", + ], + 'Editor' => [ + "/cool/edit/", + "/cool/edit/", + "/cool/edit/", + "/cool/edit/", + ], + 'Kelly (edit own)' => [ + "/cool/edit/", + "/cool/edit/", + ], + 'Media admin' => [ + "/cool/view/", + "/cool/edit/", + "/cool/view/", + "/cool/edit/", + "/cool/view/", + "/cool/edit/", + "/cool/view/", + "/cool/edit/", + ], ], $accounts, $media_entities, @@ -181,12 +213,13 @@ public function testCollaboraMediaAccess(): void { */ protected function assertEntityAccess(array $expected, array $accounts, array $entities, array $operations): void { $actual = []; - foreach ($entities as $media_key => $media) { - foreach ($operations as $operation) { - foreach ($accounts as $account_key => $account) { - $has_access = $media->access($operation, $account); + foreach ($accounts as $account_key => $account) { + $actual[$account_key] = []; + foreach ($entities as $entity_key => $entity) { + foreach ($operations as $operation) { + $has_access = $entity->access($operation, $account); if ($has_access) { - $actual[$media_key][$operation][] = $account_key; + $actual[$account_key][$entity_key][] = $operation; } } } @@ -247,13 +280,14 @@ protected function assertPathsAccessByUsers(array $expected, array $accounts, ?a // Build a report and assert it all at once, to have a more complete // overview on failure. $actual = []; - foreach ($paths as $path_key => $path) { - $url = Url::fromUserInput($path); - // Filter the user list by access to the url. - foreach ($accounts as $account_key => $account) { + foreach ($accounts as $account_key => $account) { + $actual[$account_key] = []; + foreach ($paths as $path_key => $path) { + $url = Url::fromUserInput($path); + // Filter the user list by access to the url. $has_access = $url->access($account); if ($has_access) { - $actual[$path_key][] = $account_key; + $actual[$account_key][] = $path_key; } } } From 7e3bda67f28f4fc2cf017b04cb59a7391f0de981 Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Tue, 12 Nov 2024 16:20:21 +0100 Subject: [PATCH 3/8] Issue #49: Test access for anonymous with permissions. --- tests/src/Kernel/CollaboraMediaAccessTest.php | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tests/src/Kernel/CollaboraMediaAccessTest.php b/tests/src/Kernel/CollaboraMediaAccessTest.php index 053f5c50..9c5e113f 100644 --- a/tests/src/Kernel/CollaboraMediaAccessTest.php +++ b/tests/src/Kernel/CollaboraMediaAccessTest.php @@ -197,6 +197,76 @@ public function testCollaboraMediaAccess(): void { ); } + /** + * Tests a scenario where the anonymous user has more permissions. + */ + public function testAnonymousOwnAccess(): void { + user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, [ + 'edit own document in collabora', + ]); + + /** @var \Drupal\Core\Session\AccountInterface[] $accounts */ + $accounts = [ + 'anonymous' => new AnonymousUserSession(), + 'Emilia' => $this->createUser(), + ]; + + /** @var \Drupal\media\MediaInterface[] $media_entities */ + $media_entities = [ + // Set uid = 0 to verify that anonymous is not seen as the owner. + "published document" => $this->createMediaEntity('document', [ + 'uid' => 0, + ]), + "unpublished document" => $this->createMediaEntity('document', [ + 'uid' => 0, + 'status' => 0, + ]), + "Emilia's published document" => $this->createMediaEntity('document', [ + 'uid' => $accounts['Emilia']->id(), + ]), + "Emilia's unpublished document" => $this->createMediaEntity('document', [ + 'uid' => $accounts['Emilia']->id(), + 'status' => 0, + ]), + ]; + + $this->assertEntityAccess( + [ + 'anonymous' => [], + 'Emilia' => [], + ], + $accounts, + $media_entities, + [ + 'preview in collabora', + 'edit in collabora', + ], + ); + + user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, [ + 'preview document in collabora', + 'edit any document in collabora', + ]); + + $this->assertEntityAccess( + [ + 'anonymous' => [ + "published document" => ['preview in collabora', 'edit in collabora'], + "unpublished document" => ['preview in collabora', 'edit in collabora'], + "Emilia's published document" => ['preview in collabora', 'edit in collabora'], + "Emilia's unpublished document" => ['preview in collabora', 'edit in collabora'], + ], + 'Emilia' => [], + ], + $accounts, + $media_entities, + [ + 'preview in collabora', + 'edit in collabora', + ], + ); + } + /** * Asserts which users can access which entity operations. * From c1b0253d60c3d99da5235922e491aa376c165f00 Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Tue, 12 Nov 2024 15:38:10 +0100 Subject: [PATCH 4/8] Issue #49: Introduce new 'preview own unpublished ...' permission. Also remove the recommendation section in the README. --- README.md | 39 ++++++++------- collabora_online.module | 50 +++++++++++++++---- src/CollaboraMediaPermissions.php | 5 +- tests/src/Functional/PermissionTest.php | 6 ++- tests/src/Kernel/CollaboraMediaAccessTest.php | 50 +++++++++++-------- tests/src/Kernel/PermissionTest.php | 6 ++- 6 files changed, 105 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index 40f45d29..83c72505 100644 --- a/README.md +++ b/README.md @@ -197,24 +197,27 @@ Collabora is used within Drupal. Most of the time this permission is not needed, if the Collabora instance is configured from outside of Drupal. -For each media type, the module introduces two permissions: -- (media type): Edit any media file in Collabora Users with this - permission are allowed to edit documents attached to a media entity - of the given type, using the Collabora Online editor. -- (media type): Preview media file in Collabora Users with this - permission are allowed to preview documents attached to a media - entity of the given type, using the Collabora Online editor in - preview/readonly mode. - -In the current version, preview and edit access with the Collabora -Online editor are checked independently of the publishing status of -the respective media, and independently of the regular view or edit -access on that media entity. - -For a consistent experience, it is recommended that a role with the -edit permission should also be granted the preview permission, and -that a user with any of the Collabora media permissions should also be -granted permissions to view the respective media entity in Drupal. +For each media type, the module introduces four permissions: +- "(media type): Edit any media file in Collabora" + Users with this permission are allowed to edit documents attached + to a media entity of the given type, using the Collabora Online + editor. +- "(media type): Edit own media file in Collabora" + Users with this permission are allowed to edit documents attached + to a media entity of the given type, using the Collabora Online + editor, if they are the owner/author of that media entity. +- "(media type): Preview published media file in Collabora" + Users with this permission are allowed to preview documents attached + to a published media entity of the given type, using the Collabora + Online editor in preview/readonly mode. +- "(media type): Preview own unpublished media file in Collabora" + Users with this permission are allowed to preview documents attached + to an unpublished media entity of the given type, using the Collabora Online + editor in preview/readonly mode. + +In the current version of this module, the 'administer media' permission +from Drupal core grants access to all media operations, including the use +of the Collabora Online editor for preview and edit. Developers can use entity access hooks to alter which users may edit or preview media files in Collabora. This would allow to grant access diff --git a/collabora_online.module b/collabora_online.module index 61662aad..fd2cc452 100644 --- a/collabora_online.module +++ b/collabora_online.module @@ -126,21 +126,51 @@ function collabora_online_media_access(MediaInterface $media, string $operation, $type = $media->bundle(); switch ($operation) { case 'preview in collabora': - return AccessResult::allowedIfHasPermission($account, "preview $type in collabora"); + if ($media->isPublished()) { + return AccessResult::allowedIfHasPermission($account, "preview $type in collabora") + ->addCacheableDependency($media); + } + $preview_own_permission = "preview own unpublished $type in collabora"; + $access_result = AccessResult::allowedIfHasPermission($account, $preview_own_permission) + ->addCacheableDependency($media); + if (!$access_result->isAllowed()) { + return $access_result; + } + // Use '==' because Drupal sometimes loads integers as strings. + $is_owner = ($account->id() && $account->id() == $media->getOwnerId()); + if ($is_owner) { + $access_result = AccessResult::allowed(); + } + else { + $access_result = AccessResult::neutral() + ->setReason("The user has the '$preview_own_permission' permission, but is not the owner of the media item."); + } + return $access_result + ->cachePerUser() + ->addCacheableDependency($media); case 'edit in collabora': if ($account->hasPermission("edit any $type in collabora")) { - return AccessResult::allowed()->cachePerPermissions(); + return AccessResult::allowed() + ->cachePerPermissions(); } - if ($account->hasPermission("edit own $type in collabora")) { - // Use '==' because Drupal sometimes loads integers as strings. - $is_owner = ($account->id() && $account->id() == $media->getOwnerId()); - return AccessResult::allowedIf($is_owner) - ->cachePerPermissions() - ->cachePerUser() - ->addCacheableDependency($media); + $edit_own_permission = "edit own $type in collabora"; + $access_result = AccessResult::allowedIfHasPermission($account, $edit_own_permission); + if (!$access_result->isAllowed()) { + return $access_result; + } + // Use '==' because Drupal sometimes loads integers as strings. + $is_owner = ($account->id() && $account->id() == $media->getOwnerId()); + if (!$is_owner) { + $access_result = AccessResult::neutral() + ->setReason("The user has the '$edit_own_permission' permission, but is not the owner of the media item."); + } + else { + $access_result = AccessResult::allowed(); } - return AccessResult::neutral()->cachePerPermissions(); + return $access_result + ->cachePerUser() + ->addCacheableDependency($media); default: return AccessResult::neutral(); diff --git a/src/CollaboraMediaPermissions.php b/src/CollaboraMediaPermissions.php index 956948d5..2eecaf31 100644 --- a/src/CollaboraMediaPermissions.php +++ b/src/CollaboraMediaPermissions.php @@ -77,7 +77,10 @@ protected function buildPermissions(MediaTypeInterface $type) { return [ "preview $type_id in collabora" => [ - 'title' => $this->t('%type_name: Preview media file in Collabora', $type_params), + 'title' => $this->t('%type_name: Preview published media file in Collabora', $type_params), + ], + "preview own unpublished $type_id in collabora" => [ + 'title' => $this->t('%type_name: Preview own unpublished media file in Collabora', $type_params), ], "edit own $type_id in collabora" => [ 'title' => $this->t('%type_name: Edit own media file in Collabora', $type_params), diff --git a/tests/src/Functional/PermissionTest.php b/tests/src/Functional/PermissionTest.php index 4023bc1a..2c1f2f29 100644 --- a/tests/src/Functional/PermissionTest.php +++ b/tests/src/Functional/PermissionTest.php @@ -80,8 +80,12 @@ static function (array $permission) { 'title' => 'Public wiki: Edit own media file in Collabora', 'dependencies' => ['config' => ['media.type.public_wiki']], ], + 'preview own unpublished public_wiki in collabora' => [ + 'title' => 'Public wiki: Preview own unpublished media file in Collabora', + 'dependencies' => ['config' => ['media.type.public_wiki']], + ], 'preview public_wiki in collabora' => [ - 'title' => 'Public wiki: Preview media file in Collabora', + 'title' => 'Public wiki: Preview published media file in Collabora', 'dependencies' => ['config' => ['media.type.public_wiki']], ], ], $permissions); diff --git a/tests/src/Kernel/CollaboraMediaAccessTest.php b/tests/src/Kernel/CollaboraMediaAccessTest.php index 9c5e113f..21b63799 100644 --- a/tests/src/Kernel/CollaboraMediaAccessTest.php +++ b/tests/src/Kernel/CollaboraMediaAccessTest.php @@ -86,12 +86,16 @@ public function testCollaboraMediaAccess(): void { // Permissions for 'book' do not grant access to 'document'. 'Bookworm' => $this->createUser([ 'preview book in collabora', + 'preview own unpublished book in collabora', 'edit any book in collabora', 'edit own book in collabora', ]), 'Previewer' => $this->createUser([ 'preview document in collabora', ]), + 'Sean (preview own)' => $this->createUser([ + 'preview own unpublished document in collabora', + ]), 'Editor' => $this->createUser([ 'edit any document in collabora', ]), @@ -105,8 +109,11 @@ public function testCollaboraMediaAccess(): void { /** @var \Drupal\media\MediaInterface[] $media_entities */ $media_entities = [ - 'published document' => $this->createMediaEntity('document'), - 'unpublished document' => $this->createMediaEntity('document', [ + "Sean's published document" => $this->createMediaEntity('document', [ + 'uid' => $accounts['Sean (preview own)']->id(), + ]), + "Sean's unpublished document" => $this->createMediaEntity('document', [ + 'uid' => $accounts['Sean (preview own)']->id(), 'status' => 0, ]), "Kelly's published document" => $this->createMediaEntity('document', [ @@ -125,14 +132,15 @@ public function testCollaboraMediaAccess(): void { 'media permissions only' => [], 'Bookworm' => [], 'Previewer' => [ - "published document" => ['preview in collabora'], - "unpublished document" => ['preview in collabora'], + "Sean's published document" => ['preview in collabora'], "Kelly's published document" => ['preview in collabora'], - "Kelly's unpublished document" => ['preview in collabora'], + ], + 'Sean (preview own)' => [ + "Sean's unpublished document" => ['preview in collabora'], ], 'Editor' => [ - "published document" => ['edit in collabora'], - "unpublished document" => ['edit in collabora'], + "Sean's published document" => ['edit in collabora'], + "Sean's unpublished document" => ['edit in collabora'], "Kelly's published document" => ['edit in collabora'], "Kelly's unpublished document" => ['edit in collabora'], ], @@ -141,8 +149,8 @@ public function testCollaboraMediaAccess(): void { "Kelly's unpublished document" => ['edit in collabora'], ], 'Media admin' => [ - "published document" => ['preview in collabora', 'edit in collabora'], - "unpublished document" => ['preview in collabora', 'edit in collabora'], + "Sean's published document" => ['preview in collabora', 'edit in collabora'], + "Sean's unpublished document" => ['preview in collabora', 'edit in collabora'], "Kelly's published document" => ['preview in collabora', 'edit in collabora'], "Kelly's unpublished document" => ['preview in collabora', 'edit in collabora'], ], @@ -162,14 +170,15 @@ public function testCollaboraMediaAccess(): void { 'media permissions only' => [], 'Bookworm' => [], 'Previewer' => [ - "/cool/view/", - "/cool/view/", + "/cool/view/", "/cool/view/", - "/cool/view/", + ], + 'Sean (preview own)' => [ + "/cool/view/", ], 'Editor' => [ - "/cool/edit/", - "/cool/edit/", + "/cool/edit/", + "/cool/edit/", "/cool/edit/", "/cool/edit/", ], @@ -178,10 +187,10 @@ public function testCollaboraMediaAccess(): void { "/cool/edit/", ], 'Media admin' => [ - "/cool/view/", - "/cool/edit/", - "/cool/view/", - "/cool/edit/", + "/cool/view/", + "/cool/edit/", + "/cool/view/", + "/cool/edit/", "/cool/view/", "/cool/edit/", "/cool/view/", @@ -202,6 +211,7 @@ public function testCollaboraMediaAccess(): void { */ public function testAnonymousOwnAccess(): void { user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, [ + 'preview own unpublished document in collabora', 'edit own document in collabora', ]); @@ -252,9 +262,9 @@ public function testAnonymousOwnAccess(): void { [ 'anonymous' => [ "published document" => ['preview in collabora', 'edit in collabora'], - "unpublished document" => ['preview in collabora', 'edit in collabora'], + "unpublished document" => ['edit in collabora'], "Emilia's published document" => ['preview in collabora', 'edit in collabora'], - "Emilia's unpublished document" => ['preview in collabora', 'edit in collabora'], + "Emilia's unpublished document" => ['edit in collabora'], ], 'Emilia' => [], ], diff --git a/tests/src/Kernel/PermissionTest.php b/tests/src/Kernel/PermissionTest.php index b8004b1e..20f9739a 100644 --- a/tests/src/Kernel/PermissionTest.php +++ b/tests/src/Kernel/PermissionTest.php @@ -96,8 +96,12 @@ static function (array $permission) { 'title' => 'Public wiki: Edit own media file in Collabora', 'dependencies' => ['config' => ['media.type.public_wiki']], ], + 'preview own unpublished public_wiki in collabora' => [ + 'title' => 'Public wiki: Preview own unpublished media file in Collabora', + 'dependencies' => ['config' => ['media.type.public_wiki']], + ], 'preview public_wiki in collabora' => [ - 'title' => 'Public wiki: Preview media file in Collabora', + 'title' => 'Public wiki: Preview published media file in Collabora', 'dependencies' => ['config' => ['media.type.public_wiki']], ], ], $permissions); From e5618605466a18dd518a9928537a7987ff5071de Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Tue, 12 Nov 2024 16:30:45 +0100 Subject: [PATCH 5/8] Issue #49: Drop the old functional tests for access and permissions. --- tests/src/Functional/AccessTest.php | 246 ------------------------ tests/src/Functional/PermissionTest.php | 94 --------- 2 files changed, 340 deletions(-) delete mode 100644 tests/src/Functional/AccessTest.php delete mode 100644 tests/src/Functional/PermissionTest.php diff --git a/tests/src/Functional/AccessTest.php b/tests/src/Functional/AccessTest.php deleted file mode 100644 index 1e5d1b93..00000000 --- a/tests/src/Functional/AccessTest.php +++ /dev/null @@ -1,246 +0,0 @@ -createMediaType('file', ['id' => 'document']); - $media = $this->createMediaEntity('document'); - $media_id = $media->id(); - - $users = [ - 'anonymous' => new AnonymousUserSession(), - 'authenticated' => $this->createUser(), - 'admin' => $this->createUser(admin: TRUE), - ]; - - // Build a report and assert the full result at once. - // This provides a very complete picture in case the assertion fails. - $this->assertPathsAccessByUsers( - [ - // Test the front page as an example that everybody can access. - '/' => ['anonymous', 'authenticated', 'admin'], - // Test an administration page that only admin can see. - '/admin/config' => ['admin'], - // Test the user route. - '/user/' . $users['authenticated']->id() => ['authenticated', 'admin'], - // Test the core media route for reference. - "/media/$media_id/edit" => ['admin'], - ], - $users, - ); - } - - /** - * Tests a scenario when only the administrator has access. - */ - public function testOnlyAdminHasAccess(): void { - $this->createMediaType('file', ['id' => 'document']); - $media = $this->createMediaEntity('document'); - - $users = [ - 'anonymous' => new AnonymousUserSession(), - 'authenticated' => $this->createUser(), - 'admin' => $this->createUser(admin: TRUE), - ]; - - // Both routes are only accessible for admin. - $this->assertPathsAccessByUsers( - [ - '/cool/view/' . $media->id() => ['admin'], - '/cool/edit/' . $media->id() => ['admin'], - ], - $users, - ); - } - - /** - * Tests a scenario where specific permissions are given to users. - */ - public function testCollaboraMediaPermissions(): void { - $this->createMediaType('file', ['id' => 'document']); - $this->createMediaType('file', ['id' => 'public_wiki']); - $this->createMediaType('file', ['id' => 'public_announcement']); - $this->createMediaType('file', ['id' => 'diary']); - $this->grantPermissions( - Role::load(RoleInterface::ANONYMOUS_ID), - [ - 'preview public_announcement in collabora', - 'preview public_wiki in collabora', - 'edit any public_wiki in collabora', - ], - ); - - $accounts = [ - 'anonymous' => new AnonymousUserSession(), - 'authenticated' => $this->createUser(), - 'reader' => $this->createUser([ - 'preview document in collabora', - ]), - 'editor' => $this->createUser([ - 'preview document in collabora', - 'edit any document in collabora', - ]), - // The 'writer' has write access, but no read access. - 'writer' => $this->createUser([ - 'edit any document in collabora', - ]), - 'diary keeper' => $this->createUser([ - // There is no 'preview own *' permission in this module. - 'preview diary in collabora', - 'edit own diary in collabora', - ]), - ]; - - $media_entities = [ - 'document' => $this->createMediaEntity('document'), - 'wiki' => $this->createMediaEntity('public_wiki'), - 'announcement' => $this->createMediaEntity('public_announcement'), - 'own diary' => $this->createMediaEntity('diary', [ - 'uid' => $accounts['diary keeper']->id(), - ]), - 'other diary' => $this->createMediaEntity('diary'), - ]; - - $paths = []; - foreach ($media_entities as $media_key => $media) { - $paths["/cool/view/<$media_key>"] = '/cool/view/' . $media->id(); - $paths["/cool/edit/<$media_key>"] = '/cool/edit/' . $media->id(); - } - - $this->assertPathsAccessByUsers( - [ - '/cool/view/' => ['reader', 'editor'], - '/cool/edit/' => ['editor', 'writer'], - '/cool/view/' => ['anonymous'], - '/cool/edit/' => ['anonymous'], - '/cool/view/' => ['anonymous'], - '/cool/edit/' => [], - '/cool/view/' => ['diary keeper'], - '/cool/edit/' => ['diary keeper'], - '/cool/view/' => ['diary keeper'], - '/cool/edit/' => [], - ], - $accounts, - $paths, - ); - } - - /** - * Builds a report about which users can access a given content. - * - * @param array> $expected - * Array indicating which url should be accessible by which user. - * The array keys are either paths or string keys from the $paths array. - * The array values are lists of keys from the $accounts array with access - * to that path. - * @param array $accounts - * Accounts to test access with, keyed by a distinguishable name. - * @param array|null $paths - * An array of paths, or NULL to just use the array keys from $expected. - * This parameter is useful if the paths all look very similar. - */ - protected function assertPathsAccessByUsers(array $expected, array $accounts, ?array $paths = NULL): void { - if ($paths === NULL) { - $paths = array_keys($expected); - $paths = array_combine($paths, $paths); - } - // Build a report and assert it all at once, to have a more complete - // overview on failure. - $actual = []; - foreach ($paths as $path_key => $path) { - $url = Url::fromUserInput($path); - // Filter the user list by access to the url. - $accounts_with_access = array_filter($accounts, $url->access(...)); - $actual[$path_key] = array_keys($accounts_with_access); - } - // Use yaml to avoid integer keys in list output. - $this->assertSame( - Yaml::encode($expected), - Yaml::encode($actual), - 'Users with access to given paths' - ); - } - - /** - * 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; - } - -} diff --git a/tests/src/Functional/PermissionTest.php b/tests/src/Functional/PermissionTest.php deleted file mode 100644 index 2c1f2f29..00000000 --- a/tests/src/Functional/PermissionTest.php +++ /dev/null @@ -1,94 +0,0 @@ -createMediaType('file', [ - 'id' => 'public_wiki', - 'label' => 'Public wiki', - ]); - /** @var \Drupal\user\PermissionHandlerInterface $permission_handler */ - $permission_handler = \Drupal::service(PermissionHandlerInterface::class); - $permissions = $permission_handler->getPermissions(); - $permissions = array_filter( - $permissions, - fn (array $permission) => $permission['provider'] === 'collabora_online', - ); - // Remove noise that is hard to diff. - $permissions = array_map( - static function (array $permission) { - $permission['title'] = (string) $permission['title']; - if ($permission['description'] === NULL) { - unset($permission['description']); - } - if ($permission['provider'] === 'collabora_online') { - unset($permission['provider']); - } - return $permission; - }, - $permissions, - ); - ksort($permissions); - $this->assertSame([ - 'administer collabora instance' => [ - 'title' => 'Administer the Collabora instance', - 'restrict access' => TRUE, - ], - 'edit any public_wiki in collabora' => [ - 'title' => 'Public wiki: Edit any media file in Collabora', - 'dependencies' => ['config' => ['media.type.public_wiki']], - ], - 'edit own public_wiki in collabora' => [ - 'title' => 'Public wiki: Edit own media file in Collabora', - 'dependencies' => ['config' => ['media.type.public_wiki']], - ], - 'preview own unpublished public_wiki in collabora' => [ - 'title' => 'Public wiki: Preview own unpublished media file in Collabora', - 'dependencies' => ['config' => ['media.type.public_wiki']], - ], - 'preview public_wiki in collabora' => [ - 'title' => 'Public wiki: Preview published media file in Collabora', - 'dependencies' => ['config' => ['media.type.public_wiki']], - ], - ], $permissions); - } - -} From a92b28e70109a1e64b8b872e6c3d558a8281d297 Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Tue, 19 Nov 2024 21:31:02 +0100 Subject: [PATCH 6/8] Issue #49: Remove assertBasicAssumptions(), it is not needed. --- tests/src/Kernel/CollaboraMediaAccessTest.php | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/tests/src/Kernel/CollaboraMediaAccessTest.php b/tests/src/Kernel/CollaboraMediaAccessTest.php index 21b63799..bc78d7b8 100644 --- a/tests/src/Kernel/CollaboraMediaAccessTest.php +++ b/tests/src/Kernel/CollaboraMediaAccessTest.php @@ -13,7 +13,6 @@ use Drupal\media\MediaInterface; use Drupal\Tests\media\Traits\MediaTypeCreationTrait; use Drupal\Tests\user\Traits\UserCreationTrait; -use Drupal\user\Entity\Role; use Drupal\user\RoleInterface; /** @@ -67,8 +66,6 @@ protected function setUp(): void { * Tests media access for Collabora routes and operations. */ public function testCollaboraMediaAccess(): void { - $this->assertBasicAssumptions(); - /** @var \Drupal\Core\Session\AccountInterface[] $accounts */ $accounts = [ // The default roles for anonymous and authenticated are not created @@ -379,24 +376,6 @@ protected function assertPathsAccessByUsers(array $expected, array $accounts, ?a ); } - /** - * Verifies basic assumptions for this test. - * - * It is enough to call this from one test method. - */ - protected function assertBasicAssumptions(): void { - // Normally, when media module is installed, media_install() will grant - // the 'view media' permission to anonymous and authenticated roles. - // In a kernel test this does not happen. - $this->assertSame([], Role::load(RoleInterface::ANONYMOUS_ID)->getPermissions()); - $this->assertSame([], Role::load(RoleInterface::AUTHENTICATED_ID)->getPermissions()); - - // The first user created in a test method is not user 1. - $user = $this->createUser(); - $this->assertSame(2, (int) $user->id()); - $this->assertFalse($user->hasPermission('view media')); - } - /** * Creates a media entity with attached file. * From 421a81a4bf192bfea2a0c30190c7b78df9d65be2 Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Tue, 19 Nov 2024 21:33:04 +0100 Subject: [PATCH 7/8] Issue #49: Introduce assertSameYaml() method. --- tests/src/Kernel/CollaboraMediaAccessTest.php | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/tests/src/Kernel/CollaboraMediaAccessTest.php b/tests/src/Kernel/CollaboraMediaAccessTest.php index bc78d7b8..498bfac9 100644 --- a/tests/src/Kernel/CollaboraMediaAccessTest.php +++ b/tests/src/Kernel/CollaboraMediaAccessTest.php @@ -301,10 +301,9 @@ protected function assertEntityAccess(array $expected, array $accounts, array $e } } } - // Use yaml to avoid integer keys in list output. - $this->assertSame( - "\n" . Yaml::encode($expected), - "\n" . Yaml::encode($actual), + $this->assertSameYaml( + $expected, + $actual, 'Users with access to given entities', ); } @@ -368,10 +367,9 @@ protected function assertPathsAccessByUsers(array $expected, array $accounts, ?a } } } - // Use yaml to avoid integer keys in list output. - $this->assertSame( - "\n" . Yaml::encode($expected), - "\n" . Yaml::encode($actual), + $this->assertSameYaml( + $expected, + $actual, 'Users with access to given paths', ); } @@ -402,4 +400,24 @@ protected function createMediaEntity(string $type, array $values = []): MediaInt return $media; } + /** + * Asserts that two values are the same when exported to yaml. + * + * This provides a nicer diff output, without numeric array keys. + * + * @param mixed $expected + * Expected value. + * @param mixed $actual + * Actual value. + * @param string $message + * Message. + */ + protected function assertSameYaml(mixed $expected, mixed $actual, string $message = ''): void { + $this->assertSame( + "\n" . Yaml::encode($expected), + "\n" . Yaml::encode($actual), + $message, + ); + } + } From 6fe0b0f7af44db2c6f4f48022a10f88eb4ce8f9b Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Tue, 19 Nov 2024 21:38:20 +0100 Subject: [PATCH 8/8] Issue #49: Rework the CollaboraMediaAccessTest, using separate assertions. --- tests/src/Kernel/CollaboraMediaAccessTest.php | 390 +++++++----------- 1 file changed, 144 insertions(+), 246 deletions(-) diff --git a/tests/src/Kernel/CollaboraMediaAccessTest.php b/tests/src/Kernel/CollaboraMediaAccessTest.php index 498bfac9..fd9a9c42 100644 --- a/tests/src/Kernel/CollaboraMediaAccessTest.php +++ b/tests/src/Kernel/CollaboraMediaAccessTest.php @@ -5,6 +5,7 @@ namespace Drupal\Tests\collabora_online\Kernel; use Drupal\Component\Serialization\Yaml; +use Drupal\Core\Session\AccountInterface; use Drupal\Core\Session\AnonymousUserSession; use Drupal\Core\Url; use Drupal\file\Entity\File; @@ -66,311 +67,208 @@ protected function setUp(): void { * Tests media access for Collabora routes and operations. */ public function testCollaboraMediaAccess(): void { - /** @var \Drupal\Core\Session\AccountInterface[] $accounts */ - $accounts = [ - // The default roles for anonymous and authenticated are not created - // in a kernel test. Therefore these users don't get any kind of - // access. - 'anonymous' => new AnonymousUserSession(), - 'authenticated' => $this->createUser(), - // Media permission do not grant access to Collabora operations. - 'media permissions only' => $this->createUser([ + $this->assertCollaboraMediaAccess( + [], + new AnonymousUserSession(), + 'No access for anonymous without permissions', + ); + + // Check authenticated with irrelevant permissions. + // This also covers the "no permissions" case. + $this->assertCollaboraMediaAccess( + [], + $this->createUser([ + // Add general 'media' permissions. 'administer media types', 'view media', 'update any media', 'view own unpublished media', - ]), - // Permissions for 'book' do not grant access to 'document'. - 'Bookworm' => $this->createUser([ + // Add Collabora permissions for a different media type. 'preview book in collabora', 'preview own unpublished book in collabora', 'edit any book in collabora', 'edit own book in collabora', ]), - 'Previewer' => $this->createUser([ - 'preview document in collabora', - ]), - 'Sean (preview own)' => $this->createUser([ - 'preview own unpublished document in collabora', - ]), - 'Editor' => $this->createUser([ - 'edit any document in collabora', - ]), - 'Kelly (edit own)' => $this->createUser([ - 'edit own document in collabora', - ]), - 'Media admin' => $this->createUser([ - 'administer media', - ]), - ]; + 'No access with irrelevant permissions', + ); - /** @var \Drupal\media\MediaInterface[] $media_entities */ - $media_entities = [ - "Sean's published document" => $this->createMediaEntity('document', [ - 'uid' => $accounts['Sean (preview own)']->id(), - ]), - "Sean's unpublished document" => $this->createMediaEntity('document', [ - 'uid' => $accounts['Sean (preview own)']->id(), - 'status' => 0, - ]), - "Kelly's published document" => $this->createMediaEntity('document', [ - 'uid' => $accounts['Kelly (edit own)']->id(), - ]), - "Kelly's unpublished document" => $this->createMediaEntity('document', [ - 'uid' => $accounts['Kelly (edit own)']->id(), - 'status' => 0, - ]), - ]; + $this->assertCollaboraMediaAccessForPermission( + [ + 'published document' => ['preview'], + 'own published document' => ['preview'], + ], + 'preview document in collabora', + ); - $this->assertEntityAccess( + $this->assertCollaboraMediaAccessForPermission( [ - 'anonymous' => [], - 'authenticated' => [], - 'media permissions only' => [], - 'Bookworm' => [], - 'Previewer' => [ - "Sean's published document" => ['preview in collabora'], - "Kelly's published document" => ['preview in collabora'], - ], - 'Sean (preview own)' => [ - "Sean's unpublished document" => ['preview in collabora'], - ], - 'Editor' => [ - "Sean's published document" => ['edit in collabora'], - "Sean's unpublished document" => ['edit in collabora'], - "Kelly's published document" => ['edit in collabora'], - "Kelly's unpublished document" => ['edit in collabora'], - ], - 'Kelly (edit own)' => [ - "Kelly's published document" => ['edit in collabora'], - "Kelly's unpublished document" => ['edit in collabora'], - ], - 'Media admin' => [ - "Sean's published document" => ['preview in collabora', 'edit in collabora'], - "Sean's unpublished document" => ['preview in collabora', 'edit in collabora'], - "Kelly's published document" => ['preview in collabora', 'edit in collabora'], - "Kelly's unpublished document" => ['preview in collabora', 'edit in collabora'], - ], + 'own unpublished document' => ['preview'], ], - $accounts, - $media_entities, + 'preview own unpublished document in collabora', + ); + + $this->assertCollaboraMediaAccessForPermission( + [ + 'published document' => ['edit'], + 'unpublished document' => ['edit'], + 'own published document' => ['edit'], + 'own unpublished document' => ['edit'], + ], + 'edit any document in collabora', + ); + + $this->assertCollaboraMediaAccessForPermission( [ - 'preview in collabora', - 'edit in collabora', + 'own published document' => ['edit'], + 'own unpublished document' => ['edit'], ], + 'edit own document in collabora', ); - $this->assertEntityPathsAccess( + // The 'administer media' permission grants access to everything. + $this->assertCollaboraMediaAccessForPermission( [ - 'anonymous' => [], - 'authenticated' => [], - 'media permissions only' => [], - 'Bookworm' => [], - 'Previewer' => [ - "/cool/view/", - "/cool/view/", - ], - 'Sean (preview own)' => [ - "/cool/view/", - ], - 'Editor' => [ - "/cool/edit/", - "/cool/edit/", - "/cool/edit/", - "/cool/edit/", - ], - 'Kelly (edit own)' => [ - "/cool/edit/", - "/cool/edit/", - ], - 'Media admin' => [ - "/cool/view/", - "/cool/edit/", - "/cool/view/", - "/cool/edit/", - "/cool/view/", - "/cool/edit/", - "/cool/view/", - "/cool/edit/", - ], + 'published document' => ['preview', 'edit'], + 'unpublished document' => ['preview', 'edit'], + 'own published document' => ['preview', 'edit'], + 'own unpublished document' => ['preview', 'edit'], ], - $accounts, - $media_entities, + 'administer media', + ); + + $this->assertCollaboraMediaAccess( [ - '/cool/view/%s', - '/cool/edit/%s', + 'published document' => ['preview', 'edit'], + 'unpublished document' => ['preview', 'edit'], + 'own published document' => ['preview', 'edit'], + 'own unpublished document' => ['preview', 'edit'], ], + $this->createUser(admin: TRUE), + "Access for admin user", ); } /** - * Tests a scenario where the anonymous user has more permissions. + * Tests scenarios where the anonymous user has more permissions. + * + * This verifies the special treatment of uid 0 to determine the owner of a + * media entity. */ public function testAnonymousOwnAccess(): void { user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, [ 'preview own unpublished document in collabora', 'edit own document in collabora', ]); - - /** @var \Drupal\Core\Session\AccountInterface[] $accounts */ - $accounts = [ - 'anonymous' => new AnonymousUserSession(), - 'Emilia' => $this->createUser(), - ]; - - /** @var \Drupal\media\MediaInterface[] $media_entities */ - $media_entities = [ - // Set uid = 0 to verify that anonymous is not seen as the owner. - "published document" => $this->createMediaEntity('document', [ - 'uid' => 0, - ]), - "unpublished document" => $this->createMediaEntity('document', [ - 'uid' => 0, - 'status' => 0, - ]), - "Emilia's published document" => $this->createMediaEntity('document', [ - 'uid' => $accounts['Emilia']->id(), - ]), - "Emilia's unpublished document" => $this->createMediaEntity('document', [ - 'uid' => $accounts['Emilia']->id(), - 'status' => 0, - ]), - ]; - - $this->assertEntityAccess( - [ - 'anonymous' => [], - 'Emilia' => [], - ], - $accounts, - $media_entities, - [ - 'preview in collabora', - 'edit in collabora', - ], + $this->assertCollaboraMediaAccess( + [], + new AnonymousUserSession(), + "Anonymous user with '... own ...' permissions.", ); user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, [ 'preview document in collabora', 'edit any document in collabora', ]); - - $this->assertEntityAccess( + $this->assertCollaboraMediaAccess( [ - 'anonymous' => [ - "published document" => ['preview in collabora', 'edit in collabora'], - "unpublished document" => ['edit in collabora'], - "Emilia's published document" => ['preview in collabora', 'edit in collabora'], - "Emilia's unpublished document" => ['edit in collabora'], - ], - 'Emilia' => [], - ], - $accounts, - $media_entities, - [ - 'preview in collabora', - 'edit in collabora', + 'published document' => ['preview', 'edit'], + 'unpublished document' => ['edit'], + 'own published document' => ['preview', 'edit'], + 'own unpublished document' => ['edit'], ], + new AnonymousUserSession(), + "Anonymous user with all Collabora media permissions.", ); } /** - * Asserts which users can access which entity operations. + * Creates a user with one permission, and asserts access to media entities. * - * @param array>> $expected - * Expected outcome. - * The array is keyed by entity key and operation. - * The values are lists of keys from the $accounts parameter. - * @param array $accounts - * Entities to check. - * @param array $entities - * Operations to check. - * @param list $operations - * User accounts to check. + * @param array> $expected + * Expected access. + * @param string $permission + * Permission machine name. */ - protected function assertEntityAccess(array $expected, array $accounts, array $entities, array $operations): void { - $actual = []; - foreach ($accounts as $account_key => $account) { - $actual[$account_key] = []; - foreach ($entities as $entity_key => $entity) { - foreach ($operations as $operation) { - $has_access = $entity->access($operation, $account); - if ($has_access) { - $actual[$account_key][$entity_key][] = $operation; - } - } - } - } - $this->assertSameYaml( - $expected, - $actual, - 'Users with access to given entities', - ); + protected function assertCollaboraMediaAccessForPermission(array $expected, string $permission): void { + $account = $this->createUser([$permission]); + $message = "User with '$permission' permission."; + $this->assertCollaboraMediaAccess($expected, $account, $message); } /** - * Asserts which users have access to which entity paths. + * Asserts Collabora media access for a user account. * - * @param array> $expected - * Array indicating which url should be accessible by which user. - * The array keys are either string keys from the $paths array. - * The array values are lists of keys from the $accounts array with access - * to that path. - * @param array $accounts - * Accounts to test access with, keyed by a distinguishable name. - * @param array $entities - * Entities for which to build paths. - * @param array $sprintf_path_patterns - * Path patterns with '%s' placeholder for the entity id. + * @param array $expected + * Expected access matrix. + * The keys identify media entities that are created in this test. + * The values identify operations. + * @param \Drupal\Core\Session\AccountInterface $account + * Account to check access for. + * @param string $message + * Message for the assertion. */ - protected function assertEntityPathsAccess(array $expected, array $accounts, array $entities, array $sprintf_path_patterns) { - $paths = []; - // Build Collabora media paths for all media entities. + protected function assertCollaboraMediaAccess(array $expected, AccountInterface $account, string $message): void { + $other_user = $this->createUser(); + $entities = [ + "published document" => $this->createMediaEntity('document', [ + 'uid' => $other_user->id(), + ]), + "unpublished document" => $this->createMediaEntity('document', [ + 'uid' => $other_user->id(), + 'status' => 0, + ]), + "own published document" => $this->createMediaEntity('document', [ + 'uid' => $account->id(), + ]), + "own unpublished document" => $this->createMediaEntity('document', [ + 'uid' => $account->id(), + 'status' => 0, + ]), + ]; + + // Test $entity->access() with different operations on all entities. + $operations = [ + 'preview' => 'preview in collabora', + 'edit' => 'edit in collabora', + ]; + $actual_entity_access = []; foreach ($entities as $entity_key => $entity) { - foreach ($sprintf_path_patterns as $pattern) { - $paths[sprintf($pattern, "<$entity_key>")] = sprintf($pattern, $entity->id()); + foreach ($operations as $operation_key => $operation) { + $has_entity_access = $entity->access($operation, $account); + if ($has_entity_access) { + $actual_entity_access[$entity_key][] = $operation_key; + } } } - $this->assertPathsAccessByUsers($expected, $accounts, $paths); - } + $this->assertSameYaml( + $expected, + $actual_entity_access, + 'Entity access: ' . $message, + ); - /** - * Asserts which users have access to which paths. - * - * @param array> $expected - * Array indicating which url should be accessible by which user. - * The array keys are either paths or string keys from the $paths array. - * The array values are lists of keys from the $accounts array with access - * to that path. - * @param array $accounts - * Accounts to test access with, keyed by a distinguishable name. - * @param array|null $paths - * An array of paths, or NULL to just use the array keys from $expected. - * This parameter is useful if the paths all look very similar. - */ - protected function assertPathsAccessByUsers(array $expected, array $accounts, ?array $paths = NULL): void { - if ($paths === NULL) { - $paths = array_keys($expected); - $paths = array_combine($paths, $paths); - } - // Build a report and assert it all at once, to have a more complete - // overview on failure. - $actual = []; - foreach ($accounts as $account_key => $account) { - $actual[$account_key] = []; - foreach ($paths as $path_key => $path) { - $url = Url::fromUserInput($path); - // Filter the user list by access to the url. - $has_access = $url->access($account); - if ($has_access) { - $actual[$account_key][] = $path_key; + // Test path access. + // The result is expected to be exactly the same, due to how the route + // access is configured. + // Testing the paths like this introduces some level of redundancy or + // duplication, but it is cheap and easy, so for now this is what we do. + $sprintf_path_patterns = [ + 'preview' => '/cool/view/%s', + 'edit' => '/cool/edit/%s', + ]; + $actual_path_access = []; + foreach ($entities as $entity_key => $entity) { + foreach ($sprintf_path_patterns as $pattern_key => $sprintf_path_pattern) { + $path = sprintf($sprintf_path_pattern, $entity->id()); + $has_path_access = Url::fromUserInput($path)->access($account); + if ($has_path_access) { + $actual_path_access[$entity_key][] = $pattern_key; } } } $this->assertSameYaml( $expected, - $actual, - 'Users with access to given paths', + $actual_path_access, + 'Path access: ' . $message, ); }