Hi all, I'm Alex Ezell and I'm the Engineering Manager for the Anti-Harassment Tools team at the WMF. I have some details about user blocking that I'd like to share.
*tl:dr;* On a wiki with the new Partial Blocks enabled (currently only testwiki), if the code is checking User::isBlocked() to determine edit rights, it should instead check User::isBlockedFrom( Title ). The code could also check isBlocked() && $block->isSitewide(). If it doesn’t, the code may block users that shouldn’t be blocked.
*More details:* Recently, the Anti-Harassment Tools team merged code to enable a new feature called Partial Blocks. This feature lets admins block users from editing particular pages instead of only being able to block users from the entire site. It is currently enabled on testwiki.
This means that there are now multiple types of blocks (and more to come in the future, ie namespace blocks). The specific new types are “partial” as opposed to “sitewide” and some non-editing types of blocks (send email, edit talk page, etc.) Previously, a developer could assume that if a user was “blocked” that meant the user couldn’t do much of anything because that was a “sitewide” block and the only kind of block. Now, there are more cases to be concerned about.
Specifically, we’ve seen some extensions using User::isBlocked() and then assuming that a user can’t edit the particular page that the extension might be concerned with. User::isBlockedFrom( Title ) with a Title object will be the more correct way to check because of the possibility that a user might not be blocked from that particular page. If the code isn’t concerned with editing, it would be appropriate to use User::isAllowed() which will determine blocked status by way of User::getRights().
There is also a new method Block::isSitewide() which can help a developer determine if the block is “sitewide” or some other type. This is useful if the code doesn’t care about anything but the “sitewide” block type.
We believe that keeping User::isBlocked() in its current state is the safer way to proceed because in cases where it’s being used incorrectly it would result in over-enforcing blocks rather than under-enforcing them. A user who is partially blocked might be treated like a sitewide block by an extension. That seems safer to us than potentially allowing a user more freedom than an admin intended with a partial block.
We found at least one extension using User::getRights() in a way that would over-enforce on a partially blocked user. We created a patch to change how User::getRights() works https://gerrit.wikimedia.org/r/c/mediawiki/core/+/471210. In addition to checking that a block exists, it will also ensure that the block is a sitewide block. This will spare the partially blocked user from being blocked in these cases.
In summary, all MediaWiki code (especially extension code) that is concerned with checking user blocks should be aware of the distinction between User::isBlocked() and User::isBlockedFrom( Title ) and use the appropriate method for the kind of blocking the code is concerned with. Additionally, using the helper method Block::isSitewide() is handy for certain usages.
Alex Ezell Engineering Manager, Anti-Harassment Tools Team (WMF)
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
Hi,
On 11/2/18 12:58 PM, Alex Ezell wrote:
*tl:dr;* On a wiki with the new Partial Blocks enabled (currently only testwiki), if the code is checking User::isBlocked() to determine edit rights, it should instead check User::isBlockedFrom( Title ). The code could also check isBlocked() && $block->isSitewide(). If it doesn’t, the code may block users that shouldn’t be blocked.
Based on a quick codesearch[1], this affects a lot of extensions, and MediaWiki core itself. It would be nice if bugs (or patches!) could be filed against code that needs updating.
Also, are there any good reasons to continue checking User::isBlocked()? If not, I think deprecating it would be a good idea.
[1] https://codesearch.wmflabs.org/search/?q=-%3EisBlocked%5C(&i=nope&fi... &repos=
- -- Legoktm
Also, are there any good reasons to continue checking User::isBlocked()? If not, I think deprecating it would be a good idea.
+1 for deprecating it. I think the name of that function is misleading to begin with 'cause even before "Partial Blocks" there were other actions besides edit that a block could prevent (like 'sendemail').
On Sun, Nov 4, 2018 at 9:39 PM Kunal Mehta legoktm@member.fsf.org wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
Hi,
On 11/2/18 12:58 PM, Alex Ezell wrote:
*tl:dr;* On a wiki with the new Partial Blocks enabled (currently only testwiki), if the code is checking User::isBlocked() to determine edit rights, it should instead check User::isBlockedFrom( Title ). The code could also check isBlocked() && $block->isSitewide(). If it doesn’t, the code may block users that shouldn’t be blocked.
Based on a quick codesearch[1], this affects a lot of extensions, and MediaWiki core itself. It would be nice if bugs (or patches!) could be filed against code that needs updating.
Also, are there any good reasons to continue checking User::isBlocked()? If not, I think deprecating it would be a good idea.
[1] https://codesearch.wmflabs.org/search/?q=-%3EisBlocked%5C(&i=nope&fi... &repos= https://codesearch.wmflabs.org/search/?q=-%3EisBlocked%5C(&i=nope&files=&repos=
- -- Legoktm
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCgAdFiEE+h6fmkHn9DUCyl1jUvyOe+23/KIFAlvfrU4ACgkQUvyOe+23 /KLWYg/+LR3h0UjQwwomCba9VrnwvMYApsP7xHnZUvukMqF/mjNvKFnR1Rs/pEWj 10IU0QMVxBB6cA47xLY9oYWoMTs6uO4qcLgJrHGxsHdaklOeaukqTZZWh4ubdhs1 KHJOoxNAfzZvD7f0IPsT1w5mlu68ehAMV5OfLH5QtqhViOBh8yCUSPn5dwpeY1k2 28Ped42jst4U1PUCVELSf5hQe7KUCvE0xr5mcnT0rq5rta7nw9nCXeIaIQLUSP/i ouh0ZE1vf3ScsqmM0AV2hqc2GOdtzwaMPPKXFTbP3UQjAXWYDOs7UdITLOGbQupj JAW3vEEZEu6xd0TcvuQ1o5S9szHVxqJUOelqaZMn/w+8xOBfAQ4wcskqaEcE1Y2f X0pneuAx473kJOfGVEv/RbAnf8Vc9hXkRoSK7OY6f0tJPYyMreWbc7H15gUVRA4X jzcAH6VYOxxUX4trznNArmnGIisjylweZlJvTSWUawTzMNCUEieD4hWKmey8sX1r YhQsoyuy4JkumXqY6Eu9XkVCHNQg+JZkRfGB5EudzoUAqlL5D1NhcHx+1EhaELQd qNNVtPMH79vDUHs18Hn/np9eX8cHIyfDGhI5yHt4m3bx111p3I/hOo57yaV9WbrP 3C/USolKKxNUPgoPPP/3KKa+skDt9wKvPIU5lvDd9F9TKbm8hcc= =q47C -----END PGP SIGNATURE-----
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Well, that function is also exposed externally, e. g. in the 'blocked' field of the OAuth identity (what’s returned by Special:OAuth/identify). What should happen to that in the future?
Am Mo., 5. Nov. 2018 um 15:41 Uhr schrieb Dayllan Maza <dmaza@wikimedia.org
:
Also, are there any good reasons to continue checking User::isBlocked()? If not, I think deprecating it would be a good idea.
+1 for deprecating it. I think the name of that function is misleading to begin with 'cause even before "Partial Blocks" there were other actions besides edit that a block could prevent (like 'sendemail').
On Sun, Nov 4, 2018 at 9:39 PM Kunal Mehta legoktm@member.fsf.org wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
Hi,
On 11/2/18 12:58 PM, Alex Ezell wrote:
*tl:dr;* On a wiki with the new Partial Blocks enabled (currently only testwiki), if the code is checking User::isBlocked() to determine edit rights, it should instead check User::isBlockedFrom( Title ). The code could also check isBlocked() && $block->isSitewide(). If it doesn’t, the code may block users that shouldn’t be blocked.
Based on a quick codesearch[1], this affects a lot of extensions, and MediaWiki core itself. It would be nice if bugs (or patches!) could be filed against code that needs updating.
Also, are there any good reasons to continue checking User::isBlocked()? If not, I think deprecating it would be a good idea.
[1] https://codesearch.wmflabs.org/search/?q=-%3EisBlocked%5C(&i=nope&fi... &repos= <
https://codesearch.wmflabs.org/search/?q=-%3EisBlocked%5C(&i=nope&fi...
- -- Legoktm
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCgAdFiEE+h6fmkHn9DUCyl1jUvyOe+23/KIFAlvfrU4ACgkQUvyOe+23 /KLWYg/+LR3h0UjQwwomCba9VrnwvMYApsP7xHnZUvukMqF/mjNvKFnR1Rs/pEWj 10IU0QMVxBB6cA47xLY9oYWoMTs6uO4qcLgJrHGxsHdaklOeaukqTZZWh4ubdhs1 KHJOoxNAfzZvD7f0IPsT1w5mlu68ehAMV5OfLH5QtqhViOBh8yCUSPn5dwpeY1k2 28Ped42jst4U1PUCVELSf5hQe7KUCvE0xr5mcnT0rq5rta7nw9nCXeIaIQLUSP/i ouh0ZE1vf3ScsqmM0AV2hqc2GOdtzwaMPPKXFTbP3UQjAXWYDOs7UdITLOGbQupj JAW3vEEZEu6xd0TcvuQ1o5S9szHVxqJUOelqaZMn/w+8xOBfAQ4wcskqaEcE1Y2f X0pneuAx473kJOfGVEv/RbAnf8Vc9hXkRoSK7OY6f0tJPYyMreWbc7H15gUVRA4X jzcAH6VYOxxUX4trznNArmnGIisjylweZlJvTSWUawTzMNCUEieD4hWKmey8sX1r YhQsoyuy4JkumXqY6Eu9XkVCHNQg+JZkRfGB5EudzoUAqlL5D1NhcHx+1EhaELQd qNNVtPMH79vDUHs18Hn/np9eX8cHIyfDGhI5yHt4m3bx111p3I/hOo57yaV9WbrP 3C/USolKKxNUPgoPPP/3KKa+skDt9wKvPIU5lvDd9F9TKbm8hcc= =q47C -----END PGP SIGNATURE-----
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Well, that function is also exposed externally, e. g. in the 'blocked' field of the OAuth identity (what’s returned by Special:OAuth/identify). What should happen to that in the future?
For the Userinfo API https://www.mediawiki.org/wiki/API:Userinfo we added https://phabricator.wikimedia.org/T197141 a partial field. A field like this could be added, or you could use the Block::isSitewide() method to only inform the OAuth client that the user is blocked if they are blocked from everything (errr... everything except their own user talk page... maybe).
Regardless if you expose a new flag or change the behavior of the existing flag, you would need to call User::getBlock() in order to determine if the block is sitewide or not. Example: $user->getBlock() && $user->getBlock->isSitewide()
We considered changing the behavior of User::isBlocked() to only return true if they are sitewide blocked, but as Alex pointed out, that would cause blocked users to not be blocked from things that perhaps they should be blocked from. So we decided to fall on the side of caution.
Since User::isBlocked() method doesn't really tell you anything useful (and as Dayllan pointed out, it hasn't for a very long time), I support deprecating the method. Most extensions should be calling User::isAllowed() or Title::userCan() instead.
On Mon, Nov 5, 2018 at 9:50 AM Lucas Werkmeister < lucas.werkmeister@wikimedia.de> wrote:
Well, that function is also exposed externally, e. g. in the 'blocked' field of the OAuth identity (what’s returned by Special:OAuth/identify). What should happen to that in the future?
Am Mo., 5. Nov. 2018 um 15:41 Uhr schrieb Dayllan Maza < dmaza@wikimedia.org
:
Also, are there any good reasons to continue checking User::isBlocked()? If not, I think deprecating it would be a good idea.
+1 for deprecating it. I think the name of that function is misleading to begin with 'cause even before "Partial Blocks" there were other actions besides edit that a
block
could prevent (like 'sendemail').
On Sun, Nov 4, 2018 at 9:39 PM Kunal Mehta legoktm@member.fsf.org
wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
Hi,
On 11/2/18 12:58 PM, Alex Ezell wrote:
*tl:dr;* On a wiki with the new Partial Blocks enabled (currently only testwiki), if the code is checking User::isBlocked() to determine edit rights, it should instead check User::isBlockedFrom( Title ). The code could also check isBlocked() && $block->isSitewide(). If it doesn’t, the code may block users that shouldn’t be blocked.
Based on a quick codesearch[1], this affects a lot of extensions, and MediaWiki core itself. It would be nice if bugs (or patches!) could be filed against code that needs updating.
Also, are there any good reasons to continue checking User::isBlocked()? If not, I think deprecating it would be a good idea.
[1]
https://codesearch.wmflabs.org/search/?q=-%3EisBlocked%5C(&i=nope&fi...
&repos= <
https://codesearch.wmflabs.org/search/?q=-%3EisBlocked%5C(&i=nope&fi...
- -- Legoktm
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCgAdFiEE+h6fmkHn9DUCyl1jUvyOe+23/KIFAlvfrU4ACgkQUvyOe+23 /KLWYg/+LR3h0UjQwwomCba9VrnwvMYApsP7xHnZUvukMqF/mjNvKFnR1Rs/pEWj 10IU0QMVxBB6cA47xLY9oYWoMTs6uO4qcLgJrHGxsHdaklOeaukqTZZWh4ubdhs1 KHJOoxNAfzZvD7f0IPsT1w5mlu68ehAMV5OfLH5QtqhViOBh8yCUSPn5dwpeY1k2 28Ped42jst4U1PUCVELSf5hQe7KUCvE0xr5mcnT0rq5rta7nw9nCXeIaIQLUSP/i ouh0ZE1vf3ScsqmM0AV2hqc2GOdtzwaMPPKXFTbP3UQjAXWYDOs7UdITLOGbQupj JAW3vEEZEu6xd0TcvuQ1o5S9szHVxqJUOelqaZMn/w+8xOBfAQ4wcskqaEcE1Y2f X0pneuAx473kJOfGVEv/RbAnf8Vc9hXkRoSK7OY6f0tJPYyMreWbc7H15gUVRA4X jzcAH6VYOxxUX4trznNArmnGIisjylweZlJvTSWUawTzMNCUEieD4hWKmey8sX1r YhQsoyuy4JkumXqY6Eu9XkVCHNQg+JZkRfGB5EudzoUAqlL5D1NhcHx+1EhaELQd qNNVtPMH79vDUHs18Hn/np9eX8cHIyfDGhI5yHt4m3bx111p3I/hOo57yaV9WbrP 3C/USolKKxNUPgoPPP/3KKa+skDt9wKvPIU5lvDd9F9TKbm8hcc= =q47C -----END PGP SIGNATURE-----
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
-- Lucas Werkmeister Full Stack Developer
Wikimedia Deutschland e. V. | Tempelhofer Ufer 23-24 | 10963 Berlin Phone: +49 (0)30 219 158 26-0 https://wikimedia.de
Imagine a world, in which every single human being can freely share in the sum of all knowledge. That‘s our commitment.
Wikimedia Deutschland - Gesellschaft zur Förderung Freien Wissens e. V. Eingetragen im Vereinsregister des Amtsgerichts Berlin-Charlottenburg unter der Nummer 23855 B. Als gemeinnützig anerkannt durch das Finanzamt für Körperschaften I Berlin, Steuernummer 27/029/42207. _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
We've decided it is better to *not* update User::isAllowed() https://phabricator.wikimedia.org/T208563 and User::getRights() https://phabricator.wikimedia.org/T208895 at this time (there are too many edge cases to account for).
Every extension needs to deal with what a block means for their extension. It may mean denying access, allowing access under certain conditions, or neither.
Therefore, we recommend changing any instances of: $user->isBlocked() to either: Title::userCan() // If you have the title object. OR $user->getBlock() && $user->getBlock()->isSitewide() // or some other condition, if you do not have the title object. OR $user->getBlock() && $user->getBlock()->prevents( 'upload' ) // or some other action that a block knows how to handle.
or some other combination. :)
Thanks!
On Mon, Nov 5, 2018 at 10:15 AM David Barratt dbarratt@wikimedia.org wrote:
Well, that function is also exposed externally, e. g. in the 'blocked'
field of the OAuth identity (what’s returned by Special:OAuth/identify). What should happen to that in the future?
For the Userinfo API https://www.mediawiki.org/wiki/API:Userinfo we added https://phabricator.wikimedia.org/T197141 a partial field. A field like this could be added, or you could use the Block::isSitewide() method to only inform the OAuth client that the user is blocked if they are blocked from everything (errr... everything except their own user talk page... maybe).
Regardless if you expose a new flag or change the behavior of the existing flag, you would need to call User::getBlock() in order to determine if the block is sitewide or not. Example: $user->getBlock() && $user->getBlock->isSitewide()
We considered changing the behavior of User::isBlocked() to only return true if they are sitewide blocked, but as Alex pointed out, that would cause blocked users to not be blocked from things that perhaps they should be blocked from. So we decided to fall on the side of caution.
Since User::isBlocked() method doesn't really tell you anything useful (and as Dayllan pointed out, it hasn't for a very long time), I support deprecating the method. Most extensions should be calling User::isAllowed() or Title::userCan() instead.
On Mon, Nov 5, 2018 at 9:50 AM Lucas Werkmeister < lucas.werkmeister@wikimedia.de> wrote:
Well, that function is also exposed externally, e. g. in the 'blocked' field of the OAuth identity (what’s returned by Special:OAuth/identify). What should happen to that in the future?
Am Mo., 5. Nov. 2018 um 15:41 Uhr schrieb Dayllan Maza < dmaza@wikimedia.org
:
Also, are there any good reasons to continue checking User::isBlocked()? If not, I think deprecating it would be a good
idea.
+1 for deprecating it. I think the name of that function is misleading to begin with 'cause
even
before "Partial Blocks" there were other actions besides edit that a
block
could prevent (like 'sendemail').
On Sun, Nov 4, 2018 at 9:39 PM Kunal Mehta legoktm@member.fsf.org
wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
Hi,
On 11/2/18 12:58 PM, Alex Ezell wrote:
*tl:dr;* On a wiki with the new Partial Blocks enabled (currently only testwiki), if the code is checking User::isBlocked() to determine edit rights, it should instead check User::isBlockedFrom( Title ). The code could also check isBlocked() && $block->isSitewide(). If it doesn’t, the code may block users that shouldn’t be blocked.
Based on a quick codesearch[1], this affects a lot of extensions, and MediaWiki core itself. It would be nice if bugs (or patches!) could be filed against code that needs updating.
Also, are there any good reasons to continue checking User::isBlocked()? If not, I think deprecating it would be a good
idea.
[1]
https://codesearch.wmflabs.org/search/?q=-%3EisBlocked%5C(&i=nope&fi...
&repos= <
https://codesearch.wmflabs.org/search/?q=-%3EisBlocked%5C(&i=nope&fi...
- -- Legoktm
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCgAdFiEE+h6fmkHn9DUCyl1jUvyOe+23/KIFAlvfrU4ACgkQUvyOe+23 /KLWYg/+LR3h0UjQwwomCba9VrnwvMYApsP7xHnZUvukMqF/mjNvKFnR1Rs/pEWj 10IU0QMVxBB6cA47xLY9oYWoMTs6uO4qcLgJrHGxsHdaklOeaukqTZZWh4ubdhs1 KHJOoxNAfzZvD7f0IPsT1w5mlu68ehAMV5OfLH5QtqhViOBh8yCUSPn5dwpeY1k2 28Ped42jst4U1PUCVELSf5hQe7KUCvE0xr5mcnT0rq5rta7nw9nCXeIaIQLUSP/i ouh0ZE1vf3ScsqmM0AV2hqc2GOdtzwaMPPKXFTbP3UQjAXWYDOs7UdITLOGbQupj JAW3vEEZEu6xd0TcvuQ1o5S9szHVxqJUOelqaZMn/w+8xOBfAQ4wcskqaEcE1Y2f X0pneuAx473kJOfGVEv/RbAnf8Vc9hXkRoSK7OY6f0tJPYyMreWbc7H15gUVRA4X jzcAH6VYOxxUX4trznNArmnGIisjylweZlJvTSWUawTzMNCUEieD4hWKmey8sX1r YhQsoyuy4JkumXqY6Eu9XkVCHNQg+JZkRfGB5EudzoUAqlL5D1NhcHx+1EhaELQd qNNVtPMH79vDUHs18Hn/np9eX8cHIyfDGhI5yHt4m3bx111p3I/hOo57yaV9WbrP 3C/USolKKxNUPgoPPP/3KKa+skDt9wKvPIU5lvDd9F9TKbm8hcc= =q47C -----END PGP SIGNATURE-----
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
-- Lucas Werkmeister Full Stack Developer
Wikimedia Deutschland e. V. | Tempelhofer Ufer 23-24 | 10963 Berlin Phone: +49 (0)30 219 158 26-0 https://wikimedia.de
Imagine a world, in which every single human being can freely share in the sum of all knowledge. That‘s our commitment.
Wikimedia Deutschland - Gesellschaft zur Förderung Freien Wissens e. V. Eingetragen im Vereinsregister des Amtsgerichts Berlin-Charlottenburg unter der Nummer 23855 B. Als gemeinnützig anerkannt durch das Finanzamt für Körperschaften I Berlin, Steuernummer 27/029/42207. _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
wikitech-l@lists.wikimedia.org