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

Cookies not being sent on Requests from Relay #13

Closed
reactor81 opened this issue Feb 3, 2016 · 22 comments
Closed

Cookies not being sent on Requests from Relay #13

reactor81 opened this issue Feb 3, 2016 · 22 comments

Comments

@reactor81
Copy link

Hi, I have followed the instructions around for how to add cookies to Relay POST requests:

in renderOnServer.js I added :

Relay.injectNetworkLayer(new Relay.DefaultNetworkLayer(GRAPHQL_URL, {
credentials: 'same-origin',
}));

Does it also need to be added somewhere else, for the client?

Upon inspection in Chrome, it does not seem like the Cookie header is being passed. Have you tested this part? Is it an issue with isomorphic relay router?

@reactor81
Copy link
Author

Ah, I came across this thread denvned/isomorphic-relay#1

But it seems unresolved. Is it possible for updated documentation / example around this? I'm trying to do login via cookie. Ive added express-session to renderOnServer.js but it doesn't seem like client is sending anything up. hm

@reactor81
Copy link
Author

Wow that seems to be pretty close to what I am trying to do. I have isomoprhic-relay-router up and running, and spent the day doing a login mutation, and now i'm at the cookie stage. But, isomorphic-material-relay-starter-kit seems to get me there right out of the box..

Isomorphic-relay-router has done what it promises. I guess the starter kit is heavier. Guess I should consider switching, hm

@thelordoftheboards
Copy link
Contributor

The implementation in the starter kit is just an example, and in itself uses Isomorphic-relay-router. Isomorphic-relay-router in itself does not introduce any other dependencies and is a good starting point for any project. Until it gets subsumed into Relay proper.
There are other implementations that use mutations to manage user accounts, so there are several ways to do this.

@reactor81
Copy link
Author

Hi Alex, I've looked at it for the past hour or so. Very interesting. Cassandra seems to be the big piece of the stack vs MongoDB that I was going to use (but haven't yet setup with GQL). I anticipate a bit of pain for MongoDB interfacing with GQL connections / edges.

Hmm. Is there an email or someway I can ask you about Cassandra as a GQL DB? I read the 'Cassandra meets relay' article, but I wonder, is it scalable / good for production / worthwhile learning CQL?

@thelordoftheboards
Copy link
Contributor

I would suggest you look around, there seem to be several solutions for integrating MongoDB and GraphQL, for instance:

https://blog.risingstack.com/graffiti-mongoose-mongodb-for-graphql/

The choice of Cassandra vs. Mongo DB is outside of the scope of this repository I believe. I would rather not pollute it with extraneous discussions. You can hit me up on https://twitter.com/Lordoftheboards and DM if you like to discuss in more detail.

@reactor81
Copy link
Author

Yeap, totally agree. Thanks!

@reactor81 reactor81 reopened this Feb 3, 2016
@reactor81
Copy link
Author

Aha, seem to have made some progress. Had to add the following to client.js in isomorphic-relay-router

import Relay from 'react-relay';

const GRAPHQL_URL = `http://localhost:8080/graphql`;

Relay.injectNetworkLayer(new Relay.DefaultNetworkLayer(GRAPHQL_URL, {
  credentials: 'same-origin',
}));

(got this tip from looking at starter kit).

Now, it doesnt pass the cookie up on the first render. But thats because no graphql call is made on the first render. So, I think I can fix that by checking request.cookies in the app.get(/*) part of server.js..

Good news is the /graphql queries get the cookies now.

@reactor81 reactor81 reopened this Feb 3, 2016
@reactor81
Copy link
Author

Will leave this open for a couple hours while I sort out the cookie on initial render (my guess is in renderOnServer.js)

@thelordoftheboards
Copy link
Contributor

You might be in for a bit of a surprise on this one. As you see, Relay.injectNetworkLayer is a global, so, you can't just inject multiple cookies on the server at the same time and expect it to work properly. In

https://github.com/codefoundries/isomorphic-material-relay-starter-kit/blob/master/webapp/renderOnServer.js

this is resolved with injecting this way:

 Relay.injectNetworkLayer( new Relay.DefaultNetworkLayer( GRAPHQL_URL, { headers: headers } ) );

However, it is wrapped in a queue, to make sure that the same network layer stays unchanged during the execution of the whole server render. So, one server render per core .... I would love to learn a better way to do this.

@reactor81
Copy link
Author

I see. So this is a problem for the first initial render only correct?

The injectNetworkLayer in client.js (material kits app.js) is a safe way to do it for all /graphql requests. Since they all come from the client.

Relay.injectNetworkLayer(new Relay.DefaultNetworkLayer(GRAPHQL_URL, {
credentials: 'same-origin',
}));

So is the issue then only with the initial server side request?

@reactor81
Copy link
Author

Right, so as I am thinking about it.. On the server version of the render, we do the Relay.injectNetworkLayer, but credentials: 'same-origin' or credentials: 'include' will have no effect, since this is all happening on server and we don't know anything about the clients environment..

so we need a way to get the cookie information passed down into relay without credentials: param.. And I guess that is where you did the Header: acrobatics.

@thelordoftheboards
Copy link
Contributor

This is correct. Once the server render has been performed, the application continues to work as a regular SPA and only uses the GraphQL endpoint. The code

Relay.injectNetworkLayer(new Relay.DefaultNetworkLayer(GRAPHQL_URL, {
credentials: 'same-origin',
}));

will indeed ensure that the cookie is sent to the GraphQL server every time.

The choice to go with cookies is because of the HTTP only cookies, which offer some protection against certain types of attacks. Please review:

https://stormpath.com/blog/where-to-store-your-jwts-cookies-vs-html5-web-storage/
http://blog.codinghorror.com/protecting-your-cookies-httponly/

The article provides a good overview, although the statement "Cookies, when used with the HttpOnly cookie flag, are not accessible through JavaScript, and are immune to XSS." is not an absolute:

https://www.youtube.com/watch?v=jrKOdWPZtAg

Again, this might be somewhat out of scope for the isomorphic-relay-router as such.

@reactor81
Copy link
Author

Right, I actually just got it working with seq-queue 10 seconds ago. I'm all set now. Thanks a ton!! Only 1 core though, so we'll see about load, but what can ya do. Thanks!

@thelordoftheboards
Copy link
Contributor

Not to be devil's advocate, but you could run more than one node process per server. Say good bye to core affinity in this case, though.
Good luck.

@reactor81
Copy link
Author

Yeah, I think if I felt it hurting the server too bad, maybe just spin another EC2 instance and load balance.

Thanks again, closing now :)

@terrisgit
Copy link

Closed with no PR? No code change needed?

@bfwg
Copy link

bfwg commented Mar 28, 2016

I run into the same problem, the session on the server did not passed into the inject network layer. Right now I'm making injectNetworkLayer not global like this:

export default (req, res, next) => {
  Relay.injectNetworkLayer(new Relay.DefaultNetworkLayer(GRAPHQL_URL, {
    headers: req.headers,
  }));
  match({routes, location: req.originalUrl}, (error, redirectLocation, renderProps) => {
  ...
});

I know this will cause problems but is there any better ways to do it? @thelordoftheboards

@thelordoftheboards
Copy link
Contributor

@bfwg how about: denvned/isomorphic-relay#24
and https://github.com/codefoundries/isomorphic-material-relay-starter-kit/blob/master/webapp/renderOnServer.js

         Relay.injectNetworkLayer(
            new RelayLocalSchema.NetworkLayer( {
              schema,
              rootValue: { user_id, objectManager },
              onError: (errors, request) => console.error(errors, request),
              // TODO Implement winston logging here
            } )
          );

@thelordoftheboards
Copy link
Contributor

And just to clarify, this does not resolve the problem with the injected network layer being global as such, just a simpler way of passing whatever authentication stuff you got into the root value. I am pretty happy with it, makes this simpler and more secure in my case.

@bfwg
Copy link

bfwg commented Mar 28, 2016

@thelordoftheboards I guess seq-queue is the answer for now. Thank you very much for the tip!

@thelordoftheboards
Copy link
Contributor

I am afraid that for now it is the case. Based on @denvned explanation Facebook has consistently been integrating the changes required to make Relay fully isomorphic on its own, so we might get it pretty soon. After all with version 0.7.3 we finally got facebook/relay#26

@toddtarsi
Copy link

Hey guys, just wanted to let anyone else who reads down to here to know that you don't have to use the FIFO workaround anymore. Since Relay introduced non global environments back around 0.8 ish, you can just use pass in a new network layer to each call, and this module takes care of instantiating a new environment for each request batch.

Relavant line of code: https://github.com/denvned/isomorphic-relay/blob/master/src/prepareData.js#L6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants