jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/361693 )
Change subject: Add WikiPageEntityStorePermissionChecker for consistent
permission checks
......................................................................
Add WikiPageEntityStorePermissionChecker for consistent permission checks
WikiPageEntityStorePermissionChecker is meant a central place to check
permission for editing entity wiki pages.
Bug: T166586
Change-Id: Ifb78d909cacb9e63168898ed35fd0f286ad711e1
---
M repo/includes/Store/EntityPermissionChecker.php
A repo/includes/Store/WikiPageEntityStorePermissionChecker.php
A repo/tests/phpunit/includes/Store/WikiPageEntityStorePermissionCheckerTest.php
3 files changed, 974 insertions(+), 9 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/Store/EntityPermissionChecker.php
b/repo/includes/Store/EntityPermissionChecker.php
index 17ad2fe..60de8e3 100644
--- a/repo/includes/Store/EntityPermissionChecker.php
+++ b/repo/includes/Store/EntityPermissionChecker.php
@@ -2,6 +2,7 @@
namespace Wikibase\Repo\Store;
+use InvalidArgumentException;
use Status;
use User;
use Wikibase\DataModel\Entity\EntityDocument;
@@ -15,14 +16,26 @@
*/
interface EntityPermissionChecker {
+ const ACTION_READ = 'read';
+
+ const ACTION_EDIT = 'edit';
+
+ const ACTION_CREATE = 'create';
+
+ const ACTION_EDIT_TERMS = 'term';
+
+ const ACTION_MERGE = 'merge';
+
+ const ACTION_REDIRECT = 'redirect';
+
/**
- * Check whether the given user has the given permission on an entity.
+ * Check whether the given user has the permission to perform the given
action on an entity.
* This will perform a check based on the entity's ID if the entity has
an ID set
* (that is, the entity "exists"), or based merely on the entity type,
in case
* the entity does not exist.
*
* @param User $user
- * @param string $permission
+ * @param string $action
* @param EntityDocument $entity
* @param string $quick Flag for allowing quick permission checking. If
set to
* 'quick', implementations may return inaccurate results if
determining the accurate result
@@ -30,16 +43,18 @@
* This is intended as an optimization for non-critical checks,
* e.g. for showing or hiding UI elements.
*
+ * @throws InvalidArgumentException if unknown permission is requested
+ *
* @return Status a status object representing the check's result.
*/
- public function getPermissionStatusForEntity( User $user, $permission,
EntityDocument $entity, $quick = '' );
+ public function getPermissionStatusForEntity( User $user, $action,
EntityDocument $entity, $quick = '' );
/**
- * Check whether the given user has the given permission on an entity.
+ * Check whether the given user has the permission to perform the given
action on an entity.
* This requires the ID of an existing entity.
*
* @param User $user
- * @param string $permission
+ * @param string $action
* @param EntityId $entityId
* @param string $quick Flag for allowing quick permission checking. If
set to
* 'quick', implementations may return inaccurate results if
determining the accurate result
@@ -47,19 +62,21 @@
* This is intended as an optimization for non-critical checks,
* e.g. for showing or hiding UI elements.
*
+ * @throws InvalidArgumentException if unknown permission is requested
+ *
* @return Status a status object representing the check's result.
*/
- public function getPermissionStatusForEntityId( User $user,
$permission, EntityId $entityId, $quick = '' );
+ public function getPermissionStatusForEntityId( User $user, $action,
EntityId $entityId, $quick = '' );
/**
- * Check whether the given user has the given permission on a given
entity type.
+ * Check whether the given user has the permission to perform the given
action on a given entity type.
* This does not require an entity to exist.
*
* Useful especially for checking whether the user is allowed to create
an entity
* of a given type.
*
* @param User $user
- * @param string $permission
+ * @param string $action
* @param string $type
* @param string $quick Flag for allowing quick permission checking. If
set to
* 'quick', implementations may return inaccurate results if
determining the accurate result
@@ -67,8 +84,10 @@
* This is intended as an optimization for non-critical checks,
* e.g. for showing or hiding UI elements.
*
+ * @throws InvalidArgumentException if unknown permission is requested
+ *
* @return Status a status object representing the check's result.
*/
- public function getPermissionStatusForEntityType( User $user,
$permission, $type, $quick = '' );
+ public function getPermissionStatusForEntityType( User $user, $action,
$type, $quick = '' );
}
diff --git a/repo/includes/Store/WikiPageEntityStorePermissionChecker.php
b/repo/includes/Store/WikiPageEntityStorePermissionChecker.php
new file mode 100644
index 0000000..1487ab6
--- /dev/null
+++ b/repo/includes/Store/WikiPageEntityStorePermissionChecker.php
@@ -0,0 +1,240 @@
+<?php
+
+namespace Wikibase\Repo\Store;
+
+use InvalidArgumentException;
+use Status;
+use Title;
+use User;
+use Wikibase\DataModel\Entity\EntityDocument;
+use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\Lib\Store\EntityNamespaceLookup;
+use Wikibase\Lib\Store\EntityTitleLookup;
+
+/**
+ * Checks permissions to perform actions on the entity based on MediaWiki page
permissions.
+ *
+ * @license GPL-2.0+
+ */
+class WikiPageEntityStorePermissionChecker implements EntityPermissionChecker {
+
+ /**
+ * @var EntityNamespaceLookup
+ */
+ private $namespaceLookup;
+
+ /**
+ * @var EntityTitleLookup
+ */
+ private $titleLookup;
+
+ /**
+ * @var string[]
+ */
+ private $availableRights;
+
+ /**
+ * @param EntityNamespaceLookup $namespaceLookup
+ * @param EntityTitleLookup $titleLookup
+ * @param string[] $availableRights
+ */
+ public function __construct(
+ EntityNamespaceLookup $namespaceLookup,
+ EntityTitleLookup $titleLookup,
+ array $availableRights
+ ) {
+ $this->namespaceLookup = $namespaceLookup;
+ $this->titleLookup = $titleLookup;
+ $this->availableRights = $availableRights;
+ }
+
+ /**
+ * @see EntityPermissionChecker::getPermissionStatusForEntity
+ *
+ * @param User $user
+ * @param string $action
+ * @param EntityDocument $entity
+ * @param string $quick
+ *
+ * @throws InvalidArgumentException if unknown permission is requested
+ *
+ * @return Status
+ */
+ public function getPermissionStatusForEntity( User $user, $action,
EntityDocument $entity, $quick = '' ) {
+ $id = $entity->getId();
+
+ if ( $id === null ) {
+ $entityType = $entity->getType();
+
+ if ( $action === EntityPermissionChecker::ACTION_EDIT )
{
+ // for editing a non-existing page, check the
create permission
+ return $this->getPermissionStatusForEntityType(
$user, EntityPermissionChecker::ACTION_CREATE, $entityType, $quick );
+ }
+
+ return $this->getPermissionStatusForEntityType( $user,
$action, $entityType, $quick );
+ }
+
+ return $this->getPermissionStatusForEntityId( $user, $action,
$id, $quick );
+ }
+
+ /**
+ * @see EntityPermissionChecker::getPermissionStatusForEntityId
+ *
+ * @param User $user
+ * @param string $action
+ * @param EntityId $entityId
+ * @param string $quick
+ *
+ * @throws InvalidArgumentException if unknown permission is requested
+ *
+ * @return Status
+ */
+ public function getPermissionStatusForEntityId( User $user, $action,
EntityId $entityId, $quick = '' ) {
+ $title = $this->titleLookup->getTitleForId( $entityId );
+
+ if ( $title === null || !$title->exists() ) {
+ if ( $action === EntityPermissionChecker::ACTION_EDIT )
{
+ return $this->getPermissionStatusForEntityType(
+ $user,
+ EntityPermissionChecker::ACTION_CREATE,
+ $entityId->getEntityType(),
+ $quick
+ );
+ }
+
+ return $this->getPermissionStatusForEntityType(
+ $user,
+ $action,
+ $entityId->getEntityType(),
+ $quick
+ );
+ }
+
+ return $this->checkPermissionsForAction( $user, $action,
$title, $entityId->getEntityType(), $quick );
+ }
+
+ /**
+ * @see EntityPermissionChecker::getPermissionStatusForEntityType
+ *
+ * @param User $user
+ * @param string $action
+ * @param string $type
+ * @param string $quick
+ *
+ * @throws InvalidArgumentException if unknown permission is requested
+ *
+ * @return Status
+ */
+ public function getPermissionStatusForEntityType( User $user, $action,
$type, $quick = '' ) {
+ $title = $this->getPageTitleInEntityNamespace( $type );
+
+ if ( $action === EntityPermissionChecker::ACTION_EDIT ) {
+ // Note: No entity ID given, assuming creating new
entity, i.e. create permissions will be checked
+ return $this->checkPermissionsForAction(
+ $user,
+ EntityPermissionChecker::ACTION_CREATE,
+ $title,
+ $type,
+ $quick
+ );
+ }
+
+ return $this->checkPermissionsForAction( $user, $action,
$title, $type, $quick );
+ }
+
+ /**
+ * @param string $entityType
+ *
+ * @return Title
+ */
+ private function getPageTitleInEntityNamespace( $entityType ) {
+ $namespace = $this->namespaceLookup->getEntityNamespace(
$entityType ); // TODO: can be null!
+
+ return Title::makeTitle( $namespace, '/' );
+ }
+
+ private function checkPermissionsForAction( User $user, $action, Title
$title, $entityType, $quick ='' ) {
+ $status = Status::newGood();
+
+ $mediaWikiPermissions = $this->getMediaWikiPermissionsToCheck(
$action, $entityType );
+
+ foreach ( $mediaWikiPermissions as $mwPermission ) {
+ $partialStatus = $this->getPermissionStatus( $user,
$mwPermission, $title, $quick );
+ $status->merge( $partialStatus );
+ }
+
+ return $status;
+ }
+
+ private function getMediaWikiPermissionsToCheck( $action, $entityType )
{
+ if ( $action === EntityPermissionChecker::ACTION_CREATE ) {
+ $entityTypeSpecificCreatePermission = $entityType .
'-create';
+
+ $permissions = [ 'read', 'edit', 'createpage' ];
+ if ( $this->mediawikiPermissionExists(
$entityTypeSpecificCreatePermission ) ) {
+ $permissions[] =
$entityTypeSpecificCreatePermission;
+ }
+ return $permissions;
+ }
+
+ if ( $action === EntityPermissionChecker::ACTION_EDIT_TERMS ) {
+ $entityTypeSpecificEditTermsPermission = $entityType .
'-term';
+
+ $permissions = [ 'read', 'edit' ];
+ if ( $this->mediawikiPermissionExists(
$entityTypeSpecificEditTermsPermission ) ) {
+ $permissions[] =
$entityTypeSpecificEditTermsPermission;
+ }
+ return $permissions;
+ }
+
+ if ( $action === EntityPermissionChecker::ACTION_MERGE ) {
+ $entityTypeSpecificMergePermission = $entityType .
'-merge';
+
+ $permissions = [ 'read', 'edit' ];
+ if ( $this->mediawikiPermissionExists(
$entityTypeSpecificMergePermission ) ) {
+ $permissions[] =
$entityTypeSpecificMergePermission;
+ }
+ return $permissions;
+ }
+
+ if ( $action === EntityPermissionChecker::ACTION_REDIRECT ) {
+ $entityTypeSpecificRedirectPermission = $entityType .
'-redirect';
+
+ $permissions = [ 'read', 'edit' ];
+ if ( $this->mediawikiPermissionExists(
$entityTypeSpecificRedirectPermission ) ) {
+ $permissions[] =
$entityTypeSpecificRedirectPermission;
+ }
+ return $permissions;
+ }
+
+ if ( $action === EntityPermissionChecker::ACTION_EDIT ) {
+ return [ 'read', 'edit' ];
+ }
+
+ if ( $action === EntityPermissionChecker::ACTION_READ ) {
+ return [ 'read' ];
+ }
+
+ throw new InvalidArgumentException( 'Unknown action to check
permissions for: ' . $action );
+ }
+
+ private function mediawikiPermissionExists( $permission ) {
+ return in_array( $permission, $this->availableRights );
+ }
+
+ private function getPermissionStatus( User $user, $permission, Title
$title, $quick = '' ) {
+ $status = Status::newGood();
+
+ $errors = $title->getUserPermissionsErrors( $permission, $user,
$quick !== 'quick' );
+
+ if ( $errors ) {
+ $status->setResult( false );
+ foreach ( $errors as $error ) {
+ call_user_func_array( [ $status, 'fatal' ],
$error );
+ }
+ }
+
+ return $status;
+ }
+
+}
diff --git
a/repo/tests/phpunit/includes/Store/WikiPageEntityStorePermissionCheckerTest.php
b/repo/tests/phpunit/includes/Store/WikiPageEntityStorePermissionCheckerTest.php
new file mode 100644
index 0000000..c8cfbc3
--- /dev/null
+++
b/repo/tests/phpunit/includes/Store/WikiPageEntityStorePermissionCheckerTest.php
@@ -0,0 +1,706 @@
+<?php
+
+namespace Wikibase\Repo\Tests\Store;
+
+use InvalidArgumentException;
+use Title;
+use TitleValue;
+use Wikibase\DataModel\Entity\EntityDocument;
+use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\Item;
+use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Entity\Property;
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\Lib\Store\EntityNamespaceLookup;
+use Wikibase\Lib\Store\EntityTitleLookup;
+use Wikibase\Repo\Store\EntityPermissionChecker;
+use Wikibase\Repo\Store\WikiPageEntityStorePermissionChecker;
+
+/**
+ * @covers Wikibase\Repo\Store\WikiPageEntityStorePermissionChecker
+ *
+ * @group Database
+ * @group Wikibase
+ * @group medium
+ *
+ * @license GPL-2.0+
+ */
+class WikiPageEntityStorePermissionCheckerTest extends \MediaWikiTestCase {
+
+ const EXISTING_ITEM_ID = 'Q2';
+ const NON_EXISTING_ITEM_ID = 'Q3';
+ const EXISTING_PROPERTY_ID = 'P2';
+ const NON_EXISTING_PROPERTY_ID = 'P3';
+
+ /**
+ * @dataProvider provideExistingEntities
+ */
+ public function testEditPermissionsAreRequiredToEditExistingEntity(
EntityDocument $existingEntity ) {
+ $this->anyUserHasPermissions( [ 'edit' => true ] );
+
+ $this->assertUserIsAllowedTo(
EntityPermissionChecker::ACTION_EDIT, $existingEntity );
+ }
+
+ public function provideExistingEntities() {
+ return [
+ 'existing item' => [ $this->getExistingItem() ],
+ 'existing property' => [ $this->getExistingProperty() ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideExistingEntitiesAndPermissionsThatDisallowEdit
+ */
+ public function
testEditPermissionsAreRequiredToEditExistingEntity_failures(
+ EntityDocument $existingEntity,
+ array $permissions
+ ) {
+ $this->anyUserHasPermissions( $permissions );
+
+ $this->assertItIsForbiddenForUserTo(
EntityPermissionChecker::ACTION_EDIT, $existingEntity );
+ }
+
+ public function provideExistingEntitiesAndPermissionsThatDisallowEdit()
{
+ return [
+ 'existing item, no edit permission' => [
$this->getExistingItem(), [ 'edit' => false ] ],
+ 'existing item, no read permission' => [
$this->getExistingItem(), [ 'read' => false, 'edit' => true ] ],
+ 'existing property, no edit permission' => [
$this->getExistingProperty(), [ 'edit' => false ] ],
+ 'existing property, no read permission' => [
$this->getExistingProperty(), [ 'read' => false, 'edit' => true ] ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideNonExistingEntitiesAndPermissionsThatAllowEdit
+ */
+ public function
testAllRequiredPermissionsAreNeededToEditNonExistingEntity(
+ EntityDocument $nonExistingEntity,
+ array $groupPermissions
+ ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertUserIsAllowedTo(
EntityPermissionChecker::ACTION_EDIT, $nonExistingEntity );
+ }
+
+ public function provideNonExistingEntitiesAndPermissionsThatAllowEdit()
{
+ return [
+ 'non-existing item, createpage permission' => [
+ $this->getNonExistingItem(),
+ [ 'createpage' => true ]
+ ],
+ 'non-existing item (null ID), createpage permission' =>
[
+ $this->getNonExistingItemWithNullId(),
+ [ 'createpage' => true ]
+ ],
+ 'non-existing property, createpage and property-create
permission' => [
+ $this->getNonExistingProperty(),
+ [ 'createpage' => true, 'property-create' =>
true, ]
+ ],
+ 'non-existing property (null ID), createpage and
property-create permission' => [
+ $this->getNonExistingPropertyWithNullId(),
+ [ 'createpage' => true, 'property-create' =>
true, ]
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider
provideNonExistingEntitiesAndPermissionsThatDisallowEdit
+ */
+ public function
testAllRequiredPermissionsAreNeededToEditNonExistingEntity_failures(
+ EntityDocument $nonExistingentity,
+ array $groupPermissions
+ ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertItIsForbiddenForUserTo(
EntityPermissionChecker::ACTION_EDIT, $nonExistingentity );
+ }
+
+ public function
provideNonExistingEntitiesAndPermissionsThatDisallowEdit() {
+ return [
+ 'non-existing item, no createpage permission' => [
+ $this->getNonExistingItem(),
+ [ 'createpage' => false ]
+ ],
+ 'non-existing item, no edit permission' => [
+ $this->getNonExistingItem(),
+ [ 'edit' => false, 'createpage' => true ]
+ ],
+ 'non-existing item, no read permission' => [
+ $this->getNonExistingItem(),
+ [ 'read' => false, 'edit' => true, 'createpage'
=> true ]
+ ],
+ 'non-existing item (null ID), no createpage permission'
=> [
+ $this->getNonExistingItemWithNullId(),
+ [ 'createpage' => false ]
+ ],
+ 'non-existing item (null ID), no edit permission' => [
+ $this->getNonExistingItemWithNullId(),
+ [ 'edit' => false, 'createpage' => true ]
+ ],
+ 'non-existing item (null ID), no read permission' => [
+ $this->getNonExistingItemWithNullId(),
+ [ 'read' => false, 'edit' => true, 'createpage'
=> true ]
+ ],
+ 'non-existing property, no property-create permission'
=> [
+ $this->getNonExistingProperty(),
+ [ 'createpage' => true, 'property-create' =>
false, ]
+ ],
+ 'non-existing property, no createpage permission' => [
+ $this->getNonExistingProperty(),
+ [ 'createpage' => false, 'property-create' =>
true, ]
+ ],
+ 'non-existing property, no createpage nor
property-create permission' => [
+ $this->getNonExistingProperty(),
+ [ 'createpage' => false, 'property-create' =>
false, ]
+ ],
+ 'non-existing property, no edit permission' => [
+ $this->getNonExistingProperty(),
+ [ 'edit' => false, 'createpage' => true,
'property-create' => true, ]
+ ],
+ 'non-existing property, no read permission' => [
+ $this->getNonExistingProperty(),
+ [ 'read' => false, 'edit' => true, 'createpage'
=> true, 'property-create' => true, ]
+ ],
+ 'non-existing property (null ID), no property-create
permission' => [
+ $this->getNonExistingPropertyWithNullId(),
+ [ 'createpage' => true, 'property-create' =>
false, ]
+ ],
+ 'non-existing property (null ID), no createpage
permission' => [
+ $this->getNonExistingPropertyWithNullId(),
+ [ 'createpage' => false, 'property-create' =>
true, ]
+ ],
+ 'non-existing property (null ID), no createpage nor
property-create permission' => [
+ $this->getNonExistingPropertyWithNullId(),
+ [ 'createpage' => false, 'property-create' =>
false, ]
+ ],
+ 'non-existing property (null ID), no edit permission'
=> [
+ $this->getNonExistingPropertyWithNullId(),
+ [ 'edit' => false, 'createpage' => true,
'property-create' => true, ]
+ ],
+ 'non-existing property (null ID), no read permission'
=> [
+ $this->getNonExistingPropertyWithNullId(),
+ [ 'read' => false, 'edit' => true, 'createpage'
=> true, 'property-create' => true, ]
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideNonExistingEntitiesAndPermissionsThatAllowEdit
+ */
+ public function
testAllRequiredPermissionsAreNeededToCreateNonExistingEntity(
+ EntityDocument $nonExistentEntity,
+ array $groupPermissionsAllowingCreation
+ ) {
+ $this->anyUserHasPermissions( $groupPermissionsAllowingCreation
);
+
+ $this->assertUserIsAllowedTo(
EntityPermissionChecker::ACTION_CREATE, $nonExistentEntity );
+ }
+
+ /**
+ * @dataProvider
provideNonExistingEntitiesAndPermissionsThatDisallowEdit
+ */
+ public function
testAllRequiredPermissionsAreNeededToCreateNonExistingEntity_failures(
+ EntityDocument $nonExistingEntity,
+ array $groupPermissions
+ ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertItIsForbiddenForUserTo(
EntityPermissionChecker::ACTION_CREATE, $nonExistingEntity );
+ }
+
+ /**
+ * TODO: should this be even tested? Is this correct behaviour? Or
should it rather refuse on PERMISSION_CREATE
+ * and the existing entity?
+ *
+ * @dataProvider provideExistingEntitiesAndPermissionsThatAllowCreating
+ */
+ public function
testAllRequiredPermissionsAreNeededToCreateExistingEntity(
+ EntityDocument $existingEntity,
+ array $groupPermissions
+ ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertUserIsAllowedTo(
EntityPermissionChecker::ACTION_CREATE, $existingEntity );
+ }
+
+ public function
provideExistingEntitiesAndPermissionsThatAllowCreating() {
+ return [
+ 'existing item, createpage permission' => [
+ $this->getExistingItem(),
+ [ 'createpage' => true ]
+ ],
+ 'existing item, createpage and property-create
permission' => [
+ $this->getExistingProperty(),
+ [ 'createpage' => true, 'property-create' =>
true, ]
+ ],
+ ];
+ }
+
+ /**
+ * TODO: should this be even tested? Is this correct behaviour? Or
should it rather refuse on PERMISSION_CREATE
+ * and the existing entity?
+ *
+ * @dataProvider
provideExistingEntitiesAndPermissionsThatDisallowCreating
+ */
+ public function
testAllRequiredPermissionsAreNeededToCreateExistingEntity_failures(
+ EntityDocument $existingEntity,
+ array $groupPermissions
+ ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertItIsForbiddenForUserTo(
EntityPermissionChecker::ACTION_CREATE, $existingEntity );
+ }
+
+ public function
provideExistingEntitiesAndPermissionsThatDisallowCreating() {
+ return [
+ 'existing item, no createpage permission' => [
+ $this->getExistingItem(),
+ [ 'createpage' => false ]
+ ],
+ 'existing item, no edit permission' => [
+ $this->getExistingItem(),
+ [ 'edit' => false, 'createpage' => true, ]
+ ],
+ 'existing item, no read permission' => [
+ $this->getExistingItem(),
+ [ 'read' => false, 'edit' => true, 'createpage'
=> true, ]
+ ],
+ 'existing property, no property-create permission' => [
+ $this->getExistingProperty(),
+ [ 'createpage' => true, 'property-create' =>
false, ]
+ ],
+ 'existing property, no createpage permission' => [
+ $this->getExistingProperty(),
+ [ 'createpage' => false, 'property-create' =>
true, ]
+ ],
+ 'existing property, no createpage nor property-create
permission' => [
+ $this->getExistingProperty(),
+ [ 'createpage' => false, 'property-create' =>
false, ]
+ ],
+ 'existing property, no edit permission' => [
+ $this->getExistingProperty(),
+ [ 'edit' => false, 'createpage' => true,
'property-create' => true, ]
+ ],
+ 'existing property, no read permission' => [
+ $this->getExistingProperty(),
+ [ 'read' => false, 'edit' => true, 'createpage'
=> true, 'property-create' => true, ]
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideAllEntities
+ */
+ public function testReadPermissionsAreNeededToReadAnEntity(
EntityDocument $entity ) {
+ $this->anyUserHasPermissions( [ 'read' => true ] );
+
+ $this->assertUserIsAllowedTo(
EntityPermissionChecker::ACTION_READ, $entity );
+ }
+
+ /**
+ * @dataProvider provideAllEntities
+ */
+ public function testReadPermissionsAreNeededToReadAnEntity_failures(
EntityDocument $entity ) {
+ $this->anyUserHasPermissions( [ 'read' => false ] );
+
+ $this->assertItIsForbiddenForUserTo(
EntityPermissionChecker::ACTION_READ, $entity );
+ }
+
+ public function provideAllEntities() {
+ return [
+ 'existing item' => [ $this->getExistingItem() ],
+ 'existing property' => [ $this->getExistingProperty() ],
+ 'non-existing item' => [ $this->getNonExistingItem() ],
+ 'non-existing item (null ID)' => [
$this->getNonExistingItemWithNullId() ],
+ 'non-existing property' => [
$this->getNonExistingProperty() ],
+ 'non-existing property (null ID)' => [
$this->getNonExistingPropertyWithNullId() ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideExistingEntitiesAndPermissionsThatAllowMerge
+ */
+ public function testAllRequiredPermissionsAreNeededToMergeEntity(
EntityDocument $entity, array $groupPermissions ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertUserIsAllowedTo(
EntityPermissionChecker::ACTION_MERGE, $entity );
+ }
+
+ public function provideExistingEntitiesAndPermissionsThatAllowMerge() {
+ return [
+ 'existing item, edit and item-merge permissions' => [
+ $this->getExistingItem(),
+ [ 'edit' => true, 'item-merge' => true ]
+ ],
+ // TODO: should this be even tested? Or should it
return false/throw exception for properties?
+ 'existing property, edit permission' => [
+ $this->getExistingProperty(),
+ [ 'edit' => true ]
+ ],
+ 'existing property, item-merge permission is
irrelevant' => [
+ $this->getExistingProperty(),
+ [ 'edit' => true, 'item-merge' => false ]
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideExistingEntitiesAndPermissionsThatDisallowMerge
+ */
+ public function
testAllRequiredPermissionsAreNeededToMergeEntity_failures( EntityDocument
$entity, array $groupPermissions ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertItIsForbiddenForUserTo(
EntityPermissionChecker::ACTION_MERGE, $entity );
+ }
+
+ public function
provideExistingEntitiesAndPermissionsThatDisallowMerge() {
+ return [
+ 'existing item, no item-merge permissions' => [
+ $this->getExistingItem(),
+ [ 'edit' => true, 'item-merge' => false ]
+ ],
+ 'existing item, no edit permissions' => [
+ $this->getExistingItem(),
+ [ 'edit' => false, 'item-merge' => true ]
+ ],
+ 'existing item, no edit nor item-merge permissions' => [
+ $this->getExistingItem(),
+ [ 'edit' => false, 'item-merge' => false ]
+ ],
+ 'existing item, no read permissions' => [
+ $this->getExistingItem(),
+ [ 'read' => false, 'edit' => true, 'item-merge'
=> true ]
+ ],
+ // TODO: should this be even tested? Or should it
return false/throw exception for properties?
+ 'existing property, no edit permissions' => [
+ $this->getExistingProperty(),
+ [ 'edit' => false ]
+ ],
+ 'existing property, no read permissions' => [
+ $this->getExistingProperty(),
+ [ 'read' => false, 'edit' => true ]
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideExistingEntitiesAndPermissionsThatAllowRedirect
+ */
+ public function testAllRequiredPermissionsAreNeededToRedirectEntity(
EntityDocument $entity, array $groupPermissions ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertUserIsAllowedTo(
EntityPermissionChecker::ACTION_REDIRECT, $entity );
+ }
+
+ public function
provideExistingEntitiesAndPermissionsThatAllowRedirect() {
+ return [
+ 'existing item, edit and item-redirect permissions' => [
+ $this->getExistingItem(),
+ [ 'edit' => true, 'item-redirect' => true ]
+ ],
+ // TODO: should this be even tested? Or should it
return false/throw exception for properties?
+ 'existing property, edit permission' => [
+ $this->getExistingProperty(),
+ [ 'edit' => true ]
+ ],
+ 'existing property, item-redirect permission is
irrelevant' => [
+ $this->getExistingProperty(),
+ [ 'edit' => true, 'item-redirect' => false ]
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider
provideExistingEntitiesAndPermissionsThatDisallowRedirect
+ */
+ public function
testAllRequiredPermissionsAreNeededToRedirectEntity_failures(
+ EntityDocument $entity,
+ array $groupPermissions
+ ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertItIsForbiddenForUserTo(
EntityPermissionChecker::ACTION_REDIRECT, $entity );
+ }
+
+ public function
provideExistingEntitiesAndPermissionsThatDisallowRedirect() {
+ return [
+ 'existing item, no item-redirect permission' => [
+ $this->getExistingItem(),
+ [ 'edit' => true, 'item-redirect' => false ]
+ ],
+ 'existing item, no edit permission' => [
+ $this->getExistingItem(),
+ [ 'edit' => false, 'item-redirect' => true ]
+ ],
+ 'existing item, no edit nor item-redirect permission'
=> [
+ $this->getExistingItem(),
+ [ 'edit' => false, 'item-redirect' => false ]
+ ],
+ 'existing item, no read permission' => [
+ $this->getExistingItem(),
+ [ 'read' => false, 'edit' => true,
'item-redirect' => true ]
+ ],
+ // TODO: should this be even tested? Or should it
return false/throw exception for properties?
+ 'existing property, no edit permission' => [
+ $this->getExistingProperty(),
+ [ 'edit' => false ]
+ ],
+ 'existing property, no read permission' => [
+ $this->getExistingProperty(),
+ [ 'read' => false, 'edit' => true ]
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider
provideExistingEntitiesAndPermissionsThatAllowEditingTerms
+ */
+ public function testAllRequiredPermissionsAreNeededToEditTerms(
EntityDocument $entity, array $groupPermissions ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertUserIsAllowedTo(
EntityPermissionChecker::ACTION_EDIT_TERMS, $entity );
+ }
+
+ public function
provideExistingEntitiesAndPermissionsThatAllowEditingTerms() {
+ return [
+ 'existing item, edit and item-term permissions' => [
+ $this->getExistingItem(),
+ [ 'edit' => true, 'item-term' => true ]
+ ],
+ 'existing property, edit and property-term permissions'
=> [
+ $this->getExistingProperty(),
+ [ 'edit' => true, 'property-term' => true ]
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider
provideExistingEntitiesAndPermissionsThatDisallowEditingTerms
+ */
+ public function
testAllRequiredPermissionsAreNeededToEditTerms_failures( EntityDocument
$entity, array $groupPermissions ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertItIsForbiddenForUserTo(
EntityPermissionChecker::ACTION_EDIT_TERMS, $entity );
+ }
+
+ public function
provideExistingEntitiesAndPermissionsThatDisallowEditingTerms() {
+ return [
+ 'existing item, no item-term permission' => [
+ $this->getExistingItem(),
+ [ 'edit' => true, 'item-term' => false ]
+ ],
+ 'existing item, no edit permission' => [
+ $this->getExistingItem(),
+ [ 'edit' => false, 'item-term' => true ]
+ ],
+ 'existing item, no edit nor item-term permission' => [
+ $this->getExistingItem(),
+ [ 'edit' => false, 'item-term' => false ]
+ ],
+ 'existing item, no read permission' => [
+ $this->getExistingItem(),
+ [ 'read' => false, 'edit' => true, 'item-term'
=> true ]
+ ],
+ 'existing property, no property-term permission' => [
+ $this->getExistingProperty(),
+ [ 'edit' => true, 'property-term' => false ]
+ ],
+ 'existing property, no edit permission' => [
+ $this->getExistingProperty(),
+ [ 'edit' => false, 'property-term' => true ]
+ ],
+ 'existing property, no edit nor property-term
permission' => [
+ $this->getExistingProperty(),
+ [ 'edit' => false, 'property-term' => false ]
+ ],
+ 'existing property, no read permission' => [
+ $this->getExistingProperty(),
+ [ 'read' => false, 'edit' => true,
'property-term' => true ]
+ ],
+ ];
+ }
+
+ public function
testGivenUnknownPermission_getPermissionStatusForEntityThrowsException() {
+ $checker = $this->getPermissionChecker();
+
+ $this->setExpectedException( InvalidArgumentException::class );
+
+ $checker->getPermissionStatusForEntity(
+ $this->getTestUser()->getUser(),
+ 'turn-into-an-elephant',
+ $this->getExistingItem()
+ );
+ }
+
+ public function
testGivenUnknownPermission_getPermissionStatusForEntityIdThrowsException() {
+ $checker = $this->getPermissionChecker();
+
+ $this->setExpectedException( InvalidArgumentException::class );
+
+ $checker->getPermissionStatusForEntityId(
+ $this->getTestUser()->getUser(),
+ 'turn-into-an-elephant',
+ $this->getExistingItem()->getId()
+ );
+ }
+
+ public function
testGivenUnknownPermission_getPermissionStatusForEntityTypeThrowsException() {
+ $checker = $this->getPermissionChecker();
+
+ $this->setExpectedException( InvalidArgumentException::class );
+
+ $checker->getPermissionStatusForEntityType(
+ $this->getTestUser()->getUser(),
+ 'turn-into-an-elephant',
+ $this->getExistingItem()->getType()
+ );
+ }
+
+ private function getNamespaceLookup() {
+ // TODO: do not use those constants?
+ return new EntityNamespaceLookup( [ 'item' => WB_NS_ITEM,
'property' => WB_NS_PROPERTY ] );
+ }
+
+ /**
+ * @return EntityTitleLookup
+ */
+ private function getTitleLookup() {
+ $lookup = $this->getMock( EntityTitleLookup::class );
+
+ $lookup->method( 'getTitleForId' )
+ ->will( $this->returnCallback( function( EntityId $id )
{
+ if ( $id->getSerialization() ===
self::EXISTING_ITEM_ID ) {
+ return Title::newFromTitleValue( new
TitleValue( WB_NS_ITEM, 'Test_Item' ) );
+ }
+ if ( $id->getSerialization() ===
self::EXISTING_PROPERTY_ID ) {
+ return Title::newFromTitleValue( new
TitleValue( WB_NS_PROPERTY, 'Test_Property' ) );
+ }
+ return null;
+ } ) );
+
+ return $lookup;
+ }
+
+ private function getPermissionChecker() {
+ return new WikiPageEntityStorePermissionChecker(
+ $this->getNamespaceLookup(),
+ $this->getTitleLookup(),
+ [
+ 'read',
+ 'edit',
+ 'createpage',
+ 'property-create',
+ 'item-term',
+ 'property-term',
+ 'item-redirect',
+ 'item-merge'
+ ]
+ );
+ }
+
+ private function anyUserHasPermissions( array $permissions ) {
+ // All allowed by default, have tests specify explicitly what
permission they rely on.
+ $defaultPermissions = [
+ 'read' => true,
+ 'edit' => true,
+ 'createpage' => true,
+ 'property-create' => true,
+ 'item-term' => true,
+ 'property-term' => true,
+ 'item-redirect' => true,
+ 'item-merge' => true,
+ ];
+
+ $permissions = array_merge( $defaultPermissions, $permissions );
+
+ $this->setMwGlobals( 'wgGroupPermissions', [ '*' =>
$permissions ] );
+ }
+
+ private function getExistingItem() {
+ return new Item( new ItemId( self::EXISTING_ITEM_ID ) );
+ }
+
+ private function getNonExistingItem() {
+ return new Item( new ItemId( self::NON_EXISTING_ITEM_ID ) );
+ }
+
+ private function getNonExistingItemWithNullId() {
+ return new Item( null );
+ }
+
+ private function getExistingProperty() {
+ return new Property( new PropertyId( self::EXISTING_PROPERTY_ID
), null, 'test' );
+ }
+
+ private function getNonExistingProperty() {
+ return new Property( new PropertyId(
self::NON_EXISTING_PROPERTY_ID ), null, 'test' );
+ }
+
+ private function getNonExistingPropertyWithNullId() {
+ return new Property( null, null, 'test' );
+ }
+
+ /**
+ * @param string $action
+ * @param EntityDocument $entity
+ */
+ private function assertUserIsAllowedTo( $action, EntityDocument $entity
) {
+ $user = $this->getTestUser()->getUser();
+
+ $permissionChecker = $this->getPermissionChecker();
+ $statusForEntity =
$permissionChecker->getPermissionStatusForEntity(
+ $user,
+ $action,
+ $entity
+ );
+ $statusForType =
$permissionChecker->getPermissionStatusForEntityType(
+ $user,
+ $action,
+ $entity->getType()
+ );
+
+ if ( $entity->getId() !== null ) {
+ $statusForId =
$permissionChecker->getPermissionStatusForEntityId(
+ $user,
+ $action,
+ $entity->getId()
+ );
+ $this->assertTrue( $statusForId->isOK() );
+ }
+
+ $this->assertTrue( $statusForEntity->isOK() );
+ $this->assertTrue( $statusForType->isOK() );
+ }
+
+ /**
+ * @param string $action
+ * @param EntityDocument $entity
+ */
+ private function assertItIsForbiddenForUserTo( $action, EntityDocument
$entity ) {
+ $user = $this->getTestUser()->getUser();
+
+ $permissionChecker = $this->getPermissionChecker();
+ $statusForEntity =
$permissionChecker->getPermissionStatusForEntity(
+ $user,
+ $action,
+ $entity
+ );
+ $statusForType =
$permissionChecker->getPermissionStatusForEntityType(
+ $user,
+ $action,
+ $entity->getType()
+ );
+
+ if ( $entity->getId() !== null ) {
+ $statusForId =
$permissionChecker->getPermissionStatusForEntityId(
+ $user,
+ $action,
+ $entity->getId()
+ );
+ $this->assertFalse( $statusForId->isOK() );
+ }
+
+ $this->assertFalse( $statusForEntity->isOK() );
+ $this->assertFalse( $statusForType->isOK() );
+ }
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/361693
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifb78d909cacb9e63168898ed35fd0f286ad711e1
Gerrit-PatchSet: 20
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: WMDE-leszek <[email protected]>
Gerrit-Reviewer: Aleksey Bekh-Ivanov (WMDE) <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Ladsgroup <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
