From e226b67d06523291865cdfec3edb52e64c436d8e Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Sat, 22 Apr 2017 16:30:10 +1200 Subject: [PATCH] Refactoring of authenticators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Further down the line, I'm only returning the `Member` on the doLogin, so it's possible for the Handler or Extending Handler to move to a second step. Also cleaned up some minor typos I ran in to. Nothing major. This solution works and is manually tested for now. Supports multiple login forms that end up in the correct handler. I haven't gotten past the handler yet, as I've yet to refactor my Yubiauth implementation. FIX: Corrections to the multi-login-form support. Importantly, the system provide a URL-space for each handler, e.g. “Security/login/default” and “Security/login/other”. This is much cleaner than identifying the active authenticator by a get parameter, and means that the tabbed interface is only needed on the very first view. Note that you can test this without a module simply by loading the default authenticator twice: SilverStripe\Security\Security: authenticators: default: SilverStripe\Security\MemberAuthenticator\Authenticator other: SilverStripe\Security\MemberAuthenticator\Authenticator FIX: Refactor delegateToHandler / delegateToHandlers to have less duplicated code. --- _config/security.yml | 9 +- src/Core/Config/Config_ForClass.php | 2 +- src/Forms/FormRequestHandler.php | 2 +- src/Security/Authenticator.php | 147 +++----- src/Security/BasicAuth.php | 9 +- .../Authenticator.php} | 115 +++--- .../CMSLoginHandler.php} | 8 +- .../ChangePasswordForm.php | 3 +- .../ChangePasswordHandler.php | 5 +- .../LoginForm.php} | 18 +- .../LoginHandler.php} | 169 +++++---- .../LostPasswordHandler.php | 259 +++++++++++++ src/Security/Security.php | 352 +++++++++++------- src/View/ViewableData.php | 2 +- tests/php/Security/SecurityTest.php | 8 +- 15 files changed, 700 insertions(+), 408 deletions(-) rename src/Security/{MemberAuthenticator.php => MemberAuthenticator/Authenticator.php} (61%) rename src/Security/{CMSMemberLoginHandler.php => MemberAuthenticator/CMSLoginHandler.php} (92%) rename src/Security/{ => MemberAuthenticator}/ChangePasswordForm.php (96%) rename src/Security/{ => MemberAuthenticator}/ChangePasswordHandler.php (96%) rename src/Security/{MemberLoginForm.php => MemberAuthenticator/LoginForm.php} (95%) rename src/Security/{MemberLoginHandler.php => MemberAuthenticator/LoginHandler.php} (58%) create mode 100644 src/Security/MemberAuthenticator/LostPasswordHandler.php diff --git a/_config/security.yml b/_config/security.yml index 10051c588..49e90763d 100644 --- a/_config/security.yml +++ b/_config/security.yml @@ -1,4 +1,9 @@ -SilverStripe\Security\MemberLoginForm: +SilverStripe\Security\MemberAuthenticator\LoginForm: required_fields: - Email - - Password \ No newline at end of file + - Password + +SilverStripe\Security\Security: + default_authenticator: SilverStripe\Security\MemberAuthenticator\Authenticator + authenticators: + - SilverStripe\Security\MemberAuthenticator\Authenticator diff --git a/src/Core/Config/Config_ForClass.php b/src/Core/Config/Config_ForClass.php index b89e5007b..448bd9443 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 */ - protected $class; + public $class; /** * @param string|object $class diff --git a/src/Forms/FormRequestHandler.php b/src/Forms/FormRequestHandler.php index a2115e8e0..526f488bb 100644 --- a/src/Forms/FormRequestHandler.php +++ b/src/Forms/FormRequestHandler.php @@ -228,7 +228,7 @@ class FormRequestHandler extends RequestHandler // First, try a handler method on the controller (has been checked for allowed_actions above already) $controller = $this->form->getController(); if ($controller && $controller->hasMethod($funcName)) { - return $controller->$funcName($vars, $this->form, $request); + return $controller->$funcName($vars, $this->form, $request, $this); } // Otherwise, try a handler method on the form request handler. diff --git a/src/Security/Authenticator.php b/src/Security/Authenticator.php index 66bbc7843..beee5158c 100644 --- a/src/Security/Authenticator.php +++ b/src/Security/Authenticator.php @@ -16,125 +16,74 @@ use SilverStripe\Forms\Form; * * @author Markus Lanthaler */ -abstract class Authenticator +interface Authenticator { - use Injectable; - use Configurable; - use Extensible; - public function __construct() - { - $this->constructExtensions(); - } + const LOGIN = 1; + const LOGOUT = 2; + const CHANGE_PASSWORD = 4; + const RESET_PASSWORD = 8; + const CMS_LOGIN = 16; /** - * This variable holds all authenticators that should be used + * Returns the services supported by this authenticator * - * @var array - */ - private static $authenticators = []; - - /** - * Used to influence the order of authenticators on the login-screen - * (default shows first). + * The number should be a bitwise-OR of 1 or more of the following constants: + * Authenticator::LOGIN, Authenticator::LOGOUT, Authenticator::CHANGE_PASSWORD, + * Authenticator::RESET_PASSWORD, or Authenticator::CMS_LOGIN * - * @var string + * @return int */ - private static $default_authenticator = MemberAuthenticator::class; - + public function supportedServices(); /** - * Method to authenticate an user + * Return RequestHandler to manage the log-in process. * - * @param array $RAW_data Raw data to authenticate the user - * @param Form $form Optional: If passed, better error messages can be - * produced by using - * {@link Form::sessionMessage()} - * @return bool|Member Returns FALSE if authentication fails, otherwise - * the member object - */ - public static function authenticate($RAW_data, Form $form = null) - { - } - - /** - * Method that creates the login form for this authentication method + * The default URL of the RequetHandler should return the initial log-in form, any other + * URL may be added for other steps & processing. * - * @param Controller $controller The parent controller, necessary to create the - * appropriate form action tag - * @return Form Returns the login form to use with this authentication - * method - */ - public static function get_login_form(Controller $controller) - { - } - - /** - * Method that creates the re-authentication form for the in-CMS view + * URL-handling methods may return an array [ "Form" => (form-object) ] which can then + * be merged into a default controller. * - * @param Controller $controller + * @param $link The base link to use for this RequestHnadler */ - public static function get_cms_login_form(Controller $controller) - { - } + public function getLoginHandler($link); /** - * Determine if this authenticator supports in-cms reauthentication + * @todo + */ + public function getCMSLoginHandler($link); + + /** + * Return RequestHandler to manage the change-password process. * - * @return bool - */ - public static function supports_cms() - { - return false; - } - - /** - * Check if a given authenticator is registered + * The default URL of the RequetHandler should return the initial change-password form, + * any other URL may be added for other steps & processing. * - * @param string $authenticator Name of the authenticator class to check - * @return bool Returns TRUE if the authenticator is registered, FALSE - * otherwise. - */ - public static function is_registered($authenticator) - { - $authenticators = self::config()->get('authenticators'); - if (count($authenticators) === 0) { - $authenticators = [self::config()->get('default_authenticator')]; - } - - return in_array($authenticator, $authenticators, true); - } - - - /** - * Get all registered authenticators + * URL-handling methods may return an array [ "Form" => (form-object) ] which can then + * be merged into a default controller. * - * @return array Returns an array with the class names of all registered - * authenticators. + * @param $link The base link to use for this RequestHnadler */ - public static function get_authenticators() - { - $authenticators = self::config()->get('authenticators'); - $default = self::config()->get('default_authenticator'); - - if (count($authenticators) === 0) { - $authenticators = [$default]; - } - // put default authenticator first (mainly for tab-order on loginform) - // But only if there's no other authenticator - if (($key = array_search($default, $authenticators, true)) && count($authenticators) > 1) { - unset($authenticators[$key]); - array_unshift($authenticators, $default); - } - - return $authenticators; - } + public function getChangePasswordHandler($link); /** - * @return string + * @todo */ - public static function get_default_authenticator() - { - return self::config()->get('default_authenticator'); - } + public function getLostPasswordHandler($link); + + /** + * 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 + * @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(); } diff --git a/src/Security/BasicAuth.php b/src/Security/BasicAuth.php index 1645f0909..cf4c5d8b3 100644 --- a/src/Security/BasicAuth.php +++ b/src/Security/BasicAuth.php @@ -8,6 +8,9 @@ use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Configurable; use SilverStripe\Dev\SapphireTest; +use SilverStripe\Core\Injector\Injector; + +use SilverStripe\Security\MemberAuthenticator\Authenticator; /** * Provides an interface to HTTP basic authentication. @@ -82,10 +85,12 @@ class BasicAuth $member = null; if (isset($_SERVER['PHP_AUTH_USER']) && isset($_SERVER['PHP_AUTH_PW'])) { - $member = MemberAuthenticator::authenticate(array( + $authenticator = Injector::inst()->get(Authenticator::class); + + $member = $authenticator->authenticate([ 'Email' => $_SERVER['PHP_AUTH_USER'], 'Password' => $_SERVER['PHP_AUTH_PW'], - ), null); + ], $dummy); } if (!$member && $tryUsingSessionLogin) { diff --git a/src/Security/MemberAuthenticator.php b/src/Security/MemberAuthenticator/Authenticator.php similarity index 61% rename from src/Security/MemberAuthenticator.php rename to src/Security/MemberAuthenticator/Authenticator.php index 85431bb78..527ed36e2 100644 --- a/src/Security/MemberAuthenticator.php +++ b/src/Security/MemberAuthenticator/Authenticator.php @@ -1,32 +1,50 @@ */ -class MemberAuthenticator extends Authenticator +class Authenticator implements BaseAuthenticator { + 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 | BaseAuthenticator::CMS_LOGIN; + } + /** - * Contains encryption algorithm identifiers. - * If set, will migrate to new precision-safe password hashing - * upon login. See http://open.silverstripe.org/ticket/3004 - * - * @var array + * @inherit */ - private static $migrate_legacy_hashes = array( - 'md5' => 'md5_v2.4', - 'sha1' => 'sha1_v2.4' - ); + public function authenticate($data, &$message) + { + $success = null; + + // Find authenticated member + $member = $this->authenticateMember($data, $message, $success); + + // Optionally record every login attempt as a {@link LoginAttempt} object + $this->recordLoginAttempt($data, $member, $success); + + if ($member) { + Session::clear('BackURL'); + } + + return $success ? $member : null; + } /** * Attempt to find and authenticate member if possible from the given data @@ -36,7 +54,7 @@ class MemberAuthenticator extends Authenticator * @param bool &$success Success flag * @return Member Found member, regardless of successful login */ - protected static function authenticate_member($data, $form, &$success) + protected function authenticateMember($data, &$message, &$success) { // Default success to false $success = false; @@ -94,9 +112,12 @@ class MemberAuthenticator extends Authenticator if ($member) { $member->registerFailedLogin(); } - if ($form) { - $form->setSessionValidationResult($result, true); - } + $message = implode("; ", array_map( + function ($message) { + return $message['message']; + }, + $result->getMessages() + )); } else { if ($member) { $member->registerSuccessfulLogin(); @@ -112,9 +133,8 @@ class MemberAuthenticator extends Authenticator * * @param array $data * @param Member $member - * @param bool $success */ - protected static function record_login_attempt($data, $member, $success) + protected function recordLoginAttempt($data, $member) { if (!Security::config()->login_recording) { return; @@ -154,66 +174,31 @@ class MemberAuthenticator extends Authenticator } /** - * Method to authenticate an user - * - * @param array $data Raw data to authenticate the user - * @param Form $form Optional: If passed, better error messages can be - * produced by using - * {@link Form::sessionMessage()} - * @return bool|Member Returns FALSE if authentication fails, otherwise - * the member object - * @see Security::setDefaultAdmin() + * @inherit */ - public static function authenticate($data, Form $form = null) + public function getLostPasswordHandler($link) { - // Find authenticated member - $member = static::authenticate_member($data, $form, $success); - - // Optionally record every login attempt as a {@link LoginAttempt} object - static::record_login_attempt($data, $member, $success); - - // Legacy migration to precision-safe password hashes. - // A login-event with cleartext passwords is the only time - // when we can rehash passwords to a different hashing algorithm, - // bulk-migration doesn't work due to the nature of hashing. - // See PasswordEncryptor_LegacyPHPHash class. - if ($success && $member && isset(self::$migrate_legacy_hashes[$member->PasswordEncryption])) { - $member->Password = $data['Password']; - $member->PasswordEncryption = self::$migrate_legacy_hashes[$member->PasswordEncryption]; - $member->write(); - } - - if ($success) { - Session::clear('BackURL'); - } - - return $success ? $member : null; + return LostPasswordHandler::create($link, $this); } - /** - * Method that creates the login form for this authentication method - * - * @param Controller $controller The parent controller, necessary to create the - * appropriate form action tag - * @return Form Returns the login form to use with this authentication - * method + * @inherit */ - public static function get_login_form(Controller $controller) + public function getChangePasswordHandler($link) { - /** @skipUpgrade */ - return MemberLoginForm::create($controller, self::class, "LoginForm"); + return ChangePasswordHandler::create($link, $this); } - public static function get_cms_login_form(Controller $controller) + /** + * @inherit + */ + public function getLoginHandler($link) { - /** @skipUpgrade */ - return CMSMemberLoginForm::create($controller, self::class, "LoginForm"); + return LoginHandler::create($link, $this); } - public static function supports_cms() + public function getCMSLoginHandler($link) { - // Don't automatically support subclasses of MemberAuthenticator - return get_called_class() === __CLASS__; + return CMSMemberLoginHandler::create($controller, self::class, "LoginForm"); } } diff --git a/src/Security/CMSMemberLoginHandler.php b/src/Security/MemberAuthenticator/CMSLoginHandler.php similarity index 92% rename from src/Security/CMSMemberLoginHandler.php rename to src/Security/MemberAuthenticator/CMSLoginHandler.php index cec76bc2a..ca469e23e 100644 --- a/src/Security/CMSMemberLoginHandler.php +++ b/src/Security/MemberAuthenticator/CMSLoginHandler.php @@ -1,11 +1,11 @@ performLogin($data)) { return $this->logInUserAndRedirect($data); @@ -78,7 +78,7 @@ PHP * @param array $data * @return HTTPResponse */ - protected function logInUserAndRedirect($data) + protected function logInUserAndRedirect($data, $formHandler) { // Check password expiry if (Member::currentUser()->isPasswordExpired()) { diff --git a/src/Security/ChangePasswordForm.php b/src/Security/MemberAuthenticator/ChangePasswordForm.php similarity index 96% rename from src/Security/ChangePasswordForm.php rename to src/Security/MemberAuthenticator/ChangePasswordForm.php index e2db15e14..740f7f51d 100644 --- a/src/Security/ChangePasswordForm.php +++ b/src/Security/MemberAuthenticator/ChangePasswordForm.php @@ -1,6 +1,6 @@ ' @@ -194,14 +198,6 @@ class MemberLoginForm extends LoginForm return $this; } - /** - * @return MemberLoginHandler - */ - protected function buildRequestHandler() - { - return MemberLoginHandler::create($this); - } - /** * The name of this login form, to display in the frontend * Replaces Authenticator::get_name() diff --git a/src/Security/MemberLoginHandler.php b/src/Security/MemberAuthenticator/LoginHandler.php similarity index 58% rename from src/Security/MemberLoginHandler.php rename to src/Security/MemberAuthenticator/LoginHandler.php index 0a4d2e241..f11f55cb0 100644 --- a/src/Security/MemberLoginHandler.php +++ b/src/Security/MemberAuthenticator/LoginHandler.php @@ -1,20 +1,26 @@ 'login', + ]; /** * Since the logout and dologin actions may be conditionally removed, it's necessary to ensure these @@ -24,22 +30,74 @@ class MemberLoginHandler extends FormRequestHandler * @config */ private static $allowed_actions = [ + 'login', + 'LoginForm', 'dologin', 'logout', ]; + private $link = null; + + /** + * @param string $link The URL to recreate this request handler + * @param Authenticator $authenticator The + */ + public function __construct($link, Authenticator $authenticator) + { + $this->link = $link; + $this->authenticator = $authenticator; + parent::__construct($link, $this); + } + + /** + * Return a link to this request handler. + * The link returned is supplied in the constructor + * @return string + */ + public function link($action = null) + { + if ($action) { + return Controller::join_links($this->link, $action); + } else { + return $this->link; + } + } + + /** + * URL handler for the log-in screen + */ + public function login() + { + return [ + 'Form' => $this->loginForm(), + ]; + } + + /** + * Return the MemberLoginForm form + */ + public function loginForm() + { + return LoginForm::create( + $this, + get_class($this->authenticator), + 'LoginForm' + ); + } + /** * Login form handler method * * This method is called when the user clicks on "Log in" * * @param array $data Submitted data + * @param LoginHandler $formHandler * @return HTTPResponse */ - public function dologin($data) + public function doLogin($data, $formHandler) { if ($this->performLogin($data)) { - return $this->logInUserAndRedirect($data); + return $this->logInUserAndRedirect($data, $formHandler); } /** @skipUpgrade */ @@ -48,25 +106,15 @@ class MemberLoginHandler extends FormRequestHandler Session::set('SessionForms.MemberLoginForm.Remember', isset($data['Remember'])); } + return $this->redirectBack(); // Fail to login redirects back to form - return $this->redirectBackToForm(); + return $formHandler->redirectBackToForm(); } - /** - * Redirect to password recovery form - * - * @return HTTPResponse - */ - public function redirectToLostPassword() - { - $lostPasswordLink = Security::singleton()->Link('lostpassword'); - return $this->redirect($this->addBackURLParam($lostPasswordLink)); - } public function getReturnReferer() { - // Home of login form is always this url - return Security::singleton()->Link('login'); + return $this->link(); } /** @@ -84,7 +132,7 @@ class MemberLoginHandler extends FormRequestHandler * @param array $data * @return HTTPResponse */ - protected function logInUserAndRedirect($data) + protected function logInUserAndRedirect($data, $formHandler) { Session::clear('SessionForms.MemberLoginForm.Email'); Session::clear('SessionForms.MemberLoginForm.Remember'); @@ -152,13 +200,13 @@ class MemberLoginHandler extends FormRequestHandler */ public function performLogin($data) { - $member = call_user_func_array( - [$this->authenticator_class, 'authenticate'], - [$data, $this->form] - ); + $message = null; + $member = $this->authenticator->authenticate($data, $message); if ($member) { $member->LogIn(isset($data['Remember'])); return $member; + } else { + Security::setLoginMessage($message, ValidationResult::TYPE_ERROR); } // No member, can't login @@ -166,61 +214,6 @@ class MemberLoginHandler extends FormRequestHandler return null; } - /** - * Forgot password form handler method. - * Called when the user clicks on "I've lost my password". - * Extensions can use the 'forgotPassword' method to veto executing - * the logic, by returning FALSE. In this case, the user will be redirected back - * to the form without further action. It is recommended to set a message - * in the form detailing why the action was denied. - * - * @skipUpgrade - * @param array $data Submitted data - * @return HTTPResponse - */ - public function forgotPassword($data) - { - // Ensure password is given - if (empty($data['Email'])) { - $this->form->sessionMessage( - _t('SilverStripe\\Security\\Member.ENTEREMAIL', 'Please enter an email address to get a password reset link.'), - 'bad' - ); - return $this->redirectToLostPassword(); - } - - // Find existing member - /** @var Member $member */ - $member = Member::get()->filter("Email", $data['Email'])->first(); - - // Allow vetoing forgot password requests - $results = $this->extend('forgotPassword', $member); - if ($results && is_array($results) && in_array(false, $results, true)) { - return $this->redirectToLostPassword(); - } - - if ($member) { - $token = $member->generateAutologinTokenAndStoreHash(); - - Email::create() - ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail') - ->setData($member) - ->setSubject(_t('SilverStripe\\Security\\Member.SUBJECTPASSWORDRESET', "Your password reset link", 'Email subject')) - ->addData('PasswordResetLink', Security::getPasswordResetLink($member, $token)) - ->setTo($member->Email) - ->send(); - } - - // Avoid information disclosure by displaying the same status, - // regardless wether the email address actually exists - $link = Controller::join_links( - Security::singleton()->Link('passwordsent'), - rawurlencode($data['Email']), - '/' - ); - return $this->redirect($this->addBackURLParam($link)); - } - /** * Invoked if password is expired and must be changed * @@ -229,7 +222,7 @@ class MemberLoginHandler extends FormRequestHandler */ protected function redirectToChangePassword() { - $cp = ChangePasswordForm::create($this->form->getController(), 'ChangePasswordForm'); + $cp = ChangePasswordForm::create($this, 'ChangePasswordForm'); $cp->sessionMessage( _t('SilverStripe\\Security\\Member.PASSWORDEXPIRED', 'Your password has expired. Please choose a new one.'), 'good' @@ -237,4 +230,18 @@ class MemberLoginHandler extends FormRequestHandler $changedPasswordLink = Security::singleton()->Link('changepassword'); return $this->redirect($this->addBackURLParam($changedPasswordLink)); } + + + + /** + * @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/LostPasswordHandler.php b/src/Security/MemberAuthenticator/LostPasswordHandler.php new file mode 100644 index 000000000..5deb6d5c1 --- /dev/null +++ b/src/Security/MemberAuthenticator/LostPasswordHandler.php @@ -0,0 +1,259 @@ + 'passwordsent', + '' => 'lostpassword', + ]; + + /** + * 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 + */ + private static $allowed_actions = [ + 'lostpassword', + 'LostPasswordForm', + 'passwordsent', + ]; + + private $link = null; + + /** + * @param $link The URL to recreate this request handler + */ + public function __construct($link) + { + $this->link = $link; + parent::__construct(); + } + + /** + * Return a link to this request handler. + * The link returned is supplied in the constructor + * @return string + */ + public function link($action = null) + { + if ($action) { + return Controller::join_links($this->link, $action); + } else { + return $this->link; + } + } + + /** + * URL handler for the initial lost-password screen + */ + public function lostpassword() + { + + $message = _t( + '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(), + ]; + } + + /** + * Show the "password sent" page, after a user has requested + * to reset their password. + */ + public function passwordsent() + { + $request = $this->getRequest(); + $email = Convert::raw2xml(rawurldecode($request->param('EmailAddress')) . '.' . $request->getExtension()); + + $message = _t( + 'Security.PASSWORDSENTTEXT', + "Thank you! A reset link has been sent to '{email}', provided an account exists for this email" + . " address.", + [ 'email' => Convert::raw2xml($email) ] + ); + + return [ + 'Title' => _t( + 'Security.PASSWORDSENTHEADER', + "Password reset link sent to '{email}'", + array('email' => $email) + ), + 'Content' => DBField::create_field('HTMLFragment', "

$message

"), + 'Email' => $email + ]; + } + + + /** + * Factory method for the lost password form + * + * @skipUpgrade + * @return Form Returns the lost password form + */ + public function lostPasswordForm() + { + return LoginForm::create( + $this, + $this->authenticatorClass, + 'LostPasswordForm', + new FieldList( + new EmailField('Email', _t('Member.EMAIL', 'Email')) + ), + new FieldList( + new FormAction( + 'forgotPassword', + _t('Security.BUTTONSEND', 'Send me the password reset link') + ) + ), + false + ); + } + + /** + * Redirect to password recovery form + * + * @return HTTPResponse + */ + public function redirectToLostPassword() + { + $lostPasswordLink = Security::singleton()->Link('lostpassword'); + return $this->redirect($this->addBackURLParam($lostPasswordLink)); + } + + public function getReturnReferer() + { + return $this->link(); + } + + /** + * Log out form handler method + * + * This method is called when the user clicks on "logout" on the form + * created when the parameter $checkCurrentUser of the + * {@link __construct constructor} was set to TRUE and the user was + * currently logged in. + * + * @return HTTPResponse + */ + public function logout() + { + 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". + * Extensions can use the 'forgotPassword' method to veto executing + * the logic, by returning FALSE. In this case, the user will be redirected back + * to the form without further action. It is recommended to set a message + * in the form detailing why the action was denied. + * + * @skipUpgrade + * @param array $data Submitted data + * @return HTTPResponse + */ + public function forgotPassword($data) + { + // 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.'), + 'bad' + ); + return $this->redirectToLostPassword(); + } + + // Find existing member + /** @var Member $member */ + $member = Member::get()->filter("Email", $data['Email'])->first(); + + // Allow vetoing forgot password requests + $results = $this->extend('forgotPassword', $member); + if ($results && is_array($results) && in_array(false, $results, true)) { + return $this->redirectToLostPassword(); + } + + if ($member) { + $token = $member->generateAutologinTokenAndStoreHash(); + + Email::create() + ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail') + ->setData($member) + ->setSubject(_t('Member.SUBJECTPASSWORDRESET', "Your password reset link", 'Email subject')) + ->addData('PasswordResetLink', Security::getPasswordResetLink($member, $token)) + ->setTo($member->Email) + ->send(); + } + + // Avoid information disclosure by displaying the same status, + // regardless wether the email address actually exists + $link = Controller::join_links( + $this->link('passwordsent'), + 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/Security.php b/src/Security/Security.php index 13b4fe9af..06f1b311b 100644 --- a/src/Security/Security.php +++ b/src/Security/Security.php @@ -9,7 +9,9 @@ use SilverStripe\Control\Controller; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; +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; @@ -20,6 +22,7 @@ use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\FormAction; use SilverStripe\ORM\ArrayList; +use SilverStripe\ORM\DataModel; use SilverStripe\ORM\DB; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\FieldType\DBField; @@ -30,6 +33,7 @@ use SilverStripe\View\TemplateGlobalProvider; use Exception; use SilverStripe\View\ViewableData_Customised; use Subsite; +use SilverStripe\Core\Injector\Injector; /** * Implements a basic security model @@ -206,7 +210,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. */ - static $force_database_is_ready = null; + protected static $force_database_is_ready = null; /** * When the database has once been verified as ready, it will not do the @@ -214,7 +218,58 @@ class Security extends Controller implements TemplateGlobalProvider * * @var bool */ - static $database_is_ready = false; + protected static $database_is_ready = false; + + protected static $authenticators = []; + + protected static $default_authenticator = MemberAuthenticator\Authenticator::class; + + /** + * Get all registered authenticators + * + * @return array Return an array of Authenticator objects + */ + public static function getAuthenticators() + { + $authenticatorClasses = self::config()->authenticators; + $default = self::config()->default_authenticator; + + if (!$authenticatorClasses) { + if ($default) { + $authenticatorClasses = [$default]; + } else { + return []; + } + } + + // put default authenticator first (mainly for tab-order on loginform) + // But only if there's no other authenticator + if (($key = array_search($default, $authenticatorClasses, true)) && count($$authenticatorClasses) > 1) { + unset($authenticatorClasses[$key]); + array_unshift($authenticatorClasses, $default); + } + + return array_map(function ($class) { + return Injector::inst()->get($class); + }, $authenticatorClasses); + } + + /** + * Check if a given authenticator is registered + * + * @param string $authenticator Name of the authenticator class to check + * @return bool Returns TRUE if the authenticator is registered, FALSE + * otherwise. + */ + public static function hasAuthenticator($authenticator) + { + $authenticators = self::config()->get('authenticators'); + if (count($authenticators) === 0) { + $authenticators = [self::config()->get('default_authenticator')]; + } + + return in_array($authenticator, $authenticators, true); + } /** * Register that we've had a permission failure trying to view the given page @@ -304,10 +359,10 @@ class Security extends Controller implements TemplateGlobalProvider } // Somewhat hackish way to render a login form with an error message. - $me = new Security(); - $form = $me->LoginForm(); - $form->sessionMessage($message, ValidationResult::TYPE_WARNING); - Session::set('MemberLoginForm.force_message', 1); +// $me = new Security(); +// $form = $me->LoginForm(); +// $form->sessionMessage($message, ValidationResult::TYPE_WARNING); +// Session::set('MemberLoginForm.force_message', 1); $loginResponse = $me->login(); if ($loginResponse instanceof HTTPResponse) { return $loginResponse; @@ -367,10 +422,14 @@ class Security extends Controller implements TemplateGlobalProvider protected function getAuthenticator() { $authenticator = $this->getRequest()->requestVar('AuthenticationMethod'); - if ($authenticator && Authenticator::is_registered($authenticator)) { - return $authenticator; - } elseif ($authenticator !== '' && Authenticator::is_registered(Authenticator::get_default_authenticator())) { - return Authenticator::get_default_authenticator(); + if ($authenticator && self::hasAuthenticator($authenticator)) { + return Injector::inst()->get($authenticator); + + } elseif ($authenticator !== '') { + $authenticators = self::getAuthenticators(); + if (count($authenticators) > 0) { + return $authenticators[0]; + } } throw new LogicException('No valid authenticator found'); @@ -386,7 +445,8 @@ class Security extends Controller implements TemplateGlobalProvider { $authenticator = $this->getAuthenticator(); if ($authenticator) { - return $authenticator::get_login_form($this); + $handler = $authenticator->getLoginHandler($this->Link()); + return $handler->handleRequest($this->request, DataModel::inst()); } throw new Exception('Passed invalid authentication method'); } @@ -399,16 +459,14 @@ class Security extends Controller implements TemplateGlobalProvider * * @todo Check how to activate/deactivate authentication methods */ - public function GetLoginForms() + public function getLoginForms() { - $forms = array(); - - $authenticators = Authenticator::get_authenticators(); - foreach ($authenticators as $authenticator) { - $forms[] = $authenticator::get_login_form($this); - } - - return $forms; + return array_map( + function ($authenticator) { + return $authenticator->getLoginHandler($this->Link())->handleRequest($this->getRequest(), DataModel::inst()); + }, + Security::getAuthenticators() + ); } @@ -529,7 +587,7 @@ class Security extends Controller implements TemplateGlobalProvider /** * Combine the given forms into a formset with a tabbed interface * - * @param array $forms List of LoginForm instances + * @param array $authenticators List of Authenticator instances * @return string */ protected function generateLoginFormSet($forms) @@ -598,29 +656,141 @@ class Security extends Controller implements TemplateGlobalProvider * * @return string|HTTPResponse Returns the "login" page as HTML code. */ - public function login() + public function login($request) { // Check pre-login process if ($response = $this->preLogin()) { return $response; } - // Get response handler - $controller = $this->getResponseController(_t('SilverStripe\\Security\\Security.LOGIN', 'Log in')); + $link = $this->link("login"); + + // Delegate to a single handler - Security/login//... + if ($authenticatorName = $request->param('ID')) { + $request->shift(); + + $authenticator = $this->getAuthenticator($authenticatorName); + if (!$authenticator) { + throw new HTTPResponse_Exception(404, 'No authenticator "' . $authenticatorName . '"'); + } + + $handler = $authenticator->getLoginHandler(Controller::join_links($link, $authenticatorName)); + + return $this->delegateToHandler( + $handler, + _t('Security.LOGIN', 'Log in'), + $this->getTemplatesFor('login') + ); + + // Delegate to all of them, building a tabbed view - Security/login + } else { + $handlers = $this->getAuthenticators(); + array_walk( + $handlers, + function (&$auth, $name) use ($link) { + $auth = $auth->getLoginHandler(Controller::join_links($link, $name)); + } + ); + + if (count($handlers) === 1) { + return $this->delegateToHandler( + array_values($handlers)[0], + _t('Security.LOGIN', 'Log in'), + $this->getTemplatesFor('login') + ); + + } else { + return $this->delegateToFormSet( + $handlers, + _t('Security.LOGIN', 'Log in'), + $this->getTemplatesFor('login') + ); + } + } + + } + + /** + * Delegate to an number of handlers, extracting their forms and rendering a tabbed form-set. + * This is used to built the log-in page where there are multiple authenticators active. + * + * @param string $title The title of the form + * @param array $templates + * @return array|HTTPResponse|RequestHandler|\SilverStripe\ORM\FieldType\DBHTMLText|string + */ + protected function delegateToFormSet(array $handlers, $title, array $templates) + { + + // Process each of the handlers + $results = array_map( + function ($handler) { + return $handler->handleRequest($this->getRequest(), \SilverStripe\ORM\DataModel::inst()); + }, + $handlers + ); + + // Aggregate all their forms, assuming they all return + $forms = []; + foreach ($results as $authName => $singleResult) { + // The result *must* be an array with a Form key + if (!is_array($singleResult) || !isset($singleResult['Form'])) { + user_error('Authenticator "' . $authName . '" doesn\'t support a tabbed login', E_USER_WARNING); + continue; + } + + $forms[] = $singleResult['Form']; + } + + if (!$forms) { + throw new \LogicException("No authenticators found compatible with a tabbed login"); + } + + return $this->renderWrappedController( + $title, + [ + 'Form' => $this->generateLoginFormSet($forms), + ], + $templates + ); + + } + + /** + * Delegate to another RequestHandler, rendering any fragment arrays into an appropriate. + * controller. + * + * @param string $title The title of the form + * @param array $templates + * @return array|HTTPResponse|RequestHandler|\SilverStripe\ORM\FieldType\DBHTMLText|string + */ + protected function delegateToHandler(RequestHandler $handler, $title, array $templates) + { + $result = $handler->handleRequest($this->getRequest(), \SilverStripe\ORM\DataModel::inst()); + + // Return the customised controller - used to render in a Form + // Post requests are expected to be login posts, so they'll be handled downstairs + if (is_array($result)) { + $result = $this->renderWrappedController($title, $result, $templates); + } + + return $result; + } + + /** + * Render the given fragments into a security page controller with the given title. + * @param $title string The title to give the security page + * @param $fragments A map of objects to render into the page, e.g. "Form" + * @param $templates An array of templates to use for the render + */ + protected function renderWrappedController($title, array $fragments, array $templates) + { + $controller = $this->getResponseController($title); // if the controller calls Director::redirect(), this will break early if (($response = $controller->getResponse()) && $response->isFinished()) { return $response; } - $forms = $this->GetLoginForms(); - if (!count($forms)) { - user_error( - 'No login-forms found, please use Authenticator::register_authenticator() to add one', - E_USER_ERROR - ); - } - // Handle any form messages from validation, etc. $messageType = ''; $message = $this->getLoginMessage($messageType); @@ -628,26 +798,16 @@ class Security extends Controller implements TemplateGlobalProvider // We've displayed the message in the form output, so reset it for the next run. static::clearLoginMessage(); - // only display tabs when more than one authenticator is provided - // to save bandwidth and reduce the amount of custom styling needed - if (count($forms) > 1) { - $content = $this->generateLoginFormSet($forms); - } else { - $content = $forms[0]->forTemplate(); + if ($message) { + $messageResult = [ + 'Content' => DBField::create_field('HTMLFragment', $message), + 'Message' => DBField::create_field('HTMLFragment', $message), + 'MessageType' => $messageType + ]; + $result = array_merge($fragments, $messageResult); } - // Finally, customise the controller to add any form messages and the form. - $customisedController = $controller->customise(array( - "Content" => DBField::create_field('HTMLFragment', $message), - "Message" => DBField::create_field('HTMLFragment', $message), - "MessageType" => $messageType, - "Form" => $content, - )); - - // Return the customised controller - return $customisedController->renderWith( - $this->getTemplatesFor('login') - ); + return $controller->customise($fragments)->renderWith($templates); } public function basicauthlogin() @@ -663,95 +823,17 @@ class Security extends Controller implements TemplateGlobalProvider */ public function lostpassword() { - $controller = $this->getResponseController(_t('SilverStripe\\Security\\Security.LOSTPASSWORDHEADER', 'Lost Password')); - - // if the controller calls Director::redirect(), this will break early - if (($response = $controller->getResponse()) && $response->isFinished()) { - return $response; - } - - $message = _t( - 'SilverStripe\\Security\\Security.NOTERESETPASSWORD', - 'Enter your e-mail address and we will send you a link with which you can reset your password' + $handler = $this->getAuthenticator()->getLostPasswordHandler( + Controller::join_links($this->link(), 'lostpassword') ); - /** @var ViewableData_Customised $customisedController */ - $customisedController = $controller->customise(array( - 'Content' => DBField::create_field('HTMLFragment', "

$message

"), - 'Form' => $this->LostPasswordForm(), - )); - //Controller::$currentController = $controller; - $result = $customisedController->renderWith($this->getTemplatesFor('lostpassword')); - - return $result; - } - - - /** - * Factory method for the lost password form - * - * @skipUpgrade - * @return Form Returns the lost password form - */ - public function LostPasswordForm() - { - return MemberLoginForm::create( - $this, - Config::inst()->get('Authenticator', 'default_authenticator'), - 'LostPasswordForm', - new FieldList( - new EmailField('Email', _t('SilverStripe\\Security\\Member.EMAIL', 'Email')) - ), - new FieldList( - new FormAction( - 'forgotPassword', - _t(__CLASS__.'.BUTTONSEND', 'Send me the password reset link') - ) - ), - false + return $this->delegateToHandler( + $handler, + _t('SilverStripe\\Security\\Security.LOSTPASSWORDHEADER', 'Lost Password'), + $this->getTemplatesFor('lostpassword') ); } - - /** - * Show the "password sent" page, after a user has requested - * to reset their password. - * - * @param HTTPRequest $request The HTTPRequest for this action. - * @return string Returns the "password sent" page as HTML code. - */ - public function passwordsent($request) - { - $controller = $this->getResponseController(_t('SilverStripe\\Security\\Security.LOSTPASSWORDHEADER', 'Lost Password')); - - // if the controller calls Director::redirect(), this will break early - if (($response = $controller->getResponse()) && $response->isFinished()) { - return $response; - } - - $email = Convert::raw2xml(rawurldecode($request->param('ID')) . '.' . $request->getExtension()); - - $message = _t( - 'SilverStripe\\Security\\Security.PASSWORDSENTTEXT', - "Thank you! A reset link has been sent to '{email}', provided an account exists for this email" - . " address.", - array('email' => Convert::raw2xml($email)) - ); - $customisedController = $controller->customise(array( - 'Title' => _t( - 'SilverStripe\\Security\\Security.PASSWORDSENTHEADER', - "Password reset link sent to '{email}'", - array('email' => $email) - ), - 'Content' => DBField::create_field('HTMLFragment', "

$message

"), - 'Email' => $email - )); - - //Controller::$currentController = $controller; - return $customisedController->renderWith($this->getTemplatesFor('passwordsent')); - } - - /** * Create a link to the password reset form. * @@ -867,7 +949,7 @@ class Security extends Controller implements TemplateGlobalProvider */ public function ChangePasswordForm() { - return ChangePasswordForm::create($this, 'ChangePasswordForm'); + return MemberAuthenticator\ChangePasswordForm::create($this, 'ChangePasswordForm'); } /** diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php index 9d3209d57..b681fd530 100644 --- a/src/View/ViewableData.php +++ b/src/View/ViewableData.php @@ -329,7 +329,7 @@ class ViewableData implements IteratorAggregate } // Fall back to default_cast - $default = $this->config()->get('default_cast'); + $default = self::config()->get('default_cast'); if (empty($default)) { throw new Exception("No default_cast"); } diff --git a/tests/php/Security/SecurityTest.php b/tests/php/Security/SecurityTest.php index a4d941f4c..c7ebb8abe 100644 --- a/tests/php/Security/SecurityTest.php +++ b/tests/php/Security/SecurityTest.php @@ -49,8 +49,8 @@ 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(); + // $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', []); @@ -74,8 +74,8 @@ class SecurityTest extends FunctionalTest // Restore selected authenticator // MemberAuthenticator might not actually be present - Config::modify()->set(Authenticator::class, 'authenticators', $this->priorAuthenticators); - Config::modify()->set(Authenticator::class, 'default_authenticator', $this->priorDefaultAuthenticator); + // Config::modify()->set(Authenticator::class, 'authenticators', $this->priorAuthenticators); + // Config::modify()->set(Authenticator::class, 'default_authenticator', $this->priorDefaultAuthenticator); // Restore unique identifier field Member::config()->unique_identifier_field = $this->priorUniqueIdentifierField;