Skip to content

UsageTrackingTokenStorage deprecation is Security.php #43042

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

Closed
r3dge opened this issue Sep 15, 2021 · 14 comments
Closed

UsageTrackingTokenStorage deprecation is Security.php #43042

r3dge opened this issue Sep 15, 2021 · 14 comments

Comments

@r3dge
Copy link

r3dge commented Sep 15, 2021

Symfony version(s) affected: 5.3.7

Description
After using $security->getUser() in a service as mentioned in the documentation (https://symfony.com/doc/5.4/security.html#a-fetching-the-user-object) i get a deprecation warning :

"Since symfony/security-core 5.3: Using "Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage" (service ID: "security.token_storage") outside the request-response cycle is deprecated, use the "Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage" class (service ID: "security.untracked_token_storage") instead or disable usage tracking using "disableUsageTracking()""

How to reproduce

To reproduce this issue, create a service that use Security and call $security->getUser() :

`use Symfony\Component\Security\Core\Security;

class MyService{

private $security;

public function __construct(Security $security){
    $this->security=$security;
}

public function myFunction(){
    $user = $this->security->getUser();
}

}`

Possible Solution

I find out that Security.php of the core security component is using a deprecated service :

public function getToken(): ?TokenInterface { return $this->container->get('security.token_storage')->getToken(); }

As mentioned in the deprecation notice it seems that the service "security.token_storage" should be replaced by "security.untracked_token_storage".

Thanks for your feedback !

@mbabker
Copy link
Contributor

mbabker commented Sep 15, 2021

When are you calling this helper? Looking at the deprecation, it only gets triggered if $container->get('request_stack')->getMainRequest() returns null, which would mean something is calling the token storage before a request is set in the stack.

@r3dge
Copy link
Author

r3dge commented Sep 17, 2021

This service is basically called in a controller before to create a view.

@chalasr
Copy link
Member

chalasr commented Sep 17, 2021

Can you provide a reproducer? We will need that to help.

@r3dge
Copy link
Author

r3dge commented Sep 17, 2021

I guess i have found the issue. The Security service is called from a kernel event listener (kernel.terminate) and not only from a simple service.
The purpose of this listener is to save informations from user-agent request header (browser, os etc...) for each request. Is there a best practice to do that ?

@stof
Copy link
Member

stof commented Sep 17, 2021

use the untracked storage in that listener

@r3dge
Copy link
Author

r3dge commented Sep 17, 2021

could you please provide some details to do that ? thank u

@stof
Copy link
Member

stof commented Sep 18, 2021

Use the security.untracked_token_storage service, as explained in the error message that you pasted in your issue.

@r3dge
Copy link
Author

r3dge commented Sep 20, 2021

The container is not injected in this listener. So i can't access to security.untracked_token_storage from this listener.
I don't see how to do that.

@r3dge
Copy link
Author

r3dge commented Sep 22, 2021

Workaround found.

  1. Inject TokenStorageInterface in the listener constructor :

public function __construct(TokenStorageInterface $tokenStorage) { $this->tokenStorage = $tokenStorage; }

  1. Disable Usage Tracking before to get token

public function onKernelTerminate(TerminateEvent $event) { $this->tokenStorage->disableUsageTracking(); $token = $this->tokenStorage->getToken(); }

  1. Get user

$user = $token->getUser();

Many thanks !

@jderusse
Copy link
Member

I've got similar deprecation triggered when using the Symfony\Component\Security\Core\Security helper outside a request (ie, in a Monolog Processor)

/**
 * Adds user information to a record.
*/
class UserProcessor implements ProcessorInterface
{
    public function __construct(private Security $security) {}

    public function __invoke(array $record): array
    {
        $user = $this->security->getUser();
        if (!$user) {
            return $record;
        }

        if (method_exists($user, 'getUserIdentifier')) {
            $record['username'] = $user->getUserIdentifier();
        } else {
            $record['username'] = $user->getUsername();
        }

        return $record;
    }
}

We can not call disableUsageTracking because we won't be able to restore the previous state (we don't have isUsageTracked method).

The only solution is to inject security.untracked_token_storage instead of security but this is sad to

  • not be able to use the Helper
  • having to configure the services manually (no autowire)

@chalasr
Copy link
Member

chalasr commented Sep 28, 2021

I think the helper should handle this case internally: #43235

@stof
Copy link
Member

stof commented Sep 28, 2021

I'm wondering whether the helper should handle this, or whether UsageTrackingTokenStorage should instead accept getting a token from it outside request contexts.

@jderusse
Copy link
Member

I'm wondering whether the helper should handle this, or whether UsageTrackingTokenStorage should instead accept getting a token from it outside request contexts.

Same feeling for me.. Removing the deprecation (and let the method returning null) would work for me

@chalasr
Copy link
Member

chalasr commented Sep 28, 2021

If we can get rid of that notice while keeping the BC layer efficient enough, I'm all for removing it. I'm going to investigate

@fabpot fabpot closed this as completed Sep 29, 2021
fabpot added a commit that referenced this issue Sep 29, 2021
…kenStorage` (chalasr)

This PR was merged into the 5.3 branch.

Discussion
----------

[Security] Remove annoying deprecation in `UsageTrackingTokenStorage`

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #43042
| License       | MIT
| Doc PR        | -

Commits
-------

06f4663 [Security] Remove annoying deprecation in `UsageTrackingTokenStorage`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants