-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Comments
+1. I need an ability to send cookies in handshake request when run from |
+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. |
@attentif How did you manage that? I had to patch Consider the following scenario: |
@nponeccop The session must be established before opening the socket connection. We just check That means our implementation won't work in your scenario as we require a preliminary "login" request to establish the session. |
See my approach here: socketstream/socketstream#219 It would be nice if we could join our efforts of pushing necessary patches to socket.io. |
What is the status of this? I am trying to connect with a client written in I keep getting the error Connecting from the browser works fine. |
Maybe a node-only option |
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'); |
Here: We want to loop through |
I agree, normally you should use query strings if possible. |
The main reason why we opted for using query strings is that it was the cross browser way of adding extra to a request. |
Yep, cross-browser cross-transport. |
A case for custom headers ? 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. |
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. |
+1. I'm also integration-testing an infrastructure which requires auth by XHR and cookies sent with the handshake. |
+1 Ditto @erinspice |
Let's have another look into this. For the production environment we have server sending Now for the integration tests - that means having only superagent and running on NodeJS environment - calling 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. 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 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. |
Ok, I have prepared some sort of solution. It's not nice, but works for now... https://github.com/FredyC/testing-socket.io |
@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. |
@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. |
@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. |
@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. |
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. |
@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. |
@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. |
@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. |
@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 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 Hope I am clear enough, it makes me trouble to explain these things, because I am not english native, sorry. |
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 ? |
Is this going to be merged in? |
@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. |
+1 I also ran up against this. Unit testing is hard when the testing environment and the production environment need to behave differently. |
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. |
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. |
+1 |
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. |
Maybe this also works for you: socketio/engine.io#207 (comment) |
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. |
@Tsarpf, we also use a url param retrieved using a post. |
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 ... ??? |
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? |
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. |
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'. |
Solution example here: |
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?
The text was updated successfully, but these errors were encountered: