Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(apigatewayv2): incorrect arn function causing unwanted behavior (#…
…33100) fixes: #33218 ### Reason for this change In websocket APIs in `aws-apigatewayv2`, the function arnForExecuteApi has essentially the same exact functionality as a REST API, which is not appropriate for Websockets which are fundamentally different. The way I found this issue was I used arnForExecuteApi to put the arn of the api into an IAM Role. The reason for this was because I was trying to use an IAM authorizer, which from a React standpoint meant signing iam credentials from my Cognito id pool using Amplify lib. When doing this I used arnForExecuteApi from CDK to write the policy, I did not include any arguments, just the default. The issue was that this was not working. I spent time diving deep on the issue in case it was the method in which I was signing the credentials, since I was not too familiar with this process. I also got the assistance of a Cloud Support Engineer from AWS to try and identify the problem. Shout-out Mike Sacks. The problem ended up being that that the resource policy was not correct. The policy that was generated by the function arnForExecuteApi was ``` "Resource": "arn:aws:execute-api:us-east-1:acc:apiId/*/*/*", ``` This is because the function itself has 3 values, stage, method and path, so when all are left in default states, this indicates `all` or `*`. So when adding each value at default you get `/*/*/*`, 3 x /*. This is an issue because Websocket arns are not structured like this, and as it turns out **iam prevents access if you have too many wild cards than applicable**. This means the reason for getting access denied was not because of my signed url, but because having 1 extra /* means that you no longer have permissions. Websocket arns are structured like this ``` arn:aws:execute-api:us-east-1:acc:apiId/*/$connect ``` In this example, * is the stage (this is what it shows on the console) and $connect is the `route`. You can add as many routes as you want, but the main ones by default are $connect, $disconnect and $default for no matching route. So if I want to grant an IAM role to have access to all routes and all stages, I would use this: ``` "Resource": "arn:aws:execute-api:us-east-1:acc:apiId/*/*", ``` Note 2 x /* instead of 3. Simply changing this by hand (deleting 2 characters) was enough to get the websocket to begin connecting via my signed url. ### Description of changes A re-write of the function for creating the arn. This is implemented as arnForExecuteApiV2, the original function has been changes to include the deprecated tag. This is to avoid making a breaking change since the new function only has 2 args and the original had 3. ```ts /** * Get the "execute-api" ARN. * * @default - The default behavior applies when no specific route, or stage is provided. * In this case, the ARN will cover all routes, and all stages of this API. * Specifically, if 'route' is not specified, it defaults to '*', representing all routes. * If 'stage' is not specified, it also defaults to '*', representing all stages. */ public arnForExecuteApiV2(route?: string, stage?: string): string { return Stack.of(this).formatArn({ service: 'execute-api', resource: this.apiId, arnFormat: ArnFormat.SLASH_RESOURCE_NAME, resourceName: `${stage ?? '*'}/${route ?? '*'}`, }); } ``` I removed "Method" and "Path" entirely since these are not even appropriate to use as terms for websockets. I used Route instead. ### Description of how you validated changes Updated Tests, there were 4 tests before: * get arnForExecuteApi * is now using route, and intentionally uses a route with no `$` to check that the `$` is being added correctly. * get arnForExecuteApi with default values * is now using route * get arnForExecuteApi with ANY method (removed) * doesn't make any sense here because "ANY" is not a term used with websockets, and method does not exist. Thus, removed this test * throws when call arnForExecuteApi method with specifing a string that does not start with / for the path argument. (removed) * This test is checking for a specific format for path, which is not needed since the route can be anything. Also path does not exist. This leaves 2 total tests now. Added a new integ function, `integ.api-grant-invoke.ts` and used --update-on-failed with my personal account to bootstrap new snapshots to match. For this test I included an iam role and 2 arns, one with default settings and one with `.arnForExecuteApi('connect', 'prod')` Intentionally left off the `$` to check that it's being added. All tests and integ are passing. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
- Loading branch information
