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

[LiveComponent] Invalid CSRF token exceptions are not correctly managed #1150

Open
simondaigre opened this issue Sep 28, 2023 · 12 comments
Open

Comments

@simondaigre
Copy link
Contributor

simondaigre commented Sep 28, 2023

Scenario

  • Enable CSRF in Symfony
  • Without being logged in, browse a page with a Symfony form
  • Fill the form then submit it
  • Go to /login, login normally then go back to the form with your browser and re-submit it

Actual behaviour (without Live component)

  • Form error is correctly displayed with HTTP 422 and "Invalid CSRF token" message.

Actual behaviour (with form in a Live component)

  • Error 500 is thrown and a popup appears with the exception

Error is thrown here :

if (
$this->container->has(CsrfTokenManagerInterface::class)
&& $metadata->get('csrf')
&& !$this->container->get(CsrfTokenManagerInterface::class)->isTokenValid(new CsrfToken(LiveControllerAttributesCreator::getCsrfTokeName($componentName), $request->headers->get('X-CSRF-TOKEN')))) {
throw new BadRequestHttpException('Invalid CSRF token.');
}

This issue occurs because CSRF are reset on each login/logout (which is normal behaviour)
I think we need to find a better way to handle this, since in prod environnements this leads to a 500 error to end users.

@kbond
Copy link
Member

kbond commented Sep 28, 2023

Ok, so it's not the form's csrf that's causing the problem but the live component's csrf...

I agree we should provide a better method to handle this but not quite sure how yet. I think, currently, you can handle with a component js hook but it would be complex. Feels like we could use something similar to loading states but for errors.

@simondaigre
Copy link
Contributor Author

@kbond thanks for the idea. Possible workaround here : symfony/symfony#51724

@smnandre
Copy link
Member

Not sure it would be a solution viable for the entire "symfony stack", but in UX packages, i think a lazy load of the CSRF would not be too hard ... and (yeah i'm a broken record) this would help performance / sobriety. (the CSRF token on public page restrict the caching strategies)

@gremo
Copy link
Contributor

gremo commented Apr 18, 2024

I have the same problem, getting random invalid csfr token on components with data-pool enabled. Does any solution exist?

@smnandre
Copy link
Member

@gremo thank you for reporting this!

Could you open a new issue please with ideally a small reproducer ?

@gremo
Copy link
Contributor

gremo commented Apr 18, 2024

@smnandre I currently receiving those errors in my inbox from an app that is already published. I deleted those emails a few moments ago 😄 but I not sure that the strack trace alone would be helpuful. New errors expected soon btw.

Right now I can't replicate that error while developing 😢

@smnandre
Copy link
Member

Ok i'm closing this issue for now, and you create a new one when you have those emails again, ok for you ?
(it's easier for us to maintain / track)

@gremo
Copy link
Contributor

gremo commented Apr 19, 2024

@smnandre actually I've got another error yeasterday. But it won't be easy to digg into it.

The App:AttendanceActions is a live component:

  • Configured with the data-poll attribute (around 60 secs)
  • Has a live action (triggered by the user) that emit in order to update another component in the page
Matched route "ux_live_component".

route: 	

ux_live_component

route_parameters: 	

{
    "_route": "ux_live_component",
    "_live_action": "refresh",
    "_live_component": "App:AttendanceActions"
}

request_uri: 	

https://myproject/_components/App:AttendanceActions/refresh

method: 	

POST
Uncaught PHP Exception Symfony\Component\HttpKernel\Exception\BadRequestHttpException: "Invalid CSRF token." at LiveComponentSubscriber.php line 114

{
    "class": "Symfony\\Component\\HttpKernel\\Exception\\BadRequestHttpException",
    "message": "Invalid CSRF token.",
    "code": 0,
    "file": "/var/www/vhosts/myproject/httpdocs/releases/171/vendor/symfony/ux-live-component/src/EventListener/LiveComponentSubscriber.php:114",
    "trace": [
        "/var/www/vhosts/myproject/httpdocs/releases/171/vendor/symfony/event-dispatcher/EventDispatcher.php:246",
        "/var/www/vhosts/myproject/httpdocs/releases/171/vendor/symfony/event-dispatcher/EventDispatcher.php:206",
        "/var/www/vhosts/myproject/httpdocs/releases/171/vendor/symfony/event-dispatcher/EventDispatcher.php:56",
        "/var/www/vhosts/myproject/httpdocs/releases/171/vendor/symfony/http-kernel/HttpKernel.php:154",
        "/var/www/vhosts/myproject/httpdocs/releases/171/vendor/symfony/http-kernel/HttpKernel.php:76",
        "/var/www/vhosts/myproject/httpdocs/releases/171/vendor/symfony/http-kernel/Kernel.php:185",
        "/var/www/vhosts/myproject/httpdocs/releases/171/vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php:35",
        "/var/www/vhosts/myproject/httpdocs/releases/171/vendor/autoload_runtime.php:29",
        "/var/www/vhosts/myproject/httpdocs/releases/171/public/index.php:5"
    ]
}

Do you know if it's my bad or a bug? I can paste the source code no problem for me.

@smnandre
Copy link
Member

But data-poll should not use CSRF for me .. or you are talking about the action ?

@gremo
Copy link
Contributor

gremo commented Apr 19, 2024

@smnandre about the action:

  • App:AttendanceActions has a live action named record (triggered by a button) which emit the refresh event
  • App:AttendanceWidged is a live component on the same page and has a live listener to update itself when refresh event is emitted

If data-poll doens't use CSFR than the problem is emitting from App:AttendanceActions or receiving from App:AttendanceWidged. Indeed, the stack show that the exception is thown in LiveComponentSubscriber

Again i think is will be so difficult to debug, I'll try to understand the exact condition to replicate it.

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@WebMamba
Copy link
Contributor

Should be fixed by #2251 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants