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
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!