diff --git a/.travis.yml b/.travis.yml index d376baa..5fff154 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,7 +9,6 @@ before_script: - mkdir -p build/logs - composer self-update - travis_retry composer install --prefer-source --no-interaction - - composer require --dev phpstan/phpstan - composer dump-autoload -o script: diff --git a/README.md b/README.md index c17bb2b..36ac5d5 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,7 @@ Features - [x] Enable/disable HTTP Strict Transport Security Header and set its value. - [x] Allow add `www.` prefix during redirection from http or already https. - [x] Allow remove `www.` prefix during redirection from http or already https. +- [x] Force Https for 404 pages Installation ------------ @@ -87,6 +88,8 @@ return [ // remove existing "www." prefix during redirection from http or already https // only works if previous's config 'add_www_prefix' => false 'remove_www_prefix' => false, + // Force Https for 404 pages + 'allow_404' => true, ], // ... ]; diff --git a/composer.json b/composer.json index b06f3bb..a727078 100644 --- a/composer.json +++ b/composer.json @@ -26,6 +26,7 @@ ], "require": { "php": "^7.1", + "webmozart/assert": "^1.4", "zendframework/zend-console": "^2.5" }, "conflict": { @@ -35,11 +36,14 @@ "require-dev": { "kahlan/kahlan": "^4.0", "php-coveralls/php-coveralls": "^2.1", + "phpstan/phpstan": "^0.10.8", + "phpstan/phpstan-webmozart-assert": "^0.10.0", "zendframework/zend-expressive": "^3.0", "zendframework/zend-mvc": "^3.0" }, "config": { - "bin-dir": "bin" + "bin-dir": "bin", + "sort-packages": true }, "extra": { "zf": { diff --git a/config/expressive-force-https-module.local.php.dist b/config/expressive-force-https-module.local.php.dist index 43ef745..6e5169a 100644 --- a/config/expressive-force-https-module.local.php.dist +++ b/config/expressive-force-https-module.local.php.dist @@ -17,6 +17,7 @@ return [ ], 'add_www_prefix' => false, 'remove_www_prefix' => false, + 'allow_404' => true, ], 'dependencies' => [ diff --git a/config/force-https-module.local.php.dist b/config/force-https-module.local.php.dist index e9ba2cb..3399952 100644 --- a/config/force-https-module.local.php.dist +++ b/config/force-https-module.local.php.dist @@ -15,5 +15,6 @@ return [ ], 'add_www_prefix' => false, 'remove_www_prefix' => false, + 'allow_404' => true, ], ]; diff --git a/config/module.config.php b/config/module.config.php index 561d7b4..21f985e 100644 --- a/config/module.config.php +++ b/config/module.config.php @@ -5,7 +5,8 @@ return [ 'service_manager' => [ 'factories' => [ - Listener\ForceHttps::class => Listener\ForceHttpsFactory::class, + Listener\ForceHttps::class => Listener\ForceHttpsFactory::class, + Listener\NotFoundLoggingListenerOnSharedEventManager::class => Listener\NotFoundLoggingListenerOnSharedEventManagerFactory::class, ], ], 'listeners' => [ diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..ed32266 --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,3 @@ +includes: + - vendor/phpstan/phpstan-webmozart-assert/extension.neon + diff --git a/spec/Listener/ForceHttpsSpec.php b/spec/Listener/ForceHttpsSpec.php index 870b72a..9410f86 100644 --- a/spec/Listener/ForceHttpsSpec.php +++ b/spec/Listener/ForceHttpsSpec.php @@ -10,7 +10,6 @@ use Zend\EventManager\EventManagerInterface; use Zend\Http\PhpEnvironment\Request; use Zend\Http\PhpEnvironment\Response; -use Zend\Mvc\Application; use Zend\Mvc\MvcEvent; use Zend\Router\RouteMatch; use Zend\Uri\Uri; @@ -35,6 +34,7 @@ $listener->attach($this->eventManager); expect($this->eventManager)->not->toReceive('attach')->with(MvcEvent::EVENT_ROUTE, [$listener, 'forceHttpsScheme']); + expect($this->eventManager)->not->toReceive('attach')->with(MvcEvent::EVENT_DISPATCH_ERROR, [$listener, 'forceHttpsScheme'], 1000); }); @@ -50,6 +50,7 @@ $listener->attach($this->eventManager); expect($this->eventManager)->not->toReceive('attach')->with(MvcEvent::EVENT_ROUTE, [$listener, 'forceHttpsScheme']); + expect($this->eventManager)->not->toReceive('attach')->with(MvcEvent::EVENT_DISPATCH_ERROR, [$listener, 'forceHttpsScheme'], 1000); }); @@ -63,9 +64,11 @@ ]); allow($this->eventManager)->toReceive('attach')->with(MvcEvent::EVENT_ROUTE, [$listener, 'forceHttpsScheme']); + allow($this->eventManager)->toReceive('attach')->with(MvcEvent::EVENT_DISPATCH_ERROR, [$listener, 'forceHttpsScheme'], 1000); $listener->attach($this->eventManager); expect($this->eventManager)->toReceive('attach')->with(MvcEvent::EVENT_ROUTE, [$listener, 'forceHttpsScheme']); + expect($this->eventManager)->toReceive('attach')->with(MvcEvent::EVENT_DISPATCH_ERROR, [$listener, 'forceHttpsScheme'], 1000); }); @@ -106,6 +109,27 @@ }); + it('not redirect if router not match', function () { + + $listener = new ForceHttps([ + 'enable' => true, + 'force_all_routes' => true, + 'force_specific_routes' => [], + ]); + + allow($this->mvcEvent)->toReceive('getRouteMatch')->andReturn(null); + allow($this->routeMatch)->toReceive('getMatchedRouteName')->andReturn('about'); + + allow($this->mvcEvent)->toReceive('getRequest', 'getUri', 'getScheme')->andReturn('https'); + allow($this->mvcEvent)->toReceive('getRequest', 'getUri', 'toString')->andReturn('https://www.example.com/about'); + allow($this->mvcEvent)->toReceive('getResponse')->andReturn($this->response); + expect($this->mvcEvent)->toReceive('getResponse'); + + $listener->forceHttpsScheme($this->mvcEvent); + expect($this->response)->not->toReceive('getHeaders'); + + }); + }); context('on current scheme is http', function () { @@ -129,6 +153,24 @@ $listener->forceHttpsScheme($this->mvcEvent); expect($this->mvcEvent)->toReceive('getResponse'); + expect($this->response)->not->toReceive('send'); + + }); + + it('not redirect on router not match', function () { + + $listener = new ForceHttps([ + 'enable' => true, + ]); + + allow($this->mvcEvent)->toReceive('getRequest', 'getUri', 'getScheme')->andReturn('http'); + allow($this->mvcEvent)->toReceive('getRouteMatch')->andReturn(null); + allow($this->mvcEvent)->toReceive('getResponse')->andReturn($this->response); + + $listener->forceHttpsScheme($this->mvcEvent); + + expect($this->mvcEvent)->toReceive('getResponse'); + expect($this->response)->not->toReceive('send'); }); @@ -232,6 +274,35 @@ }); + + it('redirect no router not match, but allow_404 is true', function () { + + $listener = new ForceHttps([ + 'enable' => true, + 'allow_404' => true, + ]); + + allow($this->mvcEvent)->toReceive('getRequest')->andReturn($this->request); + allow($this->request)->toReceive('getUri')->andReturn($this->uri); + allow($this->uri)->toReceive('getScheme')->andReturn('http'); + allow($this->mvcEvent)->toReceive('getRouteMatch')->andReturn(null); + allow($this->uri)->toReceive('setScheme')->with('https')->andReturn($this->uri); + allow($this->uri)->toReceive('toString')->andReturn('https://example.com/404'); + allow($this->mvcEvent)->toReceive('getResponse')->andReturn($this->response); + allow($this->response)->toReceive('setStatusCode')->with(308)->andReturn($this->response); + allow($this->response)->toReceive('getHeaders', 'addHeaderLine')->with('Location', 'https://example.com/404'); + allow($this->response)->toReceive('send'); + + $closure = function () use ($listener) { + $listener->forceHttpsScheme($this->mvcEvent); + }; + expect($closure)->toThrow(new QuitException('Exit statement occurred', 0)); + + expect($this->mvcEvent)->toReceive('getResponse'); + expect($this->response)->toReceive('getHeaders', 'addHeaderLine')->with('Location', 'https://example.com/404'); + + }); + it('redirect with www prefix with configurable "add_www_prefix" on force_all_routes', function () { $listener = new ForceHttps([ diff --git a/spec/Middleware/ForceHttpsSpec.php b/spec/Middleware/ForceHttpsSpec.php index 7d4e843..4976f7c 100644 --- a/spec/Middleware/ForceHttpsSpec.php +++ b/spec/Middleware/ForceHttpsSpec.php @@ -49,6 +49,7 @@ $match = RouteResult::fromRouteFailure(null); allow($this->router)->toReceive('match')->andReturn($match); + allow($this->request)->toReceive('getUri', 'getScheme')->andReturn('http'); $listener = new ForceHttps(['enable' => true], $this->router); @@ -61,6 +62,29 @@ }); + it('not redirect on router not match and config allow_404 is false', function () { + + $match = RouteResult::fromRouteFailure(null); + allow($this->router)->toReceive('match')->andReturn($match); + allow($this->request)->toReceive('getUri', 'getScheme')->andReturn('http'); + + $listener = new ForceHttps( + [ + 'enable' => true, + 'allow_404' => false, + ], + $this->router + ); + + $handler = Double::instance(['implements' => RequestHandlerInterface::class]); + allow($handler)->toReceive('handle')->with($this->request)->andReturn($this->response); + + $listener->process($this->request, $handler); + + expect($this->response)->not->toReceive('withStatus'); + + }); + it('not redirect on https and match but no strict_transport_security config', function () { $match = RouteResult::fromRoute(new Route('/about', Double::instance(['implements' => MiddlewareInterface::class]))); @@ -199,6 +223,33 @@ }); + it('return Response with 308 status on http and not match, but allow_404 is true', function () { + + $match = RouteResult::fromRouteFailure(null); + + allow($this->router)->toReceive('match')->andReturn($match); + allow($this->request)->toReceive('getUri', 'getScheme')->andReturn('http'); + allow($this->request)->toReceive('getUri', 'withScheme', '__toString')->andReturn('https://example.com/404'); + + $handler = Double::instance(['implements' => RequestHandlerInterface::class]); + allow($handler)->toReceive('handle')->with($this->request)->andReturn($this->response); + allow($this->response)->toReceive('withStatus')->with(308)->andReturn($this->response); + allow($this->response)->toReceive('withHeader')->with('Location', 'https://example.com/404')->andReturn($this->response); + + $listener = new ForceHttps( + [ + 'enable' => true, + 'allow_404' => true, + ], + $this->router + ); + $listener->process($this->request, $handler); + + expect($this->response)->toReceive('withStatus')->with(308); + expect($this->response)->toReceive('withHeader')->with('Location', 'https://example.com/404'); + + }); + it('return Response with 308 status with include www prefix on http and match with configurable "add_www_prefix"', function () { $match = RouteResult::fromRoute(new Route('/about', Double::instance(['implements' => MiddlewareInterface::class]))); diff --git a/src/HttpsTrait.php b/src/HttpsTrait.php index 154cf66..b43c0c1 100644 --- a/src/HttpsTrait.php +++ b/src/HttpsTrait.php @@ -5,6 +5,7 @@ namespace ForceHttpsModule; use Psr\Http\Message\ResponseInterface; +use Webmozart\Assert\Assert; use Zend\Expressive\Router\RouteResult; use Zend\Http\PhpEnvironment\Response; use Zend\Router\RouteMatch; @@ -25,10 +26,23 @@ private function isSchemeHttps(string $uriScheme) : bool /** * Check Config if is going to be forced to https. * - * @param RouteMatch|RouteResult $match + * @param RouteMatch|RouteResult|null $match */ - private function isGoingToBeForcedToHttps($match) : bool + private function isGoingToBeForcedToHttps($match = null) : bool { + $is404 = $match === null || ($match instanceof RouteResult && $match->isFailure()); + if (isset($this->config['allow_404']) && + $this->config['allow_404'] === true && + $is404 + ) { + return true; + } + + if ($is404) { + return false; + } + + Assert::notNull($match); if (! $this->config['force_all_routes'] && ! \in_array( $match->getMatchedRouteName(), @@ -44,11 +58,11 @@ private function isGoingToBeForcedToHttps($match) : bool /** * Check if Setup Strict-Transport-Security need to be skipped. * - * @param RouteMatch|RouteResult $match - * @param Response|ResponseInterface $response + * @param RouteMatch|RouteResult|null $match + * @param Response|ResponseInterface $response * */ - private function isSkippedHttpStrictTransportSecurity(string $uriScheme, $match, $response) : bool + private function isSkippedHttpStrictTransportSecurity(string $uriScheme, $match = null, $response) : bool { return ! $this->isSchemeHttps($uriScheme) || ! $this->isGoingToBeForcedToHttps($match) || diff --git a/src/Listener/ForceHttps.php b/src/Listener/ForceHttps.php index 4c5acd6..ee8afce 100644 --- a/src/Listener/ForceHttps.php +++ b/src/Listener/ForceHttps.php @@ -33,9 +33,10 @@ public function attach(EventManagerInterface $events, $priority = 1) : void } $this->listeners[] = $events->attach(MvcEvent::EVENT_ROUTE, [$this, 'forceHttpsScheme']); + $this->listeners[] = $events->attach(MvcEvent::EVENT_DISPATCH_ERROR, [$this, 'forceHttpsScheme'], 1000); } - private function setHttpStrictTransportSecurity(string $uriScheme, RouteMatch $match, Response $response) : Response + private function setHttpStrictTransportSecurity(string $uriScheme, RouteMatch $match = null, Response $response) : Response { if ($this->isSkippedHttpStrictTransportSecurity($uriScheme, $match, $response)) { return $response; @@ -67,7 +68,7 @@ public function forceHttpsScheme(MvcEvent $e) : void /** @var string $uriScheme*/ $uriScheme = $uri->getScheme(); - /** @var RouteMatch $routeMatch */ + /** @var RouteMatch|null $routeMatch */ $routeMatch = $e->getRouteMatch(); $response = $this->setHttpStrictTransportSecurity($uriScheme, $routeMatch, $response); if (! $this->isGoingToBeForcedToHttps($routeMatch)) { diff --git a/src/Middleware/ForceHttps.php b/src/Middleware/ForceHttps.php index 95a6c5b..e169352 100644 --- a/src/Middleware/ForceHttps.php +++ b/src/Middleware/ForceHttps.php @@ -49,9 +49,6 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface } $match = $this->router->match($request); - if ($match->isFailure()) { - return $response; - } $uri = $request->getUri(); $uriScheme = $uri->getScheme();