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

User client on the server with a cookie #344

Closed
diversario opened this issue Dec 2, 2011 · 44 comments
Closed

User client on the server with a cookie #344

diversario opened this issue Dec 2, 2011 · 44 comments

Comments

@diversario
Copy link

I need to connect from one server to another via sockets (target runs socket.io) and I need to be able to add session data to client socket connection, basically to make it look like it was coming from a browser with cookies. Is it possible at all?

@nponeccop
Copy link

+1. I need an ability to send cookies in handshake request when run from node.js. It seems a hook is needed to insert headers after XHR is open in https://github.com/LearnBoost/socket.io-client/blob/master/lib/socket.js

@attentif
Copy link

+1. We use Connect sessions for handshake authorization, which seems to be rather common, and we need to be able to test that in our integration tests.

@nponeccop
Copy link

@attentif How did you manage that? I had to patch socket.io so it passes Connect session object in handshake object.

Consider the following scenario: cookie and set-cookie headers are blocked by a reverse proxy for all HTTP requests but the socket.io handshake request. Can your implementation establish a session in this case?

@attentif
Copy link

@nponeccop The session must be established before opening the socket connection. We just check handshakeData.headers.cookie in the authorization function, which it seems is possible since v0.7 of Socket.IO. Our initial reference: http://www.danielbaulig.de/socket-ioexpress/

That means our implementation won't work in your scenario as we require a preliminary "login" request to establish the session.

@nponeccop
Copy link

See my approach here: socketstream/socketstream#219

It would be nice if we could join our efforts of pushing necessary patches to socket.io.

@missinglink
Copy link

What is the status of this?

I am trying to connect with a client written in node.js.
My server has an io.set('authorization',...) step defined

I keep getting the error handshake error No cookie transmitted when the client connects.
If the io.set('authorization',...) step is removed then the client connects without error.

Connecting from the browser works fine.

@rauchg
Copy link
Contributor

rauchg commented Oct 6, 2012

Maybe a node-only option extraHeaders ?

@3rd-Eden
Copy link
Contributor

Why would this be needed? We allow people to add querystrings to the connection call, which can be used on the server side again.

eg:

io.connect('/?foo=bar');

@rauchg
Copy link
Contributor

rauchg commented Oct 14, 2012

@missinglink

Here:
https://github.com/LearnBoost/socket.io-client/blob/master/lib/socket.js#L149

We want to loop through handshakeHeaders option (which needs to be added and defaulted to {}) and call setRequestHeader on each.

@rauchg
Copy link
Contributor

rauchg commented Oct 14, 2012

I agree, normally you should use query strings if possible.

@missinglink
Copy link

Thanks @3rd-Eden @guille. I'll modify my authentication to accept query string params

@3rd-Eden
Copy link
Contributor

The main reason why we opted for using query strings is that it was the cross browser way of adding extra to a request.

@rauchg
Copy link
Contributor

rauchg commented Oct 14, 2012

Yep, cross-browser cross-transport.

@sebpiq
Copy link

sebpiq commented Nov 15, 2012

A case for custom headers ?

http://stackoverflow.com/questions/13381540/with-socket-io-how-to-handle-authentication-with-a-non-browser-client#comment18284570_13381540

Basically one big issue I think is that it is quite unsecure to put credentials or other sensitive information used for authentication in the url, as this might be logged.

@danielkcz
Copy link
Contributor

I started writing some unit tests for my app using socket.io yesterday. Especially when I want to test NodeJS server alone and without browser support. During the test I can easily use superagent to get response from the server containing session id in the cookies. Then for initiating socket handshake I need to inject this cookie in the request.

I am not sure how query strings would be helpful in this. For the production environment there will be cookies used. So it would mean having "special" server code just to handle tests which is rather nonsense.

What is the problem with PR #439? It seems rather simple without braking anything and doing great job.

@erinishimoticha
Copy link

+1. I'm also integration-testing an infrastructure which requires auth by XHR and cookies sent with the handshake.

@RunnerRick
Copy link

+1 Ditto @erinspice

@danielkcz
Copy link
Contributor

Let's have another look into this.

For the production environment we have server sending Set-Cookie header . Calling io.connect() is using browser implementation of XHR and sends Cookie header back to the server to initiate socket connection including authorization correctly.

Now for the integration tests - that means having only superagent and running on NodeJS environment - calling io.connect() creates XHR using library node-XMLHttpRequest. This library essentially forbids setting Cookie header. And as there is no interconnection between superagent and this, it means no way to send cookie in headers in request. That also means, that mentioned PR #439 doesn't solve anything.

bold line here

As the @guille suggested, using query string is generally nicer approach. I didn't agreed before, but now I was mostly prepared to go this way. However I had bumped into httpOnly flag. Frankly I didn't knew about this till now.

For production environment that means a problem, because this security measure will forbid to read cookie value, so it cannot be appended to query string.

On other hand integration tests will work fine now, as session cookie can be read from superagent and appended to query string.

final bold line

Lets summarize it.

Query string can be used for tests, but fails for production.
Standardized cookies can be used for production, but fails for tests.

I don't see a point in doing both solutions just to have passing tests.

So after all of this, only one solution that comes to my mind is simply disabling httpHost flag and use query string solution. However it also means little security issue. Any injected JS would be able to read session cookie and do the nasty stuff with it. Is such risk worth it ?

Also note, that for production environment it means cookie is being sent in headers and query string. That's rather ugly, but I guess nothing can be done about this.

@danielkcz
Copy link
Contributor

Ok, I have prepared some sort of solution. It's not nice, but works for now... https://github.com/FredyC/testing-socket.io

@erinishimoticha
Copy link

@FredyC Looks like this is just a workaround requiring a completely different method of authentication. This will not be sufficient for many people, including myself. I'm not willing to settle for using query string params. Since mine would be a testing environment, using a fake cookie as enabled by PR #439 would be preferable to sending a real cookie via the query string. At least sending a fake cookie with the socket handshake will not require changes to my server side code, which could completely mask or invalidate my testing. I've decided to run modified versions of socket.io-client and xmlhttprequest so that I can programmatically test my server-side code without having to change my authentication scheme. I believe the patches are maybe 10 lines of code combined.

Am I correct in thinking the long-term solution to this would be for someone to write a user agent module for node? It seems the only "problem" here is that both socket.io-client and xmlhttprequest [rightly] assume there is a user agent underneath them handling all these things.

@danielkcz
Copy link
Contributor

@erinspice I am wondering, how did you managed to read cookie value from the server? Sending cookie over XHR is one part, but I have failed on the retrieving too. Superagent is just blocking any attempt to read it from the response.

Anyway I agree that my solution is rather simplified. It doesn't calculate with any advanced techniques for authorization. However when you think about it, middlewares like cookieParser and session are tested separately in their own project (connect). So there is no point in testing these again on your own...

That leaves just couple of tests to make sure, that connection is rejected with invalid cookie, accepted otherwise. For any subsequent test you can presume, that authorization works and you don't need to care about it anymore.

@erinishimoticha
Copy link

@FredyC The key is running a modified version of xmlhttprequest. Just remove Set-Cookie from the list of forbidden http headers. The reason it's forbidden is that it's assumed there's an underlying user agent to handle the reception and sending of cookies. The xmlhttprequest specification actually states that this and other headers can't be read from the request because the user agent handles them.

@danielkcz
Copy link
Contributor

@erinspice I understand this part, for server tests it's quite capable solution...

However I am talking about client tests. That means some browser engine behind (like PhantomJS or just Chrome). As you can see here it is using built-in XHR object that cannot be fooled like this. You cannot read cookie from it and even write it back to it.

@RunnerRick
Copy link

It seems to me that since socket.io-client is the user agent, the restrictions imposed by xmlhttprequest should not apply.

It is a design flaw that socket.io-client takes a dependency on xmlhttprequest. Instead socket.io-client should replace xmlhttprequest with superagent or something like it.

@erinishimoticha
Copy link

@FredyC Correct. In the case where a real browser (Chrome or Phantom) is present, the browser is the user agent. No separate user agent is necessary, because the browser can handle the security and cookies.

@RunnerRick That would be nice, but it's not the case. socket.io-client isn't the user agent, it is a library used by an app which runs in the user agent. In my case, my testing app uses socket.io-client as a library and is interpretted by a JS engine but the JS engine is not part of a user agent.

@danielkcz
Copy link
Contributor

@erinspice I know, for production run there is no issue, browser handles it correctly. But for tests it's not possible unfortunately. That is the reason for my solution above, because I don't see any other way around currently.

@RunnerRick I am afraid, that simple replacing with superagent doesn't solve a thing, because for the browser run it uses again built-in XMLHttpRequest object and that disallows reading or setting cookie.

Ideally it should work the same way as for user except that first request is made by XHR instead of usual one to receive session cookie. Received cookie should be stored in the usual manner and than any subsequent XHR would send that cookie away. However it doesn't works this.

Just for clarification, the first XHR is required because client test usually runs against some other server, like in my case Test`em tool. So it's not possible to receive session cookie there.

@RunnerRick
Copy link

@erinspice and @FredyC, your use cases are different than mine. It seems like you just want this to work in tests while I am interested in server-to-server communication.

(Sorry if I'm oversimplifying. It's late and I should be in bed.)

My understanding is socket.io-client was built so that you could have a Socket.IO client that was not a web browser (hence the name "socket.io-client"). In my specific scenario I have a Sails (Express-based) web server that needs to communicate with another web server. That means my web server is the client.

When you use a web browser the cookie information is preserved when switching to web sockets or some other Socket.IO transport, so when you're using socket.io-client it should behave the same way ... like the browser.

The main difference between a web browser's interaction with the server using Socket.IO and socket.io-client is you start with a non-XHR HTTP request. And that's why getting the cookie information isn't an issue. That's why I suggest socket.io-client should use something like request or superagent to handle the HTTP part of it. xmlhttprequest seems to be intended for simulating XHR, and XHR shouldn't matter in a server-to-server scenario--it's just HTTP.

@danielkcz
Copy link
Contributor

@RunnerRick you are partly right. I don't think that socket.io-client is designed to be run out of the browser. Normally when writing client app, you are requesting socket.io/socket.io.js from the server. Essentially that is socket.io-client library, but there is special builder that makes smaller library just for the enabled transports. So it's absolutely normal to use socket.io-client in browser environment.

Now you have some kind of test webpage that executes the client tests. That page wasn't retrieved from you server, hence there are no cookies received so far. Now to retrieve that cookie from server, you have to run XHR (even through superagent, which is just wrapper). Browser doesn't contain any other native code how to communicate with remote server. Sure there elements like iframe / script / img that can load remote resource. But in the end, you cannot read cookies from these either.

Anyway I gave it another thought and realized, we don't need this at all :) Thing is that whole authorization mechanism can be tested without any browser behind. Simple mocha (or whatever you use) test suite running in NodeJS can use superagent to retrieve session cookie and than establish socket connection to verify it works correctly. We don't really need to test anything in client code for this, because for the production run, it's taken care for us by browser itself. It would be double test trying to do it with browser too.

That essentially means client tests can fake session cookie using query string, because you have this covered elsewhere. So my solution is still pretty valid. It just needs little polishing. I didn't bothered to find out how to insert fake session in the session store. Once this is made, you don't need to change anything in server authorization. Just read session cookie from query and place it in the Cookie header of the handshake data. Than usual cookieParser kicks in and everything works just fine.

Hope I am clear enough, it makes me trouble to explain these things, because I am not english native, sorry.

@danielkcz
Copy link
Contributor

Ok. After latest comment I have thought about it and extended my prototype solution. Check it out. Read the README first for the explanation. To me it's now almost satisfying.

@erinspice Could you please possibly fork that and update it with your solution for using real session cookie instead of fake one ?

@allen-cook
Copy link

Is this going to be merged in?

@erinishimoticha
Copy link

@FredyC My solution required a modification to both socket.io's socket transport and XmlHttpRequest. It breaks compliance with the HTTP spec to move the cookie logic out of the user agent.

@ARAtlas
Copy link

ARAtlas commented Feb 6, 2014

+1

I also ran up against this. Unit testing is hard when the testing environment and the production environment need to behave differently.

@runspired
Copy link

TL;DR Tokens should be handled in the original handshake via a header

I understand the "query string is cross browser" statement, but quite honestly there should at least be an option for sending something along the lines of an "x-access-token" header with the connection request to support modern web CORS security practices. Using query strings is unacceptable for security minded applications. Right now in 1.0 to do token validation, either you have to modify the socket.io-client code enable setting the token header, or you have to add a custom event handlers on both ends that perform a second handshake with the token data prior to a socket being elevated to authorized status.

@danielkcz
Copy link
Contributor

I have chosen completely different path after all these issues. I am not using handshake at all. I let the client do the unauthorized connection and then I am expecting it will send message with valid authentication token. If that doesn't happen within some timeframe, I am killing connection. I am very satisfied with this solution, it's easily testable and I don't need any code manipulation.

@jimmywarting
Copy link

+1

@Tsarpf
Copy link

Tsarpf commented Nov 7, 2014

I'm dropping all the +1's I've got here. The current situation is really inconvenient for testing.

I'm really starting to regret trying out socket.io in my project instead of just using websockets and doing everything by myself like I've done before.

@peteruithoven
Copy link

Maybe this also works for you: socketio/engine.io#207 (comment)

@Tsarpf
Copy link

Tsarpf commented Nov 7, 2014

I've now added a /?cookie=session-id to the socket.io connection url in which the session id has been pregenerated by using POSTing to a login route and getting the cookie from there. I then use my own io.use(... kind of function to extract the session id and append it to the handshake data etc.

The connections 'succeed' now, but when I log the contents of the io thingy after a connection, the clients property is empty. From this it follows that if I send messages directly to a socket they work as they should, but if I use a room or try to message all connected clients nothing happens.

The same server works for browser clients, but not for the test clients. I looked at the connections' data and noticed that the 'io' cookie is missing from the test clients. Could this be the problem? What can I do about it? I have a feeling it might be something really easy I've missed.

I'm not sure if this is the right place to ask this but I'd appreciate any help with this, it has been pretty frustrating.

@peteruithoven
Copy link

@Tsarpf, we also use a url param retrieved using a post.
Why do you need a use to add this url param to the handshake? It should be accessible in: {reference to socket}.handshake.query
You can't yet retrieve a list of clients connected to your cluster (multiple node instances), see: socketio/socket.io-redis-adapter#15
To be able to broadcast to all instances their clients you need to use socket.io-redis, which is also mentioned on: socket.io/docs/using-multiple-nodes/

@Tsarpf
Copy link

Tsarpf commented Nov 7, 2014

I use the session id to get session data in and out of (mongoose) session store using get/set. I guess it's kind of like my own middleware since I couldn't get passport-socket.io to work.

I'm not using multiple/clustered nodes. The problem is the clients don't show up even on a single node instance's io.something.clients or io.engine.clients.

It's as if the socket.io client connections' authorizations were failed even though they should be accepted/authorized according to my authorization method (and console logs prove they went the through the right route).

What I guess is happening is that when they're not accepted they're not added to the clients list or something. But for some reason I'm still able to get a handle to the sockets and ... ???

@peteruithoven
Copy link

I also use my own authentication middleware. This makes sure that I only receive accepted socket connections through the connection / connect event. Maybe you can work with this event instead of the clients list?

@Tsarpf
Copy link

Tsarpf commented Nov 7, 2014

Yeah, I was just hoping I could use the socket.io rooms since they were supposed to be convenient and hopefully optimized and tested. But I guess I'll just do everything myself.

@Tsarpf
Copy link

Tsarpf commented Nov 8, 2014

Alright so we spent a while on IRC with @rase- (thanks again) figuring this out and it turns out that the problem was that I had multiple requires (one per test file) for the file where socket.io was initialized and mocha doesn't separate the test instances / files . So there were two (or more) socket.io instances running. My tests connected to the first one, which was then replaced by an empty instance on some sort of latter require. The references to the sockets kept working, but the socketio reference the tests got was 'broken'.

@hackdan
Copy link

hackdan commented May 5, 2015

Solution example here:

https://gist.github.com/jfromaniello/4087861

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