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

Bug: CORS Support in start-api #4161

Closed
sravimohan opened this issue Aug 28, 2022 · 11 comments
Closed

Bug: CORS Support in start-api #4161

sravimohan opened this issue Aug 28, 2022 · 11 comments
Labels
contributors/welcome Contributors are welcome to work on this stage/waiting-for-release Fix has been merged to develop and is waiting for a release type/bug

Comments

@sravimohan
Copy link

sravimohan commented Aug 28, 2022

"sam local start-api" does not return cors headers in the lambda responses.

This issue was marked as fixed and closed in #323. but seems to have regressed and is no longer working any more.

The options request correctly returns the CORS headers. But the lambda response does not include "access-control-allow-origin" header.

The only workaround is to also manually include "access-control-allow-origin" header in the lambda responses.

Pl. note its works fine when deployed without any workaround.

$ sam --version
SAM CLI, version 1.55.0

minimal repo for reproducing issue
https://github.com/sravimohan/bug-aws-sam-cli-4161

template.yml

AWSTemplateFormatVersion: "2010-09-09"
Transform: AWS::Serverless-2016-10-31

Globals:
  Function:
    Timeout: 3
    Runtime: nodejs16.x
    Architectures:
      - x86_64

  HttpApi:
    CorsConfiguration:
      AllowHeaders:
        - Content-Type
        - Authorization
        - Access-Control-Allow-Origin
      AllowMethods:
        - HEAD
        - OPTIONS
        - GET
        - POST
      AllowOrigins:
        - http://localhost:3010

Resources:
  HelloWorldFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: hello-world/
      Handler: app.lambdaHandler
      Events:
        HelloWorld:
          Type: HttpApi
          Properties:
            Path: /hello
            Method: GET

function

let response;

exports.lambdaHandler = async (event, context) => {
    try {
        response = {
            'statusCode': 200,
            // work around for cors issue
            // headers: {
            //     "Access-Control-Allow-Origin": "http://localhost:3010",
            // },
            'body': JSON.stringify({
                message: 'hello world',
            })
        }
    } catch (err) {
        console.log(err);
        return err;
    }

    return response
};
@sravimohan sravimohan added stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. type/bug labels Aug 28, 2022
@jfuss
Copy link
Contributor

jfuss commented Aug 29, 2022

@sravimohan According to API Gateway docs, the Lambda is responsible for adding the access-control-allow-origin headers.

See https://docs.aws.amazon.com/apigateway/latest/developerguide/how-to-cors.html
"For a Lambda proxy integration or HTTP proxy integration, you can still set up the required OPTIONS response headers in API Gateway. However, your backend is responsible for returning the Access-Control-Allow-Origin and Access-Control-Allow-Headers headers, because a proxy integration doesn't return an integration response."

SAM only support proxy integrations, both in the cli and in the Spec. Am I miss understanding something?

@jfuss jfuss added blocked/more-info-needed More info is needed from the requester. If no response in 14 days, it will become stale. and removed stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Aug 29, 2022
@sravimohan
Copy link
Author

sravimohan commented Aug 29, 2022

Hi @jfuss, Thank you for looking into this.

This is a non-proxy integration as you can see from the template above. The exact same template deployed to AWS automatically adds 'access-control-allow-origin' to the response. whereas the running it locally with 'sam local start-api' does not.

Local
local

AWS
aws

SAM only support proxy integrations, both in the cli and in the Spec. Am I miss understanding something?

I am not clear what you mean by 'SAM only support proxy integrations'? The above template was generated through sam init and deployed to AWS using 'sam deploy'. It works as expected when deployed to AWS. The issue is running locally for development.

@jfuss
Copy link
Contributor

jfuss commented Aug 29, 2022

@sravimohan The odd thing to me is API Gateway has one thing documented (what we are doing locally) but has different behavior. I can't tell if this is something in the docs that needs updated or there is an issue with API Gateway.

This is a non-proxy integration as you can see from the template above.

As a note: You are using proxy integrations, given the last comment you made.

@jfuss
Copy link
Contributor

jfuss commented Aug 29, 2022

@sravimohan Actually, I was looking at the wrong docs 🤦‍♂️. You are using Http Api not Rest. So these are the right docs: https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-cors.html which explains the difference.

Are you willing to submit a PR to patch this?

@sravimohan
Copy link
Author

@jfuss, I am not very familiar with python. So may not be able get PR ready. I can give it a go if nobody else is keen.

@whats-a-handle
Copy link

Thanks @sravimohan +1, im having the same issue using AWS::Serverless::HttpApi running sam local start-api. Used your workaround to send the headers manually, and that solved it for now. I'm using an env var to add the headers when running sam local start-api.

@samaybhavsar
Copy link

I am facing a similar issue and this is really painfull. A workaround which I am using it cors middleware in middly

Hope this is helpful for people who are stuck.

@moelasmar moelasmar added contributors/welcome Contributors are welcome to work on this and removed maintainer/need-followup blocked/more-info-needed More info is needed from the requester. If no response in 14 days, it will become stale. labels Mar 29, 2023
@sleepwithcoffee
Copy link
Contributor

hi @sravimohan @jfuss @moelasmar I might be able to work on this. A few pointers into where should I look at could make this a bit faster.
Thanks,

@qingchm
Copy link
Contributor

qingchm commented Apr 6, 2023

Hi there @sleepwithcoffee thanks for the interest! https://github.com/aws/aws-sam-cli/blob/develop/samcli/commands/local/start_api/cli.py#L193 is the starting point of a sam local start-api call.

ultimately you will trace to this class where we create the flask application and the rules. LocalApigwService's port and host there seem to be the relevant part on CORS's allowed origins for example

@hawflau hawflau added the stage/waiting-for-release Fix has been merged to develop and is waiting for a release label May 18, 2023
@moelasmar moelasmar removed the stage/waiting-for-release Fix has been merged to develop and is waiting for a release label May 25, 2023
@moelasmar
Copy link
Contributor

We had to revert the fixing PR as it causes some regressions.

@moelasmar moelasmar added the stage/waiting-for-release Fix has been merged to develop and is waiting for a release label Aug 19, 2023
@github-actions
Copy link
Contributor

Patch is released in v1.96.0. Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributors/welcome Contributors are welcome to work on this stage/waiting-for-release Fix has been merged to develop and is waiting for a release type/bug
Projects
None yet
Development

No branches or pull requests

9 participants