jenkins-bot has submitted this change and it was merged. Change subject: Check permissions in RecentChanges & Contributions feeds ......................................................................
Check permissions in RecentChanges & Contributions feeds Log & CheckUser are less easy to implement right away, and there's an upcoming ticket that will likely make that implementation easier. https://trello.com/c/S10KfqBm/62-8-history-watchlist-rc-and-contribs-changes I've added some @todo's in the code, so we can pick Log & CheckUser back up later. I'll file a bug to keep track. Change-Id: I96cf15fce23fffad6e35a779ac2bd94b1208a680 --- M FlowActions.php M includes/Data/RecentChanges.php M includes/Formatter/AbstractFormatter.php M includes/Formatter/CheckUser.php M includes/Formatter/Contributions.php M includes/Formatter/RecentChanges.php M includes/Log/Formatter.php 7 files changed, 84 insertions(+), 97 deletions(-) Approvals: EBernhardson: Looks good to me, approved jenkins-bot: Verified diff --git a/FlowActions.php b/FlowActions.php index 0c5baa3..7cf026c 100644 --- a/FlowActions.php +++ b/FlowActions.php @@ -447,102 +447,6 @@ ), ), - 'post-history' => array( - 'performs-writes' => false, - 'log_type' => false, - 'permissions' => array( - PostRevision::MODERATED_NONE => function( AbstractRevision $revision, RevisionActionPermissions $permissions ) { - // @todo: well, this does screw things up a bit because we're - // also checking against the most recent revision - // if that one was a restore, we still want all previous non- - // suppress-related stuff to show up... - - - // if a revision was the result of a restore-action, we have - // to look at the previous revision what the original moderation - // status was; permissions for the restore-actions visibility - // is the same as the moderation (e.g. if user can't see - // suppress actions, he can't see restores from suppress - if ( $revision->getChangeType() == 'restore-post' ) { - $revisionable = $revision->getRevisionable(); - $previous = $revisionable->getPreviousRevision( $revision ); - - if ( $previous->getModerationState() === AbstractRevision::MODERATED_NONE ) { - return ''; - } - - return $permissions->getPermission( $previous, 'topic-history' ); - } - - return ''; - }, - PostRevision::MODERATED_HIDDEN => '', - PostRevision::MODERATED_DELETED => '', - PostRevision::MODERATED_SUPPRESSED => 'flow-suppress', - ), - 'button-method' => 'GET', - ), - - 'topic-history' => array( - 'performs-writes' => false, - 'log_type' => false, - 'permissions' => array( - PostRevision::MODERATED_NONE => function( AbstractRevision $revision, RevisionActionPermissions $permissions ) { - // if a revision was the result of a restore-action, we have - // to look at the previous revision what the original moderation - // status was; permissions for the restore-actions visibility - // is the same as the moderation (e.g. if user can't see - // suppress actions, he can't see restores from suppress - if ( $revision->getChangeType() == 'restore-post' ) { - $revisionable = $revision->getRevisionable(); - $previous = $revisionable->getPreviousRevision( $revision ); - - if ( $previous->getModerationState() === AbstractRevision::MODERATED_NONE ) { - return ''; - } - - return $permissions->getPermission( $previous, 'topic-history' ); - } - - return ''; - }, - PostRevision::MODERATED_HIDDEN => '', - PostRevision::MODERATED_DELETED => '', - PostRevision::MODERATED_SUPPRESSED => 'flow-suppress', - ), - 'button-method' => 'GET', - ), - - 'board-history' => array( - 'performs-writes' => false, - 'log_type' => false, - 'permissions' => array( - PostRevision::MODERATED_NONE => function( AbstractRevision $revision, RevisionActionPermissions $permissions ) { - // if a revision was the result of a restore-action, we have - // to look at the previous revision what the original moderation - // status was; permissions for the restore-actions visibility - // is the same as the moderation (e.g. if user can't see - // suppress actions, he can't see restores from suppress - if ( $revision->getChangeType() == 'restore-post' ) { - $revisionable = $revision->getRevisionable(); - $previous = $revisionable->getPreviousRevision( $revision ); - - if ( $previous->getModerationState() === AbstractRevision::MODERATED_NONE ) { - return ''; - } - - return $permissions->getPermission( $previous, 'topic-history' ); - } - - return ''; - }, - PostRevision::MODERATED_HIDDEN => '', - PostRevision::MODERATED_DELETED => '', - PostRevision::MODERATED_SUPPRESSED => 'flow-suppress', - ), - 'button-method' => 'GET', - ), - 'view' => array( 'performs-writes' => false, 'log_type' => false, // don't log views @@ -593,6 +497,47 @@ ), ), + 'post-history' => array( + 'performs-writes' => false, + 'log_type' => false, + 'permissions' => array( + PostRevision::MODERATED_NONE => function( AbstractRevision $revision, RevisionActionPermissions $permissions ) { + // if a revision was the result of a restore-action, we have + // to look at the previous revision what the original moderation + // status was; permissions for the restore-actions visibility + // is the same as the moderation (e.g. if user can't see + // suppress actions, he can't see restores from suppress + if ( $revision->getChangeType() == 'restore-post' ) { + $revisionable = $revision->getRevisionable(); + $previous = $revisionable->getPreviousRevision( $revision ); + + if ( $previous->getModerationState() === AbstractRevision::MODERATED_NONE ) { + return ''; + } + + return $permissions->getPermission( $previous, 'topic-history' ); + } + + return ''; + }, + PostRevision::MODERATED_HIDDEN => '', + PostRevision::MODERATED_DELETED => '', + PostRevision::MODERATED_SUPPRESSED => 'flow-suppress', + ), + 'button-method' => 'GET', + 'history' => array() // views don't generate history + ), + + // post/topic/board history have exact same config + 'topic-history' => 'post-history', + 'board-history' => 'post-history', + + // log & all other formatters have same config as history + 'log' => 'post-history', + 'recentchanges' => 'post-history', + 'contributions' => 'post-history', + 'checkuser' => 'post-history', + /* * Backwards compatibility; these are old values that may have made their * way into the database. patch-rev_change_type_update.sql should take care diff --git a/includes/Data/RecentChanges.php b/includes/Data/RecentChanges.php index 0abb594..497ea9d 100644 --- a/includes/Data/RecentChanges.php +++ b/includes/Data/RecentChanges.php @@ -59,6 +59,7 @@ if ( $action === 'suppress-topic' || $action === 'suppress-post' ) { // @todo: should be move this into FlowActions.php somehow? // Suppression log entries should not go to recentchanges (bug 60814) + // @todo: does this still make sense in here? Formatter properly checks permissions now return; } diff --git a/includes/Formatter/AbstractFormatter.php b/includes/Formatter/AbstractFormatter.php index 1b09955..7868c19 100644 --- a/includes/Formatter/AbstractFormatter.php +++ b/includes/Formatter/AbstractFormatter.php @@ -72,6 +72,11 @@ protected $revisions = array(); /** + * @var array Array of [user id => RevisionActionPermissions object] + */ + protected $permissions = array(); + + /** * @param ManagerGroup $storage * @param FlowActions $actions * @param Templating $templating @@ -86,6 +91,26 @@ } /** + * @param User $user + * @return RevisionActionPermissions + */ + protected function getPermissions( User $user ) { + if ( $user->getId() && isset( $this->permissions[$user->getId()] ) ) { + return $this->permissions[$user->getId()]; + } + + $permissions = new RevisionActionPermissions( $this->actions, $user ); + + // cache objects per user (will usually be only the person viewing + // whatever is using this formatter) + if ( $user->getId() ) { + $this->permissions[$user->getId()] = $permissions; + } + + return $permissions; + } + + /** * @param Title $title * @param string $action * @param UUID $workflowId diff --git a/includes/Formatter/CheckUser.php b/includes/Formatter/CheckUser.php index 580ddb8..b3d0edb 100644 --- a/includes/Formatter/CheckUser.php +++ b/includes/Formatter/CheckUser.php @@ -20,6 +20,10 @@ return null; } + // @todo: this currently only implements CU links + // we'll probably want to add a hook to CheckUser that lets us blank out + // the entire line for entries that !isRevisionAllowed( <this-revision>, 'checkuser' ) + $data = explode( ',', $row->cuc_comment ); $post = null; switch( count( $data ) ) { diff --git a/includes/Formatter/Contributions.php b/includes/Formatter/Contributions.php index 30bb97c..13f70e8 100644 --- a/includes/Formatter/Contributions.php +++ b/includes/Formatter/Contributions.php @@ -30,6 +30,10 @@ $user = $pager->getUser(); $title = $workflow->getArticleTitle(); + if ( !$this->getPermissions( $user )->isRevisionAllowed( $revision, 'contributions' ) ) { + return false; + } + // Fetch required data $charDiff = $this->getCharDiff( $revision, $row->previous_revision ); $description = $this->getActionDescription( $workflow, $row->blocktype, $revision ); diff --git a/includes/Formatter/RecentChanges.php b/includes/Formatter/RecentChanges.php index ca518d0..83cfe80 100644 --- a/includes/Formatter/RecentChanges.php +++ b/includes/Formatter/RecentChanges.php @@ -57,7 +57,9 @@ return false; } - if ( $this->hideRecord( $revision, $changeData ) ) { + if ( $this->hideRecord( $revision, $changeData ) + || !$this->getPermissions( $user )->isRevisionAllowed( $revision, 'recentchanges' ) + ) { return false; } diff --git a/includes/Log/Formatter.php b/includes/Log/Formatter.php index b941424..5de483c 100644 --- a/includes/Log/Formatter.php +++ b/includes/Log/Formatter.php @@ -20,6 +20,12 @@ $skin = $this->plaintext ? null : $this->context->getSkin(); $params = $this->entry->getParameters(); + // @todo: we should probably check if user isRevisionAllowed( <this-revision>, 'log' ) + // unlike RC, Contributions, ... this one does not batch-load all Flow + // revisions & does not use the same Formatter, i18n message text, etc + // I assume this will change with https://trello.com/c/S10KfqBm/62-8-history-watchlist-rc-and-contribs-changes + // Then, we should also add in the isRevisionAllowed check! + // FIXME this is ugly. Why were we treating log parameters as // URL GET parameters in the first place? if ( isset( $params['postId'] ) ) { -- To view, visit https://gerrit.wikimedia.org/r/112583 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I96cf15fce23fffad6e35a779ac2bd94b1208a680 Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/extensions/Flow Gerrit-Branch: master Gerrit-Owner: Matthias Mullie <mmul...@wikimedia.org> Gerrit-Reviewer: EBernhardson <ebernhard...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
