Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contao.security.token_checker cannot be decorated #4967

Closed
fritzmg opened this issue Jul 8, 2022 · 5 comments · Fixed by #5888
Closed

contao.security.token_checker cannot be decorated #4967

fritzmg opened this issue Jul 8, 2022 · 5 comments · Fixed by #5888
Labels
Milestone

Comments

@fritzmg
Copy link
Contributor

fritzmg commented Jul 8, 2022

Affected version(s)

4.9+

Description

The class comment of our contao.security.token_checker service says:

/**
 * @internal Do not inherit from this class; decorate the "contao.security.token_checker" service instead
 */

However, decorating that service is not actually possible, as this service does not implement an interface and thus all other services injecting the token checker are requiring a type of

Contao\CoreBundle\Security\Authentication\Token\TokenChecker

and thus if you try to decorate this service you will end up with an error like

TypeError:
Contao\CoreBundle\Framework\ContaoFramework::__construct(): Argument #3 ($tokenChecker) must be of type Contao\CoreBundle\Security\Authentication\Token\TokenChecker, App\Security\TokenChecker given, called in var\cache\dev\ContainerSbgejui\appContao_ManagerBundle_HttpKernel_ContaoKernelDevDebugContainer.php on line 766

  at vendor\contao\core-bundle\src\Framework\ContaoFramework.php:99
@fritzmg fritzmg added this to the 4.9 milestone Jul 8, 2022
@ausi
Copy link
Member

ausi commented Jul 8, 2022

Maybe the correct text would be Do not call the constructor from an inherited class; decorate the "contao.security.token_checker" service instead and redirect the public methods to the inner service or something in that direction?

@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 8, 2022

But that would not change anything. You simply cannot decorate contao.security.token_checker due to the other dependents.

// Sorry, misunderstood you comment. Yes that would be more accurate then.

@fritzmg
Copy link
Contributor Author

fritzmg commented Oct 6, 2022

But the question is - is this what we want? Shouldn't the token checker implement an interface instead and all services requiring that service should inject that interface?

@ausi
Copy link
Member

ausi commented Oct 6, 2022

That would be valid for all services, so I’d only do this if decorating this service is common or an explicit extension point for us.

@leofeyer leofeyer added the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Feb 16, 2023
@leofeyer leofeyer modified the milestones: 4.9, 4.13 Feb 16, 2023
@leofeyer
Copy link
Member

leofeyer commented Mar 9, 2023

As discussed in the Contao call, we want to remove the comment from the @internal annotations at constructor level.

@leofeyer leofeyer removed the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Mar 9, 2023
@leofeyer leofeyer linked a pull request Mar 20, 2023 that will close this issue
leofeyer added a commit that referenced this issue Mar 20, 2023
Description
-----------

Fixes #4967

Commits
-------

a19f8aa Remove the @internal hints at constructor level
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants