symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
851 stars 313 forks source link

[Live Component] The User cannot be retrieved from a component extending AbstractController #1409

Closed Narthall closed 9 months ago

Narthall commented 9 months ago

I encountered this bug because I needed to retrieve the currently authenticated User from within a Live Component extending AbstractController. The first time, it works like a charm, but the second time, it doesn't. Here's my code and a detailed explanation:

<?php

namespace App\Twig\Components;

use App\Entity\Post;
use App\Entity\User;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
use Symfony\UX\LiveComponent\Attribute\LiveAction;
use Symfony\UX\LiveComponent\Attribute\LiveArg;
use Symfony\UX\LiveComponent\Attribute\LiveProp;
use Symfony\UX\LiveComponent\DefaultActionTrait;

#[AsLiveComponent]
class Posts extends AbstractController
{
    use DefaultActionTrait;

    private ?User $user = null;

    #[LiveProp]
    public Posts_DisplayTab_Enum $displayTab = Posts_DisplayTab_Enum::POSTS_LATEST;

    public function __construct(
        private readonly PostRepository $postRepository,
    ) {}

    #[LiveAction]
    public function setDisplayTab(#[LiveArg] Posts_DisplayTab_Enum $tabName): void
    {
        $this->displayTab = $tabName;
    }

    public function getData(): array
    {
        // This works the first time. When setDisplayTab() is called from within the component, inside the
        // function getUser() the line $this->container->get('security.token_storage')->getToken() returns null.
        $this->user = $this->getUser();

        $data = match ($this->displayTab) {
            Posts_DisplayTab_Enum::RECORDS_PENDING => $this->getTabPending(),
            default => $this->getTabLatest(),
        };

        return $data;
    }

    private function getTabLatest(): array
    {
        return $this->postRepository->getLatestPosts($this->user);
    }

    private function getTabPending(): array
    {
        return $this->postRepository->getPendingPosts($this->user);
    }
}
<div class="col-12" {{ attributes }} >
    <nav>
        <div class="nav nav-tabs" id="nav-tab" role="tablist">
            <button
                    class="nav-link {{ displayTab.value is same as 'latest' ? 'active' : '' }}"
                    type="button"
                    role="tab"
                    data-action="live#action"
                    data-action-name="setDisplayTab(tabName=latest)"
            >
                Latest
            </button>
            <button
                    class="nav-link {{ displayTab.value is same as 'pending' ? 'active' : '' }}"
                    type="button"
                    role="tab"
                    data-action="live#action"
                    data-action-name="setDisplayTab(tabName=pending)"
            >
                Pending
            </button>
        </div>
    </nav>
    <div class="tab-content" id="nav-tabContent">
        <div class="tab-pane fade show active container-fluid" id="nav-home" role="tabpanel" aria-labelledby="nav-home-tab">
            <div>
                <h6 class="widget_subtitle">Liste of the {{ this.data|length }} latest posts</h6>
                <ul>
                    {% for post in this.data %}
                        <li>Post n°{{ post.id }} - {{ post.title }}</li>
                    {% endfor %}
                </ul>
            </div>
        </div>
    </div>
</div>

I also tried to not extend AbstractController but rather inject the service Symfony\Bundle\SecurityBundle\Security in the constructor, but I encountered the same error. The second time the component loads, when setDisplayTab() is called from within the component, inside the function getUser() the line $this->container->get('security.token_storage')->getToken() returns null.

It does not see; to be linked to my configuration or my firewall.

1ed commented 9 months ago

Maybe you need to customize the live component URL to have it covered with the same firewall you use for the initial rendering https://symfony.com/bundles/ux-live-component/current/index.html#define-another-route-for-your-component

weaverryan commented 9 months ago

@1ed Yea, that's an excellent idea & thing to check.

Unrelated, I'd recommend removing the user property and calling $this->getUser() each time you need the user: there's no real overhead to calling that method multiple times and would make the component a bit less fragile.

smnandre commented 9 months ago

Made a small test and could not figure the problem.

The best way --i think-- for you there would be to inject the User in the LiveAction (just tested, it works)

#[AsLiveComponent]
class FooBar
{
    use DefaultActionTrait;

    #[LiveProp]
    public string $message = 'Hello who ?';

    #[LiveAction]
    public function foo()
    {
        $this->message = 'Hello Foo!';
    }

    #[LiveAction]
    public function bar(#[CurrentUser] ?UserInterface $user = null)
    {
        $this->message = sprintf('Hello %s', $user?->getUserIdentifier() ?? 'XXX');
    }
}
Narthall commented 9 months ago

Thanks @1ed, it worked!

Thanks for the recommendation @weaverryan, I will apply it.

I tried @smnandre solution, and it did not work for me. Maybe this attribute (that I did not know about, thank you for that!) is not available to me because of my firewall configuration. I'm using multiple firewalls, as you can see:

#config/packages/security.yaml

security:
    password_hashers:
        Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface: 'auto'
    providers:
        back_user_provider:
            entity:
                class: App\Entity\Security\Back\BackUser
                property: email
        app_user_provider:
            entity:
                class: App\Entity\Security\App\AppUser
                property: email
        all_users:
            chain:
                providers: ['back_user_provider', 'app_user_provider']

    firewalls:
        dev:
            pattern: ^/(_(profiler|wdt)|css|images|js)/
            security: false
        back:
            pattern: ^/back
            provider: back_user_provider
            form_login:
                login_path: back_login
                check_path: back_login
                default_target_path: back_index
            logout:
                path: back_logout
                target: back_login
        app:
            pattern: ^/app
            provider: app_user_provider
            form_login:
                login_path: app_login
                check_path: app_login
                default_target_path: app_index
            logout:
                path: app_logout
                target: app_login

And so it's easier using them, I'm using the following config and file structure:

#config/routes.yaml
anonymous:
    resource: ../src/Controller/Anonymous/
    type: attribute
    prefix: '/'
    name_prefix: 'anonymous_'
app:
    resource: ../src/Controller/App/
    type: attribute
    prefix: '/app'
    name_prefix: 'app_'
back:
    resource: ../src/Controller/Back/
    type: attribute
    prefix: '/back'
    name_prefix: 'back_'
api:
    resource: ../src/Controller/Api/
    type: attribute
    prefix: '/api'
    name_prefix: 'api_'
#And so on
src
├──Controller
│   ├── Anonymous
│   │   ├── ...
│   ├── App
│   │   ├── ...
│   ├── Back
│   │   ├── ...

I wanted to follow the same directory structure with my components:

src
├──Twig
│   ├── Controllers
│   │   ├── Anonymous
│   │   │   ├── ...
│   │   ├── App
│   │   │   ├── ...
│   │   ├── Back
│   │   │   ├── ...

Do you think this is achievable, so I wouldn't rely on specifying the route property of AsLiveComponent each time?

1ed commented 9 months ago

If getUser() gives back a correct user then #[CurrentUser] should too, but if you have FrameworkExtraBundle then #[CurrentUser] will not work.

https://github.com/symfony/symfony/issues/40333

The CurrentUser attribute currently does not work if your user is also a Doctrine entity and you have FrameworkExtraBundle's param converters enabled.

weaverryan commented 9 months ago

Do you think this is achievable, so I wouldn't rely on specifying the route property of AsLiveComponent each time?

The default route used by live components is something you import in your app - https://github.com/symfony/recipes/blob/main/symfony/ux-live-component/2.6/config/routes/ux_live_component.yaml - you could change that prefix to something like /back/_components assuming that you only use live components on the backend. If you also use on the frontend, then have 2 routes: one that starts with /back and the other for /app. But since only one of these routes can be called the default ux_live_component route, you will need to specify the route to point to the other for all components being used in that context.

Cheers!