api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.44k stars 869 forks source link

QueryParameterValidateListener should run after authentication #4389

Closed 1ed closed 1 year ago

1ed commented 3 years ago

API Platform version(s) affected: >=v2.3.0-beta.1

Description

I have a protected resource with a collection operation and a required integer filter. If I make a GET request without the filter and without an access token I get a 400 bad request instead of 401.

How to reproduce

See the description.

Possible Solution

The priority of the QueryParameterValidateListener seems to high to me, I think it should run just before the ReadListener at like priority 5.

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

 ------- ---------------------------------------------------------------------------------------------- ---------- 
  Order   Callable                                                                                       Priority  
 ------- ---------------------------------------------------------------------------------------------- ---------- 
  #1      Symfony\Bridge\Monolog\Processor\WebProcessor::onKernelRequest()                               4096      
  #2      Symfony\Component\HttpKernel\EventListener\DebugHandlersListener::configure()                  2048      
  #3      Symfony\Component\HttpKernel\EventListener\ValidateRequestListener::onKernelRequest()          256       
  #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\QueryParameterValidateListener::onKernelRequest()               16        
  #8      Symfony\Component\HttpKernel\EventListener\LocaleListener::onKernelRequest()                   16        
  #9      Symfony\Component\HttpKernel\EventListener\LocaleAwareListener::onKernelRequest()              15        
  #10     Symfony\Bundle\SecurityBundle\Debug\TraceableFirewallListener::configureLogoutUrlGenerator()   8         
  #11     Symfony\Bundle\SecurityBundle\Debug\TraceableFirewallListener::onKernelRequest()               8         
  #12     ApiPlatform\Core\EventListener\AddFormatListener::onKernelRequest()                            7         
  #13     Sentry\SentryBundle\EventListener\RequestListener::handleKernelRequestEvent()                  5         
  #14     Sentry\SentryBundle\EventListener\TracingRequestListener::handleKernelRequestEvent()           4         
  #15     ApiPlatform\Core\EventListener\ReadListener::onKernelRequest()                                 4         
  #16     Sentry\SentryBundle\EventListener\SubRequestListener::handleKernelRequestEvent()               3         
  #17     ApiPlatform\Core\Security\EventListener\DenyAccessListener::onSecurity()                       3         
  #18     Sentry\SentryBundle\EventListener\TracingSubRequestListener::handleKernelRequestEvent()        2         
  #19     ApiPlatform\Core\EventListener\DeserializeListener::onKernelRequest()                          2         
  #20     ApiPlatform\Core\Security\EventListener\DenyAccessListener::onSecurityPostDenormalize()        1         
  #21     ApiPlatform\Core\Bridge\Symfony\Bundle\EventListener\SwaggerUiListener::onKernelRequest()      0         
 ------- ---------------------------------------------------------------------------------------------- ---------- 

Additional Context

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

nawel-les-tilleuls commented 1 year ago

I take this issue

soyuka commented 1 year ago

still open as the fix was breaking #5540

soyuka commented 1 year ago

Closing you've ways to fix this yourself now.

maks-rafalko commented 1 year ago

Closing you've ways to fix this yourself now.

could you please point to the place where it's documented? Sorry if I missed something

soyuka commented 1 year ago

Declare the service yourself changing the priority doesn't work? We can still change priorities though it's out of the backward compatibility layer (though it may break some tests out there ^^)...

On 3.2 you can even decorate the ReadProvider or the AccessCheckerProvider to do this. It's hard to provide user-configuration over these priorities without overriding the dependency injection though.