Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

(aws-apigateway): CORS list always picks first element #30453

Closed
pviolette3 opened this issue Jun 5, 2024 · 5 comments
Closed

(aws-apigateway): CORS list always picks first element #30453

pviolette3 opened this issue Jun 5, 2024 · 5 comments
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@pviolette3
Copy link

pviolette3 commented Jun 5, 2024

Describe the bug

When specifying a list of allowed_origins to the RestApi;s default_cors_preflight_options then we always get back the first element of the list when the API runs the preflight OPTIONS method. Even though the mapping template looks like it would correctly allow the specified list of origins, the header seems to override the mapping template so the mapping template is not used.

Expected Behavior

CORS preflight should return the origin from the request, not the hard-coded header, when the header matches the mapping template.

Current Behavior

The Access-Control-Allow-Origin is hard-coded to the first element of the list, and doesn't seem to run the mapping template.

Example from the console
image

with the response

image

Possibly buggy code:
https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-apigateway/lib/resource.ts#L214-L217

Reproduction Steps

cors_allowed_origins = ["http://localhost:8080", "https://google.com"]
apigateway.RestApi(
            self,
            default_cors_preflight_options=apigateway.CorsOptions(
                # TODO: This API is not working correctly.
                # When we pass a list here such as `cors_allowed_origins` as a list the integration uses the first element of the list.
                # * When the list is apigateway.Cors.ALL_ORIGINS it is "*" and works correctly
                # * But when we pass our list it just selects localhost:8080 which will work only from localhost.
                allow_origins=cors_allowed_origins,
                allow_methods=apigateway.Cors.ALL_METHODS,
                allow_credentials=True,
                allow_headers=apigateway.Cors.DEFAULT_HEADERS,
            )
            if cors_allowed_origins
            else None,
        )

Deploy this API. Then use the OPTIONS request tester with the following headers.

Origin: https://google.com
Access-Control-Request-Method: GET

You will get back Access-Control-Allow-Origin: http://locahost:8080 and not the origin from the request (https://google.com)

Possible Solution

Remove this code to set the header if the user has passed in a list of origins.

This code will work by accident if the user passes ["*"] because that will set the Access-Control-Allow-Origin header to * but it will not work for a hard-coded list where the first element is localhost:8080

https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-apigateway/lib/resource.ts#L214-L217

    // we use the first origin here and if there are more origins in the list, we
    // will match against them in the response velocity template
    const initialOrigin = options.allowOrigins[0];
    headers['Access-Control-Allow-Origin'] = `'${initialOrigin}'`;

Additional Information/Context

No response

CDK CLI Version

2.144.0 (build 5fb15bc)

Framework Version

No response

Node.js Version

v20.12.0

OS

WSL ( 5.15.146.1-microsoft-standard-WSL2 #1 SMP Thu Jan 11 04:09:03 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux)

Language

Python

Language Version

aws-cdk-lib==2.138.0

Other information

No response

@pviolette3 pviolette3 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 5, 2024
@github-actions github-actions bot added the @aws-cdk/aws-apigateway Related to Amazon API Gateway label Jun 5, 2024
@khushail khushail added needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels Jun 10, 2024
@khushail
Copy link
Contributor

khushail commented Jul 12, 2024

Hi @pviolette3 , thanks for reporting this and sharing the detailed steps for Repro.

I can confirm that it always picks the first element as I tested with these parameters and it was dafaulted to localhost

Origin: https://google.com
Access-Control-Request-Method: GET" 

in Typescript language-

Screenshot 2024-07-12 at 4 28 20 PM

@khushail khushail added p1 effort/small Small work item – less than a day of effort and removed needs-reproduction This issue needs reproduction. labels Jul 12, 2024
@nmussy
Copy link
Contributor

nmussy commented Jul 16, 2024

Seems to be related to #25923

@stm29
Copy link
Contributor

stm29 commented Aug 16, 2024

@khushail / @pviolette3
I am trying to understand what are we trying to achieve here?

  1. If we pass Origin, which is not present in set of allowlisted headers, we are getting first Origin present the allowlist as response.
  2. If present, it is returning the Exactly Allowed Origin by the help of Mapping Templates.

NOTE:

Both the testing was done from Console, In Console, Testing is case sensitive. For the same URL, When tested with CURL, Giving spaces doesn't make any issue. But in Console-UI, on Integration Response Tab, providing headers with space affects this Expected output. (Look for spaces below). In CURL, it working as expected without any issue.

  1. curl -X OPTIONS https://[REDACTED].execute-api.us-west-2.amazonaws.com/prod -H "Origin: https://google.com" -i
  2. curl -X OPTIONS https://[REDACTED].execute-api.us-west-2.amazonaws.com/prod -H "Origin: https://google.com" -i
  3. curl -X OPTIONS https://[REDACTED].execute-api.us-west-2.amazonaws.com/prod -H "Origin:https://google.com" -i

But when same is tested in UI, it shows different response.

  • With Space - Origin: https://google.com

Screenshot 2024-08-16 at 7 47 09 PM
  • Without Space - Origin:https://google.com

Screenshot 2024-08-16 at 7 47 22 PM

In Short, I feel, this particular functionality is working as it intended (and needs to be). Please correct me / give me exact details to reproduce.

When I tried by myself, I can see, When Origin:https://google.com is passed in Request, I can see Origin is being updated properly, when it is present in request.

When Present

stm29@pc1:Desktop$
stm29@pc1:Desktop$ curl -X OPTIONS https://[REDACTED].execute-api.us-west-2.amazonaws.com/prod -H "Origin: https://google.com" -i
HTTP/2 204
date: Fri, 16 Aug 2024 13:49:01 GMT
access-control-allow-credentials: true
x-amzn-requestid: [REDACTED]
access-control-allow-origin: https://google.com
access-control-allow-headers: Content-Type,X-Amz-Date,Authorization,X-Api-Key,X-Amz-Security-Token,X-Amz-User-Agent
x-amz-apigw-id: [REDACTED]
vary: Origin
access-control-allow-methods: OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD
x-cache: Miss from cloudfront
via: 1.1 [REDACTED]
x-amz-cf-pop: [REDACTED]
x-amz-cf-id: [REDACTED]
stm29@pc1:Desktop$
stm29@pc1:Desktop$

When Not present

stm29@pc1:Desktop$
stm29@pc1:Desktop$ curl -X OPTIONS https://[REDACTED].execute-api.us-west-2.amazonaws.com/prod -H "Origin: https://some_random_not_present.com" -i
HTTP/2 204
date: Fri, 16 Aug 2024 13:49:27 GMT
access-control-allow-credentials: true
x-amzn-requestid: [REDACTED]
access-control-allow-origin: http://localhost:8080
access-control-allow-headers: Content-Type,X-Amz-Date,Authorization,X-Api-Key,X-Amz-Security-Token,X-Amz-User-Agent
x-amz-apigw-id: [REDACTED]
vary: Origin
access-control-allow-methods: OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD
x-cache: Miss from cloudfront
via: 1.1 [REDACTED]
x-amz-cf-pop: [REDACTED]
x-amz-cf-id: [REDACTED]
stm29@pc1:Desktop$
stm29@pc1:Desktop$

TS Code

import * as cdk from 'aws-cdk-lib';
import { Construct } from 'constructs';
import * as apigateway from 'aws-cdk-lib/aws-apigateway';

export class MyCdkProjectStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const allowedOrigins = [
      'http://localhost:8080',
      'https://google.com',
    ];

    new apigateway.RestApi(this, 'MyApi', {
      restApiName: 'MyService',
      description: 'This service serves my API.',
      defaultCorsPreflightOptions: {
        allowOrigins: allowedOrigins,
        allowMethods: apigateway.Cors.ALL_METHODS,
        allowHeaders: apigateway.Cors.DEFAULT_HEADERS,
        allowCredentials: true,
      },
    });
  }
}

@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 5, 2024

Thanks for the research @stm29! You figured it out exactly.

It's not entirely clear from the UI, but the API Gateway console expects no space between header name and header value:

2024-09-05 at 15 17

This means that if you do include a space:

Origin: https://google.com

That the value of the $origin header is actually " https://google.com" (note the space at the start), and then the $origin == "https://google.com" equality test fails.

This is not the behavior of the HTTP protocol, where the spaces are stripped, so it will work properly there.

That the console doesn't behave the way HTTP behaves can be considered surprising. I recommend you report this as a bug to the API Gateway team (for example by using the Feedback button at the bottom of the screen).

Closing this issue as a non-issue.

@rix0rrr rix0rrr closed this as completed Sep 5, 2024
Copy link

github-actions bot commented Sep 5, 2024

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 5, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

No branches or pull requests

5 participants