Last modified: 2014-09-23 23:43:06 UTC
Nothing is broken, per se, but I'll file this bug anyway because there are a number of generally quite very minor things that could be done in order to make it much easier for people to plug in external authentication and identity systems. This is a collection of the issues we've seen when working to let MediaWiki recognize LID ( http://lid.netmesh.org/ ) Light-Weight Digital Identities. While the AuthPlugin is a good start, to integrate with some identity schemes it may be necessary or advantageous to subclass, or entirely replace class User with some other class that can capture more, or different information about a user while presenting the same interface to the rest of MediaWiki. It may also be that structurally or behaviorally different kinds of User need to be supported. Currently, class User is kind of a catch-all, with different execution paths depending on whether the User is anonymous, registered and/or only known by an IP address. So here are the issues: 1) much code using class User continues to distinguish anonymous from known users by looking at the user id (comparing against 0). Appropriate methods that abstract away from that exist already (isAnon(), isLoggedIn()). They should be used everywhere instead of comparing against the user id. 2) We need an extension point, similar to the AuthPlugin extension point that can be configured from LocalSettings, that allows a developer to use a different class than User to instantiate $wgUser. Given the code is currently structured, it does not appear to be possible to do this from within LocalSettings.php (one of the reasons: an alternate implementation of User is most likely going to need setup information that is not available yet at the time LocalSettings is processed). The best place appears to be right where $wgUser is assigned for the first time in includes/Setup.php. Maybe one could create a new global setting called $wgUserClass, which could be require_once'd, if given, so developers could easily put whatever $wgUser instantiation code there that they need. That would be invoked from Setup.php. 3) While not strictly required, I would strongly recommend to take a hard look at User.php and refactor it into separate classes, such as AbstractUser, AnonymousUser, and RegisteredUser. It may also make sense to separate the class instantiated for the owner of the current session ($wgUser) from the class instantiated for other users who are not currently the owner of the session (e.g. in the various SpecialXXX pages) Following various execution paths through User.php, it appears that this would substantially increase code readability while likely reducing memory consumption. This would also produce a clear avenue for implementors to provide other kinds of User classes. Unfortunately, this is fairly difficult to do for an outsider like myself because it is exceedingly time-consuming trying to figure out which aspects of User.php are really needed for which use case. It does seem to carry a lot of baggage. 4) It would be nice if methods that are really statics didn't use the $this pointer and instead did something like User::method(). That way, alternate implementations of the User class can at least delegate to some of the code in User.php instead of having to copy it. For some of what I'm saying here I can create (LID-independent or LID-dependent, as desired) patches assuming they are of interest. If so, what should I do with them?
5) There should be a "equals" method on class User (or AbstractUser) so comparisons can be performed between objects and do not need to resort to comparing the id attribute directly.
Created attachment 1013 [details] Patch for issues #1 and #2 mentioned in this bug
Couple of quick notes: 1) We really should use isAnon()/isLoggedIn() methods instead of just checking getId() > 0. Patch will need some tweaking, but it's got the right idea. 2) Subclassing User for external auth systems is now possible. Simetrical added this ability in r53497. 3) I kinda like the idea of making User more modular (abstract User, AnonUser and RegisteredUser extend). Might be worth poking.
*Bulk BZ Change: +Patch to open bugs with patches attached that are missing the keyword*
Johannes Ernst, thanks for your patch. The codebase has changed substantially since you submitted your patch; if you have time and interest, please come into #mediawiki on freenode IRC to discuss how to continue working on this issue. Thanks!