api-platform / core

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

Security when using DataProvider/DataPersister: What is the state of the art ? #2694

Closed Rebolon closed 3 years ago

Rebolon commented 5 years ago

Writing units tests for my API, i saw that if Standard endpoint (i mean class that use both ORM and Api* annotation) manage well the access_control annotation. But for my Custom endpoint that uses DataProvider and/or DataPersister, i saw that methods supports/applyItem/applyCollection are called even if i don't provide any JWT token. So this means that security management is not managed in this case.

I don't understand why but i can see in the documentation that this is a choice of the team:

# in
https://api-platform.com/docs/core/security/
If you use custom data providers, you'll have to implement the filtering logic according to the persistence layer you rely on.

So my question is : what is the best way to manage security when we use DataProvider/Persister ? I expect a listener to block the access to the endpoint because i set is_granted('ROLE_USER').

The applyItem/applyCollection just use the Token to retreive the user information and modify the query.

Thanks for help

soyuka commented 5 years ago

DenyAccessListener runs on kernel.request and handles your case or I did not understood.

Rebolon commented 5 years ago

I will provide a sample in the week but i can confirm that the listener is not running for DataProvider/DataPersister. How can i say that ? Because even with attributes.access_control or accessControl annotation in an ApiResource, the methodes applyItem/applyCollection of the DataProvider are called.

I'm maybe wrong in my implementation, so that's why i will provde a sample.

soyuka commented 5 years ago

Mhh maybe it's tight to https://github.com/api-platform/core/pull/2710.

Rebolon commented 5 years ago

Sample:

API Entity

<?php
namespace App\Api\EndPoint;

use ApiPlatform\Core\Annotation\ApiResource;

/**
 * Class PingSecured
 * @ApiResource(
 *     attributes={"access_control"="is_granted('ROLE_USER', 'ROLE_API_READ')"},
 *     itemOperations={
 *         "get"={"access_control"="is_granted('ROLE_USER', 'ROLE_API_READ')", "access_control_message"="Only authenticated users can access this endpoint."}
 *     },
 *     collectionOperations={
 *         "get"={"access_control"="is_granted('ROLE_USER', 'ROLE_API_READ')", "access_control_message"="Only authenticated users can access this endpoint."}
 *     }
 * )
 * @package App\Api\EndPoint\PingSecured
 */
class PingSecured extends Ping
{
}

DataProvider

<?php
namespace App\DataProvider;

use ApiPlatform\Core\Annotation\ApiResource;
use ApiPlatform\Core\DataProvider\CollectionDataProviderInterface;
use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
use ApiPlatform\Core\DataProvider\RestrictedDataProviderInterface;
use ApiPlatform\Core\Exception\RuntimeException;
use App\Api\EndPoint\PingSecured;
use App\Api\EndPoint\Prescriptions\Nutrition\Parenterale\ByUser;
use Lexik\Bundle\JWTAuthenticationBundle\Exception\JWTFailureException;
use Symfony\Component\Security\Core\Exception\AuthenticationException;

class PingSecuredDataProvider implements ItemDataProviderInterface, CollectionDataProviderInterface, RestrictedDataProviderInterface
{
    public function supports(string $resourceClass, string $operationName = null, array $context = []): bool
    {
        return PingSecured::class === $resourceClass;
    }

    public function getItem(string $resourceClass, $id, string $operationName = null, array $context = [])
    {
        $pong = new PingSecured();
        $pong->setId(1);

        return $pong;
    }

    public function getCollection(string $resourceClass, string $operationName = null, array $context = [])
    {
        $pongOne = new PingSecured();
        $pongOne->setId(1);
        $pongTwo = new PingSecured();
        $pongTwo->setId(2);

        return [$pongOne, $pongTwo];
    }

}

And my security.yaml

security:
    encoders:
        # this one is for in_memory_provider
        Symfony\Component\Security\Core\User\User: plaintext

    # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
    providers:
        # @todo remove in production
        in_memory:
            memory:
                users:
                    test:
                        password:           test
                        roles:              [ROLE_API_READ, ROLE_API_WRITE]
                    thirtpartsi:
                        password:           test
                        roles:              [ROLE_API_THIRDPART_SI_READ, ROLE_API_THIRDPART_SI_WRITE]
                    coresi:
                        password:           test
                        roles:              [ROLE_API_CORE_SI_READ, ROLE_API_CORE_SI_WRITE]
                    admin:
                        password:           test
                        roles:              [ROLE_ADMIN]

    role_hierarchy:
        ROLE_API_READ:       ROLE_USER
        ROLE_API_WRITE:      ROLE_API_WRITE
        ROLE_API_THIRDPART_SI_READ: ROLE_API_READ
        ROLE_API_THIRDPART_SI_WRITE: [ROLE_API_READ, ROLE_API_WRITE]
        ROLE_API_CORE_SI_READ: [ROLE_ADMIN]
        ROLE_API_CORE_SI_WRITE: [ROLE]

        ROLE_ADMIN:          [ROLE_API_READ, ROLE_API_WRITE]
        ROLE_SUPER_ADMIN:    [ROLE_ADMIN, ROLE_API_THIRDPART_SI_READ, ROLE_API_THIRDPART_SI_WRITE, ROLE_API_CORE_SI_READ, ROLE_API_CORE_SI_WRITE, ROLE_ALLOWED_TO_SWITCH]

    firewalls:
        dev:
            pattern: ^/(_(profiler|wdt)|css|images|js)/
            security: false

        login:
            pattern:  ^/api/login
            stateless: true
            anonymous: true
            json_login:
                provider:                 in_memory
                check_path:               api_login_check
                success_handler:          lexik_jwt_authentication.handler.authentication_success
                failure_handler:          lexik_jwt_authentication.handler.authentication_failure
                username_path:            "%login_username_path%"
                password_path:            "%login_password_path%"
            logout:
                path:                     api_logout
                target:                   index
                invalidate_session:       false

        # @todo even if i remove this part, API always require JWT token, why ?
        # for application with a lot of JS and that are stateless
        security_jwt:
            provider: in_memory
            pattern: ^/api
            stateless:  true
            anonymous: true
            guard:
                authenticators:
                    - lexik_jwt_authentication.jwt_token_authenticator

Then if i call /api/ping_secureds without token in header (using xdebug):

I expect that at least 'getCollection' would not be called. I'm maybe wrong in my implementation and i may have done mistakes. I didn't do the same test for DataPersister, but if it call remove/persist it would be strange.

Rebolon commented 3 years ago

no update on this question ? The question is still relevant for DataProvider, because i can add any security attribute on my resource, the DataProvider methods getItem/getCollection are still called even if the is_granted('ROLE_USER') is setup whereas the uri is called anonymously.

For DataPersister, it seems to have been fixed and the behavior is the same as standard Symfony Controller, so security is checked before calling persist/remove methods.

In DataProvier, if the security control is done only in postProcess of those methods, it doesn't seems to be the expected behavior. In Symfony, a controller is not called if the security annotation is failing.

For instance i fixed this with a crappy Traits called during method supports (from interface RestrictedDataProviderInterface)

Sample of Symfony Controller:

class SecurityController extends AbstractController
{
    /**
     * @Security(expression="is_granted('ROLE_USER')")
     * @Route("/ajax/debug/shouldBeAuthenticated", methods={"GET"})
     * @return JsonResponse
     */
    public function shouldBeAuthenticated(TokenStorageInterface $token)
    {
        $user = $token->getToken()->getUser();

        return new JsonResponse(['user' => $user->getUsername()]);
    }
}
soyuka commented 3 years ago

Greetings! We appreciate your concern but weren't able to reproduce this issue or it is more of a question. As described in the API Platform contributing guide, we use GitHub issues for bugs and feature requests only.

For support question ("How To", usage advice, or troubleshooting your own code), you have several options:

Feel free reach one of the support channels above. In the meantime we're closing this issue.

iva3682 commented 2 years ago

I confirm there is such a problem. The getCollection method is called even if the api method is protected.