Page MenuHomePhabricator

Make UserIdentity objects aware of which wiki they belong to
Open, HighPublic

Description

To allow service objects for safe cross-wiki operations, we need t make all entity objects aware of which wiki they belong to. UserIdentity is so far lacking this information.

Similar to what we do with PageIdentity (and soon with RevisionRecord), UserIdentity should get the following methods:

public function getWikiId: string|false;
public function assertWiki( string|false $wikiId );

And methods that return IDs that are bound to specific databases need to assert the wikiId:

public function getUserId( $wikiId = self::LOCAL ): int;
public function getActorId( $wikiId = self::LOCAL ): int;

For backwards compatibility, calls getActorId() with no $wikiId provided should not yet fail if the user belongs to another wiki, but merely trigger a deprecation warning. The getId() emthod should be deprecated, and should emit deprecation warnings when called on an instance that belongs to another wiki.

Steps

  • add wiki ID to UserIdentity
  • make getId(), getUserId() and getActorId() trigger a warning if $wikiId is LOCAL but the object belongs to another wiki. If $wikiId is provided but mismatches, throw.
  • after ensuring the above deprecations do not cause problems, make getUserId() and getActorId() always throw if $wikiId mismatches, and remove getId().

Side notes:

  • getId() should at some point be renamed to getUserId() for clarity. If we do it now, getUserId() could immediately throw on mismatching $wikiId.
  • It should probably not be possible for a UserIdentity to have a user ID but no actor ID. If a user ID is given, an actor ID must also be given, I think. There may however be existing tests that don't follow that rule.

Event Timeline

daniel created this task.Aug 20 2020, 7:08 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 20 2020, 7:08 PM

Do you think it would be valuable to define an interface (name TBD as always, I'm terrible in naming)

interface WikiAwareObject {
  public function getWikiId() : string;
}

and make UserIdentity and other usages extend it? I can't really imagine a ton of usages for the new interface, but you can for example create a common trait for all the services that allow cross-wiki object manipulation like

trait LocalWikiAssert {
    public function assertLocalWiki( WikiAwareObject $object );
}

or even have convenience methods in, say, LoadBalancerFactory like

public function getLoadBalancerFor( WikiAwareObject $entity );

Not crazy fancy, but looks kinda neat

eprodromou triaged this task as Medium priority.Aug 25 2020, 8:30 PM
eprodromou moved this task from Inbox to Tech Planning Review on the Platform Engineering board.
eprodromou added a subscriber: eprodromou.

Worth having some review of the code design.

daniel updated the task description. (Show Details)Jan 22 2021, 11:30 AM
daniel updated the task description. (Show Details)Jan 25 2021, 8:55 PM

Change 658751 had a related patch set uploaded (by Cicalese; owner: Cicalese):
[mediawiki/core@master] Make UserIdentity objects aware of which wiki they belong to.

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

daniel renamed this task from Make User objects aware of which wiki they belong to to Make UserIdentity objects aware of which wiki they belong to.Jan 29 2021, 2:17 PM
daniel updated the task description. (Show Details)
daniel raised the priority of this task from Medium to High.Jan 29 2021, 2:31 PM

Change 658751 merged by jenkins-bot:
[mediawiki/core@master] Make UserIdentity objects aware of which wiki they belong to.

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

Pchelolo updated the task description. (Show Details)Mon, Feb 22, 7:47 PM

Deprecation warnings are being emitted now, we need to wait until starting to throw.