Add waitForAuth to subscribe method

[mattkrick]

Apologies for the recent activity, but this is a really neat package that's almost perfect.

Here's the problem scenario: a parent component calls socket.authenticate to authorize the socket connection. That same component also calls subscribe. We now have a race condition.

Next, the MIDDLEWARE_SUBSCRIBE requires a valid token. if authentication wins, then the subscription is successful. If not, the subscription fails & doesn't try again.

As a workaround, on EVERY subscription, the dev has to write code that checks if the socket is authenticated, & if not, call subscribe inside an authenticate event. We can do this behind the scenes. I propose a waitForAuth property on the subscribe method.

First, we have to store isAuthenticated in the socket state. That means when the server emits connect we save that value. We'll also set it to false on disconnect / state close.

Inside subscribe, if waitForAuth is false OR isAuthenticated is true, it's business as usual. Otherwise, we open a listener for authenticate and put _trySubscribe inside that.

In doing so, we only wait when we have to, the rest of the time, we still get the massive speed benefit of subscribing before a SC connection is open. Thoughts?

[jondubois]

Nice catch! Good to see that you're using SC very thoroughly! Yes, currently, the developer has to make sure that the socket is authenticated before creating/rendering the components which require access to privileged data.

I totally agree that it would be nice if SC could handle this behind the scenes and I think your approach is on the right track. I would like to think about it a bit more.

One thing to consider; the reason why we don't keep track of the isAuthenticated state as a property on the socket currently is because it's hard to say for sure whether or not the socket is still in fact authenticated x amount of time after the authentication/handshake took place - The token might have expired (which is easy to check on the client) or maybe the server changed its authKey in the meantime (therefore invalidating the token - Which is hard to check because it requires communicating with the server). These are unusual scenarios that need to be considered.

It doesn't feel right to tell the client that the socket is authenticated unless we're 100% sure.

Feel free to suggest ideas.

[mattkrick]

Thankfully, we don't ever trust that value on the server, so no security issues. :-D Option 1, an authExp number field. A number in this field tells us 2 things: the token was accepted on the server & it is valid until x. Then, isAuthenticated is just a derived value of that field and the current time.

Option 2, we leave it a boolean & inside #authenticate we put: setTimeout(() => {this.isAuthenticated = false}, expiration - Date.now())

Option 3, we declare true, and until we receive a token auth fail from the server, we keep it true. Your middleware currently doesn't distinguish between authorization & authentication, so this'll need a change to the server package. I'll open a separate issue for this.

Regarding the server changing authKey, I think that is out of scope of socketcluster. Currently, getAuthToken trusts the token we have. This is what the middleware & everything else is based on & having to reverify the token on every getAuthToken is computationally expensive. If folks want to change their token every 24 hours that's fine, but a token issued 1 minute before the change is still going to be valid for 23hrs 59 minutes. If they don't like that, they can spin up a redis server & blacklist it. Again, really slippery slope if socket cluster tries to listen for authKey changes.

I like booleans, so my vote is option 2.