Page MenuHomePhabricator

investigation about permissions and protection
Closed, ResolvedPublic

Description

Problem:
Sometimes we can't let certain edits happen. This might be because:

  • article is protected
  • Item is protected
  • user/ip is blocked on client
  • user/ip is blocked on repo

Ideally we don't want to let them make the edit and then have it be rejected by the API afterwards. Instead we want to intervene as early as possible by for example not showing them the edit link, or less ideally showing a message in the Bridge modal.

We need to figure out what is technically possible. New Lua function? API check?

Acceptance criteria:

  • we have a recommendation for how to handle the 4 cases above and the documentation of the investigated ideas

Open questions:

  • user specific vs. caching?!

Timeboxing this for 2 days - ideally 2 people (1 person per day)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 26 2019, 3:18 PM
jeblad added a subscriber: jeblad.Aug 28 2019, 9:57 AM

Note that some of the more restrictive states of Firefox and Chrome may impact CentralAuth, and a person can be logged in at Wikipedia and still not being logged in at Wikidata. Thus logged in state must be checked on both repo and client.

Se also T202028: Central auth fails when using "site isolation" in Google Chrome and Chromium or "first-party isolation" in Firefox

Ah good point. I'll open a separate ticket for that.

Lydia_Pintscher triaged this task as Normal priority.Fri, Sep 20, 10:51 AM
Pablo-WMDE added a subscriber: Pablo-WMDE.EditedWed, Sep 25, 11:27 AM

Maybe we can illustrate the different versions we envision in sequence diagrams to ease others understanding the differences and consequences.

Things we can do:

  • Add the protection level of the client page to the data bridge config. (We tested that protecting/unprotecting the page resets the parser cache, so this should be safe to do.)
  • Check mw.config.get( 'wgIsProbablyEditable' ) client-side as early as we want. If it’s false, we can check the protection level (previous bullet point) to estimate whether it’s due to protection or block.
  • Make an API call to the repo as soon as the app opens (but not before, that would be too many API calls – a. k. a. at bridge init), getting the user blocks+rights and entity protection with one call: action=query&meta=userinfo&uiprop=blockinfo|rights|groups&titles=Item:Q1234&prop=info&inprop=protection.
    • This, however, requires that we know the title of the entity page (and the Special:EntityPage/Q1234 workaround doesn’t work here). We might have to resurrect the repoNamespaces client option, which currently exists but seems to be unused.
    • We can also split the meta=userinfo part out of this and add a maxage parameter to it, so that it will be cached client-side for an appropriate amount of time. This probably doesn’t reduce server load (the call to get the page information will likely load user rights anyways), but it reduces the response size, saving our users some network traffic.

Things we can’t do:

  • Add whether the user is blocked or not to the data bridge config – this would be embedded in the parser cache and be shared between different IP users, where the blocked status may be different. (Hence the “probably” in the existing wgIsProbablyEditable variable.)

The earliest point where we can make these checks is at app init – so in some cases the user may see the loading screen before being told that actually, they can’t edit.

This also implies that we’re estimating permissions client-side, based on the user’s rights, blocks, and the page’s protections. If that’s not good enough, we may need to introduce an API call for checking whether a user can edit a page (effectively an interface to PermissionManager, though we only need $action = 'edit'), which currently doesn’t seem to exist. (This could be achieved by making a “null edit” to the entity – action=wbeditentity&data={}, which can even replace the current entity download – but that means a hit to the master DB every time the app opens.) It would seem that the Wikipedia Android app should have the same need for such an API, but it appears to estimate permissions client-side as well.

Thank you for the investigation!

This would mean another 1-2 API calls before we can get from the initializing to the ready stage? I guess there is no way around that :/

This also implies that we’re estimating permissions client-side, based on the user’s rights, blocks, and the page’s protections.

So, the cases where we might get things wrong is when one IPs is blocked, but wgIsProbablyEditable has been cached with the block-information from an unblocked IP (or vice versa)?
We probably don't want to show the "You are blocked"-message to innocent IPs, whereas the other way around might be less bad. Though, I guess at the end of the day, that is a trade-off that product has to decide.

So, the cases where we might get things wrong is when one IPs is blocked, but wgIsProbablyEditable has been cached with the block-information from an unblocked IP (or vice versa)?

wgIsProbablyEditable is documented as “if it is false, it is definitely not editable”, so I assume it doesn’t take IP blocks into account. So it would be possible that a page is “probably editable” even though the IP is blocked, but not vice versa.

Note that the meta=userinfo API call mentioned in the third bullet point also works for IPs (I just tested it), so we should be able to get accurate information from that.

Pablo-WMDE added a comment.EditedTue, Oct 8, 2:49 PM

Added to our sequence diagram the two possible places where I see us performing the permission checks but this is open to interpretation (especially in the client part)

  • Check mw.config.get( 'wgIsProbablyEditable' ) client-side as early as we want. If it’s false, we can check the protection level (previous bullet point) to estimate whether it’s due to protection or block.

@Lydia_Pintscher this assumes that we want to inform the user why they can’t edit – whether it’s due to a block or page protection. Do we actually want that, or should they always get a generic error message? (Possibly also a question for @Charlie_WMDE.)

Alternative suggestion from @Pablo-WMDE – perhaps we don’t even want to overload the links if the user doesn’t have permission to edit? (Checking repo permissions on page load is too expensive, but we could do the client-side check at this point.) Or perhaps hide the links in that case?

Discussed with @Lydia_Pintscher and @Charlie_WMDE

Client: destroy bridge-enabled links if user does not have permission to edit on client

Repo: show message that editing is not permitted. factor reason (e.g. a block) into the message

Lydia_Pintscher closed this task as Resolved.Thu, Oct 10, 8:14 AM
Lydia_Pintscher moved this task from Peer Review to Done on the Wikidata-Bridge-Sprint-7 board.

\o/

I opened tickets T235152 and T235154 for those two scenarios.