tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-01 05:39:10 +02:00
Code Issues Projects Releases Wiki Activity

BUGFIX Keep Member.PasswordEncryption setting on empty passwords

Browse Source 
This will prevent empty passwords to set the encryption to 'none',
which in turn will store any subsequent password changes in cleartext.
Reproduceable e.g. with ConfirmedPasswordField and setCanBeEmpty(true).
This commit is contained in:
Ingo Schommer 2013-01-06 21:20:02 +01:00
parent 3e27d27f7a
commit eecd34868f
2 changed files with 72 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

119
security/Security.php
View File

@ -5,7 +5,7 @@
* @subpackage security * @subpackage security
*/ */
class Security extends Controller { class Security extends Controller {
/** /**
* Default user name. Only used in dev-mode by {@link setDefaultAdmin()} * Default user name. Only used in dev-mode by {@link setDefaultAdmin()}
* *
@ -82,7 +82,7 @@ class Security extends Controller {
* @var array|string * @var array|string
*/ */
protected static $default_message_set = ''; protected static $default_message_set = '';
/** /**
* Get location of word list file * Get location of word list file
*/ */
@ -130,22 +130,22 @@ class Security extends Controller {
* If you don't provide a messageSet, a default will be used. * If you don't provide a messageSet, a default will be used.
* *
* @param Controller $controller The controller that you were on to cause the permission * @param Controller $controller The controller that you were on to cause the permission
* failure. * failure.
* @param string|array $messageSet The message to show to the user. This * @param string|array $messageSet The message to show to the user. This
* can be a string, or a map of different * can be a string, or a map of different
* messages for different contexts. * messages for different contexts.
* If you pass an array, you can use the * If you pass an array, you can use the
* following keys: * following keys:
* - default: The default message * - default: The default message
* - logInAgain: The message to show * - logInAgain: The message to show
* if the user has just * if the user has just
* logged out and the * logged out and the
* - alreadyLoggedIn: The message to * - alreadyLoggedIn: The message to
* show if the user * show if the user
* is already logged * is already logged
* in and lacks the * in and lacks the
* permission to * permission to
* access the item. * access the item.
* *
* The alreadyLoggedIn value can contain a '%s' placeholder that will be replaced with a link * The alreadyLoggedIn value can contain a '%s' placeholder that will be replaced with a link
* to log in. * to log in.
@ -233,7 +233,7 @@ class Security extends Controller {
} }
/** /**
* Get the login form to process according to the submitted data * Get the login form to process according to the submitted data
*/ */
protected function LoginForm() { protected function LoginForm() {
@ -255,7 +255,7 @@ class Security extends Controller {
} }
/** /**
* Get the login forms for all available authentication methods * Get the login forms for all available authentication methods
* *
* @return array Returns an array of available login forms (array of Form * @return array Returns an array of available login forms (array of Form
@ -269,8 +269,8 @@ class Security extends Controller {
$authenticators = Authenticator::get_authenticators(); $authenticators = Authenticator::get_authenticators();
foreach($authenticators as $authenticator) { foreach($authenticators as $authenticator) {
array_push($forms, array_push($forms,
call_user_func(array($authenticator, 'get_login_form'), call_user_func(array($authenticator, 'get_login_form'),
$this)); $this));
} }
@ -293,9 +293,9 @@ class Security extends Controller {
* Log the currently logged in user out * Log the currently logged in user out
* *
* @param bool $redirect Redirect the user back to where they came. * @param bool $redirect Redirect the user back to where they came.
* - If it's false, the code calling logout() is * - If it's false, the code calling logout() is
* responsible for sending the user where-ever * responsible for sending the user where-ever
* they should go. * they should go.
*/ */
public function logout($redirect = true) { public function logout($redirect = true) {
$member = Member::currentUser(); $member = Member::currentUser();
@ -328,15 +328,15 @@ class Security extends Controller {
Requirements::css($customCSS); Requirements::css($customCSS);
} }
$tmpPage = new Page(); $tmpPage = new Page();
$tmpPage->Title = _t('Security.LOGIN', 'Log in'); $tmpPage->Title = _t('Security.LOGIN', 'Log in');
$tmpPage->URLSegment = "Security"; $tmpPage->URLSegment = "Security";
// Disable ID-based caching of the log-in page by making it a random number // Disable ID-based caching of the log-in page by making it a random number
$tmpPage->ID = -1 * rand(1,10000000); $tmpPage->ID = -1 * rand(1,10000000);
$controller = new Page_Controller($tmpPage); $controller = new Page_Controller($tmpPage);
$controller->init(); $controller->init();
//Controller::$currentController = $controller; //Controller::$currentController = $controller;
$content = ''; $content = '';
$forms = $this->GetLoginForms(); $forms = $this->GetLoginForms();
@ -418,12 +418,12 @@ class Security extends Controller {
Requirements::javascript(SAPPHIRE_DIR . '/javascript/prototype_improvements.js'); Requirements::javascript(SAPPHIRE_DIR . '/javascript/prototype_improvements.js');
Requirements::javascript(THIRDPARTY_DIR . '/scriptaculous/effects.js'); Requirements::javascript(THIRDPARTY_DIR . '/scriptaculous/effects.js');
$tmpPage = new Page(); $tmpPage = new Page();
$tmpPage->Title = _t('Security.LOSTPASSWORDHEADER', 'Lost Password'); $tmpPage->Title = _t('Security.LOSTPASSWORDHEADER', 'Lost Password');
$tmpPage->URLSegment = 'Security'; $tmpPage->URLSegment = 'Security';
$tmpPage->ID = -1; // Set the page ID to -1 so we dont get the top level pages as its children $tmpPage->ID = -1; // Set the page ID to -1 so we dont get the top level pages as its children
$controller = new Page_Controller($tmpPage); $controller = new Page_Controller($tmpPage);
$controller->init(); $controller->init();
$customisedController = $controller->customise(array( $customisedController = $controller->customise(array(
'Content' => 'Content' =>
@ -478,15 +478,15 @@ class Security extends Controller {
Requirements::javascript(SAPPHIRE_DIR . '/javascript/prototype_improvements.js'); Requirements::javascript(SAPPHIRE_DIR . '/javascript/prototype_improvements.js');
Requirements::javascript(THIRDPARTY_DIR . '/scriptaculous/effects.js'); Requirements::javascript(THIRDPARTY_DIR . '/scriptaculous/effects.js');
$tmpPage = new Page(); $tmpPage = new Page();
$tmpPage->Title = _t('Security.LOSTPASSWORDHEADER'); $tmpPage->Title = _t('Security.LOSTPASSWORDHEADER');
$tmpPage->URLSegment = 'Security'; $tmpPage->URLSegment = 'Security';
$tmpPage->ID = -1; // Set the page ID to -1 so we dont get the top level pages as its children $tmpPage->ID = -1; // Set the page ID to -1 so we dont get the top level pages as its children
$controller = new Page_Controller($tmpPage); $controller = new Page_Controller($tmpPage);
$controller->init(); $controller->init();
$email = Convert::raw2xml($request->param('ID') . '.' . $request->getExtension()); $email = Convert::raw2xml($request->param('ID') . '.' . $request->getExtension());
$customisedController = $controller->customise(array( $customisedController = $controller->customise(array(
'Title' => sprintf(_t('Security.PASSWORDSENTHEADER', "Password reset link sent to '%s'"), $email), 'Title' => sprintf(_t('Security.PASSWORDSENTHEADER', "Password reset link sent to '%s'"), $email),
'Content' => 'Content' =>
@ -529,12 +529,12 @@ class Security extends Controller {
* @return string Returns the "change password" page as HTML code. * @return string Returns the "change password" page as HTML code.
*/ */
public function changepassword() { public function changepassword() {
$tmpPage = new Page(); $tmpPage = new Page();
$tmpPage->Title = _t('Security.CHANGEPASSWORDHEADER', 'Change your password'); $tmpPage->Title = _t('Security.CHANGEPASSWORDHEADER', 'Change your password');
$tmpPage->URLSegment = 'Security'; $tmpPage->URLSegment = 'Security';
$tmpPage->ID = -1; // Set the page ID to -1 so we dont get the top level pages as its children $tmpPage->ID = -1; // Set the page ID to -1 so we dont get the top level pages as its children
$controller = new Page_Controller($tmpPage); $controller = new Page_Controller($tmpPage);
$controller->init(); $controller->init();
// Extract the member from the URL. // Extract the member from the URL.
$member = null; $member = null;
@ -673,8 +673,8 @@ class Security extends Controller {
$adminGroup = $adminGroup->First(); $adminGroup = $adminGroup->First();
if($adminGroup->Members()->First()) { if($adminGroup->Members()->First()) {
$member = $adminGroup->Members()->First(); $member = $adminGroup->Members()->First();
} }
} }
if(!$adminGroup) { if(!$adminGroup) {
@ -835,18 +835,9 @@ class Security extends Controller {
* @see encrypt_passwords() * @see encrypt_passwords()
* @see set_password_encryption_algorithm() * @see set_password_encryption_algorithm()
*/ */
static function encrypt_password($password, $salt = null, $algorithm = null, $member = null) { public static function encrypt_password($password, $salt = null, $algorithm = null, $member = null) {
if( // Fall back to the default encryption algorithm
// if the password is empty, don't encrypt if(!$algorithm) $algorithm = self::$encryptionAlgorithm;
strlen(trim($password)) == 0
// if no algorithm is provided and no default is set, don't encrypt
|| (!$algorithm && self::$encryptPasswords == false)
) {
$algorithm = 'none';
} else {
// Fall back to the default encryption algorithm
if(!$algorithm) $algorithm = self::$encryptionAlgorithm;
}
$e = PasswordEncryptor::create_for_algorithm($algorithm); $e = PasswordEncryptor::create_for_algorithm($algorithm);
@ -870,7 +861,7 @@ class Security extends Controller {
public static function database_is_ready() { public static function database_is_ready() {
// Used for unit tests // Used for unit tests
if(self::$force_database_is_ready !== NULL) return self::$force_database_is_ready; if(self::$force_database_is_ready !== NULL) return self::$force_database_is_ready;
$requiredTables = ClassInfo::dataClassesFor('Member'); $requiredTables = ClassInfo::dataClassesFor('Member');
$requiredTables[] = 'Group'; $requiredTables[] = 'Group';
$requiredTables[] = 'Permission'; $requiredTables[] = 'Permission';
@ -878,7 +869,7 @@ class Security extends Controller {
foreach($requiredTables as $table) { foreach($requiredTables as $table) {
// if any of the tables aren't created in the database // if any of the tables aren't created in the database
if(!ClassInfo::hasTable($table)) return false; if(!ClassInfo::hasTable($table)) return false;
// if any of the tables don't have all fields mapped as table columns // if any of the tables don't have all fields mapped as table columns
$dbFields = DB::fieldList($table); $dbFields = DB::fieldList($table);
if(!$dbFields) return false; if(!$dbFields) return false;
@ -929,5 +920,5 @@ class Security extends Controller {
return self::$default_login_dest; return self::$default_login_dest;
} }
} }
?> ?>

17
tests/security/MemberTest.php
View File

@ -114,6 +114,23 @@ class MemberTest extends FunctionalTest {
Security::set_password_encryption_algorithm($origAlgo); Security::set_password_encryption_algorithm($origAlgo);
} }
public function testKeepsEncryptionOnEmptyPasswords() {
$member = new Member();
$member->Password = 'mypassword';
$member->PasswordEncryption = 'sha1_v2.4';
$member->write();
$member->Password = '';
$member->write();
$this->assertEquals(
$member->PasswordEncryption,
'sha1_v2.4'
);
$result = $member->checkPassword('');
$this->assertTrue($result->valid());
}
function testSetPassword() { function testSetPassword() {
$member = $this->objFromFixture('Member', 'test'); $member = $this->objFromFixture('Member', 'test');