fix: #225: GrantConfigurator should accept League\OAuth2\Server\Grant…

[TLG-Gildas]

resolve #225

[chalasr]

Cool, thanks for the PR.
What about registering a #[WithAccessTokenTTL(new \DateInterval('PTP5H')] attribute for autoconfiguration that sets the added tag attribute?

[TLG-Gildas]

Cool, thanks for the PR. What about registering a #[WithAccessTokenTTL(new \DateInterval('PTP5H')] attribute for autoconfiguration that sets the added tag attribute?

I chose not to declare the League\OAuth2\Server\Grant\GrantTypeInterface interface as autoconfigured to allow applications to choose which custom grants to enable or disable. This is equivalent to the enable_grant_xxx options in the bundle. It is, however, possible to tag a class using the standard Symfony AutoconfigureTag attribute (which also allows you to specify attributes, for example, equivalent to what I put in the documentation: #[AutoconfigureTag(name: 'league.oauth2_server.authorization_server.grant', attributes: [accessTokenTTL: 'PTP5H'])]

That said, if custom grants are provided by a library, then relying on an attribute is not viable. Indeed, these libraries are not implemented specifically for Symfony or this bundle, but for the main league/oauth-server library.

[chalasr]

Works for me. Can you add your #[AutoconfigureTag] exemple to the docs below the manual definition one?

[chalasr]

I chose not to declare the League\OAuth2\Server\Grant\GrantTypeInterface interface as autoconfigured to allow applications to choose which custom grants to enable or disable.

That's a significant behavior change I'm not sure we want: I'd expect my grant type to be autoregistered. Being able to scope/disable grant types is a different story that deserves its own discussion to me.

[TLG-Gildas]

#[AutoconfigureTag(name: 'league.oauth2_server.authorization_server.grant', attributes: [accessTokenTTL: 'PTP5H'])]

done

[TLG-Gildas]

I chose not to declare the League\OAuth2\Server\Grant\GrantTypeInterface interface as autoconfigured to allow applications to choose which custom grants to enable or disable.

That's a significant behavior change I'm not sure we want: I'd expect my grant type to be autoregistered. Being able to scope/disable grant types is a different story that deserves its own discussion to me.

To be clear, the bundle interface League\Bundle\OAuth2ServerBundle\AuthorizationServer\GrantTypeInterface remains autoconfigured, it's just the lib interface League\OAuth2\Server\Grant\GrantTypeInterface that is not.
So there is no breaking change.

[chalasr]

Yes, what I mean is that I think autoregistration of custom grant types is actually desired. So I'd register the oauth2-server lib's GrantTypeInterface for autoconfiguration

[TLG-Gildas]

Yes, what I mean is that I think autoregistration of custom grant types is actually desired. So I'd register the oauth2-server lib's GrantTypeInterface for autoconfiguration

why not, but in this case, I think we can't activate or not "grants" on some configuration or programmatic basis. However, I don't have any reel use cases yet.
So, like you say it earlier, this subject seems to need more thinking in another discusion.

I'll let you tell me if you want me to update this PR to add autoregistration.

[chalasr]

Please yes :)

[ajgarlag]

What about registering a #[WithAccessTokenTTL(new \DateInterval('PTP5H')] attribute for autoconfiguration that sets the added tag attribute?

IMO there is no need to add an attribute to the codebase for this. The #[AutoconfigureTag(name: 'league.oauth2_server.authorization_server.grant', attributes: [accessTokenTTL: 'PTP5H'])] is easy to use, and requires less code to maintain.

[TLG-Gildas]

What about registering a #[WithAccessTokenTTL(new \DateInterval('PTP5H')] attribute for autoconfiguration that sets the added tag attribute?

IMO there is no need to add an attribute to the codebase for this. The #[AutoconfigureTag(name: 'league.oauth2_server.authorization_server.grant', attributes: [accessTokenTTL: 'PTP5H'])] is easy to use, and requires less code to maintain.

Yes but with the addition of the autoconfigure defined by the bundle it is more consistent to not need to reassign the tag manually just to be able to set the ttl.
If we go back and remove the autoconfiguration by default, then as I said above, I agree.

[chalasr]

If we go back and remove the autoconfiguration by default, then as I said above, I agree.

Fair enough. Can you please rollback on the autoconfiguration part? We do need UPGRADE instructions (ideally in a UPGRADE.md file) showcasing how to switch from the legacy GrantTypeInterface to the lib one with AutoconfigureTag

[TLG-Gildas]

I just pushed a commit to revert autoconfiguration