Permalink
Browse files

BUGFIX Fixed Member->PasswordEncryption defaults when writing new Mem…

…ber without setting a password. Fixes critical issue with MemberTableField saving in admin/security, where new members are stored with a cleartext password by default instead of using the default SHA1 (see #5772) (from r107532)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@112602 467b73ca-7a2a-4603-9d3b-597d59a354a9
  • Loading branch information...
chillu committed Oct 15, 2010
1 parent bd927c9 commit 577e82a1237f1b2b0046d597a135d99ac04ed8ea
Showing with 19 additions and 8 deletions.
  1. +3 −2 security/Member.php
  2. +16 −6 tests/security/MemberTest.php
View
@@ -636,16 +636,17 @@ function onBeforeWrite() {
// The test on $this->ID is used for when records are initially created.
// Note that this only works with cleartext passwords, as we can't rehash
// existing passwords.
- if(!$this->ID || $this->isChanged('Password')) {
+ if((!$this->ID && $this->Password) || $this->isChanged('Password')) {
// Password was changed: encrypt the password according the settings
$encryption_details = Security::encrypt_password(
$this->Password, // this is assumed to be cleartext
$this->Salt,
$this->PasswordEncryption,
$this
);
+
// Overwrite the Password property with the hashed value
- $this->Password = $encryption_details['password'];
+; $this->Password = $encryption_details['password'];
$this->Salt = $encryption_details['salt'];
$this->PasswordEncryption = $encryption_details['algorithm'];
@@ -52,13 +52,20 @@ function testWriteDoesntMergeExistingMemberOnIdentifierChange() {
}
function testDefaultPasswordEncryptionOnMember() {
- $member = new Member();
- $member->Password = 'mypassword';
- $member->write();
+ $memberWithPassword = new Member();
+ $memberWithPassword->Password = 'mypassword';
+ $memberWithPassword->write();
$this->assertEquals(
- $member->PasswordEncryption,
+ $memberWithPassword->PasswordEncryption,
Security::get_password_encryption_algorithm(),
- 'Password encryption is set for new member records on first write'
+ 'Password encryption is set for new member records on first write (with setting "Password")'
+ );
+
+ $memberNoPassword = new Member();
+ $memberNoPassword->write();
+ $this->assertNull(
+ $memberNoPassword->PasswordEncryption,
+ 'Password encryption is not set for new member records on first write, when not setting a "Password")'
);
}
@@ -68,8 +75,9 @@ function testDefaultPasswordEncryptionDoesntChangeExistingMembers() {
$member->PasswordEncryption = 'sha1_v2.4';
$member->write();
+ $origAlgo = Security::get_password_encryption_algorithm();
Security::set_password_encryption_algorithm('none');
-
+
$member->Password = 'mynewpassword';
$member->write();
@@ -79,6 +87,8 @@ function testDefaultPasswordEncryptionDoesntChangeExistingMembers() {
);
$result = $member->checkPassword('mynewpassword');
$this->assertTrue($result->valid());
+
+ Security::set_password_encryption_algorithm($origAlgo);
}
function testSetPassword() {

0 comments on commit 577e82a

Please sign in to comment.