diff --git a/docs/en/02_Developer_Guides/14_Files/03_File_Security.md b/docs/en/02_Developer_Guides/14_Files/03_File_Security.md
index 69f9de238..0e5201866 100644
--- a/docs/en/02_Developer_Guides/14_Files/03_File_Security.md
+++ b/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:
+
+```
+
+ php_admin_flag engine off
+
+```
+
+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.
diff --git a/docs/en/04_Changelogs/4.0.4.md b/docs/en/04_Changelogs/4.0.4.md
new file mode 100644
index 000000000..2176d85df
--- /dev/null
+++ b/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).
+
+
+
+## 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)
diff --git a/docs/en/04_Changelogs/4.1.1.md b/docs/en/04_Changelogs/4.1.1.md
new file mode 100644
index 000000000..8cec9d216
--- /dev/null
+++ b/docs/en/04_Changelogs/4.1.1.md
@@ -0,0 +1,35 @@
+# 4.1.1
+
+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).
+
+
+
+## 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)
+
+### Features and Enhancements
+
+ * 2017-12-21 [4d60f01](https://github.com/silverstripe/silverstripe-installer/commit/4d60f01d2dd17febcf15c08ecdc07af7380694d0) add test for a `--no-dev` build (Christopher Joe)
+
+### 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-06 [5bff64b47](https://github.com/silverstripe/silverstripe-framework/commit/5bff64b47) Fix Director::test() not persisting removed session keys on teardown (Damian Mooyman)
diff --git a/src/Control/Director.php b/src/Control/Director.php
index 941c026ef..77a2312a8 100644
--- a/src/Control/Director.php
+++ b/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()) {
diff --git a/src/Core/Startup/ParameterConfirmationToken.php b/src/Core/Startup/ParameterConfirmationToken.php
index a5069102c..1c80db1d0 100644
--- a/src/Core/Startup/ParameterConfirmationToken.php
+++ b/src/Core/Startup/ParameterConfirmationToken.php
@@ -214,6 +214,7 @@ class ParameterConfirmationToken
*/
public function suppress()
{
+ unset($_GET[$this->parameterName]);
$this->request->offsetUnset($this->parameterName);
}
diff --git a/src/Dev/FixtureBlueprint.php b/src/Dev/FixtureBlueprint.php
index 9d219a36f..47e6a1805 100644
--- a/src/Dev/FixtureBlueprint.php
+++ b/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;
diff --git a/src/Dev/Install/config-form.html b/src/Dev/Install/config-form.html
index 29667f72f..41270450d 100644
--- a/src/Dev/Install/config-form.html
+++ b/src/Dev/Install/config-form.html
@@ -5,7 +5,7 @@
SilverStripe CMS / Framework Installation
-
+
diff --git a/src/Dev/Install/install5.php b/src/Dev/Install/install5.php
index b50ff0116..5b5bb2692 100755
--- a/src/Dev/Install/install5.php
+++ b/src/Dev/Install/install5.php
@@ -105,7 +105,7 @@ if ($installFromCli && ($req->hasErrors() || $dbReq->hasErrors())) {
}
// Path to client resources (copied through silverstripe/vendor-plugin)
-$base = BASE_URL;
+$base = rtrim(BASE_URL, '/') . '/';
$clientPath = PUBLIC_DIR
? 'resources/vendor/silverstripe/framework/src/Dev/Install/client'
: 'resources/silverstripe/framework/src/Dev/Install/client';
diff --git a/src/Forms/FormRequestHandler.php b/src/Forms/FormRequestHandler.php
index 408f57d77..4392ab3bc 100644
--- a/src/Forms/FormRequestHandler.php
+++ b/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();
}
diff --git a/src/Forms/GridField/GridFieldDataColumns.php b/src/Forms/GridField/GridFieldDataColumns.php
index 623515323..c8dd223da 100644
--- a/src/Forms/GridField/GridFieldDataColumns.php
+++ b/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);
diff --git a/src/Forms/PasswordField.php b/src/Forms/PasswordField.php
index cedfe0374..de357d403 100644
--- a/src/Forms/PasswordField.php
+++ b/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');
diff --git a/src/ORM/Hierarchy/MarkedSet.php b/src/ORM/Hierarchy/MarkedSet.php
index 96fd23602..a2adda78d 100644
--- a/src/ORM/Hierarchy/MarkedSet.php
+++ b/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) {
diff --git a/src/Security/Member.php b/src/Security/Member.php
index 277d6d31a..29cc3bc44 100644
--- a/src/Security/Member.php
+++ b/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,27 +61,26 @@ use SilverStripe\ORM\ValidationResult;
*/
class Member extends DataObject
{
-
private static $db = array(
- 'FirstName' => 'Varchar',
- 'Surname' => 'Varchar',
- 'Email' => 'Varchar(254)', // See RFC 5321, Section 4.5.3.1.3. (256 minus the < and > character)
- 'TempIDHash' => 'Varchar(160)', // Temporary id used for cms re-authentication
- 'TempIDExpired' => 'Datetime', // Expiry of temp login
- 'Password' => 'Varchar(160)',
- 'AutoLoginHash' => 'Varchar(160)', // Used to auto-login the user on password reset
- 'AutoLoginExpired' => 'Datetime',
+ 'FirstName' => 'Varchar',
+ 'Surname' => 'Varchar',
+ 'Email' => 'Varchar(254)', // See RFC 5321, Section 4.5.3.1.3. (256 minus the < and > character)
+ 'TempIDHash' => 'Varchar(160)', // Temporary id used for cms re-authentication
+ 'TempIDExpired' => 'Datetime', // Expiry of temp login
+ 'Password' => 'Varchar(160)',
+ 'AutoLoginHash' => 'Varchar(160)', // Used to auto-login the user on password reset
+ 'AutoLoginExpired' => 'Datetime',
// This is an arbitrary code pointing to a PasswordEncryptor instance,
// not an actual encryption algorithm.
// Warning: Never change this field after its the first password hashing without
// providing a new cleartext password as well.
'PasswordEncryption' => "Varchar(50)",
- 'Salt' => 'Varchar(50)',
- 'PasswordExpiry' => 'Date',
- 'LockedOutUntil' => 'Datetime',
- 'Locale' => 'Varchar(6)',
+ 'Salt' => 'Varchar(50)',
+ 'PasswordExpiry' => 'Date',
+ 'LockedOutUntil' => 'Datetime',
+ 'Locale' => 'Varchar(6)',
// handled in registerFailedLogin(), only used if $lock_out_after_incorrect_logins is set
- 'FailedLoginCount' => 'Int',
+ 'FailedLoginCount' => 'Int',
);
private static $belongs_many_many = array(
@@ -88,7 +88,7 @@ class Member extends DataObject
);
private static $has_many = array(
- 'LoggedPasswords' => MemberPassword::class,
+ 'LoggedPasswords' => MemberPassword::class,
'RememberLoginHashes' => RememberLoginHash::class,
);
@@ -312,7 +312,7 @@ class Member extends DataObject
break;
}
}
- return $result;
+ return $result;
}
/**
@@ -521,10 +521,9 @@ class Member extends DataObject
'This method is deprecated and now does not add value. Please use Security::getCurrentUser()'
);
- if ($member = Security::getCurrentUser()) {
- if ($member && $member->exists()) {
- return true;
- }
+ $member = Security::getCurrentUser();
+ if ($member && $member->exists()) {
+ return true;
}
return false;
@@ -655,7 +654,7 @@ class Member extends DataObject
{
/** @var Member $member */
$member = static::get()->filter([
- 'AutoLoginHash' => $hash,
+ 'AutoLoginHash' => $hash,
'AutoLoginExpired:GreaterThan' => DBDatetime::now()->getValue(),
])->first();
@@ -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;
}
/**
@@ -892,8 +892,8 @@ class Member extends DataObject
'Can\'t overwrite existing member #{id} with identical identifier ({name} = {value}))',
'Values in brackets show "fieldname = value", usually denoting an existing email address',
array(
- 'id' => $existingRecord->ID,
- 'name' => $identifierField,
+ 'id' => $existingRecord->ID,
+ 'name' => $identifierField,
'value' => $this->$identifierField
)
));
@@ -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();
}
@@ -974,17 +978,26 @@ class Member extends DataObject
* @return bool True if the change can be accepted
*/
public function onChangeGroups($ids)
+ {
+ // Ensure none of these match disallowed list
+ $disallowedGroupIDs = $this->disallowedGroups();
+ return count(array_intersect($ids, $disallowedGroupIDs)) == 0;
+ }
+
+ /**
+ * List of group IDs this user is disallowed from
+ *
+ * @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 true;
+ return [];
}
- // If there are no admin groups in this set then it's ok
- $adminGroups = Permission::get_groups_by_permission('ADMIN');
- $adminGroupIDs = ($adminGroups) ? $adminGroups->column('ID') : array();
-
- return count(array_intersect($ids, $adminGroupIDs)) == 0;
+ // Non-admins may not belong to admin groups
+ return Permission::get_groups_by_permission('ADMIN')->column('ID');
}
@@ -1160,7 +1173,7 @@ class Member extends DataObject
if (!$format) {
$format = [
'columns' => ['Surname', 'FirstName'],
- 'sep' => ' ',
+ 'sep' => ' ',
];
}
@@ -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) {
@@ -1680,8 +1703,8 @@ class Member extends DataObject
*
* This method will encrypt the password prior to writing.
*
- * @param string $password Cleartext password
- * @param bool $write Whether to write the member afterwards
+ * @param string $password Cleartext password
+ * @param bool $write Whether to write the member afterwards
* @return ValidationResult
*/
public function changePassword($password, $write = true)
diff --git a/src/Security/MemberAuthenticator/MemberAuthenticator.php b/src/Security/MemberAuthenticator/MemberAuthenticator.php
index 91dbd50a9..c51cecfd3 100644
--- a/src/Security/MemberAuthenticator/MemberAuthenticator.php
+++ b/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;
}
diff --git a/src/Security/PasswordValidator.php b/src/Security/PasswordValidator.php
index e07ef6fbb..2358aba1b 100644
--- a/src/Security/PasswordValidator.php
+++ b/src/Security/PasswordValidator.php
@@ -39,39 +39,39 @@ class PasswordValidator
/**
* @config
- * @var integer
+ * @var int
*/
private static $min_length = null;
/**
* @config
- * @var integer
+ * @var int
*/
private static $min_test_score = null;
/**
* @config
- * @var integer
+ * @var int
*/
private static $historic_count = null;
/**
- * @var integer
+ * @var int
*/
protected $minLength = null;
/**
- * @var integer
+ * @var int
*/
protected $minScore = null;
/**
- * @var array
+ * @var string[]
*/
protected $testNames = null;
/**
- * @var integer
+ * @var int
*/
protected $historicalPasswordCount = null;
@@ -95,18 +95,20 @@ class PasswordValidator
* Eg: $this->characterStrength(3, array("lowercase", "uppercase", "digits", "punctuation"))
*
* @param int $minScore The minimum number of character tests that must pass
- * @param array $testNames The names of the tests to perform
+ * @param string[] $testNames The names of the tests to perform
* @return $this
*/
public function characterStrength($minScore, $testNames = null)
{
-
Deprecation::notice(
'5.0',
'Use ->setMinTestScore($score) and ->setTextNames($names) instead.'
);
- return $this->setMinTestScore($minScore)
- ->setTestNames($testNames);
+ $this->setMinTestScore($minScore);
+ if ($testNames) {
+ $this->setTestNames($testNames);
+ }
+ return $this;
}
/**
@@ -123,7 +125,7 @@ class PasswordValidator
}
/**
- * @return integer
+ * @return int
*/
public function getMinLength()
{
@@ -134,7 +136,7 @@ class PasswordValidator
}
/**
- * @param $minLength
+ * @param int $minLength
* @return $this
*/
public function setMinLength($minLength)
@@ -155,7 +157,7 @@ class PasswordValidator
}
/**
- * @param $minScore
+ * @param int $minScore
* @return $this
*/
public function setMinTestScore($minScore)
@@ -165,7 +167,9 @@ class PasswordValidator
}
/**
- * @return array
+ * Gets the list of tests to use for this validator
+ *
+ * @return string[]
*/
public function getTestNames()
{
@@ -176,7 +180,9 @@ class PasswordValidator
}
/**
- * @param $testNames
+ * Set list of tests to use for this validator
+ *
+ * @param string[] $testNames
* @return $this
*/
public function setTestNames($testNames)
@@ -186,7 +192,7 @@ class PasswordValidator
}
/**
- * @return integer
+ * @return int
*/
public function getHistoricCount()
{
@@ -197,7 +203,7 @@ class PasswordValidator
}
/**
- * @param $count
+ * @param int $count
* @return $this
*/
public function setHistoricCount($count)
@@ -207,6 +213,8 @@ class PasswordValidator
}
/**
+ * Gets all possible tests
+ *
* @return array
*/
public function getTests()
@@ -226,7 +234,7 @@ class PasswordValidator
$minLength = $this->getMinLength();
if ($minLength && strlen($password) < $minLength) {
$error = _t(
- 'SilverStripe\\Security\\PasswordValidator.TOOSHORT',
+ __CLASS__ . '.TOOSHORT',
'Password is too short, it must be {minimum} or more characters long',
['minimum' => $this->minLength]
);
@@ -245,16 +253,16 @@ class PasswordValidator
continue;
}
$missedTests[] = _t(
- 'SilverStripe\\Security\\PasswordValidator.STRENGTHTEST' . strtoupper($name),
+ __CLASS__ . '.STRENGTHTEST' . strtoupper($name),
$name,
'The user needs to add this to their password for more complexity'
);
}
- $score = count($this->testNames) - count($missedTests);
- if ($score < $minTestScore) {
+ $score = count($testNames) - count($missedTests);
+ if ($missedTests && $score < $minTestScore) {
$error = _t(
- 'SilverStripe\\Security\\PasswordValidator.LOWCHARSTRENGTH',
+ __CLASS__ . '.LOWCHARSTRENGTH',
'Please increase password strength by adding some of the following characters: {chars}',
['chars' => implode(', ', $missedTests)]
);
@@ -271,8 +279,8 @@ class PasswordValidator
/** @var MemberPassword $previousPassword */
foreach ($previousPasswords as $previousPassword) {
if ($previousPassword->checkPassword($password)) {
- $error = _t(
- 'SilverStripe\\Security\\PasswordValidator.PREVPASSWORD',
+ $error = _t(
+ __CLASS__ . '.PREVPASSWORD',
'You\'ve already used that password in the past, please choose a new password'
);
$valid->addError($error, 'bad', 'PREVIOUS_PASSWORD');
diff --git a/src/View/SSViewer_DataPresenter.php b/src/View/SSViewer_DataPresenter.php
index cf609d117..f8d120cd8 100644
--- a/src/View/SSViewer_DataPresenter.php
+++ b/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
diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php
index e2a4ff1e0..0070f2a50 100644
--- a/tests/php/Control/DirectorTest.php
+++ b/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'));
}
/**
diff --git a/tests/php/Core/Startup/ParameterConfirmationTokenTest.php b/tests/php/Core/Startup/ParameterConfirmationTokenTest.php
index b3ae253d0..66616433f 100644
--- a/tests/php/Core/Startup/ParameterConfirmationTokenTest.php
+++ b/tests/php/Core/Startup/ParameterConfirmationTokenTest.php
@@ -20,17 +20,17 @@ class ParameterConfirmationTokenTest extends SapphireTest
protected function setUp()
{
parent::setUp();
- $get = [];
- $get['parameterconfirmationtokentest_notoken'] = 'value';
- $get['parameterconfirmationtokentest_empty'] = '';
- $get['parameterconfirmationtokentest_withtoken'] = '1';
- $get['parameterconfirmationtokentest_withtokentoken'] = 'dummy';
- $get['parameterconfirmationtokentest_nulltoken'] = '1';
- $get['parameterconfirmationtokentest_nulltokentoken'] = null;
- $get['parameterconfirmationtokentest_emptytoken'] = '1';
- $get['parameterconfirmationtokentest_emptytokentoken'] = '';
- $get['BackURL'] = 'page?parameterconfirmationtokentest_backtoken=1';
- $this->request = new HTTPRequest('GET', 'anotherpage', $get);
+ $_GET = [];
+ $_GET['parameterconfirmationtokentest_notoken'] = 'value';
+ $_GET['parameterconfirmationtokentest_empty'] = '';
+ $_GET['parameterconfirmationtokentest_withtoken'] = '1';
+ $_GET['parameterconfirmationtokentest_withtokentoken'] = 'dummy';
+ $_GET['parameterconfirmationtokentest_nulltoken'] = '1';
+ $_GET['parameterconfirmationtokentest_nulltokentoken'] = null;
+ $_GET['parameterconfirmationtokentest_emptytoken'] = '1';
+ $_GET['parameterconfirmationtokentest_emptytokentoken'] = '';
+ $_GET['BackURL'] = 'page?parameterconfirmationtokentest_backtoken=1';
+ $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()
diff --git a/tests/php/Forms/FormRequestHandlerTest.php b/tests/php/Forms/FormRequestHandlerTest.php
index 09eb8b6a1..5ce4b3120 100644
--- a/tests/php/Forms/FormRequestHandlerTest.php
+++ b/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
diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php
index de6cb7aac..ab7a91367 100644
--- a/tests/php/Forms/FormTest.php
+++ b/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(
diff --git a/tests/php/Forms/PasswordFieldTest.php b/tests/php/Forms/PasswordFieldTest.php
new file mode 100644
index 000000000..4dfb4a38c
--- /dev/null
+++ b/tests/php/Forms/PasswordFieldTest.php
@@ -0,0 +1,46 @@
+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']);
+ }
+}
diff --git a/tests/php/Security/MemberAuthenticatorTest.php b/tests/php/Security/MemberAuthenticatorTest.php
index 3aa4e1d0e..4314cf558 100644
--- a/tests/php/Security/MemberAuthenticatorTest.php
+++ b/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);
diff --git a/tests/php/Security/MemberTest.php b/tests/php/Security/MemberTest.php
index c4188e17c..53bb4883c 100644
--- a/tests/php/Security/MemberTest.php
+++ b/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);
diff --git a/tests/php/Security/MemberTest/TestPasswordValidator.php b/tests/php/Security/MemberTest/TestPasswordValidator.php
index 6e7270a8c..8907b7830 100644
--- a/tests/php/Security/MemberTest/TestPasswordValidator.php
+++ b/tests/php/Security/MemberTest/TestPasswordValidator.php
@@ -8,8 +8,9 @@ class TestPasswordValidator extends PasswordValidator
{
public function __construct()
{
- $this->minLength(7);
- $this->checkHistoricalPasswords(6);
- $this->characterStrength(3, array('lowercase', 'uppercase', 'digits', 'punctuation'));
+ $this->setMinLength(7);
+ $this->setHistoricCount(6);
+ $this->setMinTestScore(3);
+ $this->setTestNames(['lowercase', 'uppercase', 'digits', 'punctuation']);
}
}
diff --git a/tests/php/Security/PasswordValidatorTest.php b/tests/php/Security/PasswordValidatorTest.php
index 35824e036..4fd99ed08 100644
--- a/tests/php/Security/PasswordValidatorTest.php
+++ b/tests/php/Security/PasswordValidatorTest.php
@@ -28,25 +28,42 @@ class PasswordValidatorTest extends SapphireTest
{
$v = new PasswordValidator();
- $v->minLength(4);
+ $v->setMinLength(4);
$r = $v->validate('123', new Member());
$this->assertFalse($r->isValid(), 'Password too short');
- $v->minLength(4);
+ $v->setMinLength(4);
$r = $v->validate('1234', new Member());
$this->assertTrue($r->isValid(), 'Password long enough');
}
public function testValidateMinScore()
{
+ // Set both score and set of tests
$v = new PasswordValidator();
- $v->characterStrength(3, array("lowercase", "uppercase", "digits", "punctuation"));
+ $v->setMinTestScore(3);
+ $v->setTestNames(["lowercase", "uppercase", "digits", "punctuation"]);
$r = $v->validate('aA', new Member());
$this->assertFalse($r->isValid(), 'Passing too few tests');
$r = $v->validate('aA1', new Member());
$this->assertTrue($r->isValid(), 'Passing enough tests');
+
+ // Ensure min score without tests works (uses default tests)
+ $v = new PasswordValidator();
+ $v->setMinTestScore(3);
+
+ $r = $v->validate('aA', new Member());
+ $this->assertFalse($r->isValid(), 'Passing too few tests');
+
+ $r = $v->validate('aA1', new Member());
+ $this->assertTrue($r->isValid(), 'Passing enough tests');
+
+ // Ensure that min score is only triggered if there are any failing tests at all
+ $v->setMinTestScore(1000);
+ $r = $v->validate('aA1!', new Member());
+ $this->assertTrue($r->isValid(), 'Passing enough tests');
}
/**
@@ -55,7 +72,7 @@ class PasswordValidatorTest extends SapphireTest
public function testHistoricalPasswordCount()
{
$validator = new PasswordValidator;
- $validator->checkHistoricalPasswords(3);
+ $validator->setHistoricCount(3);
Member::set_password_validator($validator);
$member = new Member;
diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php
index f1c3a436b..693bc1013 100644
--- a/tests/php/View/SSViewerTest.php
+++ b/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();