From a8b0e44d980eeea708dabcab66a8ae605baeb9de Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Thu, 8 Nov 2012 16:33:19 +1300 Subject: [PATCH 1/6] API Hash autologin tokens before storing in the database. Refactor the code to make it clear the distinction is made between a plaintext token and a hashed version. Rename fields so it is more obvious what is being written and what sent out to the user. This reuses the salt and algorithm from the Member, which are kept constant throughout the Member lifetime in a normal scenario. If they do change, users will need to re-request so the hashes can be regenerated. --- security/Member.php | 87 +++++++++++++++++++++----- security/MemberLoginForm.php | 4 +- security/PasswordEncryptor.php | 4 +- security/RandomGenerator.php | 28 +++++++-- security/Security.php | 40 +++++++----- security/SecurityToken.php | 4 +- tests/security/MemberTest.php | 29 +++++++++ tests/security/RandomGeneratorTest.php | 8 +-- tests/security/SecurityTest.php | 7 ++- 9 files changed, 166 insertions(+), 45 deletions(-) diff --git a/security/Member.php b/security/Member.php index 44ef14902..30794ebf5 100644 --- a/security/Member.php +++ b/security/Member.php @@ -12,11 +12,11 @@ class Member extends DataObject implements TemplateGlobalProvider { 'Surname' => 'Varchar', 'Email' => 'Varchar(256)', // See RFC 5321, Section 4.5.3.1.3. 'Password' => 'Varchar(160)', - 'RememberLoginToken' => 'Varchar(50)', + 'RememberLoginToken' => 'Varchar(160)', // Note: this currently holds a hash, not a token. 'NumVisit' => 'Int', 'LastVisited' => 'SS_Datetime', 'Bounced' => 'Boolean', // Note: This does not seem to be used anywhere. - 'AutoLoginHash' => 'Varchar(50)', + 'AutoLoginHash' => 'Varchar(160)', 'AutoLoginExpired' => 'SS_Datetime', // This is an arbitrary code pointing to a PasswordEncryptor instance, // not an actual encryption algorithm. @@ -322,9 +322,11 @@ class Member extends DataObject implements TemplateGlobalProvider { $this->NumVisit++; if($remember) { + // Store the hash and give the client the cookie with the token. $generator = new RandomGenerator(); - $token = $generator->generateHash('sha1'); - $this->RememberLoginToken = $token; + $token = $generator->randomToken('sha1'); + $hash = $this->encryptWithUserSettings($token); + $this->RememberLoginToken = $hash; Cookie::set('alc_enc', $this->ID . ':' . $token, 90, null, null, null, true); } else { $this->RememberLoginToken = null; @@ -382,7 +384,8 @@ class Member extends DataObject implements TemplateGlobalProvider { $member = DataObject::get_one("Member", "\"Member\".\"ID\" = '$SQL_uid'"); // check if autologin token matches - if($member && (!$member->RememberLoginToken || $member->RememberLoginToken != $token)) { + $hash = $member->encryptWithUserSettings($token); + if($member && (!$member->RememberLoginToken || $member->RememberLoginToken != $hash)) { $member = null; } @@ -393,8 +396,10 @@ class Member extends DataObject implements TemplateGlobalProvider { if(self::$login_marker_cookie) Cookie::set(self::$login_marker_cookie, 1, 0, null, null, false, true); $generator = new RandomGenerator(); - $member->RememberLoginToken = $generator->generateHash('sha1'); - Cookie::set('alc_enc', $member->ID . ':' . $member->RememberLoginToken, 90, null, null, false, true); + $token = $generator->randomToken('sha1'); + $hash = $member->encryptWithUserSettings($token); + $member->RememberLoginToken = $hash; + Cookie::set('alc_enc', $member->ID . ':' . $token, 90, null, null, false, true); $member->NumVisit++; $member->write(); @@ -425,27 +430,82 @@ class Member extends DataObject implements TemplateGlobalProvider { $this->extend('memberLoggedOut'); } + /** + * Utility for generating secure password hashes for this member. + */ + public function encryptWithUserSettings($string) { + if (!$string) return null; + + // If the algorithm or salt is not available, it means we are operating + // on legacy account with unhashed password. Do not hash the string. + if (!$this->PasswordEncryption) { + return $string; + } + + // We assume we have PasswordEncryption and Salt available here. + $e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption); + return $e->encrypt($string, $this->Salt); + + } /** - * Generate an auto login hash - * - * This creates an auto login hash that can be used to reset the password. + * Generate an auto login token which can be used to reset the password, + * at the same time hashing it and storing in the database. * * @param int $lifetime The lifetime of the auto login hash in days (by default 2 days) * + * @returns string Token that should be passed to the client (but NOT persisted). + * * @todo Make it possible to handle database errors such as a "duplicate key" error */ - public function generateAutologinHash($lifetime = 2) { - + public function generateAutologinTokenAndStoreHash($lifetime = 2) { do { $generator = new RandomGenerator(); - $hash = $generator->generateHash('sha1'); + $token = $generator->randomToken(); + $hash = $this->encryptWithUserSettings($token); } while(DataObject::get_one('Member', "\"AutoLoginHash\" = '$hash'")); $this->AutoLoginHash = $hash; $this->AutoLoginExpired = date('Y-m-d', time() + (86400 * $lifetime)); $this->write(); + + return $token; + } + + /** + * @deprecated 3.0 + */ + public function generateAutologinHash($lifetime = 2) { + Deprecation::notice('3.0', + 'Member::generateAutologinHash is deprecated - tokens are no longer saved directly into the database '. + 'in plaintext. Use the return value of the Member::generateAutologinTokenAndHash to get the token '. + 'instead.', + Deprecation::SCOPE_METHOD); + + user_error( + 'Member::generateAutologinHash is deprecated - tokens are no longer saved directly into the database '. + 'in plaintext. Use the return value of the Member::generateAutologinTokenAndHash to get the token '. + 'instead.', + E_USER_ERROR); + } + + /** + * Check the token against the member. + * + * @param string $autologinToken + * + * @returns bool Is token valid? + */ + public function validateAutoLoginToken($autologinToken) { + $hash = $this->encryptWithUserSettings($autologinToken); + + $member = DataObject::get_one( + 'Member', + "\"AutoLoginHash\"='" . $hash . "' AND \"AutoLoginExpired\" > " . DB::getConn()->now() + ); + + return (bool)$member; } /** @@ -467,7 +527,6 @@ class Member extends DataObject implements TemplateGlobalProvider { return $member; } - /** * Send signup, change password or forgot password informations to an user * diff --git a/security/MemberLoginForm.php b/security/MemberLoginForm.php index a250c28bf..2e04763f4 100644 --- a/security/MemberLoginForm.php +++ b/security/MemberLoginForm.php @@ -258,12 +258,12 @@ JS $member = DataObject::get_one('Member', "\"Email\" = '{$SQL_email}'"); if($member) { - $member->generateAutologinHash(); + $token = $member->generateAutologinTokenAndStoreHash(); $e = Member_ForgotPasswordEmail::create(); $e->populateTemplate($member); $e->populateTemplate(array( - 'PasswordResetLink' => Security::getPasswordResetLink($member->AutoLoginHash) + 'PasswordResetLink' => Security::getPasswordResetLink($member, $token) )); $e->setTo($member->Email); $e->send(); diff --git a/security/PasswordEncryptor.php b/security/PasswordEncryptor.php index d2258d307..6c84f847d 100644 --- a/security/PasswordEncryptor.php +++ b/security/PasswordEncryptor.php @@ -99,7 +99,7 @@ abstract class PasswordEncryptor { */ public function salt($password, $member = null) { $generator = new RandomGenerator(); - return substr($generator->generateHash('sha1'), 0, 50); + return substr($generator->randomToken('sha1'), 0, 50); } /** @@ -281,7 +281,7 @@ class PasswordEncryptor_Blowfish extends PasswordEncryptor { */ public function salt($password, $member = null) { $generator = new RandomGenerator(); - return sprintf('%02d', self::$cost) . '$' . substr($generator->generateHash('sha1'), 0, 22); + return sprintf('%02d', self::$cost) . '$' . substr($generator->randomToken('sha1'), 0, 22); } public function check($hash, $password, $salt = null, $member = null) { diff --git a/security/RandomGenerator.php b/security/RandomGenerator.php index 3751e38c2..3f33288be 100644 --- a/security/RandomGenerator.php +++ b/security/RandomGenerator.php @@ -58,15 +58,31 @@ class RandomGenerator { // Fallback to good old mt_rand() return uniqid(mt_rand(), true); } - + /** - * Generates a hash suitable for manual session identifiers, CSRF tokens, etc. + * Generates a random token that can be used for session IDs, CSRF tokens etc., based on + * hash algorithms. + * + * If you are using it as a password equivalent (e.g. autologin token) do NOT store it + * in the database as a plain text but encrypt it with Member::encryptWithUserSettings. * * @param String $algorithm Any identifier listed in hash_algos() (Default: whirlpool) - * If possible, choose a slow algorithm which complicates brute force attacks. + * * @return String Returned length will depend on the used $algorithm */ - public function generateHash($algorithm = 'whirlpool') { + public function randomToken($algorithm = 'whirlpool') { return hash($algorithm, $this->generateEntropy()); - } -} \ No newline at end of file + } + + /** + * @deprecated 3.1 + */ + public function generateHash($algorithm = 'whirlpool') { + Deprecation::notice('3.1', + 'RandomGenerator::generateHash is deprecated because of a confusing name that hints the output is secure, '. + 'while in fact it is just a random string. Use RandomGenerator::randomToken instead.', + Deprecation::SCOPE_METHOD); + + return $this->randomToken($algorithm); + } +} diff --git a/security/Security.php b/security/Security.php index 1bc6ac837..908cb13a0 100644 --- a/security/Security.php +++ b/security/Security.php @@ -532,15 +532,20 @@ class Security extends Controller { /** - * Create a link to the password reset form + * Create a link to the password reset form. * - * @param string $autoLoginHash The auto login hash + * GET parameters used: + * - m: member ID + * - t: plaintext token + * + * @param Member $member Member object associated with this link. + * @param string $autoLoginHash The auto login token. */ - public static function getPasswordResetLink($autoLoginHash) { - $autoLoginHash = urldecode($autoLoginHash); + public static function getPasswordResetLink($member, $autologinToken) { + $autologinToken = urldecode($autologinToken); $selfControllerClass = __CLASS__; $selfController = new $selfControllerClass(); - return $selfController->Link('changepassword') . "?h=$autoLoginHash"; + return $selfController->Link('changepassword') . "?m={$member->ID}&t=$autologinToken"; } /** @@ -567,15 +572,22 @@ class Security extends Controller { $controller = $this; } - // First load with hash: Redirect to same URL without hash to avoid referer leakage - if(isset($_REQUEST['h']) && Member::member_from_autologinhash($_REQUEST['h'])) { - // The auto login hash is valid, store it for the change password form. - // Temporary value, unset in ChangePasswordForm - Session::set('AutoLoginHash', $_REQUEST['h']); + // Extract the member from the URL. + $member = null; + if (isset($_REQUEST['m'])) { + $member = Member::get()->filter('ID', (int)$_REQUEST['m'])->First(); + } + + // Check whether we are merely changin password, or resetting. + if(isset($_REQUEST['t']) && $member && $member->validateAutoLoginToken($_REQUEST['t'])) { + // On first valid password reset request redirect to the same URL without hash to avoid referrer leakage. + + // Store the hash for the change password form. Will be unset after reload within the ChangePasswordForm. + Session::set('AutoLoginHash', $member->encryptWithUserSettings($_REQUEST['t'])); return $this->redirect($this->Link('changepassword')); - // Redirection target after "First load with hash" } elseif(Session::get('AutoLoginHash')) { + // Subsequent request after the "first load with hash" (see previous if clause). $customisedController = $controller->customise(array( 'Content' => '

' . @@ -584,16 +596,16 @@ class Security extends Controller { 'Form' => $this->ChangePasswordForm(), )); } elseif(Member::currentUser()) { - // let a logged in user change his password + // Logged in user requested a password change form. $customisedController = $controller->customise(array( 'Content' => '

' . _t('Security.CHANGEPASSWORDBELOW', 'You can change your password below.') . '

', 'Form' => $this->ChangePasswordForm())); } else { - // show an error message if the auto login hash is invalid and the + // show an error message if the auto login token is invalid and the // user is not logged in - if(isset($_REQUEST['h'])) { + if(!isset($_REQUEST['t']) || !$member) { $customisedController = $controller->customise( array('Content' => _t( diff --git a/security/SecurityToken.php b/security/SecurityToken.php index d4a7bb8aa..444f87718 100644 --- a/security/SecurityToken.php +++ b/security/SecurityToken.php @@ -227,7 +227,7 @@ class SecurityToken extends Object implements TemplateGlobalProvider { */ protected function generate() { $generator = new RandomGenerator(); - return $generator->generateHash('sha1'); + return $generator->randomToken('sha1'); } public static function get_template_global_variables() { @@ -299,4 +299,4 @@ class NullSecurityToken extends SecurityToken { public function generate() { return null; } -} \ No newline at end of file +} diff --git a/tests/security/MemberTest.php b/tests/security/MemberTest.php index a08f4f552..94e94d3bf 100644 --- a/tests/security/MemberTest.php +++ b/tests/security/MemberTest.php @@ -624,6 +624,35 @@ class MemberTest extends FunctionalTest { return $extensions; } + public function testGenerateAutologinTokenAndStoreHash() { + $enc = new PasswordEncryptor_Blowfish(); + + $m = new Member(); + $m->PasswordEncryption = 'blowfish'; + $m->Salt = $enc->salt('123'); + + $token = $m->generateAutologinTokenAndStoreHash(); + + $this->assertEquals($m->encryptWithUserSettings($token), $m->AutoLoginHash, 'Stores the token as ahash.'); + } + + public function testValidateAutoLoginToken() { + $enc = new PasswordEncryptor_Blowfish(); + + $m1 = new Member(); + $m1->PasswordEncryption = 'blowfish'; + $m1->Salt = $enc->salt('123'); + $m1Token = $m1->generateAutologinTokenAndStoreHash(); + + $m2 = new Member(); + $m2->PasswordEncryption = 'blowfish'; + $m2->Salt = $enc->salt('456'); + $m2Token = $m2->generateAutologinTokenAndStoreHash(); + + $this->assertTrue($m1->validateAutoLoginToken($m1Token), 'Passes token validity test against matching member.'); + $this->assertFalse($m2->validateAutoLoginToken($m1Token), 'Fails token validity test against other member.'); + } + } class MemberTest_ViewingAllowedExtension extends DataExtension implements TestOnly { diff --git a/tests/security/RandomGeneratorTest.php b/tests/security/RandomGeneratorTest.php index 2baf6cef0..f85f3ae0d 100644 --- a/tests/security/RandomGeneratorTest.php +++ b/tests/security/RandomGeneratorTest.php @@ -14,14 +14,14 @@ class RandomGeneratorTest extends SapphireTest { public function testGenerateHash() { $r = new RandomGenerator(); - $this->assertNotNull($r->generateHash()); - $this->assertNotEquals($r->generateHash(), $r->generateHash()); + $this->assertNotNull($r->randomToken()); + $this->assertNotEquals($r->randomToken(), $r->randomToken()); } public function testGenerateHashWithAlgorithm() { $r = new RandomGenerator(); - $this->assertNotNull($r->generateHash('md5')); - $this->assertNotEquals($r->generateHash(), $r->generateHash('md5')); + $this->assertNotNull($r->randomToken('md5')); + $this->assertNotEquals($r->randomToken(), $r->randomToken('md5')); } } diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index f8a566ee4..6337e1b71 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -222,7 +222,12 @@ class SecurityTest extends FunctionalTest { // Load password link from email $admin = DataObject::get_by_id('Member', $admin->ID); $this->assertNotNull($admin->AutoLoginHash, 'Hash has been written after lost password'); - $response = $this->get('Security/changepassword/?h=' . $admin->AutoLoginHash); + + // We don't have access to the token - generate a new token and hash pair. + $token = $admin->generateAutologinTokenAndStoreHash(); + + // Check. + $response = $this->get('Security/changepassword/?m='.$admin->ID.'&t=' . $token); $this->assertEquals(302, $response->getStatusCode()); $this->assertEquals(Director::baseUrl() . 'Security/changepassword', $response->getHeader('Location')); From 05a44e8bfc683828af89a3ae656486fb234b80b4 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Fri, 9 Nov 2012 13:21:28 +0100 Subject: [PATCH 2/6] Correct branch for Travis build status image --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index a1f9833df..924e3b6a5 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ ## SilverStripe Framework -[![Build Status](https://secure.travis-ci.org/silverstripe/sapphire.png)](http://travis-ci.org/silverstripe/sapphire) +[![Build Status](https://secure.travis-ci.org/silverstripe/sapphire.png?branch=3.0)](https://travis-ci.org/silverstripe/sapphire) PHP5 framework forming the base for the SilverStripe CMS ([http://silverstripe.org](http://silverstripe.org)). Requires a [`silverstripe-installer`](http://github.com/silverstripe/silverstripe-installer) base project. Typically used alongside the [`cms`](http://github.com/silverstripe/silverstripe-cms) module. From 0dd97a38f6ef9c46232f1f2d2a4857e7e94d4e27 Mon Sep 17 00:00:00 2001 From: Hamish Friedlander Date: Fri, 16 Nov 2012 11:47:32 +1300 Subject: [PATCH 3/6] API: Form#loadDataFrom 2nd arg now sets how existing field data is merged with new data --- forms/Form.php | 108 +++++++++++++++++++++++++++------------ tests/forms/FormTest.php | 32 +++++++++++- 2 files changed, 104 insertions(+), 36 deletions(-) diff --git a/forms/Form.php b/forms/Form.php index c93d1e453..d1cf61b03 100644 --- a/forms/Form.php +++ b/forms/Form.php @@ -1080,6 +1080,10 @@ class Form extends RequestHandler { return true; } + const MERGE_DEFAULT = 0; + const MERGE_CLEAR_MISSING = 1; + const MERGE_IGNORE_FALSEISH = 2; + /** * Load data from the given DataObject or array. * It will call $object->MyField to get the value of MyField. @@ -1098,20 +1102,43 @@ class Form extends RequestHandler { * @uses FormField->setValue() * * @param array|DataObject $data - * @param boolean $clearMissingFields By default, fields which don't match - * a property or array-key of the passed {@link $data} argument are "left alone", - * meaning they retain any previous values (if present). If this flag is set to true, - * those fields are overwritten with null regardless if they have a match in {@link $data}. - * @param $fieldList An optional list of fields to process. This can be useful when you have a + * @param int $mergeStrategy + * For every field, {@link $data} is interogated whether it contains a relevant property/key, and + * what that property/key's value is. + * + * By default, if {@link $data} does contain a property/key, the fields value is always replaced by {@link $data}'s + * value, even if that value is null/false/etc. Fields which don't match any property/key in {@link $data} are + * "left alone", meaning they retain any previous value. + * + * You can pass a bitmask here to change this behaviour. + * + * Passing CLEAR_MISSING means that any fields that don't match any property/key in + * {@link $data} are cleared. + * + * Passing IGNORE_FALSEISH means that any false-ish value in {@link $data} won't replace + * a field's value. + * + * For backwards compatibility reasons, this parameter can also be set to === true, which is the same as passing + * CLEAR_MISSING + * + * @param $fieldList An optional list of fields to process. This can be useful when you have a * form that has some fields that save to one object, and some that save to another. * @return Form */ - public function loadDataFrom($data, $clearMissingFields = false, $fieldList = null) { + public function loadDataFrom($data, $mergeStrategy = 0, $fieldList = null) { if(!is_object($data) && !is_array($data)) { user_error("Form::loadDataFrom() not passed an array or an object", E_USER_WARNING); return $this; } + // Handle the backwards compatible case of passing "true" as the second argument + if ($mergeStrategy === true) { + $mergeStrategy = self::MERGE_CLEAR_MISSING; + } + else if ($mergeStrategy === false) { + $mergeStrategy = 0; + } + // if an object is passed, save it for historical reference through {@link getRecord()} if(is_object($data)) $this->record = $data; @@ -1125,37 +1152,50 @@ class Form extends RequestHandler { // First check looks for (fieldname)_unchanged, an indicator that we shouldn't overwrite the field value if(is_array($data) && isset($data[$name . '_unchanged'])) continue; - - // get value in different formats - $hasObjectValue = false; - if( - is_object($data) - && ( - isset($data->$name) - || $data->hasMethod($name) - || ($data->hasMethod('hasField') && $data->hasField($name)) - ) - ) { - // We don't actually call the method because it might be slow. - // In a later release, relation methods will just return references to the query that should be - // executed, and so we will be able to safely pass the return value of the relation method to the - // first argument of setValue - $val = $data->__get($name); - $hasObjectValue = true; - } else if(strpos($name,'[') && is_array($data) && !isset($data[$name])) { - // if field is in array-notation, we need to resolve the array-structure PHP creates from query-strings - preg_match('/' . addcslashes($name,'[]') . '=([^&]*)/', urldecode(http_build_query($data)), $matches); - $val = isset($matches[1]) ? $matches[1] : null; - } elseif(is_array($data) && array_key_exists($name, $data)) { - // else we assume its a simple keyed array - $val = $data[$name]; - } else { - $val = null; + + // Does this property exist on $data? + $exists = false; + // The value from $data for this field + $val = null; + + if(is_object($data)) { + $exists = ( + isset($data->$name) || + $data->hasMethod($name) || + ($data->hasMethod('hasField') && $data->hasField($name)) + ); + + if ($exists) { + $val = $data->__get($name); + } + } + else if(is_array($data)){ + if(array_key_exists($name, $data)) { + $exists = true; + $val = $data[$name]; + } + // If field is in array-notation we need to access nested data + else if(strpos($name,'[')) { + // First encode data using PHP's method of converting nested arrays to form data + $flatData = urldecode(http_build_query($data)); + // Then pull the value out from that flattened string + preg_match('/' . addcslashes($name,'[]') . '=([^&]*)/', $flatData, $matches); + + if (isset($matches[1])) { + $exists = true; + $val = $matches[1]; + } + } } // save to the field if either a value is given, or loading of blank/undefined values is forced - if(isset($val) || $hasObjectValue || $clearMissingFields) { - // pass original data as well so composite fields can act on the additional information + if($exists){ + if ($val != false || ($mergeStrategy & self::MERGE_IGNORE_FALSEISH) != self::MERGE_IGNORE_FALSEISH){ + // pass original data as well so composite fields can act on the additional information + $field->setValue($val, $data); + } + } + else if(($mergeStrategy & self::MERGE_CLEAR_MISSING) == self::MERGE_CLEAR_MISSING){ $field->setValue($val, $data); } } diff --git a/tests/forms/FormTest.php b/tests/forms/FormTest.php index fc356d40b..dbba1aa03 100644 --- a/tests/forms/FormTest.php +++ b/tests/forms/FormTest.php @@ -153,7 +153,7 @@ class FormTest extends FunctionalTest { $captainWithDetails = $this->objFromFixture('FormTest_Player', 'captainNoDetails'); $team2 = $this->objFromFixture('FormTest_Team', 'team2'); $form->loadDataFrom($captainWithDetails); - $form->loadDataFrom($team2, true); + $form->loadDataFrom($team2, Form::MERGE_CLEAR_MISSING); $this->assertEquals( $form->getData(), array( @@ -166,7 +166,35 @@ class FormTest extends FunctionalTest { 'LoadDataFrom() overwrites fields not found in the object with $clearMissingFields=true' ); } - + + public function testLoadDataFromIgnoreFalseish() { + $form = new Form( + new Controller(), + 'Form', + new FieldList( + new TextField('Biography', 'Biography', 'Custom Default') + ), + new FieldList() + ); + + $captainNoDetails = $this->objFromFixture('FormTest_Player', 'captainNoDetails'); + $captainWithDetails = $this->objFromFixture('FormTest_Player', 'captainWithDetails'); + + $form->loadDataFrom($captainNoDetails, Form::MERGE_IGNORE_FALSEISH); + $this->assertEquals( + $form->getData(), + array('Biography' => 'Custom Default'), + 'LoadDataFrom() doesn\'t overwrite fields when MERGE_IGNORE_FALSEISH set and values are false-ish' + ); + + $form->loadDataFrom($captainWithDetails, Form::MERGE_IGNORE_FALSEISH); + $this->assertEquals( + $form->getData(), + array('Biography' => 'Bio 1'), + 'LoadDataFrom() does overwrite fields when MERGE_IGNORE_FALSEISH set and values arent false-ish' + ); + } + public function testFormMethodOverride() { $form = $this->getStubForm(); $form->setFormMethod('GET'); From 7315be4531975998f683e41b48781d3ed7d9c74f Mon Sep 17 00:00:00 2001 From: Hamish Friedlander Date: Fri, 16 Nov 2012 11:48:31 +1300 Subject: [PATCH 4/6] FIX default values from DataObject not showing in GridField details form --- forms/gridfield/GridFieldDetailForm.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/forms/gridfield/GridFieldDetailForm.php b/forms/gridfield/GridFieldDetailForm.php index 4aeef7931..d6000e10d 100644 --- a/forms/gridfield/GridFieldDetailForm.php +++ b/forms/gridfield/GridFieldDetailForm.php @@ -325,9 +325,8 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler { $actions, $this->component->getValidator() ); - if($this->record->ID !== 0) { - $form->loadDataFrom($this->record); - } + + $form->loadDataFrom($this->record, $this->record->ID == 0 ? Form::MERGE_IGNORE_FALSEISH : Form::MERGE_DEFAULT); // TODO Coupling with CMS $toplevelController = $this->getToplevelController(); From 78ab9d3bf6855a29f0abf3b577e20d2e026e523e Mon Sep 17 00:00:00 2001 From: stojg Date: Thu, 15 Nov 2012 15:49:03 +1300 Subject: [PATCH 5/6] BUG Video embed from Add Media Feature no longer works (open #8033) Since the tinymce upgrade from 3.5.6 to 3.5.7 it seems like data attributes are forbidden on tags. This fix tells tinymce to allow data* properties on img tags --- admin/_config.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/admin/_config.php b/admin/_config.php index 1297c918e..f446d8157 100644 --- a/admin/_config.php +++ b/admin/_config.php @@ -13,7 +13,7 @@ HtmlEditorConfig::get('cms')->setOptions(array( 'use_native_selects' => false, 'valid_elements' => "@[id|class|style|title],a[id|rel|rev|dir|tabindex|accesskey|type|name|href|target|title" . "|class],-strong/-b[class],-em/-i[class],-strike[class],-u[class],#p[id|dir|class|align|style],-ol[class]," - . "-ul[class],-li[class],br,img[id|dir|longdesc|usemap|class|src|border|alt=|title|width|height|align]," + . "-ul[class],-li[class],br,img[id|dir|longdesc|usemap|class|src|border|alt=|title|width|height|align|data*]," . "-sub[class],-sup[class],-blockquote[dir|class]," . "-table[border=0|cellspacing|cellpadding|width|height|class|align|summary|dir|id|style]," . "-tr[id|dir|class|rowspan|width|height|align|valign|bgcolor|background|bordercolor|style]," @@ -25,7 +25,7 @@ HtmlEditorConfig::get('cms')->setOptions(array( . "-h4[id|dir|class|align|style],-h5[id|dir|class|align|style],-h6[id|dir|class|align|style],hr[class]," . "dd[id|class|title|dir],dl[id|class|title|dir],dt[id|class|title|dir],@[id,style,class]", 'extended_valid_elements' => "img[class|src|alt|title|hspace|vspace|width|height|align|onmouseover|onmouseout|name" - . "|usemap],iframe[src|name|width|height|align|frameborder|marginwidth|marginheight|scrolling]," + . "|usemap|data*],iframe[src|name|width|height|align|frameborder|marginwidth|marginheight|scrolling]," . "object[width|height|data|type],param[name|value],map[class|name|id],area[shape|coords|href|target|alt]", 'spellchecker_rpc_url' => THIRDPARTY_DIR . '/tinymce-spellchecker/rpc.php' )); From fb7db6de6d89849af62de81ac76ce57159cb2452 Mon Sep 17 00:00:00 2001 From: Hamish Friedlander Date: Fri, 16 Nov 2012 14:45:20 +1300 Subject: [PATCH 6/6] Add 3.0.3-rc2 changelog --- docs/en/changelogs/rc/3.0.3-rc2.md | 44 ++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 docs/en/changelogs/rc/3.0.3-rc2.md diff --git a/docs/en/changelogs/rc/3.0.3-rc2.md b/docs/en/changelogs/rc/3.0.3-rc2.md new file mode 100644 index 000000000..c442faec8 --- /dev/null +++ b/docs/en/changelogs/rc/3.0.3-rc2.md @@ -0,0 +1,44 @@ +# 3.0.3-rc2 (2012-11-16) + +## Overview + +3.0.3 provides security fixes, bugfixes and a number of minor enhancements since 3.0.2. + +Upgrading from 3.0.x should be a straightforward matter of dropping in the new release, +with the exception noted below. + +## Upgrading + +Impact of the upgrade: + +* Reset password email links generated prior to 3.0.3 will cease to work. +* Users who use the "remember me" login feature will have to log in again. + +API changes related to the below security patch: + +* `Member::generateAutologinHash` is deprecated. You can no longer get the autologin token from `AutoLoginHash` field in `Member`. Instead use the return value of the `Member::generateAutologinTokenAndStoreHash` and do not persist it. +* `Security::getPasswordResetLink` now requires `Member` object as the first parameter. The password reset URL GET parameters have changed from only `h` (for hash) to `m` (for member ID) and `t` (for plaintext token). +* `RandomGenerator::generateHash` will be deprecated with 3.1. Rename the function call to `RandomGenerator::randomToken`. + +### Security: Hash autologin tokens before storing in the database. + +Severity: Moderate + +Autologin tokens (remember me and reset password) are stored in the database as a plain text. +If attacker obtained the database he would be able to gain access to accounts that have requested a password change, or have "remember me" enabled. + +## Changelog + +### API Changes + + * 2012-11-16 [0dd97a3](https://github.com/silverstripe/sapphire/commit/0dd97a3) Form#loadDataFrom 2nd arg now sets how existing field data is merged with new data (Hamish Friedlander) + * 2012-11-08 [a8b0e44](https://github.com/silverstripe/sapphire/commit/a8b0e44) Hash autologin tokens before storing in the database. (Mateusz Uzdowski) + +### Bugfixes + + * 2012-11-16 [7315be4](https://github.com/silverstripe/sapphire/commit/7315be4) default values from DataObject not showing in GridField details form (Hamish Friedlander) + * 2012-11-15 [78ab9d3](https://github.com/silverstripe/sapphire/commit/78ab9d3) Video embed from Add Media Feature no longer works (open #8033) (stojg) + +### Other + + * 2012-11-09 [05a44e8](https://github.com/silverstripe/sapphire/commit/05a44e8) Correct branch for Travis build status image (Ingo Schommer)