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

[UX Turbo Mercure] EventSource is missing withCredentials #291

Open
Komet opened this issue Mar 2, 2022 · 8 comments · May be fixed by #1774
Open

[UX Turbo Mercure] EventSource is missing withCredentials #291

Komet opened this issue Mar 2, 2022 · 8 comments · May be fixed by #1774

Comments

@Komet
Copy link

Komet commented Mar 2, 2022

Hi, isn't {withCredentials: true} missing in

?
WIthout it the mercureAuthorization cookie will not be sent to mercure hub and it returns HTTP 401 (unless configured anonymous).

@weaverryan
Copy link
Member

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?

@chalasr
Copy link
Member

chalasr commented Mar 4, 2022

We'd better make it opt-in I guess bigskysoftware/intercooler-js#292 (comment):

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.

@dunglas
Copy link
Member

dunglas commented Mar 4, 2022

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.

@Komet
Copy link
Author

Komet commented Mar 5, 2022

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?

@robinbrisa
Copy link
Contributor

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

@robinbrisa
Copy link
Contributor

robinbrisa commented Sep 4, 2022

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?

@wouterj
Copy link
Member

wouterj commented Sep 4, 2022

@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).

@kieljohn
Copy link

kieljohn commented Apr 21, 2023

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:

#291 (comment)

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

Successfully merging a pull request may close this issue.

7 participants