apollo-server-integrations / apollo-server-integration-aws-lambda

An integration to use AWS Lambda as a hosting service with Apollo Server
MIT License
46 stars 9 forks source link

access-control-allow-origin response header missing? #85

Open tavelli opened 1 year ago

tavelli commented 1 year ago

Hi. We used 'apollo-server-lambda' for some time and worked great. I'm trying to update to apollo server 4 and the new '@as-integrations/aws-lambda' package. I got it all working and i can query from the apollo playground but i'm getting a cors error when trying to access graphql from our dev box. We are using same setup in terms of api gateway and lambda as we had before just updated in place. Did apollo-server-lambda handler send back access-control-allow-origin: * by default? because i see it coming with last version but not after the update. We are using createAPIGatewayProxyEventRequestHandler with new setup.

Thanks, Dan

tavelli commented 1 year ago

ah now i see it in apollo update docs: https://www.apollographql.com/docs/apollo-server/migration/#body-parser-and-cors

tavelli commented 1 year ago

if you can add an example of cors middleware setup in docs could be helpful!

alnaranjo commented 1 year ago

This is my approach

  CorsFunction:
    Type: AWS::Serverless::Function
    Properties:
      FunctionName: !Ref FunctionName
      CodeUri: dist
      Handler: index.cors
      Tracing: Active
      Environment:
        Variables:
          CORS_ORIGIN: !Ref CorsOrigin
      Events:
        Options:
          Type: HttpApi
          Properties:
            Path: $default
            Method: options
            RestApiId: !Ref GatewayApi
            PayloadFormatVersion: '2.0'
            Auth:
              ApiKeyRequired: false
const CORS_ORIGIN = process.env.CORS_ORIGIN || '';
const CORS_HEADERS = [
    'Accept',
    'Authorization',
    'Access-Control-Allow-Headers',
    'Access-Control-Allow-Methods',
    'Cookie',
    'Content-Type',
    'Origin',
    'Set-Cookie',
];
const CORS_METHODS = ['GET', 'POST', 'OPTIONS'];

export const cors: APIGatewayProxyHandlerV2 = async () => {
    return {
        statusCode: 200,
        headers: {
            'Access-Control-Allow-Origin': CORS_ORIGIN,
            'Access-Control-Allow-Methods': CORS_METHODS.join(','),
            'Access-Control-Allow-Headers': CORS_HEADERS.join(','),
            'Access-Control-Allow-Credentials': 'true',
        },
    };
};
import {
    handlers,
    middleware,
} from '@as-integrations/aws-lambda';

const requestHandler = handlers.createAPIGatewayProxyEventV2RequestHandler();

const corsMiddleware: middleware.MiddlewareFn<typeof requestHandler> = async (
    event
) => {
    /* eslint-disable no-param-reassign */
    return (result) => {
        result.headers = {
            ...result.headers,
            'Access-Control-Allow-Origin': CORS_ORIGIN,
            'Access-Control-Allow-Credentials': 'true',
        };
        return Promise.resolve();
    };
    /* eslint-enable no-param-reassign */
};
alnaranjo commented 1 year ago

AFAIK there's no way to return from a middleware before executing the graphql request. express cors middleware finalizes the req for preflight requests unless instructed to continue.

https://github.com/expressjs/cors/blob/f038e7722838fd83935674aa8c5bf452766741fb/lib/index.js#L159-L190

danielvouch commented 1 year ago

What is the best way to enforce cors conditionally based on a provided header from the client?

BlenderDude commented 1 year ago

Hey all! Just opened up a PR with more middleware functionality to allow for short circuiting. This will allow you to return a Lambda result from directly inside a middleware function. Let me know your thoughts over in #91

I will also add a packaged cors middleware helper into this library as it seems like quite a popular request.

BlenderDude commented 1 year ago

This is now a fixed issue, as middleware can short circuit and return the result early. Keeping this issue open as CORS is such a common request that it will be added as a builtin middleware in the next minor version.

s7dhansh commented 1 year ago

@alnaranjo sorry but i did not get the solution. can you or someone else provide a minimal reproducible example of cors config with startServerAndCreateLambdaHandler? thank you!

Edit: Worked it out. PFA for future reference.

const {ApolloServer} =  require('@apollo/server');
import {
    startServerAndCreateLambdaHandler,
    handlers,
    middleware,
} from '@as-integrations/aws-lambda';

const server = new ApolloServer({
    ...options,
});

const allowedOrigins = [
    'https://example.app',
    'https://example.another.app',
];
const requestHandler = handlers.createAPIGatewayProxyEventRequestHandler();
const corsMiddleware: middleware.MiddlewareFn<typeof requestHandler> = async(
    event,
) => {
    const origin = event.headers.origin;
    if (
        origin && allowedOrigins.includes(origin)
    ) {
        return (result) => {
            result.headers = {
                ...result.headers,
                'Access-Control-Allow-Origin': origin,
                'Vary': 'Origin',
            };
            return Promise.resolve();
        };
    }
    return () => Promise.resolve();
};

const graphqlHandler = startServerAndCreateLambdaHandler(
    server,
    handlers.createAPIGatewayProxyEventRequestHandler(),
    {
        middleware: [
            corsMiddleware,
        ],
    },
);

export {graphqlHandler};
JeffML commented 11 months ago

I can't seem to get @s7dhansh solution to work. Perhaps I've overlooked something. Here's what I've noticed.

versions: "@apollo/server": "^4.9.4", "@as-integrations/aws-lambda": "^3.1.0", "graphql": "^16.6.0",

I first tried using createAPIGatewayProxyEventV2RequestHandler without success then reverted to the non-V2 used in the solution above. I get different errors.

With V2:

{
  statusCode: 400,
  body: "Cannot read properties of undefined (reading 'http')",
  headers: {
    'Access-Control-Allow-Origin': 'http://localhost:8888',
    Vary: 'Origin'
  }
}

With non-V2:

{
  statusCode: 400,
  headers: {
    'content-type': 'application/json; charset=utf-8',
    'content-length': '1469',
    'Access-Control-Allow-Origin': 'http://localhost:8888',
    Vary: 'Origin'
  },
  body: `'{\n' +
  '    "errors": [\n' +
  '        {\n' +
  `            "message": "This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). Please either specify a 'content-type' header (with a type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) or provide a non-empty value for one of the following headers: x-apollo-operation-name, apollo-require-preflight\\\\n",\n` +
  '            "extensions": {\n' +
  '                "code": "BAD_REQUEST",\n' +
  '                "stacktrace": [\n' +
  `                    "BadRequestError: This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). Please either specify a 'content-type' header (with a type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) or provide a non-empty value for one of the following headers: x-apollo-operation-name, apollo-require-preflight",\n` +
  '                    "",\n' +
  '                    "    at new GraphQLErrorWithCode (/home/myhome/fenster-s/node_modules/@apollo/server/src/internalErrorClasses.ts:15:5)",\n' +
  '                    "    at new BadRequestError (/home/myhome/fenster-s/node_modules/@apollo/server/src/internalErrorClasses.ts:116:5)",\n' +
  '                    "    at preventCsrf (/home/myhome/fenster-s/node_modules/@apollo/server/src/preventCsrf.ts:91:9)",\n' +
  '                    "    at _ApolloServer.executeHTTPGraphQLRequest (/home/myhome/fenster-s/node_modules/@apollo/server/src/ApolloServer.ts:1048:9)",\n' +
  '                    "    at processTicksAndRejections (node:internal/process/task_queues:95:5)",\n' +
  '                    "    at Object.<anonymous> (/home/myhome/fenster-s/node_modules/@as-integrations/aws-lambda/src/lambdaHandler.ts:94:24)"\n' +
  '                ]\n' +
  '            }\n' +
  '        }\n' +
  '    ]\n' +
  '}'

I do have a context handler, but I think that is the only difference.

const allowedOrigins = ["http://localhost:8888"];

const getUser = (token) => {
    const hash = crypto.createHash("md5").update(token).digest("hex");
    if (hash !== "badb33f") return null;
    return "okay";
};
// const requestHandler = handlers.createAPIGatewayProxyEventV2RequestHandler();
const requestHandler = handlers.createAPIGatewayProxyEventRequestHandler();

const corsMiddleware = async (event) => {
    const origin = event.headers.origin;

    console.dir(event, { depth: 5 });

    if (origin && allowedOrigins.includes(origin)) {
        return (result) => {
            result.headers = {
                ...result.headers,
                "Access-Control-Allow-Origin": origin,
                Vary: "Origin",
            };

            console.dir(result, { depth: 5 });
            return Promise.resolve();
        };
    }

    return () => Promise.resolve();
};

const production = process.env.NODE_ENV === "production";

const server = new ApolloServer({
    typeDefs,
    resolvers,
    introspection: !production,
    playground: !production,
    debug: !production,
    context: ({ event: { headers } }) => {
        // get the user token from the headers
        const token = headers.authorization || "";
        // try to retrieve a user with the token
        const user = getUser(token);
        // add the user to the context
        return { user };
    },
});

const graphqlHandler = startServerAndCreateLambdaHandler(
    server,
    requestHandler,
    {
        middleware: [corsMiddleware],
    }
);

export { graphqlHandler as handler };
s7dhansh commented 11 months ago

@JeffML sorry i am occupied with other stuff for the next few days. if time permits will try to take a look at the weekend. if everything else is same, do try with the solution suggested in the error message. If you have already tried that, may be try once with a proper domain, instead of localhost. Just to confirm, it has been working fine for me.

prabukamal commented 11 months ago

This how we solved for Lambda deployment,

exports.graphqlHandler = startServerAndCreateLambdaHandler(
    server,
    handlers.createAPIGatewayProxyEventRequestHandler(),
    {
        context: async ({ event, context }) => {
            const database = await GetPoolConnection();
            const config = await GetSecret();
            const user = GetUser(event?.headers?.Authorization);
            return {
                event,
                context,
                user,
                config,
                database,               
            };
        },
        middleware: [
            async (event) => {
                return async (result) => {
                    result.headers = {
                        ...result.headers,
                        'access-control-allow-headers': '*',
                        'access-control-allow-methods': '*',
                        'access-control-allow-origin': '*',
                    };
                };
            },
        ],
    }
);
JeffML commented 11 months ago

Thank you for the quick response, @s7dhansh and @prabukamal.

s7dhansh: I appreciate whatever time you can spare. As for the error message, I see a content type header of 'application/json' passed in the event (request?), so that should be fine. As for the headers x-apollo-operation-name and apollo-require-preflight, I would expect those to be controlled by the Apollo Client. I'll research it. If localhost is a problem, I can do something with my hosts file to make it look like a proper domain.

prabukamal: I haven't yet tried your solution, but that's next. If it succeeds I will follow up here.

JeffML commented 11 months ago

Sorry, but more boring detail follows:

I didn't include the event data, but looking at it now it appears that it is the preflight request that is failing.

{
  path: '/.netlify/functions/server',
  httpMethod: 'OPTIONS',
  queryStringParameters: {},
  multiValueQueryStringParameters: {},
  headers: {
    'content-length': '0',
    'x-forwarded-for': '::1',
    'accept-language': 'en-US,en;q=0.9',
    'accept-encoding': 'gzip, deflate, br',
    referer: 'http://localhost:8888/',
    'sec-fetch-dest': 'empty',
    'sec-fetch-site': 'same-site',
    'sec-fetch-mode': 'cors',
    'user-agent': 'Mozilla/5.0 (X11; CrOS x86_64 14541.0.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/117.0.0.0 Safari/537.36',
    origin: 'http://localhost:8888',
    'access-control-request-headers': 'authorization,content-type',
    'access-control-request-method': 'POST',
    accept: '*/*',
    connection: 'close',
    host: 'localhost:8881',
    'x-nf-request-id': '01HCDY1Z1V9P61A2MHPA31GB40',
    'client-ip': '::1'
  },
  multiValueHeaders: {
    'content-length': [ '0' ],
    'x-forwarded-for': [ '::1' ],
    'accept-language': [ 'en-US,en;q=0.9' ],
    'accept-encoding': [ 'gzip, deflate, br' ],
    referer: [ 'http://localhost:8888/' ],
    'sec-fetch-dest': [ 'empty' ],
    'sec-fetch-site': [ 'same-site' ],
    'sec-fetch-mode': [ 'cors' ],
    'user-agent': [
      'Mozilla/5.0 (X11; CrOS x86_64 14541.0.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/117.0.0.0 Safari/537.36'
    ],
    origin: [ 'http://localhost:8888' ],
    'access-control-request-headers': [ 'authorization,content-type' ],
    'access-control-request-method': [ 'POST' ],
    accept: [ '*/*' ],
    connection: [ 'close' ],
    host: [ 'localhost:8881' ],
    'x-nf-request-id': [ '01HCDY1Z1V9P61A2MHPA31GB40' ],
    'client-ip': [ '::1' ]
  },
  body: undefined,
  isBase64Encoded: true,
  rawUrl: 'http://localhost:8881/.netlify/functions/server',
  rawQuery: ''
}

I was wrong about there being an application/json content type in the (preflight) request. Could this be due to a difference in Apollo Client version? I have:

    "@apollo/client": "^3.8",
    "@apollo/link-context": "^2.0",

I don't see x-apollo-operation-name nor apollo-require-preflight headers either.

TTFN

P.S.

If I disable CSRF on the server (csrfPrevention: false), then I get the following error for the GraphQL request:

"message": "Apollo Server supports only GET/POST requests."

which is because the browser is sending an OPTIONS (preflight) request before the real request. I don't think there is a practical way to prevent the browser from sending OPTIONS requests. Nor is disabling CSRF detection recommended.

JeffML commented 11 months ago

@prabukamal, I seem to be having some luck with your approach. I had to add the line:

if (event.httpMethod === "OPTIONS") result.statusCode = 200

to the middleware event handler. Now it appears that I am hitting the GraphQL service, which is failing because my credential file is not getting loaded. But that is a different problem.

P. S.

Server is working on localhost:8881 and responding to requests from localhost:8888. Some modifications to @prabukamal's original solution:

onst graphqlHandler = startServerAndCreateLambdaHandler(
    server,
    handlers.createAPIGatewayProxyEventRequestHandler(),
    {
        context: async ({ event: { headers } }) => {
            const token = headers.authorization || "";
            const user = getUser(token);
            return { user };
        },
        middleware: [
            async (event) => {
                const origin = event.headers.origin;
                if (origin && allowedOrigins.includes(origin)) {
                    return async (result) => {
                        result.headers = {
                            ...result.headers,
                            "access-control-allow-headers": "*",
                            "access-control-allow-methods": "GET, POST",
                            "access-control-allow-origin": origin,
                        };
                        if (event.httpMethod === "OPTIONS")
                            result.statusCode = 200;
                    };
                } else {
                    return async (result) => {
                        result.headers = { ...result.headers };
                        result.statusCode = 403;
                    };
                }
            },
        ],
    }

Not sure my handler is strict as it should be, but it does reject requests from, say, localhost:8882

prabukamal commented 11 months ago

@JeffML In our case, we deploy the Lambda function using the Serverless Framework, and we enable CORS on the AWS API Gateway, as detailed in the following link:

https://www.serverless.com/blog/cors-api-gateway-survival-guide

We encountered similar issues a few weeks ago, but with this combination, it is now working perfectly for us.

asen-ruuby commented 10 months ago

I had difficulties, but I ended up to the following middleware which works as expected:

export const corsMiddleware: middleware.MiddlewareFn<
  handlers.RequestHandler<APIGatewayProxyEventV2, APIGatewayProxyStructuredResultV2>
> = async event => {
  const allowedOrigins = [
    // Put here the required origins
  ];
  const origin = event.headers.origin;

  if (origin && allowedOrigins.includes(origin)) {
    /* eslint-disable no-param-reassign */
    return result => {
      result.headers = {
        ...result.headers,
        "Access-Control-Allow-Origin": origin,
        "Access-Control-Allow-Methods": "POST, GET",
        "Access-Control-Allow-Headers": "*",
        "Access-Control-Allow-Credentials": "true",
      };

      if (event.requestContext.http.method === "OPTIONS") {
        result.body = undefined;
        result.statusCode = 204;
      }

      return Promise.resolve();
    };
    /* eslint-enable no-param-reassign */
  }
};

Hope it helps!

asen-ruuby commented 10 months ago

@JeffML the solution above is a combination of yours and @alnaranjo.

Apollo returns an error response if request method is OPTIONS and that's why I explicitly clear the response body.

JeffML commented 10 months ago

Yes! I took me too much time yesterday trying to figure out why my authentication was failing. Turns out OPTIONS doesn't send an authentication header.

So now I check the httpmethod header.

On Wed, Nov 8, 2023, 1:22 PM Asen Bozhilov @.***> wrote:

@JeffML https://github.com/JeffML the solution above is a combination of yours and @alnaranjo https://github.com/alnaranjo.

Apollo returns an error response if request method is OPTIONS and that's why I explicitly clear the response body.

— Reply to this email directly, view it on GitHub https://github.com/apollo-server-integrations/apollo-server-integration-aws-lambda/issues/85#issuecomment-1802699622, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2VY3MMKP2YXMFMXUDVEX3YDPZX3AVCNFSM6AAAAAAVVIP2NOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBSGY4TSNRSGI . You are receiving this because you were mentioned.Message ID: <apollo-server-integrations/apollo-server-integration-aws-lambda/issues/85/1802699622 @github.com>