Page MenuHomePhabricator

Remove separate checks for global blocks from APIUpload and SpecialUpload
Closed, ResolvedPublic

Assigned To
Authored By
Tchanders
Sep 8 2022, 3:00 PM
Referenced Files
F35755782: T317325_SB_Partial_Uncheck_Chrome2of2.png
Nov 11 2022, 7:35 PM
F35755780: T317325_SB_Partial_Uncheck_Chrome1of2.png
Nov 11 2022, 7:35 PM
F35755694: T317325_GB_IP_Chrome.png
Nov 11 2022, 7:35 PM
F35754996: image.png
Nov 11 2022, 4:39 PM
F35754995: image.png
Nov 11 2022, 4:39 PM
F35754964: image.png
Nov 11 2022, 4:39 PM
F35754950: image.png
Nov 11 2022, 4:39 PM
F35754945: image.png
Nov 11 2022, 4:39 PM

Description

Background

APIUpload and SpecialUpload check for global blocks separately from other blocks. If a global block is found, it blocks upload.

This is an example of MediaWiki core being aware of an extension - GlobalBlocking.

After T257701: Add global blocks into CompositeBlocks rather than treating them separately and T317324: GlobalBlock::appliesToRight should return true for 'upload' this will no longer be necessary, and these checks can be removed.

Acceptance criteria
  • APIUpload and SpecialUpload no longer check for global blocks separately, but 'upload' is still blocked by a global block
Testing steps

For local testing the following config are needed:

  • $wgEnableUploads = true;
  • $wgGroupPermissions['*']['upload'] = true;

Event Timeline

Change 841454 had a related patch set uploaded (by Cyndywikime; author: Cyndywikime):

[mediawiki/core@master] Remove separate checks for global blocks from APIUpload and SpecialUpload

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

Testing notes:

I think this can be tested on Beta, though you might want to be careful about what you upload? I tested locally.

I tested this by blocking an IP address via Special:GlobalBlock, ensuring that was the only block on that IP address. Then, as that IP address:

  • I visited Special:Upload, and got a block error as expected
  • I used Special:APISandbox to make a request to the upload API, and got a block error, as expected

I encountered no block error from an unblocked IP address.

Steps for using Special:APISandbox in case it helps (though the API doesn't need to be tested via the UI):

  • Go to Special:APISandbox
  • Choose upload from the action dropdown

image.png (454×1 px, 77 KB)

  • Click on action=upload in the sidebar

image.png (157×273 px, 5 KB)

  • Enter some name in the filename input

image.png (473×1 px, 85 KB)

  • Choose a file via the file input, below

image.png (897×1 px, 92 KB)

  • Click on "Make request" (top right)

image.png (583×1 px, 133 KB)

The response should be a user blocked error

Change 841454 merged by jenkins-bot:

[mediawiki/core@master] Remove separate checks for global blocks from APIUpload and SpecialUpload

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

@Tchanders I tested Global block, composite blocks, range ips, sitewide and partial blocks which all came up with the same block error as seen in the 1st screenshot below but with a different blockid.

I did come across a possible issue in Special:Block which I wanted to run it with you first. When I don't check the Apply block to logged-in users from this IP address in Special:Block, I am still allowed to upload on Special:APISandbox. If the default was checked, I'm sure if this wouldn't be too much of a problem. Since it is, if someone does not know to check that box, that user can still do an upload as seen in the 2nd and 3rd screenshots.

Global Block Also tested Composite Blocks, Range IPs, Sitewide, Partial

T317325_GB_IP_Chrome.png (936×2 px, 311 KB)

T317325_SB_Partial_Uncheck_Chrome1of2.png (1×2 px, 192 KB)

T317325_SB_Partial_Uncheck_Chrome2of2.png (1×2 px, 273 KB)

I did come across a possible issue in Special:Block which I wanted to run it with you first. When I don't check the Apply block to logged-in users from this IP address in Special:Block, I am still allowed to upload on Special:APISandbox. If the default was checked, I'm sure if this wouldn't be too much of a problem. Since it is, if someone does not know to check that box, that user can still do an upload as seen in the 2nd and 3rd screenshots.

Thanks for raising this @GMikesell-WMF. I had a quick search to check whether any issues had been filed about this, and I didn't find anything, so looks like it hasn't been a problem so far. In the past, where we have changed form field defaults, we have discussed with community members first (e.g. this task from the partial blocks project: T208510) - so I'm happy to leave this as is until we hear that it's a problem.

@Tchanders Ok sounds good! If it's not a problem with the community then I'm fine with leaving it as is for now. I will move this to Done. Thanks!