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

Support CORS enabled servers and OPTIONS requests #14

Closed
aweary opened this Issue Sep 23, 2015 · 28 comments

Comments

Projects
None yet
@aweary
Copy link
Contributor

aweary commented Sep 23, 2015

  // GraphQL HTTP only supports GET and POST methods.
    if (request.method !== 'GET' && request.method !== 'POST') {
      response.set('Allow', 'GET, POST');
      return sendError(
        response,
        httpError(405, 'GraphQL only supports GET and POST requests.'),
        pretty
      );
    }

Currently if the Express app enables CORS express-graphql will fail due to the above check sending a 405 error when the OPTIONS request comes in.

Two options I see:

  1. Create a flag for enabling CORS in express-graphql and enable OPTION support when true
  2. Just support OPTION requests by default

I'd be happy to implement either, or another option, pending approval!

@leebyron

This comment has been minimized.

Copy link
Contributor

leebyron commented Sep 29, 2015

Sorry for the delay! I think option #2 is probably the right thing to do. Ideally express-graphql is agnostic to CORS. I'm happy to entertain PRs for this.

@aweary

This comment has been minimized.

Copy link
Contributor

aweary commented Sep 30, 2015

@leebyron no apologies needed, I know you must be swamped. So the question I have is, how do we do that? An OPTIONS request should respond with the CORS headers, meaning ideally the user would catch OPTIONS requests before letting the request fall through to express-graphql.

If express-graphql responds with any headers its by definition not being agnostic, so I'm curious how we can support both cases without a flag indicating how to respond.

Maybe it'd be best to rely on the user to catch OPTIONS request in middleware up the stack before it gets to express-graphql.

@leebyron

This comment has been minimized.

Copy link
Contributor

leebyron commented Oct 1, 2015

Maybe it'd be best to rely on the user to catch OPTIONS request in middleware up the stack before it gets to express-graphql.

This sounds right to me. I haven't implemented CORS before, so I'm not actually sure what is required to make it happen. If this is something that can be 100% done by middleware ahead of express-graphql then that is ideal and I'm happy to link to or include an example illustrating that. Otherwise if this module itself needs some slight change to work happily with a CORS middleware, I'd like to know what that would look like.

@aweary

This comment has been minimized.

Copy link
Contributor

aweary commented Oct 1, 2015

Yeah I'm going to go ahead and close this after thinking about it more, the user should know to respond to OPTIONS requests with the appropriate headers if they're enabling CORS and it's too messy and dangerous to try and support it "out of the box".

@aweary aweary closed this Oct 1, 2015

@leebyron

This comment has been minimized.

Copy link
Contributor

leebyron commented Oct 1, 2015

Let me know if you learn more about enabling this with Express


Sent from Mailbox

On Wed, Sep 30, 2015 at 8:04 PM, Brandon Dail notifications@github.com
wrote:

Yeah I'm going to go ahead and close this after thinking about it more, the user should know to respond to OPTIONS requests with the appropriate headers if they're enabling CORS and it's too messy and dangerous to try and support it "out of the box".

Reply to this email directly or view it on GitHub:
#14 (comment)

@aweary

This comment has been minimized.

Copy link
Contributor

aweary commented Oct 1, 2015

Well enabling it in Express is easy. There's the cors middleware package, or you just set the appropriate access headers. It's just that it should be implemented as its own middleware meaning we just don't need to worry about it.

@mnpenner

This comment has been minimized.

Copy link

mnpenner commented May 17, 2016

I'm still seeing

Request URL:http://localhost:8080/graphql
Request Method:OPTIONS
Status Code:405 Method Not Allowed

Does express-graphql support CORS out of box now or not? Is there an option I have to enable?

Edit: I guess not. An example would have been nice; I'm not using express for anything other than serving GraphQL so I don't have a lot of experience with it.

@leebyron

This comment has been minimized.

Copy link
Contributor

leebyron commented May 17, 2016

Correct, express-graphql only implements the graphql endpoint behavior and nothing more. To enable CORS for your express server you have to handle the OPTIONS request yourself.

I'm open to ideas on how to support CORS out of the box, but so far the feeling is that this would be a dangerous thing to do

@mnpenner

This comment has been minimized.

Copy link

mnpenner commented May 17, 2016

Would it be dangerous to just make it an option, defaulting to false? true would mean enable for all requests to the /graphql route, and anything else could be delegated to cors or some other library?

That way us dummies that don't know anything about express or CORS could just quickly set it to true for testing, and then when we need advanced whitelisting or what have you, we can fine-tune the options?

@mnpenner

This comment has been minimized.

Copy link

mnpenner commented May 17, 2016

Okay, I got CORS working for my needs. Here's a full(ish) example for anyone else who stumbles upon this:

import express from 'express';
import graphqlHttp from 'express-graphql';

const port = 8080;
const app = express();
const schema = require('schema/schema.js');
const Chalk = require('chalk');
const cors = require('cors');

app.use('/graphi',express.static(`${__dirname}/public`)); // we could have just used the `graphiql` option: https://github.com/graphql/express-graphql

app.use('/graphql', cors(), graphqlHttp(() => ({
    schema: schema
})));
app.listen(port);
console.log(`Started on ${Chalk.underline.blue(`http://localhost:${port}/`)}`);
@waltfy

This comment has been minimized.

Copy link

waltfy commented Oct 9, 2016

This bit me hard. Took me a while to figure out what was going on. Here's what I found after copying a few requests from GraphiQL and comparing to my own.

fetch('http://localhost:4000/graphql?', {
    method: 'POST',
    headers: { 'content-type': 'application/json' },
    body: JSON.stringify({
        query: "{entries {id}}"
    })
})
.then((res) => res.json())
.then((data) => console.log(data))
.catch((err) => { console.log(err); });
// Promise {[[PromiseStatus]]: "pending", [[PromiseValue]]: undefined}
// OPTIONS http://localhost:4000/graphql? (anonymous function)
// localhost/:1 Fetch API cannot load http://localhost:4000/graphql?. Response for preflight has invalid HTTP status code 405
// TypeError: Failed to fetch(…)

The TypeError experienced above is due to the fact that the OPTIONS (CORS pre-flight) request has failed since GraphQL only accepts GET and POST requests.

What we're handling on the catch part of the Promise chain above is the failure of the OPTIONS request not the POST request as one may expect.

This means that during a request made with the method OPTIONS we do not get to see in the browser the error that the library emits. This makes this simple issue really obscure.

I agree with the discussion above that it should not be this library's concern to deal with CORS, however I think that we could add some improvements through:

  • Better logging; the library could warn the developer regarding the handling of unsupported requests.
  • Better documentation on the README regarding dealing with CORS.

@mnpenner @leebyron thoughts?

@mnpenner

This comment has been minimized.

Copy link

mnpenner commented Oct 10, 2016

@waltfy I think there should definitely be an example in the README showing how to deal with CORS. I realize it's out of scope for this project, but a lot of people are going to have this question and pointing them in the right direction would be a big help. Adding a special error message for "OPTIONS" somewhere around here probably wouldn't be too much work either.

@waltfy

This comment has been minimized.

Copy link

waltfy commented Oct 10, 2016

@mnpenner ok, I'll open up a PR and get some feedback from more people — it certainly feels like it would be helpful from a developer experience point of view. Thanks for the feedback!

@gregpasha

This comment has been minimized.

Copy link

gregpasha commented Jan 14, 2017

@waltfy I am trying to run through the demo using a a node-express server and fetch instead of XHR. I am hitting this issue, but not sure how to work around it. Is there a way I can craft my fetch request to work around this issue? Thanks!!

@gregpasha

This comment has been minimized.

Copy link

gregpasha commented Jan 14, 2017

@waltfy - I got this working by enabling cors . also @mnpenner got it working as shown above. I had read his post but didn't really grock it because my knowledge of node/express is pretty minimal still.

app.options('/graphql', cors())
app.use('/graphql', graphqlHTTP({
schema: schema,
rootValue: root,
graphiql: true,
}));

@edwardaa

This comment has been minimized.

Copy link

edwardaa commented May 2, 2017

A simple way without use of the cors module:

app.use("/graphql", function (req, res, next) {
  res.header('Access-Control-Allow-Origin', '*');
  res.header('Access-Control-Allow-Headers', 'Content-Type, Authorization, Content-Length, X-Requested-With');
  if (req.method === 'OPTIONS') {
    res.sendStatus(200);
  } else {
    next();
  }
});

app.use('/graphql', graphQLHTTP({ schema }));
@nicolasxu

This comment has been minimized.

Copy link

nicolasxu commented Jun 20, 2017

Why all the above solutions do not work for me?

@n2liquid

This comment has been minimized.

Copy link

n2liquid commented Jun 23, 2017

@nicolasxu, maybe if you provide some information someone could help you :) Like what errors you're seeing on the Network tab when the requests fail, that would be a good place to start. Also how you're setting up the GraphQL route and the CORS middleware.

@imrvshah

This comment has been minimized.

Copy link

imrvshah commented Aug 17, 2017

HI,

I have used both the solutions

First one which is provided by @mnpenner

import express from 'express';
import { graphqlExpress, graphiqlExpress } from 'apollo-server-express';
import bodyParser from 'body-parser';
import { makeExecutableSchema } from 'graphql-tools';
import graphqlHttp from 'express-graphql';

var cors = require('cors');

const graphqlHTTP = require('express-graphql');

const GRAPHQL_PORT = 4400;

import typeDefs from './data/types';
import resolvers from './data/resolvers';

const schema = makeExecutableSchema({ typeDefs, resolvers });

const graphQLServer = express();
graphQLServer.use('/graphql', cors(), graphqlHttp(() => ({
    schema: schema
})));

// graphQLServer.use('/graphql', bodyParser.json(), graphqlExpress({schema}));
graphQLServer.use('/graphiql', graphiqlExpress({ endpointURL: '/graphql' }));

graphQLServer.listen(GRAPHQL_PORT, () => console.log(
  `GraphiQL is now running on http://localhost:${GRAPHQL_PORT}/graphiql`
));

My request got stuck in pre-flight and never propagate to next request which is POST

screen shot 2017-08-17 at 12 07 10 pm

For the second solution which is provided by @edwardaa

My code looks like

import express from 'express';
import { graphqlExpress, graphiqlExpress } from 'apollo-server-express';
import bodyParser from 'body-parser';
import { makeExecutableSchema } from 'graphql-tools';
import graphqlHttp from 'express-graphql';

var cors = require('cors');

const graphqlHTTP = require('express-graphql');

const GRAPHQL_PORT = 4400;

import typeDefs from './data/types';
import resolvers from './data/resolvers';

const schema = makeExecutableSchema({ typeDefs, resolvers });

const graphQLServer = express();
graphQLServer.use("/graphql", function (req, res, next) {
  res.header('Access-Control-Allow-Origin', '*');
  res.header('Access-Control-Allow-Headers', 'Content-Type, Authorization, Content-Length, X-Requested-With');
  if (req.method === 'OPTIONS') {
    res.sendStatus(200);
  } else {
    next();
  }
});
graphQLServer.use('/graphql', cors(), graphqlHttp(() => ({
    schema: schema
})));

// graphQLServer.use('/graphql', bodyParser.json(), graphqlExpress({schema}));
graphQLServer.use('/graphiql', graphiqlExpress({ endpointURL: '/graphql' }));

graphQLServer.listen(GRAPHQL_PORT, () => console.log(
  `GraphiQL is now running on http://localhost:${GRAPHQL_PORT}/graphiql`
));

My response I have 200 OK but it still got stuck in there.

screen shot 2017-08-17 at 12 11 45 pm

I am trying to access from mobile application ionic-angular

Am I doing anything wrong?

@edwardaa

This comment has been minimized.

Copy link

edwardaa commented Aug 18, 2017

Try more headers:

graphQLServer.use("/graphql", function (req, res, next) {
  res.header('Access-Control-Allow-Credentials', true);
  res.header('Access-Control-Allow-Headers', 'content-type, authorization, content-length, x-requested-with, accept, origin');
  res.header('Access-Control-Allow-Methods', 'POST, GET, OPTIONS')
  res.header('Access-Control-Allow-Origin', '*');
  ...
}
@imrvshah

This comment has been minimized.

Copy link

imrvshah commented Aug 18, 2017

@edwardaa I have tried by adding more headers but still I am getting the same result :(
Moreover I have tried app.use('*', cors()) didn't work this as well.

@alundiak

This comment has been minimized.

Copy link

alundiak commented Sep 6, 2017

Hello all, please excuse me for popping up with comment to very old thread. I have kinda similar situation, but without Express. Maybe this is silly, because I've recently started playing w/ GraphQL and GitHub API v4, but here is what I've faced/discovered.

If I use header section, and content type json in particular, then request goes as OPTIONS, but response (edges) is received from server.

let queryBody = {
	"query": `{
	  repositoryOwner(login: "alundiak") {
	    repositories(first: 30) {
	      edges {
	        org: node {
	          name
	        }
	      }
	    }
	  }
	}`
};

let options = {
    method: 'post',
    headers: {
        'Content-Type': 'application/json'
    },
    body: JSON.stringify(queryBody)
}

fetch('https://api.github.com/graphql?access_token=MY_GITHUB_TOKEN', options)
    .then(response => response.json())
    .then(data => {
    	console.log(data); // valid response received
    });

screen shot 2017-09-07 at 00 00 42

But when I omit header section and leave body the same:

...
let options = {
    method: 'post',
    body: JSON.stringify(queryBody)
}
...

I also receive my needed response from server, but then request has been POST and I also see my Request Payload:

screen shot 2017-09-07 at 00 04 40

One more time, sorry if this not quite relevant, but it was interesting I faced with this, and reading this thread helped me to figure out what exactly is going on...

@mnpenner

This comment has been minimized.

Copy link

mnpenner commented Sep 7, 2017

@alundiak Content-Type shouldn't matter. It only has to do the pre-flight OPTIONS request once -- perhaps that's why you're seeing it go straight to POST when you run it a second time without the header?

I just tested your code with the Content-Type header -- it sent two requests: OPTIONS followed by POST. Response data came back fine. Sent it again, only did once request. Tried without header, again, one request.

It's not very relevant though -- GitHub has implemented the CORS for you. There's nothing you have to do.

@alundiak

This comment has been minimized.

Copy link

alundiak commented Sep 8, 2017

@mnpenner in fact, I see, that is the same query/url/code/approach, and using Fetch API it goes as OPTIONS and only one request and data received. But when I use XmlHttpRequest then I have 2 OPTIONS and followed POST request. Looks like it's rather diff. between Fetch API and ajax.

@oasis1992

This comment has been minimized.

Copy link

oasis1992 commented Jan 20, 2018

@waltfy thanks!, It was very helpful, use the code to refresh my access tokens and refetch me request. I have to refactor my code

const authMiddleware = new ApolloLink((operation, forward) => {
    const userType = redirectionAuth.getUserType(window.location.pathname) // obteniendo el tipo de usuario
    const tokensUser = tokens.getTokens(userType)
    let accessToken = null
    let refreshToken = null 
    if(tokensUser) {
        accessToken = tokensUser.access_token
        refreshToken = tokensUser.refresh_token
    }
    
    const observer = forward(operation)
    const request = {
        ...operation,
        query: print(operation.query)
    }

    return new Observable(observer => {
        fetch(Config.REACT_APP_URL, {
            method: 'POST',
            headers: { 
                'content-type': 'application/json',
                authorization: 'Bearer' + accessToken || null,
                refresh_token: refreshToken || null
            },
            body: JSON.stringify({
                query: request.query,
                variables: request.variables
            })
            }).then((res) => res.json())
            .then((data) => {
                if(data.errors) {
                    console.log(data.errors)
                }
                if (!observer.closed) {
                    observer.next(data);
                    observer.complete();
                }
            }).catch((error) => { 
                if (!observer.closed) {
                observer.error(error);
            }
        });
    })  
})
@remy115

This comment has been minimized.

Copy link

remy115 commented Jan 27, 2018

Hello all,
just to share:
this code totally solved my problem (related to method OPTIONS not allowed)

app.use('/graphql',(req,res,next)=>{

  res.header('Access-Control-Allow-Credentials', true);
  res.header('Access-Control-Allow-Headers', 'content-type, authorization, content-length, x-requested-with, accept, origin');
  res.header('Access-Control-Allow-Methods', 'POST, GET, OPTIONS')
  res.header('Allow', 'POST, GET, OPTIONS')
  res.header('Access-Control-Allow-Origin', '*');
  if (req.method === 'OPTIONS') {
    res.sendStatus(200);
  } else {
    next();
  }
}, expressGraphQL({
  schema,
  graphiql: true
}));
@msmfsd

This comment has been minimized.

Copy link

msmfsd commented Oct 30, 2018

FYI using Express & Apollo Server v2+ I have the following config working well:

const cors = require('cors')
...

app.use(cors({
  origin: '*',
  methods: 'GET,HEAD,PUT,PATCH,POST,DELETE,OPTIONS',
  optionsSuccessStatus: 200 /* some legacy browsers (IE11, various SmartTVs) choke on 204 */,
}))
...

@abhinav-jain09

This comment has been minimized.

Copy link

abhinav-jain09 commented Nov 6, 2018

any example for Java to support OPTIONS on graphql server side . I am using spring boot with spring cloud adaptor for Azure

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