Merge remote-tracking branch 'origin/4.0.4' into 4.1.1

# Conflicts:
  #	src/Control/Director.php

docs/en/02_Developer_Guides/14_Files/03_File_Security.md

@@ -348,6 +348,24 @@ RewriteRule .* ../index.php [QSA]
 You will need to ensure that your core apache configuration has the necessary `AllowOverride`
 settings to support the local .htaccess file.
 
+Although assets have a 404 handler which routes to a PHP handler, .php files within assets itself
+should not be allowed to be marked as executable.
+
+When securing your server you should ensure that you protect against both files that can be uploaded as
+executable on the server, as well as protect against accidental upload of `.htaccess` which bypasses
+this file security.
+
+For instance your server configuration should look similar to the below:
+
+```
+<Directory "/var/www/superarcade/public/assets">
+  php_admin_flag engine off
+</Directory>
+```
+
+The `php_admin_flag` will protect against uploaded `.htaccess` files accidentally re-enabling script
+execution within the assets directory.
+
 #### Configuring Web Server: Windows IIS 7.5+
 
 Configuring via IIS requires the Rewrite extension to be installed and configured properly.

docs/en/04_Changelogs/4.0.4.md

@@ -0,0 +1,32 @@
+# 4.0.4
+
+This security release removes the following file extensions from the default whitelist of accepted types for 
+uploaded files: `dotm`, `potm`, `jar`, `css`, `js` and `xltm`.
+
+If you require the ability to upload these file types in your projects, you will need to add them back in again.
+For more information, see ["Configuring: File types"](https://docs.silverstripe.org/en/4/developer_guides/files/file_security/#configuring-file-types).
+
+<!--- Changes below this line will be automatically regenerated -->
+
+## Change Log
+
+### Security
+
+ * 2018-04-26 [299131ed2](https://github.com/silverstripe/silverstripe-framework/commit/299131ed2) File security documentation (Damian Mooyman) - See [ss-2018-012](http://www.silverstripe.org/download/security-releases/ss-2018-012)
+ * 2018-04-25 [be96858](https://github.com/silverstripe/silverstripe-installer/commit/be96858e85272ca62f6f0ff3e24a44aa0248ac4d) Remove jar, dotm, potm, xltm from file extension whitelist, hard-code CSS and JS for TinyMCE support (Robbie Averill) - See [ss-2018-014](http://www.silverstripe.org/download/security-releases/ss-2018-014)
+ * 2018-04-24 [f847f186b](https://github.com/silverstripe/silverstripe-framework/commit/f847f186b) Remove password text from session data on failed submission (Aaron Carlino) - See [ss-2018-013](http://www.silverstripe.org/download/security-releases/ss-2018-013)
+ * 2018-04-23 [aa365e0](https://github.com/silverstripe/silverstripe-assets/commit/aa365e0) Remove dotm, potm, jar, css, js, xltm from default File.allowed_extensions (Robbie Averill) - See [ss-2018-014](http://www.silverstripe.org/download/security-releases/ss-2018-014)
+ * 2018-04-23 [f9c03fa](https://github.com/silverstripe/silverstripe-installer/commit/f9c03fa623dc7237005901efd863256b7d356db7) Prevent php code execution in assets folder (Damian Mooyman) - See [ss-2018-012](http://www.silverstripe.org/download/security-releases/ss-2018-012)
+ * 2018-04-23 [1e27835](https://github.com/silverstripe/silverstripe-assets/commit/1e27835) Prevent php code execution in assets folder (Damian Mooyman) - See [ss-2018-012](http://www.silverstripe.org/download/security-releases/ss-2018-012)
+ * 2018-04-22 [beec0c0d4](https://github.com/silverstripe/silverstripe-framework/commit/beec0c0d4) regression of SS-2017-002 (Robbie Averill) - See [ss-2018-010](http://www.silverstripe.org/download/security-releases/ss-2018-010)
+ * 2018-04-11 [e409d6f67](https://github.com/silverstripe/silverstripe-framework/commit/e409d6f67) Restrict non-admins from being assigned to admin groups (Damian Mooyman) - See [ss-2018-001](http://www.silverstripe.org/download/security-releases/ss-2018-001)
+ * 2018-04-10 [9053014a7](https://github.com/silverstripe/silverstripe-framework/commit/9053014a7) Validate against malformed urls (Damian Mooyman) - See [ss-2018-008](http://www.silverstripe.org/download/security-releases/ss-2018-008)
+ * 2018-04-10 [2e13ae746](https://github.com/silverstripe/silverstripe-framework/commit/2e13ae746) Prevent code execution in template value resolution (Damian Mooyman) - See [ss-2018-006](http://www.silverstripe.org/download/security-releases/ss-2018-006)
+ * 2018-04-09 [db04ed9](https://github.com/silverstripe/silverstripe-admin/commit/db04ed9) Remove on* events as allowed properties (Damian Mooyman) - See [ss-2018-004](http://www.silverstripe.org/download/security-releases/ss-2018-004)
+ * 2018-04-08 [d935140a9](https://github.com/silverstripe/silverstripe-framework/commit/d935140a9) Prevent unauthenticated isDev / isTest being allowed (Damian Mooyman) - See [ss-2018-005](http://www.silverstripe.org/download/security-releases/ss-2018-005)
+
+### Bugfixes
+
+ * 2018-05-23 [e7e32d13a](https://github.com/silverstripe/silverstripe-framework/commit/e7e32d13a) Add namespace and encryptor to tests that expect blowfish to be available (Robbie Averill)
+ * 2018-02-13 [c6095cf](https://github.com/silverstripe/silverstripe-config/commit/c6095cfc0a07a74bb932e2191215d06f102e992a) word boundary issue with pathname matching (Christopher Joe)
+ * 2018-02-06 [5bff64b47](https://github.com/silverstripe/silverstripe-framework/commit/5bff64b47) Fix Director::test() not persisting removed session keys on teardown (Damian Mooyman)

src/Control/Director.php

@@ -473,6 +473,27 @@ class Director implements TemplateGlobalProvider
         return $host;
     }
 
+    /**
+     * Validate user and password in URL, disallowing slashes
+     *
+     * @param string $url
+     * @return bool
+     */
+    protected static function validateUserAndPass($url)
+    {
+        $parsedURL = parse_url($url);
+
+        // Validate user (disallow slashes)
+        if (!empty($parsedURL['user']) && strstr($parsedURL['user'], '\\')) {
+            return false;
+        }
+        if (!empty($parsedURL['pass']) && strstr($parsedURL['pass'], '\\')) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * A helper to determine the current hostname used to access the site.
      * The following are used to determine the host (in order)
@@ -811,6 +832,11 @@ class Director implements TemplateGlobalProvider
      */
     public static function is_site_url($url)
     {
+        // Validate user and password
+        if (!static::validateUserAndPass($url)) {
+            return false;
+        }
+
         // Validate host[:port]
         $urlHost = static::parseHost($url);
         if ($urlHost && $urlHost === static::host()) {

src/Core/Startup/ParameterConfirmationToken.php

@@ -214,6 +214,7 @@ class ParameterConfirmationToken
      */
     public function suppress()
     {
+        unset($_GET[$this->parameterName]);
         $this->request->offsetUnset($this->parameterName);
     }
 

src/Dev/FixtureBlueprint.php

@@ -121,7 +121,7 @@ class FixtureBlueprint
                         continue;
                     }
 
-                    if (is_callable($fieldVal)) {
+                    if (!is_string($fieldVal) && is_callable($fieldVal)) {
                         $obj->$fieldName = $fieldVal($obj, $data, $fixtures);
                     } else {
                         $obj->$fieldName = $fieldVal;

src/Forms/FormRequestHandler.php

@@ -157,7 +157,6 @@ class FormRequestHandler extends RequestHandler
                         "SilverStripe\\Forms\\Form.CSRF_EXPIRED_MESSAGE",
                         "Your session has expired. Please re-submit the form."
                     ));
-
                 // Return the user
                 return $this->redirectBack();
             }

src/Forms/GridField/GridFieldDataColumns.php

@@ -281,7 +281,7 @@ class GridFieldDataColumns implements GridField_ColumnProvider
         }
 
         $spec = $this->fieldFormatting[$fieldName];
-        if (is_callable($spec)) {
+        if (!is_string($spec) && is_callable($spec)) {
             return $spec($value, $item);
         } else {
             $format = str_replace('$value', "__VAL__", $spec);

src/Forms/PasswordField.php

@@ -19,6 +19,12 @@ class PasswordField extends TextField
 
     protected $inputType = 'password';
 
+    /**
+     * If true, the field can accept a value attribute, e.g. from posted form data
+     * @var bool
+     */
+    protected $allowValuePostback = false;
+
     /**
      * Returns an input field.
      *
@@ -39,12 +45,35 @@ class PasswordField extends TextField
         parent::__construct($name, $title, $value);
     }
 
+    /**
+     * @param bool $bool
+     * @return $this
+     */
+    public function setAllowValuePostback($bool)
+    {
+        $this->allowValuePostback = (bool) $bool;
+
+        return $this;
+    }
+
+    /**
+     * @return bool
+     */
+    public function getAllowValuePostback()
+    {
+        return $this->allowValuePostback;
+    }
+
     /**
      * {@inheritdoc}
      */
     public function getAttributes()
     {
-        $attributes = array();
+        $attributes = [];
+
+        if (!$this->getAllowValuePostback()) {
+            $attributes['value'] = null;
+        }
 
         $autocomplete = $this->config()->get('autocomplete');
 

src/ORM/Hierarchy/MarkedSet.php

@@ -333,7 +333,7 @@ class MarkedSet
         $parentNode->setField('markingClasses', $this->markingClasses($data['node']));
 
         // Evaluate custom context
-        if (is_callable($context)) {
+        if (!is_string($context) && is_callable($context)) {
             $context = call_user_func($context, $data['node']);
         }
         if ($context) {

src/Security/Member.php

@@ -33,6 +33,7 @@ use SilverStripe\ORM\HasManyList;
 use SilverStripe\ORM\ManyManyList;
 use SilverStripe\ORM\Map;
 use SilverStripe\ORM\SS_List;
+use SilverStripe\ORM\UnsavedRelationList;
 use SilverStripe\ORM\ValidationException;
 use SilverStripe\ORM\ValidationResult;
 
@@ -60,7 +61,6 @@ use SilverStripe\ORM\ValidationResult;
  */
 class Member extends DataObject
 {
-
     private static $db = array(
         'FirstName' => 'Varchar',
         'Surname' => 'Varchar',
@@ -521,11 +521,10 @@ class Member extends DataObject
             'This method is deprecated and now does not add value. Please use Security::getCurrentUser()'
         );
 
-        if ($member = Security::getCurrentUser()) {
+        $member = Security::getCurrentUser();
         if ($member && $member->exists()) {
             return true;
         }
-        }
 
         return false;
     }
@@ -799,6 +798,7 @@ class Member extends DataObject
      * @param Member|null|int $member Member or member ID to log in as.
      * Set to null or 0 to act as a logged out user.
      * @param callable $callback
+     * @return mixed Result of $callback
      */
     public static function actAs($member, $callback)
     {
@@ -831,11 +831,11 @@ class Member extends DataObject
             'This method is deprecated. Please use Security::getCurrentUser() or an IdentityStore'
         );
 
-        if ($member = Security::getCurrentUser()) {
+        $member = Security::getCurrentUser();
+        if ($member) {
             return $member->ID;
-        } else {
-            return 0;
         }
+        return 0;
     }
 
     /**
@@ -912,7 +912,11 @@ class Member extends DataObject
                 ->setHTMLTemplate('SilverStripe\\Control\\Email\\ChangePasswordEmail')
                 ->setData($this)
                 ->setTo($this->Email)
-                ->setSubject(_t(__CLASS__ . '.SUBJECTPASSWORDCHANGED', "Your password has been changed", 'Email subject'))
+                ->setSubject(_t(
+                    __CLASS__ . '.SUBJECTPASSWORDCHANGED',
+                    "Your password has been changed",
+                    'Email subject'
+                ))
                 ->send();
         }
 
@@ -975,16 +979,25 @@ class Member extends DataObject
      */
     public function onChangeGroups($ids)
     {
-        // unless the current user is an admin already OR the logged in user is an admin
+        // Ensure none of these match disallowed list
-        if (Permission::check('ADMIN') || Permission::checkMember($this, 'ADMIN')) {
+        $disallowedGroupIDs = $this->disallowedGroups();
-            return true;
+        return count(array_intersect($ids, $disallowedGroupIDs)) == 0;
     }
 
-        // If there are no admin groups in this set then it's ok
+    /**
-        $adminGroups = Permission::get_groups_by_permission('ADMIN');
+     * List of group IDs this user is disallowed from
-        $adminGroupIDs = ($adminGroups) ? $adminGroups->column('ID') : array();
+     *
+     * @return int[] List of group IDs
+     */
+    protected function disallowedGroups()
+    {
+        // unless the current user is an admin already OR the logged in user is an admin
+        if (Permission::check('ADMIN') || Permission::checkMember($this, 'ADMIN')) {
+            return [];
+        }
 
-        return count(array_intersect($ids, $adminGroupIDs)) == 0;
+        // Non-admins may not belong to admin groups
+        return Permission::get_groups_by_permission('ADMIN')->column('ID');
     }
 
 
@@ -1288,7 +1301,7 @@ class Member extends DataObject
     }
 
     /**
-     * @return ManyManyList
+     * @return ManyManyList|UnsavedRelationList
      */
     public function DirectGroups()
     {
@@ -1469,8 +1482,14 @@ class Member extends DataObject
             $fields->removeByName('RememberLoginHashes');
 
             if (Permission::check('EDIT_PERMISSIONS')) {
+                // Filter allowed groups
+                $groups = Group::get();
+                $disallowedGroupIDs = $this->disallowedGroups();
+                if ($disallowedGroupIDs) {
+                    $groups = $groups->exclude('ID', $disallowedGroupIDs);
+                }
                 $groupsMap = array();
-                foreach (Group::get() as $group) {
+                foreach ($groups as $group) {
                     // Listboxfield values are escaped, use ASCII char instead of »
                     $groupsMap[$group->ID] = $group->getBreadcrumbs(' > ');
                 }
@@ -1525,7 +1544,11 @@ class Member extends DataObject
         /** @skipUpgrade */
         $labels['Email'] = _t(__CLASS__ . '.EMAIL', 'Email');
         $labels['Password'] = _t(__CLASS__ . '.db_Password', 'Password');
-        $labels['PasswordExpiry'] = _t(__CLASS__ . '.db_PasswordExpiry', 'Password Expiry Date', 'Password expiry date');
+        $labels['PasswordExpiry'] = _t(
+            __CLASS__ . '.db_PasswordExpiry',
+            'Password Expiry Date',
+            'Password expiry date'
+        );
         $labels['LockedOutUntil'] = _t(__CLASS__ . '.db_LockedOutUntil', 'Locked out until', 'Security related date');
         $labels['Locale'] = _t(__CLASS__ . '.db_Locale', 'Interface Locale');
         if ($includerelations) {

src/Security/MemberAuthenticator/MemberAuthenticator.php

@@ -91,6 +91,11 @@ class MemberAuthenticator implements Authenticator
         // Validate against member if possible
         if ($member && !$asDefaultAdmin) {
             $this->checkPassword($member, $data['Password'], $result);
+        } elseif (!$asDefaultAdmin) {
+            // spoof a login attempt
+            $tempMember = Member::create();
+            $tempMember->{Member::config()->get('unique_identifier_field')} = $email;
+            $tempMember->validateCanLogin($result);
         }
 
         // Emit failure to member and form (if available)
@@ -164,7 +169,9 @@ class MemberAuthenticator implements Authenticator
      */
     protected function recordLoginAttempt($data, HTTPRequest $request, $member, $success)
     {
-        if (!Security::config()->get('login_recording')) {
+        if (!Security::config()->get('login_recording')
+            && !Member::config()->get('lock_out_after_incorrect_logins')
+        ) {
             return;
         }
 

src/View/SSViewer_DataPresenter.php

@@ -326,7 +326,7 @@ class SSViewer_DataPresenter extends SSViewer_Scope
         $override = $overrides[$property];
 
         // Late-evaluate this value
-        if (is_callable($override)) {
+        if (!is_string($override) && is_callable($override)) {
             $override = $override();
 
             // Late override may yet return null

tests/php/Control/DirectorTest.php

@@ -393,6 +393,10 @@ class DirectorTest extends SapphireTest
         $this->assertFalse(Director::is_site_url("http://test.com?url=" . Director::absoluteBaseURL()));
         $this->assertFalse(Director::is_site_url("http://test.com?url=" . urlencode(Director::absoluteBaseURL())));
         $this->assertFalse(Director::is_site_url("//test.com?url=" . Director::absoluteBaseURL()));
+        $this->assertFalse(Director::is_site_url('http://google.com\@test.com'));
+        $this->assertFalse(Director::is_site_url('http://google.com/@test.com'));
+        $this->assertFalse(Director::is_site_url('http://google.com:pass\@test.com'));
+        $this->assertFalse(Director::is_site_url('http://google.com:pass/@test.com'));
     }
 
     /**

tests/php/Core/Startup/ParameterConfirmationTokenTest.php

@@ -20,17 +20,17 @@ class ParameterConfirmationTokenTest extends SapphireTest
     protected function setUp()
     {
         parent::setUp();
-        $get = [];
+        $_GET = [];
-        $get['parameterconfirmationtokentest_notoken'] = 'value';
+        $_GET['parameterconfirmationtokentest_notoken'] = 'value';
-        $get['parameterconfirmationtokentest_empty'] = '';
+        $_GET['parameterconfirmationtokentest_empty'] = '';
-        $get['parameterconfirmationtokentest_withtoken'] = '1';
+        $_GET['parameterconfirmationtokentest_withtoken'] = '1';
-        $get['parameterconfirmationtokentest_withtokentoken'] = 'dummy';
+        $_GET['parameterconfirmationtokentest_withtokentoken'] = 'dummy';
-        $get['parameterconfirmationtokentest_nulltoken'] = '1';
+        $_GET['parameterconfirmationtokentest_nulltoken'] = '1';
-        $get['parameterconfirmationtokentest_nulltokentoken'] = null;
+        $_GET['parameterconfirmationtokentest_nulltokentoken'] = null;
-        $get['parameterconfirmationtokentest_emptytoken'] = '1';
+        $_GET['parameterconfirmationtokentest_emptytoken'] = '1';
-        $get['parameterconfirmationtokentest_emptytokentoken'] = '';
+        $_GET['parameterconfirmationtokentest_emptytokentoken'] = '';
-        $get['BackURL'] = 'page?parameterconfirmationtokentest_backtoken=1';
+        $_GET['BackURL'] = 'page?parameterconfirmationtokentest_backtoken=1';
-        $this->request = new HTTPRequest('GET', 'anotherpage', $get);
+        $this->request = new HTTPRequest('GET', 'anotherpage', $_GET);
         $this->request->setSession(new Session([]));
     }
 
@@ -129,6 +129,11 @@ class ParameterConfirmationTokenTest extends SapphireTest
             $this->request
         );
         $this->assertEquals('parameterconfirmationtokentest_backtoken', $token->getName());
+
+        // Test prepare_tokens() unsets $_GET vars
+        $this->assertArrayNotHasKey('parameterconfirmationtokentest_notoken', $_GET);
+        $this->assertArrayNotHasKey('parameterconfirmationtokentest_empty', $_GET);
+        $this->assertArrayNotHasKey('parameterconfirmationtokentest_noparam', $_GET);
     }
 
     public function dataProviderURLs()

tests/php/Forms/FormRequestHandlerTest.php

@@ -9,8 +9,10 @@ use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\FormAction;
 use SilverStripe\Forms\FormRequestHandler;
+use SilverStripe\Forms\PasswordField;
 use SilverStripe\Forms\Tests\FormRequestHandlerTest\TestForm;
 use SilverStripe\Forms\Tests\FormRequestHandlerTest\TestFormRequestHandler;
+use SilverStripe\Forms\TextField;
 
 /**
  * @skipUpgrade

tests/php/Forms/FormTest.php

@@ -3,6 +3,8 @@
 namespace SilverStripe\Forms\Tests;
 
 use SilverStripe\Control\Session;
+use SilverStripe\Core\Config\Config;
+use SilverStripe\Forms\PasswordField;
 use SilverStripe\Forms\Tests\FormTest\TestController;
 use SilverStripe\Forms\Tests\FormTest\ControllerWithSecurityToken;
 use SilverStripe\Forms\Tests\FormTest\ControllerWithStrictPostCheck;
@@ -10,6 +12,7 @@ use SilverStripe\Forms\Tests\FormTest\Player;
 use SilverStripe\Forms\Tests\FormTest\Team;
 use SilverStripe\ORM\ValidationResult;
 use SilverStripe\Security\NullSecurityToken;
+use SilverStripe\Security\Security;
 use SilverStripe\Security\SecurityToken;
 use SilverStripe\Security\RandomGenerator;
 use SilverStripe\Dev\CSSContentParser;
@@ -59,6 +62,17 @@ class FormTest extends FunctionalTest
         );
     }
 
+    /**
+     * @return array
+     */
+    public function boolDataProvider()
+    {
+        return [
+            [false],
+            [true],
+        ];
+    }
+
     public function testLoadDataFromRequest()
     {
         $form = new Form(
@@ -915,6 +929,46 @@ class FormTest extends FunctionalTest
         $this->assertEmpty($formData['ExtraFieldCheckbox']);
     }
 
+    /**
+     * @dataProvider boolDataProvider
+     * @param bool $allow
+     */
+    public function testPasswordPostback($allow)
+    {
+        $form = $this->getStubForm();
+        $form->enableSecurityToken();
+        $form->Fields()->push(
+            PasswordField::create('Password')
+                ->setAllowValuePostback($allow)
+        );
+        $form->Actions()->push(FormAction::create('doSubmit'));
+        $request = new HTTPRequest(
+            'POST',
+            'FormTest_Controller/Form',
+            [],
+            [
+                'key1' => 'foo',
+                'Password' => 'hidden',
+                SecurityToken::inst()->getName() => 'fail',
+                'action_doSubmit' => 1,
+            ]
+        );
+        $form->getRequestHandler()->httpSubmission($request);
+        $parser = new CSSContentParser($form->forTemplate());
+        $passwords = $parser->getBySelector('input#Password');
+        $this->assertNotNull($passwords);
+        $this->assertCount(1, $passwords);
+        /* @var \SimpleXMLElement $password */
+        $password = $passwords[0];
+        $attrs = iterator_to_array($password->attributes());
+        if ($allow) {
+            $this->assertArrayHasKey('value', $attrs);
+            $this->assertEquals('hidden', $attrs['value']);
+        } else {
+            $this->assertArrayNotHasKey('value', $attrs);
+        }
+    }
+
     protected function getStubForm()
     {
         return new Form(

tests/php/Forms/PasswordFieldTest.php

@@ -0,0 +1,46 @@
+<?php
+
+namespace SilverStripe\Forms\Tests;
+
+use SilverStripe\Core\Config\Config;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Forms\PasswordField;
+
+class PasswordFieldTest extends SapphireTest
+{
+    public function boolDataProvider()
+    {
+        return [
+            [false],
+            [true]
+        ];
+    }
+
+    /**
+     * @dataProvider boolDataProvider
+     * @param bool $bool
+     */
+    public function testAutocomplete($bool)
+    {
+        Config::modify()->set(PasswordField::class, 'autocomplete', $bool);
+        $field = new PasswordField('test');
+        $attrs = $field->getAttributes();
+
+        $this->assertArrayHasKey('autocomplete', $attrs);
+        $this->assertEquals($bool ? 'on' : 'off', $attrs['autocomplete']);
+    }
+
+    /**
+     * @dataProvider boolDataProvider
+     * @param bool $bool
+     */
+    public function testValuePostback($bool)
+    {
+        $field = (new PasswordField('test', 'test', 'password'))
+            ->setAllowValuePostback($bool);
+        $attrs = $field->getAttributes();
+
+        $this->assertArrayHasKey('value', $attrs);
+        $this->assertEquals($bool ? 'password' : '', $attrs['value']);
+    }
+}

tests/php/Security/MemberAuthenticatorTest.php

@@ -243,7 +243,6 @@ class MemberAuthenticatorTest extends SapphireTest
 
     public function testNonExistantMemberGetsLoginAttemptRecorded()
     {
-        Security::config()->set('login_recording', true);
         Member::config()
             ->set('lock_out_after_incorrect_logins', 1)
             ->set('lock_out_delay_mins', 10);
@@ -272,7 +271,6 @@ class MemberAuthenticatorTest extends SapphireTest
 
     public function testNonExistantMemberGetsLockedOut()
     {
-        Security::config()->set('login_recording', true);
         Member::config()
             ->set('lock_out_after_incorrect_logins', 1)
             ->set('lock_out_delay_mins', 10);

tests/php/Security/MemberTest.php

@@ -7,6 +7,7 @@ use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Dev\FunctionalTest;
+use SilverStripe\Forms\ListboxField;
 use SilverStripe\i18n\i18n;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DB;
@@ -686,6 +687,8 @@ class MemberTest extends FunctionalTest
         $staffMember = $this->objFromFixture(Member::class, 'staffmember');
         /** @var Member $adminMember */
         $adminMember = $this->objFromFixture(Member::class, 'admin');
+
+        // Construct admin and non-admin gruops
         $newAdminGroup = new Group(array('Title' => 'newadmin'));
         $newAdminGroup->write();
         Permission::grant($newAdminGroup->ID, 'ADMIN');
@@ -718,6 +721,37 @@ class MemberTest extends FunctionalTest
         );
     }
 
+    /**
+     * Ensure DirectGroups listbox disallows admin-promotion
+     */
+    public function testAllowedGroupsListbox()
+    {
+        /** @var Group $adminGroup */
+        $adminGroup = $this->objFromFixture(Group::class, 'admingroup');
+        /** @var Member $staffMember */
+        $staffMember = $this->objFromFixture(Member::class, 'staffmember');
+        /** @var Member $adminMember */
+        $adminMember = $this->objFromFixture(Member::class, 'admin');
+
+        // Ensure you can see the DirectGroups box
+        $this->logInWithPermission('EDIT_PERMISSIONS');
+
+        // Non-admin member field contains non-admin groups
+        /** @var ListboxField $staffListbox */
+        $staffListbox = $staffMember->getCMSFields()->dataFieldByName('DirectGroups');
+        $this->assertArrayNotHasKey($adminGroup->ID, $staffListbox->getSource());
+
+        // admin member field contains admin group
+        /** @var ListboxField $adminListbox */
+        $adminListbox = $adminMember->getCMSFields()->dataFieldByName('DirectGroups');
+        $this->assertArrayHasKey($adminGroup->ID, $adminListbox->getSource());
+
+        // If logged in as admin, staff listbox has admin group
+        $this->logInWithPermission('ADMIN');
+        $staffListbox = $staffMember->getCMSFields()->dataFieldByName('DirectGroups');
+        $this->assertArrayHasKey($adminGroup->ID, $staffListbox->getSource());
+    }
+
     /**
      * Test Member_GroupSet::add
      */
@@ -1486,6 +1520,7 @@ class MemberTest extends FunctionalTest
     public function testChangePasswordWithExtensionsThatModifyValidationResult()
     {
         // Default behaviour
+        /** @var Member $member */
         $member = $this->objFromFixture(Member::class, 'admin');
         $result = $member->changePassword('my-secret-new-password');
         $this->assertInstanceOf(ValidationResult::class, $result);

tests/php/View/SSViewerTest.php

@@ -109,6 +109,16 @@ class SSViewerTest extends SapphireTest
         $this->assertEquals('Test partial template: var value', trim(preg_replace("/<!--.*-->/U", '', $result)));
     }
 
+    /**
+     * Ensure global methods aren't executed
+     */
+    public function testTemplateExecution()
+    {
+        $data = new ArrayData([ 'Var' => 'phpinfo' ]);
+        $result = $data->renderWith("SSViewerTestPartialTemplate");
+        $this->assertEquals('Test partial template: phpinfo', trim(preg_replace("/<!--.*-->/U", '', $result)));
+    }
+
     public function testIncludeScopeInheritance()
     {
         $data = $this->getScopeInheritanceTestData();