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

CorsPreflightOptions: allow_origins does not work with more than 1 value #25923

Closed
gabriels1234 opened this issue Jun 9, 2023 · 7 comments
Closed
Labels
@aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@gabriels1234
Copy link

Describe the bug

The Setup I have to use requires CORS, and allow_credentials:true. That means that allow_origins cannot be ["*"].

The problem is, given that we want to allow originS (more than one), the HttpApi returns both origins as allowed.
BUT, the browsers' specification requires only one value.
Every Backend framework knows that the accepted value for Access-Control-Allow-Origin is only one.

example: cors js library:

      isAllowed = isOriginAllowed(requestOrigin, options.origin);
      // reflect origin
      headers.push([{
        key: 'Access-Control-Allow-Origin',
        value: isAllowed ? requestOrigin : false // <-- requestOrigin is only one.
      }]); 

example: starlette library (used by fastApi):

        if self.is_allowed_origin(origin=requested_origin):
            if self.preflight_explicit_allow_origin:
                headers["Access-Control-Allow-Origin"] = requested_origin

Expected Behavior

when setting allow_origins = ["https://sitea.com", "https://siteb.com"]
the response headers when calling from sitea should be:

Access-Control-Allow-Origin:https://sitea.com

Current Behavior

when setting allow_origins = ["https://sitea.com", "https://siteb.com"]
the response headers when calling from sitea is actually:

Access-Control-Allow-Origin:https://sitea.com,https://siteb.com

Reproduction Steps

requirements.txt:

aws-cdk-lib==2.83.0
aws-cdk.aws-apigatewayv2-alpha==2.83.0a0
aws-cdk.aws-apigatewayv2-authorizers-alpha==2.83.0a0
aws-cdk.aws-apigatewayv2-integrations-alpha==2.83.0a0
from aws_cdk import (
    aws_apigatewayv2_alpha as apigwv2a,
)
...
        cors_preflight = apigwv2a.CorsPreflightOptions(
            allow_headers=["Authorization"],
            allow_methods=[
                apigwv2a.CorsHttpMethod.GET,
                apigwv2a.CorsHttpMethod.HEAD,
                apigwv2a.CorsHttpMethod.OPTIONS,
                apigwv2a.CorsHttpMethod.POST,
            ],
            allow_origins=["http://127.0.0.1:5173", "http://localhost:3000"],
            allow_credentials=True,
            max_age=Duration.days(10),
        )
       http_api = apigwv2a.HttpApi(scope, id, cors_preflight=cors_preflight)

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.83.0 (build 0fd7f2b)

Framework Version

No response

Node.js Version

v16.17.0

OS

Mac OS 13.4

Language

Python

Language Version

Python 3.10

Other information

No response

@gabriels1234 gabriels1234 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 9, 2023
@github-actions github-actions bot added the @aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 label Jun 9, 2023
@gabriels1234
Copy link
Author

image

image

Perhaps there's a way to override that?

@peterwoodworth
Copy link
Contributor

Would be a bug with the service API and CloudFormation as well, the header certainly can't contain multiple values.

In the expected behavior section of your report, why would you pass in multiple values to this parameter and expect only one to show up in the header?

@peterwoodworth peterwoodworth added p2 needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. and removed needs-triage This issue or PR still needs to be triaged. labels Jun 9, 2023
@gabriels1234
Copy link
Author

@peterwoodworth Well, it's a clear need in many projects to allow multiple origins. (worth mentioning the parameter is called allow_origins). Every CORS plugin(/framework) knows how to handle is. if the Origin is in the allowed origins, then return the header Access-Control-Allow-Origin with the request Origin.

I would say that even CloudFormation is just fine, because this has to be evaluated by the API Gateway itself.
Unfortunately, all the examples and questions out there speak about allow_origins = '*' which is not allowed when allow_credentials is set to True.

Any Ideas?

@Tietew
Copy link
Contributor

Tietew commented Jun 12, 2023

Works correctly for me.

TypeScript

    const api = new apigatewayv2.HttpApi(this, 'HttpApi', {
      corsPreflight: {
        allowMethods: [
          apigatewayv2.CorsHttpMethod.GET,
          apigatewayv2.CorsHttpMethod.PUT,
          apigatewayv2.CorsHttpMethod.OPTIONS,
        ],
        allowOrigins: ['https://app.example.com', 'http://localhost:3000'],
        allowHeaders: ['content-type', 'authorization'],
        allowCredentials: true,
      },
    });

Template

  "HttpApiXXXXXXXX": {
   "Type": "AWS::ApiGatewayV2::Api",
   "Properties": {
    "CorsConfiguration": {
     "AllowCredentials": true,
     "AllowHeaders": [
      "content-type",
      "authorization"
     ],
     "AllowMethods": [
      "GET",
      "PUT",
      "OPTIONS"
     ],
     "AllowOrigins": [
      "https://app.example.com",
      "http://localhost:3000"
     ]
    },
    "Name": "HttpApi",
    "ProtocolType": "HTTP"
   }
  },

Actual response of OPTIONS

$ curl -v https://xxxxxxxxx.execute-api.ap-northeast-1.amazonaws.com/ -X OPTIONS ...
OPTIONS / HTTP/2
access-control-request-method: PUT
origin: http://localhost:3000
...

HTTP/2 204
access-control-allow-origin: http://localhost:3000
access-control-allow-methods: GET,OPTIONS,PUT
access-control-allow-headers: authorization,content-type
access-control-allow-credentials: true
...

@gabriels1234
Copy link
Author

@Tietew Well, that's actually great news!
I was getting the error I mentioned at the local "start-api" level.
I will test by deploying it, and if that works, will find a workaround when testing locally.
Thanks for putting the time into it.
Will update you here soon.

@peterwoodworth
Copy link
Contributor

Sorry, I'm not super familiar with API Gateway as a service, I don't see a key difference between the snippet posted that works and the one that doesn't. I don't see how these two use allowOrigins differently, or what APIGateway is expected to do under the hood when setting the allowOrigins property if not just adding these to the allow-origin header?

@gabriels1234 if your template appears to look good and you're still running into the error, please let me know.

@peterwoodworth peterwoodworth added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 12, 2023
@github-actions
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

3 participants