From 94e2aa7b55168ca564c7cf09412dbd93c64a86e7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gerg=C5=91=20Tisza?= Date: Sat, 27 Aug 2016 05:40:37 +0000 Subject: [PATCH] Expand SessionManager / AuthManager documentation Bug: T110628 Bug: T142154 Change-Id: Ib0a41f01b3d12267b2a94ea1375e6d13cacd2b69 --- includes/WebRequest.php | 3 + includes/auth/AuthManager.php | 57 ++++++++++++- includes/auth/AuthenticationProvider.php | 11 ++- includes/auth/AuthenticationRequest.php | 28 +++++-- includes/auth/AuthenticationResponse.php | 37 +++++++-- includes/auth/PreAuthenticationProvider.php | 28 ++++++- .../auth/PrimaryAuthenticationProvider.php | 82 +++++++++++++++---- .../auth/SecondaryAuthenticationProvider.php | 45 ++++++++-- includes/session/Session.php | 8 ++ includes/session/SessionBackend.php | 10 ++- includes/session/SessionInfo.php | 6 +- includes/session/SessionManager.php | 12 ++- includes/session/SessionManagerInterface.php | 6 +- includes/session/SessionProvider.php | 23 +++++- 14 files changed, 296 insertions(+), 60 deletions(-) diff --git a/includes/WebRequest.php b/includes/WebRequest.php index b5c57ee972..553e597b6b 100644 --- a/includes/WebRequest.php +++ b/includes/WebRequest.php @@ -683,6 +683,9 @@ class WebRequest { /** * Return the session for this request + * + * This might unpersist an existing session if it was invalid. + * * @since 1.27 * @note For performance, keep the session locally if you will be making * much use of it instead of calling this method repeatedly. diff --git a/includes/auth/AuthManager.php b/includes/auth/AuthManager.php index acdc01bfea..a5c86be555 100644 --- a/includes/auth/AuthManager.php +++ b/includes/auth/AuthManager.php @@ -37,8 +37,46 @@ use WebRequest; * In the future, it may also serve as the entry point to the authorization * system. * + * If you are looking at this because you are working on an extension that creates its own + * login or signup page, then 1) you really shouldn't do that, 2) if you feel you absolutely + * have to, subclass AuthManagerSpecialPage or build it on the client side using the clientlogin + * or the createaccount API. Trying to call this class directly will very likely end up in + * security vulnerabilities or broken UX in edge cases. + * + * If you are working on an extension that needs to integrate with the authentication system + * (e.g. by providing a new login method, or doing extra permission checks), you'll probably + * need to write an AuthenticationProvider. + * + * If you want to create a "reserved" user programmatically, User::newSystemUser() might be what + * you are looking for. If you want to change user data, use User::changeAuthenticationData(). + * Code that is related to some SessionProvider or PrimaryAuthenticationProvider can + * create a (non-reserved) user by calling AuthManager::autoCreateUser(); it is then the provider's + * responsibility to ensure that the user can authenticate somehow (see especially + * PrimaryAuthenticationProvider::autoCreatedAccount()). + * If you are writing code that is not associated with such a provider and needs to create accounts + * programmatically for real users, you should rethink your architecture. There is no good way to + * do that as such code has no knowledge of what authentication methods are enabled on the wiki and + * cannot provide any means for users to access the accounts it would create. + * + * The two main control flows when using this class are as follows: + * * Login, user creation or account linking code will call getAuthenticationRequests(), populate + * the requests with data (by using them to build a HTMLForm and have the user fill it, or by + * exposing a form specification via the API, so that the client can build it), and pass them to + * the appropriate begin* method. That will return either a success/failure response, or more + * requests to fill (either by building a form or by redirecting the user to some external + * provider which will send the data back), in which case they need to be submitted to the + * appropriate continue* method and that step has to be repeated until the response is a success + * or failure response. AuthManager will use the session to maintain internal state during the + * process. + * * Code doing an authentication data change will call getAuthenticationRequests(), select + * a single request, populate it, and pass it to allowsAuthenticationDataChange() and then + * changeAuthenticationData(). If the data change is user-initiated, the whole process needs + * to be preceded by a call to securitySensitiveOperationStatus() and aborted if that returns + * a non-OK status. + * * @ingroup Auth * @since 1.27 + * @see https://www.mediawiki.org/wiki/Manual:SessionManager_and_AuthManager */ class AuthManager implements LoggerAwareInterface { /** Log in with an existing (not necessarily local) user */ @@ -737,7 +775,10 @@ class AuthManager implements LoggerAwareInterface { /** * Determine whether a username can authenticate * - * @param string $username + * This is mainly for internal purposes and only takes authentication data into account, + * not things like blocks that can change without the authentication system being aware. + * + * @param string $username MediaWiki username * @return bool */ public function userCanAuthenticate( $username ) { @@ -832,6 +873,9 @@ class AuthManager implements LoggerAwareInterface { * If $req was returned for AuthManager::ACTION_REMOVE, using $req should * no longer result in a successful login. * + * This method should only be called if allowsAuthenticationDataChange( $req, true ) + * returned success. + * * @param AuthenticationRequest $req */ public function changeAuthenticationData( AuthenticationRequest $req ) { @@ -871,7 +915,7 @@ class AuthManager implements LoggerAwareInterface { /** * Determine whether a particular account can be created - * @param string $username + * @param string $username MediaWiki username * @param array $options * - flags: (int) Bitfield of User:READ_* constants, default User::READ_NORMAL * - creating: (bool) For internal use only. Never specify this. @@ -1474,6 +1518,13 @@ class AuthManager implements LoggerAwareInterface { /** * Auto-create an account, and log into that account + * + * PrimaryAuthenticationProviders can invoke this method by returning a PASS from + * beginPrimaryAuthentication/continuePrimaryAuthentication with the username of a + * non-existing user. SessionProviders can invoke it by returning a SessionInfo with + * the username of a non-existing user from provideSessionInfo(). Calling this method + * explicitly (e.g. from a maintenance script) is also fine. + * * @param User $user User to auto-create * @param string $source What caused the auto-creation? This must be the ID * of a PrimaryAuthenticationProvider or the constant self::AUTOCREATE_SOURCE_SESSION. @@ -2310,6 +2361,7 @@ class AuthManager implements LoggerAwareInterface { } /** + * Log the user in * @param User $user * @param bool|null $remember */ @@ -2374,6 +2426,7 @@ class AuthManager implements LoggerAwareInterface { /** * Reset the internal caching for unit testing + * @protected Unit tests only */ public static function resetCache() { if ( !defined( 'MW_PHPUNIT_TEST' ) ) { diff --git a/includes/auth/AuthenticationProvider.php b/includes/auth/AuthenticationProvider.php index 4db0a84be9..11f3e226ad 100644 --- a/includes/auth/AuthenticationProvider.php +++ b/includes/auth/AuthenticationProvider.php @@ -28,6 +28,11 @@ use Psr\Log\LoggerAwareInterface; /** * An AuthenticationProvider is used by AuthManager when authenticating users. + * + * This interface should not be implemented directly; use one of its children. + * + * Authentication providers can be registered via $wgAuthManagerAutoConfig. + * * @ingroup Auth * @since 1.27 */ @@ -83,9 +88,9 @@ interface AuthenticationProvider extends LoggerAwareInterface { * - ACTION_LINK: The local user being linked to. * - ACTION_CHANGE: The user having data changed. * - ACTION_REMOVE: The user having data removed. - * This does not need to be copied into the returned requests, you only - * need to pay attention to it if the set of requests differs based on - * the user. + * If you leave the username property of the returned requests empty, this + * will automatically be copied there (except for ACTION_CREATE where it + * wouldn't really make sense). * @return AuthenticationRequest[] */ public function getAuthenticationRequests( $action, array $options ); diff --git a/includes/auth/AuthenticationRequest.php b/includes/auth/AuthenticationRequest.php index 2474b8b830..ff4569b1d9 100644 --- a/includes/auth/AuthenticationRequest.php +++ b/includes/auth/AuthenticationRequest.php @@ -29,7 +29,7 @@ use Message; * This is a value object for authentication requests. * * An AuthenticationRequest represents a set of form fields that are needed on - * and provided from the login, account creation, or password change forms. + * and provided from a login, account creation, password change or similar form. * * @ingroup Auth * @since 1.27 @@ -39,11 +39,14 @@ abstract class AuthenticationRequest { /** Indicates that the request is not required for authentication to proceed. */ const OPTIONAL = 0; - /** Indicates that the request is required for authentication to proceed. */ + /** Indicates that the request is required for authentication to proceed. + * This will only be used for UI purposes; it is the authentication providers' + * responsibility to verify that all required requests are present. + */ const REQUIRED = 1; /** Indicates that the request is required by a primary authentication - * provdier. Since the user can choose which primary to authenticate with, + * provider. Since the user can choose which primary to authenticate with, * the request might or might not end up being actually required. */ const PRIMARY_REQUIRED = 2; @@ -60,7 +63,8 @@ abstract class AuthenticationRequest { /** @var string|null Return-to URL, in case of redirect */ public $returnToUrl = null; - /** @var string|null Username. May not be used by all subclasses. */ + /** @var string|null Username. See AuthenticationProvider::getAuthenticationRequests() + * for details of what this means and how it behaves. */ public $username = null; /** @@ -105,6 +109,11 @@ abstract class AuthenticationRequest { * - sensitive: (bool) If set and truthy, the field is considered sensitive. Code using the * request should avoid exposing the value of the field. * + * All AuthenticationRequests are populated from the same data, so most of the time you'll + * want to prefix fields names with something unique to the extension/provider (although + * in some cases sharing the field with other requests is the right thing to do, e.g. for + * a 'password' field). + * * @return array As above */ abstract public function getFieldInfo(); @@ -126,10 +135,13 @@ abstract class AuthenticationRequest { /** * Initialize form submitted form data. * - * Should always return false if self::getFieldInfo() returns an empty - * array. + * The default behavior is to to check for each key of self::getFieldInfo() + * in the submitted data, and copy the value - after type-appropriate transformations - + * to $this->$key. Most subclasses won't need to override this; if you do override it, + * make sure to always return false if self::getFieldInfo() returns an empty array. * - * @param array $data Submitted data as an associative array + * @param array $data Submitted data as an associative array (keys will correspond + * to getFieldInfo()) * @return bool Whether the request data was successfully loaded */ public function loadFromSubmission( array $data ) { @@ -250,7 +262,7 @@ abstract class AuthenticationRequest { * * Only considers requests that have a "username" field. * - * @param AuthenticationRequest[] $requests + * @param AuthenticationRequest[] $reqs * @return string|null * @throws \UnexpectedValueException If multiple different usernames are present. */ diff --git a/includes/auth/AuthenticationResponse.php b/includes/auth/AuthenticationResponse.php index 5048cf84dd..0339e451b3 100644 --- a/includes/auth/AuthenticationResponse.php +++ b/includes/auth/AuthenticationResponse.php @@ -27,6 +27,10 @@ use Message; /** * This is a value object to hold authentication response data + * + * An AuthenticationResponse represents both the status of the authentication + * (success, failure, in progress) and it its state (what data is needed to continue). + * * @ingroup Auth * @since 1.27 */ @@ -39,7 +43,8 @@ class AuthenticationResponse { /** Indicates that third-party authentication succeeded but no user exists. * Either treat this like a UI response or pass $this->createRequest to - * AuthManager::beginCreateAccount(). + * AuthManager::beginCreateAccount(). For use by AuthManager only (providers + * should just return a PASS with no username). */ const RESTART = 'RESTART'; @@ -67,7 +72,9 @@ class AuthenticationResponse { /** * @var AuthenticationRequest[] Needed AuthenticationRequests to continue - * after a UI or REDIRECT response + * after a UI or REDIRECT response. This plays the same role when continuing + * authentication as AuthManager::getAuthenticationRequests() does when + * beginning it. */ public $neededRequests = []; @@ -84,35 +91,42 @@ class AuthenticationResponse { * @var AuthenticationRequest|null * * Returned with a PrimaryAuthenticationProvider login FAIL or a PASS with - * no username, this holds a request that should result in a PASS when + * no username, this can be set to a request that should result in a PASS when * passed to that provider's PrimaryAuthenticationProvider::beginPrimaryAccountCreation(). + * The client will be able to send that back for expedited account creation where only + * the username needs to be filled. * * Returned with an AuthManager login FAIL or RESTART, this holds a * CreateFromLoginAuthenticationRequest that may be passed to * AuthManager::beginCreateAccount(), possibly in place of any * "primary-required" requests. It may also be passed to - * AuthManager::beginAuthentication() to preserve state. + * AuthManager::beginAuthentication() to preserve the list of + * accounts which can be linked after success (see $linkRequest). */ public $createRequest = null; /** - * @var AuthenticationRequest|null Returned with a PrimaryAuthenticationProvider - * login PASS with no username, this holds a request to pass to - * AuthManager::changeAuthenticationData() to link the account once the - * local user has been determined. + * @var AuthenticationRequest|null When returned with a PrimaryAuthenticationProvider + * login PASS with no username, the request this holds will be passed to + * AuthManager::changeAuthenticationData() once the local user has been determined and the + * user has confirmed the account ownership (by reviewing the information given by + * $linkRequest->describeCredentials()). The provider should handle that + * changeAuthenticationData() call by doing the actual linking. */ public $linkRequest = null; /** * @var AuthenticationRequest|null Returned with an AuthManager account * creation PASS, this holds a request to pass to AuthManager::beginAuthentication() - * to immediately log into the created account. + * to immediately log into the created account. All provider methods except + * postAuthentication will be skipped. */ public $loginRequest = null; /** * @param string|null $username Local username * @return AuthenticationResponse + * @see AuthenticationResponse::PASS */ public static function newPass( $username = null ) { $ret = new AuthenticationResponse; @@ -124,6 +138,7 @@ class AuthenticationResponse { /** * @param Message $msg * @return AuthenticationResponse + * @see AuthenticationResponse::FAIL */ public static function newFail( Message $msg ) { $ret = new AuthenticationResponse; @@ -135,6 +150,7 @@ class AuthenticationResponse { /** * @param Message $msg * @return AuthenticationResponse + * @see AuthenticationResponse::RESTART */ public static function newRestart( Message $msg ) { $ret = new AuthenticationResponse; @@ -145,6 +161,7 @@ class AuthenticationResponse { /** * @return AuthenticationResponse + * @see AuthenticationResponse::ABSTAIN */ public static function newAbstain() { $ret = new AuthenticationResponse; @@ -156,6 +173,7 @@ class AuthenticationResponse { * @param AuthenticationRequest[] $reqs AuthenticationRequests needed to continue * @param Message $msg * @return AuthenticationResponse + * @see AuthenticationResponse::UI */ public static function newUI( array $reqs, Message $msg ) { if ( !$reqs ) { @@ -174,6 +192,7 @@ class AuthenticationResponse { * @param string $redirectTarget URL * @param mixed $redirectApiData Data suitable for adding to an ApiResult * @return AuthenticationResponse + * @see AuthenticationResponse::REDIRECT */ public static function newRedirect( array $reqs, $redirectTarget, $redirectApiData = null ) { if ( !$reqs ) { diff --git a/includes/auth/PreAuthenticationProvider.php b/includes/auth/PreAuthenticationProvider.php index 13fae6eb29..8590cbd13a 100644 --- a/includes/auth/PreAuthenticationProvider.php +++ b/includes/auth/PreAuthenticationProvider.php @@ -27,16 +27,19 @@ use StatusValue; use User; /** - * A pre-authentication provider is a check that must pass for authentication - * to proceed. + * A pre-authentication provider can prevent authentication early on. * * A PreAuthenticationProvider is used to supply arbitrary checks to be * performed before the PrimaryAuthenticationProviders are consulted during the - * login process. Possible uses include checking that a per-IP throttle has not - * been reached or that a captcha has been solved. + * login / account creation / account linking process. Possible uses include + * checking that a per-IP throttle has not been reached or that a captcha has been solved. + * + * This interface also provides callbacks that are invoked after login / account creation + * / account linking succeeded or failed. * * @ingroup Auth * @since 1.27 + * @see https://www.mediawiki.org/wiki/Manual:SessionManager_and_AuthManager */ interface PreAuthenticationProvider extends AuthenticationProvider { @@ -52,10 +55,17 @@ interface PreAuthenticationProvider extends AuthenticationProvider { /** * Post-login callback + * + * This will be called at the end of a login attempt. It will not be called for unfinished + * login attempts that fail by the session timing out. + * + * @note Under certain circumstances, this can be called even when testForAuthentication + * was not; see AuthenticationRequest::$loginRequest. * @param User|null $user User that was attempted to be logged in, if known. * This may become a "UserValue" in the future, or User may be refactored * into such. * @param AuthenticationResponse $response Authentication response that will be returned + * (PASS or FAIL) */ public function postAuthentication( $user, AuthenticationResponse $response ); @@ -97,12 +107,18 @@ interface PreAuthenticationProvider extends AuthenticationProvider { /** * Post-creation callback + * + * This will be called at the end of an account creation attempt. It will not be called if + * the account creation process results in a session timeout (possibly after a successful + * user creation, while a secondary provider is waiting for a response). + * * @param User $user User that was attempted to be created. * This may become a "UserValue" in the future, or User may be refactored * into such. * @param User $creator User doing the creation. This may become a * "UserValue" in the future, or User may be refactored into such. * @param AuthenticationResponse $response Authentication response that will be returned + * (PASS or FAIL) */ public function postAccountCreation( $user, $creator, AuthenticationResponse $response ); @@ -118,10 +134,14 @@ interface PreAuthenticationProvider extends AuthenticationProvider { /** * Post-link callback + * + * This will be called at the end of an account linking attempt. + * * @param User $user User that was attempted to be linked. * This may become a "UserValue" in the future, or User may be refactored * into such. * @param AuthenticationResponse $response Authentication response that will be returned + * (PASS or FAIL) */ public function postAccountLink( $user, AuthenticationResponse $response ); diff --git a/includes/auth/PrimaryAuthenticationProvider.php b/includes/auth/PrimaryAuthenticationProvider.php index 35f3287bd4..4033613f75 100644 --- a/includes/auth/PrimaryAuthenticationProvider.php +++ b/includes/auth/PrimaryAuthenticationProvider.php @@ -27,27 +27,50 @@ use StatusValue; use User; /** - * A primary authentication provider determines which user is trying to log in. + * A primary authentication provider is responsible for associating the submitted + * authentication data with a MediaWiki account. * - * A PrimaryAuthenticationProvider is used as part of presenting a login form - * to authenticate a user. In particular, the PrimaryAuthenticationProvider - * takes form data and determines the authenticated user (if any) corresponds - * to that form data. It might do this on the basis of a username and password - * in that data, or by interacting with an external authentication service - * (e.g. using OpenID), or by some other mechanism. + * When multiple primary authentication providers are configured for a site, they + * act as alternatives; the first one that recognizes the data will handle it, + * and further primary providers are not called (although they all get a chance + * to prevent actions). * - * A PrimaryAuthenticationProvider would not be appropriate for something like + * For login, the PrimaryAuthenticationProvider takes form data and determines + * which authenticated user (if any) corresponds to that form data. It might + * do this on the basis of a username and password in that data, or by + * interacting with an external authentication service (e.g. using OpenID), + * or by some other mechanism. + * + * (A PrimaryAuthenticationProvider would not be appropriate for something like * HTTP authentication, OAuth, or SSL client certificates where each HTTP * request contains all the information needed to identify the user. In that - * case you'll want to be looking at a \\MediaWiki\\Session\\SessionProvider - * instead. + * case you'll want to be looking at a \MediaWiki\Session\SessionProvider + * instead.) + * + * For account creation, the PrimaryAuthenticationProvider takes form data and + * stores some authentication details which will allow it to verify a login by + * that user in the future. This might for example involve saving it in the + * database in a table that can be joined to the user table, or sending it to + * some external service for account creation, or authenticating the user with + * some remote service and then recording that the remote identity is linked to + * the local account. + * The creation of the local user (i.e. calling User::addToDatabase()) is handled + * by AuthManager once the primary authentication provider returns a PASS + * from begin/continueAccountCreation; do not try to do it yourself. + * + * For account linking, the PrimaryAuthenticationProvider verifies the user's + * identity at some external service (typically by redirecting the user and + * asking the external service to verify) and then records which local account + * is linked to which remote accounts. It should keep track of this and be able + * to enumerate linked accounts via getAuthenticationRequests(ACTION_REMOVE). * * This interface also provides methods for changing authentication data such - * as passwords and for creating new users who can later be authenticated with - * this provider. + * as passwords, and callbacks that are invoked after login / account creation + * / account linking succeeded or failed. * * @ingroup Auth * @since 1.27 + * @see https://www.mediawiki.org/wiki/Manual:SessionManager_and_AuthManager */ interface PrimaryAuthenticationProvider extends AuthenticationProvider { /** Provider can create accounts */ @@ -93,16 +116,25 @@ interface PrimaryAuthenticationProvider extends AuthenticationProvider { /** * Post-login callback + * + * This will be called at the end of any login attempt, regardless of whether this provider was + * the one that handled it. It will not be called for unfinished login attempts that fail by + * the session timing out. + * * @param User|null $user User that was attempted to be logged in, if known. * This may become a "UserValue" in the future, or User may be refactored * into such. * @param AuthenticationResponse $response Authentication response that will be returned + * (PASS or FAIL) */ public function postAuthentication( $user, AuthenticationResponse $response ); /** * Test whether the named user exists - * @param string $username + * + * Single-sign-on providers can use this to reserve a username for autocreation. + * + * @param string $username MediaWiki username * @param int $flags Bitfield of User:READ_* constants * @return bool */ @@ -110,7 +142,11 @@ interface PrimaryAuthenticationProvider extends AuthenticationProvider { /** * Test whether the named user can authenticate with this provider - * @param string $username + * + * Should return true if the provider has any data for this user which can be used to + * authenticate it, even if the user is temporarily prevented from authentication somehow. + * + * @param string $username MediaWiki username * @return bool */ public function testUserCanAuthenticate( $username ); @@ -184,6 +220,10 @@ interface PrimaryAuthenticationProvider extends AuthenticationProvider { * If $req was returned for AuthManager::ACTION_REMOVE, the corresponding * credentials should no longer result in a successful login. * + * It can be assumed that providerAllowsAuthenticationDataChange with $checkData === true + * was called before this, and passed. This method should never fail (other than throwing an + * exception). + * * @param AuthenticationRequest $req */ public function providerChangeAuthenticationData( AuthenticationRequest $req ); @@ -249,7 +289,8 @@ interface PrimaryAuthenticationProvider extends AuthenticationProvider { * Post-creation callback * * Called after the user is added to the database, before secondary - * authentication providers are run. + * authentication providers are run. Only called if this provider was the one that issued + * a PASS. * * @param User $user User being created (has been added to the database now). * This may become a "UserValue" in the future, or User may be refactored @@ -266,7 +307,10 @@ interface PrimaryAuthenticationProvider extends AuthenticationProvider { /** * Post-creation callback * - * Called when the account creation process ends. + * This will be called at the end of any account creation attempt, regardless of whether this + * provider was the one that handled it. It will not be called if the account creation process + * results in a session timeout (possibly after a successful user creation, while a secondary + * provider is waiting for a response). * * @param User $user User that was attempted to be created. * This may become a "UserValue" in the future, or User may be refactored @@ -274,6 +318,7 @@ interface PrimaryAuthenticationProvider extends AuthenticationProvider { * @param User $creator User doing the creation. This may become a * "UserValue" in the future, or User may be refactored into such. * @param AuthenticationResponse $response Authentication response that will be returned + * (PASS or FAIL) */ public function postAccountCreation( $user, $creator, AuthenticationResponse $response ); @@ -340,10 +385,15 @@ interface PrimaryAuthenticationProvider extends AuthenticationProvider { /** * Post-link callback + * + * This will be called at the end of any account linking attempt, regardless of whether this + * provider was the one that handled it. + * * @param User $user User that was attempted to be linked. * This may become a "UserValue" in the future, or User may be refactored * into such. * @param AuthenticationResponse $response Authentication response that will be returned + * (PASS or FAIL) */ public function postAccountLink( $user, AuthenticationResponse $response ); diff --git a/includes/auth/SecondaryAuthenticationProvider.php b/includes/auth/SecondaryAuthenticationProvider.php index 1ccc9c6972..c55e65d5ac 100644 --- a/includes/auth/SecondaryAuthenticationProvider.php +++ b/includes/auth/SecondaryAuthenticationProvider.php @@ -27,16 +27,27 @@ use StatusValue; use User; /** - * A secondary authentication provider performs additional authentication steps - * after a PrimaryAuthenticationProvider has done its thing. + * A secondary provider mostly acts when the submitted authentication data has + * already been associated to a MediaWiki user account. * - * A SecondaryAuthenticationProvider is used to perform arbitrary checks on an - * authentication request after the user itself has been authenticated. For - * example, it might implement a password reset, request the second factor for - * two-factor auth, or prevent the login if the account is blocked. + * For login, a secondary provider performs additional authentication steps + * after a PrimaryAuthenticationProvider has identified which MediaWiki user is + * trying to log in. For example, it might implement a password reset, request + * the second factor for two-factor auth, or prevent the login if the account is blocked. + * + * For account creation, a secondary provider performs optional extra steps after + * a PrimaryAuthenticationProvider has created the user; for example, it can collect + * further user information such as a biography. + * + * (For account linking, secondary providers are not involved.) + * + * This interface also provides methods for changing authentication data such + * as a second-factor token, and callbacks that are invoked after login / account creation + * succeeded or failed. * * @ingroup Auth * @since 1.27 + * @see https://www.mediawiki.org/wiki/Manual:SessionManager_and_AuthManager */ interface SecondaryAuthenticationProvider extends AuthenticationProvider { @@ -75,10 +86,15 @@ interface SecondaryAuthenticationProvider extends AuthenticationProvider { /** * Post-login callback + * + * This will be called at the end of a login attempt. It will not be called for unfinished + * login attempts that fail by the session timing out. + * * @param User|null $user User that was attempted to be logged in, if known. * This may become a "UserValue" in the future, or User may be refactored * into such. * @param AuthenticationResponse $response Authentication response that will be returned + * (PASS or FAIL) */ public function postAuthentication( $user, AuthenticationResponse $response ); @@ -129,6 +145,10 @@ interface SecondaryAuthenticationProvider extends AuthenticationProvider { * If $req was returned for AuthManager::ACTION_REMOVE, the corresponding * credentials should no longer result in a successful login. * + * It can be assumed that providerAllowsAuthenticationDataChange with $checkData === true + * was called before this, and passed. This method should never fail (other than throwing an + * exception). + * * @param AuthenticationRequest $req */ public function providerChangeAuthenticationData( AuthenticationRequest $req ); @@ -151,6 +171,12 @@ interface SecondaryAuthenticationProvider extends AuthenticationProvider { /** * Start an account creation flow + * + * @note There is no guarantee this will be called in a successful account + * creation process as the user can just abandon the process at any time + * after the primary provider has issued a PASS and still have a valid + * account. Be prepared to handle any database inconsistencies that result + * from this or continueSecondaryAccountCreation() not being called. * @param User $user User being created (has been added to the database). * This may become a "UserValue" in the future, or User may be refactored * into such. @@ -167,6 +193,7 @@ interface SecondaryAuthenticationProvider extends AuthenticationProvider { /** * Continue an authentication flow + * * @param User $user User being created (has been added to the database). * This may become a "UserValue" in the future, or User may be refactored * into such. @@ -183,12 +210,18 @@ interface SecondaryAuthenticationProvider extends AuthenticationProvider { /** * Post-creation callback + * + * This will be called at the end of an account creation attempt. It will not be called if + * the account creation process results in a session timeout (possibly after a successful + * user creation, while a secondary provider is waiting for a response). + * * @param User $user User that was attempted to be created. * This may become a "UserValue" in the future, or User may be refactored * into such. * @param User $creator User doing the creation. This may become a * "UserValue" in the future, or User may be refactored into such. * @param AuthenticationResponse $response Authentication response that will be returned + * (PASS or FAIL) */ public function postAccountCreation( $user, $creator, AuthenticationResponse $response ); diff --git a/includes/session/Session.php b/includes/session/Session.php index 8fa212ebf1..31761c31e7 100644 --- a/includes/session/Session.php +++ b/includes/session/Session.php @@ -129,6 +129,11 @@ final class Session implements \Countable, \Iterator, \ArrayAccess { /** * Make this session not be persisted across requests + * + * This will remove persistence information (e.g. delete cookies) + * from the associated WebRequest(s), and delete session data in the + * backend. The session data will still be available via get() until + * the end of the request. */ public function unpersist() { $this->backend->unpersist(); @@ -603,6 +608,9 @@ final class Session implements \Countable, \Iterator, \ArrayAccess { /** * Save the session + * + * This will update the backend data and might re-persist the session + * if needed. */ public function save() { $this->backend->save(); diff --git a/includes/session/SessionBackend.php b/includes/session/SessionBackend.php index 264e1ae0ef..bf73d3fa78 100644 --- a/includes/session/SessionBackend.php +++ b/includes/session/SessionBackend.php @@ -599,7 +599,8 @@ final class SessionBackend { } /** - * Save and persist session data, unless delayed + * Save the session, unless delayed + * @see SessionBackend::save() */ private function autosave() { if ( $this->delaySave <= 0 ) { @@ -608,7 +609,12 @@ final class SessionBackend { } /** - * Save and persist session data + * Save the session + * + * Update both the backend data and the associated WebRequest(s) to + * reflect the state of the the SessionBackend. This might include + * persisting or unpersisting the session. + * * @param bool $closing Whether the session is being closed */ public function save( $closing = false ) { diff --git a/includes/session/SessionInfo.php b/includes/session/SessionInfo.php index c235861f57..287da9dde3 100644 --- a/includes/session/SessionInfo.php +++ b/includes/session/SessionInfo.php @@ -73,7 +73,8 @@ class SessionInfo { * Defaults to true. * - forceHTTPS: (bool) Whether to force HTTPS for this session * - metadata: (array) Provider metadata, to be returned by - * Session::getProviderMetadata(). + * Session::getProviderMetadata(). See SessionProvider::mergeMetadata() + * and SessionProvider::refreshSessionInfo(). * - idIsSafe: (bool) Set true if the 'id' did not come from the user. * Generally you'll use this from SessionProvider::newEmptySession(), * and not from any other method. @@ -200,7 +201,8 @@ class SessionInfo { * The normal behavior is to discard the SessionInfo if validation against * the data stored in the session store fails. If this returns true, * SessionManager will instead delete the session store data so this - * SessionInfo may still be used. + * SessionInfo may still be used. This is important for providers which use + * deterministic IDs and so cannot just generate a random new one. * * @return bool */ diff --git a/includes/session/SessionManager.php b/includes/session/SessionManager.php index 8ccb6d13d7..87fdcd359d 100644 --- a/includes/session/SessionManager.php +++ b/includes/session/SessionManager.php @@ -35,8 +35,15 @@ use WebRequest; /** * This serves as the entry point to the MediaWiki session handling system. * + * Most methods here are for internal use by session handling code. Other callers + * should only use getGlobalSession and the methods of SessionManagerInterface; + * the rest of the functionality is exposed via MediaWiki\Session\Session methods. + * + * To provide custom session handling, implement a MediaWiki\Session\SessionProvider. + * * @ingroup Session * @since 1.27 + * @see https://www.mediawiki.org/wiki/Manual:SessionManager_and_AuthManager */ final class SessionManager implements SessionManagerInterface { /** @var SessionManager|null */ @@ -819,9 +826,9 @@ final class SessionManager implements SessionManagerInterface { } /** - * Create a session corresponding to the passed SessionInfo + * Create a Session corresponding to the passed SessionInfo * @private For use by a SessionProvider that needs to specially create its - * own session. + * own Session. Most session providers won't need this. * @param SessionInfo $info * @param WebRequest $request * @return Session @@ -941,6 +948,7 @@ final class SessionManager implements SessionManagerInterface { /** * Reset the internal caching for unit testing + * @protected Unit tests only */ public static function resetCache() { if ( !defined( 'MW_PHPUNIT_TEST' ) ) { diff --git a/includes/session/SessionManagerInterface.php b/includes/session/SessionManagerInterface.php index d4e52c7a16..3ab0f43183 100644 --- a/includes/session/SessionManagerInterface.php +++ b/includes/session/SessionManagerInterface.php @@ -36,7 +36,8 @@ use WebRequest; */ interface SessionManagerInterface extends LoggerAwareInterface { /** - * Fetch the session for a request + * Fetch the session for a request (or a new empty session if none is + * attached to it) * * @note You probably want to use $request->getSession() instead. It's more * efficient and doesn't break FauxRequests or sessions that were changed @@ -52,6 +53,7 @@ interface SessionManagerInterface extends LoggerAwareInterface { /** * Fetch a session by ID + * * @param string $id * @param bool $create If no session exists for $id, try to create a new one. * May still return null if a session for $id exists but cannot be loaded. @@ -62,7 +64,7 @@ interface SessionManagerInterface extends LoggerAwareInterface { public function getSessionById( $id, $create = false, WebRequest $request = null ); /** - * Fetch a new, empty session + * Create a new, empty session * * The first provider configured that is able to provide an empty session * will be used. diff --git a/includes/session/SessionProvider.php b/includes/session/SessionProvider.php index 4d57ad9dcb..61c7500d05 100644 --- a/includes/session/SessionProvider.php +++ b/includes/session/SessionProvider.php @@ -66,13 +66,14 @@ use WebRequest; * would make sense. * * Note that many methods that are technically "cannot persist ID" could be - * turned into "can persist ID but not changing User" using a session cookie, + * turned into "can persist ID but not change User" using a session cookie, * as implemented by ImmutableSessionProviderWithCookie. If doing so, different * session cookie names should be used for different providers to avoid * collisions. * * @ingroup Session * @since 1.27 + * @see https://www.mediawiki.org/wiki/Manual:SessionManager_and_AuthManager */ abstract class SessionProvider implements SessionProviderInterface, LoggerAwareInterface { @@ -180,14 +181,23 @@ abstract class SessionProvider implements SessionProviderInterface, LoggerAwareI /** * Merge saved session provider metadata * + * This method will be used to compare the metadata returned by + * provideSessionInfo() with the saved metadata (which has been returned by + * provideSessionInfo() the last time the session was saved), and merge the two + * into the new saved metadata, or abort if the current request is not a valid + * continuation of the session. + * * The default implementation checks that anything in both arrays is * identical, then returns $providedMetadata. * * @protected For use by \MediaWiki\Session\SessionManager only * @param array $savedMetadata Saved provider metadata - * @param array $providedMetadata Provided provider metadata + * @param array $providedMetadata Provided provider metadata (from the SessionInfo) * @return array Resulting metadata - * @throws MetadataMergeException If the metadata cannot be merged + * @throws MetadataMergeException If the metadata cannot be merged. + * Such exceptions will be handled by SessionManager and are a safe way of rejecting + * a suspicious or incompatible session. The provider is expected to write an + * appropriate message to its logger. */ public function mergeMetadata( array $savedMetadata, array $providedMetadata ) { foreach ( $providedMetadata as $k => $v ) { @@ -211,7 +221,7 @@ abstract class SessionProvider implements SessionProviderInterface, LoggerAwareI * expected to write an appropriate message to its logger. * * @protected For use by \MediaWiki\Session\SessionManager only - * @param SessionInfo $info + * @param SessionInfo $info Any changes by mergeMetadata() will already be reflected here. * @param WebRequest $request * @param array|null &$metadata Provider metadata, may be altered. * @return bool Return false to reject the SessionInfo after all. @@ -420,6 +430,11 @@ abstract class SessionProvider implements SessionProviderInterface, LoggerAwareI /** * Fetch the rights allowed the user when the specified session is active. + * + * This is mainly meant for allowing the user to restrict access to the account + * by certain methods; you probably want to use this with MWGrants. The returned + * rights will be intersected with the user's actual rights. + * * @param SessionBackend $backend * @return null|string[] Allowed user rights, or null to allow all. */ -- 2.20.1