diff --git a/_config/requestprocessors.yml b/_config/requestprocessors.yml index 7f11a8b7cce..65aa88ef8f0 100644 --- a/_config/requestprocessors.yml +++ b/_config/requestprocessors.yml @@ -11,6 +11,7 @@ SilverStripe\Core\Injector\Injector: SessionMiddleware: '%$SilverStripe\Control\Middleware\SessionMiddleware' RequestProcessorMiddleware: '%$SilverStripe\Control\RequestProcessor' FlushMiddleware: '%$SilverStripe\Control\Middleware\FlushMiddleware' + CanonicalURLMiddleware: '%$SilverStripe\Control\Middleware\CanonicalURLMiddleware' SilverStripe\Control\Middleware\AllowedHostsMiddleware: properties: AllowedHosts: '`SS_ALLOWED_HOSTS`' @@ -37,3 +38,12 @@ After: SilverStripe\Core\Injector\Injector: # Note: If Director config changes, take note it will affect this config too SilverStripe\Core\Startup\ErrorDirector: '%$SilverStripe\Control\Director' +--- +Name: canonicalurls +--- +SilverStripe\Core\Injector\Injector: + SilverStripe\Control\Middleware\CanonicalURLMiddleware: + properties: + ForceSSL: false + ForceWWW: false + diff --git a/src/Control/Director.php b/src/Control/Director.php index 2514e155b58..9e6fa784766 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -3,6 +3,7 @@ namespace SilverStripe\Control; use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\Control\Middleware\CanonicalURLMiddleware; use SilverStripe\Control\Middleware\HTTPMiddlewareAware; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Environment; @@ -242,7 +243,7 @@ public static function mockRequest( // If a port is mentioned in the absolute URL, be sure to add that into the HTTP host $newVars['_SERVER']['HTTP_HOST'] = isset($bits['port']) - ? $bits['host'].':'.$bits['port'] + ? $bits['host'] . ':' . $bits['port'] : $bits['host']; } @@ -697,10 +698,10 @@ public static function is_absolute_url($url) { // Strip off the query and fragment parts of the URL before checking if (($queryPosition = strpos($url, '?')) !== false) { - $url = substr($url, 0, $queryPosition-1); + $url = substr($url, 0, $queryPosition - 1); } if (($hashPosition = strpos($url, '#')) !== false) { - $url = substr($url, 0, $hashPosition-1); + $url = substr($url, 0, $hashPosition - 1); } $colonPosition = strpos($url, ':'); $slashPosition = strpos($url, '/'); @@ -809,7 +810,7 @@ public static function absoluteBaseURLWithAuth(HTTPRequest $request = null) $login = "$_SERVER[PHP_AUTH_USER]:$_SERVER[PHP_AUTH_PW]@"; } - return Director::protocol($request) . $login . static::host($request) . Director::baseURL(); + return Director::protocol($request) . $login . static::host($request) . Director::baseURL(); } /** @@ -855,62 +856,29 @@ protected static function force_redirect($destURL) * * @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 true if already on SSL, false if doesn't match patterns (or cannot redirect) - * @throws HTTPResponse_Exception Throws exception with redirect, if successful + * @param HTTPRequest|null $request Request object to check */ - public static function forceSSL($patterns = null, $secureDomain = null) + public static function forceSSL($patterns = null, $secureDomain = null, HTTPRequest $request = null) { - // Already on SSL - if (static::is_https()) { - return true; - } - - // Can't redirect without a url - if (!isset($_SERVER['REQUEST_URI'])) { - return false; - } - + $handler = CanonicalURLMiddleware::singleton()->setForceSSL(true); if ($patterns) { - $matched = false; - $relativeURL = self::makeRelative(Director::absoluteURL($_SERVER['REQUEST_URI'])); - - // protect portions of the site based on the pattern - foreach ($patterns as $pattern) { - if (preg_match($pattern, $relativeURL)) { - $matched = true; - break; - } - } - if (!$matched) { - return false; - } + $handler->setForceSSLPatterns($patterns); } - - // if an domain is specified, redirect to that instead of the current domain - if (!$secureDomain) { - $secureDomain = static::host(); + if ($secureDomain) { + $handler->setForceSSLDomain($secureDomain); } - $url = 'https://' . $secureDomain . $_SERVER['REQUEST_URI']; - - // Force redirect - self::force_redirect($url); - return true; + $handler->throwRedirectIfNeeded($request); } /** * Force a redirect to a domain starting with "www." + * + * @param HTTPRequest $request */ - public static function forceWWW() + public static function forceWWW(HTTPRequest $request = null) { - if (!Director::isDev() && !Director::isTest() && strpos(static::host(), 'www') !== 0) { - $destURL = str_replace( - Director::protocol(), - Director::protocol() . 'www.', - Director::absoluteURL($_SERVER['REQUEST_URI']) - ); - - self::force_redirect($destURL); - } + $handler = CanonicalURLMiddleware::singleton()->setForceWWW(true); + $handler->throwRedirectIfNeeded($request); } /** @@ -947,7 +915,7 @@ public static function is_cli() * Can also be checked with {@link Director::isDev()}, {@link Director::isTest()}, and * {@link Director::isLive()}. * - * @return bool + * @return string */ public static function get_environment_type() { diff --git a/src/Control/Middleware/CanonicalURLMiddleware.php b/src/Control/Middleware/CanonicalURLMiddleware.php new file mode 100644 index 00000000000..5f5ce025763 --- /dev/null +++ b/src/Control/Middleware/CanonicalURLMiddleware.php @@ -0,0 +1,347 @@ +forceSSLPatterns ?: []; + } + + /** + * @param array $forceSSLPatterns + * @return $this + */ + public function setForceSSLPatterns($forceSSLPatterns) + { + $this->forceSSLPatterns = $forceSSLPatterns; + return $this; + } + + /** + * @return string + */ + public function getForceSSLDomain() + { + return $this->forceSSLDomain; + } + + /** + * @param string $forceSSLDomain + * @return $this + */ + public function setForceSSLDomain($forceSSLDomain) + { + $this->forceSSLDomain = $forceSSLDomain; + return $this; + } + + /** + * @return bool + */ + public function getForceWWW() + { + return $this->forceWWW; + } + + /** + * @param bool $forceWWW + * @return $this + */ + public function setForceWWW($forceWWW) + { + $this->forceWWW = $forceWWW; + return $this; + } + + /** + * @return bool + */ + public function getForceSSL() + { + return $this->forceSSL; + } + + /** + * @param bool $forceSSL + * @return $this + */ + public function setForceSSL($forceSSL) + { + $this->forceSSL = $forceSSL; + return $this; + } + + /** + * Generate response for the given request + * + * @param HTTPRequest $request + * @param callable $delegate + * @return HTTPResponse + */ + public function process(HTTPRequest $request, callable $delegate) + { + // Handle any redirects + $redirect = $this->getRedirect($request); + if ($redirect) { + return $redirect; + } + + return $delegate($request); + } + + /** + * Given request object determine if we should redirect. + * + * @param HTTPRequest $request Pre-validated request object + * @return HTTPResponse|null If a redirect is needed return the response + */ + protected function getRedirect(HTTPRequest $request) + { + // Check global disable + if (!$this->isEnabled()) { + return null; + } + + // Check ssl + $forceSSL = $this->getRedirectForceSSL($request); + if ($forceSSL) { + return $forceSSL; + } + + // Check www + $forceWWW = $this->getRedirectForceWWW($request); + if ($forceWWW) { + return $forceWWW; + } + + return null; + } + + /** + * Handles redirection to canonical urls outside of the main middleware chain + * using HTTPResponseException. + * Will not do anything if a current HTTPRequest isn't available + * + * @param HTTPRequest|null $request Allow HTTPRequest to be used for the base comparison + * @throws HTTPResponse_Exception + */ + public function throwRedirectIfNeeded(HTTPRequest $request = null) + { + $request = $this->getOrValidateRequest($request); + if (!$request) { + return; + } + $response = $this->getRedirect($request); + if ($response) { + throw new HTTPResponse_Exception($response); + } + } + + /** + * Return a valid request, if one is available, or null if none is available + * + * @param HTTPRequest $request + * @return mixed|null + */ + protected function getOrValidateRequest(HTTPRequest $request = null) + { + if ($request instanceof HTTPRequest) { + return $request; + } + if (Injector::inst()->has(HTTPRequest::class)) { + return Injector::inst()->get(HTTPRequest::class); + } + return null; + } + + /** + * Check if a redirect for SSL is necessary + * @param HTTPRequest $request + * @return HTTPResponse|null + */ + protected function getRedirectForceSSL(HTTPRequest $request) + { + // Check if force SSL is enabled + if (!$this->getForceSSL()) { + return null; + } + + // Already on SSL + if (Director::is_https($request)) { + return null; + } + + // Filter redirect based on url + $relativeURL = $request->getURL(true); + $matched = $this->urlMatchesPatterns($relativeURL); + if (!$matched) { + return null; + } + + // If a domain is specified, redirect to that instead of the current domain + $secureDomain = $this->getForceSSLDomain(); + if (!$secureDomain) { + $secureDomain = Director::host($request); + } + $url = Controller::join_links("https://{$secureDomain}", Director::baseURL(), $relativeURL); + + // Force redirect + return $this->createRedirect($url); + } + + /** + * Check if a redirect for www. prefix if necessary + * + * @param HTTPRequest $request + * @return HTTPResponse|null + */ + protected function getRedirectForceWWW(HTTPRequest $request) + { + // Check if force www is enabled + if (!$this->getForceWWW()) { + return null; + } + + // Check if already on www + $host = Director::host($request); + if (strpos($host, 'www') === 0) { + return null; + } + + // Build URL with pre-pended hostname + $protocol = Director::protocol($request); + $url = Controller::join_links("{$protocol}www.{$host}", Director::baseURL(), $request->getURL(true)); + + // Force redirect + return $this->createRedirect($url); + } + + /** + * Create redirect response to the given url + * + * @param string $url + * @param int $code Redirect code + * @return HTTPResponse + */ + protected function createRedirect($url, $code = 301) + { + // Redirect to installer + $response = new HTTPResponse(); + $response->redirect($url, $code); + HTTP::add_cache_headers($response); + return $response; + } + + /** + * Check if the given url matches the SSL patterns + * + * @param string $relativeURL + * @return bool True if any pattern matches, or no patterns are needed + */ + protected function urlMatchesPatterns($relativeURL) + { + // Only check if patterns enabled + $patterns = $this->getForceSSLPatterns(); + if (!$patterns) { + return true; + } + foreach ($patterns as $pattern) { + if (preg_match($pattern, $relativeURL)) { + return true; + } + } + return false; + } + + /** + * Get enabled flag, or list of environments to enable in + * + * @return array|bool + */ + public function getEnabledEnvs() + { + return $this->enabledEnvs; + } + + /** + * @param array|bool $enabledEnvs + * @return $this + */ + public function setEnabledEnvs($enabledEnvs) + { + $this->enabledEnvs = $enabledEnvs; + return $this; + } + + /** + * Ensure this middleware is enabled + */ + protected function isEnabled() + { + // Filter by env vars + $enabledEnvs = $this->getEnabledEnvs(); + if (is_bool($enabledEnvs)) { + return $enabledEnvs; + } + return empty($enabledEnvs) || in_array(Director::get_environment_type(), $enabledEnvs); + } +} diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index 01cc37d71d4..719c6b68c4e 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -7,7 +7,7 @@ use SilverStripe\Control\HTTPRequestBuilder; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; -use SilverStripe\Control\Middleware\HTTPMiddleware; +use SilverStripe\Control\Middleware\CanonicalURLMiddleware; use SilverStripe\Control\Middleware\RequestHandlerMiddlewareAdapter; use SilverStripe\Control\Middleware\TrustedProxyMiddleware; use SilverStripe\Control\RequestProcessor; @@ -30,6 +30,9 @@ protected function setUp() { parent::setUp(); Director::config()->set('alternate_base_url', 'http://www.mysite.com/'); + + // Ensure redirects enabled on all environments + CanonicalURLMiddleware::singleton()->setEnabledEnvs(true); $this->expectedRedirect = null; } @@ -412,19 +415,75 @@ public function testRouteParams() ); } + public function testForceWWW() + { + Director::config()->set('alternate_base_url', 'http://mysite.com/'); + + $this->expectExceptionRedirect('http://www.mysite.com/some-url'); + Director::mockRequest(function ($request) { + Injector::inst()->registerService($request, HTTPRequest::class); + Director::forceWWW(); + }, '/some-url'); + } + + public function testPromisedForceWWW() + { + Director::config()->set('alternate_base_url', 'http://mysite.com/'); + Director::forceWWW(); + + // Flag is set but not redirected yet + $middleware = CanonicalURLMiddleware::singleton(); + $this->assertTrue($middleware->getForceWWW()); + + // Middleware forces the redirection eventually + /** @var HTTPResponse $response */ + $response = Director::mockRequest(function ($request) use ($middleware) { + return $middleware->process($request, function ($request) { + return null; + }); + }, '/some-url'); + + // Middleware returns non-exception redirect + $this->assertEquals('http://www.mysite.com/some-url', $response->getHeader('Location')); + $this->assertEquals(301, $response->getStatusCode()); + } + public function testForceSSLProtectsEntireSite() { $this->expectExceptionRedirect('https://www.mysite.com/some-url'); - Director::mockRequest(function () { + Director::mockRequest(function ($request) { + Injector::inst()->registerService($request, HTTPRequest::class); Director::forceSSL(); }, '/some-url'); } + public function testPromisedForceSSL() + { + Director::forceSSL(); + + // Flag is set but not redirected yet + $middleware = CanonicalURLMiddleware::singleton(); + $this->assertTrue($middleware->getForceSSL()); + + // Middleware forces the redirection eventually + /** @var HTTPResponse $response */ + $response = Director::mockRequest(function ($request) use ($middleware) { + return $middleware->process($request, function ($request) { + return null; + }); + }, '/some-url'); + + // Middleware returns non-exception redirect + $this->assertEquals('https://www.mysite.com/some-url', $response->getHeader('Location')); + $this->assertEquals(301, $response->getStatusCode()); + } + public function testForceSSLOnTopLevelPagePattern() { // Expect admin to trigger redirect $this->expectExceptionRedirect('https://www.mysite.com/admin'); - Director::mockRequest(function () { + Director::mockRequest(function (HTTPRequest $request) { + Injector::inst()->registerService($request, HTTPRequest::class); Director::forceSSL(array('/^admin/')); }, '/admin'); } @@ -433,7 +492,8 @@ public function testForceSSLOnSubPagesPattern() { // Expect to redirect to security login page $this->expectExceptionRedirect('https://www.mysite.com/Security/login'); - Director::mockRequest(function () { + Director::mockRequest(function (HTTPRequest $request) { + Injector::inst()->registerService($request, HTTPRequest::class); Director::forceSSL(array('/^Security/')); }, '/Security/login'); } @@ -441,14 +501,18 @@ public function testForceSSLOnSubPagesPattern() public function testForceSSLWithPatternDoesNotMatchOtherPages() { // Not on same url should not trigger redirect - Director::mockRequest(function () { - $this->assertFalse(Director::forceSSL(array('/^admin/'))); + $response = Director::mockRequest(function (HTTPRequest $request) { + Injector::inst()->registerService($request, HTTPRequest::class); + Director::forceSSL(array('/^admin/')); }, Director::baseURL() . 'normal-page'); + $this->assertNull($response, 'Non-matching patterns do not trigger redirect'); // nested url should not triger redirect either - Director::mockRequest(function () { - $this->assertFalse(Director::forceSSL(array('/^admin/', '/^Security/'))); + $response = Director::mockRequest(function (HTTPRequest $request) { + Injector::inst()->registerService($request, HTTPRequest::class); + Director::forceSSL(array('/^admin/', '/^Security/')); }, Director::baseURL() . 'just-another-page/sub-url'); + $this->assertNull($response, 'Non-matching patterns do not trigger redirect'); } public function testForceSSLAlternateDomain() @@ -456,6 +520,7 @@ public function testForceSSLAlternateDomain() // Ensure that forceSSL throws the appropriate exception $this->expectExceptionRedirect('https://secure.mysite.com/admin'); Director::mockRequest(function (HTTPRequest $request) { + Injector::inst()->registerService($request, HTTPRequest::class); return Director::forceSSL(array('/^admin/'), 'secure.mysite.com'); }, Director::baseURL() . 'admin'); }