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

Event Source should be allowed to work with credentials #549

Open
kbuntrock opened this issue Jul 16, 2021 · 10 comments
Open

Event Source should be allowed to work with credentials #549

kbuntrock opened this issue Jul 16, 2021 · 10 comments

Comments

@kbuntrock
Copy link

kbuntrock commented Jul 16, 2021

One should be allowed to use the eventsource transport with the option "withCredentials".

Cookies can be set to 'httpOnly", which prevent it from being stolen by a XSS injection.

Nowadays, it could be relatively normal to forbid any iframe or jsonp fallback (company website with restrictive versions of browsers).
Some backend implementations are already preventing this fallback in cross-domain situations for several versions : https://docs.spring.io/spring-framework/docs/5.3.9/reference/html/web.html#websocket-server-allowed-origins.

Therefore, I have the feeling it would be safer to allow developers to share cookies with eventsource (via the withCredentials option which is already implemented for xhr-streaming), even though this chapter seems to disagree :
https://github.com/sockjs/sockjs-node#authorisation

This issue is related to this one : https://github.com/sockjs/sockjs-client/pull/299/files#diff-9c48b4997bc97dd0eda039ae379b294c39ade011baed38f961990782e9a3d4bf
But a lot changed in term of browser security in 5 years.

If this point of view is shared, I could work on a PR.

@github-actions
Copy link

This issue has been inactive for 30 days. It will be in closed in 5 days without any new activity.

@kbuntrock
Copy link
Author

Up

@brycekahle
Copy link
Contributor

The problem isn't XSS, but the ability for someone to embed a SockJS iframe that will auto-authenticate, and then use the SockJS as that user.

You are correct that the server can be configured in such a way that this isn't possible, but I'd rather stick to safe defaults. Is there a reason that you cannot authenticate over the SockJS connection?

@github-actions
Copy link

This issue has been inactive for 30 days. It will be in closed in 5 days without any new activity.

@kbuntrock
Copy link
Author

Hello Brycekahle. Thanks to the github bot for the reminder notice and sorry for this long response delay!

What about not allowing both? Since they are already allowed for some other fallback modes than eventsource if I remember well.
Then we can add an extra layer of security, which is my concern.

Cookies with http-only to prevent authentication informations to be stolen via a XSS and used by someone else

  • the authenticate over the SockJS connection to prevent the iframe embedding in another website (I see it a bit like a csrf-token)

The default can stay as it is, with an extra possibility.

@github-actions
Copy link

This issue has been inactive for 30 days. It will be in closed in 5 days without any new activity.

@izogfif
Copy link

izogfif commented Dec 7, 2022

Up. I was baffled to find out that EventSource connection happens without credentials, even though WebSocket connection created by SockJS uses cookies. This has to be fixed. Having to fork SockJS just to replace

var es = this.es = new EventSourceDriver(url);

with

var es = this.es = new EventSourceDriver(url, {withCredentials: true});

seems silly.

@auvipy auvipy reopened this Dec 7, 2022
@auvipy
Copy link

auvipy commented Dec 7, 2022

I would love to see some failing test, the fix and appropriate documentation for any proposed fix/improvement in this are

@github-actions
Copy link

github-actions bot commented Jan 8, 2023

This issue has been inactive for 30 days. It will be in closed in 5 days without any new activity.

@github-actions
Copy link

This issue has been inactive for 30 days. It will be in closed in 5 days without any new activity.

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

4 participants