ux icon indicating copy to clipboard operation
ux copied to clipboard

[UX Turbo Mercure] EventSource is missing withCredentials

Open Komet opened this issue 2 years ago • 7 comments

Hi, isn't {withCredentials: true} missing in https://github.com/symfony/ux/blob/09fca7dab0cd0e3a4db4ae4884b977f8d14bea00/src/Turbo/Bridge/Mercure/Resources/assets/src/turbo_stream_controller.ts#L36 ? WIthout it the mercureAuthorization cookie will not be sent to mercure hub and it returns HTTP 401 (unless configured anonymous).

Komet avatar Mar 02 '22 20:03 Komet

From the docs - https://symfony.com/doc/current/mercure.html#authorization - I think you're correct, unless you're using anonymous (as you mentioned). I'm not sure if there is a downside to always setting this or not - @dunglas?

weaverryan avatar Mar 04 '22 00:03 weaverryan

We'd better make it opt-in I guess https://github.com/bigskysoftware/intercooler-js/issues/292#issuecomment-532805668:

When withCredentials is true for a cross-origin request, the response header Access-Control-Allow-Origin cannot use a wildcard *. This could break some existing setups.

Also, it's not supported by IE yet.

chalasr avatar Mar 04 '22 11:03 chalasr

It's only necessary if you use authorization and if your Mercure hub doesn't share the same domain as your app. To me it should be an option but not enabled by default.

dunglas avatar Mar 04 '22 21:03 dunglas

It's only necessary if you use authorization and if your Mercure hub doesn't share the same domain as your app.

In my case this is not working. The mercure hub is on the same domain (Port 3000) but the authorization cookie is not sent unless I add withCredentials: true to the EventSource. Maybe it's a little bit off topic here but any hints what could be the reason why the cookie is not sent?

Komet avatar Mar 05 '22 07:03 Komet

@Komet Did you find how to fix this? I have the same exact issue.

robinbrisa avatar Sep 04 '22 10:09 robinbrisa

Maybe I was actually mistaken, by reading the UX turbo documentation I thought that a call to the turbo_stream_listen twig function was enough to generate everything necessary for the mercure authentication but after checking the code it seems like all it does is generate the stimulus tags. So are you supposed to manually generate the cookies with the Authorization service when using turbo_stream_listen?

robinbrisa avatar Sep 04 '22 11:09 robinbrisa

@robinbrisa please keep this issue focused on the actual bug/feature. See https://symfony.com/support for some common channels to get community support (e.g. GitHub discussions or Slack).

wouterj avatar Sep 04 '22 12:09 wouterj

I/we are affected by the same issue presented here, following the documentation at:

https://github.com/symfony/ux/blame/2.x/src/Turbo/doc/index.rst#L541

It's clear that it cannot be used in a private/production environment i.e. when using authentication on subscriptions. In my opinion @robinbrisa raises a valid issue that the published documentation (also part of this repository) doesn't have a clear callout on the limitation, where turbo_stream_listen is unsupported in a cross (even sub-)domain use-case authenticated via cookies.

Advising to directing to 'support channels' are not useful as it directly related to the cross-domain cookie authentication use-case not covered in the code:

ux/src/Turbo/Bridge/Mercure/Resources/assets/src/turbo_stream_controller.ts

It's only necessary if you use authorization and if your Mercure hub doesn't share the same domain as your app

This is not entirely correct, I'm running mercure on a sub-domain (mecure.x.x) and the app is running on app.x.x) and I'm affected in latest chromium. I feel this is a common (if not primary) use-case, and I'd argue it's not 'secured by default' (aka authenticated/private) and should be called out prominently in:

https://github.com/symfony/ux/blob/0288ebd2abd1241297450e7443d49c488f0e4598/src/Turbo/doc/index.rst

Incidentally I've forked the repo, added the {withCredentials: true} and "it works". However I don't have enough Typescript knowledge to implement the full options as per the suggestion of:

https://github.com/symfony/ux/issues/291#issuecomment-1059553304

kieljohn avatar Apr 21 '23 12:04 kieljohn