Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/119058
Change subject: (bug 62491) Service for checking permissions.
......................................................................
(bug 62491) Service for checking permissions.
This introduces the EntityPermissionChecker service and
removes any methods for permission checking from EntityContent.
That allows us to get rid of the last reason ApiWikibase
needed to depend on EntityContent.
Change-Id: I3323ba922e33171563ed69ffa64a64829b563ae1
---
A lib/includes/store/EntityPermissionChecker.php
M repo/includes/EditEntity.php
M repo/includes/WikibaseRepo.php
M repo/includes/actions/ViewEntityAction.php
M repo/includes/api/ApiWikibase.php
M repo/includes/api/LinkTitles.php
M repo/includes/api/MergeItems.php
M repo/includes/api/ModifyClaim.php
M repo/includes/api/ModifyEntity.php
M repo/includes/content/EntityContent.php
M repo/includes/content/EntityContentFactory.php
M repo/tests/phpunit/includes/PermissionsHelper.php
M repo/tests/phpunit/includes/api/PermissionsTest.php
M repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
M repo/tests/phpunit/includes/content/EntityContentTest.php
15 files changed, 381 insertions(+), 350 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/58/119058/1
diff --git a/lib/includes/store/EntityPermissionChecker.php
b/lib/includes/store/EntityPermissionChecker.php
new file mode 100644
index 0000000..a4463ac
--- /dev/null
+++ b/lib/includes/store/EntityPermissionChecker.php
@@ -0,0 +1,69 @@
+<?php
+
+namespace Wikibase;
+
+use Status;
+use User;
+
+/**
+ * Service interface for checking a user's permissions on a given entity.
+ *
+ * @license GPL 2+
+ * @author Daniel Kinzler
+ */
+interface EntityPermissionChecker {
+
+ /**
+ * Check whether the given user has the given permission 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 Entity $entity
+ * @param string $relaxed Flag for allowing relaxed permission
checking. If set to
+ * 'relaxed', implementations may return inaccurate results if
determining the accurate result
+ * is deemed expensive. This is intended as an optimization for
non-critical checks,
+ * e.g. for showing or hiding UI elements.
+ *
+ * @return Status a status object representing the check's result.
+ */
+ public function getPermissionStatusForEntity( User $user, $permission,
Entity $entity, $relaxed = '' );
+
+ /**
+ * Check whether the given user has the given permission on an entity.
+ * This requires the ID of an existing entity.
+ *
+ * @param User $user
+ * @param string $permission
+ * @param EntityId $entityId
+ * @param string $relaxed Flag for allowing relaxed permission
checking. If set to
+ * 'relaxed', implementations may return inaccurate results if
determining the accurate result
+ * is deemed expensive. This is intended as an optimization for
non-critical checks,
+ * e.g. for showing or hiding UI elements.
+ *
+ * @return Status a status object representing the check's result.
+ */
+ public function getPermissionStatusForEntityId( User $user,
$permission, EntityId $entityId, $relaxed = '' );
+
+ /**
+ * Check whether the given user has the given permission 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 $type
+ * @param string $relaxed Flag for allowing relaxed permission
checking. If set to
+ * 'relaxed', implementations may return inaccurate results if
determining the accurate result
+ * is deemed expensive. This is intended as an optimization for
non-critical checks,
+ * e.g. for showing or hiding UI elements.
+ *
+ * @return Status a status object representing the check's result.
+ */
+ public function getPermissionStatusForEntityType( User $user,
$permission, $type, $relaxed = '' );
+
+}
\ No newline at end of file
diff --git a/repo/includes/EditEntity.php b/repo/includes/EditEntity.php
index 58b9350..8de7f5f 100644
--- a/repo/includes/EditEntity.php
+++ b/repo/includes/EditEntity.php
@@ -202,8 +202,10 @@
$this->titleLookup = $titleLookup;
$this->entityLookup = $entityLookup;
$this->entityStore = $entityStore;
- }
+ //FIXME: inject me!
+ $this->permissionChecker =
WikibaseRepo::getDefaultInstance()->getEntityPermissionChecker();
+ }
/**
* Sets the pre-safe checks to apply
@@ -558,7 +560,10 @@
wfProfileIn( __METHOD__ );
foreach ( $this->requiredPermissions as $action ) {
- $permissionStatus = $this->checkPermission( $action );
+ $permissionStatus =
$this->permissionChecker->getPermissionStatusForEntity(
+ $this->user,
+ $action,
+ $this->newEntity );
$this->status->merge( $permissionStatus );
@@ -569,52 +574,6 @@
}
wfProfileOut( __METHOD__ );
- }
-
- /**
- * Checks whether the user can perform the given action.
- *
- * @param string $permission the permission to check
- * @param bool $doExpensiveQueries whether to perform expensive checks
(default: true). May
- * be set to false for non-critical checks.
- *
- * @todo: move this to a separate service
- *
- * @return Status a status object representing the check's result.
- */
- public function checkPermission( $permission, $doExpensiveQueries =
true ) {
- wfProfileIn( __METHOD__ );
-
- $user = $this->user;
-
- $title = $this->getTitle();
- $errors = null;
-
- if ( !$title ) {
- $ns = $this->titleLookup->getNamespaceForType(
$this->getNewEntity()->getType() );
- $title = Title::makeTitleSafe( $ns, '/' );
-
- if ( $permission == 'edit' ) {
- // when checking for edit rights on an item
that doesn't yet exists, check create rights first.
-
- $errors = $title->getUserPermissionsErrors(
'createpage', $user, $doExpensiveQueries );
- }
- }
-
- if ( empty( $errors ) ) {
- // only do this if we don't already have errors from an
earlier check, to avoid redundant messages
- $errors = $title->getUserPermissionsErrors(
$permission, $user, $doExpensiveQueries );
- }
-
- $status = Status::newGood();
-
- foreach ( $errors as $error ) {
- call_user_func_array( array( $status, 'error'), $error
);
- $status->setResult( false );
- }
-
- wfProfileOut( __METHOD__ );
- return $status;
}
/**
diff --git a/repo/includes/WikibaseRepo.php b/repo/includes/WikibaseRepo.php
index 4ca0e96..60f8080 100644
--- a/repo/includes/WikibaseRepo.php
+++ b/repo/includes/WikibaseRepo.php
@@ -11,6 +11,7 @@
use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\EntityContentFactory;
use Wikibase\EntityLookup;
+use Wikibase\EntityPermissionChecker;
use Wikibase\EntityRevisionLookup;
use Wikibase\store\EntityStore;
use Wikibase\EntityTitleLookup;
@@ -482,4 +483,11 @@
$langCode
);
}
+
+ /**
+ * @return EntityPermissionChecker
+ */
+ public function getEntityPermissionChecker() {
+ return $this->getEntityContentFactory();
+ }
}
diff --git a/repo/includes/actions/ViewEntityAction.php
b/repo/includes/actions/ViewEntityAction.php
index 54d9117..f458072 100644
--- a/repo/includes/actions/ViewEntityAction.php
+++ b/repo/includes/actions/ViewEntityAction.php
@@ -22,7 +22,13 @@
protected $languageFallbackChain;
/**
- * Get the language fallback chain for current context.
+ * @var EntityPermissionChecker
+ */
+ protected $permissionChecker;
+
+ /**
+ * Get the language fallback chain.
+ * Uses the default WikibaseRepo instance to get the service if it was
not previously set.
*
* @since 0.4
*
@@ -46,6 +52,29 @@
*/
public function setLanguageFallbackChain( LanguageFallbackChain $chain
) {
$this->languageFallbackChain = $chain;
+ }
+
+ /**
+ * Get permission checker.
+ * Uses the default WikibaseRepo instance to get the service if it was
not previously set.
+ *
+ * @return \Wikibase\EntityPermissionChecker
+ */
+ public function getPermissionChecker() {
+ if ( $this->permissionChecker === null ) {
+ $this->permissionChecker =
WikibaseRepo::getDefaultInstance()->getEntityPermissionChecker();
+ }
+
+ return $this->permissionChecker;
+ }
+
+ /**
+ * Set permission checker.
+ *
+ * @param \Wikibase\EntityPermissionChecker $permissionChecker
+ */
+ public function setPermissionChecker( $permissionChecker ) {
+ $this->permissionChecker = $permissionChecker;
}
/**
@@ -146,7 +175,12 @@
// NOTE: page-wide property, independent of user permissions
$out->addJsConfigVars( 'wbIsEditView', $editable );
- $editable = ( $editable && $content->userCanEdit( null, false )
);
+ $permissionChecker = $this->getPermissionChecker();
+ $editable = ( $editable &&
$permissionChecker->getPermissionStatusForEntity(
+ $this->getUser(),
+ 'edit',
+ $content->getEntity(),
+ 'relaxed' ) );
// View it!
$parserOptions =
$this->getArticle()->getPage()->makeParserOptions(
$this->getContext()->getUser() );
diff --git a/repo/includes/api/ApiWikibase.php
b/repo/includes/api/ApiWikibase.php
index d346351..7c553cd 100644
--- a/repo/includes/api/ApiWikibase.php
+++ b/repo/includes/api/ApiWikibase.php
@@ -14,6 +14,7 @@
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\EntityFactory;
+use Wikibase\EntityPermissionChecker;
use Wikibase\EntityRevision;
use Wikibase\EntityRevisionLookup;
use Wikibase\EntityTitleLookup;
@@ -89,6 +90,11 @@
protected $summaryFormatter;
/**
+ * @var EntityPermissionChecker
+ */
+ protected $permissionChecker;
+
+ /**
* @param ApiMain $mainModule
* @param string $moduleName
* @param string $modulePrefix
@@ -107,6 +113,8 @@
$this->dataTypeLookup =
WikibaseRepo::getDefaultInstance()->getPropertyDataTypeLookup();
$this->summaryFormatter =
WikibaseRepo::getDefaultInstance()->getSummaryFormatter();
+
+ $this->permissionChecker =
WikibaseRepo::getDefaultInstance()->getEntityPermissionChecker();
}
/**
@@ -213,13 +221,26 @@
* Returns the permissions that are required to perform the operation
specified by
* the parameters.
*
+ * Per default, this will include the 'read' permission if
$this->isReadMode() returns true,
+ * and the 'edit' permission if $this->isWriteMode() returns true,
+ *
* @param Entity $entity The entity to check permissions for
* @param array $params Arguments for the module, describing the
operation to be performed
*
* @return array A list of permissions
*/
protected function getRequiredPermissions( Entity $entity, array
$params ) {
- return array();
+ $permissions = array();
+
+ if ( $this->isReadMode() ) {
+ $permissions[] = 'read';
+ }
+
+ if ( $this->isWriteMode() ) {
+ $permissions[] = 'edit';
+ }
+
+ return $permissions;
}
/**
@@ -236,11 +257,8 @@
$permissions = $this->getRequiredPermissions( $entity, $params
);
$status = Status::newGood();
- //TODO: factor permission check logic out of EntityContent
- $entityContent =
WikibaseRepo::getDefaultInstance()->getEntityContentFactory()->newFromEntity(
$entity );
-
foreach ( $permissions as $perm ) {
- $permStatus = $entityContent->checkPermission( $perm,
$user, true );
+ $permStatus =
$this->permissionChecker->getPermissionStatusForEntity( $user, $perm, $entity );
$status->merge( $permStatus );
}
diff --git a/repo/includes/api/LinkTitles.php b/repo/includes/api/LinkTitles.php
index 8f9dd0f..208a0b3 100644
--- a/repo/includes/api/LinkTitles.php
+++ b/repo/includes/api/LinkTitles.php
@@ -44,20 +44,6 @@
}
/**
- * @see \Wikibase\Api\ApiWikiBase::getRequiredPermissions()
- *
- * @param Entity $entity
- * @param array $params
- *
- * @return array|\Status
- */
- protected function getRequiredPermissions( Entity $entity, array
$params ) {
- $permissions = parent::getRequiredPermissions( $entity, $params
);
- $permissions[] = 'edit';
- return $permissions;
- }
-
- /**
* Main method. Does the actual work and sets the result.
*
* @since 0.1
@@ -212,13 +198,6 @@
* @see \ApiBase::isWriteMode()
*/
public function isWriteMode() {
- return true;
- }
-
- /**
- * @see ApiBase::mustBePosted
- */
- public function mustBePosted() {
return true;
}
diff --git a/repo/includes/api/MergeItems.php b/repo/includes/api/MergeItems.php
index b87ef52..bd3382f 100644
--- a/repo/includes/api/MergeItems.php
+++ b/repo/includes/api/MergeItems.php
@@ -56,7 +56,6 @@
*/
protected function getRequiredPermissions( Entity $entity, array
$params ) {
$permissions = parent::getRequiredPermissions( $entity, $params
);
- $permissions[] = 'edit';
$permissions[] = 'item-merge';
return $permissions;
}
@@ -295,13 +294,6 @@
* @return bool true
*/
public function isWriteMode() {
- return true;
- }
-
- /**
- * @see ApiBase::mustBePosted
- */
- public function mustBePosted() {
return true;
}
diff --git a/repo/includes/api/ModifyClaim.php
b/repo/includes/api/ModifyClaim.php
index 2097f71..2a2e57e 100644
--- a/repo/includes/api/ModifyClaim.php
+++ b/repo/includes/api/ModifyClaim.php
@@ -90,13 +90,6 @@
}
/**
- * @see ApiBase::mustBePosted
- */
- public function mustBePosted() {
- return true;
- }
-
- /**
* @see ApiBase::getPossibleErrors()
*/
public function getPossibleErrors() {
@@ -111,20 +104,6 @@
array( 'code' => 'invalid-entity-id', 'info' =>
$this->msg( 'wikibase-api-invalid-entity-id' )->text() ),
)
);
- }
-
- /**
- * @see \Wikibase\Api\ApiWikibase::getRequiredPermissions()
- *
- * @param Entity $entity
- * @param array $params
- *
- * @return array|\Status
- */
- protected function getRequiredPermissions( Entity $entity, array
$params ) {
- $permissions = parent::getRequiredPermissions( $entity, $params
);
- $permissions[] = 'edit';
- return $permissions;
}
/**
diff --git a/repo/includes/api/ModifyEntity.php
b/repo/includes/api/ModifyEntity.php
index 0b79917..767b0e7 100644
--- a/repo/includes/api/ModifyEntity.php
+++ b/repo/includes/api/ModifyEntity.php
@@ -84,20 +84,6 @@
protected $flags;
/**
- * @see ApiWikibase::getRequiredPermissions()
- *
- * @param Entity $entity
- * @param array $params
- *
- * @return array|\Status
- */
- protected function getRequiredPermissions( Entity $entity, array
$params ) {
- $permissions = parent::getRequiredPermissions( $entity, $params
);
- $permissions[] = 'edit';
- return $permissions;
- }
-
- /**
* Get the entity using the id, site and title params passed to the api
*
* @since 0.1
@@ -376,13 +362,6 @@
* @see ApiBase::isWriteMode()
*/
public function isWriteMode() {
- return true;
- }
-
- /**
- * @see ApiBase::mustBePosted
- */
- public function mustBePosted() {
return true;
}
diff --git a/repo/includes/content/EntityContent.php
b/repo/includes/content/EntityContent.php
index b66e113..1e21cb6 100644
--- a/repo/includes/content/EntityContent.php
+++ b/repo/includes/content/EntityContent.php
@@ -436,92 +436,6 @@
return static::newFromArray( $array );
}
-
- /**
- * Checks whether the user can perform the given action on this entity.
- *
- * Shorthand for $this->checkPermission( $permission )->isOK();
- *
- * @param String $permission the permission to check
- * @param null|User $user the user to check for. If
omitted, $wgUser is checked.
- * @param bool $doExpensiveQueries whether to perform expensive
checks (default: true). May
- * be set to false for
non-critical checks.
- *
- * @return bool True if the user has the given permission, false
otherwise.
- */
- public function userCan( $permission, User $user = null,
$doExpensiveQueries = true ) {
- $status = $this->checkPermission( $permission, $user,
$doExpensiveQueries );
- return $status->isOK();
- }
-
- /**
- * Determine whether the given user can edit this entity.
- *
- * Shorthand for $this->userCan( 'edit' );
- *
- * @param null|User $user the user to check for. If
omitted, $wgUser is checked.
- * @param bool $doExpensiveQueries whether to perform expensive
checks (default: true). May
- * be set to false for
non-critical checks.
- *
- * @return bool whether the user is allowed to edit this item.
- */
- public function userCanEdit( User $user = null, $doExpensiveQueries =
true ) {
- return $this->userCan( 'edit', $user, $doExpensiveQueries );
- }
-
- /**
- * Checks whether the user can perform the given action.
- *
- * @param String $permission the permission to check
- * @param null|User $user the user to check for. If
omitted, $wgUser is checked.
- * @param bool $doExpensiveQueries whether to perform expensive
checks (default: true). May
- * be set to false for
non-critical checks.
- *
- * @return \Status a status object representing the check's result.
- */
- public function checkPermission( $permission, User $user = null,
$doExpensiveQueries = true ) {
- global $wgUser;
- static $dummyTitle = null;
-
- wfProfileIn( __METHOD__ );
-
- if ( !$user ) {
- $user = $wgUser;
- }
-
- $title = $this->getTitle();
- $errors = null;
-
- if ( !$title ) {
- if ( !$dummyTitle ) {
- $dummyTitle = Title::makeTitleSafe(
$this->getContentHandler()->getEntityNamespace(), '/' );
- }
-
- $title = $dummyTitle;
-
- if ( $permission == 'edit' ) {
- // when checking for edit rights on an item
that doesn't yet exists, check create rights first.
-
- $errors = $title->getUserPermissionsErrors(
'createpage', $user, $doExpensiveQueries );
- }
- }
-
- if ( empty( $errors ) ) {
- // only do this if we don't already have errors from an
earlier check, to avoid redundant messages
- $errors = $title->getUserPermissionsErrors(
$permission, $user, $doExpensiveQueries );
- }
-
- $status = Status::newGood();
-
- foreach ( $errors as $error ) {
- call_user_func_array( array( $status, 'error'), $error
);
- $status->setResult( false );
- }
-
- wfProfileOut( __METHOD__ );
- return $status;
- }
-
/**
* Assigns a fresh ID to this entity.
*
diff --git a/repo/includes/content/EntityContentFactory.php
b/repo/includes/content/EntityContentFactory.php
index 85fbc09..efd1c73 100644
--- a/repo/includes/content/EntityContentFactory.php
+++ b/repo/includes/content/EntityContentFactory.php
@@ -4,7 +4,9 @@
use MWException;
use InvalidArgumentException;
+use Status;
use Title;
+use User;
use WikiPage;
use Revision;
@@ -16,7 +18,7 @@
* @licence GNU GPL v2+
* @author Jeroen De Dauw < jeroended...@gmail.com >
*/
-class EntityContentFactory implements EntityTitleLookup {
+class EntityContentFactory implements EntityTitleLookup,
EntityPermissionChecker {
// TODO: inject this map and allow extensions to somehow extend it
protected static $typeMap = array(
@@ -216,4 +218,106 @@
}
}
+ /**
+ * @see EntityPermissionChecker::getPermissionStatusForEntityId
+ *
+ * @param User $user
+ * @param string $permission
+ * @param Title $entityPage
+ * @param string $relaxed
+ *
+ * @return Status a status object representing the check's result.
+ */
+ protected function getPermissionStatus( User $user, $permission, Title
$entityPage, $relaxed = '' ) {
+ wfProfileIn( __METHOD__ );
+
+ $errors = $entityPage->getUserPermissionsErrors( $permission,
$user, $relaxed !== 'relaxed' );
+ $status = Status::newGood();
+
+ foreach ( $errors as $error ) {
+ call_user_func_array( array( $status, 'fatal'), $error
);
+ $status->setResult( false );
+ }
+
+ wfProfileOut( __METHOD__ );
+ return $status;
+ }
+
+ /**
+ * @see EntityPermissionChecker::getPermissionStatusForEntityId
+ *
+ * @param User $user
+ * @param string $permission
+ * @param EntityId $entityId
+ * @param string $relaxed
+ *
+ * @return Status a status object representing the check's result.
+ */
+ public function getPermissionStatusForEntityId( User $user,
$permission, EntityId $entityId, $relaxed = '' ) {
+ wfProfileIn( __METHOD__ );
+
+ $title = $this->getTitleForId( $entityId );
+ $status = $this->getPermissionStatus( $user, $permission,
$title );
+
+ wfProfileOut( __METHOD__ );
+ return $status;
+ }
+
+ /**
+ * @see EntityPermissionChecker::getPermissionStatusForEntityType
+ *
+ * @param User $user
+ * @param string $permission
+ * @param string $type
+ * @param string $relaxed
+ *
+ * @return Status a status object representing the check's result.
+ */
+ public function getPermissionStatusForEntityType( User $user,
$permission, $type, $relaxed = '' ) {
+ wfProfileIn( __METHOD__ );
+
+ $ns = $this->getNamespaceForType( $type );
+ $dummyTitle = Title::makeTitleSafe( $ns, '/' );
+
+ $status = $this->getPermissionStatus( $user, $permission,
$dummyTitle );
+
+ wfProfileOut( __METHOD__ );
+ return $status;
+ }
+
+ /**
+ * @see EntityPermissionChecker::getPermissionStatusForEntity
+ *
+ * @note When checking for the 'edit' permission, this will check the
'createpage'
+ * permission first in case the entity does not yet exist (i.e. if
$entity->getId()
+ * returns null).
+ *
+ * @param User $user
+ * @param string $permission
+ * @param Entity $entity
+ * @param string $relaxed
+ *
+ * @return Status a status object representing the check's result.
+ */
+ public function getPermissionStatusForEntity( User $user, $permission,
Entity $entity, $relaxed = '' ) {
+ $id = $entity->getId();
+ $status = null;
+
+ if ( !$id ) {
+ $type = $entity->getType();
+
+ if ( $permission === 'edit' ) {
+ // for editing a non-existing page, check the
createpage permission
+ $status =
$this->getPermissionStatusForEntityType( $user, 'createpage', $type );
+ }
+
+ if ( !$status || $status->isOK() ) {
+ $status =
$this->getPermissionStatusForEntityType( $user, $permission, $type );
+ }
+ } else {
+ $status = $this->getPermissionStatusForEntityId( $user,
$permission, $id );
+ }
+
+ return $status;
+ }
}
diff --git a/repo/tests/phpunit/includes/PermissionsHelper.php
b/repo/tests/phpunit/includes/PermissionsHelper.php
index 11e0d1b..4057eff 100644
--- a/repo/tests/phpunit/includes/PermissionsHelper.php
+++ b/repo/tests/phpunit/includes/PermissionsHelper.php
@@ -12,11 +12,14 @@
/**
* Utility function for applying a set of permissions to
$wgGroupPermissions.
* Automatically resets the rights cache for $wgUser.
+ * This modifies the global $wgGroupPermissions and $wgUser variables.
* No measures are taken to restore the original permissions later,
this is up to the caller.
*
* @param $permissions
+ * @param null|array $groups groups to apply to $wgUser. If not given,
group
+ * membership is not modified.
*/
- public static function applyPermissions( $permissions ) {
+ public static function applyPermissions( $permissions, $groups = null )
{
global $wgGroupPermissions;
global $wgUser;
@@ -24,6 +27,17 @@
return;
}
+ if ( is_array( $groups ) ) {
+ $oldGroups = $wgUser->getGroups();
+ foreach ( $oldGroups as $group ) {
+ $wgUser->removeGroup( $group );
+ }
+
+ foreach ( $groups as $group ) {
+ $wgUser->addGroup( $group );
+ }
+ }
+
foreach ( $permissions as $group => $rights ) {
if ( !empty( $wgGroupPermissions[ $group ] ) ) {
$wgGroupPermissions[ $group ] = array_merge(
$wgGroupPermissions[ $group ], $rights );
diff --git a/repo/tests/phpunit/includes/api/PermissionsTest.php
b/repo/tests/phpunit/includes/api/PermissionsTest.php
index e9ab6c7..4e17aa8 100644
--- a/repo/tests/phpunit/includes/api/PermissionsTest.php
+++ b/repo/tests/phpunit/includes/api/PermissionsTest.php
@@ -21,7 +21,7 @@
* @group Database
* @group medium
*/
-class PermissionsTest extends PermissionsTestCase {
+abstract class PermissionsTest extends PermissionsTestCase {
public function provideReadPermissions() {
return array(
diff --git a/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
b/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
index 0c2c819..7cd6876 100644
--- a/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
+++ b/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
@@ -17,10 +17,14 @@
* @group WikibaseContent
* @group WikibaseRepo
*
+ * @group Database
+ * ^--- just because we use the Title class
+ *
* @licence GNU GPL v2+
* @author Jeroen De Dauw < jeroended...@gmail.com >
+ * @author Daniel Kinzler
*/
-class EntityContentFactoryTest extends \PHPUnit_Framework_TestCase {
+class EntityContentFactoryTest extends \MediaWikiTestCase {
/**
* @dataProvider contentModelsProvider
@@ -125,4 +129,116 @@
$entityContent = $entityContentFactory->newFromType( $type );
}
+ public function provideGetPermissionStatusForEntity() {
+ return array(
+ 'read allowed for non-existing entity' => array(
+ 'read',
+ array( 'read' => true ),
+ null,
+ array(
+ 'getPermissionStatusForEntity' => true,
+ 'getPermissionStatusForEntityType' =>
true,
+ ),
+ ),
+ 'edit and createpage allowed for new entity' => array(
+ 'edit',
+ array( 'read' => true, 'edit' => true,
'createpage' => true ),
+ null,
+ array(
+ 'getPermissionStatusForEntity' => true,
+ 'getPermissionStatusForEntityType' =>
true,
+ ),
+ ),
+ 'implicit createpage not allowed for new entity' =>
array(
+ 'edit',
+ array( 'read' => true, 'edit' => true,
'createpage' => false ),
+ null,
+ array(
+ 'getPermissionStatusForEntity' =>
false, // "createpage" is implicitly needed
+ 'getPermissionStatusForEntityType' =>
true, // "edit" is allowed for type
+ ),
+ ),
+ 'createpage not allowed' => array(
+ 'createpage',
+ array( 'read' => true, 'edit' => true,
'createpage' => false ),
+ null,
+ array(
+ 'getPermissionStatusForEntity' =>
false, // "createpage" is implicitly needed
+ 'getPermissionStatusForEntityType' =>
false, // "createpage" is not allowed
+ ),
+ ),
+ 'edit allowed for existing item' => array(
+ 'edit',
+ array( 'read' => true, 'edit' => true,
'createpage' => false ),
+ 'Q23',
+ array(
+ 'getPermissionStatusForEntity' => true,
+ 'getPermissionStatusForEntityType' =>
true,
+ 'getPermissionStatusForEntityId' =>
true,
+ ),
+ ),
+ 'edit not allowed' => array(
+ 'edit',
+ array( 'read' => true, 'edit' => false ),
+ 'Q23',
+ array(
+ 'getPermissionStatusForEntity' => false,
+ 'getPermissionStatusForEntityType' =>
false,
+ 'getPermissionStatusForEntityId' =>
false,
+ ),
+ ),
+ 'delete not allowed' => array(
+ 'delete',
+ array( 'read' => true, 'delete' => false ),
+ null,
+ array(
+ 'getPermissionStatusForEntity' => false,
+ 'getPermissionStatusForEntityType' =>
false,
+ ),
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider provideGetPermissionStatusForEntity
+ */
+ public function testGetPermissionStatusForEntity( $action,
$permissions, $id, $expectations ) {
+ global $wgUser;
+
+ $entity = Item::newEmpty();
+
+ if ( $id ) {
+ // "exists"
+ $entity->setId( new ItemId( $id ) );
+ }
+
+ $this->stashMwGlobals( 'wgUser' );
+ $this->stashMwGlobals( 'wgGroupPermissions' );
+
+ PermissionsHelper::applyPermissions(
+ // set permissions for implicit groups
+ array( '*' => $permissions,
+ 'user' => $permissions,
+ 'autoconfirmed' => $permissions,
+ 'emailconfirmed' => $permissions ),
+ array() // remove all groups not implied
+ );
+
+ $factory = $this->newFactory();
+
+ if ( isset( $expectations['getPermissionStatusForEntity'] ) ) {
+ $status = $factory->getPermissionStatusForEntity(
$wgUser, $action, $entity );
+ $this->assertEquals(
$expectations['getPermissionStatusForEntity'], $status->isOK() );
+ }
+
+ if ( isset( $expectations['getPermissionStatusForEntityType'] )
) {
+ $status = $factory->getPermissionStatusForEntityType(
$wgUser, $action, $entity->getType() );
+ $this->assertEquals(
$expectations['getPermissionStatusForEntityType'], $status->isOK() );
+ }
+
+ if ( isset( $expectations['getPermissionStatusForEntityId'] ) )
{
+ $status = $factory->getPermissionStatusForEntityId(
$wgUser, $action, $entity->getId() );
+ $this->assertEquals(
$expectations['getPermissionStatusForEntityId'], $status->isOK() );
+ }
+ }
}
diff --git a/repo/tests/phpunit/includes/content/EntityContentTest.php
b/repo/tests/phpunit/includes/content/EntityContentTest.php
index 2454970..a06c78f 100644
--- a/repo/tests/phpunit/includes/content/EntityContentTest.php
+++ b/repo/tests/phpunit/includes/content/EntityContentTest.php
@@ -195,140 +195,6 @@
$this->assertEquals( $prev_id,
$entityContent->getWikiPage()->getLatest(), "revision ID should stay the same
if no change was made" );
}
- public function dataCheckPermissions() {
- // FIXME: this is testing for some specific configuration and
will break if the config is changed
- return array(
- array( //0: read allowed
- 'read',
- 'user',
- array( 'read' => true ),
- false,
- true,
- ),
- array( //1: edit and createpage allowed for new item
- 'edit',
- 'user',
- array( 'read' => true, 'edit' => true,
'createpage' => true ),
- false,
- true,
- ),
- array( //2: edit allowed but createpage not allowed for
new item
- 'edit',
- 'user',
- array( 'read' => true, 'edit' => true,
'createpage' => false ),
- false,
- false,
- ),
- array( //3: edit allowed but createpage not allowed for
existing item
- 'edit',
- 'user',
- array( 'read' => true, 'edit' => true,
'createpage' => false ),
- true,
- true,
- ),
- array( //4: edit not allowed for existing item
- 'edit',
- 'user',
- array( 'read' => true, 'edit' => false ),
- true,
- false,
- ),
- array( //5: delete not allowed
- 'delete',
- 'user',
- array( 'read' => true, 'delete' => false ),
- false,
- false,
- ),
- );
- }
-
- protected function prepareItemForPermissionCheck( $group, $permissions,
$create ) {
- global $wgUser;
-
- // TODO: Figure out what is leaking the sysop group membership
- $wgUser->removeGroup('sysop');
-
- $content = $this->newEmpty();
-
- if ( $create ) {
- $label = get_class( $this ) . '/' . $group . '/' .
implode( ', ', $permissions );
- $content->getEntity()->setLabel( 'de', $label );
- $content->save( "testing", null, EDIT_NEW );
- }
-
- if ( !in_array( $group, $wgUser->getEffectiveGroups() ) ) {
- $wgUser->addGroup( $group );
- }
-
- if ( $permissions !== null ) {
- PermissionsHelper::applyPermissions( array(
- '*' => $permissions,
- 'user' => $permissions,
- $group => $permissions,
- ) );
- }
-
- return $content;
- }
-
- /**
- * @dataProvider dataCheckPermissions
- */
- public function testCheckPermission( $action, $group, $permissions,
$create, $expectedOK ) {
- $content = $this->prepareItemForPermissionCheck( $group,
$permissions, $create );
-
- $status = $content->checkPermission( $action );
-
- $this->assertEquals( $expectedOK, $status->isOK() );
- }
-
- /**
- * @dataProvider dataCheckPermissions
- */
- public function testUserCan( $action, $group, $permissions, $create,
$expectedOK ) {
- $content = $this->prepareItemForPermissionCheck( $group,
$permissions, $create );
-
- $content->checkPermission( $action );
-
- $this->assertEquals( $expectedOK, $content->userCan( $action )
);
- }
-
-
- public function dataUserCanEdit() {
- return array(
- array( //0: edit and createpage allowed for new item
- array( 'read' => true, 'edit' => true,
'createpage' => true ),
- false,
- true,
- ),
- array( //1: edit allowed but createpage not allowed for
new item
- array( 'read' => true, 'edit' => true,
'createpage' => false ),
- false,
- false,
- ),
- array( //2: edit allowed but createpage not allowed for
existing item
- array( 'read' => true, 'edit' => true,
'createpage' => false ),
- true,
- true,
- ),
- array( //3: edit not allowed for existing item
- array( 'read' => true, 'edit' => false ),
- true,
- false,
- ),
- );
- }
-
- /**
- * @dataProvider dataUserCanEdit
- */
- public function testUserCanEdit( $permissions, $create, $expectedOK ) {
- $content = $this->prepareItemForPermissionCheck( 'user',
$permissions, $create );
-
- $this->assertEquals( $expectedOK, $content->userCanEdit() );
- }
-
public function testGetParserOutput() {
$content = $this->newEmpty();
--
To view, visit https://gerrit.wikimedia.org/r/119058
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3323ba922e33171563ed69ffa64a64829b563ae1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
