Providers are executed before any security check

[Cocray]

API Platform version(s) affected: >=3.1.0

Description

I don't really know if it's a bug, but in the AccessCheckerProvider, the provider is executed before any security check. I may have missed something, but IMHO, we may have:

• a security failure if the provider is executing something the user is not allowed to do.
• a perf killer if the provider is executing long stuff whereas in the end, the response will be "you can't access this endpoint"

So I have to rewrite the security in some of my providers to check the access rights in order to avoid what I mentioned above

How to reproduce

Any code such as this small one:

#[ApiResource(
    operations: [
        new GetCollection(
            security: 'is_granted("TEST")',
            provider: TestProvider:class,
        ),
    ]
)]
class Test
{
}

Possible Solution

In AccessCheckerProvider.php, this line is executed before any security check:
$body = $this->decorated->provide($operation, $uriVariables, $context);

By the way, I really don't know the impacts for this part a few lines below :(
if (!$this->resourceAccessChecker->isGranted($operation->getClass(), $isGranted, $resourceAccessCheckerContext)) {

Additional Context

[soyuka]

As the resource access checker has an object param we need to fetch the data. As the data is never sent (in case of a falsy security check) I don't see the security issue. Also please if you find a security issue report it on https://github.com/api-platform/core/security/advisories/new thanks!

You can also create your provider that decorates ours applying some security checks before if needed.

[Cocray]

Thanks @soyuka for your answer, I understand the need.

By the way, when I said it could be a security issue, I don't mean the endpoint response, for the reason you explained, but anything the developer could do in the provider. This is not my case, but I have in mind something like that:

• In the provider, I identify the email adress of the current user and send an email with some data returned
• If the user is not granted by voters, he'll receive the email anyway

I think that's just a good point to know to be careful of what we should do / shouldn't do in the provider because a developer could think that the security part blocks any users not granted.

I close this (false) issue :)

[soyuka]

Sending an email should be done inside processors instead.

[soyuka]

@Miras4207 @Cocray you're very welcome to update our documentation with this. It's also quite easy to decorate the ReadProvider https://api-platform.com/docs/core/extending/#decoration-example and adding your security layer there, let me know if you need more assistance.

[Miras4207]

@soyuka

How is this not considered to be a security issue? The attribute argument is literally called security - and it checks the security condition after the operation is already executed. It does not matter the data are not sent in a response, the problem is that an unauthorized client executed an operation that should be accessible to authorized clients only.

The State Provider is supposed to be used for GET operations - and it's not always just about simply obtaining and providing data. There could be a lot more going on, i.e. resource or time expensive operations, generating or persisting data and other wanted side effects for various reasons. Some examples were given here, others were given in my issue post.

The State Processor is not supposed to be used for GET operations (correct me if I am wrong). That is meant for operations such as POST. However, some operations are supposed to be GET and at the same time manipulate data or have other side effects - as a wild example, you may want to have GET /api/sequence-number endpoint that increments number in a database and returns the new value. The current implementation would allow unauthorized clients to increment this sequence, even if the new value is not sent in a response.

I understand that you may want to use object in the security expression, but due to such security implications, this should not be allowed within security argument. Instead, there should be something like securityPostProvider (or better name) that would allow to perform security check based on data returned by the provider.

Having to use Symfony Firewall or extension points to actually achieve security undermines the utility of the security argument.

Please give this matter a second thought.

[soyuka]

Great news!
We had another discussion at ForumPHP 2025 and actually you have 2 solutions:

• use the security firewall directly
• use condition inside any operation, this will be transposed to route condition and it does support an esl which is almost the same as we do for security https://symfony.com/doc/current/routing.html

[Cocray]

Thank you Soyuka!

[tomxw]

It's not really a good solution though.

What is there in the code is against good security principles

Access should be verified as soon as possible and in case of an unauthorized request, it should be denied as soon as possible without executing any unnecessary code.

Checking it too late opens up all the code in the provider for possible exploits. The most likely exploit in this case would be trying to overload the server by performing unneeded queries.

It's not up to the developers using API platform to figure out that they should use a workaround.

Consistent HTTP response codes

The expected response code when an authenticated user does not have access to an entity is 403 (Forbidden).
When the provider is executed first you may get a 404 (Not found) instead.

Using the security firewall or condition may prevent triggering unneeded database queries but will result in a 404 (Not found) as well. This is typically annoying when you are trying to unit test the permissions in your application.

Solution

Ideally, the global permission is checked first (without checking access to the specific object), only then the providers should execute and then later the access should be checked to a specific object.

[PingusPepan]

I'd also welcome something like securityBeforeProvider. Your provider can be connecting to some external resource and anyone with access to the endpoint is then able to spam your endpoint and potentially overload the external resource. In those cases it's better to let in just users who are at least probable to be able to access the data. And the individual by object check through security will still work

[coissay]

It makes no sense for me that a security param is not checking roles and user access before everything else. I agree with @tomxw and @PingusPepan

[soyuka]

@PingusPepan indeed for the 404 with condition, I found that later. Access control looks like the best option (https://symfony.com/doc/current/security.html#access-control-authorization), maybe we should make that IsGranted compatible with our operation/resource mechanism?