Git Product home page Git Product logo

Comments (9)

jondubois avatar jondubois commented on September 23, 2024

@simplesmiler Are you running the socketcluster-client in a WebView? Is it Android or iOS?
I expect cookies to work inside the WebView. What's your specific use case? I wasn't aware of this issue.

As you might have guessed, right now the token (if it exists) is delivered from the client to the server through the 'Cookie' header during the 'Upgrade' request as part of the WebSocket handshake. Browsers do this automatically - It is a nice feature because, on the server side, it gives us the opportunity to check the token before the SCSocket object has been initialized, also, it means that we don't have to explicitly send the token to the server (the browser does that for us automatically).

Cookies are especially nice because they give us two features we need:

  • They are automatically shared between multiple tabs in the browser
  • They are automatically passed along with any HTTP request (including the WebSocket upgrade request handshake)

As you suggested, we can achieve the same result with more extra work by using LocalStorage instead of cookies and by sending the token explicitly over WebSocket instead of relying on the Cookie HTTP header.

Thanks for raising this issue. I will try to think of a clean solution for this.

from socketcluster-client.

simplesmiler avatar simplesmiler commented on September 23, 2024

I'm using ionic (cordova) on iOS. I did not dig into details, but as far as I'm aware cordova serves pages from the file://. So even if the cookies are getting set, their origin will be different from the remote socketcluster server, and thus WebView will not send them to the server.

A clean solution I can think of would be:
On the server side: If token was not received via header, then before sending the #status message server requests the token from the client via special message #requestToken.
On the client side: Refactor storing and loading a token into customizable functions. Respond to #requestToken with the token.

This may break compatibility however. As far as I can tell, currently MIDDLEWARE_HANDSHAKE suggests that token was already received. This will not be true if the token may be sent after the connection is established. Probably there should another type of middleware for a 'soft handshake', that get invoked either just before the #status is sent (connection has been established) or just after the token was received and verified (connection may or may not have been established).

Hope this helps.

from socketcluster-client.

simplesmiler avatar simplesmiler commented on September 23, 2024

Few more thoughts:

  • Local storage is also shared between multiple tabs.
  • Functions for storing and loading a token should probably have async interface.
  • I think that socketcluster-client should not implement default fallbacks for this. Instead it should throw an error (or log to console.error) if the token was required via #requireToken, but corresponding functions were not defined.
  • Maybe there should be an option to disable the default mechanism (storing token in the cookies) so that it does not interfere with the custom one.

from socketcluster-client.

jondubois avatar jondubois commented on September 23, 2024

@simplesmiler After reading this article (and some of the comments): https://stormpath.com/blog/where-to-store-your-jwts-cookies-vs-html5-web-storage/

It seems that both the cookie and localStorage approach both have some drawbacks. Making this work over WebSockets makes things even more complicated because it seems that we cannot set custom headers on the initial 'Upgrade' request (except indirectly if we use cookies).

I made a list of pros and cons and it appears that there is no single 'best way' to do this.

With the cookie approach:

  • It makes the system more vulnerable to CSRF attacks
  • It mitigates XSS attacks (if cookie is specified with the httpOnly flag)

With the localStorage approach:

  • It mitigates CSRF attacks
  • It makes the system more vulnerable to XSS attacks

So I somewhat agree that we need to offer the option for the user to choose.
I also strongly agree with the idea of being able to customize how the token is stored and loaded on the client side.
I'm also thinking that maybe the user should be able to provide a custom function to do token validation on the server side instead of relying on the default one.

I like your suggested flow - It appears to be the best from a functional standpoint, it would support for both the cookie and localStorage approach simultaneously (essentially, it lets each client choose which one it wants to use).

After reading the stormpath article though, I'm also starting to think about this from a security standpoint CSRF vs XSS.

Maybe we shouldn't try to support both cases at the same time (both cookie and localStorage approach simultaneously) - Maybe the server should decide which one to use, not the client - If we let the client choose, then we are losing the security benefit (XSS mitigation) of the cookie approach (because a malicious user could now use an XSS attack to forefully clear all cookies and as a result, trick the server into falling back to the localStorage approach - Which is itself vulnerable to XSS).

So I think there should be a new start option for SocketCluster to allow choosing which token storage technique to use. I think it should use localStorage (with a simple fallback to store directly on the socket object in case localStorage in not supported) by default.

One big problem with the cookie approach is that it doesn't work if socketcluster-client is running inside Node.js (which is a valid and supported use case). And as you mentioned, it appears to be a problem on Cordova too.

If we switch to localStorage as a default (with ability to customize to use any other kind of storage), then all clients (running on any device) will be able to use the authentication feature by default. The cookie approach would only make sense if the app is 100% web-based with no requirement for native client support.

I agree with all the points you made regarding being able to customize the loading and saving of the token on the client (maybe they don't want to use localStorage). I also think that we should allow providing a custom function to do verification on the server too (instead of default one).

EDIT

All things considered, I'm starting to think that maybe we should just completely get rid of the cookie-based approach. I feel that supporting both techniques adds a lot of complexity for very little benefit - We don't want to offer too many ways to do something.

The user can still use a custom cookie-based JWT solution if they don't like the default local-storage one which will come with SC (but the opposite case is not true).

from socketcluster-client.

jondubois avatar jondubois commented on September 23, 2024

If we forget the cookie approach, we can simplify the whole authentication to simply:

  1. Client connects to server
  2. As soon as the connection opens, client emits a #sendToken (or similar) event which carries the token (if it exists or otherwise, the value should be null)
  3. Once the server receives the event, it verifies the token and saves it internally if it exists and is valid
  4. The server sends the '#status' event as normal
    ...

If we try to support the cookie approach too, it makes things much more complicated (especially when you factor in the ability to customize the load/save token functions).

If there is one thing I've learned while working on SC, it's that simplicity is very important and it's often better to cut back on features instead (so long as users are able to implement that feature themselves if they really want it).

from socketcluster-client.

jondubois avatar jondubois commented on September 23, 2024

Comments/feedback welcome.

from socketcluster-client.

jondubois avatar jondubois commented on September 23, 2024

If we get rid of the cookie approach, the MIDDLEWARE_HANDSHAKE middleware won't be able to see the token but that's not a very big deal - The middleware function doesn't get a reference to the socket object anyway (only the Upgrade HTTP req object) so users have to manually parse the req.headers.cookie and then use the jwt library manually to decode - This solution is not really supported so we are not breaking backwards compatibility.

I feel that having the token inside MIDDLEWARE_HANDSHAKE is probably not that useful anyway - I thought of a few use cases, but none of these cases seemed necessary (during the handshake sometimes the user would have a token, sometimes not).

from socketcluster-client.

jondubois avatar jondubois commented on September 23, 2024

Actually, in our case the cookie approach offers no real benefit (no XSS mitigation advantage) because we cannot use the httpOnly flag - In our case, the cookie MUST be set using JavaScript. (Because WebSocket doesn't allow us to respond to the WebSocket 'Upgrade' request).

So we might as well just use localStorage.

from socketcluster-client.

jondubois avatar jondubois commented on September 23, 2024

Ok this was resolved in v2.2.25 - Now the token is stored in localStorage by default. If localStorage is not available, the token will be stored as an instance property - This would be the case if you run the client on Node.js. So client sockets can now use SC's authentication flow regardless of where they are running.

The other improvement which was added in v2.2.25 is that you can now fully customize the authentication behaviour on both the client and the server.

On the client, when creating the socket, you can provide a custom authEngine instance with the following methods:

  • saveToken(name, token, options, callback)
  • loadToken(name, callback)
  • removeToken(name, callback) method

Note that callback needs to be invoked with error as first argument and the value as the second argument.

On the server, you can also provide a custom auth engine by calling scServer.setAuthEngine(authEngine) - On the server-side, the auth engine needs to provide the following methods:

  • signToken(token, key, options, callback)
  • verifyToken(encryptedToken, key, callback)

from socketcluster-client.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.