Page MenuHomePhabricator

FormSpecialPage::checkExecutePermissions should only block a FormSpecialPage if the user is sitewide blocked
Closed, ResolvedPublic3 Estimated Story Points

Description

Problem
If a special form page (like Special:Block) extends FormSpecialPage then users who are partially blocked are also blocked from that Special page.

if ( $this->requiresUnblock() && $user->isBlocked() ) {
  $block = $user->getBlock();
  throw new UserBlockedError( $block );
}

Solution
This should be changed to something like:

if ( $this->requiresUnblock() ) {
  $block = $user->getBlock();
  if ( $block && $block->isSitewide() ) {
    throw new UserBlockedError( $block );
  }
}

Affected Special Pages

Important to fix, release blocker for Commons:

Should fix eventually:

I have no idea what these are, and they're not on EN or IT Wikipedia. Maybe never fix:

Event Timeline

dbarratt updated the task description. (Show Details)

@dbarratt — will the fixes to all of these be the same, or should I break these into prioritized other Phab tasks?

AbuseLog was fixed but the others are not. Just verified on Test Wiki.

Change 492010 had a related patch set uploaded (by Dbarratt; owner: Dbarratt):
[mediawiki/core@master] Only block Special pages if the user is sitewide blocked.

https://gerrit.wikimedia.org/r/492010

@TBolliger The patch addresses all pages in your list except for AbuseLog and UploadWizard, since they extend SpecialPage, instead of FormSpecialPage. It sounds like AbuseLog is already working. It looks like UploadWizard should be fixed in SpecialUploadWizard::isUserUploadAllowed.

The patch also affects any special pages that extend the class FormSpecialPage and don't override requiresUnblock or checkExecutePermissions. From a cursory glance through the classes in this list on codesearch, that's quite a few special pages. Partially blocked users would now be able to access all such pages (assuming they have the right permissions). Is that OK?

One more thing - after the patch, in order to block someone from any of these special pages, the admin would have to apply a sitewide block now, since it's not possible to specify special pages in a partial block. Is that acceptable?

@TBolliger The patch addresses all pages in your list except for AbuseLog and UploadWizard, since they extend SpecialPage, instead of FormSpecialPage. It sounds like AbuseLog is already working. It looks like UploadWizard should be fixed in SpecialUploadWizard::isUserUploadAllowed.

Good deal — sounds like we need a new Phab task to address SpecialUploadWizard::isUserUploadAllowed ?

The patch also affects any special pages that extend the class FormSpecialPage and don't override requiresUnblock or checkExecutePermissions. From a cursory glance through the classes in this list on codesearch, that's quite a few special pages. Partially blocked users would now be able to access all such pages (assuming they have the right permissions). Is that OK?

Yes, perfect. Partial blocks should not prohibit the PB'd user from utilizing any Special: pages (except for Special:MovePage on pages defined within the PB.)

One more thing - after the patch, in order to block someone from any of these special pages, the admin would have to apply a sitewide block now, since it's not possible to specify special pages in a partial block. Is that acceptable?

Yes, 100% acceptable and as designed. Thank you for double checking!

Good deal — sounds like we need a new Phab task to address SpecialUploadWizard::isUserUploadAllowed ?

That would be fantastic.

Good deal — sounds like we need a new Phab task to address SpecialUploadWizard::isUserUploadAllowed ?

That would be fantastic.

On it!

Done: T217255: Partially blocked users should be allowed to use Special:UploadWizard

Change 492010 merged by jenkins-bot:
[mediawiki/core@master] Block Special pages only if the user is sitewide blocked

https://gerrit.wikimedia.org/r/492010

On beta, I cannot create an account from an IP that is blocked from creating accounts, even if my user has "ipblock-exempt".

This looks like the same bug as T189362 (which was raised on 10th March 2018). However, I could not reproduce the bug on test.

Beta: 1.33.0-alpha (278ac40) 23:37, 10 March 2019
Test: 1.33.0-wmf.20 (rMWf929e2a50696) 23:04, 6 March 2019

@dbarratt Might it be related to work the team has done recently?

@dom_walden What error message do you get?

More or less the same as in T189362. You are shown the account creation form, but get a message when you type something in the Username field. Same message shows, above the form, if you try to submit.

ac_message.png (851×390 px, 45 KB)

For this block:

block_ac.png (151×1 px, 16 KB)

@TBolliger Special:MassMessage does not seem to respect blocks (partial or sitewide). For example, if you are blocked from someone's user_talk page you can still send a message to it.

It would require someone who is blocked also having the massmessage right. Unlikely to be given to a user who is sitewide blocked, but could happen if they are only partially blocked.

Separate bug?

Also, are we concerned about Special:StructuredDiscussions considering it is a beta feature?

I ask because I see some unusual behaviour. A blocked user can post to a structured discussion, but not use the "thank" function (possibly because they are blocked from sending email; I have no idea). I am not sure if it is worth following up.

@TBolliger Special:MassMessage does not seem to respect blocks (partial or sitewide). For example, if you are blocked from someone's user_talk page you can still send a message to it.

It would require someone who is blocked also having the massmessage right. Unlikely to be given to a user who is sitewide blocked, but could happen if they are only partially blocked.

Separate bug?

Yes, this seems like an extremely rare edge case. We can file to just have it on the MediaWiki-User-management board, but I don't think it's worth AHT's time to address.

Also, are we concerned about Special:StructuredDiscussions considering it is a beta feature?

I ask because I see some unusual behaviour. A blocked user can post to a structured discussion, but not use the "thank" function (possibly because they are blocked from sending email; I have no idea). I am not sure if it is worth following up.

Let's wait on making any further changes to Structured Discussions, as the 2019 Talk Page Consultation may illuminate the future direction of that product.

Tested in this bug:

ActionBlockOutcome
Special:EmailUserUser partial (send)Can send
Special:EmailUserIP partial (send)Cannot send
Special:EmailUserCookie partial (send)Cannot send
Special:EmailUserUser partial (nosend)Blocked
Special:EmailUserIP partial (nosend)Cannot send
Special:EmailUserCookie partial (nosend)Cannot send
Special:PasswordResetUser partialSuccess
Special:PasswordResetIP partialSuccess (for User)
Special:PasswordResetCookie partialSuccess
Special:PasswordResetUser sitewideBlocked
Special:PasswordResetIP sitewideBlocked
Special:PasswordResetCookie sitewideBlocked
Special:UploadUser partial (blocked from File ns)Blocked (on submission)
Special:UploadUser partial (not blocked File ns)Success
Special:UploadUser sitewideBlocked on access
Special:AbuseLogUser partialCan view logs and search
Special:AbuseLogUser sitewideCan view logs and search
Special:BotPasswordsUser sitewideBlocked on access
Special:BotPasswordsUser partial (non-admin user)Can create bot p/w

Also in this bug (T209097#5021714)

  • Special:MassMessage
  • Special:StructuredDiscussions

Elsewhere (T211621) I tested:

  • Special:UserLogin
  • Special:PasswordReset
  • Special:ChangeCredentials

I will test Special:UploadWizard in T217255.

The only thing outstanding is T209097#5015139, if @dbarratt would like me to raise it as a separate bug.

The only thing outstanding is T209097#5015139, if @dbarratt would like me to raise it as a separate bug.

As this is already raised as T189362, I will just move this on.

As this is already raised as T189362, I will just move this on.

Thanks! sorry I hadn't had a chance to circle back to it. :)