Content Negotiation should be performed before Authentication / FirewallListener

[kralos]

API Platform version(s) affected: 2.5.5

Description
Currently, AddFormatListener (priority 7) is executed after FirewallListener (priority 8). See registered kernel.request event listeners. This means the Security system cannot rely on Symfony\HttpFoundation\Request::getRequestFormat() as it has not yet been initialized.

When setting a response for the kernel.request event, the propagation is stopped. This means listeners with lower priority won't be executed.

How to reproduce
What if we had a Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface which relied on the request format to decide if it should return a HTTP 302 (see https://tools.ietf.org/html/rfc6749#section-4.1) redirect a user to login. But also supports https://tools.ietf.org/html/rfc6750#section-3 - notify an API client they must authenticate with a Bearer token?

    public function start(
        HttpFoundation\Request $request,
        AuthenticationException $authException = null
    ): HttpFoundation\Response {
        if ($request->getRequestFormat() === 'html') {
            /**
             * @see https://tools.ietf.org/html/rfc6749#section-4.1.1
             */
            $state = sha1(openssl_random_pseudo_bytes(1000));
            $stateKey = AuthorizationCodeAuthenticator::getStateKey($state);
            $this->cache->save($stateKey, $state, 300);

            $uri = $this->router->generate('auth_client_authorization_v2_authorize', [
                'response_type' => 'code',
                'client_id' => $this->oAuthClientId,
                'state' => $state,
            ], UrlGeneratorInterface::ABSOLUTE_URL);

            return new HttpFoundation\RedirectResponse($uri);
        } else {
            /**
             * @see https://tools.ietf.org/html/rfc6750#section-3
             */
            $data = [
                'message' => 'Authentication Required',
            ];
            $wwwAuthenticateHeader = sprintf(
                'Bearer realm="%s"',
                $this->realm
            );
            if (
                $request->headers->has('Authorization')
                && 'Bearer' === substr($request->headers->get('Authorization'), 0, 6)
            ) {
                $wwwAuthenticateHeader .= ', error="invalid_token"';
            }
            return new HttpFoundation\JsonResponse($data, HttpFoundation\Response::HTTP_UNAUTHORIZED, [
                'WWW-Authenticate' => $wwwAuthenticateHeader,
            ]);
        }
    }

Possible Solution
Move AddFormatListener to a priority > 8

Additional Context

3b3f10d#r39354527

but we must double check that it has no bad side effects in term of security before changing this.

The side effects of running ApiPlatform\Core\EventListener\AddFormatListener are:

• the Request::$formats are registered (Map from format names to mime types e.g. 'json' => ['application/json'])
• the Request::$format is set (The format of the current request)
• A NotAcceptableHttpException could be thrown (I think this is a good thing, content negotiation is generally cheaper than authentication)

When priority is < 8 the Request::$format is left null (unless manipulated outside of api-platform/core by another higher priority listener).

Since event dispatcher priorities are not strictly defined by Symfony, we should look to other projects for consensus on a good priority to use for AddFormatListener.

We should start a list of notable bundles that register kernel.request event listeners which might affect the chosen priority for AddFormatListener. Interoperability with other libraries is an important consideration.

Example order of event listeners in a standard symfony 5 project:

Listener	Priority
Symfony\Component\HttpKernel\EventListener\DebugHandlersListener::configure()	2048
Symfony\Component\HttpKernel\EventListener\ValidateRequestListener::onKernelRequest()	256
Symfony\Component\HttpKernel\EventListener\SessionListener::onKernelRequest()	128
Symfony\Component\HttpKernel\EventListener\LocaleListener::setDefaultLocale()	100
Symfony\Component\HttpKernel\EventListener\RouterListener::onKernelRequest()	32
Symfony\Component\HttpKernel\EventListener\LocaleListener::onKernelRequest()	16
Symfony\Component\HttpKernel\EventListener\LocaleAwareListener::onKernelRequest()	15
Symfony\Bundle\SecurityBundle\Debug\TraceableFirewallListener::configureLogoutUrlGenerator()	8
Symfony\Bundle\SecurityBundle\Debug\TraceableFirewallListener::onKernelRequest()	8
Sentry\SentryBundle\EventListener\RequestListener::onKernelRequest()	1
Sentry\SentryBundle\EventListener\SubRequestListener::onKernelRequest()	1

[kralos]

FosRestBundle uses priority 34 - https://symfony.com/doc/master/bundles/FOSRestBundle/3-listener-support.html#priorities

[soyuka]

I definitely agree on the fix here. Would you be able to open a PR?

[kralos]

Sorry, finally got around to this. I had a play with priorities but since AddFormatListener is only supposed to act if within an api-platform route (check for things like $request->attributes->get('_api_resource_class')) we need to be after Symfony\Component\HttpKernel\EventListener\RouterListener::onKernelRequest() (32)

I settled on 28 leaving a little room between router and controller name resolution:

$ APP_ENV=test tests/Fixtures/app/console -v debug:event-dispatcher kernel.request

Registered Listeners for "kernel.request" Event
===============================================

 ------- ------------------------------------------------------------------------------------------------------- ---------- 
  Order   Callable                                                                                                Priority  
 ------- ------------------------------------------------------------------------------------------------------- ---------- 
  #1      Symfony\Component\HttpKernel\EventListener\DebugHandlersListener::configure()                           2048      
  #2      Symfony\Component\HttpKernel\EventListener\ValidateRequestListener::onKernelRequest()                   256       
  #3      Symfony\Component\HttpKernel\EventListener\TestSessionListener::onKernelRequest()                       192       
  #4      Symfony\Component\HttpKernel\EventListener\SessionListener::onKernelRequest()                           128       
  #5      Symfony\Component\HttpKernel\EventListener\LocaleListener::setDefaultLocale()                           100       
  #6      Symfony\Component\HttpKernel\EventListener\RouterListener::onKernelRequest()                            32        
  #7      ApiPlatform\Core\EventListener\AddFormatListener::onKernelRequest()                                     28        
  #8      Symfony\Bundle\FrameworkBundle\EventListener\ResolveControllerNameSubscriber::resolveControllerName()   24        
  #9      ApiPlatform\Core\Filter\QueryParameterValidateListener::onKernelRequest()                               16        
  #10     Symfony\Component\HttpKernel\EventListener\LocaleListener::onKernelRequest()                            16        
  #11     Symfony\Component\HttpKernel\EventListener\LocaleAwareListener::onKernelRequest()                       15        
  #12     Symfony\Bundle\SecurityBundle\EventListener\FirewallListener::configureLogoutUrlGenerator()             8         
  #13     Symfony\Bundle\SecurityBundle\EventListener\FirewallListener::onKernelRequest()                         8         
  #14     ApiPlatform\Core\JsonApi\EventListener\TransformPaginationParametersListener::onKernelRequest()         5         
  #15     ApiPlatform\Core\JsonApi\EventListener\TransformSortingParametersListener::onKernelRequest()            5         
  #16     ApiPlatform\Core\JsonApi\EventListener\TransformFieldsetsParametersListener::onKernelRequest()          5         
  #17     ApiPlatform\Core\JsonApi\EventListener\TransformFilteringParametersListener::onKernelRequest()          5         
  #18     ApiPlatform\Core\EventListener\ReadListener::onKernelRequest()                                          4         
  #19     ApiPlatform\Core\Security\EventListener\DenyAccessListener::onSecurity()                                3         
  #20     ApiPlatform\Core\EventListener\DeserializeListener::onKernelRequest()                                   2         
  #21     ApiPlatform\Core\Security\EventListener\DenyAccessListener::onSecurityPostDenormalize()                 1         
  #22     ApiPlatform\Core\Bridge\Symfony\Bundle\EventListener\SwaggerUiListener::onKernelRequest()               0         
 ------- ------------------------------------------------------------------------------------------------------- ----------