Hotfix: isGranted for failed routing

[michalbundyra]

Q	A
Bugfix	yes
BC Break	no

Description

We have RouteResult in the request attributes but routing failed so getMatchedRouteName will return false.

In that case everyone can access non-existing resource, so the result of isGranted should be true.

I hit the error with my application when navigating to non-existing page. I got exception instead of proper 404 page. My pipeline is as follows:

    $app->pipe(ErrorHandler::class);
    $app->pipe(ServerUrlMiddleware::class);

    $app->pipe(AuthenticationMiddleware::class);

    $app->pipe(SessionMiddleware::class);
    $app->pipe(CsrfMiddleware::class);

    $app->pipe(RouteMiddleware::class);

    $app->pipe(ImplicitHeadMiddleware::class);
    $app->pipe(ImplicitOptionsMiddleware::class);
    $app->pipe(MethodNotAllowedMiddleware::class);

    $app->pipe(UrlHelperMiddleware::class);

    $app->pipe(AuthorizationMiddleware::class);

    $app->pipe(DispatchMiddleware::class);

    $app->pipe(NotFoundHandler::class);

[michalbundyra]

The problem here is that even if we are going to return false on isGranted as the result we are gonna get 403 response instead of 404, see:
https://github.com/mezzio/mezzio-authorization/blob/master/src/AuthorizationMiddleware.php#L44-L57

Maybe it should be fixed somehow in mezzio/mezzio-authorization then?

[michalbundyra]

The same issue we will have also in ACL implementation:
https://github.com/mezzio/mezzio-authorization-acl/blob/master/src/LaminasAcl.php#L49

false value is not allowed in
https://github.com/laminas/laminas-permissions-acl/blob/master/src/Acl.php#L712-L717
(2nd parameter).

When routing failed RouteResult::getMatchedRouteName() returns false, see:
https://github.com/mezzio/mezzio-router/blob/master/src/RouteResult.php#L141-L145

[samsonasik]

I got 500 as well for ACL

[weierophinney]

I'm a bit confused by the test results. They indicate a TypeError thrown because isGranted() returned null instead of a boolean. Did you mock it correctly?

[michalbundyra]

@weierophinney PR is now updated with the fix as we discussed - adapter returns true from isGranted when there is no matched routing. With this solution we can keep original middleware unchanged and without mezzio-router dependency, so it can implement any custom adapter.

The same hotfix I've provided for acl adapter: mezzio/mezzio-authorization-acl#1

[weierophinney]

This makes sense for me. Since these adapters are route-based, they cannot examine anything that is a routing failure, and that case will more than likely mean a 404 scenario.

If users want to do differently (e.g., return a 403 for routing failures), they can decorate the AuthorizationMiddleware to do so.