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/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';