diff --git a/src/Control/HTTPApplication.php b/src/Control/HTTPApplication.php index 4d0f7cb9d..35f453d6d 100644 --- a/src/Control/HTTPApplication.php +++ b/src/Control/HTTPApplication.php @@ -41,7 +41,7 @@ class HTTPApplication implements Application */ public function handle(HTTPRequest $request) { - $flush = array_key_exists('flush', $request->getVars()) || strpos($request->getURL(), 'dev/build') === 0; + $flush = array_key_exists('flush', $request->getVars()) || ($request->getURL() === 'dev/build'); // Ensure boot is invoked return $this->execute($request, function (HTTPRequest $request) { diff --git a/src/Core/Startup/ConfirmationToken.php b/src/Core/Startup/ConfirmationToken.php new file mode 100644 index 000000000..d23563f46 --- /dev/null +++ b/src/Core/Startup/ConfirmationToken.php @@ -0,0 +1,182 @@ +reloadRequired() || $token->reloadRequiredIfError()) { + $token->suppress(); + $target = $token; + } + } + return $target; + } + + /** + * Generate a local filesystem path to store a token + * + * @param $token + * @return string + */ + protected function pathForToken($token) + { + return TEMP_PATH . DIRECTORY_SEPARATOR . 'token_' . preg_replace('/[^a-z0-9]+/', '', $token); + } + + /** + * Generate a new random token and store it + * + * @return string Token name + */ + protected function genToken() + { + // Generate a new random token (as random as possible) + $rg = new RandomGenerator(); + $token = $rg->randomToken('md5'); + + // Store a file in the session save path (safer than /tmp, as open_basedir might limit that) + file_put_contents($this->pathForToken($token), $token); + + return $token; + } + + /** + * Is the necessary token provided for this parameter? + * A value must be provided for the token + * + * @return bool + */ + public function tokenProvided() + { + return !empty($this->token); + } + + /** + * Validate a token + * + * @param string $token + * @return boolean True if the token is valid + */ + protected function checkToken($token) + { + if (!$token) { + return false; + } + + $file = $this->pathForToken($token); + $content = null; + + if (file_exists($file)) { + $content = file_get_contents($file); + unlink($file); + } + + return $content === $token; + } + + /** + * Get redirect url, excluding querystring + * + * @return string + */ + public function currentURL() + { + return Controller::join_links(Director::baseURL(), $this->request->getURL(false)); + } + + /** + * Forces a reload of the request with the token included + * + * @return HTTPResponse + */ + public function reloadWithToken() + { + $location = $this->redirectURL(); + $locationJS = Convert::raw2js($location); + $locationATT = Convert::raw2att($location); + $body = <<location.href='$locationJS'; + +You are being redirected. If you are not redirected soon, click here to continue +HTML; + + // Build response + $result = new HTTPResponse($body); + $result->redirect($location); + return $result; + } + + /** + * Is this parameter requested without a valid token? + * + * @return bool True if the parameter is given without a valid token + */ + abstract public function reloadRequired(); + + /** + * Check if this token is provided either in the backurl, or directly, + * but without a token + * + * @return bool + */ + abstract public function reloadRequiredIfError(); + + /** + * Suppress the current parameter for the duration of this request + */ + abstract public function suppress(); + + /** + * Determine the querystring parameters to include + * + * @param bool $includeToken Include the token value? + * @return array List of querystring parameters, possibly including token parameter + */ + abstract public function params($includeToken = true); + + /** + * Get redirection URL + * + * @return string + */ + abstract protected function redirectURL(); +} diff --git a/src/Core/Startup/ErrorControlChain.php b/src/Core/Startup/ErrorControlChain.php index f34a2c802..e2d14db65 100644 --- a/src/Core/Startup/ErrorControlChain.php +++ b/src/Core/Startup/ErrorControlChain.php @@ -15,8 +15,7 @@ use Exception; * $chain = new ErrorControlChain(); * $chain->then($callback1)->then($callback2)->thenIfErrored($callback3)->execute(); * - * WARNING: This class is experimental and designed specifically for use pre-startup. - * It will likely be heavily refactored before the release of 3.2 + * @internal This class is designed specifically for use pre-startup and may change without warning */ class ErrorControlChain { diff --git a/src/Core/Startup/ErrorControlChainMiddleware.php b/src/Core/Startup/ErrorControlChainMiddleware.php index bdb5ff0a3..e81444629 100644 --- a/src/Core/Startup/ErrorControlChainMiddleware.php +++ b/src/Core/Startup/ErrorControlChainMiddleware.php @@ -12,6 +12,8 @@ use SilverStripe\Security\Security; /** * Decorates application bootstrapping with errorcontrolchain + * + * @internal This class is designed specifically for use pre-startup and may change without warning */ class ErrorControlChainMiddleware implements HTTPMiddleware { @@ -30,27 +32,42 @@ class ErrorControlChainMiddleware implements HTTPMiddleware $this->application = $application; } + /** + * @param HTTPRequest $request + * @return ConfirmationToken|null + */ + protected function prepareConfirmationTokenIfRequired(HTTPRequest $request) + { + $token = URLConfirmationToken::prepare_tokens(['dev/build'], $request); + + if (!$token) { + $token = ParameterConfirmationToken::prepare_tokens( + ['isTest', 'isDev', 'flush'], + $request + ); + } + + return $token; + } + public function process(HTTPRequest $request, callable $next) { $result = null; // Prepare tokens and execute chain - $reloadToken = ParameterConfirmationToken::prepare_tokens( - ['isTest', 'isDev', 'flush'], - $request - ); + $confirmationToken = $this->prepareConfirmationTokenIfRequired($request); $chain = new ErrorControlChain(); $chain - ->then(function () use ($request, $chain, $reloadToken, $next, &$result) { + ->then(function () use ($request, $chain, $confirmationToken, $next, &$result) { // If no redirection is necessary then we can disable error supression - if (!$reloadToken) { + if (!$confirmationToken) { $chain->setSuppression(false); } try { // Check if a token is requesting a redirect - if ($reloadToken && $reloadToken->reloadRequired()) { - $result = $this->safeReloadWithToken($request, $reloadToken); + if ($confirmationToken && $confirmationToken->reloadRequired()) { + $result = $this->safeReloadWithToken($request, $confirmationToken); } else { // If no reload necessary, process application $result = call_user_func($next, $request); @@ -60,9 +77,9 @@ class ErrorControlChainMiddleware implements HTTPMiddleware } }) // Finally if a token was requested but there was an error while figuring out if it's allowed, do it anyway - ->thenIfErrored(function () use ($reloadToken) { - if ($reloadToken && $reloadToken->reloadRequiredIfError()) { - $result = $reloadToken->reloadWithToken(); + ->thenIfErrored(function () use ($confirmationToken) { + if ($confirmationToken && $confirmationToken->reloadRequiredIfError()) { + $result = $confirmationToken->reloadWithToken(); $result->output(); } }) @@ -85,7 +102,7 @@ class ErrorControlChainMiddleware implements HTTPMiddleware // Ensure session is started $request->getSession()->init($request); - + // Request with ErrorDirector $result = ErrorDirector::singleton()->handleRequestWithToken( $request, @@ -98,7 +115,7 @@ class ErrorControlChainMiddleware implements HTTPMiddleware // Fail and redirect the user to the login page $params = array_merge($request->getVars(), $reloadToken->params(false)); - $backURL = $request->getURL() . '?' . http_build_query($params); + $backURL = $reloadToken->currentURL() . '?' . http_build_query($params); $loginPage = Director::absoluteURL(Security::config()->get('login_url')); $loginPage .= "?BackURL=" . urlencode($backURL); $result = new HTTPResponse(); diff --git a/src/Core/Startup/ErrorDirector.php b/src/Core/Startup/ErrorDirector.php index 3c994c6af..54001fd05 100644 --- a/src/Core/Startup/ErrorDirector.php +++ b/src/Core/Startup/ErrorDirector.php @@ -21,11 +21,11 @@ class ErrorDirector extends Director * Redirect with token if allowed, or null if not allowed * * @param HTTPRequest $request - * @param ParameterConfirmationToken $token + * @param ConfirmationToken $token * @param Kernel $kernel * @return null|HTTPResponse Redirection response, or null if not able to redirect */ - public function handleRequestWithToken(HTTPRequest $request, ParameterConfirmationToken $token, Kernel $kernel) + public function handleRequestWithToken(HTTPRequest $request, ConfirmationToken $token, Kernel $kernel) { Injector::inst()->registerService($request, HTTPRequest::class); diff --git a/src/Core/Startup/ParameterConfirmationToken.php b/src/Core/Startup/ParameterConfirmationToken.php index 1c80db1d0..4e90f1ef7 100644 --- a/src/Core/Startup/ParameterConfirmationToken.php +++ b/src/Core/Startup/ParameterConfirmationToken.php @@ -9,30 +9,21 @@ use SilverStripe\Core\Convert; use SilverStripe\Security\RandomGenerator; /** - * Class ParameterConfirmationToken + * This is used to protect dangerous GET parameters that need to be detected early in the request + * lifecycle by generating a one-time-use token & redirecting with that token included in the + * redirected URL * - * When you need to use a dangerous GET parameter that needs to be set before core/Core.php is - * established, this class takes care of allowing some other code of confirming the parameter, - * by generating a one-time-use token & redirecting with that token included in the redirected URL - * - * WARNING: This class is experimental and designed specifically for use pre-startup. - * It will likely be heavily refactored before the release of 3.2 + * @internal This class is designed specifically for use pre-startup and may change without warning */ -class ParameterConfirmationToken +class ParameterConfirmationToken extends ConfirmationToken { - /** * The name of the parameter * * @var string */ protected $parameterName = null; - - /** - * @var HTTPRequest - */ - protected $request = null; - + /** * The parameter given in the main request * @@ -48,60 +39,6 @@ class ParameterConfirmationToken protected $parameterBackURL = null; /** - * The validated and checked token for this parameter - * - * @var string|null A string value, or null if either not provided or invalid - */ - protected $token = null; - - protected function pathForToken($token) - { - return TEMP_PATH . DIRECTORY_SEPARATOR . 'token_' . preg_replace('/[^a-z0-9]+/', '', $token); - } - - /** - * Generate a new random token and store it - * - * @return string Token name - */ - protected function genToken() - { - // Generate a new random token (as random as possible) - $rg = new RandomGenerator(); - $token = $rg->randomToken('md5'); - - // Store a file in the session save path (safer than /tmp, as open_basedir might limit that) - file_put_contents($this->pathForToken($token), $token); - - return $token; - } - - /** - * Validate a token - * - * @param string $token - * @return boolean True if the token is valid - */ - protected function checkToken($token) - { - if (!$token) { - return false; - } - - $file = $this->pathForToken($token); - $content = null; - - if (file_exists($file)) { - $content = file_get_contents($file); - unlink($file); - } - - return $content == $token; - } - - /** - * Create a new ParameterConfirmationToken - * * @param string $parameterName Name of the querystring parameter to check * @param HTTPRequest $request */ @@ -176,54 +113,23 @@ class ParameterConfirmationToken return $this->parameterBackURL !== null; } - /** - * Is the necessary token provided for this parameter? - * A value must be provided for the token - * - * @return bool - */ - public function tokenProvided() - { - return !empty($this->token); - } - - /** - * Is this parameter requested without a valid token? - * - * @return bool True if the parameter is given without a valid token - */ public function reloadRequired() { return $this->parameterProvided() && !$this->tokenProvided(); } - /** - * Check if this token is provided either in the backurl, or directly, - * but without a token - * - * @return bool - */ public function reloadRequiredIfError() { // Don't reload if token exists return $this->reloadRequired() || $this->existsInReferer(); } - - /** - * Suppress the current parameter by unsetting it from $_GET - */ + public function suppress() { unset($_GET[$this->parameterName]); $this->request->offsetUnset($this->parameterName); } - /** - * Determine the querystring parameters to include - * - * @param bool $includeToken Include the token value as well? - * @return array List of querystring parameters with name and token parameters - */ public function params($includeToken = true) { $params = array( @@ -234,25 +140,7 @@ class ParameterConfirmationToken } return $params; } - - /** - * Get redirect url, excluding querystring - * - * @return string - */ - protected function currentURL() - { - return Controller::join_links( - BASE_URL ?: '/', - $this->request->getURL(false) - ); - } - - /** - * Get redirection URL - * - * @return string - */ + protected function redirectURL() { // If url is encoded via BackURL, defer to home page (prevent redirect to form action) @@ -267,48 +155,4 @@ class ParameterConfirmationToken // Merge get params with current url return Controller::join_links($url, '?' . http_build_query($params)); } - - /** - * Forces a reload of the request with the token included - * - * @return HTTPResponse - */ - public function reloadWithToken() - { - $location = $this->redirectURL(); - $locationJS = Convert::raw2js($location); - $locationATT = Convert::raw2att($location); - $body = <<location.href='$locationJS'; - -You are being redirected. If you are not redirected soon, click here to continue the flush -HTML; - - // Build response - $result = new HTTPResponse($body); - $result->redirect($location); - return $result; - } - - /** - * Given a list of token names, suppress all tokens that have not been validated, and - * return the non-validated token with the highest priority - * - * @param array $keys List of token keys in ascending priority (low to high) - * @param HTTPRequest $request - * @return ParameterConfirmationToken The token container for the unvalidated $key given with the highest priority - */ - public static function prepare_tokens($keys, HTTPRequest $request) - { - $target = null; - foreach ($keys as $key) { - $token = new ParameterConfirmationToken($key, $request); - // Validate this token - if ($token->reloadRequired() || $token->reloadRequiredIfError()) { - $token->suppress(); - $target = $token; - } - } - return $target; - } } diff --git a/src/Core/Startup/URLConfirmationToken.php b/src/Core/Startup/URLConfirmationToken.php new file mode 100644 index 000000000..8176a41b8 --- /dev/null +++ b/src/Core/Startup/URLConfirmationToken.php @@ -0,0 +1,136 @@ +urlToCheck = $urlToCheck; + $this->request = $request; + $this->currentURL = $request->getURL(false); + + $this->tokenParameterName = preg_replace('/[^a-z0-9]/i', '', $urlToCheck) . 'token'; + $this->urlExistsInBackURL = $this->getURLExistsInBackURL($request); + + // If the token provided is valid, mark it as such + $token = $request->getVar($this->tokenParameterName); + if ($this->checkToken($token)) { + $this->token = $token; + } + } + + /** + * @param HTTPRequest $request + * @return bool + */ + protected function getURLExistsInBackURL(HTTPRequest $request) + { + $backURL = $request->getVar('BackURL'); + return (strpos($backURL, $this->urlToCheck) === 0); + } + + /** + * @return bool + */ + protected function urlMatches() + { + return ($this->currentURL === $this->urlToCheck); + } + + /** + * @return string + */ + public function getURLToCheck() + { + return $this->urlToCheck; + } + + /** + * @return bool + */ + public function urlExistsInBackURL() + { + return $this->urlExistsInBackURL; + } + + public function reloadRequired() + { + return $this->urlMatches() && !$this->tokenProvided(); + } + + public function reloadRequiredIfError() + { + return $this->reloadRequired() || $this->urlExistsInBackURL(); + } + + public function suppress() + { + $_SERVER['REQUEST_URI'] = '/'; + $this->request->setURL('/'); + } + + public function params($includeToken = true) + { + $params = []; + if ($includeToken) { + $params[$this->tokenParameterName] = $this->genToken(); + } + + return $params; + } + + public function currentURL() + { + return Controller::join_links(Director::baseURL(), $this->currentURL); + } + + protected function redirectURL() + { + // If url is encoded via BackURL, defer to home page (prevent redirect to form action) + if ($this->urlExistsInBackURL && !$this->urlMatches()) { + $url = BASE_URL ?: '/'; + $params = $this->params(); + } else { + $url = $this->currentURL(); + $params = array_merge($this->request->getVars(), $this->params()); + } + + // Merge get params with current url + return Controller::join_links($url, '?' . http_build_query($params)); + } +} diff --git a/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php b/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php index 5cce89d3e..7df6f6ebf 100644 --- a/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php +++ b/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php @@ -73,4 +73,52 @@ class ErrorControlChainMiddlewareTest extends SapphireTest $this->assertNotContains('?flush=1&flushtoken=', $location); $this->assertContains('Security/login', $location); } + + public function testLiveBuildAdmin() + { + // Mock admin + $adminID = $this->logInWithPermission('ADMIN'); + $this->logOut(); + + // Mock app + $app = new HTTPApplication(new BlankKernel(BASE_PATH)); + $app->getKernel()->setEnvironment(Kernel::LIVE); + + // Test being logged in as admin + $chain = new ErrorControlChainMiddleware($app); + $request = new HTTPRequest('GET', '/dev/build/'); + $request->setSession(new Session(['loggedInAs' => $adminID])); + $result = $chain->process($request, function () { + return null; + }); + + $this->assertInstanceOf(HTTPResponse::class, $result); + $location = $result->getHeader('Location'); + $this->assertContains('/dev/build', $location); + $this->assertContains('?devbuildtoken=', $location); + $this->assertNotContains('Security/login', $location); + } + + public function testLiveBuildUnauthenticated() + { + // Mock app + $app = new HTTPApplication(new BlankKernel(BASE_PATH)); + $app->getKernel()->setEnvironment(Kernel::LIVE); + + // Test being logged in as no one + Security::setCurrentUser(null); + $chain = new ErrorControlChainMiddleware($app); + $request = new HTTPRequest('GET', '/dev/build'); + $request->setSession(new Session(['loggedInAs' => 0])); + $result = $chain->process($request, function () { + return null; + }); + + // Should be directed to login, not to flush + $this->assertInstanceOf(HTTPResponse::class, $result); + $location = $result->getHeader('Location'); + $this->assertNotContains('/dev/build', $location); + $this->assertNotContains('?devbuildtoken=', $location); + $this->assertContains('Security/login', $location); + } } diff --git a/tests/php/Core/Startup/ParameterConfirmationTokenTest.php b/tests/php/Core/Startup/ParameterConfirmationTokenTest.php index 66616433f..e28af8ea5 100644 --- a/tests/php/Core/Startup/ParameterConfirmationTokenTest.php +++ b/tests/php/Core/Startup/ParameterConfirmationTokenTest.php @@ -149,14 +149,14 @@ class ParameterConfirmationTokenTest extends SapphireTest } /** - * currentAbsoluteURL needs to handle base or url being missing, or any combination of slashes. + * currentURL needs to handle base or url being missing, or any combination of slashes. * * There should always be exactly one slash between each part in the result, and any trailing slash * should be preserved. * * @dataProvider dataProviderURLs */ - public function testCurrentAbsoluteURLHandlesSlashes($url) + public function testCurrentURLHandlesSlashes($url) { $this->request->setUrl($url); diff --git a/tests/php/Core/Startup/URLConfirmationTokenTest.php b/tests/php/Core/Startup/URLConfirmationTokenTest.php new file mode 100644 index 000000000..73b07eb13 --- /dev/null +++ b/tests/php/Core/Startup/URLConfirmationTokenTest.php @@ -0,0 +1,148 @@ + 'value']); + $validToken = new StubValidToken('token/test/url', $request); + $this->assertTrue($validToken->urlMatches()); + $this->assertFalse($validToken->urlExistsInBackURL()); + $this->assertTrue($validToken->tokenProvided()); // Actually forced to true for this test + $this->assertFalse($validToken->reloadRequired()); + $this->assertFalse($validToken->reloadRequiredIfError()); + $this->assertStringStartsWith(Controller::join_links(BASE_URL, '/', 'token/test/url'), $validToken->redirectURL()); + } + + public function testTokenWithLeadingSlashInUrl() + { + $request = new HTTPRequest('GET', '/leading/slash/url', []); + $leadingSlash = new StubToken('leading/slash/url', $request); + $this->assertTrue($leadingSlash->urlMatches()); + $this->assertFalse($leadingSlash->urlExistsInBackURL()); + $this->assertFalse($leadingSlash->tokenProvided()); + $this->assertTrue($leadingSlash->reloadRequired()); + $this->assertTrue($leadingSlash->reloadRequiredIfError()); + $this->assertContains('leading/slash/url', $leadingSlash->redirectURL()); + $this->assertContains('leadingslashurltoken', $leadingSlash->redirectURL()); + } + + public function testTokenWithTrailingSlashInUrl() + { + $request = new HTTPRequest('GET', 'trailing/slash/url/', []); + $trailingSlash = new StubToken('trailing/slash/url', $request); + $this->assertTrue($trailingSlash->urlMatches()); + $this->assertFalse($trailingSlash->urlExistsInBackURL()); + $this->assertFalse($trailingSlash->tokenProvided()); + $this->assertTrue($trailingSlash->reloadRequired()); + $this->assertTrue($trailingSlash->reloadRequiredIfError()); + $this->assertContains('trailing/slash/url', $trailingSlash->redirectURL()); + $this->assertContains('trailingslashurltoken', $trailingSlash->redirectURL()); + } + + public function testTokenWithUrlMatchedInBackUrl() + { + $request = new HTTPRequest('GET', '/', ['BackURL' => 'back/url']); + $backUrl = new StubToken('back/url', $request); + $this->assertFalse($backUrl->urlMatches()); + $this->assertTrue($backUrl->urlExistsInBackURL()); + $this->assertFalse($backUrl->tokenProvided()); + $this->assertFalse($backUrl->reloadRequired()); + $this->assertTrue($backUrl->reloadRequiredIfError()); + $home = (BASE_URL ?: '/') . '?'; + $this->assertStringStartsWith($home, $backUrl->redirectURL()); + $this->assertContains('backurltoken', $backUrl->redirectURL()); + } + + public function testUrlSuppressionWhenTokenMissing() + { + // Check suppression + $request = new HTTPRequest('GET', 'test/url', []); + $token = new StubToken('test/url', $request); + $this->assertEquals('test/url', $request->getURL(false)); + $token->suppress(); + $this->assertEquals('', $request->getURL(false)); + } + + public function testPrepareTokens() + { + $request = new HTTPRequest('GET', 'test/url', []); + $token = URLConfirmationToken::prepare_tokens( + [ + 'test/url', + 'test', + 'url' + ], + $request + ); + // Test no invalid tokens + $this->assertEquals('test/url', $token->getURLToCheck()); + $this->assertNotEquals('test/url', $request->getURL(false), 'prepare_tokens() did not suppress URL'); + } + + public function testPrepareTokensDoesntSuppressWhenNotMatched() + { + $request = new HTTPRequest('GET', 'test/url', []); + $token = URLConfirmationToken::prepare_tokens( + ['another/url'], + $request + ); + $this->assertEmpty($token); + $this->assertEquals('test/url', $request->getURL(false), 'prepare_tokens() incorrectly suppressed URL'); + } + + public function testPrepareTokensWithUrlMatchedInBackUrl() + { + // Test backurl token + $request = new HTTPRequest('GET', '/', ['BackURL' => 'back/url']); + $token = URLConfirmationToken::prepare_tokens( + [ 'back/url' ], + $request + ); + $this->assertNotEmpty($token); + $this->assertEquals('back/url', $token->getURLToCheck()); + $this->assertNotEquals('back/url', $request->getURL(false), 'prepare_tokens() did not suppress URL'); + } + + public function dataProviderURLs() + { + return [ + [''], + ['/'], + ['bar'], + ['bar/'], + ['/bar'], + ['/bar/'], + ]; + } + + /** + * currentURL needs to handle base or url being missing, or any combination of slashes. + * + * There should always be exactly one slash between each part in the result, and any trailing slash + * should be preserved. + * + * @dataProvider dataProviderURLs + */ + public function testCurrentURLHandlesSlashes($url) + { + $request = new HTTPRequest('GET', $url, []); + + $token = new StubToken( + 'another/url', + $request + ); + $expected = rtrim(Controller::join_links(BASE_URL, '/', $url), '/') ?: '/'; + $this->assertEquals($expected, $token->currentURL(), "Invalid redirect for request url $url"); + } +} diff --git a/tests/php/Core/Startup/URLConfirmationTokenTest/StubToken.php b/tests/php/Core/Startup/URLConfirmationTokenTest/StubToken.php new file mode 100644 index 000000000..ca08d3e1f --- /dev/null +++ b/tests/php/Core/Startup/URLConfirmationTokenTest/StubToken.php @@ -0,0 +1,27 @@ +