WMDE-leszek has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/365582 )
Change subject: Consider create permissions when entity is creating with API
action for editing entity terms
......................................................................
Consider create permissions when entity is creating with API action for editing
entity terms
Bug: T166586
Change-Id: I6148d0a86197f1ad75bad09937be2fddc4e419e4
---
M repo/includes/Store/WikiPageEntityStorePermissionChecker.php
M repo/tests/phpunit/includes/Api/SetAliasesTest.php
M repo/tests/phpunit/includes/Api/SetDescriptionTest.php
M repo/tests/phpunit/includes/Api/SetLabelTest.php
M repo/tests/phpunit/includes/Api/SetSiteLinkTest.php
M repo/tests/phpunit/includes/Store/WikiPageEntityStorePermissionCheckerTest.php
6 files changed, 350 insertions(+), 22 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/82/365582/1
diff --git a/repo/includes/Store/WikiPageEntityStorePermissionChecker.php
b/repo/includes/Store/WikiPageEntityStorePermissionChecker.php
index 0831a7f..6cda6b3 100644
--- a/repo/includes/Store/WikiPageEntityStorePermissionChecker.php
+++ b/repo/includes/Store/WikiPageEntityStorePermissionChecker.php
@@ -66,11 +66,6 @@
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 );
}
@@ -93,15 +88,6 @@
$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,
@@ -110,7 +96,7 @@
);
}
- return $this->checkPermissionsForAction( $user, $action,
$title, $entityId->getEntityType(), $quick );
+ return $this->checkPermissionsForActions( $user, [ $action ],
$title, $entityId->getEntityType(), $quick );
}
/**
@@ -128,18 +114,27 @@
public function getPermissionStatusForEntityType( User $user, $action,
$type, $quick = '' ) {
$title = $this->getPageTitleInEntityNamespace( $type );
- if ( $action === EntityPermissionChecker::ACTION_EDIT ) {
+ if ( $this->isEditAction( $action ) ) {
// Note: No entity ID given, assuming creating new
entity, i.e. create permissions will be checked
- return $this->checkPermissionsForAction(
+ return $this->checkPermissionsForActions(
$user,
- EntityPermissionChecker::ACTION_CREATE,
+ [ $action,
EntityPermissionChecker::ACTION_CREATE ],
$title,
$type,
$quick
);
}
- return $this->checkPermissionsForAction( $user, $action,
$title, $type, $quick );
+ return $this->checkPermissionsForActions( $user, [ $action ],
$title, $type, $quick );
+ }
+
+ /**
+ * @param string $action
+ *
+ * @return bool
+ */
+ private function isEditAction( $action ) {
+ return $action === EntityPermissionChecker::ACTION_EDIT ||
$action === EntityPermissionChecker::ACTION_EDIT_TERMS;
}
/**
@@ -153,10 +148,19 @@
return Title::makeTitle( $namespace, '/' );
}
- private function checkPermissionsForAction( User $user, $action, Title
$title, $entityType, $quick ='' ) {
+ private function checkPermissionsForActions( User $user, array
$actions, Title $title, $entityType, $quick ='' ) {
$status = Status::newGood();
- $mediaWikiPermissions = $this->getMediaWikiPermissionsToCheck(
$action, $entityType );
+ $mediaWikiPermissions = [];
+
+ foreach ( $actions as $action ) {
+ $mediaWikiPermissions = array_merge(
+ $mediaWikiPermissions,
+ $this->getMediaWikiPermissionsToCheck( $action,
$entityType )
+ );
+ }
+
+ $mediaWikiPermissions = array_unique( $mediaWikiPermissions );
foreach ( $mediaWikiPermissions as $mwPermission ) {
$partialStatus = $this->getPermissionStatus( $user,
$mwPermission, $title, $quick );
@@ -171,9 +175,11 @@
$entityTypeSpecificCreatePermission = $entityType .
'-create';
$permissions = [ 'read', 'edit', 'createpage' ];
+
if ( $this->mediawikiPermissionExists(
$entityTypeSpecificCreatePermission ) ) {
$permissions[] =
$entityTypeSpecificCreatePermission;
}
+
return $permissions;
}
diff --git a/repo/tests/phpunit/includes/Api/SetAliasesTest.php
b/repo/tests/phpunit/includes/Api/SetAliasesTest.php
index c64df6a..15b2d15 100644
--- a/repo/tests/phpunit/includes/Api/SetAliasesTest.php
+++ b/repo/tests/phpunit/includes/Api/SetAliasesTest.php
@@ -303,6 +303,45 @@
);
}
+ public function
testUserCanCreateItemWithAliasWhenTheyHaveSufficientPermissions() {
+ $userWithAllPermissions = $this->createUserWithGroup(
'all-permission' );
+
+ $this->setMwGlobals( 'wgGroupPermissions', [
+ 'all-permission' => [ 'item-term' => true, 'createpage'
=> true ],
+ '*' => [ 'read' => true, 'edit' => true, 'writeapi' =>
true ]
+ ] );
+
+ list ( $result, ) = $this->doApiRequestWithToken(
+ $this->getCreateItemAndSetAliasRequestParams(),
+ null,
+ $userWithAllPermissions
+ );
+
+ $this->assertEquals( 1, $result['success'] );
+ $this->assertSame( 'an alias',
$result['entity']['aliases']['en'][0]['value'] );
+ }
+
+ public function testUserCannotCreateItemWhenTheyLackPermission() {
+ $userWithInsufficientPermissions = $this->createUserWithGroup(
'no-permission' );
+
+ $this->setMwGlobals( 'wgGroupPermissions', [
+ 'no-permission' => [ 'createpage' => false ],
+ '*' => [ 'read' => true, 'edit' => true, 'item-term' =>
true, 'writeapi' => true ]
+ ] );
+
+ // Then the request is denied
+ $expected = [
+ 'type' => ApiUsageException::class,
+ 'code' => 'permissiondenied'
+ ];
+
+ $this->doTestQueryExceptions(
+ $this->getCreateItemAndSetAliasRequestParams(),
+ $expected,
+ $userWithInsufficientPermissions
+ );
+ }
+
/**
* @param User $user
* @return Item
@@ -330,4 +369,13 @@
];
}
+ private function getCreateItemAndSetAliasRequestParams() {
+ return [
+ 'action' => 'wbsetaliases',
+ 'new' => 'item',
+ 'language' => 'en',
+ 'add' => 'an alias',
+ ];
+ }
+
}
diff --git a/repo/tests/phpunit/includes/Api/SetDescriptionTest.php
b/repo/tests/phpunit/includes/Api/SetDescriptionTest.php
index 99e8998..1dd35dd 100644
--- a/repo/tests/phpunit/includes/Api/SetDescriptionTest.php
+++ b/repo/tests/phpunit/includes/Api/SetDescriptionTest.php
@@ -98,6 +98,45 @@
);
}
+ public function
testUserCanCreateItemWithDescriptionWhenTheyHaveSufficientPermissions() {
+ $userWithAllPermissions = $this->createUserWithGroup(
'all-permission' );
+
+ $this->setMwGlobals( 'wgGroupPermissions', [
+ 'all-permission' => [ 'item-term' => true, 'createpage'
=> true ],
+ '*' => [ 'read' => true, 'edit' => true, 'writeapi' =>
true ]
+ ] );
+
+ list ( $result, ) = $this->doApiRequestWithToken(
+ $this->getCreateItemAndSetDescriptionRequestParams(),
+ null,
+ $userWithAllPermissions
+ );
+
+ $this->assertEquals( 1, $result['success'] );
+ $this->assertSame( 'some description',
$result['entity']['descriptions']['en']['value'] );
+ }
+
+ public function testUserCannotCreateItemWhenTheyLackPermission() {
+ $userWithInsufficientPermissions = $this->createUserWithGroup(
'no-permission' );
+
+ $this->setMwGlobals( 'wgGroupPermissions', [
+ 'no-permission' => [ 'createpage' => false ],
+ '*' => [ 'read' => true, 'edit' => true, 'item-term' =>
true, 'writeapi' => true ]
+ ] );
+
+ // Then the request is denied
+ $expected = [
+ 'type' => ApiUsageException::class,
+ 'code' => 'permissiondenied'
+ ];
+
+ $this->doTestQueryExceptions(
+ $this->getCreateItemAndSetDescriptionRequestParams(),
+ $expected,
+ $userWithInsufficientPermissions
+ );
+ }
+
/**
* @param User $user
* @return Item
@@ -125,4 +164,13 @@
];
}
+ private function getCreateItemAndSetDescriptionRequestParams() {
+ return [
+ 'action' => 'wbsetdescription',
+ 'new' => 'item',
+ 'language' => 'en',
+ 'value' => 'some description',
+ ];
+ }
+
}
diff --git a/repo/tests/phpunit/includes/Api/SetLabelTest.php
b/repo/tests/phpunit/includes/Api/SetLabelTest.php
index 4875845..1dff140 100644
--- a/repo/tests/phpunit/includes/Api/SetLabelTest.php
+++ b/repo/tests/phpunit/includes/Api/SetLabelTest.php
@@ -98,6 +98,45 @@
);
}
+ public function
testUserCanCreateItemWithLabelWhenTheyHaveSufficientPermissions() {
+ $userWithAllPermissions = $this->createUserWithGroup(
'all-permission' );
+
+ $this->setMwGlobals( 'wgGroupPermissions', [
+ 'all-permission' => [ 'item-term' => true, 'createpage'
=> true ],
+ '*' => [ 'read' => true, 'edit' => true, 'writeapi' =>
true ]
+ ] );
+
+ list ( $result, ) = $this->doApiRequestWithToken(
+ $this->getCreateItemAndSetLabelRequestParams(),
+ null,
+ $userWithAllPermissions
+ );
+
+ $this->assertEquals( 1, $result['success'] );
+ $this->assertSame( 'a label',
$result['entity']['labels']['en']['value'] );
+ }
+
+ public function testUserCannotCreateItemWhenTheyLackPermission() {
+ $userWithInsufficientPermissions = $this->createUserWithGroup(
'no-permission' );
+
+ $this->setMwGlobals( 'wgGroupPermissions', [
+ 'no-permission' => [ 'createpage' => false ],
+ '*' => [ 'read' => true, 'edit' => true, 'item-term' =>
true, 'writeapi' => true ]
+ ] );
+
+ // Then the request is denied
+ $expected = [
+ 'type' => ApiUsageException::class,
+ 'code' => 'permissiondenied'
+ ];
+
+ $this->doTestQueryExceptions(
+ $this->getCreateItemAndSetLabelRequestParams(),
+ $expected,
+ $userWithInsufficientPermissions
+ );
+ }
+
/**
* @param User $user
* @return Item
@@ -116,6 +155,15 @@
}
+ private function getCreateItemAndSetLabelRequestParams() {
+ return [
+ 'action' => 'wbsetlabel',
+ 'new' => 'item',
+ 'language' => 'en',
+ 'value' => 'a label',
+ ];
+ }
+
private function getSetLabelRequestParams( ItemId $id ) {
return [
'action' => 'wbsetlabel',
diff --git a/repo/tests/phpunit/includes/Api/SetSiteLinkTest.php
b/repo/tests/phpunit/includes/Api/SetSiteLinkTest.php
index 4ae4ecd..e93990d 100644
--- a/repo/tests/phpunit/includes/Api/SetSiteLinkTest.php
+++ b/repo/tests/phpunit/includes/Api/SetSiteLinkTest.php
@@ -610,6 +610,45 @@
);
}
+ public function
testUserCanCreateItemWithSiteLinkWhenTheyHaveSufficientPermissions() {
+ $userWithAllPermissions = $this->createUserWithGroup(
'all-permission' );
+
+ $this->setMwGlobals( 'wgGroupPermissions', [
+ 'all-permission' => [ 'edit' => true, 'createpage' =>
true ],
+ '*' => [ 'read' => true, 'writeapi' => true ]
+ ] );
+
+ list ( $result, ) = $this->doApiRequestWithToken(
+ $this->getCreateItemAndSetSiteLinkRequestParams(),
+ null,
+ $userWithAllPermissions
+ );
+
+ $this->assertEquals( 1, $result['success'] );
+ $this->assertSame( 'Another Cool Page',
$result['entity']['sitelinks']['enwiki']['title'] );
+ }
+
+ public function testUserCannotCreateItemWhenTheyLackPermission() {
+ $userWithInsufficientPermissions = $this->createUserWithGroup(
'no-permission' );
+
+ $this->setMwGlobals( 'wgGroupPermissions', [
+ 'no-permission' => [ 'createpage' => false ],
+ '*' => [ 'read' => true, 'edit' => true, 'writeapi' =>
true ]
+ ] );
+
+ // Then the request is denied
+ $expected = [
+ 'type' => ApiUsageException::class,
+ 'code' => 'permissiondenied'
+ ];
+
+ $this->doTestQueryExceptions(
+ $this->getCreateItemAndSetSiteLinkRequestParams(),
+ $expected,
+ $userWithInsufficientPermissions
+ );
+ }
+
/**
* @param User $user
* @return Item
@@ -633,7 +672,16 @@
'action' => 'wbsetsitelink',
'id' => $id->getSerialization(),
'linksite' => 'enwiki',
- 'linktitle' => 'Come Cool Page',
+ 'linktitle' => 'Some Cool Page',
+ ];
+ }
+
+ private function getCreateItemAndSetSiteLinkRequestParams() {
+ return [
+ 'action' => 'wbsetsitelink',
+ 'new' => 'item',
+ 'linksite' => 'enwiki',
+ 'linktitle' => 'Another Cool Page',
];
}
diff --git
a/repo/tests/phpunit/includes/Store/WikiPageEntityStorePermissionCheckerTest.php
b/repo/tests/phpunit/includes/Store/WikiPageEntityStorePermissionCheckerTest.php
index c8cfbc3..a8d3551 100644
---
a/repo/tests/phpunit/includes/Store/WikiPageEntityStorePermissionCheckerTest.php
+++
b/repo/tests/phpunit/includes/Store/WikiPageEntityStorePermissionCheckerTest.php
@@ -519,6 +519,136 @@
];
}
+ /**
+ * @dataProvider
provideNonExistingEntitiesAndPermissionsThatAllowEditingTerms
+ */
+ public function
testAllRequiredPermissionsAreNeededToEditTermsOfNonExistingEntity(
+ EntityDocument $nonExistingEntity,
+ array $groupPermissions
+ ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertUserIsAllowedTo(
EntityPermissionChecker::ACTION_EDIT_TERMS, $nonExistingEntity );
+ }
+
+ public function
provideNonExistingEntitiesAndPermissionsThatAllowEditingTerms() {
+ 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
provideNonExistingEntitiesAndPermissionsThatDisallowEditingTerms
+ */
+ public function
testAllRequiredPermissionsAreNeededToEditTermsOfNonExistingEntity_failures(
+ EntityDocument $nonExistingEntity,
+ array $groupPermissions
+ ) {
+ $this->anyUserHasPermissions( $groupPermissions );
+
+ $this->assertItIsForbiddenForUserTo(
EntityPermissionChecker::ACTION_EDIT_TERMS, $nonExistingEntity );
+ }
+
+ public function
provideNonExistingEntitiesAndPermissionsThatDisallowEditingTerms() {
+ return [
+ 'non-existing item, no createpage permission' => [
+ $this->getNonExistingItem(),
+ [ 'createpage' => false ]
+ ],
+ 'non-existing item, no item-term permission' => [
+ $this->getNonExistingItem(),
+ [ 'item-term' => false, 'createpage' => true ]
+ ],
+ 'non-existing item, no edit permission' => [
+ $this->getNonExistingItem(),
+ [ 'edit' => false, 'item-term' => true,
'createpage' => true ]
+ ],
+ 'non-existing item, no read permission' => [
+ $this->getNonExistingItem(),
+ [ 'read' => false, 'edit' => true, 'item-term'
=> true, 'createpage' => true ]
+ ],
+ 'non-existing item (null ID), no createpage permission'
=> [
+ $this->getNonExistingItemWithNullId(),
+ [ 'createpage' => false ]
+ ],
+ 'non-existing item (null ID), no item-term permission'
=> [
+ $this->getNonExistingItemWithNullId(),
+ [ 'item-term' => false, 'createpage' => true ]
+ ],
+ 'non-existing item (null ID), no edit permission' => [
+ $this->getNonExistingItemWithNullId(),
+ [ 'edit' => false, 'item-term' => true,
'createpage' => true ]
+ ],
+ 'non-existing item (null ID), no read permission' => [
+ $this->getNonExistingItemWithNullId(),
+ [ 'read' => false, 'edit' => true, 'item-term'
=> 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 property-term permission' =>
[
+ $this->getNonExistingProperty(),
+ [ 'property-term' => false, 'createpage' =>
true, 'property-create' => true, ]
+ ],
+ 'non-existing property, no edit permission' => [
+ $this->getNonExistingProperty(),
+ [ 'edit' => false, 'property-term' => true,
'createpage' => true, 'property-create' => true, ]
+ ],
+ 'non-existing property, no read permission' => [
+ $this->getNonExistingProperty(),
+ [ 'read' => false, 'edit' => true,
'property-term' => 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 property-term
permission' => [
+ $this->getNonExistingPropertyWithNullId(),
+ [ 'property-term' => false, 'createpage' =>
true, 'property-create' => true, ]
+ ],
+ 'non-existing property (null ID), no edit permission'
=> [
+ $this->getNonExistingPropertyWithNullId(),
+ [ 'edit' => false, 'property-term' => true,
'createpage' => true, 'property-create' => true, ]
+ ],
+ 'non-existing property (null ID), no read permission'
=> [
+ $this->getNonExistingPropertyWithNullId(),
+ [ 'read' => false, 'edit' => true,
'property-term' => true, 'createpage' => true, 'property-create' => true, ]
+ ],
+ ];
+ }
+
public function
testGivenUnknownPermission_getPermissionStatusForEntityThrowsException() {
$checker = $this->getPermissionChecker();
--
To view, visit https://gerrit.wikimedia.org/r/365582
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6148d0a86197f1ad75bad09937be2fddc4e419e4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: WMDE-leszek <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
