-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(apigatewayv2): websocket api: api keys #16636
Conversation
I'm running into a bit of trouble when I try to add When I build
If I dont include this dependency, I don't get any errors during build and everything works just fine. I'm also importing I checked the objects the error is referring to, and they are all static readonly so I'm a bit confused. |
b45ca7f
to
4c168ce
Compare
that issue ended up being a circular dependency between apigateway and apigateway-integrations. I made some code and test adjustments. This pr should be ready for review now |
4c168ce
to
a0723b6
Compare
924c117
to
ebfd5f2
Compare
9b69ac1
to
729bb75
Compare
9aeac9c
to
0f3c41e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @alpacamybags118 - please see my comments below.
@@ -336,3 +336,12 @@ webSocketApi.addRoute('sendmessage', { | |||
}), | |||
}); | |||
``` | |||
|
|||
In addition, you can enable API key authentication to your WebSocket api: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a separate section called 'api keys' and explain them a little more? I'm not familiar with these.
/** | ||
* x-api-key type | ||
*/ | ||
X_API_KEY = '$request.header.x-api-key', | ||
|
||
/** | ||
* usageIdentifierKey type | ||
*/ | ||
USAGE_IDENTIFIER_KEY = '$context.authorizer.usageIdentifierKey' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add more documentation to these on what these mean?
@@ -26,6 +41,12 @@ export interface WebSocketApiProps { | |||
*/ | |||
readonly apiName?: string; | |||
|
|||
/** | |||
* An API key selection expression. Currently only supports '$request.header.x-api-key' and '$context.authorizer.usageIdentifierKey' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. Think of a user who is coming to the cdk and looking at these. They should be able to understand at a basic level what this is, whether they should use it, etc.
thanks for the feedback @nija-at will get to updating tonight |
0f3c41e
to
8c5de24
Compare
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
732b66a
to
3d1ddee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comments below.
@@ -0,0 +1,11 @@ | |||
#!/usr/bin/env node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need an integ test for this change. Drop.
/** | ||
* Represents the currently available API Key Selection Expressions | ||
*/ | ||
export enum ApiKeySelectionExpression { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use an enum-like class here instead of an enum - https://github.com/aws/aws-cdk/blob/master/docs/DESIGN_GUIDELINES.md#enums
/** | ||
* Represents the currently available API Key Selection Expressions | ||
*/ | ||
export enum ApiKeySelectionExpression { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module holds both http api and websocket api, so name this WebSocketApiKeySelection
* x-api-key type | ||
* This represents an API Key that is provided via an `x-api-key` header in the user request. | ||
*/ | ||
X_API_KEY = '$request.header.x-api-key', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call this HEADER_X_API_KEY and the other one AUTHORIZER_USAGE_IDENTIFIER_KEY
Thanks for the feedback again @nija-at - ill be on vacation for a week so I'll look at this once I'm back! |
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
---- This PR adds support for requiring an API Key on Websocket API routes. Specifically, it does the following: * Exposes `apiKeyRequired` on route object (defaults to false) * Exposes `apiKeySelectionExpression` on api object In addition, the following has been added: * Logic to ensure `apiKeySelectionExpression` falls within the two currently supported values * Created a few basic integration tests for the api and route objects for websockets *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR adds support for requiring an API Key on Websocket API routes. Specifically, it does the following:
apiKeyRequired
on route object (defaults to false)apiKeySelectionExpression
on api objectIn addition, the following has been added:
apiKeySelectionExpression
falls within the two currently supported valuesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license