thephpleague / oauth2-server-bundle

Symfony bundle for the OAuth2 Server.
MIT License
181 stars 88 forks source link

Add extensibility point for custom storage managers #145

Closed Nattfarinn closed 6 months ago

Nattfarinn commented 1 year ago

Bundle provides abstraction for custom mangers but it is actually not possible to plug in custom, non-Doctrine storage due to rigid Configuration (it only allows to define either in_memory or doctrine as persistence methods).

This simple workaround provides third option custom and exposes persistence method as a parameter. Intention is to be able to detect which persistence method is currently set in CompilerPass and declare proper service aliases.

Usage example:

league_oauth2_server:
    # ...
    persistence:
        custom: ibexa
$container->getParameter('league.oauth2_server.persistence.method'); // ibexa

Without extension point we would have to brute-force custom services regardless of actual configuration.

chalasr commented 1 year ago

Thanks for the PR and apologies for the late feedback @Nattfarinn. I support the intent for this, but I feel like we could provide a more structured extension point through the semantic config.

Without extension point we would have to brute-force custom services regardless of actual configuration.

Would you mind sharing some example to showcase how is this supposed to be used so that I can have the full picture? Such sample code could help documenting the feature also, which is needed to move forward anyway

Nattfarinn commented 1 year ago

Thanks for the PR and apologies for the late feedback @Nattfarinn. I support the intent for this, but I feel like we could provide a more structured extension point through the semantic config.

Yes, it would be best. I avoided any bigger changes to the configuration structure and implementation as I was worried about backward compatibility. Hence simple and crude (yet working) solution.

Without extension point we would have to brute-force custom services regardless of actual configuration.

Would you mind sharing some example to showcase how is this supposed to be used so that I can have the full picture? Such sample code could help documenting the feature also, which is needed to move forward anyway

On your side, selected persistence method does not match any switch/case so it does not load any related services definitions. On our side, we provide Symfony services for each Manager but we do not alias interfaces unless persistence method matches our identifier. We plug them in on-demand with CompilerPass instead:

final class PersistenceManagerPass implements CompilerPassInterface
{
    private const PERSISTENCE_METHOD = 'ibexa';

    public function process(ContainerBuilder $container): void
    {
        $persistenceMethod = $container->getParameter('league.oauth2_server.persistence.method');

        if ($persistenceMethod !== self::PERSISTENCE_METHOD) {
            return;
        }

        $container->setAlias(AccessTokenManagerInterface::class, AccessTokenManager::class);
        $container->setAlias(AuthorizationCodeManagerInterface::class, AuthorizationCodeManager::class);
        $container->setAlias(ClientManagerInterface::class, ClientManager::class);
        $container->setAlias(RefreshTokenManagerInterface::class, RefreshTokenManager::class);
        $container->setAlias(CredentialsRevokerInterface::class, TokenRevoker::class);
    }
}
Nattfarinn commented 10 months ago

@mtarld, do you require more information from our side? I like https://github.com/thephpleague/oauth2-server-bundle/pull/171 approach. If you wish to proceed with it feel free to close this one. :)

chalasr commented 6 months ago

Closing in favor of #171 as it's more flexible. Thanks a lot for pushing this @Nattfarinn.