diff --git a/src/Control/Middleware/CanonicalURLMiddleware.php b/src/Control/Middleware/CanonicalURLMiddleware.php index a43dab80a..b108a479e 100644 --- a/src/Control/Middleware/CanonicalURLMiddleware.php +++ b/src/Control/Middleware/CanonicalURLMiddleware.php @@ -34,6 +34,14 @@ class CanonicalURLMiddleware implements HTTPMiddleware */ protected $forceSSL = false; + /** + * Set if we should automatically redirect basic auth requests to HTTPS. A null value (default) will + * cause this property to return the value of the forceSSL property. + * + * @var bool|null + */ + protected $forceBasicAuthToSSL = null; + /** * Redirect type * @@ -137,6 +145,29 @@ class CanonicalURLMiddleware implements HTTPMiddleware return $this; } + /** + * @param bool|null $forceBasicAuth + * @return $this + */ + public function setForceBasicAuthToSSL($forceBasicAuth) + { + $this->forceBasicAuthToSSL = $forceBasicAuth; + return $this; + } + + /** + * @return bool + */ + public function getForceBasicAuthToSSL() + { + // Check if explicitly set + if (isset($this->forceBasicAuthToSSL)) { + return $this->forceBasicAuthToSSL; + } + // If not explicitly set, default to on if ForceSSL is on + return $this->getForceSSL(); + } + /** * Generate response for the given request * @@ -152,7 +183,16 @@ class CanonicalURLMiddleware implements HTTPMiddleware return $redirect; } - return $delegate($request); + /** @var HTTPResponse $response */ + $response = $delegate($request); + if ($this->hasBasicAuthPrompt($response) + && $request->getScheme() !== 'https' + && $this->getForceBasicAuthToSSL() + ) { + return $this->redirectToScheme($request, 'https'); + } + + return $response; } /** @@ -190,14 +230,7 @@ class CanonicalURLMiddleware implements HTTPMiddleware return null; } - // Rebuild url for request - $url = Controller::join_links("{$scheme}://{$host}", Director::baseURL(), $request->getURL(true)); - - // Force redirect - $response = new HTTPResponse(); - $response->redirect($url, $this->getRedirectType()); - HTTP::add_cache_headers($response); - return $response; + return $this->redirectToScheme($request, $scheme, $host); } /** @@ -224,7 +257,7 @@ class CanonicalURLMiddleware implements HTTPMiddleware * Return a valid request, if one is available, or null if none is available * * @param HTTPRequest $request - * @return mixed|null + * @return HTTPRequest|null */ protected function getOrValidateRequest(HTTPRequest $request = null) { @@ -328,4 +361,42 @@ class CanonicalURLMiddleware implements HTTPMiddleware } return empty($enabledEnvs) || in_array(Director::get_environment_type(), $enabledEnvs); } + + /** + * Determine whether the executed middlewares have added a basic authentication prompt + * + * @param HTTPResponse $response + * @return bool + */ + protected function hasBasicAuthPrompt(HTTPResponse $response = null) + { + if (!$response) { + return false; + } + return ($response->getStatusCode() === 401 && $response->getHeader('WWW-Authenticate')); + } + + /** + * Redirect the current URL to the specified HTTP scheme + * + * @param HTTPRequest $request + * @param string $scheme + * @param string $host + * @return HTTPResponse + */ + protected function redirectToScheme(HTTPRequest $request, $scheme, $host = null) + { + if (!$host) { + $host = $request->getHost(); + } + + $url = Controller::join_links("{$scheme}://{$host}", Director::baseURL(), $request->getURL(true)); + + // Force redirect + $response = HTTPResponse::create(); + $response->redirect($url, $this->getRedirectType()); + HTTP::add_cache_headers($response); + + return $response; + } } diff --git a/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php b/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php new file mode 100644 index 000000000..e4e3caa56 --- /dev/null +++ b/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php @@ -0,0 +1,98 @@ +middleware = $this->getMockBuilder(CanonicalURLMiddleware::class) + ->setMethods(['getRedirect']) + ->getMock(); + + $this->middleware->expects($this->any())->method('getRedirect')->willReturn(false); + + $this->middleware->setForceBasicAuthToSSL(true); + } + + public function testHttpsIsForcedForBasicAuth() + { + $this->middleware->expects($this->once())->method('getRedirect'); + + $request = new HTTPRequest('GET', '/'); + $mockResponse = (new HTTPResponse) + ->addHeader('WWW-Authenticate', 'basic') + ->setStatusCode(401); + + $result = $this->middleware->process($request, function () use ($mockResponse) { + return $mockResponse; + }); + + $this->assertNotSame($mockResponse, $result, 'New response is created and returned'); + $this->assertEquals(301, $result->getStatusCode(), 'Basic auth responses are redirected'); + $this->assertContains('https://', $result->getHeader('Location'), 'HTTPS is in the redirect location'); + } + + public function testMiddlewareDelegateIsReturnedWhenBasicAuthRedirectIsDisabled() + { + $this->middleware->expects($this->once())->method('getRedirect'); + $this->middleware->setForceBasicAuthToSSL(false); + + $request = new HTTPRequest('GET', '/'); + $mockResponse = (new HTTPResponse) + ->addHeader('WWW-Authenticate', 'basic') + ->setStatusCode(401); + + $result = $this->middleware->process($request, function () use ($mockResponse) { + return $mockResponse; + }); + $this->assertSame($mockResponse, $result, 'Response returned verbatim with auto redirect disabled'); + } + + public function testMiddlewareDelegateIsReturnedWhenNoBasicAuthIsPresent() + { + $this->middleware->expects($this->once())->method('getRedirect'); + + $request = new HTTPRequest('GET', '/'); + $mockResponse = (new HTTPResponse)->addHeader('Foo', 'bar'); + + $result = $this->middleware->process($request, function () use ($mockResponse) { + return $mockResponse; + }); + + $this->assertSame($mockResponse, $result, 'Non basic-auth responses are returned verbatim'); + } + + public function testGetForceBasicAuthToSSL() + { + $this->middleware->setForceBasicAuthToSSL(null); + + $this->middleware->setForceSSL(true); + $this->assertTrue($this->middleware->getForceBasicAuthToSSL(), 'Default falls over to forceSSL'); + + $this->middleware->setForceSSL(false); + $this->assertFalse($this->middleware->getForceBasicAuthToSSL(), 'Default falls over to forceSSL'); + + $this->middleware->setForceBasicAuthToSSL(true); + $this->assertTrue($this->middleware->getForceBasicAuthToSSL(), 'Explicitly set is returned'); + + $this->middleware->setForceBasicAuthToSSL(false); + $this->middleware->setForceSSL(true); + $this->assertFalse($this->middleware->getForceBasicAuthToSSL(), 'Explicitly set is returned'); + } +}