diff --git a/.upgrade.yml b/.upgrade.yml index 74caf9ebc..eb8ba58b0 100644 --- a/.upgrade.yml +++ b/.upgrade.yml @@ -261,7 +261,7 @@ mappings: Cookie_Backend: SilverStripe\Control\Cookie_Backend CookieJar: SilverStripe\Control\CookieJar Director: SilverStripe\Control\Director - FlushRequestFilter: SilverStripe\Control\FlushRequestFilter + FlushRequestFilter: SilverStripe\Control\Middleware\FlushMiddleware HTTP: SilverStripe\Control\HTTP SS_HTTPRequest: SilverStripe\Control\HTTPRequest SS_HTTPResponse: SilverStripe\Control\HTTPResponse @@ -496,8 +496,8 @@ mappings: TestRequestFilter: SilverStripe\Control\Tests\DirectorTest\TestRequestFilter DirectorTestRequest_Controller: SilverStripe\Control\Tests\DirectorTest\TestController FakeController: SilverStripe\Control\Tests\FakeController - FlushRequestFilterTest: SilverStripe\Control\Tests\FlushRequestFilterTest - FlushRequestFilterTest_Flushable: SilverStripe\Control\Tests\FlushRequestFilterTest\TestFlushable + FlushRequestFilterTest: SilverStripe\Control\Tests\FlushMiddlewareTest + FlushRequestFilterTest_Flushable: SilverStripe\Control\Tests\FlushMiddlewareTest\TestFlushable HTTPRequestTest: SilverStripe\Control\Tests\HTTPRequestTest HTTPResponseTest: SilverStripe\Control\Tests\HTTPResponseTest HTTPTest: SilverStripe\Control\Tests\HTTPTest diff --git a/_config/requestprocessors.yml b/_config/requestprocessors.yml index 66846879b..59cce470a 100644 --- a/_config/requestprocessors.yml +++ b/_config/requestprocessors.yml @@ -2,10 +2,17 @@ Name: requestprocessors --- SilverStripe\Core\Injector\Injector: - FlushRequestFilter: - class: SilverStripe\Control\FlushRequestFilter - SilverStripe\Control\RequestProcessor: + SilverStripe\Control\Director: properties: - filters: - - '%$FlushRequestFilter' - + Middlewares: + TrustedProxyMiddleware: %$SilverStripe\Control\Middleware\TrustedProxyMiddleware + AllowedHostsMiddleware: %$SilverStripe\Control\Middleware\AllowedHostsMiddleware + SessionMiddleware: %$SilverStripe\Control\Middleware\SessionMiddleware + RequestProcessor: %$SilverStripe\Control\RequestProcessor + FlushMiddleware: %$SilverStripe\Control\Middleware\FlushMiddleware + SilverStripe\Control\AllowedHostsMiddleware: + properties: + AllowedHosts: "`SS_ALLOWED_HOSTS`" + SilverStripe\Control\TrustedProxyMiddleware: + properties: + TrustedProxyIPs: "`SS_TRUSTED_PROXY_IPS`" diff --git a/_config/security.yml b/_config/security.yml index 8930f15bb..f5af02dd2 100644 --- a/_config/security.yml +++ b/_config/security.yml @@ -18,15 +18,17 @@ SilverStripe\Core\Injector\Injector: alc: %$SilverStripe\Security\MemberAuthenticator\CookieAuthenticationHandler --- Name: coresecurity +After: + - requestprocessors --- SilverStripe\Core\Injector\Injector: - SilverStripe\Security\AuthenticationRequestFilter: + SilverStripe\Control\Director: + properties: + Middlewares: + AuthenticationMiddleware: %$SilverStripe\Security\AuthenticationMiddleware + SilverStripe\Security\AuthenticationMiddleware: properties: AuthenticationHandler: %$SilverStripe\Security\AuthenticationHandler - SilverStripe\Control\RequestProcessor: - properties: - filters: - - %$SilverStripe\Security\AuthenticationRequestFilter SilverStripe\Security\Security: properties: Authenticators: diff --git a/docs/en/00_Getting_Started/03_Environment_Management.md b/docs/en/00_Getting_Started/03_Environment_Management.md index 4af8b9d7e..b56a8bcd6 100644 --- a/docs/en/00_Getting_Started/03_Environment_Management.md +++ b/docs/en/00_Getting_Started/03_Environment_Management.md @@ -80,9 +80,6 @@ SilverStripe core environment variables are listed here, though you're free to d | `SS_ERROR_LOG` | Relative path to the log file. | | `SS_PROTECTED_ASSETS_PATH` | Path to secured assets - defaults to ASSET_PATH/.protected | | `SS_DATABASE_MEMORY` | Used for SQLite3 DBs | -| `SS_TRUSTED_PROXY_PROTOCOL_HEADER` | Used to define the proxy header to be used to determine HTTPS status | -| `SS_TRUSTED_PROXY_IP_HEADER` | Used to define the proxy header to be used to determine request IPs | -| `SS_TRUSTED_PROXY_HOST_HEADER` | Used to define the proxy header to be used to determine the requested host name | | `SS_TRUSTED_PROXY_IPS` | IP address or CIDR range to trust proxy headers from. If left blank no proxy headers are trusted. Can be set to 'none' (trust none) or '*' (trust all) | | `SS_ALLOWED_HOSTS` | A comma deliminated list of hostnames the site is allowed to respond to | | `SS_MANIFESTCACHE` | The manifest cache to use (defaults to file based caching). Must be a CacheInterface or CacheFactory class name | diff --git a/docs/en/02_Developer_Guides/02_Controllers/05_Middlewares.md b/docs/en/02_Developer_Guides/02_Controllers/05_Middlewares.md new file mode 100644 index 000000000..1069cc84c --- /dev/null +++ b/docs/en/02_Developer_Guides/02_Controllers/05_Middlewares.md @@ -0,0 +1,118 @@ +title: HTTP Middlewares +summary: Create objects for modifying request and response objects across controllers. + +# HTTP Middlewares + +HTTP Middlewares allow you to put code that will run before or after. These might be used for +authentication, logging, caching, request processing, and many other purposes. Note this interface +replaces the SilverStripe 3 interface, [api:RequestFilter], which still works but is deprecated. + +To create a middleware class, implement `SilverStripe\Control\HTTPMiddleware` and define the +`process(HTTPRequest $request, callbale $delegate)` method. You can do anything you like in this +method, but to continue normal execution, you should call `$response = $delegate($request)` +at some point in this method. + +In addition, you should return an HTTPResponse object. In normal cases, this should be the +$response object returned by `$delegate`, perhaps with some modification. However, sometimes you +will deliberately return a different response, e.g. an error response or a redirection. + +**mysite/code/CustomMiddleware.php** + + :::php + getHeader('X-Special-Header') !== $this->Secret) { + return new HTTPResponse('You missed the special header', 400); + } + + // You can modify the request before + // For example, this might force JSON responses + $request->addHeader('Accept', 'application/json'); + + // If you want normal behaviour to occur, make sure you call $delegate($request) + $response = $delegate($request); + + // You can modify the response after it has been generated + $response->addHeader('X-Middleware-Applied', 'CustomMiddleware') + + // Don't forget to the return the response! + return $response; + } + } + +Once you have created your middleware class, you must attach it to the Director config to make +use of it. + +## Global middleware + +By adding the service or class name to the Director::Middlewares property via injector, +array, a middleware will be executed on every request: + +**mysite/_config/app.yml** + + + :::yml + --- + Name: myrequestprocessors + After: + - requestprocessors + --- + SilverStripe\Core\Injector\Injector: + SilverStripe\Control\Director: + properties: + Middlewares: + CustomMiddleware: %$CustomMiddleware + + +Because these are service names, you can configure properties into a custom service if you would +like: + +**mysite/_config/app.yml** + + :::yml + SilverStripe\Core\Injector\Injector: + SilverStripe\Control\Director: + properties: + Middlewares: + CustomMiddleware: %$ConfiguredMiddleware + ConfiguredMiddleware: + class: 'CustomMiddleware' + properties: + Secret: "DIFFERENT-ONE" + +## Route-specific middleware + +Alternatively, you can apply middlewares to a specific route. These will be processed after the +global middlewares. You can do this by using the `RequestHandlerMiddlewareAdapter` class +as a replacement for your controller, and register it as a service with a `Middlewares` +property. The controller which does the work should be registered under the +`RequestHandler` property. + +**mysite/_config/app.yml** + + :::yml + SilverStripe\Core\Injector\Injector: + SpecialRouteMiddleware: + class: SilverStripe\Control\Middleware\RequestHandlerMiddlewareAdapter + properties + RequestHandler: %$MyController + Middlewares: + - %$CustomMiddleware + - %$AnotherMiddleware + SilverStripe\Control\Director: + rules: + special\section: + Controller: %$SpecialRouteMiddleware + +## API Documentation + +* [api:SilverStripe\Control\HTTPMiddleware] diff --git a/docs/en/02_Developer_Guides/02_Controllers/05_RequestFilters.md b/docs/en/02_Developer_Guides/02_Controllers/05_RequestFilters.md deleted file mode 100644 index 4761064da..000000000 --- a/docs/en/02_Developer_Guides/02_Controllers/05_RequestFilters.md +++ /dev/null @@ -1,57 +0,0 @@ -title: Request Filters -summary: Create objects for modifying request and response objects across controllers. - -# Request Filters - -[api:RequestFilter] is an interface that provides two key methods. `preRequest` and `postRequest`. These methods are -executed before and after a request occurs to give developers a hook to modify any global state, add request tracking or -perform operations wrapped around responses and request objects. A `RequestFilter` is defined as: - -**mysite/code/CustomRequestFilter.php** - - :::php - General and Core Removed API diff --git a/src/Control/ContentNegotiator.php b/src/Control/ContentNegotiator.php index c3f532cbb..8e5f621c3 100644 --- a/src/Control/ContentNegotiator.php +++ b/src/Control/ContentNegotiator.php @@ -84,7 +84,7 @@ class ContentNegotiator return false; } - if (static::config()->enabled) { + if (static::config()->get('enabled')) { return true; } else { return (substr($response->getBody(), 0, 5) == '<' . '?xml'); @@ -106,7 +106,7 @@ class ContentNegotiator ); $q = array(); if (headers_sent()) { - $chosenFormat = static::config()->default_format; + $chosenFormat = static::config()->get('default_format'); } elseif (isset($_GET['forceFormat'])) { $chosenFormat = $_GET['forceFormat']; } else { diff --git a/src/Control/Director.php b/src/Control/Director.php index f5f2d920f..1977cffa6 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -3,8 +3,10 @@ namespace SilverStripe\Control; use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\Control\Middleware\HTTPMiddlewareAware; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Environment; +use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Kernel; use SilverStripe\Dev\Deprecation; @@ -16,19 +18,21 @@ use SilverStripe\View\TemplateGlobalProvider; /** * Director is responsible for processing URLs, and providing environment information. * - * The most important part of director is {@link Director::direct()}, which is passed a URL and will + * The most important part of director is {@link Director::handleRequest()}, which is passed an HTTPRequest and will * execute the appropriate controller. * * Director also has a number of static methods that provide information about the environment, such as * {@link Director::$environment_type}. * - * @see Director::direct() + * @see Director::handleRequest() * @see Director::$rules * @see Director::$environment_type */ class Director implements TemplateGlobalProvider { use Configurable; + use Injectable; + use HTTPMiddlewareAware; /** * Specifies this url is relative to the base. @@ -100,66 +104,10 @@ class Director implements TemplateGlobalProvider protected static $environment_type; /** - * Process the given URL, creating the appropriate controller and executing it. - * - * Request processing is handled as follows: - * - Director::direct() creates a new HTTPResponse object and passes this to - * Director::handleRequest(). - * - Director::handleRequest($request) checks each of the Director rules and identifies a controller - * to handle this request. - * - Controller::handleRequest($request) is then called. This will find a rule to handle the URL, - * and call the rule handling method. - * - RequestHandler::handleRequest($request) is recursively called whenever a rule handling method - * returns a RequestHandler object. - * - * In addition to request processing, Director will manage the session, and perform the output of - * the actual response to the browser. - * - * @uses handleRequest() rule-lookup logic is handled by this. - * @uses TestController::handleRequest() This handles the page logic for a Director::direct() call. - * @param HTTPRequest $request - * @return HTTPResponse - * @throws HTTPResponse_Exception - */ - public static function direct(HTTPRequest $request) - { - // check allowed hosts - if (getenv('SS_ALLOWED_HOSTS') && !static::is_cli()) { - $allowedHosts = explode(',', getenv('SS_ALLOWED_HOSTS')); - if (!in_array(static::host(), $allowedHosts)) { - return new HTTPResponse('Invalid Host', 400); - } - } - - // Pre-request - $output = RequestProcessor::singleton()->preRequest($request); - if ($output === false) { - return new HTTPResponse(_t(__CLASS__.'.INVALID_REQUEST', 'Invalid request'), 400); - } - - // Generate output - $result = static::handleRequest($request); - - // Save session data. Note that save() will start/resume the session if required. - $request->getSession()->save(); - - // Post-request handling - $postRequest = RequestProcessor::singleton()->postRequest($request, $result); - if ($postRequest === false) { - return new HTTPResponse(_t(__CLASS__ . '.REQUEST_ABORTED', 'Request aborted'), 500); - } - - // Return - return $result; - } - - /** - * Test a URL request, returning a response object. This method is the counterpart of - * Director::direct() that is used in functional testing. It will execute the URL given, and + * Test a URL request, returning a response object. This method is a wrapper around + * Director::handleRequest() to assist with functional testing. It will execute the URL given, and * return the result as an HTTPResponse object. * - * @uses TestController::handleRequest() Handles the page logic for a Director::direct() call. - * * @param string $url The URL to visit. * @param array $postVars The $_POST & $_FILES variables. * @param array|Session $session The {@link Session} object representing the current session. @@ -188,7 +136,7 @@ class Director implements TemplateGlobalProvider ) { return static::mockRequest( function (HTTPRequest $request) { - return static::direct($request); + return Director::singleton()->handleRequest($request); }, $url, $postVars, @@ -341,17 +289,41 @@ class Director implements TemplateGlobalProvider } /** - * Handle an HTTP request, defined with a HTTPRequest object. + * Process the given URL, creating the appropriate controller and executing it. + * + * Request processing is handled as follows: + * - Director::handleRequest($request) checks each of the Director rules and identifies a controller + * to handle this request. + * - Controller::handleRequest($request) is then called. This will find a rule to handle the URL, + * and call the rule handling method. + * - RequestHandler::handleRequest($request) is recursively called whenever a rule handling method + * returns a RequestHandler object. + * + * In addition to request processing, Director will manage the session, and perform the output of + * the actual response to the browser. * - * @skipUpgrade * @param HTTPRequest $request * @return HTTPResponse + * @throws HTTPResponse_Exception */ - protected static function handleRequest(HTTPRequest $request) + public function handleRequest(HTTPRequest $request) { + Injector::inst()->registerService($request, HTTPRequest::class); + $rules = Director::config()->uninherited('rules'); + // Default handler - mo URL rules matched, so return a 404 error. + $handler = function () { + return new HTTPResponse('No URL rule was matched', 404); + }; + foreach ($rules as $pattern => $controllerOptions) { + // Match pattern + $arguments = $request->match($pattern, true); + if ($arguments == false) { + continue; + } + // Normalise route rule if (is_string($controllerOptions)) { if (substr($controllerOptions, 0, 2) == '->') { @@ -360,41 +332,49 @@ class Director implements TemplateGlobalProvider $controllerOptions = array('Controller' => $controllerOptions); } } + $request->setRouteParams($controllerOptions); - // Match pattern - $arguments = $request->match($pattern, true); - if ($arguments !== false) { - $request->setRouteParams($controllerOptions); - // controllerOptions provide some default arguments - $arguments = array_merge($controllerOptions, $arguments); + // controllerOptions provide some default arguments + $arguments = array_merge($controllerOptions, $arguments); - // Pop additional tokens from the tokenizer if necessary - if (isset($controllerOptions['_PopTokeniser'])) { - $request->shift($controllerOptions['_PopTokeniser']); - } + // Pop additional tokens from the tokenizer if necessary + if (isset($controllerOptions['_PopTokeniser'])) { + $request->shift($controllerOptions['_PopTokeniser']); + } - // Handle redirection - if (isset($arguments['Redirect'])) { + // Handler for redirection + if (isset($arguments['Redirect'])) { + $handler = function () use ($arguments) { // Redirection $response = new HTTPResponse(); $response->redirect(static::absoluteURL($arguments['Redirect'])); return $response; - } + }; + break; + } - // Find the controller name - $controller = $arguments['Controller']; - $controllerObj = Injector::inst()->create($controller); + /** @var RequestHandler $controllerObj */ + $controllerObj = Injector::inst()->create($arguments['Controller']); + // Handler for calling a controller + $handler = function (HTTPRequest $request) use ($controllerObj) { try { return $controllerObj->handleRequest($request); } catch (HTTPResponse_Exception $responseException) { return $responseException->getResponse(); } - } + }; + break; } - // No URL rules matched, so return a 404 error. - return new HTTPResponse('No URL rule was matched', 404); + // Call the handler with the configured middlewares + $response = $this->callMiddleware($request, $handler); + + // Note that if a different request was previously registered, this will now be lost + // In these cases it's better to use Kernel::nest() prior to kicking off a nested request + Injector::inst()->unregisterNamedObject(HTTPRequest::class); + + return $response; } /** @@ -481,9 +461,10 @@ class Director implements TemplateGlobalProvider * - SERVER_NAME * - gethostname() * + * @param HTTPRequest $request * @return string */ - public static function host() + public static function host(HTTPRequest $request = null) { // Check if overridden by alternate_base_url if ($baseURL = self::config()->get('alternate_base_url')) { @@ -494,18 +475,9 @@ class Director implements TemplateGlobalProvider } } - // Validate proxy-specific headers - if (TRUSTED_PROXY) { - // Check headers to validate - $headers = getenv('SS_TRUSTED_PROXY_HOST_HEADER') - ? explode(',', getenv('SS_TRUSTED_PROXY_HOST_HEADER')) - : ['HTTP_X_FORWARDED_HOST']; // Backwards compatible defaults - foreach ($headers as $header) { - if (!empty($_SERVER[$header])) { - // Get the first host, in case there's multiple separated through commas - return strtok($_SERVER[$header], ','); - } - } + $request = static::currentRequest($request); + if ($request && ($host = $request->getHeader('Host'))) { + return $host; } // Check given header @@ -530,29 +502,32 @@ class Director implements TemplateGlobalProvider * Returns the domain part of the URL 'http://www.mysite.com'. Returns FALSE is this environment * variable isn't set. * + * @param HTTPRequest $request * @return bool|string */ - public static function protocolAndHost() + public static function protocolAndHost(HTTPRequest $request = null) { - return static::protocol() . static::host(); + return static::protocol($request) . static::host($request); } /** * Return the current protocol that the site is running under. * + * @param HTTPRequest $request * @return string */ - public static function protocol() + public static function protocol(HTTPRequest $request = null) { - return (self::is_https()) ? 'https://' : 'http://'; + return (self::is_https($request)) ? 'https://' : 'http://'; } /** * Return whether the site is running as under HTTPS. * + * @param HTTPRequest $request * @return bool */ - public static function is_https() + public static function is_https(HTTPRequest $request = null) { // Check override from alternate_base_url if ($baseURL = self::config()->uninherited('alternate_base_url')) { @@ -563,26 +538,10 @@ class Director implements TemplateGlobalProvider } } - // See https://en.wikipedia.org/wiki/List_of_HTTP_header_fields - // See https://support.microsoft.com/en-us/kb/307347 - if (TRUSTED_PROXY) { - $headers = getenv('SS_TRUSTED_PROXY_PROTOCOL_HEADER') - ? explode(',', getenv('SS_TRUSTED_PROXY_PROTOCOL_HEADER')) - : ['HTTP_X_FORWARDED_PROTO', 'HTTP_X_FORWARDED_PROTOCOL', 'HTTP_FRONT_END_HTTPS']; - foreach ($headers as $header) { - $headerCompareVal = ($header === 'HTTP_FRONT_END_HTTPS' ? 'on' : 'https'); - if (!empty($_SERVER[$header]) && strtolower($_SERVER[$header]) == $headerCompareVal) { - return true; - } - } - } - - // Check common $_SERVER - if ((!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off')) { - return true; - } - if (isset($_SERVER['SSL'])) { - return true; + // Check the current request + $request = static::currentRequest($request); + if ($request && ($scheme = $request->getScheme())) { + return $scheme === 'https'; } // Check default_base_url @@ -842,9 +801,10 @@ class Director implements TemplateGlobalProvider * Returns the Absolute URL of the site root, embedding the current basic-auth credentials into * the URL. * + * @param HTTPRequest|null $request * @return string */ - public static function absoluteBaseURLWithAuth() + public static function absoluteBaseURLWithAuth(HTTPRequest $request = null) { $login = ""; @@ -852,7 +812,7 @@ class Director implements TemplateGlobalProvider $login = "$_SERVER[PHP_AUTH_USER]:$_SERVER[PHP_AUTH_PW]@"; } - return Director::protocol() . $login . static::host() . Director::baseURL(); + return Director::protocol($request) . $login . static::host($request) . Director::baseURL(); } /** @@ -960,12 +920,14 @@ class Director implements TemplateGlobalProvider * Checks if the current HTTP-Request is an "Ajax-Request" by checking for a custom header set by * jQuery or whether a manually set request-parameter 'ajax' is present. * + * @param HTTPRequest $request * @return bool */ - public static function is_ajax() + public static function is_ajax(HTTPRequest $request = null) { - if (Controller::has_curr()) { - return Controller::curr()->getRequest()->isAjax(); + $request = self::currentRequest($request); + if ($request) { + return $request->isAjax(); } else { return ( isset($_REQUEST['ajax']) || @@ -1046,4 +1008,20 @@ class Director implements TemplateGlobalProvider 'BaseHref' => 'absoluteBaseURL', //@deprecated 3.0 ); } + + /** + * Helper to validate or check the current request object + * + * @param HTTPRequest $request + * @return HTTPRequest Request object if one is both current and valid + */ + protected static function currentRequest(HTTPRequest $request = null) + { + // Ensure we only use a registered HTTPRequest and don't + // incidentally construct a singleton + if (!$request && Injector::inst()->has(HTTPRequest::class)) { + $request = Injector::inst()->get(HTTPRequest::class); + } + return $request; + } } diff --git a/src/Control/HTTPApplication.php b/src/Control/HTTPApplication.php index f3610f113..7f765feb7 100644 --- a/src/Control/HTTPApplication.php +++ b/src/Control/HTTPApplication.php @@ -2,8 +2,8 @@ namespace SilverStripe\Control; +use SilverStripe\Control\Middleware\HTTPMiddlewareAware; use SilverStripe\Core\Application; -use SilverStripe\Control\HTTPMiddleware; use SilverStripe\Core\Kernel; /** @@ -11,10 +11,7 @@ use SilverStripe\Core\Kernel; */ class HTTPApplication implements Application { - /** - * @var HTTPMiddleware[] - */ - protected $middlewares = []; + use HTTPMiddlewareAware; /** * @var Kernel @@ -26,54 +23,6 @@ class HTTPApplication implements Application $this->kernel = $kernel; } - /** - * @return HTTPMiddleware[] - */ - public function getMiddlewares() - { - return $this->middlewares; - } - - /** - * @param HTTPMiddleware[] $middlewares - * @return $this - */ - public function setMiddlewares($middlewares) - { - $this->middlewares = $middlewares; - return $this; - } - - /** - * @param HTTPMiddleware $middleware - * @return $this - */ - public function addMiddleware(HTTPMiddleware $middleware) - { - $this->middlewares[] = $middleware; - return $this; - } - - /** - * Call middleware - * - * @param HTTPRequest $request - * @param callable $last Last config to call - * @return HTTPResponse - */ - protected function callMiddleware(HTTPRequest $request, $last) - { - // Reverse middlewares - $next = $last; - /** @var HTTPMiddleware $middleware */ - foreach (array_reverse($this->getMiddlewares()) as $middleware) { - $next = function ($request) use ($middleware, $next) { - return $middleware->process($request, $next); - }; - } - return call_user_func($next, $request); - } - /** * Get the kernel for this application * @@ -96,9 +45,7 @@ class HTTPApplication implements Application // Ensure boot is invoked return $this->execute($request, function (HTTPRequest $request) { - // Start session and execute - $request->getSession()->init(); - return Director::direct($request); + return Director::singleton()->handleRequest($request); }, $flush); } diff --git a/src/Control/HTTPRequest.php b/src/Control/HTTPRequest.php index 72dc820a9..6c54f8a2c 100644 --- a/src/Control/HTTPRequest.php +++ b/src/Control/HTTPRequest.php @@ -4,6 +4,7 @@ namespace SilverStripe\Control; use ArrayAccess; use BadMethodCallException; +use InvalidArgumentException; use SilverStripe\Core\ClassInfo; use SilverStripe\ORM\ArrayLib; @@ -52,6 +53,20 @@ class HTTPRequest implements ArrayAccess */ protected $httpMethod; + /** + * The URL scheme in lowercase: http or https + * + * @var string + */ + protected $scheme; + + /** + * The client IP address + * + * @var string + */ + protected $ip; + /** * Contains alls HTTP GET parameters passed into this request. * @@ -146,6 +161,7 @@ class HTTPRequest implements ArrayAccess $this->getVars = (array) $getVars; $this->postVars = (array) $postVars; $this->body = $body; + $this->scheme = "http"; } /** @@ -757,58 +773,29 @@ class HTTPRequest implements ArrayAccess } /** - * Returns the client IP address which - * originated this request. + * Returns the client IP address which originated this request. * * @return string */ public function getIP() { - $headerOverrideIP = null; - if (TRUSTED_PROXY) { - $headers = (getenv('SS_TRUSTED_PROXY_IP_HEADER')) ? array(getenv('SS_TRUSTED_PROXY_IP_HEADER')) : null; - if (!$headers) { - // Backwards compatible defaults - $headers = array('HTTP_CLIENT_IP', 'HTTP_X_FORWARDED_FOR'); - } - foreach ($headers as $header) { - if (!empty($_SERVER[$header])) { - $headerOverrideIP = $_SERVER[$header]; - break; - } - } - } - - if ($headerOverrideIP && filter_var($headerOverrideIP, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) { - return $this->getIPFromHeaderValue($headerOverrideIP); - } elseif (isset($_SERVER['REMOTE_ADDR'])) { - return $_SERVER['REMOTE_ADDR']; - } else { - return null; - } + return $this->ip; } /** - * Extract an IP address from a header value that has been obtained. Accepts single IP or comma separated string of - * IPs + * Sets the client IP address which originated this request. + * Use setIPFromHeaderValue if assigning from header value. * - * @param string $headerValue The value from a trusted header - * @return string The IP address + * @param $ip string + * @return $this */ - protected function getIPFromHeaderValue($headerValue) + public function setIP($ip) { - if (strpos($headerValue, ',') !== false) { - //sometimes the IP from a load balancer could be "x.x.x.x, y.y.y.y, z.z.z.z" so we need to find the most - // likely candidate - $ips = explode(',', $headerValue); - foreach ($ips as $ip) { - $ip = trim($ip); - if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE)) { - return $ip; - } - } + if (!filter_var($ip, FILTER_VALIDATE_IP)) { + throw new InvalidArgumentException("Invalid ip $ip"); } - return $headerValue; + $this->ip = $ip; + return $this; } /** @@ -837,6 +824,30 @@ class HTTPRequest implements ArrayAccess return $this->httpMethod; } + /** + * Return the URL scheme (e.g. "http" or "https"). + * Equivalent to PSR-7 getUri()->getScheme() + * + * @return string + */ + public function getScheme() + { + return $this->scheme; + } + + /** + * Set the URL scheme (e.g. "http" or "https"). + * Equivalent to PSR-7 getUri()->getScheme(), + * + * @param string $scheme + * @return $this + */ + public function setScheme($scheme) + { + $this->scheme = $scheme; + return $this; + } + /** * Gets the "real" HTTP method for a request. * @@ -846,7 +857,7 @@ class HTTPRequest implements ArrayAccess * Using GET for the "_method" override is not supported, * as GET should never carry out state changes. * Alternatively you can use a custom HTTP header 'X-HTTP-Method-Override' - * to override the original method in {@link Director::direct()}. + * to override the original method. * The '_method' POST parameter overrules the custom HTTP header. * * @param string $origMethod Original HTTP method from the browser request @@ -857,7 +868,7 @@ class HTTPRequest implements ArrayAccess { if (isset($postVars['_method'])) { if (!in_array(strtoupper($postVars['_method']), array('GET','POST','PUT','DELETE','HEAD'))) { - user_error('Director::direct(): Invalid "_method" parameter', E_USER_ERROR); + user_error('HTTPRequest::detect_method(): Invalid "_method" parameter', E_USER_ERROR); } return strtoupper($postVars['_method']); } else { diff --git a/src/Control/HTTPRequestBuilder.php b/src/Control/HTTPRequestBuilder.php index a30657c72..c1d039cdc 100644 --- a/src/Control/HTTPRequestBuilder.php +++ b/src/Control/HTTPRequestBuilder.php @@ -45,6 +45,17 @@ class HTTPRequestBuilder $input ); + // Set the scheme to HTTPS if needed + if ((!empty($variables['_SERVER']['HTTPS']) && $variables['_SERVER']['HTTPS'] != 'off') + || isset($variables['_SERVER']['SSL'])) { + $request->setScheme('https'); + } + + // Set the client IP + if (!empty($variables['_SERVER']['REMOTE_ADDR'])) { + $request->setIP($variables['_SERVER']['REMOTE_ADDR']); + } + // Add headers $headers = static::extractRequestHeaders($variables['_SERVER']); foreach ($headers as $header => $value) { diff --git a/src/Control/Middleware/AllowedHostsMiddleware.php b/src/Control/Middleware/AllowedHostsMiddleware.php new file mode 100644 index 000000000..f3e50826a --- /dev/null +++ b/src/Control/Middleware/AllowedHostsMiddleware.php @@ -0,0 +1,62 @@ +allowedHosts; + } + + /** + * Sets the list of allowed Host header values + * Can also specify a comma separated list + * + * @param array|string $allowedHosts + * @return $this + */ + public function setAllowedHosts($allowedHosts) + { + if (is_string($allowedHosts)) { + $allowedHosts = preg_split('/ *, */', $allowedHosts); + } + $this->allowedHosts = $allowedHosts; + return $this; + } + + /** + * @inheritdoc + */ + public function process(HTTPRequest $request, callable $delegate) + { + $allowedHosts = $this->getAllowedHosts(); + + // check allowed hosts + if ($allowedHosts + && !Director::is_cli() + && !in_array($request->getHeader('Host'), $allowedHosts) + ) { + return new HTTPResponse('Invalid Host', 400); + } + + return $delegate($request); + } +} diff --git a/src/Control/FlushRequestFilter.php b/src/Control/Middleware/FlushMiddleware.php similarity index 52% rename from src/Control/FlushRequestFilter.php rename to src/Control/Middleware/FlushMiddleware.php index 2f7c0a694..0d8f8b482 100644 --- a/src/Control/FlushRequestFilter.php +++ b/src/Control/Middleware/FlushMiddleware.php @@ -1,27 +1,28 @@ getVars())) { foreach (ClassInfo::implementorsOf(Flushable::class) as $class) { + /** @var Flushable|string $class */ $class::flush(); } } - return true; - } - public function postRequest(HTTPRequest $request, HTTPResponse $response) - { - return true; + return $delegate($request); } } diff --git a/src/Control/HTTPMiddleware.php b/src/Control/Middleware/HTTPMiddleware.php similarity index 79% rename from src/Control/HTTPMiddleware.php rename to src/Control/Middleware/HTTPMiddleware.php index a1be72d93..f8c8a1697 100644 --- a/src/Control/HTTPMiddleware.php +++ b/src/Control/Middleware/HTTPMiddleware.php @@ -1,6 +1,9 @@ middlewares; + } + + /** + * @param HTTPMiddleware[] $middlewares + * @return $this + */ + public function setMiddlewares($middlewares) + { + // Allow nulls in the middlewares array to deal with limitations of yml config + $this->middlewares = array_filter((array)$middlewares); + return $this; + } + + /** + * @param HTTPMiddleware $middleware + * @return $this + */ + public function addMiddleware(HTTPMiddleware $middleware) + { + $this->middlewares[] = $middleware; + return $this; + } + + /** + * Call middleware + * + * @param HTTPRequest $request The request to pass to the middlewares and callback + * @param callable $last The callback to call after all middlewares + * @return HTTPResponse + */ + protected function callMiddleware(HTTPRequest $request, callable $last) + { + // Reverse middlewares + $next = $last; + /** @var HTTPMiddleware $middleware */ + foreach (array_reverse($this->getMiddlewares()) as $middleware) { + $next = function ($request) use ($middleware, $next) { + return $middleware->process($request, $next); + }; + } + return $next($request); + } +} diff --git a/src/Control/Middleware/RequestHandlerMiddlewareAdapter.php b/src/Control/Middleware/RequestHandlerMiddlewareAdapter.php new file mode 100644 index 000000000..561199fb6 --- /dev/null +++ b/src/Control/Middleware/RequestHandlerMiddlewareAdapter.php @@ -0,0 +1,59 @@ +setRequestHandler($handler); + } + parent::__construct(); + } + + public function Link($action = null) + { + return $this->getRequestHandler()->Link($action); + } + + /** + * @return RequestHandler + */ + public function getRequestHandler() + { + return $this->requestHandler; + } + + /** + * @param RequestHandler $requestHandler + * @return $this + */ + public function setRequestHandler(RequestHandler $requestHandler) + { + $this->requestHandler = $requestHandler; + return $this; + } + + public function handleRequest(HTTPRequest $request) + { + return $this->callMiddleware($request, function (HTTPRequest $request) { + $this->setRequest($request); + return $this->getRequestHandler()->handleRequest($request); + }); + } +} diff --git a/src/Control/Middleware/SessionMiddleware.php b/src/Control/Middleware/SessionMiddleware.php new file mode 100644 index 000000000..a1b788d5a --- /dev/null +++ b/src/Control/Middleware/SessionMiddleware.php @@ -0,0 +1,30 @@ +getSession()->init($request); + + // Generate output + $response = $delegate($request); + + // Save session data, even if there was an exception. + // Note that save() will start/resume the session if required. + } finally { + $request->getSession()->save($request); + } + + return $response; + } +} diff --git a/src/Control/Middleware/TrustedProxyMiddleware.php b/src/Control/Middleware/TrustedProxyMiddleware.php new file mode 100644 index 000000000..0d865e6c3 --- /dev/null +++ b/src/Control/Middleware/TrustedProxyMiddleware.php @@ -0,0 +1,237 @@ +trustedProxyIPs; + } + + /** + * Set the comma-separated list of IP ranges that are trusted to provide proxy headers + * Can also be 'none' or '*' (all) + * + * @param string $trustedProxyIPs + * @return $this + */ + public function setTrustedProxyIPs($trustedProxyIPs) + { + $this->trustedProxyIPs = $trustedProxyIPs; + return $this; + } + + /** + * Return the array of headers from which to lookup the hostname + * + * @return array + */ + public function getProxyHostHeaders() + { + return $this->proxyHostHeaders; + } + + /** + * Set the array of headers from which to lookup the hostname. + * + * @param array $proxyHostHeaders + * @return $this + */ + public function setProxyHostHeaders($proxyHostHeaders) + { + $this->proxyHostHeaders = $proxyHostHeaders ?: []; + return $this; + } + + /** + * Return the array of headers from which to lookup the client IP + * + * @return array + */ + public function getProxyIPHeaders() + { + return $this->proxyIPHeaders; + } + + /** + * Set the array of headers from which to lookup the client IP. + * + * @param array $proxyIPHeaders + * @return $this + */ + public function setProxyIPHeaders($proxyIPHeaders) + { + $this->proxyIPHeaders = $proxyIPHeaders ?: []; + return $this; + } + + /** + * Return the array of headers from which to lookup the client scheme (http/https) + * + * @return array + */ + public function getProxySchemeHeaders() + { + return $this->proxySchemeHeaders; + } + + /** + * Set array of headers from which to lookup the client scheme (http/https) + * Can also specify comma-separated list as a single string. + * + * @param array $proxySchemeHeaders + * @return $this + */ + public function setProxySchemeHeaders($proxySchemeHeaders) + { + $this->proxySchemeHeaders = $proxySchemeHeaders ?: []; + return $this; + } + + public function process(HTTPRequest $request, callable $delegate) + { + // If this is a trust proxy + if ($this->isTrustedProxy($request)) { + // Replace host + foreach ($this->getProxyHostHeaders() as $header) { + $hostList = $request->getHeader($header); + if ($hostList) { + $request->addHeader('Host', strtok($hostList, ',')); + break; + } + } + + // Replace scheme + foreach ($this->getProxySchemeHeaders() as $header) { + $headerValue = $request->getHeader($header); + if ($headerValue) { + $request->setScheme(strtolower($headerValue)); + break; + } + } + + // Replace IP + foreach ($this->proxyIPHeaders as $header) { + $headerValue = $request->getHeader($header); + if ($headerValue) { + $ipHeader = $this->getIPFromHeaderValue($headerValue); + if ($ipHeader) { + $request->setIP($ipHeader); + break; + } + } + } + } + + return $delegate($request); + } + + /** + * Determine if the current request is coming from a trusted proxy + * + * @param HTTPRequest $request + * @return bool True if the request's source IP is a trusted proxy + */ + protected function isTrustedProxy(HTTPRequest $request) + { + $trustedIPs = $this->getTrustedProxyIPs(); + + // Disabled + if (empty($trustedIPs) || $trustedIPs === 'none') { + return false; + } + + // Allow all + if ($trustedIPs === '*') { + return true; + } + + // Validate IP address + $ip = $request->getIP(); + if ($ip) { + return IPUtils::checkIP($ip, preg_split('/\s*,\s*/', $trustedIPs)); + } + + return false; + } + + /** + * Extract an IP address from a header value that has been obtained. + * Accepts single IP or comma separated string of IPs + * + * @param string $headerValue The value from a trusted header + * @return string The IP address + */ + protected function getIPFromHeaderValue($headerValue) + { + // Sometimes the IP from a load balancer could be "x.x.x.x, y.y.y.y, z.z.z.z" + // so we need to find the most likely candidate + $ips = preg_split('/\s*,\s*/', $headerValue); + + // Prioritise filters + $filters = [ + FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE, + FILTER_FLAG_NO_PRIV_RANGE, + null + ]; + foreach ($filters as $filter) { + // Find best IP + foreach ($ips as $ip) { + if (filter_var($ip, FILTER_VALIDATE_IP, $filter)) { + return $ip; + } + } + } + return null; + } +} diff --git a/src/Control/RequestFilter.php b/src/Control/RequestFilter.php index 0d64980a8..12b333f56 100644 --- a/src/Control/RequestFilter.php +++ b/src/Control/RequestFilter.php @@ -9,6 +9,7 @@ namespace SilverStripe\Control; * * @author marcus@silverstripe.com.au * @license BSD License http://silverstripe.org/bsd-license/ + * @deprecated 4.0..5.0 Use HTTPMiddleware instead */ interface RequestFilter { diff --git a/src/Control/RequestHandler.php b/src/Control/RequestHandler.php index 86f4460aa..14c06ff53 100644 --- a/src/Control/RequestHandler.php +++ b/src/Control/RequestHandler.php @@ -2,17 +2,17 @@ namespace SilverStripe\Control; +use BadMethodCallException; +use Exception; use InvalidArgumentException; +use ReflectionClass; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\Debug; -use SilverStripe\Security\Security; -use SilverStripe\Security\PermissionFailureException; use SilverStripe\Security\Permission; +use SilverStripe\Security\PermissionFailureException; +use SilverStripe\Security\Security; use SilverStripe\View\ViewableData; -use ReflectionClass; -use Exception; -use BadMethodCallException; /** * This class is the base class of any SilverStripe object that can be used to handle HTTP requests. @@ -46,6 +46,7 @@ use BadMethodCallException; */ class RequestHandler extends ViewableData { + /** * Optional url_segment for this request handler * diff --git a/src/Control/RequestProcessor.php b/src/Control/RequestProcessor.php index a1e0cad1a..c8f8a006a 100644 --- a/src/Control/RequestProcessor.php +++ b/src/Control/RequestProcessor.php @@ -2,22 +2,31 @@ namespace SilverStripe\Control; +use SilverStripe\Control\Middleware\HTTPMiddleware; use SilverStripe\Core\Injector\Injectable; +use SilverStripe\Dev\Deprecation; /** - * Represents a request processer that delegates pre and post request handling to nested request filters + * Middleware that provides back-support for the deprecated RequestFilter API. + * + * @deprecated 4.0..5.0 Use HTTPMiddleware directly instead. */ -class RequestProcessor implements RequestFilter +class RequestProcessor implements HTTPMiddleware { use Injectable; /** * List of currently assigned request filters * - * @var array + * @var RequestFilter[] */ private $filters = array(); + /** + * Construct new RequestFilter with a list of filter objects + * + * @param RequestFilter[] $filters + */ public function __construct($filters = array()) { $this->filters = $filters; @@ -26,32 +35,43 @@ class RequestProcessor implements RequestFilter /** * Assign a list of request filters * - * @param array $filters + * @param RequestFilter[] $filters + * @return $this */ public function setFilters($filters) { $this->filters = $filters; + return $this; } - public function preRequest(HTTPRequest $request) + /** + * @inheritdoc + */ + public function process(HTTPRequest $request, callable $delegate) { + if ($this->filters) { + Deprecation::notice( + '5.0', + 'Deprecated RequestFilters are in use. Apply HTTPMiddleware to Director.middlewares instead.' + ); + } + foreach ($this->filters as $filter) { $res = $filter->preRequest($request); if ($res === false) { - return false; + return new HTTPResponse(_t(__CLASS__.'.INVALID_REQUEST', 'Invalid request'), 400); } } - return null; - } - public function postRequest(HTTPRequest $request, HTTPResponse $response) - { + $response = $delegate($request); + foreach ($this->filters as $filter) { $res = $filter->postRequest($request, $response); if ($res === false) { - return false; + return new HTTPResponse(_t(__CLASS__ . '.REQUEST_ABORTED', 'Request aborted'), 500); } } - return null; + + return $response; } } diff --git a/src/Control/Session.php b/src/Control/Session.php index bf5f3f230..12db7c69a 100644 --- a/src/Control/Session.php +++ b/src/Control/Session.php @@ -143,15 +143,12 @@ class Session /** * Get user agent for this request * + * @param HTTPRequest $request * @return string */ - protected function userAgent() + protected function userAgent(HTTPRequest $request) { - if (isset($_SERVER['HTTP_USER_AGENT'])) { - return $_SERVER['HTTP_USER_AGENT']; - } else { - return ''; - } + return $request->getHeader('User-Agent'); } /** @@ -171,30 +168,34 @@ class Session /** * Init this session instance before usage + * + * @param HTTPRequest $request */ - public function init() + public function init(HTTPRequest $request) { if (!$this->isStarted()) { - $this->start(); + $this->start($request); } // Funny business detected! if (isset($this->data['HTTP_USER_AGENT'])) { - if ($this->data['HTTP_USER_AGENT'] !== $this->userAgent()) { + if ($this->data['HTTP_USER_AGENT'] !== $this->userAgent($request)) { $this->clearAll(); $this->destroy(); - $this->start(); + $this->start($request); } } } /** * Destroy existing session and restart + * + * @param HTTPRequest $request */ - public function restart() + public function restart(HTTPRequest $request) { $this->destroy(); - $this->init(); + $this->init($request); } /** @@ -210,9 +211,9 @@ class Session /** * Begin session * - * @param string $sid + * @param HTTPRequest $request The request for which to start a session */ - public function start($sid = null) + public function start(HTTPRequest $request) { if ($this->isStarted()) { throw new BadMethodCallException("Session has already started"); @@ -223,7 +224,7 @@ class Session $path = Director::baseURL(); } $domain = $this->config()->get('cookie_domain'); - $secure = Director::is_https() && $this->config()->get('cookie_secure'); + $secure = Director::is_https($request) && $this->config()->get('cookie_secure'); $session_path = $this->config()->get('session_store_path'); $timeout = $this->config()->get('timeout'); @@ -255,9 +256,6 @@ class Session session_name('SECSESSID'); } - if ($sid) { - session_id($sid); - } session_start(); $this->data = isset($_SESSION) ? $_SESSION : array(); @@ -470,23 +468,27 @@ class Session /** * Set user agent key + * + * @param HTTPRequest $request */ - public function finalize() + public function finalize(HTTPRequest $request) { - $this->set('HTTP_USER_AGENT', $this->userAgent()); + $this->set('HTTP_USER_AGENT', $this->userAgent($request)); } /** * Save data to session * Only save the changes, so that anyone manipulating $_SESSION directly doesn't get burned. + * + * @param HTTPRequest $request */ - public function save() + public function save(HTTPRequest $request) { if ($this->changedData) { - $this->finalize(); + $this->finalize($request); if (!$this->isStarted()) { - $this->start(); + $this->start($request); } $this->recursivelyApply($this->changedData, $_SESSION); diff --git a/src/Core/Flushable.php b/src/Core/Flushable.php index 94b6674f8..7b30d3d32 100644 --- a/src/Core/Flushable.php +++ b/src/Core/Flushable.php @@ -2,6 +2,8 @@ namespace SilverStripe\Core; +use SilverStripe\Control\Middleware\FlushMiddleware; + /** * Provides an interface for classes to implement their own flushing functionality * whenever flush=1 is requested. @@ -14,7 +16,7 @@ interface Flushable * parameter has been set. Each class that implements Flushable implements * this function which looks after it's own specific flushing functionality. * - * @see FlushRequestFilter + * @see FlushMiddleware */ public static function flush(); } diff --git a/src/Core/Injector/Injector.php b/src/Core/Injector/Injector.php index 6b3f28abd..2156c1969 100644 --- a/src/Core/Injector/Injector.php +++ b/src/Core/Injector/Injector.php @@ -856,6 +856,7 @@ class Injector implements ContainerInterface public function unregisterNamedObject($name) { unset($this->serviceCache[$name]); + unset($this->specs[$name]); return $this; } @@ -879,7 +880,7 @@ class Injector implements ContainerInterface throw new InvalidArgumentException("Global unregistration is not allowed"); } if ($object instanceof $filterClass) { - unset($this->serviceCache[$key]); + $this->unregisterNamedObject($key); break; } } @@ -929,6 +930,11 @@ class Injector implements ContainerInterface */ protected function getNamedService($name, $asSingleton = true, $constructorArgs = []) { + // Allow service names of the form "%$ServiceName" + if (substr($name, 0, 2) == '%$') { + $name = substr($name, 2); + } + // Normalise service / args list($name, $constructorArgs) = $this->normaliseArguments($name, $constructorArgs); diff --git a/src/Core/Startup/ErrorControlChainMiddleware.php b/src/Core/Startup/ErrorControlChainMiddleware.php index d5a10ef2a..0e0d78d8c 100644 --- a/src/Core/Startup/ErrorControlChainMiddleware.php +++ b/src/Core/Startup/ErrorControlChainMiddleware.php @@ -7,7 +7,7 @@ use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Core\Application; -use SilverStripe\Control\HTTPMiddleware; +use SilverStripe\Control\Middleware\HTTPMiddleware; use SilverStripe\Security\Permission; use SilverStripe\Security\Security; @@ -84,7 +84,7 @@ class ErrorControlChainMiddleware implements HTTPMiddleware $this->getApplication()->getKernel()->boot(false); // Ensure session is started - $request->getSession()->init(); + $request->getSession()->init($request); // Next, check if we're in dev mode, or the database doesn't have any security data, or we are admin if (Director::isDev() || !Security::database_is_ready() || Permission::check('ADMIN')) { diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php index e5604faf8..a8455bf3e 100644 --- a/src/Dev/SapphireTest.php +++ b/src/Dev/SapphireTest.php @@ -911,7 +911,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly // Custom application $app->execute($request, function (HTTPRequest $request) { // Start session and execute - $request->getSession()->init(); + $request->getSession()->init($request); // Invalidate classname spec since the test manifest will now pull out new subclasses for each internal class // (e.g. Member will now have various subclasses of DataObjects that implement TestOnly) diff --git a/src/Forms/Form.php b/src/Forms/Form.php index c1bb1dc8b..a7254fd98 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -1032,7 +1032,7 @@ class Form extends ViewableData implements HasRequestHandler * As most browsers only support GET and POST in * form submissions, all other HTTP methods are * added as a hidden field "_method" that - * gets evaluated in {@link Director::direct()}. + * gets evaluated in {@link HTTPRequest::detect_method()}. * See {@link FormMethod()} to get a HTTP method * for safe insertion into a
tag. * diff --git a/src/Security/AuthenticationRequestFilter.php b/src/Security/AuthenticationMiddleware.php similarity index 50% rename from src/Security/AuthenticationRequestFilter.php rename to src/Security/AuthenticationMiddleware.php index c276e8174..525b25277 100644 --- a/src/Security/AuthenticationRequestFilter.php +++ b/src/Security/AuthenticationMiddleware.php @@ -4,12 +4,11 @@ namespace SilverStripe\Security; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; -use SilverStripe\Control\HTTPResponse_Exception; -use SilverStripe\Control\RequestFilter; +use SilverStripe\Control\Middleware\HTTPMiddleware; use SilverStripe\Core\Config\Configurable; use SilverStripe\ORM\ValidationException; -class AuthenticationRequestFilter implements RequestFilter +class AuthenticationMiddleware implements HTTPMiddleware { use Configurable; @@ -40,35 +39,24 @@ class AuthenticationRequestFilter implements RequestFilter * Identify the current user from the request * * @param HTTPRequest $request - * @return bool|void - * @throws HTTPResponse_Exception + * @param callable $delegate + * @return HTTPResponse */ - public function preRequest(HTTPRequest $request) + public function process(HTTPRequest $request, callable $delegate) { - if (!Security::database_is_ready()) { - return; + if (Security::database_is_ready()) { + try { + $this + ->getAuthenticationHandler() + ->authenticateRequest($request); + } catch (ValidationException $e) { + return new HTTPResponse( + "Bad log-in details: " . $e->getMessage(), + 400 + ); + } } - try { - $this - ->getAuthenticationHandler() - ->authenticateRequest($request); - } catch (ValidationException $e) { - throw new HTTPResponse_Exception( - "Bad log-in details: " . $e->getMessage(), - 400 - ); - } - } - - /** - * No-op - * - * @param HTTPRequest $request - * @param HTTPResponse $response - * @return bool|void - */ - public function postRequest(HTTPRequest $request, HTTPResponse $response) - { + return $delegate($request); } } diff --git a/src/Security/Member.php b/src/Security/Member.php index ac778ecba..30cac32d6 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -415,7 +415,7 @@ class Member extends DataObject */ public function beforeMemberLoggedIn() { - // @todo Move to middleware on the AuthenticationRequestFilter IdentityStore + // @todo Move to middleware on the AuthenticationMiddleware IdentityStore $this->extend('beforeMemberLoggedIn'); } diff --git a/src/includes/constants.php b/src/includes/constants.php index 2eee0dacc..7adad4dd7 100644 --- a/src/includes/constants.php +++ b/src/includes/constants.php @@ -73,27 +73,6 @@ if (!getenv('SS_IGNORE_DOT_ENV')) { }); } -/** - * Validate whether the request comes directly from a trusted server or not - * This is necessary to validate whether or not the values of X-Forwarded- - * or Client-IP HTTP headers can be trusted - */ -if (!defined('TRUSTED_PROXY')) { - define('TRUSTED_PROXY', call_user_func(function () { - $trustedIPs = getenv('SS_TRUSTED_PROXY_IPS'); - if (empty($trustedIPs) || $trustedIPs === 'none') { - return false; - } - if ($trustedIPs === '*') { - return true; - } - // Validate IP address - if (isset($_SERVER['REMOTE_ADDR'])) { - return IPUtils::checkIP($_SERVER['REMOTE_ADDR'], explode(',', $trustedIPs)); - } - return false; - })); -} if (!defined('BASE_URL')) { define('BASE_URL', call_user_func(function () { diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index cb2e94b62..e9fd2b741 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -1,5 +1,4 @@ markTestSkipped('Test cannot be run without trusted proxy'); - } + // Trust all IPs for this test + /** @var TrustedProxyMiddleware $trustedProxyMiddleware */ + $trustedProxyMiddleware + = Injector::inst()->get(TrustedProxyMiddleware::class); + $trustedProxyMiddleware->setTrustedProxyIPs('*'); + + // Clear alternate_base_url for this test + Director::config()->remove('alternate_base_url'); + // nothing available $headers = array( 'HTTP_X_FORWARDED_PROTOCOL', 'HTTPS', 'SSL' ); - - $origServer = $_SERVER; - foreach ($headers as $header) { if (isset($_SERVER[$header])) { unset($_SERVER['HTTP_X_FORWARDED_PROTOCOL']); } } - $this->assertFalse(Director::is_https()); + $this->assertEquals( + 'no', + Director::test('TestController/returnIsSSL')->getBody() + ); - $_SERVER['HTTP_X_FORWARDED_PROTOCOL'] = 'https'; - $this->assertTrue(Director::is_https()); + $this->assertEquals( + 'yes', + Director::test( + 'TestController/returnIsSSL', + null, + null, + null, + null, + [ 'X-Forwarded-Protocol' => 'https' ] + )->getBody() + ); - $_SERVER['HTTP_X_FORWARDED_PROTOCOL'] = 'http'; - $this->assertFalse(Director::is_https()); + $this->assertEquals( + 'no', + Director::test( + 'TestController/returnIsSSL', + null, + null, + null, + null, + [ 'X-Forwarded-Protocol' => 'http' ] + )->getBody() + ); - $_SERVER['HTTP_X_FORWARDED_PROTOCOL'] = 'ftp'; - $this->assertFalse(Director::is_https()); - - $_SERVER['HTTP_X_FORWARDED_PROTO'] = 'https'; - $this->assertTrue(Director::is_https()); - - $_SERVER['HTTP_X_FORWARDED_PROTO'] = 'http'; - $this->assertFalse(Director::is_https()); - - $_SERVER['HTTP_X_FORWARDED_PROTO'] = 'ftp'; - $this->assertFalse(Director::is_https()); - - $_SERVER['HTTP_FRONT_END_HTTPS'] = 'On'; - $this->assertTrue(Director::is_https()); - - $_SERVER['HTTP_FRONT_END_HTTPS'] = 'Off'; - $this->assertFalse(Director::is_https()); + $this->assertEquals( + 'no', + Director::test( + 'TestController/returnIsSSL', + null, + null, + null, + null, + [ 'X-Forwarded-Protocol' => 'ftp' ] + )->getBody() + ); // https via HTTPS $_SERVER['HTTPS'] = 'true'; - $this->assertTrue(Director::is_https()); + $this->assertEquals( + 'yes', + Director::test('TestController/returnIsSSL')->getBody() + ); $_SERVER['HTTPS'] = '1'; - $this->assertTrue(Director::is_https()); + $this->assertEquals( + 'yes', + Director::test('TestController/returnIsSSL')->getBody() + ); $_SERVER['HTTPS'] = 'off'; - $this->assertFalse(Director::is_https()); + $this->assertEquals( + 'no', + Director::test('TestController/returnIsSSL')->getBody() + ); // https via SSL $_SERVER['SSL'] = ''; - $this->assertTrue(Director::is_https()); - - $_SERVER = $origServer; + $this->assertEquals( + 'yes', + Director::test('TestController/returnIsSSL')->getBody() + ); } public function testTestIgnoresHashes() @@ -646,4 +677,88 @@ class DirectorTest extends SapphireTest // preCall 'true' will trigger an exception and prevent post call execution $this->assertEquals(2, $filter->postCalls); } + + public function testGlobalMiddleware() + { + $middleware = new DirectorTest\TestMiddleware; + Director::singleton()->setMiddlewares([$middleware]); + + $response = Director::test('some-dummy-url'); + $this->assertEquals(404, $response->getStatusCode()); + + // Both triggered + $this->assertEquals(1, $middleware->preCalls); + $this->assertEquals(1, $middleware->postCalls); + + $middleware->failPost = true; + + $response = Director::test('some-dummy-url'); + $this->assertEquals(500, $response->getStatusCode()); + + // Both triggered + $this->assertEquals(2, $middleware->preCalls); + $this->assertEquals(2, $middleware->postCalls); + + $middleware->failPre = true; + + $response = Director::test('some-dummy-url'); + $this->assertEquals(400, $response->getStatusCode()); + + // Pre triggered, post not + $this->assertEquals(3, $middleware->preCalls); + $this->assertEquals(2, $middleware->postCalls); + } + + public function testRouteSpecificMiddleware() + { + // Inject adapter in place of controller + $specificMiddleware = new DirectorTest\TestMiddleware; + Injector::inst()->registerService($specificMiddleware, 'SpecificMiddleware'); + + // Register adapter as factory for creating this controller + Config::modify()->merge( + Injector::class, + 'ControllerWithMiddleware', + [ + 'class' => RequestHandlerMiddlewareAdapter::class, + 'constructor' => [ + '%$' . TestController::class + ], + 'properties' => [ + 'Middlewares' => [ + '%$SpecificMiddleware', + ], + ], + ] + ); + + // Global middleware + $middleware = new DirectorTest\TestMiddleware; + Director::singleton()->setMiddlewares([ $middleware ]); + + // URL rules, one of which has a specific middleware + Config::modify()->set( + Director::class, + 'rules', + [ + 'url-one' => TestController::class, + 'url-two' => [ + 'Controller' => 'ControllerWithMiddleware', + ], + ] + ); + + // URL without a route-specific middleware + Director::test('url-one'); + + // Only the global middleware triggered + $this->assertEquals(1, $middleware->preCalls); + $this->assertEquals(0, $specificMiddleware->postCalls); + + Director::test('url-two'); + + // Both triggered on the url with the specific middleware applied + $this->assertEquals(2, $middleware->preCalls); + $this->assertEquals(1, $specificMiddleware->postCalls); + } } diff --git a/tests/php/Control/DirectorTest/TestController.php b/tests/php/Control/DirectorTest/TestController.php index e3e4400be..d210ea4de 100644 --- a/tests/php/Control/DirectorTest/TestController.php +++ b/tests/php/Control/DirectorTest/TestController.php @@ -3,6 +3,7 @@ namespace SilverStripe\Control\Tests\DirectorTest; use SilverStripe\Control\Controller; +use SilverStripe\Control\Director; use SilverStripe\Dev\TestOnly; class TestController extends Controller implements TestOnly @@ -22,6 +23,7 @@ class TestController extends Controller implements TestOnly 'returnPostValue', 'returnRequestValue', 'returnCookieValue', + 'returnIsSSL', ); public function returnGetValue($request) @@ -55,4 +57,9 @@ class TestController extends Controller implements TestOnly } return null; } + + public function returnIsSSL() + { + return Director::is_https() ? 'yes': 'no'; + } } diff --git a/tests/php/Control/DirectorTest/TestMiddleware.php b/tests/php/Control/DirectorTest/TestMiddleware.php new file mode 100644 index 000000000..1390e8d48 --- /dev/null +++ b/tests/php/Control/DirectorTest/TestMiddleware.php @@ -0,0 +1,40 @@ +preCalls++; + if ($this->failPre) { + return new HTTPResponse('Fail pre', 400); + } + + $response = $delegate($request); + + $this->postCalls++; + if ($this->failPost) { + return new HTTPResponse('Fail post', 500); + } + + return $response; + } + + public function reset() + { + $this->preCalls = 0; + $this->postCalls = 0; + } +} diff --git a/tests/php/Control/FlushRequestFilterTest.php b/tests/php/Control/FlushMiddlewareTest.php similarity index 74% rename from tests/php/Control/FlushRequestFilterTest.php rename to tests/php/Control/FlushMiddlewareTest.php index 8644cc014..df141f1dd 100644 --- a/tests/php/Control/FlushRequestFilterTest.php +++ b/tests/php/Control/FlushMiddlewareTest.php @@ -2,10 +2,10 @@ namespace SilverStripe\Control\Tests; -use SilverStripe\Control\Tests\FlushRequestFilterTest\TestFlushable; +use SilverStripe\Control\Tests\FlushMiddlewareTest\TestFlushable; use SilverStripe\Dev\FunctionalTest; -class FlushRequestFilterTest extends FunctionalTest +class FlushMiddlewareTest extends FunctionalTest { /** * Assert that classes that implement flushable are called diff --git a/tests/php/Control/FlushRequestFilterTest/TestFlushable.php b/tests/php/Control/FlushMiddlewareTest/TestFlushable.php similarity index 79% rename from tests/php/Control/FlushRequestFilterTest/TestFlushable.php rename to tests/php/Control/FlushMiddlewareTest/TestFlushable.php index c867696f0..7a28aa6ba 100644 --- a/tests/php/Control/FlushRequestFilterTest/TestFlushable.php +++ b/tests/php/Control/FlushMiddlewareTest/TestFlushable.php @@ -1,6 +1,6 @@ assertEquals('home', $req->getURL()); } - public function testGetIPFromHeaderValue() + public function testSetIPFromHeaderValue() { - $req = new HTTPRequest('GET', '/'); + $req = new TrustedProxyMiddleware(); $reflectionMethod = new ReflectionMethod($req, 'getIPFromHeaderValue'); $reflectionMethod->setAccessible(true); diff --git a/tests/php/Control/SessionTest.php b/tests/php/Control/SessionTest.php index afdda1013..7d92c28b5 100644 --- a/tests/php/Control/SessionTest.php +++ b/tests/php/Control/SessionTest.php @@ -4,6 +4,7 @@ namespace SilverStripe\Control\Tests; use SilverStripe\Control\Session; use SilverStripe\Dev\SapphireTest; +use SilverStripe\Control\HTTPRequest; /** * Tests to cover the {@link Session} class @@ -107,20 +108,22 @@ class SessionTest extends SapphireTest public function testUserAgentLockout() { // Set a user agent - $_SERVER['HTTP_USER_AGENT'] = 'Test Agent'; + $req1 = new HTTPRequest('GET', '/'); + $req1->addHeader('User-Agent', 'Test Agent'); // Generate our session $s = new Session(array()); - $s->init(); + $s->init($req1); $s->set('val', 123); - $s->finalize(); + $s->finalize($req1); // Change our UA - $_SERVER['HTTP_USER_AGENT'] = 'Fake Agent'; + $req2 = new HTTPRequest('GET', '/'); + $req2->addHeader('User-Agent', 'Fake Agent'); // Verify the new session reset our values $s2 = new Session($s); - $s2->init(); + $s2->init($req2); $this->assertNotEquals($s2->get('val'), 123); } } diff --git a/tests/php/Core/Injector/InjectorTest.php b/tests/php/Core/Injector/InjectorTest.php index 620df8875..4501bbaa1 100644 --- a/tests/php/Core/Injector/InjectorTest.php +++ b/tests/php/Core/Injector/InjectorTest.php @@ -24,6 +24,7 @@ use SilverStripe\Core\Tests\Injector\InjectorTest\TestObject; use SilverStripe\Core\Tests\Injector\InjectorTest\TestSetterInjections; use SilverStripe\Core\Tests\Injector\InjectorTest\TestStaticInjections; use SilverStripe\Dev\SapphireTest; +use SilverStripe\Dev\TestOnly; use stdClass; define('TEST_SERVICES', __DIR__ . '/AopProxyServiceTest'); @@ -802,10 +803,26 @@ class InjectorTest extends SapphireTest public function testNamedServices() { $injector = new Injector(); - $service = new stdClass(); + $service = new TestObject(); + $service->setSomething('injected'); + // Test registering with non-class name $injector->registerService($service, 'NamedService'); + $this->assertTrue($injector->has('NamedService')); $this->assertEquals($service, $injector->get('NamedService')); + + // Unregister service by name + $injector->unregisterNamedObject('NamedService'); + $this->assertFalse($injector->has('NamedService')); + + // Test registered with class name + $injector->registerService($service); + $this->assertTrue($injector->has(TestObject::class)); + $this->assertEquals($service, $injector->get(TestObject::class)); + + // Unregister service by class + $injector->unregisterNamedObject(TestObject::class); + $this->assertFalse($injector->has(TestObject::class)); } public function testCreateConfiggedObjectWithCustomConstructorArgs()