From 5d235e64f341d6251bfe9f4833f15cc8593c5034 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 21 Jun 2017 12:13:15 +1200 Subject: [PATCH] API HTTPRequestBuilder::createFromEnvironment() now cleans up live globals BUG Fix issue with SSViewer Fix Security / View tests --- src/Control/CLIRequestBuilder.php | 2 +- src/Control/Director.php | 3 + src/Control/HTTPRequestBuilder.php | 10 +- src/Core/TempFolder.php | 2 +- src/Dev/CSSContentParser.php | 4 +- src/Dev/SapphireTest.php | 6 +- src/Forms/Form.php | 30 +++++- src/Security/BasicAuth.php | 2 +- src/Security/DefaultAdminService.php | 18 +++- .../MemberAuthenticator.php | 21 ++--- .../MemberAuthenticator/MemberLoginForm.php | 9 +- src/Security/Security.php | 14 ++- src/View/Requirements_Backend.php | 9 +- .../Control/PjaxResponseNegotiatorTest.php | 9 +- tests/php/Core/CoreTest.php | 15 +-- .../mysite/_config.php | 5 - .../ParameterConfirmationTokenTest.php | 2 + tests/php/Forms/FormRequestHandlerTest.php | 3 + tests/php/Forms/FormTest.php | 3 + .../GridField/GridFieldDeleteActionTest.php | 2 + tests/php/Forms/TreeDropdownFieldTest.php | 3 + tests/php/Security/BasicAuthTest.php | 16 ++-- .../ControllerSecuredWithPermission.php | 1 + tests/php/Security/GroupTest.php | 21 +++-- .../php/Security/MemberAuthenticatorTest.php | 17 ++-- tests/php/Security/SecurityTest.php | 94 ++++++++++--------- tests/php/View/RequirementsTest.php | 2 + tests/php/View/SSViewerCacheBlockTest.php | 3 +- tests/php/View/SSViewerTest.php | 45 ++++----- .../View/SSViewerTest/TestViewableData.php | 4 + 30 files changed, 228 insertions(+), 147 deletions(-) diff --git a/src/Control/CLIRequestBuilder.php b/src/Control/CLIRequestBuilder.php index bdca68282..660ffb7e0 100644 --- a/src/Control/CLIRequestBuilder.php +++ b/src/Control/CLIRequestBuilder.php @@ -7,7 +7,7 @@ namespace SilverStripe\Control; */ class CLIRequestBuilder extends HTTPRequestBuilder { - protected static function cleanEnvironment(array $variables) + public static function cleanEnvironment(array $variables) { // Create all blank vars foreach (['_REQUEST', '_GET', '_POST', '_SESSION', '_SERVER', '_COOKIE', '_ENV', '_FILES'] as $key) { diff --git a/src/Control/Director.php b/src/Control/Director.php index a4043db6f..f5f2d920f 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -315,6 +315,9 @@ class Director implements TemplateGlobalProvider $newVars['_SERVER']['REQUEST_URI'] = Director::baseURL() . $url; $newVars['_REQUEST'] = array_merge($newVars['_GET'], $newVars['_POST']); + // Normalise vars + $newVars = HTTPRequestBuilder::cleanEnvironment($newVars); + // Create new request $request = HTTPRequestBuilder::createFromVariables($newVars, $body); if ($headers) { diff --git a/src/Control/HTTPRequestBuilder.php b/src/Control/HTTPRequestBuilder.php index 94159b538..53903b3fa 100644 --- a/src/Control/HTTPRequestBuilder.php +++ b/src/Control/HTTPRequestBuilder.php @@ -15,8 +15,12 @@ class HTTPRequestBuilder */ public static function createFromEnvironment() { + // Clean and update live global variables + $variables = static::cleanEnvironment(Environment::getVariables()); + Environment::setVariables($variables); // Currently necessary for SSViewer, etc to work + // Health-check prior to creating environment - return static::createFromVariables(Environment::getVariables(), @file_get_contents('php://input')); + return static::createFromVariables($variables, @file_get_contents('php://input')); } /** @@ -28,8 +32,6 @@ class HTTPRequestBuilder */ public static function createFromVariables(array $variables, $input) { - $variables = static::cleanEnvironment($variables); - // Strip `url` out of querystring $url = $variables['_GET']['url']; unset($variables['_GET']['url']); @@ -92,7 +94,7 @@ class HTTPRequestBuilder * @param array $variables * @return array Cleaned variables */ - protected static function cleanEnvironment(array $variables) + public static function cleanEnvironment(array $variables) { // IIS will sometimes generate this. if (!empty($variables['_SERVER']['HTTP_X_ORIGINAL_URL'])) { diff --git a/src/Core/TempFolder.php b/src/Core/TempFolder.php index 4556382f8..821f88dde 100644 --- a/src/Core/TempFolder.php +++ b/src/Core/TempFolder.php @@ -34,7 +34,7 @@ class TempFolder * * @return string */ - protected static function getTempFolderUsername() + public static function getTempFolderUsername() { $user = getenv('APACHE_RUN_USER'); if (!$user) { diff --git a/src/Dev/CSSContentParser.php b/src/Dev/CSSContentParser.php index 56f201617..ed63515d0 100644 --- a/src/Dev/CSSContentParser.php +++ b/src/Dev/CSSContentParser.php @@ -69,7 +69,7 @@ class CSSContentParser * See {@link getByXpath()} for a more direct selector syntax. * * @param String $selector - * @return SimpleXMLElement + * @return SimpleXMLElement[] */ public function getBySelector($selector) { @@ -81,7 +81,7 @@ class CSSContentParser * Allows querying the content through XPATH selectors. * * @param String $xpath SimpleXML compatible XPATH statement - * @return SimpleXMLElement|false + * @return SimpleXMLElement[] */ public function getByXpath($xpath) { diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php index 4baa0d733..254a860ed 100644 --- a/src/Dev/SapphireTest.php +++ b/src/Dev/SapphireTest.php @@ -6,6 +6,7 @@ use Exception; use LogicException; use PHPUnit_Framework_TestCase; use SilverStripe\CMS\Controllers\RootURLController; +use SilverStripe\Control\CLIRequestBuilder; use SilverStripe\Control\Controller; use SilverStripe\Control\Cookie; use SilverStripe\Control\Director; @@ -889,9 +890,8 @@ class SapphireTest extends PHPUnit_Framework_TestCase static::set_is_running_test(true); // Mock request - $session = new Session(isset($_SESSION) ? $_SESSION : array()); - $request = new HTTPRequest('GET', '/'); - $request->setSession($session); + $_SERVER['argv'] = ['vendor/bin/phpunit', '/']; + $request = CLIRequestBuilder::createFromEnvironment(); // Test application $kernel = new TestKernel(BASE_PATH); diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 8d67bd058..c1bb1dc8b 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -2,9 +2,12 @@ namespace SilverStripe\Forms; +use BadMethodCallException; use SilverStripe\Control\Controller; use SilverStripe\Control\HasRequestHandler; use SilverStripe\Control\HTTP; +use SilverStripe\Control\HTTPRequest; +use SilverStripe\Control\NullHTTPRequest; use SilverStripe\Control\RequestHandler; use SilverStripe\Control\Session; use SilverStripe\Core\ClassInfo; @@ -341,6 +344,25 @@ class Form extends ViewableData implements HasRequestHandler return $this; } + /** + * Helper to get current request for this form + * + * @return HTTPRequest + */ + protected function getRequest() + { + // Check if current request handler has a request object + $controller = $this->getController(); + if ($controller && !($controller->getRequest() instanceof NullHTTPRequest)) { + return $controller->getRequest(); + } + // Fall back to current controller + if (Controller::has_curr() && !(Controller::curr()->getRequest() instanceof NullHTTPRequest)) { + return Controller::curr()->getRequest(); + } + return null; + } + /** * Get session for this form * @@ -348,9 +370,11 @@ class Form extends ViewableData implements HasRequestHandler */ protected function getSession() { - // Note: Session may not be available if this form doesn't have a request handler - $controller = $this->getController() ?: Controller::curr(); - return $controller->getRequest()->getSession(); + $request = $this->getRequest(); + if ($request) { + return $request->getSession(); + } + throw new BadMethodCallException("Session not available in the current context"); } /** diff --git a/src/Security/BasicAuth.php b/src/Security/BasicAuth.php index 9682e778e..cc9ca0bd9 100644 --- a/src/Security/BasicAuth.php +++ b/src/Security/BasicAuth.php @@ -8,7 +8,7 @@ use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Core\Config\Configurable; -use SilverStripe\Dev\SapphireTest; +use SilverStripe\Dev\Debug; use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator; /** diff --git a/src/Security/DefaultAdminService.php b/src/Security/DefaultAdminService.php index 0569c13ea..005de5df6 100644 --- a/src/Security/DefaultAdminService.php +++ b/src/Security/DefaultAdminService.php @@ -18,9 +18,12 @@ class DefaultAdminService use Injectable; /** - * @var bool + * Can be set to explicitly true or false, or left null. + * If null, hasDefaultAdmin() will be inferred from environment. + * + * @var bool|null */ - protected static $has_default_admin = false; + protected static $has_default_admin = null; /** * @var string @@ -72,7 +75,7 @@ class DefaultAdminService "No default admin configured. Please call hasDefaultAdmin() before getting default admin username" ); } - return static::$default_username; + return static::$default_username ?: getenv('SS_DEFAULT_ADMIN_USERNAME'); } /** @@ -86,7 +89,7 @@ class DefaultAdminService "No default admin configured. Please call hasDefaultAdmin() before getting default admin password" ); } - return static::$default_password; + return static::$default_password ?: getenv('SS_DEFAULT_ADMIN_PASSWORD'); } /** @@ -96,11 +99,16 @@ class DefaultAdminService */ public static function hasDefaultAdmin() { + // Check environment if not explicitly set + if (!isset(static::$has_default_admin)) { + return !empty(getenv('SS_DEFAULT_ADMIN_USERNAME')) + && !empty(getenv('SS_DEFAULT_ADMIN_PASSWORD')); + } return static::$has_default_admin; } /** - * Flush the default admin credentials + * Flush the default admin credentials. */ public static function clearDefaultAdmin() { diff --git a/src/Security/MemberAuthenticator/MemberAuthenticator.php b/src/Security/MemberAuthenticator/MemberAuthenticator.php index d2fd78122..0eddde86d 100644 --- a/src/Security/MemberAuthenticator/MemberAuthenticator.php +++ b/src/Security/MemberAuthenticator/MemberAuthenticator.php @@ -3,17 +3,16 @@ namespace SilverStripe\Security\MemberAuthenticator; use InvalidArgumentException; -use SilverStripe\Control\Controller; -use SilverStripe\Control\Session; -use SilverStripe\Core\Extensible; use SilverStripe\Control\HTTPRequest; +use SilverStripe\Core\Extensible; +use SilverStripe\Dev\Debug; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Authenticator; +use SilverStripe\Security\DefaultAdminService; use SilverStripe\Security\LoginAttempt; use SilverStripe\Security\Member; use SilverStripe\Security\PasswordEncryptor; use SilverStripe\Security\Security; -use SilverStripe\Security\DefaultAdminService; /** * Authenticator for the default "member" method @@ -69,15 +68,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) { diff --git a/src/Security/MemberAuthenticator/MemberLoginForm.php b/src/Security/MemberAuthenticator/MemberLoginForm.php index 472ac9c74..23e83acfa 100644 --- a/src/Security/MemberAuthenticator/MemberLoginForm.php +++ b/src/Security/MemberAuthenticator/MemberLoginForm.php @@ -4,7 +4,6 @@ namespace SilverStripe\Security\MemberAuthenticator; use SilverStripe\Control\Director; use SilverStripe\Control\RequestHandler; -use SilverStripe\Control\Session; use SilverStripe\Forms\CheckboxField; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormAction; @@ -124,11 +123,11 @@ class MemberLoginForm extends BaseLoginForm */ protected function getFormFields() { - $request = $this->getController()->getRequest(); + $request = $this->getRequest(); if ($request->getVar('BackURL')) { $backURL = $request->getVar('BackURL'); } else { - $backURL = Session::get('BackURL'); + $backURL = $request->getSession()->get('BackURL'); } $label = Member::singleton()->fieldLabel(Member::config()->get('unique_identifier_field')); @@ -191,11 +190,13 @@ class MemberLoginForm extends BaseLoginForm return $actions; } + + public function restoreFormState() { parent::restoreFormState(); - $session = Controller::curr()->getRequest()->getSession(); + $session = $this->getSession(); $forceMessage = $session->get('MemberLoginForm.force_message'); if (($member = Security::getCurrentUser()) && !$forceMessage) { $message = _t( diff --git a/src/Security/Security.php b/src/Security/Security.php index cfdc50edc..e38efd33e 100644 --- a/src/Security/Security.php +++ b/src/Security/Security.php @@ -391,7 +391,7 @@ class Security extends Controller implements TemplateGlobalProvider } static::singleton()->setLoginMessage($message, ValidationResult::TYPE_WARNING); - $loginResponse = static::singleton()->login(); + $loginResponse = static::singleton()->login($controller ? $controller->getRequest() : $controller); if ($loginResponse instanceof HTTPResponse) { return $loginResponse; } @@ -702,16 +702,20 @@ class Security extends Controller implements TemplateGlobalProvider */ public function login($request = null, $service = Authenticator::LOGIN) { + if ($request) { + $this->setRequest($request); + } elseif ($request) { + $request = $this->getRequest(); + } else { + throw new HTTPResponse_Exception("No request available", 500); + } + // Check pre-login process if ($response = $this->preLogin()) { return $response; } $authName = null; - if (!$request) { - $request = $this->getRequest(); - } - if ($request && $request->param('ID')) { $authName = $request->param('ID'); } diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index 83ea74dbd..d95240246 100644 --- a/src/View/Requirements_Backend.php +++ b/src/View/Requirements_Backend.php @@ -1325,9 +1325,12 @@ class Requirements_Backend if ($minify && !$this->minifier) { throw new Exception( sprintf( - 'Cannot minify files without a minification service defined. - Set %s::minifyCombinedFiles to false, or inject a %s service on - %s.properties.minifier', + <<setSession(new Session([])); $response = $negotiator->respond($request); $this->assertEquals('default response', $response->getBody()); } @@ -36,6 +38,7 @@ class PjaxResponseNegotiatorTest extends SapphireTest ) ); $request = new HTTPRequest('GET', '/'); + $request->setSession(new Session([])); $request->addHeader('X-Pjax', 'myfragment'); $response = $negotiator->respond($request); $this->assertEquals('{"myfragment":"myfragment response"}', $response->getBody()); @@ -57,6 +60,7 @@ class PjaxResponseNegotiatorTest extends SapphireTest ) ); $request = new HTTPRequest('GET', '/'); + $request->setSession(new Session([])); $request->addHeader('X-Pjax', 'myfragment,otherfragment'); $request->addHeader('Accept', 'text/json'); $response = $negotiator->respond($request); @@ -81,6 +85,7 @@ class PjaxResponseNegotiatorTest extends SapphireTest ); $request = new HTTPRequest('GET', '/'); + $request->setSession(new Session([])); $request->addHeader('X-Pjax', 'alpha'); $request->addHeader('Accept', 'text/json'); diff --git a/tests/php/Core/CoreTest.php b/tests/php/Core/CoreTest.php index 3d6700764..2e1d453bf 100644 --- a/tests/php/Core/CoreTest.php +++ b/tests/php/Core/CoreTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\Core\Tests; +use SilverStripe\Core\TempFolder; use SilverStripe\Dev\SapphireTest; use SilverStripe\Control\Director; @@ -22,31 +23,31 @@ class CoreTest extends SapphireTest public function testGetTempPathInProject() { - $user = getTempFolderUsername(); + $user = TempFolder::getTempFolderUsername(); if (file_exists($this->tempPath)) { - $this->assertEquals(getTempFolder(BASE_PATH), $this->tempPath . DIRECTORY_SEPARATOR . $user); + $this->assertEquals(TempFolder::getTempFolder(BASE_PATH), $this->tempPath . DIRECTORY_SEPARATOR . $user); } else { - $user = getTempFolderUsername(); + $user = TempFolder::getTempFolderUsername(); $base = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'silverstripe-cache-php' . preg_replace('/[^\w-\.+]+/', '-', PHP_VERSION); // A typical Windows location for where sites are stored on IIS $this->assertEquals( $base . 'C--inetpub-wwwroot-silverstripe-test-project' . DIRECTORY_SEPARATOR . $user, - getTempFolder('C:\\inetpub\\wwwroot\\silverstripe-test-project') + TempFolder::getTempFolder('C:\\inetpub\\wwwroot\\silverstripe-test-project') ); // A typical Mac OS X location for where sites are stored $this->assertEquals( $base . '-Users-joebloggs-Sites-silverstripe-test-project' . DIRECTORY_SEPARATOR . $user, - getTempFolder('/Users/joebloggs/Sites/silverstripe-test-project') + TempFolder::getTempFolder('/Users/joebloggs/Sites/silverstripe-test-project') ); // A typical Linux location for where sites are stored $this->assertEquals( $base . '-var-www-silverstripe-test-project' . DIRECTORY_SEPARATOR . $user, - getTempFolder('/var/www/silverstripe-test-project') + TempFolder::getTempFolder('/var/www/silverstripe-test-project') ); } } @@ -54,7 +55,7 @@ class CoreTest extends SapphireTest protected function tearDown() { parent::tearDown(); - $user = getTempFolderUsername(); + $user = TempFolder::getTempFolderUsername(); $base = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'silverstripe-cache-php' . preg_replace('/[^\w-\.+]+/', '-', PHP_VERSION); foreach (array( diff --git a/tests/php/Core/Manifest/fixtures/configmanifest_dynamicenv/mysite/_config.php b/tests/php/Core/Manifest/fixtures/configmanifest_dynamicenv/mysite/_config.php index 2443214d9..b3d9bbc7f 100644 --- a/tests/php/Core/Manifest/fixtures/configmanifest_dynamicenv/mysite/_config.php +++ b/tests/php/Core/Manifest/fixtures/configmanifest_dynamicenv/mysite/_config.php @@ -1,6 +1 @@ request = new HTTPRequest('GET', '/', $get); + $this->request->setSession(new Session([])); } public function testParameterDetectsParameters() diff --git a/tests/php/Forms/FormRequestHandlerTest.php b/tests/php/Forms/FormRequestHandlerTest.php index efdebbe14..7d273d16a 100644 --- a/tests/php/Forms/FormRequestHandlerTest.php +++ b/tests/php/Forms/FormRequestHandlerTest.php @@ -4,6 +4,7 @@ namespace SilverStripe\Forms\Tests; use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; +use SilverStripe\Control\Session; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormAction; @@ -24,6 +25,7 @@ class FormRequestHandlerTest extends SapphireTest $form->disableSecurityToken(); $handler = new TestFormRequestHandler($form); $request = new HTTPRequest('POST', '/', null, ['action_mySubmitOnFormHandler' => 1]); + $request->setSession(new Session([])); $response = $handler->httpSubmission($request); $this->assertFalse($response->isError()); } @@ -39,6 +41,7 @@ class FormRequestHandlerTest extends SapphireTest $form->disableSecurityToken(); $handler = new FormRequestHandler($form); $request = new HTTPRequest('POST', '/', null, ['action_mySubmitOnForm' => 1]); + $request->setSession(new Session([])); $response = $handler->httpSubmission($request); $this->assertFalse($response->isError()); } diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php index 8e3a3e4c0..e44ffc88a 100644 --- a/tests/php/Forms/FormTest.php +++ b/tests/php/Forms/FormTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\Forms\Tests; +use SilverStripe\Control\Session; use SilverStripe\Forms\Tests\FormTest\TestController; use SilverStripe\Forms\Tests\FormTest\ControllerWithSecurityToken; use SilverStripe\Forms\Tests\FormTest\ControllerWithStrictPostCheck; @@ -778,6 +779,7 @@ class FormTest extends FunctionalTest 'action_doSubmit' => 1 ) ); + $request->setSession(new Session([])); $form->getRequestHandler()->httpSubmission($request); $button = $form->getRequestHandler()->buttonClicked(); @@ -798,6 +800,7 @@ class FormTest extends FunctionalTest 'action_doSubmit' => 1 ) ); + $request->setSession(new Session([])); $form->getRequestHandler()->httpSubmission($request); $button = $form->getRequestHandler()->buttonClicked(); diff --git a/tests/php/Forms/GridField/GridFieldDeleteActionTest.php b/tests/php/Forms/GridField/GridFieldDeleteActionTest.php index 055c72fac..c3777b0c4 100644 --- a/tests/php/Forms/GridField/GridFieldDeleteActionTest.php +++ b/tests/php/Forms/GridField/GridFieldDeleteActionTest.php @@ -5,6 +5,7 @@ namespace SilverStripe\Forms\Tests\GridField; use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse_Exception; +use SilverStripe\Control\Session; use SilverStripe\Dev\CSSContentParser; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\FieldList; @@ -108,6 +109,7 @@ class GridFieldDeleteActionTest extends SapphireTest 'SecurityID' => null, ) ); + $request->setSession(new Session([])); $this->gridField->gridFieldAlterAction(array('StateID'=>$stateID), $this->form, $request); } diff --git a/tests/php/Forms/TreeDropdownFieldTest.php b/tests/php/Forms/TreeDropdownFieldTest.php index cf7a9e23a..fc77c7145 100644 --- a/tests/php/Forms/TreeDropdownFieldTest.php +++ b/tests/php/Forms/TreeDropdownFieldTest.php @@ -4,6 +4,7 @@ namespace SilverStripe\Forms\Tests; use SilverStripe\Assets\File; use SilverStripe\Assets\Folder; +use SilverStripe\Control\Session; use SilverStripe\Dev\CSSContentParser; use SilverStripe\Dev\SapphireTest; use SilverStripe\Control\HTTPRequest; @@ -21,6 +22,7 @@ class TreeDropdownFieldTest extends SapphireTest // case insensitive search against keyword 'sub' for folders $request = new HTTPRequest('GET', 'url', array('search'=>'sub')); + $request->setSession(new Session([])); $response = $field->tree($request); $tree = $response->getBody(); @@ -58,6 +60,7 @@ class TreeDropdownFieldTest extends SapphireTest // case insensitive search against keyword 'sub' for files $request = new HTTPRequest('GET', 'url', array('search'=>'sub')); + $request->setSession(new Session([])); $response = $field->tree($request); $tree = $response->getBody(); diff --git a/tests/php/Security/BasicAuthTest.php b/tests/php/Security/BasicAuthTest.php index 031fd4438..33dd1073d 100644 --- a/tests/php/Security/BasicAuthTest.php +++ b/tests/php/Security/BasicAuthTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\Security\Tests; +use SilverStripe\Security\BasicAuth; use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\Dev\FunctionalTest; @@ -29,24 +30,21 @@ class BasicAuthTest extends FunctionalTest parent::setUp(); // Fixtures assume Email is the field used to identify the log in identity - Member::config()->unique_identifier_field = 'Email'; + Member::config()->set('unique_identifier_field', 'Email'); Security::force_database_is_ready(true); // Prevents Member test subclasses breaking ready test - Member::config()->lock_out_after_incorrect_logins = 10; + Member::config()->set('lock_out_after_incorrect_logins', 10); + + // Temp disable is_cli() exemption for tests + BasicAuth::config()->set('ignore_cli', false); } public function testBasicAuthEnabledWithoutLogin() { - $origUser = isset($_SERVER['PHP_AUTH_USER']) ? $_SERVER['PHP_AUTH_USER'] : null; - $origPw = isset($_SERVER['PHP_AUTH_PW']) ? $_SERVER['PHP_AUTH_PW'] : null; - unset($_SERVER['PHP_AUTH_USER']); unset($_SERVER['PHP_AUTH_PW']); - $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', null, $_SESSION, null, null, $_SERVER); + $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission'); $this->assertEquals(401, $response->getStatusCode()); - - $_SERVER['PHP_AUTH_USER'] = $origUser; - $_SERVER['PHP_AUTH_PW'] = $origPw; } public function testBasicAuthDoesntCallActionOrFurtherInitOnAuthFailure() diff --git a/tests/php/Security/BasicAuthTest/ControllerSecuredWithPermission.php b/tests/php/Security/BasicAuthTest/ControllerSecuredWithPermission.php index c7453c45b..0bc99381e 100644 --- a/tests/php/Security/BasicAuthTest/ControllerSecuredWithPermission.php +++ b/tests/php/Security/BasicAuthTest/ControllerSecuredWithPermission.php @@ -29,6 +29,7 @@ class ControllerSecuredWithPermission extends Controller implements TestOnly public function index() { self::$index_called = true; + return "index"; } public function Link($action = null) diff --git a/tests/php/Security/GroupTest.php b/tests/php/Security/GroupTest.php index 8c9b0d64e..35893153a 100644 --- a/tests/php/Security/GroupTest.php +++ b/tests/php/Security/GroupTest.php @@ -2,18 +2,18 @@ namespace SilverStripe\Security\Tests; +use InvalidArgumentException; use SilverStripe\Control\Controller; +use SilverStripe\Dev\FunctionalTest; use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\DataObject; use SilverStripe\Security\Group; -use SilverStripe\Dev\FunctionalTest; -use SilverStripe\Control\Session; +use SilverStripe\Security\Member; use SilverStripe\Security\Permission; use SilverStripe\Security\Tests\GroupTest\TestMember; class GroupTest extends FunctionalTest { - protected static $fixture_file = 'GroupTest.yml'; protected static $extra_dataobjects = [ @@ -44,13 +44,14 @@ class GroupTest extends FunctionalTest */ public function testMemberGroupRelationForm() { - Session::set('loggedInAs', $this->idFromFixture(TestMember::class, 'admin')); + $this->logInAs($this->idFromFixture(TestMember::class, 'admin')); $adminGroup = $this->objFromFixture(Group::class, 'admingroup'); $parentGroup = $this->objFromFixture(Group::class, 'parentgroup'); // Test single group relation through checkboxsetfield $form = new GroupTest\MemberForm(Controller::curr(), 'Form'); + /** @var Member $member */ $member = $this->objFromFixture(TestMember::class, 'admin'); $form->loadDataFrom($member); $checkboxSetField = $form->Fields()->fieldByName('Groups'); @@ -110,7 +111,9 @@ class GroupTest extends FunctionalTest public function testCollateAncestorIDs() { + /** @var Group $parentGroup */ $parentGroup = $this->objFromFixture(Group::class, 'parentgroup'); + /** @var Group $childGroup */ $childGroup = $this->objFromFixture(Group::class, 'childgroup'); $orphanGroup = new Group(); $orphanGroup->ParentID = 99999; @@ -144,6 +147,7 @@ class GroupTest extends FunctionalTest */ public function testCollateFamilyIds() { + /** @var Group $group */ $group = $this->objFromFixture(Group::class, 'parentgroup'); $groupIds = $this->allFixtureIDs(Group::class); $ids = array_intersect_key($groupIds, array_flip(['parentgroup', 'childgroup', 'grandchildgroup'])); @@ -152,11 +156,11 @@ class GroupTest extends FunctionalTest /** * Test that an exception is thrown if collateFamilyIDs is called on an unsaved Group - * @expectedException InvalidArgumentException - * @expectedExceptionMessage Cannot call collateFamilyIDs on unsaved Group. */ public function testCannotCollateUnsavedGroupFamilyIds() { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Cannot call collateFamilyIDs on unsaved Group.'); $group = new Group; $group->collateFamilyIDs(); } @@ -166,8 +170,8 @@ class GroupTest extends FunctionalTest */ public function testGetAllChildren() { + /** @var Group $group */ $group = $this->objFromFixture(Group::class, 'parentgroup'); - $children = $group->getAllChildren(); $this->assertInstanceOf(ArrayList::class, $children); $this->assertSame(['childgroup', 'grandchildgroup'], $children->column('Code')); @@ -204,7 +208,9 @@ class GroupTest extends FunctionalTest public function testValidatesPrivilegeLevelOfParent() { + /** @var Group $nonAdminGroup */ $nonAdminGroup = $this->objFromFixture(Group::class, 'childgroup'); + /** @var Group $adminGroup */ $adminGroup = $this->objFromFixture(Group::class, 'admingroup'); // Making admin group parent of a non-admin group, effectively expanding is privileges @@ -226,6 +232,7 @@ class GroupTest extends FunctionalTest $nonAdminGroup->write(); $this->logInWithPermission('ADMIN'); + /** @var Group $inheritedAdminGroup */ $inheritedAdminGroup = $this->objFromFixture(Group::class, 'group1'); $inheritedAdminGroup->ParentID = $adminGroup->ID; $inheritedAdminGroup->write(); // only works with ADMIN login diff --git a/tests/php/Security/MemberAuthenticatorTest.php b/tests/php/Security/MemberAuthenticatorTest.php index 9eaf77edd..475138ded 100644 --- a/tests/php/Security/MemberAuthenticatorTest.php +++ b/tests/php/Security/MemberAuthenticatorTest.php @@ -3,21 +3,20 @@ namespace SilverStripe\Security\Tests; use SilverStripe\Control\Controller; +use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Dev\SapphireTest; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Authenticator; +use SilverStripe\Security\DefaultAdminService; +use SilverStripe\Security\IdentityStore; +use SilverStripe\Security\Member; use SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator; use SilverStripe\Security\MemberAuthenticator\CMSMemberLoginForm; use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator; -use SilverStripe\Security\Security; -use SilverStripe\Security\Member; use SilverStripe\Security\MemberAuthenticator\MemberLoginForm; -use SilverStripe\Security\IdentityStore; -use SilverStripe\Core\Config\Config; -use SilverStripe\Dev\SapphireTest; -use SilverStripe\Control\HTTPRequest; -use SilverStripe\Security\DefaultAdminService; +use SilverStripe\Security\Security; class MemberAuthenticatorTest extends SapphireTest { @@ -54,6 +53,8 @@ class MemberAuthenticatorTest extends SapphireTest public function testCustomIdentifierField() { + Member::config()->set('unique_identifier_field', 'Username'); + $label = Member::singleton() ->fieldLabel(Member::config()->get('unique_identifier_field')); @@ -69,7 +70,7 @@ class MemberAuthenticatorTest extends SapphireTest // Create basic login form $frontendResponse = $authenticator ->getLoginHandler($controller->link()) - ->handleRequest(new HTTPRequest('get', '/')); + ->handleRequest(Controller::curr()->getRequest()); $this->assertTrue(is_array($frontendResponse)); $this->assertTrue(isset($frontendResponse['Form'])); diff --git a/tests/php/Security/SecurityTest.php b/tests/php/Security/SecurityTest.php index 446017599..121459106 100644 --- a/tests/php/Security/SecurityTest.php +++ b/tests/php/Security/SecurityTest.php @@ -2,25 +2,22 @@ namespace SilverStripe\Security\Tests; -use SilverStripe\Dev\Debug; +use SilverStripe\Control\Controller; +use SilverStripe\Control\Director; +use SilverStripe\Control\HTTPResponse; +use SilverStripe\Core\Config\Config; +use SilverStripe\Core\Convert; +use SilverStripe\Dev\FunctionalTest; +use SilverStripe\i18n\i18n; use SilverStripe\ORM\DataObject; -use SilverStripe\ORM\FieldType\DBDatetime; -use SilverStripe\ORM\FieldType\DBClassName; use SilverStripe\ORM\DB; +use SilverStripe\ORM\FieldType\DBClassName; +use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\LoginAttempt; use SilverStripe\Security\Member; use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator; use SilverStripe\Security\Security; -use SilverStripe\Core\Config\Config; -use SilverStripe\Core\Convert; -use SilverStripe\Dev\FunctionalTest; -use SilverStripe\Dev\TestOnly; -use SilverStripe\Control\HTTPResponse; -use SilverStripe\Control\Session; -use SilverStripe\Control\Director; -use SilverStripe\Control\Controller; -use SilverStripe\i18n\i18n; /** * Test the security class, including log-in form, change password form, etc @@ -45,11 +42,11 @@ class SecurityTest extends FunctionalTest /** * @skipUpgrade */ - Member::config()->unique_identifier_field = 'Email'; + Member::config()->set('unique_identifier_field', 'Email'); parent::setUp(); - Config::modify()->merge('SilverStripe\\Control\\Director', 'alternate_base_url', '/'); + Director::config()->set('alternate_base_url', '/'); } public function testAccessingAuthenticatedPageRedirectsToLoginForm() @@ -73,10 +70,9 @@ class SecurityTest extends FunctionalTest public function testPermissionFailureSetsCorrectFormMessages() { - Config::nest(); - // Controller that doesn't attempt redirections $controller = new SecurityTest\NullController(); + $controller->setRequest(Controller::curr()->getRequest()); $controller->setResponse(new HTTPResponse()); $session = Controller::curr()->getRequest()->getSession(); @@ -84,7 +80,7 @@ class SecurityTest extends FunctionalTest $this->assertEquals('Oops, not allowed', $session->get('Security.Message.message')); // Test that config values are used correctly - Config::inst()->update(Security::class, 'default_message_set', 'stringvalue'); + Config::modify()->set(Security::class, 'default_message_set', 'stringvalue'); Security::permissionFailure($controller); $this->assertEquals( 'stringvalue', @@ -106,7 +102,7 @@ class SecurityTest extends FunctionalTest // been fetched and output as part of it, so has been removed from the session $this->logInWithPermission('EDITOR'); - Config::inst()->update( + Config::modify()->set( Security::class, 'default_message_set', array('default' => 'default', 'alreadyLoggedIn' => 'You are already logged in!') @@ -127,8 +123,6 @@ class SecurityTest extends FunctionalTest $controller->getResponse()->getBody(), "Message set passed to Security::permissionFailure() didn't override Config values" ); - - Config::unnest(); } /** @@ -200,7 +194,7 @@ class SecurityTest extends FunctionalTest Security::setCurrentUser($member); /* View the Security/login page */ - $response = $this->get(Config::inst()->get(Security::class, 'login_url')); + $this->get(Config::inst()->get(Security::class, 'login_url')); $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm input.action'); @@ -234,7 +228,7 @@ class SecurityTest extends FunctionalTest $this->autoFollowRedirection = true; /* Attempt to get into the admin section */ - $response = $this->get(Config::inst()->get(Security::class, 'login_url')); + $this->get(Config::inst()->get(Security::class, 'login_url')); $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm input.text'); @@ -251,7 +245,7 @@ class SecurityTest extends FunctionalTest { // Test that username does not persist $this->session()->set('SessionForms.MemberLoginForm.Email', 'myuser@silverstripe.com'); - Security::config()->remember_username = false; + Security::config()->set('remember_username', false); $this->get(Config::inst()->get(Security::class, 'login_url')); $items = $this ->cssParser() @@ -265,7 +259,7 @@ class SecurityTest extends FunctionalTest // Test that username does persist when necessary $this->session()->set('SessionForms.MemberLoginForm.Email', 'myuser@silverstripe.com'); - Security::config()->remember_username = true; + Security::config()->set('remember_username', true); $this->get(Config::inst()->get(Security::class, 'login_url')); $items = $this ->cssParser() @@ -373,7 +367,7 @@ class SecurityTest extends FunctionalTest public function testChangePasswordForLoggedInUsers() { - $goodResponse = $this->doTestLoginForm('testuser@example.com', '1nitialPassword'); + $this->doTestLoginForm('testuser@example.com', '1nitialPassword'); // Change the password $this->get('Security/changepassword?BackURL=test/back'); @@ -398,6 +392,7 @@ class SecurityTest extends FunctionalTest public function testChangePasswordFromLostPassword() { + /** @var Member $admin */ $admin = $this->objFromFixture(Member::class, 'test'); $admin->FailedLoginCount = 99; $admin->LockedOutUntil = DBDatetime::now()->getValue(); @@ -406,8 +401,8 @@ class SecurityTest extends FunctionalTest $this->assertNull($admin->AutoLoginHash, 'Hash is empty before lost password'); // Request new password by email - $response = $this->get('Security/lostpassword'); - $response = $this->post('Security/lostpassword/LostPasswordForm', array('Email' => 'testuser@example.com')); + $this->get('Security/lostpassword'); + $this->post('Security/lostpassword/LostPasswordForm', array('Email' => 'testuser@example.com')); $this->assertEmailSent('testuser@example.com'); @@ -427,8 +422,8 @@ class SecurityTest extends FunctionalTest ); // Follow redirection to form without hash in GET parameter - $response = $this->get('Security/changepassword'); - $changedResponse = $this->doTestChangepasswordForm('1nitialPassword', 'changedPassword'); + $this->get('Security/changepassword'); + $this->doTestChangepasswordForm('1nitialPassword', 'changedPassword'); $this->assertEquals($this->idFromFixture(Member::class, 'test'), $this->session()->get('loggedInAs')); // Check if we can login with the new password @@ -444,18 +439,17 @@ class SecurityTest extends FunctionalTest public function testRepeatedLoginAttemptsLockingPeopleOut() { - $local = i18n::get_locale(); i18n::set_locale('en_US'); - - Member::config()->lock_out_after_incorrect_logins = 5; - Member::config()->lock_out_delay_mins = 15; + Member::config()->set('lock_out_after_incorrect_logins', 5); + Member::config()->set('lock_out_delay_mins', 15); // Login with a wrong password for more than the defined threshold - for ($i = 1; $i <= (Member::config()->lock_out_after_incorrect_logins+1); $i++) { + for ($i = 1; $i <= 6; $i++) { $this->doTestLoginForm('testuser@example.com', 'incorrectpassword'); + /** @var Member $member */ $member = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'test')); - if ($i < Member::config()->get('lock_out_after_incorrect_logins')) { + if ($i < 5) { $this->assertNull( $member->LockedOutUntil, 'User does not have a lockout time set if under threshold for failed attempts' @@ -480,7 +474,7 @@ class SecurityTest extends FunctionalTest 'Your account has been temporarily disabled because of too many failed attempts at ' . 'logging in. Please try again in {count} minutes.', null, - array('count' => Member::config()->lock_out_delay_mins) + array('count' => 15) ); $this->assertHasMessage($msg); @@ -506,7 +500,7 @@ class SecurityTest extends FunctionalTest $this->logOut(); // Login again with wrong password, but less attempts than threshold - for ($i = 1; $i < Member::config()->lock_out_after_incorrect_logins; $i++) { + for ($i = 1; $i < 5; $i++) { $this->doTestLoginForm('testuser@example.com', 'incorrectpassword'); } $this->assertNull($this->session()->get('loggedInAs')); @@ -521,13 +515,11 @@ class SecurityTest extends FunctionalTest $member->ID, 'The user can login successfully after lockout expires, if staying below the threshold' ); - - i18n::set_locale($local); } public function testAlternatingRepeatedLoginAttempts() { - Member::config()->lock_out_after_incorrect_logins = 3; + Member::config()->set('lock_out_after_incorrect_logins', 3); // ATTEMPTING LOG-IN TWICE WITH ONE ACCOUNT AND TWICE WITH ANOTHER SHOULDN'T LOCK ANYBODY OUT @@ -537,7 +529,9 @@ class SecurityTest extends FunctionalTest $this->doTestLoginForm('noexpiry@silverstripe.com', 'incorrectpassword'); $this->doTestLoginForm('noexpiry@silverstripe.com', 'incorrectpassword'); + /** @var Member $member1 */ $member1 = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'test')); + /** @var Member $member2 */ $member2 = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'noexpiry')); $this->assertNull($member1->LockedOutUntil); @@ -557,17 +551,18 @@ class SecurityTest extends FunctionalTest public function testUnsuccessfulLoginAttempts() { - Security::config()->login_recording = true; + Security::config()->set('login_recording', true); /* UNSUCCESSFUL ATTEMPTS WITH WRONG PASSWORD FOR EXISTING USER ARE LOGGED */ $this->doTestLoginForm('testuser@example.com', 'wrongpassword'); + /** @var LoginAttempt $attempt */ $attempt = DataObject::get_one( LoginAttempt::class, array( '"LoginAttempt"."Email"' => 'testuser@example.com' ) ); - $this->assertTrue(is_object($attempt)); + $this->assertInstanceOf(LoginAttempt::class, $attempt); $member = DataObject::get_one( Member::class, array( @@ -594,16 +589,18 @@ class SecurityTest extends FunctionalTest public function testSuccessfulLoginAttempts() { - Security::config()->login_recording = true; + Security::config()->set('login_recording', true); /* SUCCESSFUL ATTEMPTS ARE LOGGED */ $this->doTestLoginForm('testuser@example.com', '1nitialPassword'); + /** @var LoginAttempt $attempt */ $attempt = DataObject::get_one( LoginAttempt::class, array( '"LoginAttempt"."Email"' => 'testuser@example.com' ) ); + /** @var Member $member */ $member = DataObject::get_one( Member::class, array( @@ -646,7 +643,7 @@ class SecurityTest extends FunctionalTest public function testDoNotSendEmptyRobotsHeaderIfNotDefined() { - Config::inst()->remove(Security::class, 'robots_tag'); + Config::modify()->remove(Security::class, 'robots_tag'); $response = $this->get(Config::inst()->get(Security::class, 'login_url')); $robotsHeader = $response->getHeader('X-Robots-Tag'); $this->assertNull($robotsHeader); @@ -655,6 +652,11 @@ class SecurityTest extends FunctionalTest /** * Execute a log-in form using Director::test(). * Helper method for the tests above + * + * @param string $email + * @param string $password + * @param string $backURL + * @return HTTPResponse */ public function doTestLoginForm($email, $password, $backURL = 'test/link') { @@ -676,6 +678,10 @@ class SecurityTest extends FunctionalTest /** * Helper method to execute a change password form + * + * @param string $oldPassword + * @param string $newPassword + * @return HTTPResponse */ public function doTestChangepasswordForm($oldPassword, $newPassword) { diff --git a/tests/php/View/RequirementsTest.php b/tests/php/View/RequirementsTest.php index 806320b94..6c73f2f32 100644 --- a/tests/php/View/RequirementsTest.php +++ b/tests/php/View/RequirementsTest.php @@ -86,6 +86,7 @@ class RequirementsTest extends SapphireTest $backend->clearCombinedFiles(); $backend->setCombinedFilesFolder('_combinedfiles'); $backend->setMinifyCombinedFiles(false); + $backend->setCombinedFilesEnabled(true); Requirements::flush(); } @@ -193,6 +194,7 @@ class RequirementsTest extends SapphireTest { /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create(Requirements_Backend::class); + $backend->setCombinedFilesEnabled(true); $this->setupCombinedRequirements($backend); $combinedFileName = '/_combinedfiles/RequirementsTest_bc-2a55d56.js'; diff --git a/tests/php/View/SSViewerCacheBlockTest.php b/tests/php/View/SSViewerCacheBlockTest.php index 3e50fa38c..0e28949a8 100644 --- a/tests/php/View/SSViewerCacheBlockTest.php +++ b/tests/php/View/SSViewerCacheBlockTest.php @@ -3,6 +3,7 @@ namespace SilverStripe\View\Tests; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Core\TempFolder; use SilverStripe\Versioned\Versioned; use Psr\SimpleCache\CacheInterface; use SilverStripe\Dev\SapphireTest; @@ -41,7 +42,7 @@ class SSViewerCacheBlockTest extends SapphireTest $cache = null; if ($cacheOn) { - $cache = new FilesystemCache('cacheblock', 0, getTempFolder()); // cache indefinitely + $cache = new FilesystemCache('cacheblock', 0, TempFolder::getTempFolder(BASE_PATH)); // cache indefinitely } else { $cache = new NullCache(); } diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php index 11a66f7f7..b8b6394b6 100644 --- a/tests/php/View/SSViewerTest.php +++ b/tests/php/View/SSViewerTest.php @@ -2,35 +2,35 @@ namespace SilverStripe\View\Tests; +use Exception; +use InvalidArgumentException; use PHPUnit_Framework_MockObject_MockObject; +use SilverStripe\Assets\Tests\Storage\AssetStoreTest\TestAssetStore; +use SilverStripe\Control\ContentNegotiator; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; -use SilverStripe\Control\ContentNegotiator; use SilverStripe\Control\HTTPResponse; use SilverStripe\Core\Convert; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\SapphireTest; use SilverStripe\i18n\i18n; +use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\FieldType\DBField; -use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\PaginatedList; -use SilverStripe\Security\Member; +use SilverStripe\Security\Permission; use SilverStripe\Security\Security; use SilverStripe\Security\SecurityToken; -use SilverStripe\Security\Permission; use SilverStripe\View\ArrayData; +use SilverStripe\View\Requirements; use SilverStripe\View\Requirements_Backend; use SilverStripe\View\Requirements_Minifier; +use SilverStripe\View\SSTemplateParser; use SilverStripe\View\SSViewer; -use SilverStripe\View\Requirements; +use SilverStripe\View\SSViewer_FromString; use SilverStripe\View\Tests\SSViewerTest\SSViewerTestModel; use SilverStripe\View\Tests\SSViewerTest\SSViewerTestModelController; use SilverStripe\View\ViewableData; -use SilverStripe\View\SSViewer_FromString; -use SilverStripe\View\SSTemplateParser; -use SilverStripe\Assets\Tests\Storage\AssetStoreTest\TestAssetStore; -use Exception; class SSViewerTest extends SapphireTest { @@ -213,8 +213,11 @@ class SSViewerTest extends SapphireTest public function testRequirementsCombine() { + /** @var Requirements_Backend $testBackend */ $testBackend = Injector::inst()->create(Requirements_Backend::class); $testBackend->setSuffixRequirements(false); + $testBackend->setCombinedFilesEnabled(true); + //$combinedTestFilePath = BASE_PATH . '/' . $testBackend->getCombinedFilesFolder() . '/testRequirementsCombine.js'; $jsFile = $this->getCurrentRelativePath() . '/SSViewerTest/javascript/bad.js'; @@ -237,9 +240,12 @@ class SSViewerTest extends SapphireTest public function testRequirementsMinification() { + /** @var Requirements_Backend $testBackend */ $testBackend = Injector::inst()->create(Requirements_Backend::class); $testBackend->setSuffixRequirements(false); $testBackend->setMinifyCombinedFiles(true); + $testBackend->setCombinedFilesEnabled(true); + $testFile = $this->getCurrentRelativePath() . '/SSViewerTest/javascript/RequirementsTest_a.js'; $testFileContent = file_get_contents($testFile); @@ -263,10 +269,8 @@ class SSViewerTest extends SapphireTest ->method('minify'); $testBackend->processCombinedFiles(); - $this->setExpectedExceptionRegExp( - Exception::class, - '/minification service/' - ); + $this->expectException(Exception::class); + $this->expectExceptionMessageRegExp('/^Cannot minify files without a minification service defined./'); $testBackend->setMinifyCombinedFiles(true); $testBackend->setMinifier(null); @@ -1614,8 +1618,8 @@ after' ); // Let's throw something random in there. - $this->setExpectedException('InvalidArgumentException'); - SSViewer::get_templates_by_class(array()); + $this->expectException(InvalidArgumentException::class); + SSViewer::get_templates_by_class(null); } ); } @@ -1732,7 +1736,6 @@ EOC; public function testRenderWithSourceFileComments() { - Director::set_environment_type('dev'); SSViewer::config()->update('source_file_comments', true); $i = __DIR__ . '/SSViewerTest/templates/Includes'; $f = __DIR__ . '/SSViewerTest/templates/SSViewerTestComments'; @@ -1853,13 +1856,13 @@ EOC; Requirements::set_backend($backend); - $this->assertEquals(1, substr_count($template->process(array()), "a.css")); - $this->assertEquals(1, substr_count($template->process(array()), "b.css")); + $this->assertEquals(1, substr_count($template->process(new ViewableData()), "a.css")); + $this->assertEquals(1, substr_count($template->process(new ViewableData()), "b.css")); // if we disable the requirements then we should get nothing $template->includeRequirements(false); - $this->assertEquals(0, substr_count($template->process(array()), "a.css")); - $this->assertEquals(0, substr_count($template->process(array()), "b.css")); + $this->assertEquals(0, substr_count($template->process(new ViewableData()), "a.css")); + $this->assertEquals(0, substr_count($template->process(new ViewableData()), "b.css")); } public function testRequireCallInTemplateInclude() @@ -1873,7 +1876,7 @@ EOC; $this->assertEquals( 1, substr_count( - $template->process(array()), + $template->process(new ViewableData()), "tests/php/View/SSViewerTest/javascript/RequirementsTest_a.js" ) ); diff --git a/tests/php/View/SSViewerTest/TestViewableData.php b/tests/php/View/SSViewerTest/TestViewableData.php index cf9254dc8..78b9d6aef 100644 --- a/tests/php/View/SSViewerTest/TestViewableData.php +++ b/tests/php/View/SSViewerTest/TestViewableData.php @@ -5,6 +5,10 @@ namespace SilverStripe\View\Tests\SSViewerTest; use SilverStripe\Dev\TestOnly; use SilverStripe\View\ViewableData; +/** + * @property string $TextValue + * @property string $HTMLValue + */ class TestViewableData extends ViewableData implements TestOnly {