Merge pull request #44 from silverstripe-security/patch/3.5/authenticator-fix

FIX Authenticators are more resilient to incomplete configuration
This commit is contained in:
Daniel Hensby 2017-09-20 11:38:38 +01:00 committed by GitHub
commit 6b198336a8
4 changed files with 44 additions and 13 deletions

View File

@ -102,9 +102,12 @@ abstract class Authenticator extends Object {
if(is_subclass_of($authenticator, 'Authenticator') == false) if(is_subclass_of($authenticator, 'Authenticator') == false)
return false; return false;
if(in_array($authenticator, self::$authenticators) == false) { $authenticators = Config::inst()->get(__CLASS__, 'authenticators');
if(in_array($authenticator, $authenticators) == false) {
if(call_user_func(array($authenticator, 'on_register')) === true) { if(call_user_func(array($authenticator, 'on_register')) === true) {
array_push(self::$authenticators, $authenticator); Config::inst()->update(__CLASS__, 'authenticators', array(
$authenticator,
));
} else { } else {
return false; return false;
} }
@ -125,8 +128,11 @@ abstract class Authenticator extends Object {
*/ */
public static function unregister_authenticator($authenticator) { public static function unregister_authenticator($authenticator) {
if(call_user_func(array($authenticator, 'on_unregister')) === true) { if(call_user_func(array($authenticator, 'on_unregister')) === true) {
if(in_array($authenticator, self::$authenticators)) { $authenticators = Config::inst()->get(__CLASS__, 'authenticators');
unset(self::$authenticators[array_search($authenticator, self::$authenticators)]); if(($key = array_search($authenticator, $authenticators)) !== false) {
unset($authenticators[$key]);
Config::inst()->remove(__CLASS__, 'authenticators');
Config::inst()->update(__CLASS__, 'authenticators', $authenticators);
} }
} }
} }
@ -140,7 +146,7 @@ abstract class Authenticator extends Object {
* otherwise. * otherwise.
*/ */
public static function is_registered($authenticator) { public static function is_registered($authenticator) {
return in_array($authenticator, self::$authenticators); return in_array($authenticator, Config::inst()->get(__CLASS__, 'authenticators'));
} }
@ -151,13 +157,16 @@ abstract class Authenticator extends Object {
* authenticators. * authenticators.
*/ */
public static function get_authenticators() { public static function get_authenticators() {
// put default authenticator first (mainly for tab-order on loginform) $authenticators = Config::inst()->get(__CLASS__, 'authenticators');
if($key = array_search(self::$default_authenticator,self::$authenticators)) { $defaultAuthenticator = Config::inst()->get(__CLASS__, 'default_authenticator');
unset(self::$authenticators[$key]);
array_unshift(self::$authenticators, self::$default_authenticator); // put default authenticator first if it isn't already
if (reset($authenticators) !== $defaultAuthenticator && ($key = array_search($defaultAuthenticator, $authenticators)) !== false) {
unset($authenticators[$key]);
array_unshift($authenticators, $defaultAuthenticator);
} }
return self::$authenticators; return $authenticators;
} }
/** /**
@ -175,7 +184,9 @@ abstract class Authenticator extends Object {
* @return string * @return string
*/ */
public static function get_default_authenticator() { public static function get_default_authenticator() {
return self::$default_authenticator; $authenticators = static::get_authenticators();
// the first authenticator is the default one
return reset($authenticators);
} }

View File

@ -44,5 +44,15 @@ abstract class LoginForm extends Form {
return $authClass::get_name(); return $authClass::get_name();
} }
public function setAuthenticatorClass($class)
{
$this->authenticator_class = $class;
$authenticatorField = $this->Fields()->dataFieldByName('AuthenticationMethod');
if ($authenticatorField) {
$authenticatorField->setValue($class);
}
return $this;
}
} }

View File

@ -179,14 +179,18 @@ class MemberAuthenticator extends Authenticator {
* *
* @param Controller The parent controller, necessary to create the * @param Controller The parent controller, necessary to create the
* appropriate form action tag * appropriate form action tag
* @return Form Returns the login form to use with this authentication * @return MemberLoginForm Returns the login form to use with this authentication
* method * method
*/ */
public static function get_login_form(Controller $controller) { public static function get_login_form(Controller $controller) {
return MemberLoginForm::create($controller, "LoginForm"); return MemberLoginForm::create($controller, "LoginForm");
} }
public static function get_cms_login_form(\Controller $controller) { /**
* @param Controller $controller
* @return CMSMemberLoginForm
*/
public static function get_cms_login_form(Controller $controller) {
return CMSMemberLoginForm::create($controller, "LoginForm"); return CMSMemberLoginForm::create($controller, "LoginForm");
} }

View File

@ -221,4 +221,10 @@ class MemberAuthenticatorTest extends SapphireTest {
$this->assertTrue($member->isLockedOut()); $this->assertTrue($member->isLockedOut());
$this->assertFalse($member->canLogIn()->valid()); $this->assertFalse($member->canLogIn()->valid());
} }
public function testDefaultAuthenticatorWontReturnIfDisabled()
{
Authenticator::unregister('MemberAuthenticator');
$this->assertNotEquals('MemberAuthenticator', Authenticator::get_default_authenticator());
}
} }