From 001d5596621404892de0a5413392379eff990641 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 12 Jun 2017 17:39:38 +1200 Subject: [PATCH] Remove references to SapphireTest::is_running_test() Upgrade various code --- src/Control/Controller.php | 10 +++- src/Control/Director.php | 60 ++++++++----------- src/Control/Email/Email.php | 4 +- src/Dev/DevelopmentAdmin.php | 13 +++- src/Dev/SapphireTest.php | 6 +- src/Dev/TaskRunner.php | 4 +- src/Forms/GridField/GridField_FormAction.php | 1 - src/ORM/DataObject.php | 20 +------ src/ORM/DatabaseAdmin.php | 14 ++--- src/Security/AuthenticationRequestFilter.php | 9 +-- src/Security/Authenticator.php | 4 +- src/Security/BasicAuth.php | 12 +++- .../MemberAuthenticator/LoginHandler.php | 33 ++++++---- .../MemberAuthenticator.php | 52 ++++++++-------- src/View/Requirements_Backend.php | 18 ++---- src/View/ViewableData.php | 2 +- src/conf/ConfigureFromEnv.php | 3 + .../php/Security/MemberAuthenticatorTest.php | 29 +++++---- 18 files changed, 144 insertions(+), 150 deletions(-) diff --git a/src/Control/Controller.php b/src/Control/Controller.php index c0d088e5d..2ac3febff 100644 --- a/src/Control/Controller.php +++ b/src/Control/Controller.php @@ -52,6 +52,14 @@ class Controller extends RequestHandler implements TemplateGlobalProvider */ protected static $controller_stack = array(); + /** + * Assign templates for this controller. + * Map of action => template name + * + * @var array + */ + protected $templates = []; + /** * @var bool */ @@ -507,7 +515,7 @@ class Controller extends RequestHandler implements TemplateGlobalProvider $template = $this->getViewer($this->getAction()); // if the object is already customised (e.g. through Controller->run()), use it - $obj = ($this->customisedObj) ? $this->customisedObj : $this; + $obj = $this->getCustomisedObj() ?: $this; if ($params) { $obj = $this->customise($params); diff --git a/src/Control/Director.php b/src/Control/Director.php index ee9a8d9a1..05ca57f2f 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -8,7 +8,6 @@ use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Kernel; use SilverStripe\Dev\Deprecation; -use SilverStripe\Dev\SapphireTest; use SilverStripe\ORM\ArrayLib; use SilverStripe\Versioned\Versioned; use SilverStripe\View\Requirements; @@ -254,7 +253,7 @@ class Director implements TemplateGlobalProvider $_REQUEST = ArrayLib::array_merge_recursive((array) $getVars, (array) $postVars); $_GET = (array) $getVars; $_POST = (array) $postVars; - $_SESSION = $session ? $session->inst_getAll() : array(); + $_SESSION = $session ? $session->getAll() : array(); $_COOKIE = $cookieJar->getAll(false); Injector::inst()->registerService($cookieJar, Cookie_Backend::class); $_SERVER['REQUEST_URI'] = Director::baseURL() . $urlWithQuerystring; @@ -288,7 +287,7 @@ class Director implements TemplateGlobalProvider } } - $output = $requestProcessor->postRequest($request, $result, $model); + $output = $requestProcessor->postRequest($request, $result); if ($output === false) { throw new HTTPResponse_Exception("Invalid response"); } @@ -836,17 +835,15 @@ class Director implements TemplateGlobalProvider * Skip any further processing and immediately respond with a redirect to the passed URL. * * @param string $destURL + * @throws HTTPResponse_Exception */ protected static function force_redirect($destURL) { + // Redirect to installer $response = new HTTPResponse(); $response->redirect($destURL, 301); - HTTP::add_cache_headers($response); - - // TODO: Use an exception - ATM we can be called from _config.php, before Director#handleRequest's try block - $response->output(); - die; + throw new HTTPResponse_Exception($response); } /** @@ -877,19 +874,23 @@ class Director implements TemplateGlobalProvider * * @param array $patterns Array of regex patterns to match URLs that should be HTTPS. * @param string $secureDomain Secure domain to redirect to. Defaults to the current domain. - * - * @return bool|string String of URL when unit tests running, boolean FALSE if patterns don't match request URI. + * @return bool true if already on SSL, false if doesn't match patterns (or cannot redirect) + * @throws HTTPResponse_Exception Throws exception with redirect, if successful */ public static function forceSSL($patterns = null, $secureDomain = null) { - // Calling from the command-line? + // Already on SSL + if (static::is_https()) { + return true; + } + + // Can't redirect without a url if (!isset($_SERVER['REQUEST_URI'])) { return false; } - $matched = false; - if ($patterns) { + $matched = false; $relativeURL = self::makeRelative(Director::absoluteURL($_SERVER['REQUEST_URI'])); // protect portions of the site based on the pattern @@ -899,31 +900,20 @@ class Director implements TemplateGlobalProvider break; } } - } else { - // protect the entire site - $matched = true; + if (!$matched) { + return false; + } } - if ($matched && !self::is_https()) { - // if an domain is specified, redirect to that instead of the current domain - if ($secureDomain) { - $url = 'https://' . $secureDomain . $_SERVER['REQUEST_URI']; - } else { - $url = $_SERVER['REQUEST_URI']; - } - - $destURL = str_replace('http:', 'https:', Director::absoluteURL($url)); - - // This coupling to SapphireTest is necessary to test the destination URL and to not interfere with tests - if (class_exists('SilverStripe\\Dev\\SapphireTest', false) && SapphireTest::is_running_test()) { - return $destURL; - } else { - self::force_redirect($destURL); - return true; - } - } else { - return false; + // if an domain is specified, redirect to that instead of the current domain + if (!$secureDomain) { + $secureDomain = static::host(); } + $url = 'https://' . $secureDomain . $_SERVER['REQUEST_URI']; + + // Force redirect + self::force_redirect($url); + return true; } /** diff --git a/src/Control/Email/Email.php b/src/Control/Email/Email.php index 627888e4c..c879a6af0 100644 --- a/src/Control/Email/Email.php +++ b/src/Control/Email/Email.php @@ -263,7 +263,7 @@ class Email extends ViewableData public function setSwiftMessage($swiftMessage) { $swiftMessage->setDate(DBDatetime::now()->getTimestamp()); - if (!$swiftMessage->getFrom() && ($defaultFrom = $this->config()->admin_email)) { + if (!$swiftMessage->getFrom() && ($defaultFrom = $this->config()->get('admin_email'))) { $swiftMessage->setFrom($defaultFrom); } $this->swiftMessage = $swiftMessage; @@ -304,7 +304,7 @@ class Email extends ViewableData } /** - * @return array + * @return string */ public function getSender() { diff --git a/src/Dev/DevelopmentAdmin.php b/src/Dev/DevelopmentAdmin.php index c946f8494..c71c58262 100644 --- a/src/Dev/DevelopmentAdmin.php +++ b/src/Dev/DevelopmentAdmin.php @@ -42,6 +42,16 @@ class DevelopmentAdmin extends Controller 'generatesecuretoken', ); + /** + * Assume that CLI equals admin permissions + * If set to false, normal permission model will apply even in CLI mode + * Applies to all development admin tasks (E.g. TaskRunner, DatabaseAdmin) + * + * @config + * @var bool + */ + private static $allow_all_cli = true; + protected function init() { parent::init(); @@ -52,10 +62,11 @@ class DevelopmentAdmin extends Controller // We allow access to this controller regardless of live-status or ADMIN permission only // if on CLI. Access to this controller is always allowed in "dev-mode", or of the user is ADMIN. + $allowAllCLI = static::config()->get('allow_all_cli'); $canAccess = ( $requestedDevBuild || Director::isDev() - || Director::is_cli() + || (Director::is_cli() && $allowAllCLI) // Its important that we don't run this check if dev/build was requested || Permission::check("ADMIN") ); diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php index f46fe663d..17b2f1788 100644 --- a/src/Dev/SapphireTest.php +++ b/src/Dev/SapphireTest.php @@ -33,12 +33,10 @@ use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\FieldType\DBField; -use SilverStripe\ORM\SS_List; use SilverStripe\Security\Group; use SilverStripe\Security\Member; use SilverStripe\Security\Permission; use SilverStripe\Security\Security; -use SilverStripe\Versioned\Versioned; use SilverStripe\View\Requirements; use SilverStripe\View\SSViewer; use SilverStripe\View\ThemeManifest; @@ -183,12 +181,12 @@ class SapphireTest extends PHPUnit_Framework_TestCase * * @return boolean */ - public static function is_running_test() + protected static function is_running_test() { return self::$is_running_test; } - public static function set_is_running_test($bool) + protected static function set_is_running_test($bool) { self::$is_running_test = $bool; } diff --git a/src/Dev/TaskRunner.php b/src/Dev/TaskRunner.php index b98f18072..1b5e2eb9b 100644 --- a/src/Dev/TaskRunner.php +++ b/src/Dev/TaskRunner.php @@ -29,12 +29,12 @@ class TaskRunner extends Controller { parent::init(); - $isRunningTests = (class_exists('SilverStripe\\Dev\\SapphireTest', false) && SapphireTest::is_running_test()); + $allowAllCLI = DevelopmentAdmin::config()->get('allow_all_cli'); $canAccess = ( Director::isDev() // We need to ensure that DevelopmentAdminTest can simulate permission failures when running // "dev/tasks" from CLI. - || (Director::is_cli() && !$isRunningTests) + || (Director::is_cli() && $allowAllCLI) || Permission::check("ADMIN") ); if (!$canAccess) { diff --git a/src/Forms/GridField/GridField_FormAction.php b/src/Forms/GridField/GridField_FormAction.php index 27ee4a217..627ef8b87 100644 --- a/src/Forms/GridField/GridField_FormAction.php +++ b/src/Forms/GridField/GridField_FormAction.php @@ -3,7 +3,6 @@ namespace SilverStripe\Forms\GridField; use SilverStripe\Control\Controller; -use SilverStripe\Control\Session; use SilverStripe\Forms\Form; use SilverStripe\Forms\FormAction; diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index d7d862726..26ad22580 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -528,7 +528,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $originalClass = $this->ClassName; /** @var DataObject $newInstance */ - $newInstance = Injector::inst()->create($newClassName, $this->record, false, $this->model); + $newInstance = Injector::inst()->create($newClassName, $this->record, false); // Modify ClassName if ($newClassName != $originalClass) { @@ -1526,7 +1526,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } if (empty($component)) { - $component = $this->model->$class->newObject(); + $component = Injector::inst()->create($class); if ($polymorphic) { $component->{$joinField.'ID'} = $this->ID; $component->{$joinField.'Class'} = static::class; @@ -1582,10 +1582,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $result = HasManyList::create($componentClass, $joinField); } - if ($this->model) { - $result->setDataModel($this->model); - } - return $result ->setDataQueryParam($this->getInheritableQueryParams()) ->forForeignID($this->ID); @@ -1707,9 +1703,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $joinField = "{$remoteRelation}ID"; $componentClass = $schema->classForField($remoteClass, $joinField); $result = HasManyList::create($componentClass, $joinField); - if ($this->model) { - $result->setDataModel($this->model); - } return $result ->setDataQueryParam($this->getInheritableQueryParams()) ->forForeignID($this->ID); @@ -1753,9 +1746,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $manyMany['childField'], // Reversed parent / child field $extraFields ); - if ($this->model) { - $result->setDataModel($this->model); - } $this->extend('updateManyManyComponents', $result); // If this is called on a singleton, then we return an 'orphaned relation' that can have the @@ -1814,10 +1804,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $query->setQueryParam('Component.ExtraFields', $extraFields); }); - if ($this->model) { - $result->setDataModel($this->model); - } - $this->extend('updateManyManyComponents', $result); // If this is called on a singleton, then we return an 'orphaned relation' that can have the @@ -3126,7 +3112,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if (!$hasData) { $className = static::class; foreach ($defaultRecords as $record) { - $obj = $this->model->$className->newObject($record); + $obj = Injector::inst()->create($className, $record); $obj->write(); } DB::alteration_message("Added default records to $className table", "created"); diff --git a/src/ORM/DatabaseAdmin.php b/src/ORM/DatabaseAdmin.php index bf3abc4d9..0419378e2 100644 --- a/src/ORM/DatabaseAdmin.php +++ b/src/ORM/DatabaseAdmin.php @@ -6,16 +6,13 @@ use SilverStripe\Control\Director; use SilverStripe\Control\Controller; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Manifest\ClassLoader; +use SilverStripe\Dev\DevelopmentAdmin; use SilverStripe\Dev\SapphireTest; use SilverStripe\Dev\TestOnly; -use SilverStripe\Dev\Deprecation; use SilverStripe\Versioned\Versioned; use SilverStripe\Security\Security; use SilverStripe\Security\Permission; -// Include the DB class -require_once("DB.php"); - /** * DatabaseAdmin class * @@ -58,13 +55,13 @@ class DatabaseAdmin extends Controller // if on CLI or with the database not ready. The latter makes it less errorprone to do an // initial schema build without requiring a default-admin login. // Access to this controller is always allowed in "dev-mode", or of the user is ADMIN. - $isRunningTests = (class_exists('SilverStripe\\Dev\\SapphireTest', false) && SapphireTest::is_running_test()); + $allowAllCLI = DevelopmentAdmin::config()->get('allow_all_cli'); $canAccess = ( Director::isDev() || !Security::database_is_ready() // We need to ensure that DevelopmentAdminTest can simulate permission failures when running // "dev/tests" from CLI. - || (Director::is_cli() && !$isRunningTests) + || (Director::is_cli() && $allowAllCLI) || Permission::check("ADMIN") ); if (!$canAccess) { @@ -87,14 +84,14 @@ class DatabaseAdmin extends Controller $allClasses = get_declared_classes(); $rootClasses = []; foreach ($allClasses as $class) { - if (get_parent_class($class) == 'SilverStripe\ORM\DataObject') { + if (get_parent_class($class) == DataObject::class) { $rootClasses[$class] = array(); } } // Assign every other data object one of those foreach ($allClasses as $class) { - if (!isset($rootClasses[$class]) && is_subclass_of($class, 'SilverStripe\ORM\DataObject')) { + if (!isset($rootClasses[$class]) && is_subclass_of($class, DataObject::class)) { foreach ($rootClasses as $rootClass => $dummy) { if (is_subclass_of($class, $rootClass)) { $rootClasses[$rootClass][] = $class; @@ -260,7 +257,6 @@ class DatabaseAdmin extends Controller // Initiate schema update $dbSchema = DB::get_schema(); $dbSchema->schemaUpdate(function () use ($dataClasses, $testMode, $quiet) { - /** @var SilverStripe\ORM\DataObjectSchema $dataObjectSchema */ $dataObjectSchema = DataObject::getSchema(); foreach ($dataClasses as $dataClass) { diff --git a/src/Security/AuthenticationRequestFilter.php b/src/Security/AuthenticationRequestFilter.php index f4f68c847..d06732c7c 100644 --- a/src/Security/AuthenticationRequestFilter.php +++ b/src/Security/AuthenticationRequestFilter.php @@ -6,9 +6,7 @@ use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Control\RequestFilter; -use SilverStripe\Control\Session; use SilverStripe\Core\Config\Configurable; -use SilverStripe\ORM\DataModel; use SilverStripe\ORM\ValidationException; class AuthenticationRequestFilter implements RequestFilter @@ -42,12 +40,10 @@ class AuthenticationRequestFilter implements RequestFilter * Identify the current user from the request * * @param HTTPRequest $request - * @param Session $session - * @param DataModel $model * @return bool|void * @throws HTTPResponse_Exception */ - public function preRequest(HTTPRequest $request, Session $session, DataModel $model) + public function preRequest(HTTPRequest $request) { try { $this @@ -66,10 +62,9 @@ class AuthenticationRequestFilter implements RequestFilter * * @param HTTPRequest $request * @param HTTPResponse $response - * @param DataModel $model * @return bool|void */ - public function postRequest(HTTPRequest $request, HTTPResponse $response, DataModel $model) + public function postRequest(HTTPRequest $request, HTTPResponse $response) { } } diff --git a/src/Security/Authenticator.php b/src/Security/Authenticator.php index c4f66b121..856f65654 100644 --- a/src/Security/Authenticator.php +++ b/src/Security/Authenticator.php @@ -2,6 +2,7 @@ namespace SilverStripe\Security; +use SilverStripe\Control\HTTPRequest; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\MemberAuthenticator\LoginHandler; use SilverStripe\Security\MemberAuthenticator\LogoutHandler; @@ -105,10 +106,11 @@ interface Authenticator * Method to authenticate an user. * * @param array $data Raw data to authenticate the user. + * @param HTTPRequest $request * @param ValidationResult $result A validationresult which is either valid or contains the error message(s) * @return Member The matched member, or null if the authentication fails */ - public function authenticate($data, ValidationResult &$result = null); + public function authenticate(array $data, HTTPRequest $request, ValidationResult &$result = null); /** * Check if the passed password matches the stored one (if the member is not locked out). diff --git a/src/Security/BasicAuth.php b/src/Security/BasicAuth.php index 7f6d13983..9682e778e 100644 --- a/src/Security/BasicAuth.php +++ b/src/Security/BasicAuth.php @@ -30,6 +30,13 @@ class BasicAuth */ private static $entire_site_protected = false; + /** + * Set to true to ignore in CLI mode + * + * @var bool + */ + private static $ignore_cli = true; + /** * @config * @var String|array Holds a {@link Permission} code that is required @@ -65,8 +72,7 @@ class BasicAuth $permissionCode = null, $tryUsingSessionLogin = true ) { - $isRunningTests = (class_exists(SapphireTest::class, false) && SapphireTest::is_running_test()); - if (!Security::database_is_ready() || (Director::is_cli() && !$isRunningTests)) { + if (!Security::database_is_ready() || (Director::is_cli() && static::config()->get('ignore_cli'))) { return true; } @@ -96,7 +102,7 @@ class BasicAuth $member = $authenticator->authenticate([ 'Email' => $request->getHeader('PHP_AUTH_USER'), 'Password' => $request->getHeader('PHP_AUTH_PW'), - ]); + ], $request); if ($member instanceof Member) { break; } diff --git a/src/Security/MemberAuthenticator/LoginHandler.php b/src/Security/MemberAuthenticator/LoginHandler.php index 31a2b00c4..39b590f61 100644 --- a/src/Security/MemberAuthenticator/LoginHandler.php +++ b/src/Security/MemberAuthenticator/LoginHandler.php @@ -6,7 +6,6 @@ use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\RequestHandler; -use SilverStripe\Control\Session; use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Authenticator; @@ -105,17 +104,18 @@ class LoginHandler extends RequestHandler * * @param array $data Submitted data * @param MemberLoginForm $form + * @param HTTPRequest $request * @return HTTPResponse */ - public function doLogin($data, $form) + public function doLogin($data, MemberLoginForm $form, HTTPRequest $request) { $failureMessage = null; $this->extend('beforeLogin'); // Successful login /** @var ValidationResult $result */ - if ($member = $this->checkLogin($data, $result)) { - $this->performLogin($member, $data, $form->getRequestHandler()->getRequest()); + if ($member = $this->checkLogin($data, $request, $result)) { + $this->performLogin($member, $data, $request); // Allow operations on the member after successful login $this->extend('afterLogin', $member); @@ -138,8 +138,11 @@ class LoginHandler extends RequestHandler /** @skipUpgrade */ if (array_key_exists('Email', $data)) { $rememberMe = (isset($data['Remember']) && Security::config()->get('autologin_enabled') === true); - Session::set('SessionForms.MemberLoginForm.Email', $data['Email']); - Session::set('SessionForms.MemberLoginForm.Remember', $rememberMe); + $this + ->getRequest() + ->getSession() + ->set('SessionForms.MemberLoginForm.Email', $data['Email']) + ->set('SessionForms.MemberLoginForm.Remember', $rememberMe); } // Fail to login redirects back to form @@ -167,8 +170,11 @@ class LoginHandler extends RequestHandler */ protected function redirectAfterSuccessfulLogin() { - Session::clear('SessionForms.MemberLoginForm.Email'); - Session::clear('SessionForms.MemberLoginForm.Remember'); + $this + ->getRequest() + ->getSession() + ->clear('SessionForms.MemberLoginForm.Email') + ->clear('SessionForms.MemberLoginForm.Remember'); $member = Security::getCurrentUser(); if ($member->isPasswordExpired()) { @@ -206,13 +212,14 @@ class LoginHandler extends RequestHandler * Try to authenticate the user * * @param array $data Submitted data + * @param HTTPRequest $request * @param ValidationResult $result * @return Member Returns the member object on successful authentication * or NULL on failure. */ - public function checkLogin($data, ValidationResult &$result = null) + public function checkLogin($data, HTTPRequest $request, ValidationResult &$result = null) { - $member = $this->authenticator->authenticate($data, $result); + $member = $this->authenticator->authenticate($data, $request, $result); if ($member instanceof Member) { return $member; } @@ -229,11 +236,13 @@ class LoginHandler extends RequestHandler * @return Member Returns the member object on successful authentication * or NULL on failure. */ - public function performLogin($member, $data, $request) + public function performLogin($member, $data, HTTPRequest $request) { /** IdentityStore */ $rememberMe = (isset($data['Remember']) && Security::config()->get('autologin_enabled')); - Injector::inst()->get(IdentityStore::class)->logIn($member, $rememberMe, $request); + /** @var IdentityStore $identityStore */ + $identityStore = Injector::inst()->get(IdentityStore::class); + $identityStore->logIn($member, $rememberMe, $request); return $member; } diff --git a/src/Security/MemberAuthenticator/MemberAuthenticator.php b/src/Security/MemberAuthenticator/MemberAuthenticator.php index 0068b6270..d2fd78122 100644 --- a/src/Security/MemberAuthenticator/MemberAuthenticator.php +++ b/src/Security/MemberAuthenticator/MemberAuthenticator.php @@ -6,6 +6,7 @@ use InvalidArgumentException; use SilverStripe\Control\Controller; use SilverStripe\Control\Session; use SilverStripe\Core\Extensible; +use SilverStripe\Control\HTTPRequest; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Authenticator; use SilverStripe\Security\LoginAttempt; @@ -31,21 +32,16 @@ class MemberAuthenticator implements Authenticator | Authenticator::RESET_PASSWORD | Authenticator::CHECK_PASSWORD; } - /** - * @param array $data - * @param null|ValidationResult $result - * @return null|Member - */ - public function authenticate($data, ValidationResult &$result = null) + public function authenticate(array $data, HTTPRequest $request, ValidationResult &$result = null) { // Find authenticated member $member = $this->authenticateMember($data, $result); // Optionally record every login attempt as a {@link LoginAttempt} object - $this->recordLoginAttempt($data, $member, $result->isValid()); + $this->recordLoginAttempt($data, $request, $member, $result->isValid()); if ($member) { - Session::clear('BackURL'); + $request->getSession()->clear('BackURL'); } return $result->isValid() ? $member : null; @@ -73,15 +69,15 @@ class MemberAuthenticator implements Authenticator if ($result->isValid()) { // Check if default admin credentials are correct if (DefaultAdminService::isDefaultAdminCredentials($email, $data['Password'])) { - return $member; - } else { - $result->addError(_t( - 'SilverStripe\\Security\\Member.ERRORWRONGCRED', - "The provided details don't seem to be correct. Please try again." - )); - } + return $member; + } else { + $result->addError(_t( + 'SilverStripe\\Security\\Member.ERRORWRONGCRED', + "The provided details don't seem to be correct. Please try again." + )); } } + } // Attempt to identify user by email if (!$member && $email) { @@ -104,13 +100,13 @@ class MemberAuthenticator implements Authenticator $member->registerFailedLogin(); } } elseif ($member) { - $member->registerSuccessfulLogin(); - } else { - // A non-existing member occurred. This will make the result "valid" so let's invalidate - $result->addError(_t( - 'SilverStripe\\Security\\Member.ERRORWRONGCRED', - "The provided details don't seem to be correct. Please try again." - )); + $member->registerSuccessfulLogin(); + } else { + // A non-existing member occurred. This will make the result "valid" so let's invalidate + $result->addError(_t( + 'SilverStripe\\Security\\Member.ERRORWRONGCRED', + "The provided details don't seem to be correct. Please try again." + )); return null; } @@ -144,7 +140,7 @@ class MemberAuthenticator implements Authenticator // Check a password is set on this member if (empty($member->Password) && $member->exists()) { $result->addError(_t(__CLASS__ . '.NoPassword', 'There is no password on this member.')); - } + } $encryptor = PasswordEncryptor::create_for_algorithm($member->PasswordEncryption); if (!$encryptor->check($member->Password, $password, $member->Salt, $member)) { @@ -163,10 +159,11 @@ class MemberAuthenticator implements Authenticator * TODO We could handle this with an extension * * @param array $data + * @param HTTPRequest $request * @param Member $member * @param boolean $success */ - protected function recordLoginAttempt($data, $member, $success) + protected function recordLoginAttempt($data, HTTPRequest $request, $member, $success) { if (!Security::config()->get('login_recording')) { return; @@ -193,15 +190,16 @@ class MemberAuthenticator implements Authenticator if ($member) { // Audit logging hook $attempt->MemberID = $member->ID; - $member->extend('authenticationFailed'); + $member->extend('authenticationFailed', $data, $request); } else { // Audit logging hook - Member::singleton()->extend('authenticationFailedUnknownUser', $data); + Member::singleton() + ->extend('authenticationFailedUnknownUser', $data, $request); } } $attempt->Email = $email; - $attempt->IP = Controller::curr()->getRequest()->getIP(); + $attempt->IP = $request->getIP(); $attempt->write(); } diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index 16337d938..3bc6a77df 100644 --- a/src/View/Requirements_Backend.php +++ b/src/View/Requirements_Backend.php @@ -33,9 +33,9 @@ class Requirements_Backend /** * Whether to combine CSS and JavaScript files * - * @var bool + * @var bool|null */ - protected $combinedFilesEnabled = true; + protected $combinedFilesEnabled = null; /** * Determine if files should be combined automatically on dev mode. @@ -1394,18 +1394,8 @@ class Requirements_Backend */ public function getCombinedFilesEnabled() { - if (!$this->combinedFilesEnabled) { - return false; - } - - // Tests should be combined - if (class_exists('SilverStripe\\Dev\\SapphireTest', false) && SapphireTest::is_running_test()) { - return true; - } - - // Check if specified via querystring - if (isset($_REQUEST['combine'])) { - return true; + if (isset($this->combinedFilesEnabled)) { + return $this->combinedFilesEnabled; } // Non-dev sites are always combined diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php index da2db5944..5f372c022 100644 --- a/src/View/ViewableData.php +++ b/src/View/ViewableData.php @@ -385,7 +385,7 @@ class ViewableData implements IteratorAggregate $template = new SSViewer($template); } - $data = ($this->customisedObject) ? $this->customisedObject : $this; + $data = $this->getCustomisedObj() ?: $this; if ($customFields instanceof ViewableData) { $data = $data->customise($customFields); diff --git a/src/conf/ConfigureFromEnv.php b/src/conf/ConfigureFromEnv.php index aadf2a7bd..8a6fd4a90 100644 --- a/src/conf/ConfigureFromEnv.php +++ b/src/conf/ConfigureFromEnv.php @@ -5,3 +5,6 @@ * * Placeholder empty file */ +use SilverStripe\Dev\Deprecation; + +Deprecation::notice('5.0', 'ConfigureFromEnv.php is deprecated', Deprecation::SCOPE_GLOBAL); diff --git a/tests/php/Security/MemberAuthenticatorTest.php b/tests/php/Security/MemberAuthenticatorTest.php index 139128b59..9eaf77edd 100644 --- a/tests/php/Security/MemberAuthenticatorTest.php +++ b/tests/php/Security/MemberAuthenticatorTest.php @@ -2,8 +2,8 @@ namespace SilverStripe\Security\Tests; +use SilverStripe\Control\Controller; use SilverStripe\Core\Injector\Injector; -use SilverStripe\ORM\DataModel; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Authenticator; @@ -54,8 +54,6 @@ class MemberAuthenticatorTest extends SapphireTest public function testCustomIdentifierField() { - Member::config()->set('unique_identifier_field', 'Username'); - $label = Member::singleton() ->fieldLabel(Member::config()->get('unique_identifier_field')); @@ -71,7 +69,7 @@ class MemberAuthenticatorTest extends SapphireTest // Create basic login form $frontendResponse = $authenticator ->getLoginHandler($controller->link()) - ->handleRequest(new HTTPRequest('get', '/'), DataModel::inst()); + ->handleRequest(new HTTPRequest('get', '/')); $this->assertTrue(is_array($frontendResponse)); $this->assertTrue(isset($frontendResponse['Form'])); @@ -116,10 +114,11 @@ class MemberAuthenticatorTest extends SapphireTest // Test correct login /** @var ValidationResult $message */ $result = $authenticator->authenticate( - array( + [ 'tempid' => $tempID, 'Password' => 'mypassword' - ), + ], + Controller::curr()->getRequest(), $message ); @@ -129,10 +128,11 @@ class MemberAuthenticatorTest extends SapphireTest // Test incorrect login $result = $authenticator->authenticate( - array( + [ 'tempid' => $tempID, 'Password' => 'notmypassword' - ), + ], + Controller::curr()->getRequest(), $message ); @@ -154,10 +154,11 @@ class MemberAuthenticatorTest extends SapphireTest // Test correct login /** @var ValidationResult $message */ $result = $authenticator->authenticate( - array( + [ 'Email' => 'admin', 'Password' => 'password' - ), + ], + Controller::curr()->getRequest(), $message ); $this->assertNotEmpty($result); @@ -166,10 +167,11 @@ class MemberAuthenticatorTest extends SapphireTest // Test incorrect login $result = $authenticator->authenticate( - array( + [ 'Email' => 'admin', 'Password' => 'notmypassword' - ), + ], + Controller::curr()->getRequest(), $message ); $messages = $message->getMessages(); @@ -193,7 +195,8 @@ class MemberAuthenticatorTest extends SapphireTest [ 'Email' => 'admin', 'Password' => 'wrongpassword' - ] + ], + Controller::curr()->getRequest() ); $defaultAdmin = DefaultAdminService::singleton()->findOrCreateDefaultAdmin();