Matthias Mullie has uploaded a new change for review.
https://gerrit.wikimedia.org/r/99416
Change subject: Make header use *ActionPermissions
......................................................................
Make header use *ActionPermissions
Renamed PostActionPermissions to more general RevisionActionPermissions & made
it accept any AbstractRevision object (that's where the moderation lives anyway)
Changed header code to use this class.
Made the class accept null for revision (like for checking permissions to create
a post/header)
Updated a couple of permissions: create & edit should check for core 'edit' perm
Change-Id: Ic173c47f90b6ac1c7b7dd80befef34bdced23d66
---
M Flow.php
M FlowActions.php
M includes/Block/Header.php
M includes/Block/Topic.php
M includes/Block/TopicList.php
R includes/RevisionActionPermissions.php
M includes/Templating.php
M includes/View/PostActionMenu.php
M templates/topiclist.html.php
9 files changed, 70 insertions(+), 51 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/16/99416/1
diff --git a/Flow.php b/Flow.php
index 69fa191..539eb9a 100755
--- a/Flow.php
+++ b/Flow.php
@@ -70,7 +70,7 @@
$wgAutoloadClasses['Flow\NotificationFormatter'] = $dir .
'includes/Notifications/Formatter.php';
$wgAutoloadClasses['Flow\NotificationController'] = $dir .
'includes/Notifications/Controller.php';
$wgAutoloadClasses['Flow\FlowActions'] = $dir . 'includes/FlowActions.php';
-$wgAutoloadClasses['Flow\PostActionPermissions'] = $dir .
'includes/PostActionPermissions.php';
+$wgAutoloadClasses['Flow\RevisionActionPermissions'] = $dir .
'includes/RevisionActionPermissions.php';
// Classes that model our data
$wgAutoloadClasses['Flow\Model\Definition'] = $dir .
'includes/Model/Definition.php';
diff --git a/FlowActions.php b/FlowActions.php
index 30413d3..909b3f3 100644
--- a/FlowActions.php
+++ b/FlowActions.php
@@ -2,7 +2,7 @@
use Flow\Model\PostRevision;
use Flow\Model\Header;
-use Flow\PostActionPermissions;
+use Flow\RevisionActionPermissions;
use Flow\Log\Logger;
use Flow\Block\Block;
use Flow\Templating;
@@ -31,13 +31,9 @@
'create-header' => array(
'performs-writes' => true,
'log_type' => false,
- /*
- * null doesn't actually have any meaning here; we just have
not (yet?)
- * implemented any permissions for headers (anyone can edit
them),
- * only for posts (PostActionPermissions). This 1 key can, for
now,
- * safely be ignored here.
- */
- 'permissions' => null,
+ 'permissions' => array(
+ Header::MODERATED_NONE => 'edit',
+ ),
'button-method' => 'POST',
'history' => array(
'i18n-message' => 'flow-rev-message-create-header',
@@ -56,13 +52,9 @@
'edit-header' => array(
'performs-writes' => true,
'log_type' => false,
- /*
- * null doesn't actually have any meaning here; we just have
not (yet?)
- * implemented any permissions for headers (anyone can edit
them),
- * only for posts (PostActionPermissions). This 1 key can, for
now,
- * safely be ignored here.
- */
- 'permissions' => null,
+ 'permissions' => array(
+ Header::MODERATED_NONE => 'edit',
+ ),
'button-method' => 'POST',
'history' => array(
'i18n-message' => 'flow-rev-message-edit-header',
@@ -82,7 +74,7 @@
'performs-writes' => true,
'log_type' => false,
'permissions' => array(
- PostRevision::MODERATED_NONE => '',
+ PostRevision::MODERATED_NONE => 'edit',
),
'button-method' => 'GET',
'history' => array(
@@ -121,7 +113,9 @@
'new-post' => array(
'performs-writes' => true,
'log_type' => false,
- 'permissions' => null,
+ 'permissions' => array(
+ PostRevision::MODERATED_NONE => 'edit',
+ ),
'button-method' => 'POST',
'history' => array(
'i18n-message' => 'flow-rev-message-new-post',
@@ -150,8 +144,8 @@
'log_type' => false,
'permissions' => array(
// no permissions needed for own posts
- PostRevision::MODERATED_NONE => function( PostRevision
$post, PostActionPermissions $permissions ) {
- return $post->isCreator(
$permissions->getUser() ) ? '' : 'flow-edit-post';
+ PostRevision::MODERATED_NONE => function( PostRevision
$post, RevisionActionPermissions $permissions ) {
+ return $post->isCreator(
$permissions->getUser() ) ? 'edit' : 'flow-edit-post';
}
),
'button-method' => 'GET',
@@ -490,7 +484,7 @@
'performs-writes' => true,
'log_type' => false,
'permissions' => array(
- PostRevision::MODERATED_NONE => '',
+ PostRevision::MODERATED_NONE => 'edit',
),
'button-method' => 'GET',
'history' => array(
diff --git a/includes/Block/Header.php b/includes/Block/Header.php
index faf7824..ed184d0 100644
--- a/includes/Block/Header.php
+++ b/includes/Block/Header.php
@@ -2,6 +2,8 @@
namespace Flow\Block;
+use Flow\Container;
+use Flow\RevisionActionPermissions;
use Flow\View\History\History;
use Flow\View\History\HistoryRenderer;
use Flow\DbFactory;
@@ -17,6 +19,11 @@
protected $header;
protected $needCreate = false;
protected $supportedActions = array( 'edit-header' );
+
+ /**
+ * @var RevisionActionPermissions $permissions Allows or denies actions
to be performed
+ */
+ protected $permissions;
public function init( $action, $user ) {
parent::init( $action, $user );
@@ -35,6 +42,8 @@
if ( $found ) {
$this->header = reset( $found );
}
+
+ $this->permissions = new RevisionActionPermissions(
Container::get( 'flow_actions' ), $user );
}
protected function validate() {
@@ -43,6 +52,11 @@
}
if ( $this->header ) {
+ if ( !$this->permissions->isAllowed( $this->header,
'edit-header' ) ) {
+ $this->errors['permissions'] = wfMessage(
'flow-error-not-allowed' );
+ return;
+ }
+
if ( empty( $this->submitted['prev_revision'] ) ) {
$this->errors['prev_revision'] = wfMessage(
'flow-error-missing-prev-revision-identifier' );
} elseif ( $this->header->getRevisionId()->getHex() !==
$this->submitted['prev_revision'] ) {
@@ -56,9 +70,14 @@
// this isnt really part of validate, but we want the
error-rendering template to see the users edited header
$this->header = $this->header->newNextRevision(
$this->user, $this->submitted['content'], 'edit-header' );
} else {
+ if ( !$this->permissions->isAllowed( null,
'create-header' ) ) {
+ $this->errors['permissions'] = wfMessage(
'flow-error-not-allowed' );
+ return;
+ }
+
if ( empty( $this->submitted['prev_revision'] ) ) {
// this isnt really part of validate either,
should validate be renamed or should this logic be redone?
- $this->header = Header::create(
$this->workflow, $this->user, $this->submitted['content'] );
+ $this->header = Header::create(
$this->workflow, $this->user, $this->submitted['content'], 'create-header' );
} else {
// User submitted a previous revision, but we
couldn't find one. This is likely
// an internal error and not a user error,
consider better handling
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index ab3c18e..9ed2e3b 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -12,7 +12,7 @@
use Flow\Model\AbstractRevision;
use Flow\Model\PostRevision;
use Flow\NotificationController;
-use Flow\PostActionPermissions;
+use Flow\RevisionActionPermissions;
use Flow\Templating;
use Flow\Container;
use EchoEvent;
@@ -41,7 +41,7 @@
);
/**
- * @var PostActionPermissions $permissions Allows or denies actions to
be performed
+ * @var RevisionActionPermissions $permissions Allows or denies actions
to be performed
*/
protected $permissions;
@@ -60,9 +60,7 @@
public function init( $action, $user ) {
parent::init( $action, $user );
-
- // @todo: I don't like pulling stuff from container in here,
improve this some day
- $this->permissions = new PostActionPermissions( Container::get(
'flow_actions' ), $user );
+ $this->permissions = new RevisionActionPermissions(
Container::get( 'flow_actions' ), $user );
}
protected function validate() {
diff --git a/includes/Block/TopicList.php b/includes/Block/TopicList.php
index 885fb5e..f99a8c4 100644
--- a/includes/Block/TopicList.php
+++ b/includes/Block/TopicList.php
@@ -13,7 +13,7 @@
use Flow\Model\UUID;
use Flow\Model\Workflow;
use Flow\NotificationController;
-use Flow\PostActionPermissions;
+use Flow\RevisionActionPermissions;
use Flow\Templating;
use User;
@@ -35,14 +35,16 @@
public function init( $action, $user ) {
parent::init( $action, $user );
- $this->permissions = new PostActionPermissions( Container::get(
'flow_actions' ), $user );
+ $this->permissions = new RevisionActionPermissions(
Container::get( 'flow_actions' ), $user );
}
protected function validate() {
- // @todo some sort of restriction along the lines of article
protection
- if ( !$this->user->isAllowed( 'edit' ) ) {
+ // for now, new topic is considered a new post; perhaps some
day topic creation should get it's own permissions?
+ if ( !$this->permissions->isAllowed( null, 'new-post' ) ) {
$this->errors['permissions'] = wfMessage(
'flow-error-not-allowed' );
+ return;
}
+
if ( !isset( $this->submitted['topic'] ) || !is_string(
$this->submitted['topic'] ) || strlen( $this->submitted['topic'] === 0 ) ) {
$this->errors['topic'] = wfMessage(
'flow-error-missing-title' );
} elseif ( strlen( $this->submitted['topic'] ) >
PostRevision::MAX_TOPIC_LENGTH ) {
diff --git a/includes/PostActionPermissions.php
b/includes/RevisionActionPermissions.php
similarity index 66%
rename from includes/PostActionPermissions.php
rename to includes/RevisionActionPermissions.php
index d3d3e16..70208ce 100644
--- a/includes/PostActionPermissions.php
+++ b/includes/RevisionActionPermissions.php
@@ -2,14 +2,14 @@
namespace Flow;
-use Flow\Model\PostRevision;
+use Flow\Model\AbstractRevision;
use Closure;
use User;
/**
- * role based security for posts based on moderation state
+ * Role based security for revisions based on moderation state
*/
-class PostActionPermissions {
+class RevisionActionPermissions {
/**
* @var FlowActions
*/
@@ -32,13 +32,13 @@
/**
* Get the name of all the actions the user is allowed to perform.
*
- * @param PostRevision $post The post to check permissions against
+ * @param AbstractRevision[optional] $revision The revision to check
permissions against
* @return array Array of action names that are allowed
*/
- public function getAllowedActions( PostRevision $post ) {
+ public function getAllowedActions( AbstractRevision $revision = null ) {
$allowed = array();
foreach( array_keys( $this->actions->getActions() ) as $action
) {
- if ( $this->isAllowedAny( $post, $action ) ) {
+ if ( $this->isAllowedAny( $revision, $action ) ) {
$allowed[] = $action;
}
}
@@ -48,17 +48,23 @@
/**
* Check if a user is allowed to perform a certain action.
*
- * @param PostRevision $post
+ * @param AbstractRevision[optional] $revision
* @param string $action
* @return bool
*/
- public function isAllowed( PostRevision $post, $action ) {
+ public function isAllowed( AbstractRevision $revision = null, $action )
{
// Users must have the core 'edit' permission to perform any
write action in flow
$performsWrites = $this->actions->getValue( $action,
'performs-writes' );
if ( $performsWrites && !$this->user->isAllowed( 'edit' ) ) {
return false;
}
- $permission = $this->actions->getValue( $action, 'permissions',
$post->getModerationState() );
+
+ // $revision may be null if the revision has yet to be created
+ $moderationState = AbstractRevision::MODERATED_NONE;
+ if ( $revision instanceof AbstractRevision ) {
+ $moderationState = $revision->getModerationState();
+ }
+ $permission = $this->actions->getValue( $action, 'permissions',
$moderationState );
// If no permission is defined for this state, then the action
is not allowed
// check if permission is set for this action
@@ -68,9 +74,9 @@
// Some permissions may be more complex to be defined as simple
array
// values, in which case they're a Closure (which will accept
- // PostRevision & PostActionPermissions as arguments)
+ // AbstractRevision & FlowActionPermissions as arguments)
if ( $permission instanceof Closure ) {
- $permission = $permission( $post, $this );
+ $permission = $permission( $revision, $this );
}
// check if user is allowed to perform action
@@ -83,19 +89,19 @@
/**
* Check if a user is allowed to perform certain actions.
*
- * @param PostRevision $post
+ * @param AbstractRevision[optional] $revision
* @param string $action
* @param string[optional] $action2 Overloadable to check if either of
the provided actions are allowed
* @return bool
*/
- public function isAllowedAny( PostRevision $post, $action /* [,
$action2 [, ... ]] */ ) {
+ public function isAllowedAny( AbstractRevision $revision = null,
$action /* [, $action2 [, ... ]] */ ) {
$actions = func_get_args();
- // Pull $post out of the actions list
+ // Pull $revision out of the actions list
array_shift( $actions );
$allowed = false;
foreach ( $actions as $action ) {
- $allowed |= $this->isAllowed( $post, $action );
+ $allowed |= $this->isAllowed( $revision, $action );
// as soon as we've found one that is allowed, break
if ( $allowed ) {
diff --git a/includes/Templating.php b/includes/Templating.php
index 91ec312..2098a03 100644
--- a/includes/Templating.php
+++ b/includes/Templating.php
@@ -160,7 +160,7 @@
return new PostActionMenu(
$this->urlGenerator,
$container['flow_actions'],
- new PostActionPermissions( $container['flow_actions'],
$this->globals['user'] ),
+ new RevisionActionPermissions(
$container['flow_actions'], $this->globals['user'] ),
$block,
$post,
$this->globals['editToken']
diff --git a/includes/View/PostActionMenu.php b/includes/View/PostActionMenu.php
index 1408220..b9f29b7 100644
--- a/includes/View/PostActionMenu.php
+++ b/includes/View/PostActionMenu.php
@@ -5,7 +5,7 @@
use Flow\Block\Block;
use Flow\FlowActions;
use Flow\Model\PostRevision;
-use Flow\PostActionPermissions;
+use Flow\RevisionActionPermissions;
use Flow\UrlGenerator;
use Html;
@@ -21,12 +21,12 @@
/**
* @param UrlGenerator $urlGenerator
* @param FlowActions $actions
- * @param PostActionPermissions $permissions
+ * @param RevisionActionPermissions $permissions
* @param Block $block
* @param PostRevision $post
* @param string $editToken
*/
- public function __construct( UrlGenerator $urlGenerator, FlowActions
$actions, PostActionPermissions $permissions, Block $block, PostRevision $post,
$editToken ) {
+ public function __construct( UrlGenerator $urlGenerator, FlowActions
$actions, RevisionActionPermissions $permissions, Block $block, PostRevision
$post, $editToken ) {
$this->urlGenerator = $urlGenerator;
$this->actions = $actions;
$this->permissions = $permissions;
diff --git a/templates/topiclist.html.php b/templates/topiclist.html.php
index 6f918a7..1f247b6 100644
--- a/templates/topiclist.html.php
+++ b/templates/topiclist.html.php
@@ -1,6 +1,6 @@
<?php
-// @todo consolidate into FlowActions, currently isAllowed method only exists
in PostActionPermissions
+// @todo consolidate into FlowActions, currently isAllowed method only exists
in RevisionActionPermissions
if ( $user->isAllowed( 'edit' ) ) {
echo Html::openElement( 'div', array( 'class' =>
'flow-new-topic-container flow-element-container' ) );
echo Html::openElement( 'form', array(
--
To view, visit https://gerrit.wikimedia.org/r/99416
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic173c47f90b6ac1c7b7dd80befef34bdced23d66
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
