diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 7a657a2ec..220fdfea0 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -827,6 +827,7 @@ Usages of `UploadField` will need to be upgraded as below. use SilverStripe\Forms\FieldList; -use SilverStripe\AssetAdmin\Forms\UploadField; +use SilverStripe\Forms\FileHandleField; ++use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\DataObject; class MyClass extends DataObject diff --git a/docs/en/04_Changelogs/rc/4.0.0-rc3.md b/docs/en/04_Changelogs/rc/4.0.0-rc3.md new file mode 100644 index 000000000..3ddb65ae7 --- /dev/null +++ b/docs/en/04_Changelogs/rc/4.0.0-rc3.md @@ -0,0 +1,13 @@ +# 4.0.0-rc3 + + + +## Change Log + +### Bugfixes + + * 2017-11-03 [1929ec46b](https://github.com/silverstripe/silverstripe-framework/commit/1929ec46bba443c870b0a756df791520a485e352) Prevent logOut() from clearing site stage during bootstrapping due to flushed session (Damian Mooyman) + * 2017-11-02 [6a73466b4](https://github.com/silverstripe/silverstripe-framework/commit/6a73466b41a1fbc1947bf2608c5ea35f78934f50) Fix basicauth (Damian Mooyman) + * 2017-11-02 [1b190de7](https://github.com/silverstripe/silverstripe-cms/commit/1b190de7e1844f817caa0f65d3bfe5fdff31d264) Fix regression to cancel draft changes (Christopher Joe) + * 2017-11-02 [a61ce077c](https://github.com/silverstripe/silverstripe-framework/commit/a61ce077c64851fdf5dd4aa041cfd74509590011) Sessions must be destroyed on logout (Daniel Hensby) + * 2017-11-02 [27907304](https://github.com/silverstripe/silverstripe-cms/commit/27907304c16950ccd3b9e4a81e6952dad96ee64b) Ensure we publish pages to update permissions during testing (Damian Mooyman) diff --git a/src/Control/HTTPRequestBuilder.php b/src/Control/HTTPRequestBuilder.php index 7bdad706e..378ea3b10 100644 --- a/src/Control/HTTPRequestBuilder.php +++ b/src/Control/HTTPRequestBuilder.php @@ -99,6 +99,23 @@ class HTTPRequestBuilder $headers['Content-Length'] = $server['CONTENT_LENGTH']; } + // Ensure basic auth is available via headers + if (isset($server['PHP_AUTH_USER']) && isset($server['PHP_AUTH_PW'])) { + // Shift PHP_AUTH_* into headers so they are available via request + $headers['PHP_AUTH_USER'] = $server['PHP_AUTH_USER']; + $headers['PHP_AUTH_PW'] = $server['PHP_AUTH_PW']; + } elseif (!empty($headers['Authorization']) && preg_match('/Basic\s+(.*)$/i', $headers['Authorization'], $matches)) { + // Enable HTTP Basic authentication workaround for PHP running in CGI mode with Apache + // Depending on server configuration the auth header may be in HTTP_AUTHORIZATION or + // REDIRECT_HTTP_AUTHORIZATION + // + // The follow rewrite rule must be in the sites .htaccess file to enable this workaround + // RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + list($name, $password) = explode(':', base64_decode($matches[1])); + $headers['PHP_AUTH_USER'] = $name; + $headers['PHP_AUTH_PW'] = $password; + } + return $headers; } diff --git a/src/Core/Config/Configurable.php b/src/Core/Config/Configurable.php index 57f3173a3..6cc007bdb 100644 --- a/src/Core/Config/Configurable.php +++ b/src/Core/Config/Configurable.php @@ -25,6 +25,7 @@ trait Configurable /** * Get inherited config value * + * @deprecated 5.0 Use ->config()->get() instead * @param string $name * @return mixed */ @@ -48,6 +49,7 @@ trait Configurable /** * Update the config value for a given property * + * @deprecated 5.0 Use ->config()->set() instead * @param string $name * @param mixed $value * @return $this diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php index 6d265cae4..7e14e944c 100644 --- a/src/Dev/FunctionalTest.php +++ b/src/Dev/FunctionalTest.php @@ -100,6 +100,9 @@ class FunctionalTest extends SapphireTest implements TestOnly SSViewer::config()->update('theme_enabled', false); } + // Flush user + $this->logOut(); + // Switch to draft site, if necessary if (static::get_use_draft_site()) { $this->useDraftSite(); @@ -109,8 +112,6 @@ class FunctionalTest extends SapphireTest implements TestOnly // basis. BasicAuth::protect_entire_site(false); - $this->logOut(); - SecurityToken::disable(); } diff --git a/src/Dev/Install/InstallRequirements.php b/src/Dev/Install/InstallRequirements.php index d650a6b49..fce65e1cb 100644 --- a/src/Dev/Install/InstallRequirements.php +++ b/src/Dev/Install/InstallRequirements.php @@ -279,9 +279,9 @@ class InstallRequirements null )); - $this->requireWriteable('mysite/_config/config.yml', array( + $this->requireWriteable('mysite/_config/theme.yml', array( "File permissions", - "Is the mysite/_config/config.yml file writeable?", + "Is the mysite/_config/theme.yml file writeable?", null )); diff --git a/src/Security/BasicAuth.php b/src/Security/BasicAuth.php index c0df0a800..ddb03bfbb 100644 --- a/src/Security/BasicAuth.php +++ b/src/Security/BasicAuth.php @@ -87,22 +87,6 @@ class BasicAuth return true; } - /* - * Enable HTTP Basic authentication workaround for PHP running in CGI mode with Apache - * Depending on server configuration the auth header may be in HTTP_AUTHORIZATION or - * REDIRECT_HTTP_AUTHORIZATION - * - * The follow rewrite rule must be in the sites .htaccess file to enable this workaround - * RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] - */ - $authHeader = $request->getHeader('Authorization'); - $matches = array(); - if ($authHeader && preg_match('/Basic\s+(.*)$/i', $authHeader, $matches)) { - list($name, $password) = explode(':', base64_decode($matches[1])); - $request->addHeader('PHP_AUTH_USER', strip_tags($name)); - $request->addHeader('PHP_AUTH_PW', strip_tags($password)); - } - $member = null; try { diff --git a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php index 152518b77..a5c9ed240 100644 --- a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php @@ -104,6 +104,6 @@ class SessionAuthenticationHandler implements AuthenticationHandler public function logOut(HTTPRequest $request = null) { $request = $request ?: Controller::curr()->getRequest(); - $request->getSession()->clear($this->getSessionVariable()); + $request->getSession()->restart($request); } } diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index b4d8041d4..fa3237545 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -4,7 +4,6 @@ namespace SilverStripe\Control\Tests; use SilverStripe\Control\Cookie_Backend; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; -use SilverStripe\Control\HTTPRequestBuilder; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Control\Middleware\CanonicalURLMiddleware; @@ -691,39 +690,6 @@ class DirectorTest extends SapphireTest } } - /** - * @covers \SilverStripe\Control\HTTPRequestBuilder::extractRequestHeaders() - */ - public function testExtractRequestHeaders() - { - $request = array( - 'REDIRECT_STATUS' => '200', - 'HTTP_HOST' => 'host', - 'HTTP_USER_AGENT' => 'User Agent', - 'HTTP_ACCEPT' => 'text/html', - 'HTTP_ACCEPT_LANGUAGE' => 'en-us', - 'HTTP_COOKIE' => 'MyCookie=1', - 'SERVER_PROTOCOL' => 'HTTP/1.1', - 'REQUEST_METHOD' => 'GET', - 'REQUEST_URI' => '/', - 'SCRIPT_NAME' => FRAMEWORK_DIR . '/main.php', - 'CONTENT_TYPE' => 'text/xml', - 'CONTENT_LENGTH' => 10 - ); - - $headers = array( - 'Host' => 'host', - 'User-Agent' => 'User Agent', - 'Accept' => 'text/html', - 'Accept-Language' => 'en-us', - 'Cookie' => 'MyCookie=1', - 'Content-Type' => 'text/xml', - 'Content-Length' => '10' - ); - - $this->assertEquals($headers, HTTPRequestBuilder::extractRequestHeaders($request)); - } - public function testUnmatchedRequestReturns404() { // Remove non-tested rules diff --git a/tests/php/Control/HTTPRequestBuilderTest.php b/tests/php/Control/HTTPRequestBuilderTest.php new file mode 100644 index 000000000..4d5063c54 --- /dev/null +++ b/tests/php/Control/HTTPRequestBuilderTest.php @@ -0,0 +1,66 @@ + '200', + 'HTTP_HOST' => 'host', + 'HTTP_USER_AGENT' => 'User Agent', + 'HTTP_ACCEPT' => 'text/html', + 'HTTP_ACCEPT_LANGUAGE' => 'en-us', + 'HTTP_COOKIE' => 'MyCookie=1', + 'SERVER_PROTOCOL' => 'HTTP/1.1', + 'REQUEST_METHOD' => 'GET', + 'REQUEST_URI' => '/', + 'SCRIPT_NAME' => FRAMEWORK_DIR . '/main.php', + 'CONTENT_TYPE' => 'text/xml', + 'CONTENT_LENGTH' => 10 + ]; + + $headers = [ + 'Host' => 'host', + 'User-Agent' => 'User Agent', + 'Accept' => 'text/html', + 'Accept-Language' => 'en-us', + 'Cookie' => 'MyCookie=1', + 'Content-Type' => 'text/xml', + 'Content-Length' => '10' + ]; + + $this->assertEquals($headers, HTTPRequestBuilder::extractRequestHeaders($request)); + } + + /** + * Ensure basic auth is properly assigned to request headers + */ + public function testExtractRequestHeadersBasicAuth() + { + $request = [ + 'HTTP_AUTHORIZATION' => 'Basic YWRtaW46cGFzc3dvcmQ=', + ]; + $headers = [ + 'PHP_AUTH_USER' => 'admin', + 'PHP_AUTH_PW' => 'password', + 'Authorization' => 'Basic YWRtaW46cGFzc3dvcmQ=', + ]; + $this->assertEquals($headers, HTTPRequestBuilder::extractRequestHeaders($request)); + + + $request = [ + 'PHP_AUTH_USER' => 'admin', + 'PHP_AUTH_PW' => 'password', + ]; + $headers = [ + 'PHP_AUTH_USER' => 'admin', + 'PHP_AUTH_PW' => 'password', + ]; + $this->assertEquals($headers, HTTPRequestBuilder::extractRequestHeaders($request)); + } +} diff --git a/tests/php/Security/BasicAuthTest.php b/tests/php/Security/BasicAuthTest.php index 33dd1073d..7c9e75645 100644 --- a/tests/php/Security/BasicAuthTest.php +++ b/tests/php/Security/BasicAuthTest.php @@ -36,84 +36,79 @@ class BasicAuthTest extends FunctionalTest // Temp disable is_cli() exemption for tests BasicAuth::config()->set('ignore_cli', false); + + // Reset statics + BasicAuthTest\ControllerSecuredWithPermission::$index_called = false; + BasicAuthTest\ControllerSecuredWithPermission::$post_init_called = false; } public function testBasicAuthEnabledWithoutLogin() { - unset($_SERVER['PHP_AUTH_USER']); - unset($_SERVER['PHP_AUTH_PW']); - $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission'); $this->assertEquals(401, $response->getStatusCode()); } public function testBasicAuthDoesntCallActionOrFurtherInitOnAuthFailure() { - $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); + Director::test('BasicAuthTest_ControllerSecuredWithPermission'); $this->assertFalse(BasicAuthTest\ControllerSecuredWithPermission::$index_called); $this->assertFalse(BasicAuthTest\ControllerSecuredWithPermission::$post_init_called); - $_SERVER['PHP_AUTH_USER'] = 'user-in-mygroup@test.com'; - $_SERVER['PHP_AUTH_PW'] = 'test'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', null, $_SESSION, null, null, $_SERVER); + $headers = [ + 'PHP_AUTH_USER' => 'user-in-mygroup@test.com', + 'PHP_AUTH_PW' => 'test', + ]; + Director::test('BasicAuthTest_ControllerSecuredWithPermission', [], [], null, null, $headers); $this->assertTrue(BasicAuthTest\ControllerSecuredWithPermission::$index_called); $this->assertTrue(BasicAuthTest\ControllerSecuredWithPermission::$post_init_called); - - $_SERVER['PHP_AUTH_USER'] = $origUser; - $_SERVER['PHP_AUTH_PW'] = $origPw; } public function testBasicAuthEnabledWithPermission() { - $origUser = isset($_SERVER['PHP_AUTH_USER']) ? $_SERVER['PHP_AUTH_USER'] : null; - $origPw = isset($_SERVER['PHP_AUTH_PW']) ? $_SERVER['PHP_AUTH_PW'] : null; - - $_SERVER['PHP_AUTH_USER'] = 'user-in-mygroup@test.com'; - $_SERVER['PHP_AUTH_PW'] = 'wrongpassword'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', null, $_SESSION, null, null, $_SERVER); + $headers = [ + 'PHP_AUTH_USER' => 'user-in-mygroup@test.com', + 'PHP_AUTH_PW' => 'wrongpassword', + ]; + $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', [], [], null, null, $headers); $this->assertEquals(401, $response->getStatusCode(), 'Invalid users dont have access'); - $_SERVER['PHP_AUTH_USER'] = 'user-without-groups@test.com'; - $_SERVER['PHP_AUTH_PW'] = 'test'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', null, $_SESSION, null, null, $_SERVER); + $headers = [ + 'PHP_AUTH_USER' => 'user-without-groups@test.com', + 'PHP_AUTH_PW' => 'test', + ]; + $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', [], [], null, null, $headers); $this->assertEquals(401, $response->getStatusCode(), 'Valid user without required permission has no access'); - $_SERVER['PHP_AUTH_USER'] = 'user-in-mygroup@test.com'; - $_SERVER['PHP_AUTH_PW'] = 'test'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', null, $_SESSION, null, null, $_SERVER); + $headers = [ + 'PHP_AUTH_USER' => 'user-in-mygroup@test.com', + 'PHP_AUTH_PW' => 'test', + ]; + $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', [], [], null, null, $headers); $this->assertEquals(200, $response->getStatusCode(), 'Valid user with required permission has access'); - - $_SERVER['PHP_AUTH_USER'] = $origUser; - $_SERVER['PHP_AUTH_PW'] = $origPw; } public function testBasicAuthEnabledWithoutPermission() { - $origUser = isset($_SERVER['PHP_AUTH_USER']) ? $_SERVER['PHP_AUTH_USER'] : null; - $origPw = isset($_SERVER['PHP_AUTH_PW']) ? $_SERVER['PHP_AUTH_PW'] : null; - - $_SERVER['PHP_AUTH_USER'] = 'user-without-groups@test.com'; - $_SERVER['PHP_AUTH_PW'] = 'wrongpassword'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', null, $_SESSION, null, null, $_SERVER); + $headers = [ + 'PHP_AUTH_USER' => 'user-without-groups@test.com', + 'PHP_AUTH_PW' => 'wrongpassword', + ]; + $response = Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', [], [], null, null, $headers); $this->assertEquals(401, $response->getStatusCode(), 'Invalid users dont have access'); - $_SERVER['PHP_AUTH_USER'] = 'user-without-groups@test.com'; - $_SERVER['PHP_AUTH_PW'] = 'test'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', null, $_SESSION, null, null, $_SERVER); + $headers = [ + 'PHP_AUTH_USER' => 'user-without-groups@test.com', + 'PHP_AUTH_PW' => 'test', + ]; + $response = Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', [], [], null, null, $headers); $this->assertEquals(200, $response->getStatusCode(), 'All valid users have access'); - $_SERVER['PHP_AUTH_USER'] = 'user-in-mygroup@test.com'; - $_SERVER['PHP_AUTH_PW'] = 'test'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', null, $_SESSION, null, null, $_SERVER); + $headers = [ + 'PHP_AUTH_USER' => 'user-in-mygroup@test.com', + 'PHP_AUTH_PW' => 'test', + ]; + $response = Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', [], [], null, null, $headers); $this->assertEquals(200, $response->getStatusCode(), 'All valid users have access'); - - $_SERVER['PHP_AUTH_USER'] = $origUser; - $_SERVER['PHP_AUTH_PW'] = $origPw; } public function testBasicAuthFailureIncreasesFailedLoginCount() @@ -123,21 +118,23 @@ class BasicAuthTest extends FunctionalTest $this->assertEquals(0, $check->FailedLoginCount); // First failed attempt - $_SERVER['PHP_AUTH_USER'] = 'failedlogin@test.com'; - $_SERVER['PHP_AUTH_PW'] = 'test'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', null, $_SESSION, null, null, $_SERVER); + $headers = [ + 'PHP_AUTH_USER' => 'failedlogin@test.com', + 'PHP_AUTH_PW' => 'test', + ]; + Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', [], [], null, null, $headers); $check = Member::get()->filter('Email', 'failedlogin@test.com')->first(); $this->assertEquals(1, $check->FailedLoginCount); // Second failed attempt - $_SERVER['PHP_AUTH_PW'] = 'testwrong'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', null, $_SESSION, null, null, $_SERVER); + $headers['PHP_AUTH_PW'] = 'testwrong'; + Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', [], [], null, null, $headers); $check = Member::get()->filter('Email', 'failedlogin@test.com')->first(); $this->assertEquals(2, $check->FailedLoginCount); // successful basic auth should reset failed login count - $_SERVER['PHP_AUTH_PW'] = 'Password'; - $response = Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', null, $_SESSION, null, null, $_SERVER); + $headers['PHP_AUTH_PW'] = 'Password'; + Director::test('BasicAuthTest_ControllerSecuredWithoutPermission', [], [], null, null, $headers); $check = Member::get()->filter('Email', 'failedlogin@test.com')->first(); $this->assertEquals(0, $check->FailedLoginCount); } diff --git a/tests/php/Security/BasicAuthTest/ControllerSecuredWithPermission.php b/tests/php/Security/BasicAuthTest/ControllerSecuredWithPermission.php index 822773694..bf2dc2e6d 100644 --- a/tests/php/Security/BasicAuthTest/ControllerSecuredWithPermission.php +++ b/tests/php/Security/BasicAuthTest/ControllerSecuredWithPermission.php @@ -11,10 +11,9 @@ use SilverStripe\Security\BasicAuth; */ class ControllerSecuredWithPermission extends Controller implements TestOnly { + public static $post_init_called = false; - static $post_init_called = false; - - static $index_called = false; + public static $index_called = false; protected $template = 'BlankPage';