jenkins-bot has submitted this change and it was merged.
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(-)
Approvals:
Bsitu: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index b8422a7..402cbaa 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'] !==
'reopen'
+ )
+ ) {
+ $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 AbstractRevision|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: merged
Gerrit-Change-Id: I12f83df3a1f3d49fd5abd9bc9de8d0a4ae4ed29c
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <ebernhard...@wikimedia.org>
Gerrit-Reviewer: Bsitu <bs...@wikimedia.org>
Gerrit-Reviewer: EBernhardson <ebernhard...@wikimedia.org>
Gerrit-Reviewer: SG <shah...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
