diff --git a/_config.php b/_config.php index 80a3b2e13..8aaf823c7 100644 --- a/_config.php +++ b/_config.php @@ -12,12 +12,6 @@ use SilverStripe\View\Parsers\ShortcodeParser; * Here you can make different settings for the Framework module (the core * module). * - * For example you can register the authentication methods you wish to use - * on your site, e.g. to register the OpenID authentication method type - * - * - * Authenticator::register_authenticator('OpenIDAuthenticator'); - * */ ShortcodeParser::get('default') diff --git a/_config/security.yml b/_config/security.yml index 60f128f6b..72b8cd1a4 100644 --- a/_config/security.yml +++ b/_config/security.yml @@ -1,18 +1,16 @@ -SilverStripe\Security\MemberAuthenticator\LoginForm: +--- +Name: coresecurity +--- +SilverStripe\Security\MemberAuthenticator\MemberLoginForm: required_fields: - Email - Password -SilverStripe\Security\Security: - authenticators: - default: SilverStripe\Security\MemberAuthenticator\Authenticator - cms: SilverStripe\Security\MemberAuthenticator\CMSAuthenticator - SilverStripe\Core\Injector\Injector: SilverStripe\Control\RequestProcessor: properties: filters: - - '%$SilverStripe\Security\AuthenticationRequestFilter' + - %$SilverStripe\Security\AuthenticationRequestFilter SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler: properties: SessionVariable: loggedInAs @@ -20,14 +18,15 @@ SilverStripe\Core\Injector\Injector: properties: TokenCookieName: alc_enc DeviceCookieName: alc_device - CascadeLogInTo: %$SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler + CascadeInTo: %$SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler SilverStripe\Security\IdentityStore: class: SilverStripe\Security\AuthenticationRequestFilter - + SilverStripe\Security\Security: + properties: + authenticators: + default: %$SilverStripe\Security\MemberAuthenticator\MemberAuthenticator + cms: %$SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator SilverStripe\Security\AuthenticationRequestFilter: handlers: session: SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler alc: SilverStripe\Security\MemberAuthenticator\CookieAuthenticationHandler - -SilverStripe\Security\MemberAuthenticator\CMSSecurity: - reauth_enabled: true diff --git a/src/Control/RequestHandler.php b/src/Control/RequestHandler.php index 43fdfef95..0a087835f 100644 --- a/src/Control/RequestHandler.php +++ b/src/Control/RequestHandler.php @@ -296,6 +296,20 @@ class RequestHandler extends ViewableData return null; } + /** + * @param string $link + * @return string + */ + protected function addBackURLParam($link) + { + $backURL = $this->getBackURL(); + if ($backURL) { + return Controller::join_links($link, '?BackURL=' . urlencode($backURL)); + } + + return $link; + } + /** * Given a request, and an action name, call that action name on this RequestHandler * diff --git a/src/Core/Config/Config_ForClass.php b/src/Core/Config/Config_ForClass.php index 448bd9443..b89e5007b 100644 --- a/src/Core/Config/Config_ForClass.php +++ b/src/Core/Config/Config_ForClass.php @@ -9,7 +9,7 @@ class Config_ForClass /** * @var string $class */ - public $class; + protected $class; /** * @param string|object $class diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php index 8d5f4ec6f..cef88598d 100644 --- a/src/Dev/FunctionalTest.php +++ b/src/Dev/FunctionalTest.php @@ -106,8 +106,7 @@ class FunctionalTest extends SapphireTest // basis. BasicAuth::protect_entire_site(false); - $this->session()->inst_clear('loggedInAs'); - Security::setCurrentUser(null); + $this->logOut(); SecurityToken::disable(); } @@ -412,8 +411,7 @@ class FunctionalTest extends SapphireTest $member = $this->objFromFixture('SilverStripe\\Security\\Member', $member); } - $this->session()->inst_set('loggedInAs', $member->ID); - Security::setCurrentUser($member); + $this->logIn($member); } /** @@ -422,10 +420,15 @@ class FunctionalTest extends SapphireTest */ public function logOut() { - $this->session()->inst_set('loggedInAs', null); + $this->session()->inst_clear('loggedInAs'); Security::setCurrentUser(null); } + public function logIn($member) + { + Security::setCurrentUser($member); + } + /** * Use the draft (stage) site for testing. * This is helpful if you're not testing publication functionality and don't want "stage management" cluttering diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php index 3651d6fc6..f557f5a40 100644 --- a/src/Dev/SapphireTest.php +++ b/src/Dev/SapphireTest.php @@ -25,6 +25,7 @@ use SilverStripe\Core\Resettable; use SilverStripe\i18n\i18n; use SilverStripe\ORM\DataExtension; use SilverStripe\ORM\SS_List; +use SilverStripe\Security\IdentityStore; use SilverStripe\Versioned\Versioned; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataModel; @@ -1250,7 +1251,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase $this->cache_generatedMembers[$permCode] = $member; } - Security::setCurrentUser($member); + Injector::inst()->get(IdentityStore::class)->logIn($member); return $member->ID; } diff --git a/src/Forms/FormRequestHandler.php b/src/Forms/FormRequestHandler.php index 526f488bb..f1f82606d 100644 --- a/src/Forms/FormRequestHandler.php +++ b/src/Forms/FormRequestHandler.php @@ -233,18 +233,18 @@ class FormRequestHandler extends RequestHandler // Otherwise, try a handler method on the form request handler. if ($this->hasMethod($funcName)) { - return $this->$funcName($vars, $this->form, $request); + return $this->$funcName($vars, $this->form, $request, $this); } // Otherwise, try a handler method on the form itself if ($this->form->hasMethod($funcName)) { - return $this->form->$funcName($vars, $this->form, $request); + return $this->form->$funcName($vars, $this->form, $request, $this); } // Check for inline actions $field = $this->checkFieldsForAction($this->form->Fields(), $funcName); if ($field) { - return $field->$funcName($vars, $this->form, $request); + return $field->$funcName($vars, $this->form, $request, $this); } } catch (ValidationException $e) { // The ValdiationResult contains all the relevant metadata diff --git a/src/Security/AuthenticationRequestFilter.php b/src/Security/AuthenticationRequestFilter.php index ec44fa281..743d582e3 100644 --- a/src/Security/AuthenticationRequestFilter.php +++ b/src/Security/AuthenticationRequestFilter.php @@ -2,15 +2,16 @@ namespace SilverStripe\Security; -use Exception; use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Control\RequestFilter; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\Session; +use SilverStripe\Dev\Debug; use SilverStripe\ORM\DataModel; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Injector\Injector; +use SilverStripe\ORM\ValidationException; class AuthenticationRequestFilter implements RequestFilter, IdentityStore { @@ -18,10 +19,21 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore use Configurable; /** - * @return array|IdentityStore[] + * @var array|AuthenticationHandler[] + */ + protected $handlers; + + /** + * This method currently uses a fallback as loading the handlers via YML has proven unstable + * + * @return array|AuthenticationHandler[] */ protected function getHandlers() { + if (is_array($this->handlers)) { + return $this->handlers; + } + return array_map( function ($identifier) { return Injector::inst()->get($identifier); @@ -30,24 +42,39 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore ); } + /** + * Set an associative array of handlers + * + * @param array|AuthenticationHandler[] $handlers + */ + public function setHandlers($handlers) + { + $this->handlers = $handlers; + } + /** * Identify the current user from the request + * + * @param HTTPRequest $request + * @param Session $session + * @param DataModel $model + * @return bool|void + * @throws HTTPResponse_Exception */ public function preRequest(HTTPRequest $request, Session $session, DataModel $model) { try { /** @var AuthenticationHandler $handler */ - foreach ($this->getHandlers() as $handler) { + foreach ($this->getHandlers() as $name => $handler) { // @todo Update requestfilter logic to allow modification of initial response // in order to add cookies, etc - $member = $handler->authenticateRequest($request, new HTTPResponse()); + $member = $handler->authenticateRequest($request); if ($member) { - // @todo Remove the static coupling here Security::setCurrentUser($member); break; } } - } catch (Exception $e) { // There's no valid exception currently. I would say AuthenticationException? + } catch (ValidationException $e) { throw new HTTPResponse_Exception( "Bad log-in details: " . $e->getMessage(), 400 @@ -57,6 +84,11 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore /** * No-op + * + * @param HTTPRequest $request + * @param HTTPResponse $response + * @param DataModel $model + * @return bool|void */ public function postRequest(HTTPRequest $request, HTTPResponse $response, DataModel $model) { @@ -65,11 +97,13 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore /** * Log into the identity-store handlers attached to this request filter * - * @inherit + * @param Member $member + * @param bool $persistent + * @param HTTPRequest $request + * @return HTTPResponse|void */ - public function logIn(Member $member, $persistent, HTTPRequest $request) + public function logIn(Member $member, $persistent = false, HTTPRequest $request = null) { - // @todo Coupling here isn't ideal. $member->beforeMemberLoggedIn(); foreach ($this->getHandlers() as $handler) { @@ -78,7 +112,6 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore } } - // @todo Coupling here isn't ideal. Security::setCurrentUser($member); $member->afterMemberLoggedIn(); } @@ -86,9 +119,10 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore /** * Log out of all the identity-store handlers attached to this request filter * - * @inherit + * @param HTTPRequest $request + * @return HTTPResponse|void */ - public function logOut(HTTPRequest $request) + public function logOut(HTTPRequest $request = null) { foreach ($this->getHandlers() as $handler) { if ($handler instanceof IdentityStore) { @@ -96,7 +130,6 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore } } - // @todo Coupling here isn't ideal. Security::setCurrentUser(null); } } diff --git a/src/Security/Authenticator.php b/src/Security/Authenticator.php index cc929108b..5077361da 100644 --- a/src/Security/Authenticator.php +++ b/src/Security/Authenticator.php @@ -7,6 +7,7 @@ use SilverStripe\Core\Extensible; use SilverStripe\Core\Injector\Injectable; use SilverStripe\Control\Controller; use SilverStripe\Forms\Form; +use SilverStripe\ORM\ValidationResult; /** * Abstract base class for an authentication method @@ -74,7 +75,8 @@ interface Authenticator /** - * @todo + * @param $link + * @return mixed */ public function getLostPasswordHandler($link); @@ -82,14 +84,8 @@ interface Authenticator * Method to authenticate an user. * * @param array $data Raw data to authenticate the user. - * @param string $message A variable to return an error message if authentication fails + * @param ValidationResult $result A validationresult which is either valid or contains the error message(s) * @return Member The matched member, or null if the authentication fails */ - public function authenticate($data, &$message); - - /** - * Return the keys that should be passed to authenticate() - * @return array - */ -// public function getAuthenticateFields(); + public function authenticate($data, &$result); } diff --git a/src/Security/BasicAuth.php b/src/Security/BasicAuth.php index d4235c4dd..768ad6f94 100644 --- a/src/Security/BasicAuth.php +++ b/src/Security/BasicAuth.php @@ -7,13 +7,10 @@ use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; -use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Configurable; -use SilverStripe\Dev\Debug; use SilverStripe\Dev\SapphireTest; -use SilverStripe\Core\Injector\Injector; -use SilverStripe\Security\MemberAuthenticator\Authenticator; +use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator; /** * Provides an interface to HTTP basic authentication. @@ -47,7 +44,7 @@ class BasicAuth * @var String Message that shows in the authentication box. * Set this value through {@link protect_entire_site()}. */ - private static $entire_site_protected_message = "SilverStripe test website. Use your CMS login."; + private static $entire_site_protected_message = 'SilverStripe test website. Use your CMS login.'; /** * Require basic authentication. Will request a username and password if none is given. @@ -63,9 +60,13 @@ class BasicAuth * @return bool|Member * @throws HTTPResponse_Exception */ - public static function requireLogin(HTTPRequest $request, $realm, $permissionCode = null, $tryUsingSessionLogin = true) - { - $isRunningTests = (class_exists('SilverStripe\\Dev\\SapphireTest', false) && SapphireTest::is_running_test()); + public static function requireLogin( + HTTPRequest $request, + $realm, + $permissionCode = null, + $tryUsingSessionLogin = true + ) { + $isRunningTests = (class_exists(SapphireTest::class, false) && SapphireTest::is_running_test()); if (!Security::database_is_ready() || (Director::is_cli() && !$isRunningTests)) { return true; } @@ -89,16 +90,21 @@ class BasicAuth $member = null; if ($request->getHeader('PHP_AUTH_USER') && $request->getHeader('PHP_AUTH_PW')) { - /** @var Authenticator $authenticator */ - $authenticator = Injector::inst()->get(Authenticator::class); + /** @var MemberAuthenticator $authenticator */ + $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::LOGIN); - $member = $authenticator->authenticate([ - 'Email' => $request->getHeader('PHP_AUTH_USER'), - 'Password' => $request->getHeader('PHP_AUTH_PW'), - ], $dummy); + foreach ($authenticators as $name => $authenticator) { + $member = $authenticator->authenticate([ + 'Email' => $request->getHeader('PHP_AUTH_USER'), + 'Password' => $request->getHeader('PHP_AUTH_PW'), + ]); + if ($member instanceof Member) { + break; + } + } } - if($member) { + if ($member instanceof Member) { Security::setCurrentUser($member); } @@ -112,9 +118,19 @@ class BasicAuth $response->addHeader('WWW-Authenticate', "Basic realm=\"$realm\""); if ($request->getHeader('PHP_AUTH_USER')) { - $response->setBody(_t('SilverStripe\\Security\\BasicAuth.ERRORNOTREC', "That username / password isn't recognised")); + $response->setBody( + _t( + 'SilverStripe\\Security\\BasicAuth.ERRORNOTREC', + "That username / password isn't recognised" + ) + ); } else { - $response->setBody(_t('SilverStripe\\Security\\BasicAuth.ENTERINFO', "Please enter a username and password.")); + $response->setBody( + _t( + 'SilverStripe\\Security\\BasicAuth.ENTERINFO', + 'Please enter a username and password.' + ) + ); } // Exception is caught by RequestHandler->handleRequest() and will halt further execution @@ -128,7 +144,12 @@ class BasicAuth $response->addHeader('WWW-Authenticate', "Basic realm=\"$realm\""); if ($request->getHeader('PHP_AUTH_USER')) { - $response->setBody(_t('SilverStripe\\Security\\BasicAuth.ERRORNOTADMIN', "That user is not an administrator.")); + $response->setBody( + _t( + 'SilverStripe\\Security\\BasicAuth.ERRORNOTADMIN', + 'That user is not an administrator.' + ) + ); } // Exception is caught by RequestHandler->handleRequest() and will halt further execution @@ -160,9 +181,9 @@ class BasicAuth */ public static function protect_entire_site($protect = true, $code = 'ADMIN', $message = null) { - Config::modify()->set(self::class, 'entire_site_protected', $protect); - Config::modify()->set(self::class, 'entire_site_protected_code', $code); - Config::modify()->set(self::class, 'entire_site_protected_message', $message); + static::config()->set('entire_site_protected', $protect); + static::config()->set('entire_site_protected_code', $code); + static::config()->set('entire_site_protected_message', $message); } /** @@ -174,7 +195,7 @@ class BasicAuth */ public static function protect_site_if_necessary() { - $config = Config::forClass(BasicAuth::class); + $config = static::config(); $request = Controller::curr()->getRequest(); if ($config->get('entire_site_protected')) { /** @noinspection ExceptionsAnnotatingAndHandlingInspection */ @@ -182,7 +203,8 @@ class BasicAuth $request, $config->get('entire_site_protected_message'), $config->get('entire_site_protected_code'), - false); + false + ); } } } diff --git a/src/Security/CMSMemberLoginForm.php b/src/Security/CMSMemberLoginForm.php index e71865b50..197240037 100644 --- a/src/Security/CMSMemberLoginForm.php +++ b/src/Security/CMSMemberLoginForm.php @@ -15,7 +15,7 @@ use SilverStripe\Security\Security; /** * Provides the in-cms session re-authentication form for the "member" authenticator */ -class CMSMemberLoginForm extends LoginForm +class CMSMemberLoginForm extends MemberLoginForm { /** diff --git a/src/Security/CMSSecurity.php b/src/Security/CMSSecurity.php index 1a89f1c9e..97b5ffd9a 100644 --- a/src/Security/CMSSecurity.php +++ b/src/Security/CMSSecurity.php @@ -10,7 +10,7 @@ use SilverStripe\Control\Director; use SilverStripe\Control\Controller; use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\FieldType\DBField; -use SilverStripe\Security\MemberAuthenticator\CMSAuthenticator; +use SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator; use SilverStripe\View\Requirements; /** @@ -44,7 +44,7 @@ class CMSSecurity extends Security Requirements::javascript(FRAMEWORK_ADMIN_DIR . '/client/dist/js/vendor.js'); } - public function login($request, $service = Authenticator::CMS_LOGIN) + public function login($request = null, $service = Authenticator::CMS_LOGIN) { return parent::login($request, Authenticator::CMS_LOGIN); } @@ -60,9 +60,9 @@ class CMSSecurity extends Security return parent::getAuthenticator($name); } - public static function getAuthenticators($service = Authenticator::CMS_LOGIN) + public function getApplicableAuthenticators($service = Authenticator::CMS_LOGIN) { - return parent::getAuthenticators($service); + return parent::getApplicableAuthenticators($service); } /** @@ -97,7 +97,7 @@ class CMSSecurity extends Security public function getTitle() { // Check if logged in already - if (Member::currentUserID()) { + if (Security::getCurrentUser()) { return _t('SilverStripe\\Security\\CMSSecurity.SUCCESS', 'Success'); } @@ -174,19 +174,7 @@ PHP return false; } - /** @var [] $authenticators */ - $authenticators = Security::config()->get('authenticators'); - foreach ($authenticators as $name => $authenticator) { - // Supported if at least one authenticator is supported - $authenticator = Injector::inst()->get($authenticator); - if (($authenticator->supportedServices() & Authenticator::CMS_LOGIN) - && Security::hasAuthenticator($name) - ) { - return true; - } - } - - return false; + return count(Security::singleton()->getApplicableAuthenticators(Authenticator::CMS_LOGIN)) > 0; } /** @@ -197,7 +185,7 @@ PHP public function success() { // Ensure member is properly logged in - if (!Member::currentUserID() || !class_exists(AdminRootController::class)) { + if (!Security::getCurrentUser() || !class_exists(AdminRootController::class)) { return $this->redirectToExternalLogin(); } diff --git a/src/Security/IdentityStore.php b/src/Security/IdentityStore.php index f39688106..259db694b 100644 --- a/src/Security/IdentityStore.php +++ b/src/Security/IdentityStore.php @@ -4,6 +4,7 @@ namespace SilverStripe\Security; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; +use SilverStripe\Core\Config\Configurable; /** * Represents an authentication handler that can have identities logged into & out of it. @@ -18,15 +19,15 @@ interface IdentityStore * @param Member $member The member to log in. * @param Boolean $persistent boolean If set to true, the login may persist beyond the current session. * @param HTTPRequest $request The request of the visitor that is logging in, to get, for example, cookies. - * @param HTTPResponse $response The response object to modify, if needed. + * @return HTTPResponse $response The response object to modify, if needed. */ - public function logIn(Member $member, $persistent, HTTPRequest $request); + public function logIn(Member $member, $persistent = false, HTTPRequest $request = null); /** * Log any logged-in member out of this identity store. * * @param HTTPRequest $request The request of the visitor that is logging out, to get, for example, cookies. - * @param HTTPResponse $response The response object to modify, if needed. + * @return HTTPResponse $response The response object to modify, if needed. */ - public function logOut(HTTPRequest $request); + public function logOut(HTTPRequest $request = null); } diff --git a/src/Security/Member.php b/src/Security/Member.php index 6aa638a23..21a5439e7 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -5,6 +5,7 @@ namespace SilverStripe\Security; use IntlDateFormatter; use SilverStripe\Admin\LeftAndMain; use SilverStripe\CMS\Controllers\CMSMain; +use SilverStripe\Control\Controller; use SilverStripe\Control\Cookie; use SilverStripe\Control\Director; use SilverStripe\Control\Email\Email; @@ -12,6 +13,8 @@ use SilverStripe\Control\Email\Mailer; use SilverStripe\Control\Session; use SilverStripe\Core\Convert; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Dev\Debug; +use SilverStripe\Dev\Deprecation; use SilverStripe\Dev\SapphireTest; use SilverStripe\Dev\TestMailer; use SilverStripe\Forms\ConfirmedPasswordField; @@ -22,6 +25,7 @@ use SilverStripe\Forms\ListboxField; use SilverStripe\i18n\i18n; use SilverStripe\MSSQL\MSSQLDatabase; use SilverStripe\ORM\ArrayList; +use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBDatetime; @@ -61,25 +65,25 @@ class Member extends DataObject { private static $db = array( - 'FirstName' => 'Varchar', - 'Surname' => 'Varchar', - 'Email' => 'Varchar(254)', // See RFC 5321, Section 4.5.3.1.3. (256 minus the < and > character) - 'TempIDHash' => 'Varchar(160)', // Temporary id used for cms re-authentication - 'TempIDExpired' => 'Datetime', // Expiry of temp login - 'Password' => 'Varchar(160)', - 'AutoLoginHash' => 'Varchar(160)', // Used to auto-login the user on password reset - 'AutoLoginExpired' => 'Datetime', + 'FirstName' => 'Varchar', + 'Surname' => 'Varchar', + 'Email' => 'Varchar(254)', // See RFC 5321, Section 4.5.3.1.3. (256 minus the < and > character) + 'TempIDHash' => 'Varchar(160)', // Temporary id used for cms re-authentication + 'TempIDExpired' => 'Datetime', // Expiry of temp login + 'Password' => 'Varchar(160)', + 'AutoLoginHash' => 'Varchar(160)', // Used to auto-login the user on password reset + 'AutoLoginExpired' => 'Datetime', // This is an arbitrary code pointing to a PasswordEncryptor instance, // not an actual encryption algorithm. // Warning: Never change this field after its the first password hashing without // providing a new cleartext password as well. 'PasswordEncryption' => "Varchar(50)", - 'Salt' => 'Varchar(50)', - 'PasswordExpiry' => 'Date', - 'LockedOutUntil' => 'Datetime', - 'Locale' => 'Varchar(6)', + 'Salt' => 'Varchar(50)', + 'PasswordExpiry' => 'Date', + 'LockedOutUntil' => 'Datetime', + 'Locale' => 'Varchar(6)', // handled in registerFailedLogin(), only used if $lock_out_after_incorrect_logins is set - 'FailedLoginCount' => 'Int', + 'FailedLoginCount' => 'Int', ); private static $belongs_many_many = array( @@ -87,7 +91,7 @@ class Member extends DataObject ); private static $has_many = array( - 'LoggedPasswords' => MemberPassword::class, + 'LoggedPasswords' => MemberPassword::class, 'RememberLoginHashes' => RememberLoginHash::class, ); @@ -191,6 +195,12 @@ class Member extends DataObject */ private static $password_expiry_days = null; + /** + * @config + * @var bool enable or disable logging of previously used passwords. See {@link onAfterWrite} + */ + private static $password_logging_enabled = true; + /** * @config * @var Int Number of incorrect logins after which @@ -276,7 +286,7 @@ class Member extends DataObject // Find member /** @skipUpgrade */ - $admin = Member::get() + $admin = static::get() ->filter('Email', Security::default_admin_username()) ->first(); if (!$admin) { @@ -284,7 +294,7 @@ class Member extends DataObject // persistent logins in the database. See Security::setDefaultAdmin(). // Set 'Email' to identify this as the default admin $admin = Member::create(); - $admin->FirstName = _t(__CLASS__.'.DefaultAdminFirstname', 'Default Admin'); + $admin->FirstName = _t(__CLASS__ . '.DefaultAdminFirstname', 'Default Admin'); $admin->Email = Security::default_admin_username(); $admin->write(); } @@ -323,14 +333,15 @@ class Member extends DataObject // Check a password is set on this member if (empty($this->Password) && $this->exists()) { - $result->addError(_t(__CLASS__.'.NoPassword', 'There is no password on this member.')); + $result->addError(_t(__CLASS__ . '.NoPassword', 'There is no password on this member.')); + return $result; } $e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption); if (!$e->check($this->Password, $password, $this->Salt, $this)) { $result->addError(_t( - __CLASS__.'.ERRORWRONGCRED', + __CLASS__ . '.ERRORWRONGCRED', 'The provided details don\'t seem to be correct. Please try again.' )); } @@ -364,7 +375,7 @@ class Member extends DataObject if ($this->isLockedOut()) { $result->addError( _t( - __CLASS__.'.ERRORLOCKEDOUT2', + __CLASS__ . '.ERRORLOCKEDOUT2', 'Your account has been temporarily disabled because of too many failed attempts at ' . 'logging in. Please try again in {count} minutes.', null, @@ -374,6 +385,7 @@ class Member extends DataObject } $this->extend('canLogIn', $result); + return $result; } @@ -382,11 +394,12 @@ class Member extends DataObject * * @return bool */ - protected function isLockedOut() + public function isLockedOut() { if (!$this->LockedOutUntil) { return false; } + return DBDatetime::now()->getTimestamp() < $this->dbObject('LockedOutUntil')->getTimestamp(); } @@ -416,17 +429,20 @@ class Member extends DataObject if (!$this->PasswordExpiry) { return false; } + return strtotime(date('Y-m-d')) >= strtotime($this->PasswordExpiry); } /** - * @deprecated Use Security::setCurrentUser() or IdentityStore::logIn() + * @deprecated 5.0.0 Use Security::setCurrentUser() or IdentityStore::logIn() * - * @param bool $remember If set to TRUE, the member will be logged in automatically the next time. */ public function logIn() { - user_error("This method is deprecated and now only logs in for the current request", E_USER_WARNING); + Deprecation::notice( + '5.0.0', + 'This method is deprecated and only logs in for the current request. Please use Security::setCurrentUser($user) or an IdentityStore' + ); Security::setCurrentUser($this); } @@ -478,15 +494,20 @@ class Member extends DataObject * has a database record of the same ID. If there is * no logged in user, FALSE is returned anyway. * + * @deprecated Not needed anymore, as it returns Security::getCurrentUser(); + * * @return boolean TRUE record found FALSE no record found */ public static function logged_in_session_exists() { - if ($id = Member::currentUserID()) { - if ($member = DataObject::get_by_id(Member::class, $id)) { - if ($member->exists()) { - return true; - } + Deprecation::notice( + '5.0.0', + 'This method is deprecated and now does not add value. Please use Security::getCurrentUser()' + ); + + if ($member = Security::getCurrentUser()) { + if ($member && $member->exists()) { + return true; } } @@ -494,37 +515,21 @@ class Member extends DataObject } /** + * @deprecated Use Security::setCurrentUser(null) or an IdentityStore * Logs this member out. */ public function logOut() { + Deprecation::notice( + '5.0.0', + 'This method is deprecated and now does not persist. Please use Security::setCurrentUser(null) or an IdenityStore' + ); + $this->extend('beforeMemberLoggedOut'); - Session::clear("loggedInAs"); - if (Member::config()->login_marker_cookie) { - Cookie::set(Member::config()->login_marker_cookie, null, 0); - } - - Session::destroy(); - - $this->extend('memberLoggedOut'); - - // Clears any potential previous hashes for this member - RememberLoginHash::clear($this, Cookie::get('alc_device')); - - Cookie::set('alc_enc', null); // // Clear the Remember Me cookie - Cookie::force_expiry('alc_enc'); - Cookie::set('alc_device', null); - Cookie::force_expiry('alc_device'); - - // Switch back to live in order to avoid infinite loops when - // redirecting to the login screen (if this login screen is versioned) - Session::clear('readingMode'); - - $this->write(); - + Injector::inst()->get(IdentityStore::class)->logOut(Controller::curr()->getRequest()); // Audit logging hook - $this->extend('memberLoggedOut'); + $this->extend('afterMemberLoggedOut'); } /** @@ -548,6 +553,7 @@ class Member extends DataObject // We assume we have PasswordEncryption and Salt available here. $e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption); + return $e->encrypt($string, $this->Salt); } @@ -590,6 +596,7 @@ class Member extends DataObject { $hash = $this->encryptWithUserSettings($autologinToken); $member = self::member_from_autologinhash($hash, false); + return (bool)$member; } @@ -605,13 +612,13 @@ class Member extends DataObject public static function member_from_autologinhash($hash, $login = false) { /** @var Member $member */ - $member = Member::get()->filter([ - 'AutoLoginHash' => $hash, + $member = static::get()->filter([ + 'AutoLoginHash' => $hash, 'AutoLoginExpired:GreaterThan' => DBDatetime::now()->getValue(), ])->first(); if ($login && $member) { - $member->logIn(); + Injector::inst()->get(IdentityStore::class)->logIn($member); } return $member; @@ -625,11 +632,12 @@ class Member extends DataObject */ public static function member_from_tempid($tempid) { - $members = Member::get() + $members = static::get() ->filter('TempIDHash', $tempid); // Exclude expired if (static::config()->get('temp_id_lifetime')) { + /** @var DataList|Member[] $members */ $members = $members->filter('TempIDExpired:GreaterThan', DBDatetime::now()->getValue()); } @@ -640,6 +648,8 @@ class Member extends DataObject * Returns the fields for the member form - used in the registration/profile module. * It should return fields that are editable by the admin and the logged-in user. * + * @todo possibly move this to an extension + * * @return FieldList Returns a {@link FieldList} containing the fields for * the member form. */ @@ -660,6 +670,7 @@ class Member extends DataObject $this->extend('updateMemberFormFields', $fields); + return $fields; } @@ -672,7 +683,7 @@ class Member extends DataObject { $editingPassword = $this->isInDB(); $label = $editingPassword - ? _t(__CLASS__.'.EDIT_PASSWORD', 'New Password') + ? _t(__CLASS__ . '.EDIT_PASSWORD', 'New Password') : $this->fieldLabel('Password'); /** @var ConfirmedPasswordField $password */ $password = ConfirmedPasswordField::create( @@ -684,12 +695,13 @@ class Member extends DataObject ); // If editing own password, require confirmation of existing - if ($editingPassword && $this->ID == Member::currentUserID()) { + if ($editingPassword && $this->ID == Security::getCurrentUser()->ID) { $password->setRequireExistingPassword(true); } $password->setCanBeEmpty(true); $this->extend('updateMemberPasswordField', $password); + return $password; } @@ -717,12 +729,17 @@ class Member extends DataObject /** * Returns the current logged in user * - * @deprecated use Security::getCurrentUser() + * @deprecated 5.0.0 use Security::getCurrentUser() * * @return Member */ public static function currentUser() { + Deprecation::notice( + '5.0.0', + 'This method is deprecated. Please use Security::getCurrentUser() or an IdentityStore' + ); + return Security::getCurrentUser(); } @@ -761,12 +778,17 @@ class Member extends DataObject /** * Get the ID of the current logged in user * - * @deprecated use Security::getCurrentUser() + * @deprecated 5.0.0 use Security::getCurrentUser() * * @return int Returns the ID of the current logged in user or 0. */ public static function currentUserID() { + Deprecation::notice( + '5.0.0', + 'This method is deprecated. Please use Security::getCurrentUser() or an IdentityStore' + ); + if ($member = Security::getCurrentUser()) { return $member->ID; } else { @@ -774,12 +796,12 @@ class Member extends DataObject } } - /* - * Generate a random password, with randomiser to kick in if there's no words file on the - * filesystem. - * - * @return string Returns a random password. - */ + /** + * Generate a random password, with randomiser to kick in if there's no words file on the + * filesystem. + * + * @return string Returns a random password. + */ public static function create_new_password() { $words = Security::config()->uninherited('word_list'); @@ -788,16 +810,17 @@ class Member extends DataObject $words = file($words); list($usec, $sec) = explode(' ', microtime()); - srand($sec + ((float) $usec * 100000)); + mt_srand($sec + ((float)$usec * 100000)); - $word = trim($words[rand(0, sizeof($words)-1)]); - $number = rand(10, 999); + $word = trim($words[random_int(0, count($words) - 1)]); + $number = random_int(10, 999); return $word . $number; } else { - $random = rand(); + $random = mt_rand(); $string = md5($random); $output = substr($string, 0, 8); + return $output; } } @@ -827,12 +850,12 @@ class Member extends DataObject if ($existingRecord) { throw new ValidationException(_t( - __CLASS__.'.ValidationIdentifierFailed', + __CLASS__ . '.ValidationIdentifierFailed', 'Can\'t overwrite existing member #{id} with identical identifier ({name} = {value}))', 'Values in brackets show "fieldname = value", usually denoting an existing email address', array( - 'id' => $existingRecord->ID, - 'name' => $identifierField, + 'id' => $existingRecord->ID, + 'name' => $identifierField, 'value' => $this->$identifierField ) )); @@ -841,6 +864,7 @@ class Member extends DataObject // We don't send emails out on dev/tests sites to prevent accidentally spamming users. // However, if TestMailer is in use this isn't a risk. + // @todo some developers use external tools, so emailing might be a good idea anyway if ((Director::isLive() || Injector::inst()->get(Mailer::class) instanceof TestMailer) && $this->isChanged('Password') && $this->record['Password'] @@ -850,7 +874,7 @@ class Member extends DataObject ->setHTMLTemplate('SilverStripe\\Control\\Email\\ChangePasswordEmail') ->setData($this) ->setTo($this->Email) - ->setSubject(_t(__CLASS__.'.SUBJECTPASSWORDCHANGED', "Your password has been changed", 'Email subject')) + ->setSubject(_t(__CLASS__ . '.SUBJECTPASSWORDCHANGED', "Your password has been changed", 'Email subject')) ->send(); } @@ -858,7 +882,7 @@ class Member extends DataObject // Note that this only works with cleartext passwords, as we can't rehash // existing passwords. if ((!$this->ID && $this->Password) || $this->isChanged('Password')) { - //reset salt so that it gets regenerated - this will invalidate any persistant login cookies + //reset salt so that it gets regenerated - this will invalidate any persistent login cookies // or other information encrypted with this Member's settings (see self::encryptWithUserSettings) $this->Salt = ''; // Password was changed: encrypt the password according the settings @@ -878,8 +902,8 @@ class Member extends DataObject // If we haven't manually set a password expiry if (!$this->isChanged('PasswordExpiry')) { // then set it for us - if (self::config()->password_expiry_days) { - $this->PasswordExpiry = date('Y-m-d', time() + 86400 * self::config()->password_expiry_days); + if (static::config()->get('password_expiry_days')) { + $this->PasswordExpiry = date('Y-m-d', time() + 86400 * static::config()->get('password_expiry_days')); } else { $this->PasswordExpiry = null; } @@ -900,7 +924,7 @@ class Member extends DataObject Permission::reset(); - if ($this->isChanged('Password')) { + if ($this->isChanged('Password') && static::config()->get('password_logging_enabled')) { MemberPassword::log($this); } } @@ -924,6 +948,7 @@ class Member extends DataObject $password->delete(); $password->destroy(); } + return $this; } @@ -942,9 +967,10 @@ class Member extends DataObject } // If there are no admin groups in this set then it's ok - $adminGroups = Permission::get_groups_by_permission('ADMIN'); - $adminGroupIDs = ($adminGroups) ? $adminGroups->column('ID') : array(); - return count(array_intersect($ids, $adminGroupIDs)) == 0; + $adminGroups = Permission::get_groups_by_permission('ADMIN'); + $adminGroupIDs = ($adminGroups) ? $adminGroups->column('ID') : array(); + + return count(array_intersect($ids, $adminGroupIDs)) == 0; } @@ -1081,6 +1107,7 @@ class Member extends DataObject foreach ($format['columns'] as $col) { $values[] = $this->getField($col); } + return implode($format['sep'], $values); } if ($this->getField('ID') === 0) { @@ -1114,17 +1141,18 @@ class Member extends DataObject if (!$format) { $format = [ 'columns' => ['Surname', 'FirstName'], - 'sep' => ' ', + 'sep' => ' ', ]; } - $columnsWithTablename = array(); + $columnsWithTablename = array(); foreach ($format['columns'] as $column) { $columnsWithTablename[] = static::getSchema()->sqlColumnForField(__CLASS__, $column); } $sepSQL = Convert::raw2sql($format['sep'], true); - return "(".join(" $op $sepSQL $op ", $columnsWithTablename).")"; + + return "(" . join(" $op $sepSQL $op ", $columnsWithTablename) . ")"; } @@ -1195,6 +1223,7 @@ class Member extends DataObject if ($locale) { return $locale; } + return i18n::get_locale(); } @@ -1271,7 +1300,7 @@ class Member extends DataObject // No groups, return all Members if (!$groupIDList) { - return Member::get()->sort(array('Surname'=>'ASC', 'FirstName'=>'ASC'))->map(); + return static::get()->sort(array('Surname' => 'ASC', 'FirstName' => 'ASC'))->map(); } $membersList = new ArrayList(); @@ -1281,6 +1310,7 @@ class Member extends DataObject } $membersList->removeDuplicates('ID'); + return $membersList->map(); } @@ -1335,7 +1365,7 @@ class Member extends DataObject } /** @skipUpgrade */ - $members = Member::get() + $members = static::get() ->innerJoin("Group_Members", '"Group_Members"."MemberID" = "Member"."ID"') ->innerJoin("Group", '"Group"."ID" = "Group_Members"."GroupID"'); if ($groupIDList) { @@ -1395,12 +1425,12 @@ class Member extends DataObject $mainFields->replaceField('Locale', new DropdownField( "Locale", - _t(__CLASS__.'.INTERFACELANG', "Interface Language", 'Language of the CMS'), + _t(__CLASS__ . '.INTERFACELANG', "Interface Language", 'Language of the CMS'), i18n::getSources()->getKnownLocales() )); $mainFields->removeByName(static::config()->get('hidden_fields')); - if (! static::config()->get('lock_out_after_incorrect_logins')) { + if (!static::config()->get('lock_out_after_incorrect_logins')) { $mainFields->removeByName('FailedLoginCount'); } @@ -1426,7 +1456,7 @@ class Member extends DataObject ->setSource($groupsMap) ->setAttribute( 'data-placeholder', - _t(__CLASS__.'.ADDGROUP', 'Add group', 'Placeholder text for a dropdown') + _t(__CLASS__ . '.ADDGROUP', 'Add group', 'Placeholder text for a dropdown') ) ); @@ -1465,21 +1495,22 @@ class Member extends DataObject { $labels = parent::fieldLabels($includerelations); - $labels['FirstName'] = _t(__CLASS__.'.FIRSTNAME', 'First Name'); - $labels['Surname'] = _t(__CLASS__.'.SURNAME', 'Surname'); + $labels['FirstName'] = _t(__CLASS__ . '.FIRSTNAME', 'First Name'); + $labels['Surname'] = _t(__CLASS__ . '.SURNAME', 'Surname'); /** @skipUpgrade */ - $labels['Email'] = _t(__CLASS__.'.EMAIL', 'Email'); - $labels['Password'] = _t(__CLASS__.'.db_Password', 'Password'); - $labels['PasswordExpiry'] = _t(__CLASS__.'.db_PasswordExpiry', 'Password Expiry Date', 'Password expiry date'); - $labels['LockedOutUntil'] = _t(__CLASS__.'.db_LockedOutUntil', 'Locked out until', 'Security related date'); - $labels['Locale'] = _t(__CLASS__.'.db_Locale', 'Interface Locale'); + $labels['Email'] = _t(__CLASS__ . '.EMAIL', 'Email'); + $labels['Password'] = _t(__CLASS__ . '.db_Password', 'Password'); + $labels['PasswordExpiry'] = _t(__CLASS__ . '.db_PasswordExpiry', 'Password Expiry Date', 'Password expiry date'); + $labels['LockedOutUntil'] = _t(__CLASS__ . '.db_LockedOutUntil', 'Locked out until', 'Security related date'); + $labels['Locale'] = _t(__CLASS__ . '.db_Locale', 'Interface Locale'); if ($includerelations) { $labels['Groups'] = _t( - __CLASS__.'.belongs_many_many_Groups', + __CLASS__ . '.belongs_many_many_Groups', 'Groups', 'Security Groups this member belongs to' ); } + return $labels; } @@ -1511,6 +1542,7 @@ class Member extends DataObject if ($this->ID == $member->ID) { return true; } + //standard check return Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin'); } @@ -1547,9 +1579,11 @@ class Member extends DataObject if ($this->ID == $member->ID) { return true; } + //standard check return Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin'); } + /** * Users can edit their own record. * Otherwise they'll need ADMIN or CMS_ACCESS_SecurityAdmin permissions @@ -1582,10 +1616,11 @@ class Member extends DataObject // this is a hack because what this should do is to stop a user // deleting a member who has more privileges (e.g. a non-Admin deleting an Admin) if (Permission::checkMember($this, 'ADMIN')) { - if (! Permission::checkMember($member, 'ADMIN')) { + if (!Permission::checkMember($member, 'ADMIN')) { return false; } } + //standard check return Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin'); } @@ -1644,7 +1679,7 @@ class Member extends DataObject if ($this->FailedLoginCount >= self::config()->lock_out_after_incorrect_logins) { $lockoutMins = self::config()->lock_out_delay_mins; - $this->LockedOutUntil = date('Y-m-d H:i:s', DBDatetime::now()->getTimestamp() + $lockoutMins*60); + $this->LockedOutUntil = date('Y-m-d H:i:s', DBDatetime::now()->getTimestamp() + $lockoutMins * 60); $this->FailedLoginCount = 0; } } diff --git a/src/Security/MemberAuthenticator/CMSAuthenticator.php b/src/Security/MemberAuthenticator/CMSMemberAuthenticator.php similarity index 66% rename from src/Security/MemberAuthenticator/CMSAuthenticator.php rename to src/Security/MemberAuthenticator/CMSMemberAuthenticator.php index 5d3faf217..5f3cd3403 100644 --- a/src/Security/MemberAuthenticator/CMSAuthenticator.php +++ b/src/Security/MemberAuthenticator/CMSMemberAuthenticator.php @@ -2,10 +2,11 @@ namespace SilverStripe\Security\MemberAuthenticator; +use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Authenticator as BaseAuthenticator; use SilverStripe\Security\Member; -class CMSAuthenticator extends Authenticator +class CMSMemberAuthenticator extends MemberAuthenticator { public function supportedServices() @@ -15,11 +16,11 @@ class CMSAuthenticator extends Authenticator /** * @param array $data - * @param $message - * @param bool $success + * @param ValidationResult|null $result + * @param Member|null $member * @return Member */ - protected function authenticateMember($data, &$message, &$success, $member = null) + protected function authenticateMember($data, &$result = null, $member = null) { // Attempt to identify by temporary ID if (!empty($data['tempid'])) { @@ -30,9 +31,13 @@ class CMSAuthenticator extends Authenticator } } - return parent::authenticateMember($data, $message, $success, $member); + return parent::authenticateMember($data, $result, $member); } + /** + * @param string $link + * @return CMSLoginHandler + */ public function getLoginHandler($link) { return CMSLoginHandler::create($link, $this); diff --git a/src/Security/MemberAuthenticator/ChangePasswordForm.php b/src/Security/MemberAuthenticator/ChangePasswordForm.php index c0eb829de..155ae00b4 100644 --- a/src/Security/MemberAuthenticator/ChangePasswordForm.php +++ b/src/Security/MemberAuthenticator/ChangePasswordForm.php @@ -31,36 +31,51 @@ class ChangePasswordForm extends Form $backURL = $controller->getBackURL() ?: Session::get('BackURL'); if (!$fields) { - $fields = new FieldList(); - - // Security/changepassword?h=XXX redirects to Security/changepassword - // without GET parameter to avoid potential HTTP referer leakage. - // In this case, a user is not logged in, and no 'old password' should be necessary. - if (Security::getCurrentUser()) { - $fields->push(new PasswordField("OldPassword", _t('SilverStripe\\Security\\Member.YOUROLDPASSWORD', "Your old password"))); - } - - $fields->push(new PasswordField("NewPassword1", _t('SilverStripe\\Security\\Member.NEWPASSWORD', "New Password"))); - $fields->push(new PasswordField("NewPassword2", _t('SilverStripe\\Security\\Member.CONFIRMNEWPASSWORD', "Confirm New Password"))); + $fields = $this->getFormFields(); } if (!$actions) { - $actions = new FieldList( - new FormAction("doChangePassword", _t('SilverStripe\\Security\\Member.BUTTONCHANGEPASSWORD', "Change Password")) - ); + $actions = $this->getFormActions(); } if ($backURL) { - $fields->push(new HiddenField('BackURL', false, $backURL)); + $fields->push(HiddenField::create('BackURL', false, $backURL)); } parent::__construct($controller, $name, $fields, $actions); } /** - * @return ChangePasswordHandler + * @return FieldList */ - protected function buildRequestHandler() + protected function getFormFields() { - return ChangePasswordHandler::create($this); + $fields = FieldList::create(); + + // Security/changepassword?h=XXX redirects to Security/changepassword + // without GET parameter to avoid potential HTTP referer leakage. + // In this case, a user is not logged in, and no 'old password' should be necessary. + if (Security::getCurrentUser()) { + $fields->push(PasswordField::create('OldPassword', _t('SilverStripe\\Security\\Member.YOUROLDPASSWORD', 'Your old password'))); + } + + $fields->push(PasswordField::create('NewPassword1', _t('SilverStripe\\Security\\Member.NEWPASSWORD', 'New Password'))); + $fields->push(PasswordField::create('NewPassword2', _t('SilverStripe\\Security\\Member.CONFIRMNEWPASSWORD', 'Confirm New Password'))); + + return $fields; + } + + /** + * @return FieldList + */ + protected function getFormActions() + { + $actions = FieldList::create( + FormAction::create( + 'doChangePassword', + _t('SilverStripe\\Security\\Member.BUTTONCHANGEPASSWORD', 'Change Password') + ) + ); + + return $actions; } } diff --git a/src/Security/MemberAuthenticator/ChangePasswordHandler.php b/src/Security/MemberAuthenticator/ChangePasswordHandler.php index 2f87eb74d..0bc6e8369 100644 --- a/src/Security/MemberAuthenticator/ChangePasswordHandler.php +++ b/src/Security/MemberAuthenticator/ChangePasswordHandler.php @@ -3,34 +3,226 @@ namespace SilverStripe\Security\MemberAuthenticator; +use SilverStripe\Control\Controller; +use SilverStripe\Control\RequestHandler; use SilverStripe\Core\Injector\Injector; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\Session; -use SilverStripe\Forms\FormRequestHandler; +use SilverStripe\ORM\FieldType\DBDatetime; +use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\ORM\FieldType\DBHTMLText; +use SilverStripe\Security\Authenticator; +use SilverStripe\Security\CMSSecurity; use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\Security\IdentityStore; -class ChangePasswordHandler extends FormRequestHandler +class ChangePasswordHandler extends RequestHandler { + /** + * @var Authenticator + */ + protected $authenticator; + + /** + * @var string + */ + protected $link; + + /** + * @var array Allowed Actions + */ + private static $allowed_actions = [ + 'changepassword', + 'changePasswordForm', + ]; + + /** + * @var array URL Handlers. All should point to changepassword + */ + private static $url_handlers = [ + '' => 'changepassword', + ]; + + /** + * @param string $link The URL to recreate this request handler + * @param MemberAuthenticator $authenticator + */ + public function __construct($link, MemberAuthenticator $authenticator) + { + $this->link = $link; + $this->authenticator = $authenticator; + parent::__construct(); + } + + /** + * Handle the change password request + * @todo this could use some spring cleaning + * + * @return HTTPResponse|DBHTMLText + */ + public function changepassword() + { + $request = $this->getRequest(); + + // Extract the member from the URL. + /** @var Member $member */ + $member = null; + if ($request->getVar('m') !== null) { + $member = Member::get()->filter(['ID' => (int)$request->getVar('m')])->first(); + } + $token = $request->getVar('t'); + + // Check whether we are merely changin password, or resetting. + if ($token !== null && $member && $member->validateAutoLoginToken($token)) { + $this->setSessionToken($member, $token); + + // Redirect to myself, but without the hash in the URL + return $this->redirect($this->link); + } + + if (Session::get('AutoLoginHash')) { + $message = DBField::create_field( + 'HTMLFragment', + '

' . _t( + 'SilverStripe\\Security\\Security.ENTERNEWPASSWORD', + 'Please enter a new password.' + ) . '

' + ); + + // Subsequent request after the "first load with hash" (see previous if clause). + return $this->buildResponse($message); + } + + if (Security::getCurrentUser()) { + // Logged in user requested a password change form. + $message = DBField::create_field( + 'HTMLFragment', + '

' . _t( + 'SilverStripe\\Security\\Security.CHANGEPASSWORDBELOW', + 'You can change your password below.' + ) . '

' + ); + + return $this->buildResponse($message); + } + // Show a friendly message saying the login token has expired + if ($token !== null && $member && !$member->validateAutoLoginToken($token)) { + $customisedController = Controller::curr()->customise( + array( + 'Content' => DBField::create_field( + 'HTMLFragment', + _t( + 'SilverStripe\\Security\\Security.NOTERESETLINKINVALID', + '

The password reset link is invalid or expired.

' + . '

You can request a new one here or change your password after' + . ' you logged in.

', + [ + 'link1' => $this->Link('lostpassword'), + 'link2' => $this->Link('login') + ] + ) + ) + ) + ); + + return $customisedController->renderWith('changepassword'); + } + + // Someone attempted to go to changepassword without token or being logged in + return Security::permissionFailure( + Controller::curr(), + _t( + 'SilverStripe\\Security\\Security.ERRORPASSWORDPERMISSION', + 'You must be logged in in order to change your password!' + ) + ); + } + + /** + * @param DBField $message + * @return DBHTMLText + */ + protected function buildResponse($message) + { + $customisedController = Controller::curr()->customise( + [ + 'Content' => $message, + 'Form' => $this->changePasswordForm() + ] + ); + + return $customisedController->renderWith(Security::singleton()->getTemplatesFor('changepassword')); + } + + /** + * @param Member $member + * @param string $token + */ + protected function setSessionToken($member, $token) + { + // if there is a current member, they should be logged out + if ($curMember = Security::getCurrentUser()) { + /** @var LogoutHandler $handler */ + Injector::inst()->get(IdentityStore::class)->logOut(); + } + + // Store the hash for the change password form. Will be unset after reload within the ChangePasswordForm. + Session::set('AutoLoginHash', $member->encryptWithUserSettings($token)); + } + + /** + * Return a link to this request handler. + * The link returned is supplied in the constructor + * @param null $action + * @return string + */ + public function link($action = null) + { + if ($action) { + return Controller::join_links($this->link, $action); + } + + return $this->link; + } + + /** + * Factory method for the lost password form + * + * @skipUpgrade + * @return ChangePasswordForm Returns the lost password form + */ + public function changePasswordForm() + { + return ChangePasswordForm::create( + $this, + 'ChangePasswordForm' + ); + } + /** * Change the password * * @param array $data The user submitted data * @return HTTPResponse */ - public function doChangePassword(array $data, $form) + public function doChangePassword(array $data) { $member = Security::getCurrentUser(); // The user was logged in, check the current password if ($member && ( - empty($data['OldPassword']) || - !$member->checkPassword($data['OldPassword'])->isValid() - )) { + empty($data['OldPassword']) || + !$member->checkPassword($data['OldPassword'])->isValid() + ) + ) { $this->form->sessionMessage( - _t('SilverStripe\\Security\\Member.ERRORPASSWORDNOTMATCH', "Your current password does not match, please try again"), + _t( + 'SilverStripe\\Security\\Member.ERRORPASSWORDNOTMATCH', + "Your current password does not match, please try again" + ), "bad" ); + // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. return $this->redirectBackToForm(); } @@ -43,6 +235,7 @@ class ChangePasswordHandler extends FormRequestHandler // The user is not logged in and no valid auto login hash is available if (!$member) { Session::clear('AutoLoginHash'); + return $this->redirect($this->addBackURLParam(Security::singleton()->Link('login'))); } } @@ -50,7 +243,10 @@ class ChangePasswordHandler extends FormRequestHandler // Check the new password if (empty($data['NewPassword1'])) { $this->form->sessionMessage( - _t('SilverStripe\\Security\\Member.EMPTYNEWPASSWORD', "The new password can't be empty, please try again"), + _t( + 'SilverStripe\\Security\\Member.EMPTYNEWPASSWORD', + "The new password can't be empty, please try again" + ), "bad" ); @@ -61,9 +257,13 @@ class ChangePasswordHandler extends FormRequestHandler // Fail if passwords do not match if ($data['NewPassword1'] !== $data['NewPassword2']) { $this->form->sessionMessage( - _t('SilverStripe\\Security\\Member.ERRORNEWPASSWORD', "You have entered your new password differently, try again"), + _t( + 'SilverStripe\\Security\\Member.ERRORNEWPASSWORD', + "You have entered your new password differently, try again" + ), "bad" ); + // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. return $this->redirectBackToForm(); } @@ -72,17 +272,20 @@ class ChangePasswordHandler extends FormRequestHandler $validationResult = $member->changePassword($data['NewPassword1']); if (!$validationResult->isValid()) { $this->form->setSessionValidationResult($validationResult); + return $this->redirectBackToForm(); } // Clear locked out status $member->LockedOutUntil = null; $member->FailedLoginCount = null; + // Clear the members login hashes + $member->AutoLoginHash = null; + $member->AutoLoginExpired = DBDatetime::create()->now(); $member->write(); if ($member->canLogIn()->isValid()) { - Injector::inst()->get(IdentityStore::class) - ->logIn($member, false, $form->getRequestHandler()->getRequest()); + Injector::inst()->get(IdentityStore::class)->logIn($member, false, $this->getRequest()); } // TODO Add confirmation message to login redirect @@ -96,6 +299,7 @@ class ChangePasswordHandler extends FormRequestHandler // Redirect to default location - the login form saying "You are logged in as..." $url = Security::singleton()->Link('login'); + return $this->redirect($url); } @@ -103,6 +307,7 @@ class ChangePasswordHandler extends FormRequestHandler { // Redirect back to form $url = $this->addBackURLParam(CMSSecurity::singleton()->Link('changepassword')); + return $this->redirect($url); } } diff --git a/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php b/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php index 9fd2059d9..ca1085c6a 100644 --- a/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php @@ -2,6 +2,7 @@ namespace SilverStripe\Security\MemberAuthenticator; +use SilverStripe\Control\HTTPResponse; use SilverStripe\Security\Member; use SilverStripe\Control\HTTPRequest; use SilverStripe\Security\AuthenticationHandler as AuthenticationHandlerInterface; @@ -30,7 +31,7 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide /** * @var IdentityStore */ - private $cascadeLogInTo; + private $cascadeInTo; /** * Get the name of the cookie used to track this device @@ -45,7 +46,7 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide /** * Set the name of the cookie used to track this device * - * @param $deviceCookieName + * @param string $deviceCookieName * @return null */ public function setDeviceCookieName($deviceCookieName) @@ -66,8 +67,7 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide /** * Set the name of the cookie used to store an login token * - * @param $tokenCookieName - * @return null + * @param string $tokenCookieName */ public function setTokenCookieName($tokenCookieName) { @@ -81,22 +81,23 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide */ public function getCascadeLogInTo() { - return $this->cascadeLogInTo; + return $this->cascadeInTo; } /** * Set the name of the cookie used to store an login token * - * @param $cascadeLogInTo + * @param IdentityStore $cascadeInTo * @return null */ - public function setCascadeLogInTo(IdentityStore $cascadeLogInTo) + public function setCascadeLogInTo(IdentityStore $cascadeInTo) { - $this->cascadeLogInTo = $cascadeLogInTo; + $this->cascadeInTo = $cascadeInTo; } /** - * @inherit + * @param HTTPRequest $request + * @return null|Member */ public function authenticateRequest(HTTPRequest $request) { @@ -104,14 +105,14 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide $deviceID = Cookie::get($this->getDeviceCookieName()); // @todo Consider better placement of database_is_ready test - if (!$deviceID || strpos($uidAndToken, ':') === false || !Security::database_is_ready()) { - return; + if ($deviceID === null || strpos($uidAndToken, ':') === false || !Security::database_is_ready()) { + return null; } list($uid, $token) = explode(':', $uidAndToken, 2); if (!$uid || !$token) { - return; + return null; } /** @var Member $member */ @@ -127,7 +128,7 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide ->filter(array( 'MemberID' => $member->ID, 'DeviceID' => $deviceID, - 'Hash' => $hash + 'Hash' => $hash ))->first(); if (!$rememberLoginHash) { @@ -144,11 +145,10 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide } if ($member) { - if ($this->cascadeLogInTo) { + if ($this->cascadeInTo) { // @todo look at how to block "regular login" triggers from happening here // @todo deal with the fact that the Session::current_session() isn't correct here :-/ - $this->cascadeLogInTo->logIn($member, false, $request); - //\SilverStripe\Dev\Debug::message('here'); + $this->cascadeInTo->logIn($member, false, $request); } // @todo Consider whether response should be part of logIn() as well @@ -168,20 +168,21 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide ); } - return $member; - // Audit logging hook $member->extend('memberAutoLoggedIn'); + + return $member; } } /** - * @inherit + * @param Member $member + * @param bool $persistent + * @param HTTPRequest $request + * @return HTTPResponse|void */ - public function logIn(Member $member, $persistent, HTTPRequest $request) + public function logIn(Member $member, $persistent = false, HTTPRequest $request = null) { - // @todo couple the cookies to the response object - // Cleans up any potential previous hash for this member on this device if ($alcDevice = Cookie::get($this->getDeviceCookieName())) { RememberLoginHash::get()->filter('DeviceID', $alcDevice)->removeAll(); @@ -210,29 +211,39 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide null, true ); - - // Clear a cookie for non-persistent log-ins } else { - $this->logOut($request); + // Clear a cookie for non-persistent log-ins + $this->clearCookies(); } } /** - * @inherit + * @param HTTPRequest|null $request + * @return HTTPResponse|void */ - public function logOut(HTTPRequest $request) + public function logOut(HTTPRequest $request = null) { $member = Security::getCurrentUser(); if ($member) { RememberLoginHash::clear($member, Cookie::get('alc_device')); } - // @todo couple the cookies to the response object + $this->clearCookies(); + if ($this->cascadeInTo) { + $this->cascadeInTo->logOut($request); + } + + Security::setCurrentUser(null); + } + + /** + * Clear the cookies set for the user + */ + protected function clearCookies() + { Cookie::set($this->getTokenCookieName(), null); Cookie::set($this->getDeviceCookieName(), null); Cookie::force_expiry($this->getTokenCookieName()); Cookie::force_expiry($this->getDeviceCookieName()); - - Security::setCurrentUser(null); } } diff --git a/src/Security/MemberAuthenticator/LoginHandler.php b/src/Security/MemberAuthenticator/LoginHandler.php index 4da92efaa..a344d64c4 100644 --- a/src/Security/MemberAuthenticator/LoginHandler.php +++ b/src/Security/MemberAuthenticator/LoginHandler.php @@ -8,6 +8,7 @@ use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\Session; use SilverStripe\Control\RequestHandler; use SilverStripe\ORM\ValidationResult; +use SilverStripe\Security\Authenticator; use SilverStripe\Security\Security; use SilverStripe\Security\Member; use SilverStripe\Core\Injector\Injector; @@ -23,14 +24,14 @@ class LoginHandler extends RequestHandler */ protected $authenticator; + /** + * @var array + */ private static $url_handlers = [ '' => 'login', ]; /** - * Since the logout and dologin actions may be conditionally removed, it's necessary to ensure these - * remain valid actions regardless of the member login state. - * * @var array * @config */ @@ -40,23 +41,26 @@ class LoginHandler extends RequestHandler 'logout', ]; + /** + * @var string Called link on this handler + */ private $link; /** * @param string $link The URL to recreate this request handler - * @param Authenticator $authenticator The + * @param MemberAuthenticator $authenticator The authenticator to use */ - public function __construct($link, Authenticator $authenticator) + public function __construct($link, MemberAuthenticator $authenticator) { $this->link = $link; $this->authenticator = $authenticator; - parent::__construct($link, $this); + parent::__construct(); } /** * Return a link to this request handler. * The link returned is supplied in the constructor - * @param null $action + * @param null|string $action * @return string */ public function link($action = null) @@ -70,6 +74,8 @@ class LoginHandler extends RequestHandler /** * URL handler for the log-in screen + * + * @return array */ public function login() { @@ -80,10 +86,12 @@ class LoginHandler extends RequestHandler /** * Return the MemberLoginForm form + * + * @return MemberLoginForm */ public function loginForm() { - return LoginForm::create( + return MemberLoginForm::create( $this, get_class($this->authenticator), 'LoginForm' @@ -96,28 +104,41 @@ class LoginHandler extends RequestHandler * This method is called when the user finishes the login flow * * @param array $data Submitted data - * @param LoginForm $form + * @param MemberLoginForm $form * @return HTTPResponse */ public function doLogin($data, $form) { $failureMessage = null; + $this->extend('beforeLogin'); // Successful login - if ($member = $this->checkLogin($data, $failureMessage)) { + if ($member = $this->checkLogin($data, $result)) { $this->performLogin($member, $data, $form->getRequestHandler()->getRequest()); + // Allow operations on the member after successful login + $this->extend('afterLogin', $member); return $this->redirectAfterSuccessfulLogin(); } - $form->sessionMessage($failureMessage, 'bad'); + $this->extend('failedLogin'); + + $message = implode("; ", array_map( + function ($message) { + return $message['message']; + }, + $result->getMessages() + )); + + $form->sessionMessage($message, 'bad'); // Failed login /** @skipUpgrade */ if (array_key_exists('Email', $data)) { + $rememberMe = (isset($data['Remember']) && Security::config()->get('autologin_enabled') === true); Session::set('SessionForms.MemberLoginForm.Email', $data['Email']); - Session::set('SessionForms.MemberLoginForm.Remember', isset($data['Remember'])); + Session::set('SessionForms.MemberLoginForm.Remember', $rememberMe); } // Fail to login redirects back to form @@ -173,7 +194,7 @@ class LoginHandler extends RequestHandler 'Welcome Back, {firstname}', ['firstname' => $member->FirstName] ); - Security::setLoginMessage($message, ValidationResult::TYPE_GOOD); + Security::singleton()->setLoginMessage($message, ValidationResult::TYPE_GOOD); } // Redirect back @@ -184,19 +205,16 @@ class LoginHandler extends RequestHandler * Try to authenticate the user * * @param array $data Submitted data - * @param string $message + * @param ValidationResult $result * @return Member Returns the member object on successful authentication * or NULL on failure. */ - public function checkLogin($data, &$message) + public function checkLogin($data, &$result) { - $message = null; - $member = $this->authenticator->authenticate($data, $message); - if ($member) { + $member = $this->authenticator->authenticate($data, $result); + if ($member instanceof Member) { return $member; } - // No member, can't login - $this->extend('authenticationFailed', $data); return null; } @@ -212,8 +230,9 @@ class LoginHandler extends RequestHandler */ public function performLogin($member, $data, $request) { - // @todo pass request/response - Injector::inst()->get(IdentityStore::class)->logIn($member, !empty($data['Remember']), $request); + /** IdentityStore */ + $rememberMe = (isset($data['Remember']) && Security::config()->get('autologin_enabled')); + Injector::inst()->get(IdentityStore::class)->logIn($member, $rememberMe, $request); return $member; } @@ -235,20 +254,4 @@ class LoginHandler extends RequestHandler return $this->redirect($this->addBackURLParam($changedPasswordLink)); } - - - /** - * @todo copypaste from FormRequestHandler - refactor - * @param string $link - * @return string - */ - protected function addBackURLParam($link) - { - $backURL = $this->getBackURL(); - if ($backURL) { - return Controller::join_links($link, '?BackURL=' . urlencode($backURL)); - } - - return $link; - } } diff --git a/src/Security/MemberAuthenticator/LogoutHandler.php b/src/Security/MemberAuthenticator/LogoutHandler.php index db4b3771e..2c51899f8 100644 --- a/src/Security/MemberAuthenticator/LogoutHandler.php +++ b/src/Security/MemberAuthenticator/LogoutHandler.php @@ -16,7 +16,6 @@ use SilverStripe\Security\Security; * The logout process destroys all traces of the member on the server (not the actual computer user * at the other end of the line, don't worry) * - * @package SilverStripe\Security\MemberAuthenticator */ class LogoutHandler extends RequestHandler { diff --git a/src/Security/MemberAuthenticator/LostPasswordHandler.php b/src/Security/MemberAuthenticator/LostPasswordHandler.php index 5deb6d5c1..28cce5f60 100644 --- a/src/Security/MemberAuthenticator/LostPasswordHandler.php +++ b/src/Security/MemberAuthenticator/LostPasswordHandler.php @@ -7,10 +7,13 @@ use SilverStripe\Control\Email\Email; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\Session; use SilverStripe\Control\RequestHandler; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\Forms\Form; use SilverStripe\ORM\ValidationResult; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\EmailField; use SilverStripe\Forms\FormAction; +use SilverStripe\Security\IdentityStore; use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\Core\Convert; @@ -21,11 +24,18 @@ use SilverStripe\ORM\FieldType\DBField; */ class LostPasswordHandler extends RequestHandler { + /** + * Authentication class to use + * @var string + */ protected $authenticatorClass = MemberAuthenticator::class; + /** + * @var array + */ private static $url_handlers = [ 'passwordsent/$EmailAddress' => 'passwordsent', - '' => 'lostpassword', + '' => 'lostpassword', ]; /** @@ -44,7 +54,7 @@ class LostPasswordHandler extends RequestHandler private $link = null; /** - * @param $link The URL to recreate this request handler + * @param string $link The URL to recreate this request handler */ public function __construct($link) { @@ -55,37 +65,43 @@ class LostPasswordHandler extends RequestHandler /** * Return a link to this request handler. * The link returned is supplied in the constructor + * + * @param string $action * @return string */ public function link($action = null) { if ($action) { return Controller::join_links($this->link, $action); - } else { - return $this->link; } + + return $this->link; } /** * URL handler for the initial lost-password screen + * + * @return array */ public function lostpassword() { $message = _t( - 'Security.NOTERESETPASSWORD', + 'SilverStripe\\Security\\Security.NOTERESETPASSWORD', 'Enter your e-mail address and we will send you a link with which you can reset your password' ); return [ 'Content' => DBField::create_field('HTMLFragment', "

$message

"), - 'Form' => $this->lostPasswordForm(), + 'Form' => $this->lostPasswordForm(), ]; } /** * Show the "password sent" page, after a user has requested * to reset their password. + * + * @return array */ public function passwordsent() { @@ -93,20 +109,20 @@ class LostPasswordHandler extends RequestHandler $email = Convert::raw2xml(rawurldecode($request->param('EmailAddress')) . '.' . $request->getExtension()); $message = _t( - 'Security.PASSWORDSENTTEXT', + 'SilverStripe\\Security\\Security.PASSWORDSENTTEXT', "Thank you! A reset link has been sent to '{email}', provided an account exists for this email" . " address.", - [ 'email' => Convert::raw2xml($email) ] + ['email' => Convert::raw2xml($email)] ); return [ - 'Title' => _t( - 'Security.PASSWORDSENTHEADER', + 'Title' => _t( + 'SilverStripe\\Security\\Security.PASSWORDSENTHEADER', "Password reset link sent to '{email}'", array('email' => $email) ), 'Content' => DBField::create_field('HTMLFragment', "

$message

"), - 'Email' => $email + 'Email' => $email ]; } @@ -119,17 +135,17 @@ class LostPasswordHandler extends RequestHandler */ public function lostPasswordForm() { - return LoginForm::create( + return MemberLoginForm::create( $this, $this->authenticatorClass, - 'LostPasswordForm', + 'lostPasswordForm', new FieldList( - new EmailField('Email', _t('Member.EMAIL', 'Email')) + new EmailField('Email', _t('SilverStripe\\Security\\Member.EMAIL', 'Email')) ), new FieldList( new FormAction( 'forgotPassword', - _t('Security.BUTTONSEND', 'Send me the password reset link') + _t('SilverStripe\\Security\\Security.BUTTONSEND', 'Send me the password reset link') ) ), false @@ -144,12 +160,8 @@ class LostPasswordHandler extends RequestHandler public function redirectToLostPassword() { $lostPasswordLink = Security::singleton()->Link('lostpassword'); - return $this->redirect($this->addBackURLParam($lostPasswordLink)); - } - public function getReturnReferer() - { - return $this->link(); + return $this->redirect($this->addBackURLParam($lostPasswordLink)); } /** @@ -167,29 +179,6 @@ class LostPasswordHandler extends RequestHandler return Security::singleton()->logout(); } - /** - * Try to authenticate the user - * - * @param array $data Submitted data - * @return Member Returns the member object on successful authentication - * or NULL on failure. - */ - public function performLogin($data) - { - $member = call_user_func_array( - [$this->authenticator_class, 'authenticate'], - [$data, $this->form] - ); - if ($member) { - $member->LogIn(isset($data['Remember'])); - return $member; - } - - // No member, can't login - $this->extend('authenticationFailed', $data); - return null; - } - /** * Forgot password form handler method. * Called when the user clicks on "I've lost my password". @@ -207,15 +196,20 @@ class LostPasswordHandler extends RequestHandler // Ensure password is given if (empty($data['Email'])) { $this->form->sessionMessage( - _t('Member.ENTEREMAIL', 'Please enter an email address to get a password reset link.'), + _t( + 'SilverStripe\\Security\\Member.ENTEREMAIL', + 'Please enter an email address to get a password reset link.' + ), 'bad' ); + return $this->redirectToLostPassword(); } // Find existing member + $field = Member::config()->get('unique_identifier_field'); /** @var Member $member */ - $member = Member::get()->filter("Email", $data['Email'])->first(); + $member = Member::get()->filter([$field => $data['Email']])->first(); // Allow vetoing forgot password requests $results = $this->extend('forgotPassword', $member); @@ -229,7 +223,11 @@ class LostPasswordHandler extends RequestHandler Email::create() ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail') ->setData($member) - ->setSubject(_t('Member.SUBJECTPASSWORDRESET', "Your password reset link", 'Email subject')) + ->setSubject(_t( + 'SilverStripe\\Security\\Member.SUBJECTPASSWORDRESET', + "Your password reset link", + 'Email subject' + )) ->addData('PasswordResetLink', Security::getPasswordResetLink($member, $token)) ->setTo($member->Email) ->send(); @@ -242,18 +240,7 @@ class LostPasswordHandler extends RequestHandler rawurlencode($data['Email']), '/' ); + return $this->redirect($this->addBackURLParam($link)); } - - /** - * @todo copypaste from FormRequestHandler - refactor - */ - protected function addBackURLParam($link) - { - $backURL = $this->getBackURL(); - if ($backURL) { - return Controller::join_links($link, '?BackURL=' . urlencode($backURL)); - } - return $link; - } } diff --git a/src/Security/MemberAuthenticator/Authenticator.php b/src/Security/MemberAuthenticator/MemberAuthenticator.php similarity index 65% rename from src/Security/MemberAuthenticator/Authenticator.php rename to src/Security/MemberAuthenticator/MemberAuthenticator.php index 745396ac2..879c118bd 100644 --- a/src/Security/MemberAuthenticator/Authenticator.php +++ b/src/Security/MemberAuthenticator/MemberAuthenticator.php @@ -6,7 +6,7 @@ use SilverStripe\Control\Controller; use SilverStripe\Control\Session; use SilverStripe\ORM\ValidationResult; use InvalidArgumentException; -use SilverStripe\Security\Authenticator as BaseAuthenticator; +use SilverStripe\Security\Authenticator; use SilverStripe\Security\Security; use SilverStripe\Security\Member; use SilverStripe\Security\LoginAttempt; @@ -14,62 +14,68 @@ use SilverStripe\Security\LoginAttempt; /** * Authenticator for the default "member" method * - * @author Markus Lanthaler + * @author Sam Minnee + * @author Simon Erkelens */ -class Authenticator implements BaseAuthenticator +class MemberAuthenticator implements Authenticator { public function supportedServices() { - // Bitwise-OR of all the supported services, to make a bitmask - return BaseAuthenticator::LOGIN | BaseAuthenticator::LOGOUT | BaseAuthenticator::CHANGE_PASSWORD - | BaseAuthenticator::RESET_PASSWORD; + // Bitwise-OR of all the supported services in this Authenticator, to make a bitmask + return Authenticator::LOGIN | Authenticator::LOGOUT | Authenticator::CHANGE_PASSWORD + | Authenticator::RESET_PASSWORD; } /** - * @inherit + * @param array $data + * @param null|ValidationResult $result + * @return null|Member */ - public function authenticate($data, &$message) + public function authenticate($data, &$result = null) { - $success = null; - // Find authenticated member - $member = $this->authenticateMember($data, $message, $success); + $member = $this->authenticateMember($data, $result); // Optionally record every login attempt as a {@link LoginAttempt} object - $this->recordLoginAttempt($data, $member, $success); + $this->recordLoginAttempt($data, $member, $result->isValid()); if ($member) { Session::clear('BackURL'); } - return $success ? $member : null; + return $result->isValid() ? $member : null; } /** * Attempt to find and authenticate member if possible from the given data * * @param array $data Form submitted data - * @param $message - * @param bool &$success Success flag - * @param null|Member $member If the parent method already identified the member, it can be passed in + * @param ValidationResult $result + * @param Member|null This third parameter is used in the CMSAuthenticator(s) * @return Member Found member, regardless of successful login */ - protected function authenticateMember($data, &$message, &$success, $member = null) + protected function authenticateMember($data, &$result = null, $member = null) { // Default success to false - $success = false; - $email = !empty($data['Email']) ? $data['Email'] : null ; - + $email = !empty($data['Email']) ? $data['Email'] : null; + $result = new ValidationResult(); + // Check default login (see Security::setDefaultAdmin()) $asDefaultAdmin = $email === Security::default_admin_username(); if ($asDefaultAdmin) { // If logging is as default admin, ensure record is setup correctly $member = Member::default_admin(); - $success = $member->canLogin()->isValid() && Security::check_default_admin($email, $data['Password']); + $success = Security::check_default_admin($email, $data['Password']); + $result = $member->canLogIn(); //protect against failed login - if ($success) { + if ($success && $result->isValid()) { return $member; + } else { + $result->addError(_t( + 'SilverStripe\\Security\\Member.ERRORWRONGCRED', + "The provided details don't seem to be correct. Please try again." + )); } } @@ -85,28 +91,23 @@ class Authenticator implements BaseAuthenticator // Validate against member if possible if ($member && !$asDefaultAdmin) { $result = $member->checkPassword($data['Password']); - $success = $result->isValid(); - } else { - $result = ValidationResult::create()->addError(_t( - 'SilverStripe\\Security\\Member.ERRORWRONGCRED', - 'The provided details don\'t seem to be correct. Please try again.' - )); } // Emit failure to member and form (if available) - if (!$success) { + if (!$result->isValid()) { if ($member) { $member->registerFailedLogin(); } - $message = implode("; ", array_map( - function ($message) { - return $message['message']; - }, - $result->getMessages() - )); } else { - if ($member) { // How can success be true and member false? + if ($member) { $member->registerSuccessfulLogin(); + } else { + // A non-existing member occurred. This will make the result "valid" so let's invalidate + $result->addError(_t( + 'SilverStripe\\Security\\Member.ERRORWRONGCRED', + "The provided details don't seem to be correct. Please try again." + )); + $member = null; } } @@ -119,6 +120,7 @@ class Authenticator implements BaseAuthenticator * * @param array $data * @param Member $member + * @param boolean $success */ protected function recordLoginAttempt($data, $member, $success) { @@ -134,7 +136,7 @@ class Authenticator implements BaseAuthenticator } $attempt = LoginAttempt::create(); - if ($success) { + if ($success && $member) { // successful login (member is existing with matching password) $attempt->MemberID = $member->ID; $attempt->Status = 'Success'; @@ -160,7 +162,8 @@ class Authenticator implements BaseAuthenticator } /** - * @inherit + * @param $link + * @return LostPasswordHandler */ public function getLostPasswordHandler($link) { @@ -168,7 +171,8 @@ class Authenticator implements BaseAuthenticator } /** - * @inherit + * @param string $link + * @return ChangePasswordHandler */ public function getChangePasswordHandler($link) { @@ -176,7 +180,8 @@ class Authenticator implements BaseAuthenticator } /** - * @inherit + * @param string $link + * @return LoginHandler */ public function getLoginHandler($link) { @@ -184,7 +189,8 @@ class Authenticator implements BaseAuthenticator } /** - * @inherit + * @param string $link + * @return LogoutHandler */ public function getLogoutHandler($link) { diff --git a/src/Security/MemberAuthenticator/LoginForm.php b/src/Security/MemberAuthenticator/MemberLoginForm.php similarity index 97% rename from src/Security/MemberAuthenticator/LoginForm.php rename to src/Security/MemberAuthenticator/MemberLoginForm.php index c32298c67..8f9c941dc 100644 --- a/src/Security/MemberAuthenticator/LoginForm.php +++ b/src/Security/MemberAuthenticator/MemberLoginForm.php @@ -31,7 +31,7 @@ use SilverStripe\View\Requirements; * allowing extensions to "veto" execution by returning FALSE. * Arguments: $member containing the detected Member record */ -class LoginForm extends BaseLoginForm +class MemberLoginForm extends BaseLoginForm { /** @@ -87,7 +87,7 @@ class LoginForm extends BaseLoginForm $backURL = Session::get('BackURL'); } - if ($checkCurrentUser && Security::getCurrentUser() && Member::logged_in_session_exists()) { + if ($checkCurrentUser && Security::getCurrentUser()) { // @todo find a more elegant way to handle this $logoutAction = Security::logout_url(); $fields = FieldList::create( @@ -145,7 +145,7 @@ class LoginForm extends BaseLoginForm $this->setAttribute('autocomplete', 'off'); $emailField->setAttribute('autocomplete', 'off'); } - if (Security::config()->autologin_enabled) { + if (Security::config()->get('autologin_enabled')) { $fields->push( CheckboxField::create( "Remember", diff --git a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php index 1408df9ba..4c0bac4bc 100644 --- a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php @@ -3,22 +3,24 @@ namespace SilverStripe\Security\MemberAuthenticator; use SilverStripe\Control\Cookie; +use SilverStripe\Control\HTTPResponse; use SilverStripe\ORM\DataObject; use SilverStripe\Security\Member; use SilverStripe\Control\HTTPRequest; -use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\Session; use SilverStripe\Control\Director; -use SilverStripe\Security\AuthenticationHandler as AuthenticationHandlerInterface; -use SilverStripe\ORM\ValidationException; +use SilverStripe\Security\AuthenticationHandler; use SilverStripe\Security\IdentityStore; /** * Authenticate a member pased on a session cookie */ -class SessionAuthenticationHandler implements AuthenticationHandlerInterface, IdentityStore +class SessionAuthenticationHandler implements AuthenticationHandler, IdentityStore { + /** + * @var string + */ private $sessionVariable; /** @@ -35,7 +37,6 @@ class SessionAuthenticationHandler implements AuthenticationHandlerInterface, Id * Set the session variable name used to track member ID * * @param string $sessionVariable - * @return null */ public function setSessionVariable($sessionVariable) { @@ -44,12 +45,11 @@ class SessionAuthenticationHandler implements AuthenticationHandlerInterface, Id /** * @inherit + * @param HTTPRequest $request + * @return null|DataObject|Member */ public function authenticateRequest(HTTPRequest $request) { - // @todo couple the session to a request object - // $session = $request->getSession(); - if ($id = Session::get($this->getSessionVariable())) { // If ID is a bad ID it will be treated as if the user is not logged in, rather than throwing a // ValidationException @@ -61,17 +61,18 @@ class SessionAuthenticationHandler implements AuthenticationHandlerInterface, Id /** * @inherit + * @param Member $member + * @param bool $persistent + * @param HTTPRequest|null $request + * @return HTTPResponse|void */ - public function logIn(Member $member, $persistent, HTTPRequest $request) + public function logIn(Member $member, $persistent = false, HTTPRequest $request = null) { - // @todo couple the session to a request object - // $session = $request->getSession(); - static::regenerateSessionId(); Session::set($this->getSessionVariable(), $member->ID); // This lets apache rules detect whether the user has logged in - // @todo make this a settign on the authentication handler + // @todo make this a setting on the authentication handler if (Member::config()->get('login_marker_cookie')) { Cookie::set(Member::config()->get('login_marker_cookie'), 1, 0); } @@ -82,7 +83,7 @@ class SessionAuthenticationHandler implements AuthenticationHandlerInterface, Id */ protected static function regenerateSessionId() { - if (!Member::config()->session_regenerate_id) { + if (!Member::config()->get('session_regenerate_id')) { return; } @@ -100,14 +101,13 @@ class SessionAuthenticationHandler implements AuthenticationHandlerInterface, Id @session_regenerate_id(true); } } - /** - * @inherit - */ - public function logOut(HTTPRequest $request) - { - // @todo couple the session to a request object - // $session = $request->getSession(); + /** + * @param HTTPRequest|null $request + * @return HTTPResponse|void + */ + public function logOut(HTTPRequest $request = null) + { Session::clear($this->getSessionVariable()); } } diff --git a/src/Security/Permission.php b/src/Security/Permission.php index cb71526c2..433339a3a 100644 --- a/src/Security/Permission.php +++ b/src/Security/Permission.php @@ -4,6 +4,7 @@ namespace SilverStripe\Security; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Resettable; +use SilverStripe\Dev\Debug; use SilverStripe\Dev\TestOnly; use SilverStripe\i18n\i18nEntityProvider; use SilverStripe\ORM\DB; @@ -131,10 +132,10 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl public static function check($code, $arg = "any", $member = null, $strict = true) { if (!$member) { - if (!Member::currentUserID()) { + if (!Security::getCurrentUser()) { return false; } - $member = Member::currentUserID(); + $member = Security::getCurrentUser(); } return self::checkMember($member, $code, $arg, $strict); @@ -171,10 +172,9 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl public static function checkMember($member, $code, $arg = "any", $strict = true) { if (!$member) { - $memberID = $member = Member::currentUserID(); - } else { - $memberID = (is_object($member)) ? $member->ID : $member; + $member = Security::getCurrentUser(); } + $memberID = ($member instanceof Member) ? $member->ID : $member; if (!$memberID) { return false; diff --git a/src/Security/Security.php b/src/Security/Security.php index 2c995e537..f028e68ba 100644 --- a/src/Security/Security.php +++ b/src/Security/Security.php @@ -13,29 +13,21 @@ use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Control\Session; use SilverStripe\Control\RequestHandler; use SilverStripe\Core\ClassInfo; -use SilverStripe\Core\Config\Config; use SilverStripe\Core\Convert; use SilverStripe\Dev\Deprecation; use SilverStripe\Dev\TestOnly; -use SilverStripe\Forms\EmailField; -use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; -use SilverStripe\Forms\FormAction; use SilverStripe\ORM\ArrayList; -use SilverStripe\ORM\Connect\Database; use SilverStripe\ORM\DataModel; use SilverStripe\ORM\DB; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\ORM\ValidationResult; -use SilverStripe\Security\MemberAuthenticator\LogoutHandler; use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; use SilverStripe\View\TemplateGlobalProvider; -use Exception; -use SilverStripe\View\ViewableData_Customised; use Subsite; -use SilverStripe\Core\Injector\Injector; /** * Implements a basic security model @@ -52,13 +44,10 @@ class Security extends Controller implements TemplateGlobalProvider 'passwordsent', 'changepassword', 'ping', - 'LoginForm', - 'ChangePasswordForm', - 'LostPasswordForm', ); /** - * Default user name. Only used in dev-mode by {@link setDefaultAdmin()} + * Default user name. {@link setDefaultAdmin()} * * @var string * @see setDefaultAdmin() @@ -66,7 +55,7 @@ class Security extends Controller implements TemplateGlobalProvider protected static $default_username; /** - * Default password. Only used in dev-mode by {@link setDefaultAdmin()} + * Default password. {@link setDefaultAdmin()} * * @var string * @see setDefaultAdmin() @@ -80,7 +69,7 @@ class Security extends Controller implements TemplateGlobalProvider * @config * @var bool */ - protected static $strict_path_checking = false; + private static $strict_path_checking = false; /** * The password encryption algorithm to use by default. @@ -102,7 +91,7 @@ class Security extends Controller implements TemplateGlobalProvider /** * Determine if login username may be remembered between login sessions - * If set to false this will disable autocomplete and prevent username persisting in the session + * If set to false this will disable auto-complete and prevent username persisting in the session * * @config * @var bool @@ -124,7 +113,7 @@ class Security extends Controller implements TemplateGlobalProvider private static $template = 'BlankPage'; /** - * Template thats used to render the pages. + * Template that is used to render the pages. * * @var string * @config @@ -163,7 +152,7 @@ class Security extends Controller implements TemplateGlobalProvider * * @var string */ - private static $login_url = "Security/login"; + private static $login_url = 'Security/login'; /** * The default logout URL @@ -172,7 +161,7 @@ class Security extends Controller implements TemplateGlobalProvider * * @var string */ - private static $logout_url = "Security/logout"; + private static $logout_url = 'Security/logout'; /** * The default lost password URL @@ -181,7 +170,7 @@ class Security extends Controller implements TemplateGlobalProvider * * @var string */ - private static $lost_password_url = "Security/lostpassword"; + private static $lost_password_url = 'Security/lostpassword'; /** * Value of X-Frame-Options header @@ -212,7 +201,7 @@ class Security extends Controller implements TemplateGlobalProvider * @var boolean If set to TRUE or FALSE, {@link database_is_ready()} * will always return FALSE. Used for unit testing. */ - protected static $force_database_is_ready = null; + protected static $force_database_is_ready; /** * When the database has once been verified as ready, it will not do the @@ -223,19 +212,30 @@ class Security extends Controller implements TemplateGlobalProvider protected static $database_is_ready = false; /** - * @var array available authenticators + * @var Authenticator[] available authenticators */ - protected static $authenticators = []; - - /** - * @var string Default authenticator - */ - protected static $default_authenticator = MemberAuthenticator\Authenticator::class; + private static $authenticators = []; /** * @var Member Currently logged in user (if available) */ - private static $currentUser; + protected static $currentUser; + + /** + * @return array + */ + public static function getAuthenticators() + { + return self::$authenticators; + } + + /** + * @param array|Authenticator $authenticators + */ + public static function setAuthenticators(array $authenticators) + { + self::$authenticators = $authenticators; + } /** * @inheritdoc @@ -268,18 +268,16 @@ class Security extends Controller implements TemplateGlobalProvider /** * Get the selected authenticator for this request * - * @param $name string The identifier of the authenticator in your config + * @param string $name The identifier of the authenticator in your config * @return Authenticator Class name of Authenticator * @throws LogicException */ - protected function getAuthenticator($name) + protected function getAuthenticator($name = 'default') { - $authenticators = self::config()->get('authenticators'); - - $name = $name ?: 'default'; + $authenticators = static::$authenticators; if (isset($authenticators[$name])) { - return Injector::inst()->get($authenticators[$name]); + return $authenticators[$name]; } throw new LogicException('No valid authenticator found'); @@ -288,18 +286,16 @@ class Security extends Controller implements TemplateGlobalProvider /** * Get all registered authenticators * + * @param int $service The type of service that is requested * @return array Return an array of Authenticator objects */ - public static function getAuthenticators($service = Authenticator::LOGIN) + public function getApplicableAuthenticators($service = Authenticator::LOGIN) { - $authenticators = self::config()->get('authenticators'); + $authenticators = static::$authenticators; - foreach ($authenticators as $name => &$class) { - /** @var Authenticator $authenticator */ - $authenticator = Injector::inst()->get($class); - if ($authenticator->supportedServices() & $service) { - $class = $authenticator; - } else { + /** @var Authenticator $class */ + foreach ($authenticators as $name => $class) { + if (!($class->supportedServices() & $service)) { unset($authenticators[$name]); } } @@ -314,9 +310,10 @@ class Security extends Controller implements TemplateGlobalProvider * @return bool Returns TRUE if the authenticator is registered, FALSE * otherwise. */ - public static function hasAuthenticator($authenticator) + public function hasAuthenticator($authenticator) { - $authenticators = self::config()->get('authenticators'); + $authenticators = static::$authenticators; + return !empty($authenticators[$authenticator]); } @@ -357,13 +354,18 @@ class Security extends Controller implements TemplateGlobalProvider $response = ($controller) ? $controller->getResponse() : new HTTPResponse(); $response->setStatusCode(403); if (!static::getCurrentUser()) { - $response->setBody(_t('SilverStripe\\CMS\\Controllers\\ContentController.NOTLOGGEDIN', 'Not logged in')); - $response->setStatusDescription(_t('SilverStripe\\CMS\\Controllers\\ContentController.NOTLOGGEDIN', 'Not logged in')); - // Tell the CMS to allow re-aunthentication + $response->setBody( + _t('SilverStripe\\CMS\\Controllers\\ContentController.NOTLOGGEDIN', 'Not logged in') + ); + $response->setStatusDescription( + _t('SilverStripe\\CMS\\Controllers\\ContentController.NOTLOGGEDIN', 'Not logged in') + ); + // Tell the CMS to allow re-authentication if (CMSSecurity::enabled()) { $response->addHeader('X-Reauthenticate', '1'); } } + return $response; } @@ -373,15 +375,15 @@ class Security extends Controller implements TemplateGlobalProvider $messageSet = $configMessageSet; } else { $messageSet = array( - 'default' => _t( + 'default' => _t( 'SilverStripe\\Security\\Security.NOTEPAGESECURED', "That page is secured. Enter your credentials below and we will send " - . "you right along." + . "you right along." ), 'alreadyLoggedIn' => _t( 'SilverStripe\\Security\\Security.ALREADYLOGGEDIN', "You don't have access to this page. If you have another account that " - . "can access that page, you can log in again below.", + . "can access that page, you can log in again below.", "%s will be replaced with a link to log in." ) ); @@ -407,8 +409,8 @@ class Security extends Controller implements TemplateGlobalProvider $message = $messageSet['default']; } - Security::setLoginMessage($message, ValidationResult::TYPE_WARNING); - $loginResponse = (new Security())->login(new HTTPRequest('GET', '/')); + static::singleton()->setLoginMessage($message, ValidationResult::TYPE_WARNING); + $loginResponse = static::singleton()->login(); if ($loginResponse instanceof HTTPResponse) { return $loginResponse; } @@ -422,7 +424,7 @@ class Security extends Controller implements TemplateGlobalProvider $message = $messageSet['default']; } - static::setLoginMessage($message, ValidationResult::TYPE_WARNING); + static::singleton()->setLoginMessage($message, ValidationResult::TYPE_WARNING); Session::set("BackURL", $_SERVER['REQUEST_URI']); @@ -436,11 +438,17 @@ class Security extends Controller implements TemplateGlobalProvider )); } - public static function setCurrentUser($currentUser) + /** + * @param null|Member $currentUser + */ + public static function setCurrentUser($currentUser = null) { self::$currentUser = $currentUser; } + /** + * @return null|Member + */ public static function getCurrentUser() { return self::$currentUser; @@ -449,18 +457,21 @@ class Security extends Controller implements TemplateGlobalProvider /** * Get the login forms for all available authentication methods * + * @deprecated 5.0.0 Now handled by {@link static::delegateToMultipleHandlers} + * * @return array Returns an array of available login forms (array of Form * objects). * - * @todo Check how to activate/deactivate authentication methods */ public function getLoginForms() { + Deprecation::notice('5.0.0', 'Now handled by delegateToMultipleHandlers'); + return array_map( function ($authenticator) { - return $authenticator->getLoginHandler($this->Link())->handleRequest($this->getRequest(), DataModel::inst()); + return [$authenticator->getLoginHandler($this->Link())->loginForm()]; }, - Security::getAuthenticators() + $this->getApplicableAuthenticators() ); } @@ -507,11 +518,16 @@ class Security extends Controller implements TemplateGlobalProvider $member = static::getCurrentUser(); if ($member) { // If we don't have a member, there's not much to log out. - /** @var Authenticator $authenticator */ - $authenticator = $this->getAuthenticator('default'); // Always use the default authenticator to log out - $handler = $authenticator->getLogOutHandler(Controller::join_links($this->Link(), 'logout')); - $result = $this->delegateToHandler($handler, 'default', []); - if ($result !== true) { + /** @var array|Authenticator[] $authenticators */ + $authenticators = $this->getApplicableAuthenticators(Authenticator::LOGOUT); + + /** @var Authenticator[] $authenticator */ + foreach ($authenticators as $name => $authenticator) { + $handler = $authenticator->getLogOutHandler(Controller::join_links($this->Link(), 'logout')); + $this->delegateToHandler($handler, $name); + } + // In the rare case, but plausible with e.g. an external IdentityStore, the user is not logged out. + if (static::getCurrentUser() !== null) { $this->extend('failureMemberLoggedOut', $authenticator); return $this->redirectBack(); @@ -558,10 +574,10 @@ class Security extends Controller implements TemplateGlobalProvider // This step is necessary in cases such as automatic redirection where a user is authenticated // upon landing on an SSL secured site and is automatically logged in, or some other case // where the user has permissions to continue but is not given the option. - if ($this->getRequest()->requestVar('BackURL') - && !$this->getLoginMessage() + if (!$this->getLoginMessage() && ($member = static::getCurrentUser()) && $member->exists() + && $this->getRequest()->requestVar('BackURL') ) { return $this->redirectBack(); } @@ -590,20 +606,21 @@ class Security extends Controller implements TemplateGlobalProvider /** @skipUpgrade */ $holderPage->URLSegment = 'Security'; // Disable ID-based caching of the log-in page by making it a random number - $holderPage->ID = -1 * rand(1, 10000000); + $holderPage->ID = -1 * random_int(1, 10000000); $controllerClass = $holderPage->getControllerName(); /** @var ContentController $controller */ $controller = $controllerClass::create($holderPage); $controller->setDataModel($this->model); $controller->doInit(); + return $controller; } /** * Combine the given forms into a formset with a tabbed interface * - * @param $forms + * @param array|Form[] $forms * @return string */ protected function generateLoginFormSet($forms) @@ -611,6 +628,7 @@ class Security extends Controller implements TemplateGlobalProvider $viewData = new ArrayData(array( 'Forms' => new ArrayList($forms), )); + return $viewData->renderWith( $this->getTemplatesFor('MultiAuthenticatorLogin') ); @@ -635,6 +653,7 @@ class Security extends Controller implements TemplateGlobalProvider if ($messageCast !== ValidationResult::CAST_HTML) { $message = Convert::raw2xml($message); } + return sprintf('

%s

', Convert::raw2att($messageType), $message); } @@ -645,14 +664,14 @@ class Security extends Controller implements TemplateGlobalProvider * @param string $messageType Message type. One of ValidationResult::TYPE_* * @param string $messageCast Message cast. One of ValidationResult::CAST_* */ - public static function setLoginMessage( + public function setLoginMessage( $message, $messageType = ValidationResult::TYPE_WARNING, $messageCast = ValidationResult::CAST_TEXT ) { - Session::set("Security.Message.message", $message); - Session::set("Security.Message.type", $messageType); - Session::set("Security.Message.cast", $messageCast); + Session::set('Security.Message.message', $message); + Session::set('Security.Message.type', $messageType); + Session::set('Security.Message.cast', $messageCast); } /** @@ -660,7 +679,7 @@ class Security extends Controller implements TemplateGlobalProvider */ public static function clearLoginMessage() { - Session::clear("Security.Message"); + Session::clear('Security.Message'); } @@ -670,37 +689,48 @@ class Security extends Controller implements TemplateGlobalProvider * For multiple authenticators, Security_MultiAuthenticatorLogin is used. * See getTemplatesFor and getIncludeTemplate for how to override template logic * - * @param $request + * @param null|HTTPRequest $request + * @param int $service * @return HTTPResponse|string Returns the "login" page as HTML code. * @throws HTTPResponse_Exception */ - public function login($request, $service = Authenticator::LOGIN) + public function login($request = null, $service = Authenticator::LOGIN) { // Check pre-login process if ($response = $this->preLogin()) { return $response; } + $authName = null; - $link = $this->link("login"); - - // Delegate to a single handler - Security/login//... - if (($name = $request->param('ID')) && self::hasAuthenticator($request->param('ID'))) { - $request->shift(); - - $authenticator = $this->getAuthenticator($name); - // @todo handle different Authenticator situations - if (!$authenticator->supportedServices() & $service) { - throw new HTTPResponse_Exception('Invalid Authenticator "' . $name . '" for login action', 418); - } - - $authenticators = [ $name => $authenticator ]; - - // Delegate to all of them, building a tabbed view - Security/login - } else { - $authenticators = static::getAuthenticators($service); + if (!$request) { + $request = $this->getRequest(); + } + + if ($request && $request->param('ID')) { + $authName = $request->param('ID'); + } + + $link = $this->Link('login'); + + // Delegate to a single handler - Security/login//... + if ($authName && $this->hasAuthenticator($authName) + ) { + if ($request) { + $request->shift(); + } + + $authenticator = $this->getAuthenticator($authName); + + if (!$authenticator->supportedServices() & $service) { + throw new HTTPResponse_Exception('Invalid Authenticator "' . $authName . '" for login action', 418); + } + + $handlers = [$authName => $authenticator]; + } else { + // Delegate to all of them, building a tabbed view - Security/login + $handlers = $this->getApplicableAuthenticators($service); } - $handlers = $authenticators; array_walk( $handlers, function (&$auth, $name) use ($link) { @@ -721,9 +751,10 @@ class Security extends Controller implements TemplateGlobalProvider * * If a single handler is passed, delegateToHandler() will be called instead * + * @param array|RequestHandler[] $handlers * @param string $title The title of the form * @param array $templates - * @return array|HTTPResponse|RequestHandler|\SilverStripe\ORM\FieldType\DBHTMLText|string + * @return array|HTTPResponse|RequestHandler|DBHTMLText|string */ protected function delegateToMultipleHandlers(array $handlers, $title, array $templates) { @@ -736,7 +767,7 @@ class Security extends Controller implements TemplateGlobalProvider // Process each of the handlers $results = array_map( function ($handler) { - return $handler->handleRequest($this->getRequest(), \SilverStripe\ORM\DataModel::inst()); + return $handler->handleRequest($this->getRequest(), DataModel::inst()); }, $handlers ); @@ -754,7 +785,7 @@ class Security extends Controller implements TemplateGlobalProvider } if (!$forms) { - throw new \LogicException("No authenticators found compatible with a tabbed login"); + throw new \LogicException('No authenticators found compatible with a tabbed login'); } return $this->renderWrappedController( @@ -770,11 +801,12 @@ class Security extends Controller implements TemplateGlobalProvider * Delegate to another RequestHandler, rendering any fragment arrays into an appropriate. * controller. * + * @param RequestHandler $handler * @param string $title The title of the form * @param array $templates - * @return array|HTTPResponse|RequestHandler|\SilverStripe\ORM\FieldType\DBHTMLText|string + * @return array|HTTPResponse|RequestHandler|DBHTMLText|string */ - protected function delegateToHandler(RequestHandler $handler, $title, array $templates) + protected function delegateToHandler(RequestHandler $handler, $title, array $templates = []) { $result = $handler->handleRequest($this->getRequest(), DataModel::inst()); @@ -792,7 +824,7 @@ class Security extends Controller implements TemplateGlobalProvider * @param string $title string The title to give the security page * @param array $fragments A map of objects to render into the page, e.g. "Form" * @param array $templates An array of templates to use for the render - * @return HTTPResponse|\SilverStripe\ORM\FieldType\DBHTMLText + * @return HTTPResponse|DBHTMLText */ protected function renderWrappedController($title, array $fragments, array $templates) { @@ -824,7 +856,7 @@ class Security extends Controller implements TemplateGlobalProvider public function basicauthlogin() { - $member = BasicAuth::requireLogin($this->getRequest(), "SilverStripe login", 'ADMIN'); + $member = BasicAuth::requireLogin($this->getRequest(), 'SilverStripe login', 'ADMIN'); static::setCurrentUser($member); } @@ -835,12 +867,17 @@ class Security extends Controller implements TemplateGlobalProvider */ public function lostpassword() { - $handler = $this->getAuthenticator('default')->getLostPasswordHandler( - Controller::join_links($this->link(), 'lostpassword') - ); + $handlers = []; + $authenticators = $this->getApplicableAuthenticators(Authenticator::RESET_PASSWORD); + /** @var Authenticator $authenticator */ + foreach ($authenticators as $authenticator) { + $handlers[] = $authenticator->getLostPasswordHandler( + Controller::join_links($this->Link(), 'lostpassword') + ); + } - return $this->delegateToHandler( - $handler, + return $this->delegateToMultipleHandlers( + $handlers, _t('SilverStripe\\Security\\Security.LOSTPASSWORDHEADER', 'Lost Password'), $this->getTemplatesFor('lostpassword') ); @@ -860,79 +897,18 @@ class Security extends Controller implements TemplateGlobalProvider */ public function changepassword() { - $controller = $this->getResponseController(_t('SilverStripe\\Security\\Security.CHANGEPASSWORDHEADER', 'Change your password')); - - // if the controller calls Director::redirect(), this will break early - if (($response = $controller->getResponse()) && $response->isFinished()) { - return $response; + /** @var array|Authenticator[] $authenticators */ + $authenticators = $this->getApplicableAuthenticators(Authenticator::CHANGE_PASSWORD); + $handlers = []; + foreach ($authenticators as $authenticator) { + $handlers[] = $authenticator->getChangePasswordHandler($this->Link('changepassword')); } - // Extract the member from the URL. - /** @var Member $member */ - $member = null; - if (isset($_REQUEST['m'])) { - $member = Member::get()->filter('ID', (int)$_REQUEST['m'])->first(); - } - - // Check whether we are merely changin password, or resetting. - if (isset($_REQUEST['t']) && $member && $member->validateAutoLoginToken($_REQUEST['t'])) { - // On first valid password reset request redirect to the same URL without hash to avoid referrer leakage. - - // if there is a current member, they should be logged out - if ($curMember = static::getCurrentUser()) { - /** @var LogoutHandler $handler */ - $handler = $this->getAuthenticator('default')->getLogoutHandler($this->Link('logout')); - $handler->doLogOut($curMember); - } - - // Store the hash for the change password form. Will be unset after reload within the ChangePasswordForm. - Session::set('AutoLoginHash', $member->encryptWithUserSettings($_REQUEST['t'])); - - return $this->redirect($this->Link('changepassword')); - } elseif (Session::get('AutoLoginHash')) { - // Subsequent request after the "first load with hash" (see previous if clause). - $customisedController = $controller->customise(array( - 'Content' => DBField::create_field( - 'HTMLFragment', - '

' . _t('SilverStripe\\Security\\Security.ENTERNEWPASSWORD', 'Please enter a new password.') . '

' - ), - 'Form' => $this->ChangePasswordForm(), - )); - } elseif (static::getCurrentUser()) { - // Logged in user requested a password change form. - $customisedController = $controller->customise(array( - 'Content' => DBField::create_field( - 'HTMLFragment', - '

' . _t('SilverStripe\\Security\\Security.CHANGEPASSWORDBELOW', 'You can change your password below.') . '

' - ), - 'Form' => $this->ChangePasswordForm())); - } else { - // Show friendly message if it seems like the user arrived here via password reset feature. - if (isset($_REQUEST['m']) || isset($_REQUEST['t'])) { - $customisedController = $controller->customise( - array('Content' => DBField::create_field( - 'HTMLFragment', - _t( - 'SilverStripe\\Security\\Security.NOTERESETLINKINVALID', - '

The password reset link is invalid or expired.

' - . '

You can request a new one here or change your password after' - . ' you logged in.

', - [ - 'link1' => $this->Link('lostpassword'), - 'link2' => $this->Link('login') - ] - ) - )) - ); - } else { - return self::permissionFailure( - $this, - _t('SilverStripe\\Security\\Security.ERRORPASSWORDPERMISSION', 'You must be logged in in order to change your password!') - ); - } - } - - return $customisedController->renderWith($this->getTemplatesFor('changepassword')); + return $this->delegateToMultipleHandlers( + $handlers, + _t('SilverStripe\\Security\\Security.CHANGEPASSWORDHEADER', 'Change your password'), + $this->getTemplatesFor('changepassword') + ); } /** @@ -949,21 +925,8 @@ class Security extends Controller implements TemplateGlobalProvider public static function getPasswordResetLink($member, $autologinToken) { $autologinToken = urldecode($autologinToken); - $selfControllerClass = __CLASS__; - /** @var static $selfController */ - $selfController = new $selfControllerClass(); - return $selfController->Link('changepassword') . "?m={$member->ID}&t=$autologinToken"; - } - /** - * Factory method for the lost password form - * - * @skipUpgrade - * @return MemberAuthenticator\ChangePasswordForm - */ - public function ChangePasswordForm() - { - return MemberAuthenticator\ChangePasswordForm::create($this, 'ChangePasswordForm'); + return static::singleton()->Link('changepassword') . "?m={$member->ID}&t=$autologinToken"; } /** @@ -976,6 +939,7 @@ class Security extends Controller implements TemplateGlobalProvider public function getTemplatesFor($action) { $templates = SSViewer::get_templates_by_class(static::class, "_{$action}", __CLASS__); + return array_merge( $templates, [ @@ -1002,12 +966,7 @@ class Security extends Controller implements TemplateGlobalProvider */ public static function findAnAdministrator() { - // coupling to subsites module - $origSubsite = null; - if (is_callable('Subsite::changeSubsite')) { - $origSubsite = Subsite::currentSubsiteID(); - Subsite::changeSubsite(0); - } + static::singleton()->extend('beforeFindAdministrator'); /** @var Member $member */ $member = null; @@ -1015,19 +974,13 @@ class Security extends Controller implements TemplateGlobalProvider // find a group with ADMIN permission $adminGroup = Permission::get_groups_by_permission('ADMIN')->first(); - if (is_callable('Subsite::changeSubsite')) { - Subsite::changeSubsite($origSubsite); - } - - if ($adminGroup) { - $member = $adminGroup->Members()->First(); - } - if (!$adminGroup) { Group::singleton()->requireDefaultRecords(); $adminGroup = Permission::get_groups_by_permission('ADMIN')->first(); } + $member = $adminGroup->Members()->First(); + if (!$member) { Member::singleton()->requireDefaultRecords(); $member = Permission::get_members_by_permission('ADMIN')->first(); @@ -1049,6 +1002,8 @@ class Security extends Controller implements TemplateGlobalProvider ->add($member); } + static::singleton()->extend('afterFindAdministrator'); + return $member; } @@ -1083,6 +1038,7 @@ class Security extends Controller implements TemplateGlobalProvider self::$default_username = $username; self::$default_password = $password; + return true; } @@ -1105,11 +1061,10 @@ class Security extends Controller implements TemplateGlobalProvider /** * Check that the default admin account has been set. - * @todo Check if we _actually_ only want this to work on dev */ public static function has_default_admin() { - return !empty(self::$default_username) && !empty(self::$default_password) && (Director::get_environment_type() === 'dev'); + return !empty(self::$default_username) && !empty(self::$default_password); } /** @@ -1172,8 +1127,8 @@ class Security extends Controller implements TemplateGlobalProvider $salt = ($salt) ? $salt : $e->salt($password); return array( - 'password' => $e->encrypt($password, $salt, $member), - 'salt' => $salt, + 'password' => $e->encrypt($password, $salt, $member), + 'salt' => $salt, 'algorithm' => $algorithm, 'encryptor' => $e ); @@ -1251,29 +1206,6 @@ class Security extends Controller implements TemplateGlobalProvider self::$force_database_is_ready = $isReady; } - /** - * Enable or disable recording of login attempts - * through the {@link LoginRecord} object. - * - * @deprecated 4.0 Use the "Security.login_recording" config setting instead - * @param boolean $bool - */ - public static function set_login_recording($bool) - { - Deprecation::notice('4.0', 'Use the "Security.login_recording" config setting instead'); - self::$login_recording = (bool)$bool; - } - - /** - * @deprecated 4.0 Use the "Security.login_recording" config setting instead - * @return boolean - */ - public static function login_recording() - { - Deprecation::notice('4.0', 'Use the "Security.login_recording" config setting instead'); - return self::$login_recording; - } - /** * @config * @var string Set the default login dest @@ -1345,11 +1277,11 @@ class Security extends Controller implements TemplateGlobalProvider public static function get_template_global_variables() { return array( - "LoginURL" => "login_url", - "LogoutURL" => "logout_url", + "LoginURL" => "login_url", + "LogoutURL" => "logout_url", "LostPasswordURL" => "lost_password_url", - "CurrentMember" => "getCurrentUser", - "currentUser" => "getCurrentUser" + "CurrentMember" => "getCurrentUser", + "currentUser" => "getCurrentUser" ); } } diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php index b681fd530..da2db5944 100644 --- a/src/View/ViewableData.php +++ b/src/View/ViewableData.php @@ -312,7 +312,7 @@ class ViewableData implements IteratorAggregate */ public function castingHelper($field) { - $specs = $this->config()->get('casting'); + $specs = static::config()->get('casting'); if (isset($specs[$field])) { return $specs[$field]; } @@ -329,7 +329,7 @@ class ViewableData implements IteratorAggregate } // Fall back to default_cast - $default = self::config()->get('default_cast'); + $default = $this->config()->get('default_cast'); if (empty($default)) { throw new Exception("No default_cast"); } diff --git a/tests/behat/features/login.feature b/tests/behat/features/login.feature index f4ae26290..577b20c25 100644 --- a/tests/behat/features/login.feature +++ b/tests/behat/features/login.feature @@ -6,7 +6,7 @@ Feature: Log in Scenario: Bad login Given I log in with "bad@example.com" and "badpassword" - Then I will see a "error" log-in message + Then I should see "The provided details don't seem to be correct" Scenario: Valid login Given I am logged in with "ADMIN" permissions diff --git a/tests/php/Forms/GridField/GridFieldEditButtonTest.php b/tests/php/Forms/GridField/GridFieldEditButtonTest.php index 03166cf7b..8c6c5abaa 100644 --- a/tests/php/Forms/GridField/GridFieldEditButtonTest.php +++ b/tests/php/Forms/GridField/GridFieldEditButtonTest.php @@ -64,7 +64,7 @@ class GridFieldEditButtonTest extends SapphireTest public function testShowEditLinks() { if (Security::getCurrentUser()) { - Security::getCurrentUser()->logOut(); + Security::setCurrentUser(null); } $content = new CSSContentParser($this->gridField->FieldHolder()); diff --git a/tests/php/Security/BasicAuthTest.php b/tests/php/Security/BasicAuthTest.php index 1092b7fc5..031fd4438 100644 --- a/tests/php/Security/BasicAuthTest.php +++ b/tests/php/Security/BasicAuthTest.php @@ -87,7 +87,7 @@ class BasicAuthTest extends FunctionalTest $_SERVER['PHP_AUTH_USER'] = 'user-in-mygroup@test.com'; $_SERVER['PHP_AUTH_PW'] = 'test'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', null, $_SESSION, null, null, $_SERVER);; + $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', null, $_SESSION, null, null, $_SERVER); $this->assertEquals(200, $response->getStatusCode(), 'Valid user with required permission has access'); $_SERVER['PHP_AUTH_USER'] = $origUser; diff --git a/tests/php/Security/MemberAuthenticatorTest.php b/tests/php/Security/MemberAuthenticatorTest.php index 4c2bb4896..699ccac8c 100644 --- a/tests/php/Security/MemberAuthenticatorTest.php +++ b/tests/php/Security/MemberAuthenticatorTest.php @@ -3,22 +3,18 @@ namespace SilverStripe\Security\Tests; use SilverStripe\Core\Injector\Injector; -use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DataModel; use SilverStripe\ORM\FieldType\DBDatetime; -use SilverStripe\ORM\ValidationResult; -use SilverStripe\Security\MemberAuthenticator\CMSAuthenticator; -use SilverStripe\Security\PasswordEncryptor; -use SilverStripe\Security\PasswordEncryptor_PHPHash; +use SilverStripe\Security\Authenticator; +use SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator; +use SilverStripe\Security\MemberAuthenticator\CMSMemberLoginForm; +use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator; use SilverStripe\Security\Security; use SilverStripe\Security\Member; -use SilverStripe\Security\MemberAuthenticator\Authenticator; -use SilverStripe\Security\MemberAuthenticator\LoginForm; -use SilverStripe\Security\CMSMemberLoginForm; +use SilverStripe\Security\MemberAuthenticator\MemberLoginForm; use SilverStripe\Security\IdentityStore; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\SapphireTest; -use SilverStripe\Forms\FieldList; -use SilverStripe\Forms\Form; use SilverStripe\Control\HTTPRequest; class MemberAuthenticatorTest extends SapphireTest @@ -60,31 +56,31 @@ class MemberAuthenticatorTest extends SapphireTest public function testGenerateLoginForm() { - $authenticator = new Authenticator(); + $authenticator = new MemberAuthenticator(); $controller = new Security(); // Create basic login form $frontendResponse = $authenticator ->getLoginHandler($controller->link()) - ->handleRequest(new HTTPRequest('get', '/'), \SilverStripe\ORM\DataModel::inst()); + ->handleRequest(new HTTPRequest('get', '/'), DataModel::inst()); $this->assertTrue(is_array($frontendResponse)); $this->assertTrue(isset($frontendResponse['Form'])); - $this->assertTrue($frontendResponse['Form'] instanceof LoginForm); + $this->assertTrue($frontendResponse['Form'] instanceof MemberLoginForm); } - /* TO DO - reenable public function testGenerateCMSLoginForm() { - $authenticator = new Authenticator(); + /** @var CMSMemberAuthenticator $authenticator */ + $authenticator = new CMSMemberAuthenticator(); // Supports cms login form - $this->assertTrue(MemberAuthenticator::supports_cms()); - $cmsForm = MemberAuthenticator::get_cms_login_form($controller); + $this->assertGreaterThan(0, ($authenticator->supportedServices() & Authenticator::CMS_LOGIN)); + $cmsHandler = $authenticator->getLoginHandler('/'); + $cmsForm = $cmsHandler->loginForm(); $this->assertTrue($cmsForm instanceof CMSMemberLoginForm); } - */ /** @@ -92,7 +88,7 @@ class MemberAuthenticatorTest extends SapphireTest */ public function testAuthenticateByTempID() { - $authenticator = new CMSAuthenticator(); + $authenticator = new CMSMemberAuthenticator(); $member = new Member(); $member->Email = 'test1@test.com'; @@ -105,7 +101,7 @@ class MemberAuthenticatorTest extends SapphireTest $this->assertEmpty($tempID); // If the user logs in then they have a temp id - Injector::inst()->get(IdentityStore::class)->logIn($member, true, new HTTPRequest('GET', '/')); + Injector::inst()->get(IdentityStore::class)->logIn($member, true); $tempID = $member->TempIDHash; $this->assertNotEmpty($tempID); @@ -120,7 +116,7 @@ class MemberAuthenticatorTest extends SapphireTest $this->assertNotEmpty($result); $this->assertEquals($result->ID, $member->ID); - $this->assertEmpty($message); + $this->assertTrue($message->isValid()); // Test incorrect login $result = $authenticator->authenticate( @@ -132,9 +128,10 @@ class MemberAuthenticatorTest extends SapphireTest ); $this->assertEmpty($result); + $messages = $message->getMessages(); $this->assertEquals( _t('SilverStripe\\Security\\Member.ERRORWRONGCRED', 'The provided details don\'t seem to be correct. Please try again.'), - $message + $messages[0]['message'] ); } @@ -143,7 +140,7 @@ class MemberAuthenticatorTest extends SapphireTest */ public function testDefaultAdmin() { - $authenticator = new Authenticator(); + $authenticator = new MemberAuthenticator(); // Test correct login $result = $authenticator->authenticate( @@ -155,7 +152,7 @@ class MemberAuthenticatorTest extends SapphireTest ); $this->assertNotEmpty($result); $this->assertEquals($result->Email, Security::default_admin_username()); - $this->assertEmpty($message); + $this->assertTrue($message->isValid()); // Test incorrect login $result = $authenticator->authenticate( @@ -165,16 +162,17 @@ class MemberAuthenticatorTest extends SapphireTest ), $message ); + $messages = $message->getMessages(); $this->assertEmpty($result); $this->assertEquals( 'The provided details don\'t seem to be correct. Please try again.', - $message + $messages[0]['message'] ); } public function testDefaultAdminLockOut() { - $authenticator = new Authenticator(); + $authenticator = new MemberAuthenticator(); Config::inst()->update(Member::class, 'lock_out_after_incorrect_logins', 1); Config::inst()->update(Member::class, 'lock_out_delay_mins', 10); @@ -185,8 +183,7 @@ class MemberAuthenticatorTest extends SapphireTest [ 'Email' => 'admin', 'Password' => 'wrongpassword' - ], - $dummy + ] ); $this->assertFalse(Member::default_admin()->canLogin()->isValid()); diff --git a/tests/php/Security/MemberTest.php b/tests/php/Security/MemberTest.php index abbb5efc9..5d9f5b931 100644 --- a/tests/php/Security/MemberTest.php +++ b/tests/php/Security/MemberTest.php @@ -11,7 +11,6 @@ use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\Security\Member; -use SilverStripe\Security\MemberAuthenticator; use SilverStripe\Security\Security; use SilverStripe\Security\MemberPassword; use SilverStripe\Security\Group; @@ -875,7 +874,7 @@ class MemberTest extends FunctionalTest { $m1 = $this->objFromFixture(Member::class, 'grouplessmember'); - Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/')); + Injector::inst()->get(IdentityStore::class)->logIn($m1, true); $hashes = RememberLoginHash::get()->filter('MemberID', $m1->ID); $this->assertEquals($hashes->count(), 1); @@ -891,7 +890,7 @@ class MemberTest extends FunctionalTest */ $m1 = $this->objFromFixture(Member::class, 'noexpiry'); - Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/')); + Injector::inst()->get(IdentityStore::class)->logIn($m1, true); $firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->first(); $this->assertNotNull($firstHash); @@ -970,7 +969,7 @@ class MemberTest extends FunctionalTest * @var Member $m1 */ $m1 = $this->objFromFixture(Member::class, 'noexpiry'); - Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/')); + Injector::inst()->get(IdentityStore::class)->logIn($m1, true); $firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->first(); $this->assertNotNull($firstHash); @@ -1029,10 +1028,10 @@ class MemberTest extends FunctionalTest $m1 = $this->objFromFixture(Member::class, 'noexpiry'); // First device - Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/')); + Injector::inst()->get(IdentityStore::class)->logIn($m1, true); Cookie::set('alc_device', null); // Second device - Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/')); + Injector::inst()->get(IdentityStore::class)->logIn($m1, true); // Hash of first device $firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->first(); @@ -1105,7 +1104,7 @@ class MemberTest extends FunctionalTest // Logging out from any device when all login hashes should be removed RememberLoginHash::config()->update('logout_across_devices', true); - Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/')); + Injector::inst()->get(IdentityStore::class)->logIn($m1, true); $response = $this->get('Security/logout', $this->session()); $this->assertEquals( RememberLoginHash::get()->filter('MemberID', $m1->ID)->count(), @@ -1423,17 +1422,17 @@ class MemberTest extends FunctionalTest /** @var Member $adminMember */ $adminMember = $this->objFromFixture(Member::class, 'admin'); - $memberID = Member::actAs($adminMember, function () { - return Member::currentUserID(); + $member = Member::actAs($adminMember, function () { + return Security::getCurrentUser(); }); - $this->assertEquals($adminMember->ID, $memberID); + $this->assertEquals($adminMember->ID, $member->ID); // Check nesting - $memberID = Member::actAs($adminMember, function () { + $member = Member::actAs($adminMember, function () { return Member::actAs(null, function () { - return Member::currentUserID(); + return Security::getCurrentUser(); }); }); - $this->assertEmpty($memberID); + $this->assertEmpty($member); } } diff --git a/tests/php/Security/SecurityTest.php b/tests/php/Security/SecurityTest.php index 1b21b6c6d..adbf3ffc4 100644 --- a/tests/php/Security/SecurityTest.php +++ b/tests/php/Security/SecurityTest.php @@ -2,18 +2,16 @@ namespace SilverStripe\Security\Tests; -use PhpConsole\Auth; +use SilverStripe\Dev\Debug; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\FieldType\DBClassName; use SilverStripe\ORM\DB; use SilverStripe\ORM\ValidationResult; -use SilverStripe\Security\Authenticator; use SilverStripe\Security\LoginAttempt; use SilverStripe\Security\Member; -use SilverStripe\Security\MemberAuthenticator; +use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator; use SilverStripe\Security\Security; -use SilverStripe\Security\Permission; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Convert; use SilverStripe\Dev\FunctionalTest; @@ -48,13 +46,9 @@ class SecurityTest extends FunctionalTest protected function setUp() { - // This test assumes that MemberAuthenticator is present and the default - // $this->priorAuthenticators = Authenticator::get_authenticators(); - // $this->priorDefaultAuthenticator = Authenticator::get_default_authenticator(); - // Set to an empty array of authenticators to enable the default - Config::modify()->set(Authenticator::class, 'authenticators', []); - Config::modify()->set(Authenticator::class, 'default_authenticator', MemberAuthenticator::class); + Config::modify()->set(MemberAuthenticator::class, 'authenticators', []); + Config::modify()->set(MemberAuthenticator::class, 'default_authenticator', MemberAuthenticator::class); // And that the unique identified field is 'Email' $this->priorUniqueIdentifierField = Member::config()->unique_identifier_field; @@ -233,7 +227,7 @@ class SecurityTest extends FunctionalTest /* View the Security/login page */ $response = $this->get(Config::inst()->get(Security::class, 'login_url')); - $items = $this->cssParser()->getBySelector('#LoginForm_LoginForm input.action'); + $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm input.action'); /* We have only 1 input, one to allow the user to log in as someone else */ $this->assertEquals(count($items), 1, 'There is 1 input, allowing the user to log in as someone else.'); @@ -242,7 +236,7 @@ class SecurityTest extends FunctionalTest /* Submit the form, using only the logout action and a hidden field for the authenticator */ $response = $this->submitForm( - 'LoginForm_LoginForm', + 'MemberLoginForm_LoginForm', null, array( 'action_logout' => 1, @@ -267,7 +261,7 @@ class SecurityTest extends FunctionalTest /* Attempt to get into the admin section */ $response = $this->get(Config::inst()->get(Security::class, 'login_url')); - $items = $this->cssParser()->getBySelector('#LoginForm_LoginForm input.text'); + $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm input.text'); /* We have 2 text inputs - one for email, and another for the password */ $this->assertEquals(count($items), 2, 'There are 2 inputs - one for email, another for password'); @@ -286,11 +280,11 @@ class SecurityTest extends FunctionalTest $this->get(Config::inst()->get(Security::class, 'login_url')); $items = $this ->cssParser() - ->getBySelector('#LoginForm_LoginForm #LoginForm_LoginForm_Email'); + ->getBySelector('#MemberLoginForm_LoginForm #MemberLoginForm_LoginForm_Email'); $this->assertEquals(1, count($items)); $this->assertEmpty((string)$items[0]->attributes()->value); $this->assertEquals('off', (string)$items[0]->attributes()->autocomplete); - $form = $this->cssParser()->getBySelector('#LoginForm_LoginForm'); + $form = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm'); $this->assertEquals(1, count($form)); $this->assertEquals('off', (string)$form[0]->attributes()->autocomplete); @@ -300,11 +294,11 @@ class SecurityTest extends FunctionalTest $this->get(Config::inst()->get(Security::class, 'login_url')); $items = $this ->cssParser() - ->getBySelector('#LoginForm_LoginForm #LoginForm_LoginForm_Email'); + ->getBySelector('#MemberLoginForm_LoginForm #MemberLoginForm_LoginForm_Email'); $this->assertEquals(1, count($items)); $this->assertEquals('myuser@silverstripe.com', (string)$items[0]->attributes()->value); $this->assertNotEquals('off', (string)$items[0]->attributes()->autocomplete); - $form = $this->cssParser()->getBySelector('#LoginForm_LoginForm'); + $form = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm'); $this->assertEquals(1, count($form)); $this->assertNotEquals('off', (string)$form[0]->attributes()->autocomplete); } @@ -482,11 +476,11 @@ class SecurityTest extends FunctionalTest Member::config()->lock_out_delay_mins = 15; // Login with a wrong password for more than the defined threshold - for ($i = 1; $i <= Member::config()->lock_out_after_incorrect_logins+1; $i++) { + for ($i = 1; $i <= (Member::config()->lock_out_after_incorrect_logins+1); $i++) { $this->doTestLoginForm('testuser@example.com', 'incorrectpassword'); $member = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'test')); - if ($i < Member::config()->lock_out_after_incorrect_logins) { + if ($i < Member::config()->get('lock_out_after_incorrect_logins')) { $this->assertNull( $member->LockedOutUntil, 'User does not have a lockout time set if under threshold for failed attempts' @@ -505,18 +499,16 @@ class SecurityTest extends FunctionalTest 'User has a lockout time set after too many failed attempts' ); } - - $msg = _t( - 'SilverStripe\\Security\\Member.ERRORLOCKEDOUT2', - 'Your account has been temporarily disabled because of too many failed attempts at ' . - 'logging in. Please try again in {count} minutes.', - null, - array('count' => Member::config()->lock_out_delay_mins) - ); - if ($i > Member::config()->lock_out_after_incorrect_logins) { - $this->assertHasMessage($msg); - } } + $msg = _t( + 'SilverStripe\\Security\\Member.ERRORLOCKEDOUT2', + 'Your account has been temporarily disabled because of too many failed attempts at ' . + 'logging in. Please try again in {count} minutes.', + null, + array('count' => Member::config()->lock_out_delay_mins) + ); + $this->assertHasMessage($msg); + $this->doTestLoginForm('testuser@example.com', '1nitialPassword'); $this->assertNull( @@ -597,14 +589,14 @@ class SecurityTest extends FunctionalTest $attempt = DataObject::get_one( LoginAttempt::class, array( - '"LoginAttempt"."Email"' => 'testuser@example.com' + '"LoginAttempt"."Email"' => 'testuser@example.com' ) ); $this->assertTrue(is_object($attempt)); $member = DataObject::get_one( Member::class, array( - '"Member"."Email"' => 'testuser@example.com' + '"Member"."Email"' => 'testuser@example.com' ) ); $this->assertEquals($attempt->Status, 'Failure'); @@ -696,7 +688,7 @@ class SecurityTest extends FunctionalTest $this->get(Config::inst()->get(Security::class, 'login_url')); return $this->submitForm( - "LoginForm_LoginForm", + "MemberLoginForm_LoginForm", null, array( 'Email' => $email, @@ -750,7 +742,7 @@ class SecurityTest extends FunctionalTest */ protected function getValidationResult() { - $result = $this->session()->inst_get('FormInfo.LoginForm_LoginForm.result'); + $result = $this->session()->inst_get('FormInfo.MemberLoginForm_LoginForm.result'); if ($result) { return unserialize($result); }