EBernhardson has uploaded a new change for review.
https://gerrit.wikimedia.org/r/150383
Change subject: Cleanup failure case for moderated topic view
......................................................................
Cleanup failure case for moderated topic view
TopicBlock::loadTopicTitle was only returning the revision instance
when the user had 'view' accessto the revision, which is reasonable
but not everwhere was checking this return value appropriatly.
Additionally there are cases when we only need the lower level of 'history'
access and not the full-content 'view' access. To accomidate this
A new parameter $action was added to TopicBlock::loadTopicTitle
to specify the level of access needed as well as reviewing and cleaning up
return value usage for all places that use the method.
Bug: 67595
Change-Id: I12f83df3a1f3d49fd5abd9bc9de8d0a4ae4ed29c
---
M includes/Block/Topic.php
1 file changed, 31 insertions(+), 19 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/83/150383/1
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index b8422a7..d7e187b 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -113,16 +113,19 @@
protected function validate() {
// If the topic is closed, the only allowed action is to reopen
it
$topicTitle = $this->loadTopicTitle();
- if ( $topicTitle ) {
- if (
- $topicTitle->isClosed()
- && (
- $this->action !== 'close-open-topic'
- || $this->submitted['moderationState']
!== 'reopen'
- )
- ) {
- $this->addError( 'moderate', wfMessage(
'flow-error-topic-is-closed' ) );
- }
+ if ( !$topicTitle ) {
+ // permissions issue, self::loadTopicTitle should have
added appropriate
+ // error messages already.
+ return;
+ }
+ if (
+ $topicTitle->isClosed()
+ && (
+ $this->action !== 'close-open-topic'
+ || $this->submitted['moderationState'] !==
'restore'
+ )
+ ) {
+ $this->addError( 'moderate', wfMessage(
'flow-error-topic-is-closed' ) );
}
switch( $this->action ) {
@@ -182,7 +185,7 @@
}
$topicTitle = $this->loadTopicTitle();
if ( !$topicTitle ) {
- throw new InvalidInputException( 'No revision
associated with workflow?', 'missing-revision' );
+ return;
}
if ( !$this->permissions->isAllowed( $topicTitle, 'edit-title'
) ) {
$this->addError( 'permissions', wfMessage(
'flow-error-not-allowed' ) );
@@ -434,6 +437,11 @@
}
public function renderAPI( Templating $templating, array $options ) {
+ $topic = $this->loadTopicTitle( $this->action === 'history' ?
'history' : 'view' );
+ if ( !$topic ) {
+ throw new PermissionException( 'Not Allowed',
'insufficient-permission' );
+ }
+
// there's probably some OO way to turn this stack of if/else
into
// something nicer. Consider better ways before extending this
with
// more conditionals
@@ -476,7 +484,6 @@
}
$output['type'] = $this->getName();
- $topic = $this->loadTopicTitle();
$output['topicTitle'] = $templating->getContent( $topic,
'wikitext' );
if ( $this->wasSubmitted() ) {
@@ -655,11 +662,15 @@
return null;
}
- // Loads only the title, as opposed to loadRootPost which gets the
entire tree of posts.
- public function loadTopicTitle() {
+ /**
+ * @param string $action Permissions action to require to return
revision
+ * @return AbstratRevision|null
+ */
+ public function loadTopicTitle( $action = 'view' ) {
if ( $this->workflow->isNew() ) {
throw new InvalidDataException( 'New workflows do not
have any related content', 'missing-topic-title' );
}
+
if ( $this->topicTitle === null ) {
$found = $this->storage->find(
'PostRevision',
@@ -676,12 +687,13 @@
$this->topicTitle->setChildren( array() );
$this->topicTitle->setDepth( 0 );
$this->topicTitle->setRootPost( $this->topicTitle );
-
- if ( !$this->permissions->isAllowed( $this->topicTitle,
'view' ) ) {
- $this->topicTitle = null;
- $this->addError( 'permissions', wfMessage(
'flow-error-not-allowed' ) );
- }
}
+
+ if ( !$this->permissions->isAllowed( $this->topicTitle, $action
) ) {
+ $this->addError( 'permissions', wfMessage(
'flow-error-not-allowed' ) );
+ return null;
+ }
+
return $this->topicTitle;
}
--
To view, visit https://gerrit.wikimedia.org/r/150383
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I12f83df3a1f3d49fd5abd9bc9de8d0a4ae4ed29c
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <ebernhard...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
