Merge branch 'feature/allow-404-redirect-to-https'

.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:

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,
     ],
     // ...
 ];

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": {

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' => [

config/force-https-module.local.php.dist

@@ -15,5 +15,6 @@ return [
         ],
         'add_www_prefix'        => false,
         'remove_www_prefix'     => false,
+        'allow_404'             => true,
     ],
 ];

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' => [

phpstan.neon

@@ -0,0 +1,3 @@
+includes:
+	- vendor/phpstan/phpstan-webmozart-assert/extension.neon
+

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([

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])));

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) ||

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)) {

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();