We need to know if we need to wrap AuthManager up in an AuthPlugin shim and if so what that shim needs to be able to accomplish.
Description
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Resolved | • Deskana | T75616 Tracking: API/backend issues blocking Wikipedia app development | |||
Resolved | Anomie | T32788 Allow triggering of user password reset email via the API | |||
Open | None | T90925 General authentication improvements for MediaWiki | |||
Resolved | Anomie | T48179 Allow a challenge stage during authentication | |||
Open | None | T5709 Refactoring to make external authentication and identity systems easier | |||
Resolved | Tgr | T43201 UserLoadFromSession considered evil | |||
Resolved | Anomie | T67493 Session is started by EditAction (problem for extensions using UserLoadFromSession hook) | |||
Open | Feature | None | T55156 Provide option to force a login session to end within a certain time | ||
Open | None | T89459 Modernize MediaWiki authentication system (AuthManager) | |||
Resolved | Anomie | T110412 Find and assess extensions affected by AuthManager | |||
Resolved | Anomie | T91303 Audit extensions for use of $wgAuth |
Event Timeline
AuthPlugin-implementing extensions:
- LdapAuthentication
- CentralAuth
- Auth_remoteuser (not WMF deployed)
- MediaWikiAuth (not WMF deployed, evil)
- GoogleLogin (not WMF deployed)
WMF-deployed extensions:
- Echo calls ->allowPropChange( 'emailaddress' )
- Seems likely to be easy, although it's not something we touched on at all in the RFC.
- MoodBar calls ->allowPropChange( 'emailaddress' )
- Seems likely to be easy, although it's not something we touched on at all in the RFC.
- CheckUser calls ->getUserInstance(), and on the AuthPluginUser ->getId()
- Basically assumes CentralAuth, it's trying to decide whether to display "Not unified" in its UI. AuthManager will probably need a "userExists( $name )" method, but to support this it would have to return which primary AuthenticationProviders say "yes" and CheckUser would have to decide which should trigger this message.
- OpenStackManager assumes $wgAuth is provided by LdapAuthentication
- Maybe not as painful as it seems, since it doesn't call any AuthPlugin methods. We may just be able to replace its $wgAuth access with something that fetches the (primary) AuthenticationProvider for the session and checks if it's Ldap's, and have Ldap provide the same interface on its AuthenticationProvider.
- UserDailyContribs calls ->userExists()
- Renameuser calls ->getUserInstance(), ->updateExternalDB(), and AuthPluginUser ->resetAuthToken()
- "resetAuthToken" is implied in bullet 4 here.
- The other bit is basically "hey, user data changed, if you care", which Ldap uses to update the Ldap info with the new email, nickname, realname, and language. Not really sure whether that event belongs in AuthManager or would be better as a hook, really.
Non-WMF-deployed extensions:
- OpenID calls ->allowPasswordChange(), ->initUser(), and ->updateUser().
- "allowPasswordChange" is for showing a password reset link in a few places. Not clear what exactly.
- The other two are because it's implementing its own version of the "Attempted login with no local user" flow.
- Configure notes it exists.
- Probably just delete it.
- ConfirmAccount calls ->userExists(), ->addUser()
- Probably wants a rewrite as a SecondaryAuthenticationProvider.
- AjaxLogin calls ->allowPasswordChange()
- No idea why it's implementing its own API endpoint for login instead of using core's action=login.
- The wgAuth call is for deciding to show/allow resetting passwords via its custom UI.
- SemanticSignup calls ->validDomain(), ->canCreateAccounts(), ->userExists(), ->authenticate(), ->allowPasswordChange(), ->modifyUITemplate(), ->addUser(), ->initUser()
- NFI. Probably completely reimplementing Special:UserLogin, and should gather its info in a SecondaryAuthenticationProvider.
- TwitterLogin calls ->allowPasswordChange()
- It's implementing its own version of the "Attempted login with no local user" flow, and calls this before $user->setPassword(User::randomPassword())
An evaluation of AuthPlugin methods:
- AuthPlugin::userExists( $username )
- AuthManager will probably need this. Do we also want to support querying which AuthenticationProviders say "yes"?
- AuthPlugin::authenticate( $username, $password )
- Not as-is, but in a version that takes a set of AuthenticationRequest instances.
- AuthPlugin::modifyUITemplate( &$eew, &$wtf )
- No, kill it. UI specification is (indirectly) via AuthenticationRequest types.
- AuthPlugin::setDomain( $domain )
- AuthPlugin::getDomain()
- AuthPlugin::validDomain( $domain )
- AuthPlugin::domainList()
- These likely belong in LdapAuthenticationRequest somehow, or DomainUserPasswordAuthenticationRequest if we think something besides Ldap might have a use for it.
- AuthPlugin::updateUser( &$user )
- The intent here seems to be "This user just logged in"; Ldap and CentralAuth use it to set user data (email, etc) from their global databases, and CentralAuth also replaces $user entirely on migration. This seems more likely as a hook to me.
- AuthPlugin::initUser( &$user )
- Same, but for local account creation rather than login. This also seems more likely as a hook to me.
- AuthPlugin::updateExternalDB( $user )
- AuthPlugin::updateExternalDBGroups( $user, $addgroups, $delgroups )
- Counterpart to AuthPlugin::updateUser() for user data being updated. The idea is that the plugin should write its global database. This also seems more likely as a hook to me.
- AuthPlugin::autoCreate()
- Not sure whether AuthManager actually needs this, versus the AuthenticationProvider just returning "PASS, no local user, local username should be $X" versus "PASS, no local user, no suggestion for a local username".
- AuthPlugin::allowPropChange( $prop )
- AuthPlugin::allowPasswordChange()
- Ldap might deny password changes, and Auth_remoteuser tries to deny realname, email, and password.
- I suppose we probably need something along these lines to support backends that simply cannot change certain properties. I'd just have allowPropChange( 'password' ) rather than a separate method though.
- AuthPlugin::allowSetLocalPassword()
- AuthPlugin::strict()
- Makes no sense in AuthManager, "local" is just another AuthenticationProvider.
- AuthPlugin::strictUserAuth( $username )
- "FAIL" rather than "ABSTAIN"
- AuthPlugin::setPassword( $user, $password )
- Should probably be replaced with something that takes an AuthenticationRequest and tells the AuthenticationProvider to set data rather than validate it.
- AuthPlugin::canCreateAccounts()
- We have 'link' and 'create' type AuthenticationProviders, probably we just need to add 'no'.
- AuthPlugin::addUser( $user, $password, $email, $realname )
- In AuthManager, this would take a $user and an AuthenticationRequest instance instead, and probably pick up $email and $realname from whatever replaces AuthPlugin::updateExternalDB.
- AuthPlugin::getCanonicalName( $username )
- What happens if multiple things have different ideas on what makes a good "canonical" name?
- AuthPlugin::getUserInstance( &$user )
- The concept of AuthPluginUser is probably ok (except kill ->getId()), but the naming and CentralAuth's implementation are awful.
What about extensions which mess with user data (e.g. EditAccount)? Can we generate a list of them?
Opened T110412 as a more general audit task to avoid spamming this one.
What was the search method? Grepping in the mediawiki/extensions directory? Do we ignore non-Wikimedia-hosted extensions? (Seems reasonable to me.)
Yes, just gerrit hosted extensions were audited. I'm sure there are some (many?) AuthPlugins in the known universe that this doesn't cover, but we can't grep what we can't see. :)
mediawiki.org categories could be used to find some non-wikimedia-hosted extensions, although that of course depends on correct self-reporting (the AuthPluginSetup category for example is not exactly flooded with extensions). For T110414 and T110431 checking the categories would be generally straightforward; and one can google, and search on GitHub and whatnot. (Just by searching for "authplugin" on mediawiki.org I could find a long list of things: PwAuthPlugin, IlchAuth, AuthIMAP, DNNAuthentication, CrowdAuthentication, Siteminder Authentication, AuthPOP3, Shibboleth Authentication, NTLMActiveDirectory, Wget Authentication, AuthSymfony, PHP-Fusion Auth, SAMLAuth, Facebook, AuthDrupal, QISSingleSignOn.) I was just wondering if it is worth the effort; I guess it's not.
No. AuthManager did get an AuthPlugin shim though, so not sure what that would have added.
Extensions hosted in Gerrit (i.e. mediawiki/extensions/*) were audited. Extensions that might have pages in that category were not audited if they were not hosted in Gerrit.