@WMDE-leszek can we sit down together and get this ready for pickup?

In T207893#5109784, @Lydia_Pintscher wrote:

@WMDE-leszek can we sit down together and get this ready for pickup?

Hi @Lydia_Pintscher! Is there an update on this? We're getting ready to deploy partial blocks across all wikis.

I just had another quick look at this today.

In T207893#4787089, @dbarratt wrote:

In T204991 we've added the method Block::appliesToNamespace() which can be used for new entity creation.

So I guess we use BlockManager to get the blocks of a user?
And then check the blocks to see if they apply to the namespace we are creating in?

Blockmanager::getUserBlock only returns one block ("the most relevant one"), what happens in the situation when a user has 2 block for different namespaces?
Is there a way to get all blocks of a user? Or a way to see if any of a users blocks apply to a namespace?

Maybe we need a User::isBlockedFromNamespace, or rather PermissionManager::isBlockedFromNamespace() ?

In T207893#5109784, @Lydia_Pintscher wrote:

@WMDE-leszek can we sit down together and get this ready for pickup?

The work that is needed for editing special pages is already outlined in the task description, although some refactoring will be needed there in order to get the Title to the method where we check blocks, or just do the block checking in each special page class itself.

Then we need to resolve my questions at the top of this comment about appliesToNamespace for entity creation.

As for edits through the API, as far as I can tell these block checks are already handled via the edit actions an EditEntityAction -> Action.php in core. So nothing to do there.

Going to move to waiting until the above comment has a response regarding the namespace checks.

In T207893#5250838, @Addshore wrote:

So I guess we use BlockManager to get the blocks of a user?

You can use either User::getBlock() or BlockManager::getUserBlock(). They are slightly different, but for your purposes, it should always return the same result.

And then check the blocks to see if they apply to the namespace we are creating in?

Yes. :)

Blockmanager::getUserBlock only returns one block ("the most relevant one"), what happens in the situation when a user has 2 block for different namespaces?

You will get a CompositeBlock object which will extend from AbstractBlock class and have all of the same methods. MediaWiki has, up until this week, always returned a single, most-relevant block anyways. Starting soon, you'll get a CompositeBlock which can be used the same way.

Is there a way to get all blocks of a user? Or a way to see if any of a users blocks apply to a namespace?

We could probably build that into CompositeBlock if you really need them, but calling ::appliesToNamespace() on the AbstractBlock returned from User::getBlock() or BlockManager::getUserBlock() should be what you want.

Maybe we need a User::isBlockedFromNamespace, or rather PermissionManager::isBlockedFromNamespace() ?

I think the best way would be to determine if the AbstractBlock applies to the namespace with AbstractBlock::appliesToNamespace().

In T207893#5251961, @dbarratt wrote:

In T207893#5250838, @Addshore wrote:

So I guess we use BlockManager to get the blocks of a user?

You can use either User::getBlock() or BlockManager::getUserBlock(). They are slightly different, but for your purposes, it should always return the same result.

You should use User::getBlock, since that will call BlockManager::getUserBlock and run the hooks too.

(BlockManager::getUserBlock should be treated as internal and only be called by User::getBlockedStatus (as documented). Eventually, the blocks logic should be taken out of User, as part of the wider effort to untangle such dependencies, at which point the way to get a block will be via the BlockManager, but we're not there yet.)

And then check the blocks to see if they apply to the namespace we are creating in?

Yes. :)

Yes, so somethig like:

$block = $user->getBlock();
if ( $block && $block->appliesToNamespace( $ns ) ) { ... }

Blockmanager::getUserBlock only returns one block ("the most relevant one"), what happens in the situation when a user has 2 block for different namespaces?

You will get a CompositeBlock object which will extend from AbstractBlock class and have all of the same methods. MediaWiki has, up until this week, always returned a single, most-relevant block anyways. Starting soon, you'll get a CompositeBlock which can be used the same way.

A few things to add here:

• Two blocks can't be stored against exactly the same target, but a user can be affected by more than one block if there are blocks against their account and IP address, or against some ranges that cover their IP address
• "the most relevant one" is a not-terribly-helpful way of saying the most "specific" one, meaning that a user account block is chosen over an IP block, which is chosen over an IP range block (and narrower IP range blocks are favoured over wider ones). Thanks for pointing this out, will improve the docs.
• AHT should be merging this soon: https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/497668/ After this, if BlockManager::getUserBlock finds more than one block, then instead of choosing just one, it will return a composite block combining the strictest features of all the blocks. So if there are blocks that apply to different namespaces, this composite block will apply to all of them.

Is there a way to get all blocks of a user? Or a way to see if any of a users blocks apply to a namespace?

We could probably build that into CompositeBlock if you really need them, but calling ::appliesToNamespace() on the AbstractBlock returned from User::getBlock() or BlockManager::getUserBlock() should be what you want.

Maybe we need a User::isBlockedFromNamespace, or rather PermissionManager::isBlockedFromNamespace() ?

I think the best way would be to determine if the AbstractBlock applies to the namespace with AbstractBlock::appliesToNamespace().

After the above patch is merged, doing $user->getBlock() and $block->appliesToNamespace( $ns ) as shown above will do all of this.

Thanks for all of the details.
I believe that 1 bit of phpdoc, which sounds like it will now be altered, is what was throwing me a bit off.

@lydia I believe we have all of the details we need to write this up for pickup. :)
I'll try and get that done this week (unless someone wants to jump in before me) or if we just want to pick it up and have whoever implements it read through the comments.!

@Niharika would you mind reminding us what is the planned timeline of the deployment of partial blocks? Just so that we know how urgently the fix is needed (I do anticipate that ASAP is preferred, but given you've "only" raised the priority up to "Normal" we would also only prioritize it accordingly)

In T207893#5255918, @WMDE-leszek wrote:

@Niharika would you mind reminding us what is the planned timeline of the deployment of partial blocks? Just so that we know how urgently the fix is needed (I do anticipate that ASAP is preferred, but given you've "only" raised the priority up to "Normal" we would also only prioritize it accordingly)

Our planned rollout for Partial blocks is end of this month (June). I was actually going to raise the priority higher but I didn't want to step on anyone's toes (whoever prioritizes tickets at your end). ASAP is indeed preferred.

This is rather lots of work, SpecialWikibasePage is not the right place to keep the list, Making it per special page also is complex, like Special:SetLabel can be used on property and item and if the person is blocked on item namespace, there's no way to make sure it's applied until the user actually tries it (in which EntityStore should handle it properly using PermissionManager), so maybe we should just drop it? I would prefer if the special page still checks for site-wide bans.

Change 517442 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Make SpecialWikibasePage only respect site-wide block

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

Change 517450 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Add namespace-based block check on SpecialNewEntity

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

In T207893#5262838, @Ladsgroup wrote:

This is rather lots of work, SpecialWikibasePage is not the right place to keep the list, Making it per special page also is complex, like Special:SetLabel can be used on property and item and if the person is blocked on item namespace, there's no way to make sure it's applied until the user actually tries it (in which EntityStore should handle it properly using PermissionManager), so maybe we should just drop it? I would prefer if the special page still checks for site-wide bans.

It's fine if the user doesn't know until it's attempted. For instance, any edits made with the Action API this is the case. We try to notify the user before hand, but that's not always possible. For instance we added T194585 so a preflight can be created to determine if the user is blocked from a particular title. However, if it's not possible, I would throw a validation error to the user.

You can see an example of not knowing in MediaWiki by blocking a test user from a namespace (say MediaWiki) and then attempting to create a new page in that namespace.

Change 520200 had a related patch set uploaded (by Hoo man; owner: Hoo man):
[mediawiki/extensions/WikibaseLexeme@master] Add namespace-based block check to SpecialNewLexeme

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

Change 520200 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Add namespace-based block check to SpecialNewLexeme

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

Change 517442 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make SpecialWikibasePage only respect site-wide block

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

Change 517450 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add namespace-based block check on SpecialNewEntity

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

I assume this is done now… feel free to move this back if anything is missing.

Thanks @hoo. What's the best way to test this?

In T207893#5326979, @Niharika wrote:

Thanks @hoo. What's the best way to test this?

Enabling partial blocks on wikidata and commons in beta cluster would be a good point IMO

In T207893#5326996, @Ladsgroup wrote:

In T207893#5326979, @Niharika wrote:

Thanks @hoo. What's the best way to test this?

Enabling partial blocks on wikidata and commons in beta cluster would be a good point IMO

Thanks @Ladsgroup!

It's also enabled in production at:
https://test.wikidata.org/wiki/Special:Block

In T207893#5329617, @dbarratt wrote:

It's also enabled in production at:
https://test.wikidata.org/wiki/Special:Block

@dbarratt Do you have rights on that site to give me and @dom_walden access to Special:Block?

@Niharika User:NKohli (WMF) doesn’t seem to exist on testwikidata yet, I think you’ll have to create the account there first before rights can be given to it. (@dbarratt exists, but I don’t have the rights to make him admin, apparently.)

In T207893#5329793, @Niharika wrote:

In T207893#5329617, @dbarratt wrote:

It's also enabled in production at:
https://test.wikidata.org/wiki/Special:Block

@dbarratt Do you have rights on that site to give me and @dom_walden access to Special:Block?

I can access it myself, but not grant access.

But we can also enable on beta. :)

It's already enabled there! :)
https://wikidata.beta.wmflabs.org/wiki/Special:Block
and
https://commons.wikimedia.beta.wmflabs.org/wiki/Special:Block

@dbarratt @Niharika I added you both to the admin group on test wikidata

@dbarratt @Niharika have you tested this? is it good?

In T207893#5409768, @alaa_wmde wrote:

@dbarratt @Niharika have you tested this? is it good?

It works beautifully for me! I will allow @dom_walden to take a look if he wants too, otherwise I think this is good to enable on production. :)

I can access and submit the Special:NewItem and Special:NewProperty forms unless I am partially blocked from the Main or Property namespace (resp.)

I setup partial blocks for a user and their IP (i.e. a composite block), which used page restrictions to block the user from specific wikidata items.

I performed the following special page actions on items they were and were not blocked from, and was allowed or not allowed as appropriate.

• Special:MergeItems
• Special:RedirectEntity
• Special:SetSiteLink
• Special:SetAliases
• Special:SetDescription
• Special:SetLabel
• Special:SetLabelDescriptionAliases
• Using the "Add links" on client wikidata wikis

I could also edit items (e.g. adding statements, links, etc.) unless I was blocked from them specifically.

I note that some of the block messages were a bit uninformative (e.g. some just said "Permission denied.", no extra info.) I assume this is a preexisting bug.

As for edits through the API, these block checks are already handled via the edit actions an EditEntityAction -> Action.php in core. So nothing to do there...

I did not test the API, apart from the API calls that appear to happen when editing an item. Although, I see there are some phpunit checks that simulate performing certain API requests while blocked.

I did all this testing on my local Vagrant environment, with the wikidata role enabled.

• MediaWiki 1.34.0-alpha (dbdbeb7) 06:23, 14 August 2019
• WikibaseRepository (dfadb52) 07:16, 14 August 2019

I was not able to test Special:NewLexeme. It is not included in the vagrant role nor on https://commons.wikimedia.beta.wmflabs.org/wiki/. It is on https://wikidata.beta.wmflabs.org/wiki/, but I am not an administrator there. @Addshore, may I be added to the admin group on wikidata.beta, if you think this matters? (My user is "Dom walden")

@dom_walden I've just added you to admin group on https://wikidata.beta.wmflabs.org (rights expire in 24 hours, let me know if this is a silly restriction, so I will make you unexpirying admin)

I am blocked from https://wikidata.beta.wmflabs.org/wiki/Special:NewLexeme even if I am only partially blocked.

Maybe the appropriate change needs to be made here: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/WikibaseLexeme/+/master/src/MediaWiki/Specials/SpecialNewLexeme.php#120

Change 531347 had a related patch set uploaded (by Noa wmde; owner: Noa wmde):
[mediawiki/extensions/WikibaseLexeme@master] Add site wide block check to checkBlocked

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

Change 531347 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Add site wide block check to checkBlocked

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

Oversight on my part, it looks like the creation code (in both patches) is using AbstractBlock::isSitewide() instead of AbstractBlock::appliesToNamespace() (as is outlined in the task description). Using the former means that someone who is blocked from a namespace will be allowed to create an item in that namespace, using the latter will ensure they are blocked properly from that namespace.

In T207893#5429435, @dbarratt wrote:

Oversight on my part, it looks like the creation code (in both patches) is using AbstractBlock::isSitewide() instead of AbstractBlock::appliesToNamespace() (as is outlined in the task description). Using the former means that someone who is blocked from a namespace will be allowed to create an item in that namespace, using the latter will ensure they are blocked properly from that namespace.

As I understand it, the namespace blocking for Lexemes was done in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikibaseLexeme/+/520200. But please confirm this.

Testing on beta, I cannot create a new Lexeme (either via Special:NewLexeme or api.php?action=wbeditentity) if I am partially blocked from the "Lexeme" namespace or if I am sitewide blocked.

I can if I am partially blocked but not from the Lexeme namespace.

In T207893#5429435, @dbarratt wrote:

Oversight on my part, it looks like the creation code (in both patches) is using AbstractBlock::isSitewide() instead of AbstractBlock::appliesToNamespace() (as is outlined in the task description). Using the former means that someone who is blocked from a namespace will be allowed to create an item in that namespace, using the latter will ensure they are blocked properly from that namespace.

I just want to mention those parts are for basic access to the special page, the real action is being handled by PermissionManager so nothing is broken here.

In T207893#5431250, @Ladsgroup wrote:

I just want to mention those parts are for basic access to the special page, the real action is being handled by PermissionManager so nothing is broken here.

Ah! perfect. thanks!

In T207893#5431241, @dom_walden wrote:

In T207893#5429435, @dbarratt wrote:

Oversight on my part, it looks like the creation code (in both patches) is using AbstractBlock::isSitewide() instead of AbstractBlock::appliesToNamespace() (as is outlined in the task description). Using the former means that someone who is blocked from a namespace will be allowed to create an item in that namespace, using the latter will ensure they are blocked properly from that namespace.

As I understand it, the namespace blocking for Lexemes was done in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikibaseLexeme/+/520200. But please confirm this.

Testing on beta, I cannot create a new Lexeme (either via Special:NewLexeme or api.php?action=wbeditentity) if I am partially blocked from the "Lexeme" namespace or if I am sitewide blocked.

I can if I am partially blocked but not from the Lexeme namespace.

thanks @dom_walden ! So are we fine to conclude this is now resolved?

In T207893#5431725, @WMDE-leszek wrote:

thanks @dom_walden ! So are we fine to conclude this is now resolved?

If everyone else is happy, then I'm happy :)