diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 66faf69f4..c0266346a 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -1015,6 +1015,23 @@ to update those with the appropriate function or config call. See [CMS architecture](/developer_guides/customising_the_admin_interface/cms-architecture#the-admin-url) for language specific functions. +#### Upgrading custom Authenticators + +The methods `register` and `unregister` on `Authenticator` are deprecated in favor of the `Config` system. This means that any custom Authenticator needs to be registered through the yml config: +```yaml +SilverStripe\Security\Authenticator; + authenticators: + - MyVendor\MyModule\MyAuthenticator +``` +If there is no authenticator registered, `Authenticator` will try to fall back on the `default_authenticator`, which can be changed using the following config, replacing the MemberAuthenticator with your authenticator: +```yaml +SilverStripe\Security\Authenticator: + default_authenticator: SilverStripe\Security\MemberAuthenticator +``` + +As soon as a custom authenticator is registered, the default authenticator will not be available anymore, unless enabled specifically in the config. + +By default, the `SilverStripe\Security\MemberAuthenticator` is seen as the default authenticator until it's explicitly set in the config. #### Upgrading Config API usages @@ -1784,3 +1801,4 @@ New `TimeField` methods replace `getConfig()` / `setConfig()` you will need to define this method and return a short name describing the login method. * `MemberLoginForm` has a new constructor argument for the authenticator class, athough tis is usually constructed by `MemberAuthenticator` and won't affect normal use. + * `Authenticator` methods `register` and `unregister` are deprecated in favor of using `Config` \ No newline at end of file diff --git a/src/Security/Authenticator.php b/src/Security/Authenticator.php index b19fbfdd7..1e0191742 100644 --- a/src/Security/Authenticator.php +++ b/src/Security/Authenticator.php @@ -4,6 +4,7 @@ namespace SilverStripe\Security; use SilverStripe\Core\Object; use SilverStripe\Control\Controller; +use SilverStripe\Dev\Deprecation; use SilverStripe\Forms\Form; /** @@ -22,7 +23,7 @@ abstract class Authenticator extends Object * * @var array */ - private static $authenticators = array(MemberAuthenticator::class); + private static $authenticators = []; /** * Used to influence the order of authenticators on the login-screen @@ -77,69 +78,7 @@ abstract class Authenticator extends Object { return false; } - - - public static function register($authenticator) - { - self::register_authenticator($authenticator); - } - - - /** - * Register a new authenticator - * - * The new authenticator has to exist and to be derived from the - * {@link Authenticator}. - * Every authenticator can be registered only once. - * - * @param string $authenticator Name of the authenticator class to - * register - * @return bool Returns TRUE on success, FALSE otherwise. - */ - public static function register_authenticator($authenticator) - { - $authenticator = trim($authenticator); - - if (class_exists($authenticator) == false) { - return false; - } - - if (is_subclass_of($authenticator, self::class) == false) { - return false; - } - - if (in_array($authenticator, self::$authenticators) == false) { - if (call_user_func(array($authenticator, 'on_register')) === true) { - array_push(self::$authenticators, $authenticator); - } else { - return false; - } - } - - return true; - } - - public static function unregister($authenticator) - { - self::unregister_authenticator($authenticator); - } - - /** - * Remove a previously registered authenticator - * - * @param string $authenticator Name of the authenticator class to register - * @return bool Returns TRUE on success, FALSE otherwise. - */ - public static function unregister_authenticator($authenticator) - { - if (call_user_func(array($authenticator, 'on_unregister')) === true) { - if (in_array($authenticator, self::$authenticators)) { - unset(self::$authenticators[array_search($authenticator, self::$authenticators)]); - } - } - } - - + /** * Check if a given authenticator is registered * @@ -149,7 +88,12 @@ abstract class Authenticator extends Object */ public static function is_registered($authenticator) { - return in_array($authenticator, self::$authenticators); + $authenticators = self::config()->get('authenticators'); + if (count($authenticators) === 0) { + $authenticators = [self::config()->get('default_authenticator')]; + } + + return in_array($authenticator, $authenticators, true); } @@ -161,23 +105,20 @@ abstract class Authenticator extends Object */ 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) - if ($key = array_search(self::$default_authenticator, self::$authenticators)) { - unset(self::$authenticators[$key]); - array_unshift(self::$authenticators, self::$default_authenticator); + // 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 self::$authenticators; - } - - /** - * Set a default authenticator (shows first in tabs) - * - * @param string - */ - public static function set_default_authenticator($authenticator) - { - self::$default_authenticator = $authenticator; + return $authenticators; } /** @@ -185,33 +126,6 @@ abstract class Authenticator extends Object */ public static function get_default_authenticator() { - return self::$default_authenticator; - } - - - /** - * Callback function that is called when the authenticator is registered - * - * Use this method for initialization of a newly registered authenticator. - * Just overload this method and it will be called when the authenticator - * is registered. - * If the method returns FALSE, the authenticator won't be - * registered! - * - * @return bool Returns TRUE on success, FALSE otherwise. - */ - protected static function on_register() - { - return true; - } - - /** - * Callback function that is called when an authenticator is removed. - * - * @return bool - */ - protected static function on_unregister() - { - return true; + return self::config()->get('default_authenticator'); } } diff --git a/src/Security/CMSMemberLoginForm.php b/src/Security/CMSMemberLoginForm.php index 03a5db36c..7ad3039be 100644 --- a/src/Security/CMSMemberLoginForm.php +++ b/src/Security/CMSMemberLoginForm.php @@ -2,16 +2,13 @@ namespace SilverStripe\Security; -use SilverStripe\Control\HTTPResponse; -use SilverStripe\Core\Convert; use SilverStripe\Control\Controller; -use SilverStripe\Control\Session; -use SilverStripe\Forms\HiddenField; -use SilverStripe\Forms\PasswordField; -use SilverStripe\Forms\LiteralField; -use SilverStripe\Forms\FieldList; use SilverStripe\Forms\CheckboxField; +use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormAction; +use SilverStripe\Forms\HiddenField; +use SilverStripe\Forms\LiteralField; +use SilverStripe\Forms\PasswordField; /** * Provides the in-cms session re-authentication form for the "member" authenticator @@ -29,12 +26,34 @@ class CMSMemberLoginForm extends LoginForm return Security::singleton()->Link($action); } - public function __construct(Controller $controller, $name) + /** + * CMSMemberLoginForm constructor. + * @param Controller $controller + * @param string $authenticatorClass + * @param FieldList $name + */ + public function __construct(Controller $controller, $authenticatorClass, $name) + { + $this->controller = $controller; + + $this->authenticator_class = $authenticatorClass; + + $fields = $this->getFormFields(); + + $actions = $this->getFormActions(); + + parent::__construct($controller, $name, $fields, $actions); + } + + /** + * @return FieldList + */ + public function getFormFields() { // Set default fields $fields = new FieldList( HiddenField::create("AuthenticationMethod", null, $this->authenticator_class, $this), - HiddenField::create('tempid', null, $controller->getRequest()->requestVar('tempid')), + HiddenField::create('tempid', null, $this->controller->getRequest()->requestVar('tempid')), PasswordField::create("Password", _t('Member.PASSWORD', 'Password')), LiteralField::create( 'forgotPassword', @@ -53,10 +72,19 @@ class CMSMemberLoginForm extends LoginForm )); } + return $fields; + } + + /** + * @return FieldList + */ + public function getFormActions() + { + // Determine returnurl to redirect to parent page $logoutLink = $this->getExternalLink('logout'); - if ($returnURL = $controller->getRequest()->requestVar('BackURL')) { - $logoutLink = Controller::join_links($logoutLink, '?BackURL='.urlencode($returnURL)); + if ($returnURL = $this->controller->getRequest()->requestVar('BackURL')) { + $logoutLink = Controller::join_links($logoutLink, '?BackURL=' . urlencode($returnURL)); } // Make actions @@ -72,7 +100,7 @@ class CMSMemberLoginForm extends LoginForm ) ); - parent::__construct($controller, $name, $fields, $actions); + return $actions; } protected function buildRequestHandler() diff --git a/src/Security/LoginForm.php b/src/Security/LoginForm.php index d2ef00000..2bb6d60ad 100644 --- a/src/Security/LoginForm.php +++ b/src/Security/LoginForm.php @@ -3,6 +3,7 @@ namespace SilverStripe\Security; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; /** @@ -32,4 +33,18 @@ abstract class LoginForm extends Form * @return string */ abstract public function getAuthenticatorName(); + + /** + * Required FieldList creation on a LoginForm + * + * @return FieldList + */ + abstract protected function getFormFields(); + + /** + * Required FieldList creation for the login actions on this LoginForm + * + * @return FieldList + */ + abstract protected function getFormActions(); } diff --git a/src/Security/MemberAuthenticator.php b/src/Security/MemberAuthenticator.php index 36717b3a1..e24bcdec2 100644 --- a/src/Security/MemberAuthenticator.php +++ b/src/Security/MemberAuthenticator.php @@ -208,7 +208,7 @@ class MemberAuthenticator extends Authenticator public static function get_cms_login_form(Controller $controller) { /** @skipUpgrade */ - return CMSMemberLoginForm::create($controller, "LoginForm"); + return CMSMemberLoginForm::create($controller, self::class, "LoginForm"); } public static function supports_cms() diff --git a/src/Security/Security.php b/src/Security/Security.php index 62a8e11d2..e2647601a 100644 --- a/src/Security/Security.php +++ b/src/Security/Security.php @@ -369,7 +369,7 @@ class Security extends Controller implements TemplateGlobalProvider $authenticator = $this->getRequest()->requestVar('AuthenticationMethod'); if ($authenticator && Authenticator::is_registered($authenticator)) { return $authenticator; - } elseif ($authenticator !== "" && Authenticator::is_registered(Authenticator::get_default_authenticator())) { + } elseif ($authenticator !== '' && Authenticator::is_registered(Authenticator::get_default_authenticator())) { return Authenticator::get_default_authenticator(); } diff --git a/tests/php/Security/SecurityTest.php b/tests/php/Security/SecurityTest.php index 6de6e42d1..ccaa890c4 100644 --- a/tests/php/Security/SecurityTest.php +++ b/tests/php/Security/SecurityTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\Security\Tests; +use PhpConsole\Auth; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\FieldType\DBClassName; @@ -50,24 +51,22 @@ class SecurityTest extends FunctionalTest // This test assumes that MemberAuthenticator is present and the default $this->priorAuthenticators = Authenticator::get_authenticators(); $this->priorDefaultAuthenticator = Authenticator::get_default_authenticator(); - foreach ($this->priorAuthenticators as $authenticator) { - Authenticator::unregister($authenticator); - } - Authenticator::register(MemberAuthenticator::class); - Authenticator::set_default_authenticator(MemberAuthenticator::class); + // 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); // And that the unique identified field is 'Email' $this->priorUniqueIdentifierField = Member::config()->unique_identifier_field; $this->priorRememberUsername = Security::config()->remember_username; /** - * @skipUpgrade -*/ + * @skipUpgrade + */ Member::config()->unique_identifier_field = 'Email'; parent::setUp(); - Config::inst()->update('SilverStripe\\Control\\Director', 'alternate_base_url', '/'); + Config::modify()->merge('SilverStripe\\Control\\Director', 'alternate_base_url', '/'); } protected function tearDown() @@ -75,13 +74,8 @@ class SecurityTest extends FunctionalTest // Restore selected authenticator // MemberAuthenticator might not actually be present - if (!in_array(MemberAuthenticator::class, $this->priorAuthenticators)) { - Authenticator::unregister(MemberAuthenticator::class); - } - foreach ($this->priorAuthenticators as $authenticator) { - Authenticator::register($authenticator); - } - Authenticator::set_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; @@ -129,8 +123,8 @@ class SecurityTest extends FunctionalTest 'Default permission failure message value was not present' ); - Config::inst()->remove(Security::class, 'default_message_set'); - Config::inst()->update(Security::class, 'default_message_set', array('default' => 'arrayvalue')); + Config::modify()->remove(Security::class, 'default_message_set'); + Config::modify()->merge(Security::class, 'default_message_set', array('default' => 'arrayvalue')); Security::permissionFailure($controller); $this->assertEquals( 'arrayvalue',