sockjs / sockjs-client

WebSocket emulation - Javascript client
MIT License
8.43k stars 1.3k forks source link

Event Source should be allowed to work with credentials #549

Open kbuntrock opened 3 years ago

kbuntrock commented 3 years ago

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[bot] commented 3 years ago

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

kbuntrock commented 3 years ago

Up

brycekahle commented 3 years ago

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[bot] commented 3 years ago

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

kbuntrock commented 3 years ago

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 default can stay as it is, with an extra possibility.

github-actions[bot] commented 2 years ago

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

izogfif commented 1 year ago

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 commented 1 year ago

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

github-actions[bot] commented 1 year ago

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

github-actions[bot] commented 1 year ago

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