From 37d6cee83cfab87ba1c6deb24bbc25b53090f071 Mon Sep 17 00:00:00 2001 From: Uncle Cheese Date: Thu, 20 Apr 2017 11:50:45 +1200 Subject: [PATCH 01/15] Remove .tx folder --- .tx/config | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 .tx/config diff --git a/.tx/config b/.tx/config deleted file mode 100644 index 23770d0d4..000000000 --- a/.tx/config +++ /dev/null @@ -1,20 +0,0 @@ -[main] -host = https://www.transifex.com - -[silverstripe-framework.master] -file_filter = lang/.yml -source_file = lang/en.yml -source_lang = en -type = YML - -[silverstripe-framework.master-js] -file_filter = javascript/lang/src/.js -source_file = javascript/lang/src/en.js -source_lang = en -type = KEYVALUEJSON - -[silverstripe-framework.master-admin-js] -file_filter = admin/javascript/lang/src/.js -source_file = admin/javascript/lang/src/en.js -source_lang = en -type = KEYVALUEJSON \ No newline at end of file From 0ef6d4fb795d3456adfba5d58542401f0afe4f2f Mon Sep 17 00:00:00 2001 From: Uncle Cheese Date: Thu, 20 Apr 2017 11:51:46 +1200 Subject: [PATCH 02/15] Remove .tx folder --- .tx/config | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 .tx/config diff --git a/.tx/config b/.tx/config deleted file mode 100644 index 23770d0d4..000000000 --- a/.tx/config +++ /dev/null @@ -1,20 +0,0 @@ -[main] -host = https://www.transifex.com - -[silverstripe-framework.master] -file_filter = lang/.yml -source_file = lang/en.yml -source_lang = en -type = YML - -[silverstripe-framework.master-js] -file_filter = javascript/lang/src/.js -source_file = javascript/lang/src/en.js -source_lang = en -type = KEYVALUEJSON - -[silverstripe-framework.master-admin-js] -file_filter = admin/javascript/lang/src/.js -source_file = admin/javascript/lang/src/en.js -source_lang = en -type = KEYVALUEJSON \ No newline at end of file From 1d36f354e8349616c7b39fcade859fbcf0f9c362 Mon Sep 17 00:00:00 2001 From: Gregory Smirnov Date: Mon, 24 Apr 2017 21:53:20 +0200 Subject: [PATCH 03/15] FIX Create Image_Cached with Injector. --- model/Image.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/Image.php b/model/Image.php index 1003e4f87..433e00863 100644 --- a/model/Image.php +++ b/model/Image.php @@ -727,7 +727,7 @@ class Image extends File implements Flushable { call_user_func_array(array($this, "generateFormattedImage"), $args); } - $cached = new Image_Cached($cacheFile, false, $this); + $cached = Injector::inst()->createWithArgs('Image_Cached', array($cacheFile, false, $this)); return $cached; } } From a511e3511cace405dab7589a3406a0858cb6edf2 Mon Sep 17 00:00:00 2001 From: Patrick Nelson Date: Fri, 28 Apr 2017 01:32:18 -0700 Subject: [PATCH 04/15] FIX #6855: Mangled JS in Requirements, escaping replacement values prior to passing to preg_replace(). --- view/Requirements.php | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/view/Requirements.php b/view/Requirements.php index 4b1bd7b33..971b943e4 100644 --- a/view/Requirements.php +++ b/view/Requirements.php @@ -869,10 +869,10 @@ class Requirements_Backend { // Forcefully put the scripts at the bottom of the body instead of before the first // script tag. - $replacements["/(<\/body[^>]*>)/i"] = $jsRequirements . "\\1"; + $replacements["/(<\/body[^>]*>)/i"] = $this->escapeReplacement($jsRequirements) . "\\1"; // Put CSS at the bottom of the head - $replacements["/(<\/head>)/i"] = $requirements . "\\1"; + $replacements["/(<\/head>)/i"] = $this->escapeReplacement($requirements) . "\\1"; } elseif ($this->write_js_to_body) { $jsRequirements = $this->removeNewlinesFromCode($jsRequirements); @@ -894,14 +894,14 @@ class Requirements_Backend { if ($canWriteToBody) { $content = substr($content, 0, $p1) . $jsRequirements . substr($content, $p1); } else { - $replacements["/(<\/body[^>]*>)/i"] = $jsRequirements . "\\1"; + $replacements["/(<\/body[^>]*>)/i"] = $this->escapeReplacement($jsRequirements) . "\\1"; } // Put CSS at the bottom of the head - $replacements["/(<\/head>)/i"] = $requirements . "\\1"; + $replacements["/(<\/head>)/i"] = $this->escapeReplacement($requirements) . "\\1"; } else { // Put CSS and Javascript together before the closing head tag - $replacements["/(<\/head>)/i"] = $requirements . $jsRequirements. "\\1"; + $replacements["/(<\/head>)/i"] = $this->escapeReplacement($requirements . $jsRequirements) . "\\1"; } if (!empty($replacements)) { @@ -923,6 +923,16 @@ class Requirements_Backend { return preg_replace('/>\n*/', '>', $code); } + /** + * Safely escape a literal string for use in preg_replace replacement + * + * @param string $replacement + * @return string + */ + protected function escapeReplacement($replacement) { + return addcslashes($replacement, '\\$'); + } + /** * Attach requirements inclusion to X-Include-JS and X-Include-CSS headers on the given * HTTP Response From 2187c160b936620621fe746a1ffe36af568b21ff Mon Sep 17 00:00:00 2001 From: 3Dgoo Date: Wed, 3 May 2017 06:23:47 +0930 Subject: [PATCH 05/15] Fixing pagination api doc typo --- core/PaginatedList.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/PaginatedList.php b/core/PaginatedList.php index a332f57c8..5a655cd86 100644 --- a/core/PaginatedList.php +++ b/core/PaginatedList.php @@ -270,7 +270,7 @@ class PaginatedList extends SS_ListDecorator { * * * @param int $context The number of pages to display around the current - * page. The number should be event, as half the number of each pages + * page. The number should be even, as half the number of each pages * are displayed on either side of the current one. * @return SS_List */ From 2d138b0ef06bd93958cc0678a0afa95560648fb9 Mon Sep 17 00:00:00 2001 From: Gregory Smirnov Date: Wed, 3 May 2017 09:10:40 +0200 Subject: [PATCH 06/15] Fix class name reference consistency --- core/Object.php | 49 ++++++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/core/Object.php b/core/Object.php index 8d6c0b41b..660a1d328 100755 --- a/core/Object.php +++ b/core/Object.php @@ -726,16 +726,17 @@ abstract class Object { * @return mixed */ public function __call($method, $arguments) { + $class = get_class($this); // If the method cache was cleared by an an Object::add_extension() / Object::remove_extension() // call, then we should rebuild it. - if(empty(self::$extra_methods[get_class($this)])) { + if(empty(self::$extra_methods[$class])) { $this->defineMethods(); } $method = strtolower($method); - if(isset(self::$extra_methods[$this->class][$method])) { - $config = self::$extra_methods[$this->class][$method]; + if(isset(self::$extra_methods[$class][$method])) { + $config = self::$extra_methods[$class][$method]; switch(true) { case isset($config['property']) : @@ -752,11 +753,11 @@ abstract class Object { if($this->destroyed) { throw new Exception ( - "Object->__call(): attempt to call $method on a destroyed $this->class object" + "Object->__call(): attempt to call $method on a destroyed $class object" ); } else { throw new Exception ( - "Object->__call(): $this->class cannot pass control to $config[property]($config[index])." + "Object->__call(): $class cannot pass control to $config[property]($config[index])." . ' Perhaps this object was mistakenly destroyed?' ); } @@ -770,13 +771,12 @@ abstract class Object { default : throw new Exception ( - "Object->__call(): extra method $method is invalid on $this->class:" + "Object->__call(): extra method $method is invalid on $class:" . var_export($config, true) ); } } else { // Please do not change the exception code number below. - $class = get_class($this); throw new Exception("Object->__call(): the method '$method' does not exist on '$class', or the method is not public.", 2175); } } @@ -793,7 +793,7 @@ abstract class Object { * @return bool */ public function hasMethod($method) { - return method_exists($this, $method) || isset(self::$extra_methods[$this->class][strtolower($method)]); + return method_exists($this, $method) || isset(self::$extra_methods[get_class($this)][strtolower($method)]); } /** @@ -803,14 +803,15 @@ abstract class Object { * @return array */ public function allMethodNames($custom = false) { - if(!isset(self::$built_in_methods[$this->class])) { - self::$built_in_methods[$this->class] = array_map('strtolower', get_class_methods($this)); + $class = get_class($this); + if(!isset(self::$built_in_methods[$class])) { + self::$built_in_methods[$class] = array_map('strtolower', get_class_methods($this)); } - if($custom && isset(self::$extra_methods[$this->class])) { - return array_merge(self::$built_in_methods[$this->class], array_keys(self::$extra_methods[$this->class])); + if($custom && isset(self::$extra_methods[$class])) { + return array_merge(self::$built_in_methods[$class], array_keys(self::$extra_methods[$class])); } else { - return self::$built_in_methods[$this->class]; + return self::$built_in_methods[$class]; } } @@ -826,11 +827,12 @@ abstract class Object { $this->addMethodsFrom('extension_instances', $key); } - if(isset($_REQUEST['debugmethods']) && isset(self::$built_in_methods[$this->class])) { + $class = get_class($this); + if(isset($_REQUEST['debugmethods']) && isset(self::$built_in_methods[$class])) { Debug::require_developer_login(); - echo '

Methods defined on ' . $this->class . '

    '; - foreach(self::$built_in_methods[$this->class] as $method) { + echo "

    Methods defined on $class

      "; + foreach(self::$built_in_methods[$class] as $method) { echo "
    • $method
      • "; } echo '
    '; @@ -844,11 +846,12 @@ abstract class Object { * @param string|int $index an index to use if the property is an array */ protected function addMethodsFrom($property, $index = null) { + $class = get_class($this); $extension = ($index !== null) ? $this->{$property}[$index] : $this->$property; if(!$extension) { throw new InvalidArgumentException ( - "Object->addMethodsFrom(): could not add methods from {$this->class}->{$property}[$index]" + "Object->addMethodsFrom(): could not add methods from {$class}->{$property}[$index]" ); } @@ -873,11 +876,11 @@ abstract class Object { $newMethods = array_fill_keys($methods, $methodInfo); - if(isset(self::$extra_methods[$this->class])) { - self::$extra_methods[$this->class] = - array_merge(self::$extra_methods[$this->class], $newMethods); + if(isset(self::$extra_methods[$class])) { + self::$extra_methods[$class] = + array_merge(self::$extra_methods[$class], $newMethods); } else { - self::$extra_methods[$this->class] = $newMethods; + self::$extra_methods[$class] = $newMethods; } } } @@ -890,7 +893,7 @@ abstract class Object { * @param string $wrap the method name to wrap to */ protected function addWrapperMethod($method, $wrap) { - self::$extra_methods[$this->class][strtolower($method)] = array ( + self::$extra_methods[get_class($this)][strtolower($method)] = array ( 'wrap' => $wrap, 'method' => $method ); @@ -905,7 +908,7 @@ abstract class Object { * function */ protected function createMethod($method, $code) { - self::$extra_methods[$this->class][strtolower($method)] = array ( + self::$extra_methods[get_class($this)][strtolower($method)] = array ( 'function' => create_function('$obj, $args', $code) ); } From 197bc53c4963898d2c10621ca6d6031fdb14fe85 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 5 Apr 2017 14:24:00 +1200 Subject: [PATCH 07/15] FIX Add transparency percent argument to Image::generatePad to ensure transparency works from ::Pad --- model/Image.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/model/Image.php b/model/Image.php index 57c6cca8b..cd7b87056 100644 --- a/model/Image.php +++ b/model/Image.php @@ -339,10 +339,11 @@ class Image extends File implements Flushable { * * @param integer $width The width to size to * @param integer $height The height to size to + * @param string $backgroundColor The background colour to use on padded sides (default white) * @param integer $transparencyPercent Level of transparency * @return Image|null */ - public function Pad($width, $height, $backgroundColor='FFFFFF', $transparencyPercent = 0) { + public function Pad($width, $height, $backgroundColor = 'FFFFFF', $transparencyPercent = 0) { return $this->isSize($width, $height) && !Config::inst()->get('Image', 'force_resample') ? $this : $this->getFormattedImage('Pad', $width, $height, $backgroundColor, $transparencyPercent); @@ -354,12 +355,16 @@ class Image extends File implements Flushable { * @param Image_Backend $backend * @param integer $width The width to size to * @param integer $height The height to size to + * @param string $backgroundColor The background colour to use on padded sides (default white) + * @param integer $transparencyPercent Level of transparency * @return Image_Backend * @deprecated 4.0 Generate methods are no longer applicable */ - public function generatePad(Image_Backend $backend, $width, $height, $backgroundColor='FFFFFF') { + public function generatePad(Image_Backend $backend, $width, $height, $backgroundColor = 'FFFFFF', + $transparencyPercent = 0 + ) { Deprecation::notice('4.0', 'Generate methods are no longer applicable'); - return $backend->paddedResize($width, $height, $backgroundColor); + return $backend->paddedResize($width, $height, $backgroundColor, $transparencyPercent); } /** From efbf14be63d668d55749b819304bc2615b2b5ddc Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Mon, 3 Oct 2016 10:23:58 +0200 Subject: [PATCH 08/15] Allow filtering if a relation is defined or a formatting --- forms/gridfield/GridFieldFilterHeader.php | 38 ++++++++++++++++--- .../gridfield/GridFieldFilterHeaderTest.php | 27 +++++++++++++ 2 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 tests/forms/gridfield/GridFieldFilterHeaderTest.php diff --git a/forms/gridfield/GridFieldFilterHeader.php b/forms/gridfield/GridFieldFilterHeader.php index f55157e4b..39d17aa0b 100755 --- a/forms/gridfield/GridFieldFilterHeader.php +++ b/forms/gridfield/GridFieldFilterHeader.php @@ -102,6 +102,29 @@ class GridFieldFilterHeader implements GridField_HTMLProvider, GridField_DataMan return $dataListClone; } + /** + * @param string $class + * @param string $column + * @return string + */ + protected function columnToFilterField($class, $column) + { + if (strpos($column, '.') === false) { + return $column; + } + /** @var DataObject $model */ + $model = singleton($class); + $columnParts = explode('.', $column); + if ($model->getRelationClass($columnParts[0])) { + return $column; + } + return $columnParts[0]; + } + + /** + * @param GridField $gridField + * @return array + */ public function getHTMLFragments($gridField) { if(!$this->checkDataType($gridField->getList())) return; @@ -110,18 +133,21 @@ class GridFieldFilterHeader implements GridField_HTMLProvider, GridField_DataMan $columns = $gridField->getColumns(); $filterArguments = $gridField->State->GridFieldFilterHeader->Columns->toArray(); $currentColumn = 0; + foreach($columns as $columnField) { - $currentColumn++; + ++$currentColumn; $metadata = $gridField->getColumnMetadata($columnField); $title = $metadata['title']; $fields = new FieldGroup(); - if($title && $gridField->getList()->canFilterBy($columnField)) { + $filterField = $this->columnToFilterField($gridField->getModelClass(), $columnField); + + if($title && $gridField->getList()->canFilterBy($filterField)) { $value = ''; - if(isset($filterArguments[$columnField])) { - $value = $filterArguments[$columnField]; + if(isset($filterArguments[$filterField])) { + $value = $filterArguments[$filterField]; } - $field = new TextField('filter[' . $gridField->getName() . '][' . $columnField . ']', '', $value); + $field = new TextField('filter[' . $gridField->getName() . '][' . $filterField . ']', '', $value); $field->addExtraClass('ss-gridfield-sort'); $field->addExtraClass('no-change-track'); @@ -133,7 +159,7 @@ class GridFieldFilterHeader implements GridField_HTMLProvider, GridField_DataMan GridField_FormAction::create($gridField, 'reset', false, 'reset', null) ->addExtraClass('ss-gridfield-button-reset') ->setAttribute('title', _t('GridField.ResetFilter', "Reset")) - ->setAttribute('id', 'action_reset_' . $gridField->getModelClass() . '_' . $columnField) + ->setAttribute('id', 'action_reset_' . $gridField->getModelClass() . '_' . $filterField) ); } diff --git a/tests/forms/gridfield/GridFieldFilterHeaderTest.php b/tests/forms/gridfield/GridFieldFilterHeaderTest.php new file mode 100644 index 000000000..a7c715b4c --- /dev/null +++ b/tests/forms/gridfield/GridFieldFilterHeaderTest.php @@ -0,0 +1,27 @@ +setAccessible(true); + $this->assertEquals('Title', $method->invoke($header, $class,'Title.ATT')); + $this->assertEquals('isTest', $method->invoke($header, $class, 'isTest.Nice')); + } + +} + +class GridFieldFilterHeaderTest_DataObject extends DataObject implements TestOnly { + + private static $db = array( + 'Title' => 'Varchar', + 'isTest' => 'Boolean', + ); + +} From 49a0354998b28319588fdd63d3b913bb8f93aa00 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Wed, 17 May 2017 23:01:42 +0100 Subject: [PATCH 09/15] Make sure that nested relations dont break --- tests/forms/gridfield/GridFieldFilterHeaderTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/forms/gridfield/GridFieldFilterHeaderTest.php b/tests/forms/gridfield/GridFieldFilterHeaderTest.php index a7c715b4c..fb47d1fe4 100644 --- a/tests/forms/gridfield/GridFieldFilterHeaderTest.php +++ b/tests/forms/gridfield/GridFieldFilterHeaderTest.php @@ -13,6 +13,7 @@ class GridFieldFilterHeaderTest extends SapphireTest { $method->setAccessible(true); $this->assertEquals('Title', $method->invoke($header, $class,'Title.ATT')); $this->assertEquals('isTest', $method->invoke($header, $class, 'isTest.Nice')); + $this->assertEquals('Self.isTest.Nice', $method->invoke($header, $class, 'Self.isTest.Nice')); } } @@ -24,4 +25,8 @@ class GridFieldFilterHeaderTest_DataObject extends DataObject implements TestOnl 'isTest' => 'Boolean', ); + private static $has_one = array( + 'Self' => 'GridFieldFilterHeaderTest_DataObject', + ); + } From 85f06507963acc53d5e1515cc8c20a8639541d6f Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Wed, 24 May 2017 14:13:46 +0100 Subject: [PATCH 10/15] Remove unnecessary nesting of config/injector in tests --- tests/control/CookieTest.php | 7 ----- tests/filesystem/UploadTest.php | 6 +---- tests/forms/FormFieldTest.php | 4 --- tests/forms/FormTest.php | 4 --- tests/forms/HtmlEditorFieldToolbarTest.php | 7 ----- tests/model/DataObjectTest.php | 31 +++------------------- tests/model/VersionableExtensionsTest.php | 7 +++++ tests/security/MemberTest.php | 1 - tests/security/SecurityTest.php | 4 --- 9 files changed, 11 insertions(+), 60 deletions(-) diff --git a/tests/control/CookieTest.php b/tests/control/CookieTest.php index 7b1b14a5d..2eeb670da 100644 --- a/tests/control/CookieTest.php +++ b/tests/control/CookieTest.php @@ -6,16 +6,9 @@ class CookieTest extends SapphireTest { public function setUp() { parent::setUp(); - Injector::nest(); Injector::inst()->registerService(new CookieJar($_COOKIE), 'Cookie_Backend'); } - public function tearDown() { - //restore the cookie_backend - Injector::unnest(); - parent::tearDown(); - } - /** * Check a new cookie inst will be loaded with the superglobal by default */ diff --git a/tests/filesystem/UploadTest.php b/tests/filesystem/UploadTest.php index 5c02e9447..58faa8e4e 100644 --- a/tests/filesystem/UploadTest.php +++ b/tests/filesystem/UploadTest.php @@ -134,8 +134,6 @@ class UploadTest extends SapphireTest { } public function testGetAllowedMaxFileSize() { - Config::nest(); - // Check the max file size uses the config values $configMaxFileSizes = array( '[image]' => '1k', @@ -176,15 +174,13 @@ class UploadTest extends SapphireTest { $retrievedSize = $v->getAllowedMaxFileSize('txt'); $this->assertEquals(4096, $retrievedSize, 'Max file size check on instance values failed (instance extension set check)'); - + // Check a wildcard max file size against a file with an extension $v = new UploadTest_Validator(); $v->setAllowedMaxFileSize(2000); $retrievedSize = $v->getAllowedMaxFileSize('.jpg'); $this->assertEquals(2000, $retrievedSize, 'Max file size check on instance values failed (wildcard max file size)'); - - Config::unnest(); } public function testAllowedSizeOnFileWithNoExtension() { diff --git a/tests/forms/FormFieldTest.php b/tests/forms/FormFieldTest.php index 221091dad..76658c8b2 100644 --- a/tests/forms/FormFieldTest.php +++ b/tests/forms/FormFieldTest.php @@ -10,8 +10,6 @@ class FormFieldTest extends SapphireTest { ); public function testDefaultClasses() { - Config::nest(); - Config::inst()->update('FormField', 'default_classes', array( 'class1', )); @@ -50,8 +48,6 @@ class FormFieldTest extends SapphireTest { //check default classes inherit $this->assertContains('class3', $field->extraClass(), 'Class list does not contain inherited class'); $this->assertContains('textfield-class', $field->extraClass(), 'Class list does not contain expected class'); - - Config::unnest(); } public function testAddExtraClass() { diff --git a/tests/forms/FormTest.php b/tests/forms/FormTest.php index 910bbc0ce..26e0288aa 100644 --- a/tests/forms/FormTest.php +++ b/tests/forms/FormTest.php @@ -549,8 +549,6 @@ class FormTest extends FunctionalTest { } public function testDefaultClasses() { - Config::nest(); - Config::inst()->update('Form', 'default_classes', array( 'class1', )); @@ -579,8 +577,6 @@ class FormTest extends FunctionalTest { $form->removeExtraClass('class3'); $this->assertNotContains('class3', $form->extraClass(), 'Class list contains unexpected class'); - - Config::unnest(); } public function testAttributes() { diff --git a/tests/forms/HtmlEditorFieldToolbarTest.php b/tests/forms/HtmlEditorFieldToolbarTest.php index 029de1c56..729647fd2 100644 --- a/tests/forms/HtmlEditorFieldToolbarTest.php +++ b/tests/forms/HtmlEditorFieldToolbarTest.php @@ -25,17 +25,10 @@ class HtmlEditorFieldToolbarTest extends SapphireTest { public function setUp() { parent::setUp(); - Config::nest(); Config::inst()->update('HtmlEditorField_Toolbar', 'fileurl_scheme_whitelist', array('http')); Config::inst()->update('HtmlEditorField_Toolbar', 'fileurl_domain_whitelist', array('example.com')); } - public function tearDown() { - Config::unnest(); - - parent::tearDown(); - } - public function testValidLocalReference() { list($file, $url) = $this->getToolbar()->viewfile_getLocalFileByURL('folder/subfolder/example.pdf'); $this->assertEquals($this->objFromFixture('File', 'example_file'), $file); diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index 79a685981..2e4e840fc 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -1126,7 +1126,6 @@ class DataObjectTest extends SapphireTest { } public function testValidateModelDefinitionsFailsWithArray() { - Config::nest(); $object = new DataObjectTest_Team; $method = $this->makeAccessible($object, 'validateModelDefinitions'); @@ -1134,54 +1133,33 @@ class DataObjectTest extends SapphireTest { Config::inst()->update('DataObjectTest_Team', 'has_one', array('NotValid' => array('NoArraysAllowed'))); $this->setExpectedException('LogicException'); - try { - $method->invoke($object); - } catch(Exception $e) { - Config::unnest(); // Catch the exception so we can unnest config before failing the test - throw $e; - } + $method->invoke($object); } public function testValidateModelDefinitionsFailsWithIntKey() { - Config::nest(); - $object = new DataObjectTest_Team; $method = $this->makeAccessible($object, 'validateModelDefinitions'); Config::inst()->update('DataObjectTest_Team', 'has_many', array(12 => 'DataObjectTest_Player')); $this->setExpectedException('LogicException'); - try { - $method->invoke($object); - } catch(Exception $e) { - Config::unnest(); // Catch the exception so we can unnest config before failing the test - throw $e; - } + $method->invoke($object); } public function testValidateModelDefinitionsFailsWithIntValue() { - Config::nest(); - $object = new DataObjectTest_Team; $method = $this->makeAccessible($object, 'validateModelDefinitions'); Config::inst()->update('DataObjectTest_Team', 'many_many', array('Players' => 12)); $this->setExpectedException('LogicException'); - try { - $method->invoke($object); - } catch(Exception $e) { - Config::unnest(); // Catch the exception so we can unnest config before failing the test - throw $e; - } + $method->invoke($object); } /** * many_many_extraFields is allowed to have an array value, so shouldn't throw an exception */ public function testValidateModelDefinitionsPassesWithExtraFields() { - Config::nest(); - $object = new DataObjectTest_Team; $method = $this->makeAccessible($object, 'validateModelDefinitions'); @@ -1191,12 +1169,9 @@ class DataObjectTest extends SapphireTest { try { $method->invoke($object); } catch(Exception $e) { - Config::unnest(); $this->fail('Exception should not be thrown'); throw $e; } - - Config::unnest(); } public function testNewClassInstance() { diff --git a/tests/model/VersionableExtensionsTest.php b/tests/model/VersionableExtensionsTest.php index e08209c52..59750db8c 100644 --- a/tests/model/VersionableExtensionsTest.php +++ b/tests/model/VersionableExtensionsTest.php @@ -37,6 +37,13 @@ class VersionableExtensionsTest extends SapphireTest parent::setUpOnce(); } + public function tearDownOnce() + { + parent::tearDownOnce(); + + Config::unnest(); + } + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/tests/security/MemberTest.php b/tests/security/MemberTest.php index 595b7a821..d1db139e5 100644 --- a/tests/security/MemberTest.php +++ b/tests/security/MemberTest.php @@ -846,7 +846,6 @@ class MemberTest extends FunctionalTest { public function testFailedLoginCount() { $maxFailedLoginsAllowed = 3; //set up the config variables to enable login lockouts - Config::nest(); Config::inst()->update('Member', 'lock_out_after_incorrect_logins', $maxFailedLoginsAllowed); $member = $this->objFromFixture('Member', 'test'); diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index 45463d6fe..c709fbb06 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -75,8 +75,6 @@ class SecurityTest extends FunctionalTest { } public function testPermissionFailureSetsCorrectFormMessages() { - Config::nest(); - // Controller that doesn't attempt redirections $controller = new SecurityTest_NullController(); $controller->setResponse(new SS_HTTPResponse()); @@ -111,8 +109,6 @@ class SecurityTest extends FunctionalTest { array('default' => 'default', 'alreadyLoggedIn' => 'One-off failure message')); $this->assertContains('One-off failure message', $controller->getResponse()->getBody(), "Message set passed to Security::permissionFailure() didn't override Config values"); - - Config::unnest(); } /** From 447ce0f84f880c2bc969a89e4be528c53caeabe0 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 9 May 2017 21:24:15 +0100 Subject: [PATCH 11/15] [SS-2017-002] FIX Lock out users who dont exist in the DB --- security/Member.php | 33 +++++++++++++++-- security/MemberAuthenticator.php | 10 +++++- tests/security/MemberAuthenticatorTest.php | 41 +++++++++++++++++++++- tests/security/SecurityTest.php | 18 +++++----- 4 files changed, 88 insertions(+), 14 deletions(-) diff --git a/security/Member.php b/security/Member.php index 692ff61a5..2648c800d 100644 --- a/security/Member.php +++ b/security/Member.php @@ -398,7 +398,35 @@ class Member extends DataObject implements TemplateGlobalProvider { * Returns true if this user is locked out */ public function isLockedOut() { - return $this->LockedOutUntil && SS_Datetime::now()->Format('U') < strtotime($this->LockedOutUntil); + global $debug; + if ($this->LockedOutUntil && $this->dbObject('LockedOutUntil')->InFuture()) { + return true; + } + + if ($this->config()->lock_out_after_incorrect_logins <= 0) { + return false; + } + + $attempts = LoginAttempt::get()->filter($filter = array( + 'Email' => $this->{static::config()->unique_identifier_field}, + ))->sort('Created', 'DESC')->limit($this->config()->lock_out_after_incorrect_logins); + + if ($attempts->count() < $this->config()->lock_out_after_incorrect_logins) { + return false; + } + + foreach ($attempts as $attempt) { + if ($attempt->Status === 'Success') { + return false; + } + } + + $lockedOutUntil = $attempts->first()->dbObject('Created')->Format('U') + ($this->config()->lock_out_delay_mins * 60); + if (SS_Datetime::now()->Format('U') < $lockedOutUntil) { + return true; + } + + return false; } /** @@ -1660,7 +1688,7 @@ class Member extends DataObject implements TemplateGlobalProvider { public function registerFailedLogin() { if(self::config()->lock_out_after_incorrect_logins) { // Keep a tally of the number of failed log-ins so that we can lock people out - $this->FailedLoginCount = $this->FailedLoginCount + 1; + ++$this->FailedLoginCount; if($this->FailedLoginCount >= self::config()->lock_out_after_incorrect_logins) { $lockoutMins = self::config()->lock_out_delay_mins; @@ -1679,6 +1707,7 @@ class Member extends DataObject implements TemplateGlobalProvider { if(self::config()->lock_out_after_incorrect_logins) { // Forgive all past login failures $this->FailedLoginCount = 0; + $this->LockedOutUntil = null; $this->write(); } } diff --git a/security/MemberAuthenticator.php b/security/MemberAuthenticator.php index 4a1635ab6..01586ce65 100644 --- a/security/MemberAuthenticator.php +++ b/security/MemberAuthenticator.php @@ -70,6 +70,14 @@ class MemberAuthenticator extends Authenticator { if($member && !$asDefaultAdmin) { $result = $member->checkPassword($data['Password']); $success = $result->valid(); + } elseif (!$asDefaultAdmin) { + // spoof a login attempt + $member = Member::create(); + $member->Email = $email; + $member->{Member::config()->unique_identifier_field} = $data['Password'] . '-wrong'; + $member->PasswordEncryption = 'none'; + $result = $member->checkPassword($data['Password']); + $member = null; } else { $result = new ValidationResult(false, _t('Member.ERRORWRONGCRED')); } @@ -94,7 +102,7 @@ class MemberAuthenticator extends Authenticator { * @param bool $success */ protected static function record_login_attempt($data, $member, $success) { - if(!Security::config()->login_recording) return; + if(!Security::config()->login_recording && !Member::config()->lock_out_after_incorrect_logins) return; // Check email is valid $email = isset($data['Email']) ? $data['Email'] : null; diff --git a/tests/security/MemberAuthenticatorTest.php b/tests/security/MemberAuthenticatorTest.php index d5b9ac8d2..8606e6f58 100644 --- a/tests/security/MemberAuthenticatorTest.php +++ b/tests/security/MemberAuthenticatorTest.php @@ -180,6 +180,45 @@ class MemberAuthenticatorTest extends SapphireTest { ), $form); $this->assertTrue(Member::default_admin()->isLockedOut()); - $this->assertEquals(Member::default_admin()->LockedOutUntil, '2016-04-18 00:10:00'); + $this->assertEquals('2016-04-18 00:10:00', Member::default_admin()->LockedOutUntil); + } + + public function testNonExistantMemberGetsLoginAttemptRecorded() + { + Config::inst()->update('Member', 'lock_out_after_incorrect_logins', 1); + $email = 'notreal@example.com'; + $this->assertFalse(Member::get()->filter(array('Email' => $email))->exists()); + $this->assertCount(0, LoginAttempt::get()); + $response = MemberAuthenticator::authenticate(array( + 'Email' => $email, + 'Password' => 'password', + )); + $this->assertNull($response); + $this->assertCount(1, LoginAttempt::get()); + $attempt = LoginAttempt::get()->first(); + $this->assertEquals($email, $attempt->Email); + $this->assertEquals('Failure', $attempt->Status); + + } + + public function testNonExistantMemberGetsLockedOut() + { + Config::inst()->update('Member', 'lock_out_after_incorrect_logins', 1); + Config::inst()->update('Member', 'lock_out_delay_mins', 10); + $email = 'notreal@example.com'; + + $this->assertFalse(Member::get()->filter(array('Email' => $email))->exists()); + + $response = MemberAuthenticator::authenticate(array( + 'Email' => $email, + 'Password' => 'password' + )); + + $this->assertNull($response); + $member = new Member(); + $member->Email = $email; + + $this->assertTrue($member->isLockedOut()); + $this->assertFalse($member->canLogIn()->valid()); } } diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index 45463d6fe..4edc90a7c 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -402,6 +402,7 @@ class SecurityTest extends FunctionalTest { public function testRepeatedLoginAttemptsLockingPeopleOut() { $local = i18n::get_locale(); i18n::set_locale('en_US'); + SS_Datetime::set_mock_now(DBField::create_field('SS_Datetime', '2017-05-22 00:00:00')); Member::config()->lock_out_after_incorrect_logins = 5; Member::config()->lock_out_delay_mins = 15; @@ -418,10 +419,9 @@ class SecurityTest extends FunctionalTest { ); $this->assertContains($this->loginErrorMessage(), Convert::raw2xml(_t('Member.ERRORWRONGCRED'))); } else { - // Fuzzy matching for time to avoid side effects from slow running tests - $this->assertGreaterThan( - time() + 14*60, - strtotime($member->LockedOutUntil), + $this->assertEquals( + SS_Datetime::now()->Format('U') + (15 * 60), + $member->dbObject('LockedOutUntil')->Format('U'), 'User has a lockout time set after too many failed attempts' ); } @@ -444,14 +444,12 @@ class SecurityTest extends FunctionalTest { 'The user can\'t log in after being locked out, even with the right password' ); - // (We fake this by re-setting LockedOutUntil) - $member = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'test')); - $member->LockedOutUntil = date('Y-m-d H:i:s', time() - 30); - $member->write(); + // Move into the future so we can login again + SS_Datetime::set_mock_now(DBField::create_field('SS_Datetime', '2017-06-22 00:00:00')); $this->doTestLoginForm('testuser@example.com' , '1nitialPassword'); $this->assertEquals( - $this->session()->inst_get('loggedInAs'), $member->ID, + $this->session()->inst_get('loggedInAs'), 'After lockout expires, the user can login again' ); @@ -471,8 +469,8 @@ class SecurityTest extends FunctionalTest { $this->doTestLoginForm('testuser@example.com' , '1nitialPassword'); $this->assertEquals( - $this->session()->inst_get('loggedInAs'), $member->ID, + $this->session()->inst_get('loggedInAs'), 'The user can login successfully after lockout expires, if staying below the threshold' ); From eeb549faf37df5aee1840423d5aefc25bb51c612 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Sun, 28 May 2017 21:34:38 +0000 Subject: [PATCH 12/15] Added 3.4.6-rc1 changelog --- docs/en/04_Changelogs/rc/3.4.6-rc1.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 docs/en/04_Changelogs/rc/3.4.6-rc1.md diff --git a/docs/en/04_Changelogs/rc/3.4.6-rc1.md b/docs/en/04_Changelogs/rc/3.4.6-rc1.md new file mode 100644 index 000000000..0bea252f0 --- /dev/null +++ b/docs/en/04_Changelogs/rc/3.4.6-rc1.md @@ -0,0 +1,18 @@ +# 3.4.6-rc1 + + + +## Change Log + +### Security + + * 2017-05-24 [41270fc](https://github.com/silverstripe/silverstripe-cms/commit/41270fcf9980c4be2529d2750c717675548eb617) Only allow HTTP(S) links for external redirector pages (Daniel Hensby) - See [ss-2017-003](http://www.silverstripe.org/download/security-releases/ss-2017-003) + * 2017-05-09 [447ce0f](https://github.com/silverstripe/silverstripe-framework/commit/447ce0f84f880c2bc969a89e4be528c53caeabe0) Lock out users who dont exist in the DB (Daniel Hensby) - See [ss-2017-002](http://www.silverstripe.org/download/security-releases/ss-2017-002) + * 2017-05-09 [61cf72c](https://github.com/silverstripe/silverstripe-cms/commit/61cf72c08dafddef416d73f943ccd45e70c5d43d) Unescaped fields in CMSPageHistroyController::compare() (Daniel Hensby) - See [ss-2017-004](http://www.silverstripe.org/download/security-releases/ss-2017-004) + +### Bugfixes + + * 2017-05-03 [2d138b0](https://github.com/silverstripe/silverstripe-framework/commit/2d138b0ef06bd93958cc0678a0afa95560648fb9) class name reference consistency (Gregory Smirnov) + * 2017-04-24 [1d36f35](https://github.com/silverstripe/silverstripe-framework/commit/1d36f354e8349616c7b39fcade859fbcf0f9c362) Create Image_Cached with Injector. (Gregory Smirnov) + * 2017-02-15 [3072591](https://github.com/silverstripe/silverstripe-framework/commit/30725916dbb0ffc66b77f26c069a86581636ae55) Array to string conversion message after CSV export (#6622) (Juan van den Anker) + * 2017-02-14 [7122e1f](https://github.com/silverstripe/silverstripe-framework/commit/7122e1fde79bdb9aad3c8714a6ce02b7ecedd735) Comments ignored by classmanifest (#6619) (Daniel Hensby) From 16a74bc8a9fdee7cfb4f6f24493c271f90a76341 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Sun, 28 May 2017 23:59:00 +0100 Subject: [PATCH 13/15] FIX DataDifferencer needs to expliclty cast HTMLText values --- model/DataDifferencer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/DataDifferencer.php b/model/DataDifferencer.php index 0c33b1206..0cd573259 100644 --- a/model/DataDifferencer.php +++ b/model/DataDifferencer.php @@ -103,7 +103,7 @@ class DataDifferencer extends ViewableData { // Show changes between the two, if any exist if($fromValue != $toValue) { - $diffed->setField($field, Diff::compareHTML($fromValue, $toValue)); + $diffed->setField($field, DBField::create_field('HTMLText', Diff::compareHTML($fromValue, $toValue))); } } From b5ad4bdcc622cbe7aed496e7ad3d92708624b80e Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Sun, 28 May 2017 23:49:04 +0000 Subject: [PATCH 14/15] Added 3.4.6-rc2 changelog --- docs/en/04_Changelogs/rc/3.4.6-rc2.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 docs/en/04_Changelogs/rc/3.4.6-rc2.md diff --git a/docs/en/04_Changelogs/rc/3.4.6-rc2.md b/docs/en/04_Changelogs/rc/3.4.6-rc2.md new file mode 100644 index 000000000..d3a388e73 --- /dev/null +++ b/docs/en/04_Changelogs/rc/3.4.6-rc2.md @@ -0,0 +1,9 @@ +# 3.4.6-rc2 + + + +## Change Log + +### Bugfixes + + * 2017-05-28 [16a74bc](https://github.com/silverstripe/silverstripe-framework/commit/16a74bc8a9fdee7cfb4f6f24493c271f90a76341) DataDifferencer needs to expliclty cast HTMLText values (Daniel Hensby) From 9a38bedd18187b820bfd46cb26e34ce44ffc3037 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Mon, 29 May 2017 00:08:27 +0000 Subject: [PATCH 15/15] Added 3.5.4-rc1 changelog --- docs/en/04_Changelogs/rc/3.5.4-rc1.md | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 docs/en/04_Changelogs/rc/3.5.4-rc1.md diff --git a/docs/en/04_Changelogs/rc/3.5.4-rc1.md b/docs/en/04_Changelogs/rc/3.5.4-rc1.md new file mode 100644 index 000000000..eae808d35 --- /dev/null +++ b/docs/en/04_Changelogs/rc/3.5.4-rc1.md @@ -0,0 +1,28 @@ +# 3.5.4-rc1 + + + +## Change Log + +### Security + + * 2017-05-24 [41270fc](https://github.com/silverstripe/silverstripe-cms/commit/41270fcf9980c4be2529d2750c717675548eb617) Only allow HTTP(S) links for external redirector pages (Daniel Hensby) - See [ss-2017-003](http://www.silverstripe.org/download/security-releases/ss-2017-003) + * 2017-05-09 [447ce0f](https://github.com/silverstripe/silverstripe-framework/commit/447ce0f84f880c2bc969a89e4be528c53caeabe0) Lock out users who dont exist in the DB (Daniel Hensby) - See [ss-2017-002](http://www.silverstripe.org/download/security-releases/ss-2017-002) + * 2017-05-09 [61cf72c](https://github.com/silverstripe/silverstripe-cms/commit/61cf72c08dafddef416d73f943ccd45e70c5d43d) Unescaped fields in CMSPageHistroyController::compare() (Daniel Hensby) - See [ss-2017-004](http://www.silverstripe.org/download/security-releases/ss-2017-004) + +### Bugfixes + + * 2017-05-28 [16a74bc](https://github.com/silverstripe/silverstripe-framework/commit/16a74bc8a9fdee7cfb4f6f24493c271f90a76341) DataDifferencer needs to expliclty cast HTMLText values (Daniel Hensby) + * 2017-05-08 [1454072](https://github.com/silverstripe/silverstripe-cms/commit/14540729caa30dd2e782e4fd52afe518dc156ed8) Use framework 3.5 to test cms 3.5 (Sam Minnee) + * 2017-05-03 [2d138b0](https://github.com/silverstripe/silverstripe-framework/commit/2d138b0ef06bd93958cc0678a0afa95560648fb9) class name reference consistency (Gregory Smirnov) + * 2017-05-02 [2187c16](https://github.com/silverstripe/silverstripe-framework/commit/2187c160b936620621fe746a1ffe36af568b21ff) ing pagination api doc typo (3Dgoo) + * 2017-04-28 [a511e35](https://github.com/silverstripe/silverstripe-framework/commit/a511e3511cace405dab7589a3406a0858cb6edf2) #6855: Mangled JS in Requirements, escaping replacement values prior to passing to preg_replace(). (Patrick Nelson) + * 2017-04-24 [1d36f35](https://github.com/silverstripe/silverstripe-framework/commit/1d36f354e8349616c7b39fcade859fbcf0f9c362) Create Image_Cached with Injector. (Gregory Smirnov) + * 2017-04-07 [55eb7eb](https://github.com/silverstripe/silverstripe-framework/commit/55eb7ebdcc9ba767f978dff510614bbd2e0c309d) Do not insert requirements more than once in includeInHTML (Robbie Averill) + * 2017-04-05 [a7920b1](https://github.com/silverstripe/silverstripe-framework/commit/a7920b1f9866b6eb5f4bad9de84eef84b88673ad) regression from #6668 - ModelAdmin form widths (Loz Calver) + * 2017-04-05 [197bc53](https://github.com/silverstripe/silverstripe-framework/commit/197bc53c4963898d2c10621ca6d6031fdb14fe85) Add transparency percent argument to Image::generatePad to ensure transparency works from ::Pad (Robbie Averill) + * 2017-02-15 [3072591](https://github.com/silverstripe/silverstripe-framework/commit/30725916dbb0ffc66b77f26c069a86581636ae55) Array to string conversion message after CSV export (#6622) (Juan van den Anker) + * 2017-02-14 [7122e1f](https://github.com/silverstripe/silverstripe-framework/commit/7122e1fde79bdb9aad3c8714a6ce02b7ecedd735) Comments ignored by classmanifest (#6619) (Daniel Hensby) + * 2017-02-09 [6e2797f](https://github.com/silverstripe/silverstripe-framework/commit/6e2797ffc0e9632b60acc5a66e52aeb44f0e2b78) es for using dblib PDO driver. (Andrew O'Neil) + * 2017-02-08 [c25c443](https://github.com/silverstripe/silverstripe-framework/commit/c25c443d95fc305fb3545b1393b7da85923dcf8b) Fix minor mysql 5.7 warning in SQLQueryTest (#6608) (Damian Mooyman) + * 2017-01-18 [72b6fb4](https://github.com/silverstripe/silverstripe-framework/commit/72b6fb49b698bc3a51c8f6b32d2bf08213729493) bug: In addOrderBy method, _SortColumn will only keep the last one if there are more than 1 multi-word columns (Shawn)