jenkins-bot has submitted this change and it was merged.
Change subject: Use FlowAction permissions in AbstractRevision
......................................................................
Use FlowAction permissions in AbstractRevision
... instead of the hard-coded permissions, which make it impossible to have
different permissions per action (e.g. to hide posts, users must have flow-hide
permission, but they don't need that permission to see hidden posts)
FlowActions already check permissions against the current moderation status, so
mostRestrictivePermission is useless now.
Also stop using AbstractRevision::isAllowed outside of AbstractRevision objects.
Meanwhile also changed permissions for hidden posts; logged-in users can now see
them (as requested in mingle 421)
Mingle: 421
Change-Id: Iaca064314cca91b66ab9064e5c7ecaff73fda508
---
M FlowActions.php
M includes/Model/AbstractRevision.php
M includes/Model/PostRevision.php
M includes/Templating.php
M templates/post.html.php
5 files changed, 51 insertions(+), 47 deletions(-)
Approvals:
EBernhardson: Looks good to me, approved
jenkins-bot: Verified
diff --git a/FlowActions.php b/FlowActions.php
index 35114aa..b8d5501 100644
--- a/FlowActions.php
+++ b/FlowActions.php
@@ -190,7 +190,8 @@
},
function ( PostRevision $revision, Templating
$templating, User $user, Block $block ) {
$fragment = '';
- if ( $revision->isAllowed( $user,
PostRevision::MODERATED_HIDDEN ) ) {
+ $permissions =
$templating->getActionPermissions( $user );
+ if ( $permissions->isAllowed(
$revision, 'view' ) ) {
$fragment = 'flow-post-' .
$revision->getPostId()->getHex();
}
return
$templating->getUrlGenerator()->generateUrl( $block->getWorkflowId(), 'view',
array(), $fragment );
@@ -255,7 +256,8 @@
},
function ( PostRevision $revision, Templating
$templating, User $user, Block $block ) {
$fragment = '';
- if ( $revision->isAllowed( $user,
PostRevision::MODERATED_DELETED ) ) {
+ $permissions =
$templating->getActionPermissions( $user );
+ if ( $permissions->isAllowed(
$revision, 'view' ) ) {
$fragment = 'flow-post-' .
$revision->getPostId()->getHex();
}
return
$templating->getUrlGenerator()->generateUrl( $block->getWorkflowId(), 'view',
array(), $fragment );
@@ -322,7 +324,8 @@
},
function ( PostRevision $revision, Templating
$templating, User $user, Block $block ) {
$fragment = '';
- if ( $revision->isAllowed( $user,
PostRevision::MODERATED_SUPPRESSED ) ) {
+ $permissions =
$templating->getActionPermissions( $user );
+ if ( $permissions->isAllowed(
$revision, 'view' ) ) {
$fragment = 'flow-post-' .
$revision->getPostId()->getHex();
}
return
$templating->getUrlGenerator()->generateUrl( $block->getWorkflowId(), 'view',
array(), $fragment );
@@ -472,7 +475,10 @@
'log_type' => false, // don't log views
'permissions' => array(
PostRevision::MODERATED_NONE => '',
- PostRevision::MODERATED_HIDDEN => array( 'flow-hide',
'flow-delete', 'flow-suppress' ),
+ PostRevision::MODERATED_HIDDEN => function(
PostRevision $post, RevisionActionPermissions $permissions ) {
+ // visible for logged in users (or
anyone with hide permission)
+ return
$permissions->getUser()->isLoggedIn() ? '' : 'flow-hide';
+ },
PostRevision::MODERATED_DELETED => array(
'flow-delete', 'flow-suppress' ),
PostRevision::MODERATED_SUPPRESSED => 'flow-suppress',
),
diff --git a/includes/Model/AbstractRevision.php
b/includes/Model/AbstractRevision.php
index 9e32b7b..db6a4ec 100644
--- a/includes/Model/AbstractRevision.php
+++ b/includes/Model/AbstractRevision.php
@@ -2,6 +2,8 @@
namespace Flow\Model;
+use Flow\Container;
+use Flow\RevisionActionPermissions;
use MWTimestamp;
use User;
use Flow\ParsoidUtils;
@@ -18,26 +20,18 @@
**/
static public $perms = array(
self::MODERATED_NONE => array(
- // The permission needed from User::isAllowed to see
and create new revisions
- 'perm' => null,
// Whether or not to apply transition to this
moderation state to historical revisions
'historical' => true,
),
self::MODERATED_HIDDEN => array(
- // The permission needed from User::isAllowed to see
and create new revisions
- 'perm' => 'flow-hide',
// Whether or not to apply transition to this
moderation state to historical revisions
'historical' => false,
),
self::MODERATED_DELETED => array(
- // The permission needed from User::isAllowed to see
and create new revisions
- 'perm' => 'flow-delete',
// Whether or not to apply transition to this
moderation state to historical revisions
'historical' => true,
),
self::MODERATED_SUPPRESSED => array(
- // The permission needed from User::isAllowed to see
and create new revisions
- 'perm' => 'flow-suppress',
// Whether or not to apply transition to this
moderation state to historical revisions
'historical' => true,
),
@@ -176,18 +170,6 @@
return $obj;
}
- protected function mostRestrictivePermission( $a, $b ) {
- $keys = array_keys( self::$perms );
- $aPos = array_search( $a, $keys );
- $bPos = array_search( $b, $keys );
- if ( $aPos === false || $bPos === false ) {
- wfWarn( __METHOD__ . ": Invalid permissions provided:
'$a' '$b'" );
- // err on the side of safety, most restrictive
- return end( $keys );
- }
- return $keys[max( $aPos, $bPos )];
- }
-
/**
* $historical revisions must be provided when
self::needsModerateHistorical
* returns true.
@@ -198,8 +180,8 @@
return null;
}
- $mostRestrictive = self::mostRestrictivePermission( $state,
$this->moderationState );
- if ( !$this->isAllowed( $user, $mostRestrictive ) ) {
+ // doublecheck if user has permissions for moderation action
+ if ( !$this->isAllowed( $user, $changeType ) ) {
return null;
}
if ( !$historical && $this->needsModerateHistorical( $state ) )
{
@@ -213,7 +195,7 @@
$timestamp = wfTimestampNow();
foreach ( $historical as $rev ) {
- if ( !$rev->isAllowed( $user ) ) {
+ if ( !$rev->isAllowed( $user, $changeType ) ) {
continue;
}
$rev->moderationState = $state;
@@ -255,23 +237,24 @@
}
/**
- * Is the user allowed to see this revision?
+ * Is the user allowed to perform a certain action on this revision?
*
- * @param User $user The user requesting access. When null assumes a
user with no permissions.
- * @param int $state One of the self::MODERATED_* constants. When null
the internal moderation state is used.
+ * Uses permissions defined in FlowActions.
+ *
+ * @param User[optional] $user The user requesting access. When null
assumes a user with no permissions.
+ * @param string $action Action to check if allowed.
* @return boolean True when the user is allowed to see the current
revision
*/
- public function isAllowed( $user = null, $state = null ) {
- // allowing a $state to be passed is a bit hackish
- if ( $state === null ) {
- $state = $this->moderationState;
- }
- if ( !isset( self::$perms[$state] ) ) {
- throw new \MWException( 'Unknown stored moderation
state' );
+ protected function isAllowed( $user = null, $action ) {
+ // if no user specified, assume anonymous user
+ if ( !$user instanceof User ) {
+ $user = new User;
}
- $perm = self::$perms[$state]['perm'];
- return $perm === null || ( $user && $user->isAllowed( $perm ) );
+ $actions = Container::get( 'flow_actions' );
+ $permissions = new RevisionActionPermissions( $actions, $user );
+
+ return $permissions->isAllowed( $this, $action );
}
public function hasHiddenContent() {
@@ -483,7 +466,7 @@
// oversighting works. Prefer to create an external security
class that is
// configurable per-wiki, pass revisions into it(or wrap them
in it for
// view objects?) to get possibly protected content.
- if ( $this->isAllowed( $user ) ) {
+ if ( $this->isAllowed( $user, 'view' ) ) {
return $this->lastEditUserText;
} else {
return '';
diff --git a/includes/Model/PostRevision.php b/includes/Model/PostRevision.php
index 697ba8c..ce0f1dc 100644
--- a/includes/Model/PostRevision.php
+++ b/includes/Model/PostRevision.php
@@ -136,7 +136,7 @@
* @return User|bool The username of the User who created this post.
*/
public function getCreator( $user = null ) {
- if ( $this->isAllowed( $user ) ) {
+ if ( $this->isAllowed( $user, 'view' ) ) {
if ( $this->getCreatorIdRaw() != 0 ) {
$user = User::newFromId(
$this->getCreatorIdRaw() );
} else {
diff --git a/includes/Templating.php b/includes/Templating.php
index 1e57d61..f8af6e4 100644
--- a/includes/Templating.php
+++ b/includes/Templating.php
@@ -160,11 +160,22 @@
return new PostActionMenu(
$this->urlGenerator,
$container['flow_actions'],
- new RevisionActionPermissions(
$container['flow_actions'], $this->globals['user'] ),
+ $this->getActionPermissions( $this->globals['user'] ),
$block,
$post,
$this->globals['editToken']
);
+ }
+
+ // An ideal world may pull this from the container, but for now this is
fine. This templating
+ // class has too many responsibilities to keep receiving all required
objects in the constructor.
+ public function getActionPermissions( User $user = null ) {
+ // if no user defined, assume anonymous user
+ if ( !$user instanceof User ) {
+ $user = new User;
+ }
+
+ return new RevisionActionPermissions( Container::get(
'flow_actions' ), $user );
}
public function getPagingLink( $block, $direction, $offset, $limit ) {
@@ -264,7 +275,8 @@
// Messages: flow-hide-usertext, flow-delete-usertext,
flow-suppress-usertext
$message = wfMessage( "flow-$state-usertext", $username );
- if ( !$revision->isAllowed( $permissionsUser ) &&
$message->exists() ) {
+ $permissions = $this->getActionPermissions( $permissionsUser );
+ if ( !$permissions->isAllowed( $revision, 'view' ) &&
$message->exists() ) {
return $message->text();
} else {
return $username;
@@ -289,7 +301,8 @@
// Messages: flow-hide-usertext, flow-delete-usertext,
flow-suppress-usertext
$message = wfMessage( "flow-$state-usertext", $username );
- if ( !$revision->isAllowed( $permissionsUser ) &&
$message->exists() ) {
+ $permissions = $this->getActionPermissions( $permissionsUser );
+ if ( !$permissions->isAllowed( $revision, 'view' ) &&
$message->exists() ) {
return $message->text();
} else {
return Linker::userLink( $userid, $username ) .
Linker::userToolLinks( $userid, $username );
@@ -317,7 +330,8 @@
// Messages: flow-hide-usertext, flow-delete-usertext,
flow-suppress-usertext
$message = wfMessage( "flow-$state-usertext", $username );
- if ( !$revision->isAllowed( $permissionsUser ) &&
$message->exists() ) {
+ $permissions = $this->getActionPermissions( $permissionsUser );
+ if ( !$permissions->isAllowed( $revision, 'view' ) &&
$message->exists() ) {
return $message->text();
} else {
return $username;
@@ -346,7 +360,8 @@
// Messages: flow-hide-content, flow-delete-content,
flow-suppress-content
$message = wfMessage( "flow-$state-content", $user );
- if ( !$revision->isAllowed( $permissionsUser ) ) {
+ $permissions = $this->getActionPermissions( $permissionsUser );
+ if ( !$permissions->isAllowed( $revision, 'view' ) ) {
if ( $message->exists() ) {
return $message->text();
} else {
diff --git a/templates/post.html.php b/templates/post.html.php
index 3a423d0..a78b451 100644
--- a/templates/post.html.php
+++ b/templates/post.html.php
@@ -66,7 +66,7 @@
<?php
if ( $post->isModerated() ):
$moderationState = $post->getModerationState();
- $allowed = $post->isAllowed( $user ) ? 'allowed' :
'disallowed';
+ $allowed = $postView->actions()->isAllowed( 'view' ) ?
'allowed' : 'disallowed';
echo Html::rawElement(
'p',
array( 'class' => "flow-post-moderated-message
flow-post-moderated-$moderationState flow-post-content-$allowed", ),
--
To view, visit https://gerrit.wikimedia.org/r/102154
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaca064314cca91b66ab9064e5c7ecaff73fda508
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
Gerrit-Reviewer: EBernhardson <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
