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

fix(apigatewayv2): defaultAuthorizer cannot be applied to HttpRoute #27576

Merged
merged 28 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
21785e2
docs(secretsmanager): doc when automaticallyAfter for RotationSchedul…
go-to-k Oct 17, 2023
de950c7
fix(apigatewayv2): can't apply defaultAuthorizer to HttpRoute
go-to-k Oct 17, 2023
1efb3d6
add an integ test
go-to-k Oct 17, 2023
1e10a0d
refactor for route.ts
go-to-k Oct 17, 2023
21e4aee
fix miss
go-to-k Oct 17, 2023
ebacc8e
change integ tests for circular dependency
go-to-k Oct 19, 2023
aa00748
tweak
go-to-k Oct 19, 2023
afb9bda
tweak
go-to-k Oct 19, 2023
df3155b
tweak
go-to-k Oct 19, 2023
9062e3a
add comments
go-to-k Oct 19, 2023
4fc75f4
apply defaultAuthorizationScopes
go-to-k Oct 19, 2023
5c2208f
change tests
go-to-k Oct 19, 2023
81bcb9c
change comments and an integ
go-to-k Oct 19, 2023
4e76464
change integ snapshots
go-to-k Oct 20, 2023
ae2a40c
add properties to IHttpApi
go-to-k Oct 23, 2023
2a76067
change integs
go-to-k Oct 23, 2023
5df276c
changed integ.lambda.ts
go-to-k Oct 23, 2023
e3e3f9e
change integ.user-pool
go-to-k Oct 23, 2023
afd941c
change integ.lambda.ts
go-to-k Oct 23, 2023
5f248aa
change integ.lambda
go-to-k Oct 23, 2023
4e5942f
change integ.lambda before modification
go-to-k Oct 23, 2023
0a188ee
change integs
go-to-k Oct 23, 2023
9e09c7c
change integ.user-pool
go-to-k Oct 23, 2023
40819a1
change align and docs
go-to-k Oct 24, 2023
ee11394
change integ.lambda
go-to-k Oct 24, 2023
7ea3c89
change message
go-to-k Oct 24, 2023
0bb478e
tweak for msgs
go-to-k Oct 24, 2023
375b130
Merge branch 'main' into fix/apgwv2-route-authorizer
mergify[bot] Oct 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions packages/@aws-cdk/aws-apigatewayv2-alpha/lib/http/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,20 @@ export interface IHttpApi extends IApi {
*/
readonly httpApiId: string;

/**
* Default Authorizer to applied to all routes in the gateway
* @attribute
* @default - No authorizer
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
* Default Authorizer to applied to all routes in the gateway
* @attribute
* @default - No authorizer
*/
/**
* Default Authorizer applied to all routes in the gateway.
*
* @attribute
* @default - no default authorizer
*/

Can you please align this as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check.

40819a1

readonly defaultAuthorizer?: IHttpRouteAuthorizer;

/**
* Default OIDC scopes attached to all routes in the gateway, unless explicitly configured on the route.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Default OIDC scopes attached to all routes in the gateway, unless explicitly configured on the route.
* Default OIDC scopes attached to all routes in the gateway, unless explicitly configured on the route.
* The scopes are used with a COGNITO_USER_POOLS authorizer to authorize the method invocation.
*

Can you please align this as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check.

I also added a JWT authorizer to this doc.

40819a1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please provide documentation that describes the usage with a JWT authorizer?
I was only able to verify the COGNITO_USER_POOLS authorizer.

Copy link
Contributor Author

@go-to-k go-to-k Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... I tried in the integ.lambda with defaultAuthorizationScopes in httpApiWithDefaultAuthorizer, CFn occurred the following error message.

I'll see if I can find any documentation.

UPDATE_ROLLBACK_COMPLETE: Resource handler returned message: "Invalid Route authorization type specified. Authorization Scopes are only valid for COGNITO_USER_POOLS and JWT authorization types (Service: AmazonApiGatewayV2; Status Code: 400; Error Code: BadRequestException; Request ID: xxxxxx; Proxy: null)" (RequestToken: xxxxxx, HandlerErrorCode: GeneralServiceException)

Copy link
Contributor Author

@go-to-k go-to-k Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is If you specify the AuthorizerId property, specify CUSTOM or COGNITO_USER_POOLS for this property. in the CFn doc.

I guess this CFn message represents CUSTOM COGNITO_USER_POOLS as JWT.

What should we do in CDK doc?

Copy link
Contributor Author

@go-to-k go-to-k Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm starting to feel that COGNITO_USER_POOLS is all I need.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that COGNITO_USER_POOLS is sufficient since it generates a JWT authorizer.

Copy link
Contributor Author

@go-to-k go-to-k Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the meantime, please review again as I have added only COGNITO_USER_POOLS for the message.

* @attribute
* @default - no default authorization scopes
*/
readonly defaultAuthorizationScopes?: string[];

/**
* Metric for the number of client-side errors captured in a given period.
*
Expand Down Expand Up @@ -340,8 +354,8 @@ export class HttpApi extends HttpApiBase {

private readonly _apiEndpoint: string;

private readonly defaultAuthorizer?: IHttpRouteAuthorizer;
private readonly defaultAuthorizationScopes?: string[];
public readonly defaultAuthorizer?: IHttpRouteAuthorizer;
public readonly defaultAuthorizationScopes?: string[];

constructor(scope: Construct, id: string, props?: HttpApiProps) {
super(scope, id);
Expand Down
7 changes: 4 additions & 3 deletions packages/@aws-cdk/aws-apigatewayv2-alpha/lib/http/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ export class HttpRoute extends Resource implements IHttpRoute {
scope: this,
});

this.authBindResult = props.authorizer?.bind({
const authorizer = props.authorizer ?? this.httpApi.defaultAuthorizer;
this.authBindResult = authorizer?.bind({
route: this,
scope: this.httpApi instanceof Construct ? this.httpApi : this, // scope under the API if it's not imported
});
Expand All @@ -204,10 +205,10 @@ export class HttpRoute extends Resource implements IHttpRoute {

let authorizationScopes = this.authBindResult?.authorizationScopes;

if (this.authBindResult && props.authorizationScopes) {
if (this.authBindResult && (props.authorizationScopes || this.httpApi.defaultAuthorizationScopes)) {
authorizationScopes = Array.from(new Set([
...authorizationScopes ?? [],
...props.authorizationScopes,
...props.authorizationScopes ?? this.httpApi.defaultAuthorizationScopes ?? [],
]));
}

Expand Down
90 changes: 90 additions & 0 deletions packages/@aws-cdk/aws-apigatewayv2-alpha/test/http/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,96 @@ describe('HttpRoute', () => {
});
});

test('can create route without an authorizer when api has defaultAuthorizer', () => {
const stack = new Stack();

const authorizer = new DummyAuthorizer();
const httpApi = new HttpApi(stack, 'HttpApi', {
defaultAuthorizer: authorizer,
defaultAuthorizationScopes: ['read:books'],
});

const route = new HttpRoute(stack, 'HttpRoute', {
httpApi,
integration: new DummyIntegration(),
routeKey: HttpRouteKey.with('/books', HttpMethod.GET),
});

Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::Integration', {
ApiId: stack.resolve(httpApi.apiId),
IntegrationType: 'HTTP_PROXY',
PayloadFormatVersion: '2.0',
IntegrationUri: 'some-uri',
});

Template.fromStack(stack).resourceCountIs('AWS::ApiGatewayV2::Authorizer', 1);
Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::Route', {
AuthorizerId: stack.resolve(authorizer.bind({ scope: stack, route: route }).authorizerId),
AuthorizationType: 'JWT',
AuthorizationScopes: ['read:books'],
});
});

test('authorizationScopes can be applied to route without authorizer when api has defaultAuthorizer', () => {
const stack = new Stack();

const authorizer = new DummyAuthorizer();
const httpApi = new HttpApi(stack, 'HttpApi', {
defaultAuthorizer: authorizer,
});

const route = new HttpRoute(stack, 'HttpRoute', {
httpApi,
integration: new DummyIntegration(),
routeKey: HttpRouteKey.with('/books', HttpMethod.GET),
authorizationScopes: ['read:books'],
});

Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::Integration', {
ApiId: stack.resolve(httpApi.apiId),
IntegrationType: 'HTTP_PROXY',
PayloadFormatVersion: '2.0',
IntegrationUri: 'some-uri',
});

Template.fromStack(stack).resourceCountIs('AWS::ApiGatewayV2::Authorizer', 1);
Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::Route', {
AuthorizerId: stack.resolve(authorizer.bind({ scope: stack, route: route }).authorizerId),
AuthorizationType: 'JWT',
AuthorizationScopes: ['read:books'],
});
});

test('defaultAuthorizationScopes can be applied to route', () => {
const stack = new Stack();

const authorizer = new DummyAuthorizer();
const httpApi = new HttpApi(stack, 'HttpApi', {
defaultAuthorizationScopes: ['read:books'],
});

const route = new HttpRoute(stack, 'HttpRoute', {
httpApi,
integration: new DummyIntegration(),
routeKey: HttpRouteKey.with('/books', HttpMethod.GET),
authorizer,
});

Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::Integration', {
ApiId: stack.resolve(httpApi.apiId),
IntegrationType: 'HTTP_PROXY',
PayloadFormatVersion: '2.0',
IntegrationUri: 'some-uri',
});

Template.fromStack(stack).resourceCountIs('AWS::ApiGatewayV2::Authorizer', 1);
Template.fromStack(stack).hasResourceProperties('AWS::ApiGatewayV2::Route', {
AuthorizerId: stack.resolve(authorizer.bind({ scope: stack, route: route }).authorizerId),
AuthorizationType: 'JWT',
AuthorizationScopes: ['read:books'],
});
});

test('can attach additional scopes to a route with an authorizer attached', () => {
const stack = new Stack();
const httpApi = new HttpApi(stack, 'HttpApi');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as path from 'path';
import { HttpApi, HttpMethod } from '@aws-cdk/aws-apigatewayv2-alpha';
import { HttpApi, HttpMethod, HttpRoute, HttpRouteKey } from '@aws-cdk/aws-apigatewayv2-alpha';
import { HttpLambdaIntegration } from '@aws-cdk/aws-apigatewayv2-integrations-alpha';
import * as cognito from 'aws-cdk-lib/aws-cognito';
import * as lambda from 'aws-cdk-lib/aws-lambda';
Expand All @@ -16,11 +16,17 @@ import { HttpUserPoolAuthorizer } from '../../lib';
const app = new App();
const stack = new Stack(app, 'AuthorizerInteg');

const httpApi = new HttpApi(stack, 'MyHttpApi');

const userPool = new cognito.UserPool(stack, 'userpool');
const userPoolForDefaultAuthorizer = new cognito.UserPool(stack, 'userpoolForDefaultAuthorizer');

const authorizer = new HttpUserPoolAuthorizer('UserPoolAuthorizer', userPool);
const authorizerWithDefaultAuthorizer = new HttpUserPoolAuthorizer('UserPoolAuthorizerWithDefaultAuthorizer', userPoolForDefaultAuthorizer);

const httpApi = new HttpApi(stack, 'MyHttpApi');
const httpApiWithDefaultAuthorizer = new HttpApi(stack, 'MyHttpApiWithDefaultAuthorizer', {
defaultAuthorizer: authorizerWithDefaultAuthorizer,
defaultAuthorizationScopes: ['scope1', 'scope2'],
});

const handler = new lambda.Function(stack, 'lambda', {
runtime: lambda.Runtime.NODEJS_18_X,
Expand All @@ -34,3 +40,9 @@ httpApi.addRoutes({
integration: new HttpLambdaIntegration('RootIntegratin', handler),
authorizer,
});

new HttpRoute(stack, 'Route', {
httpApi: httpApiWithDefaultAuthorizer,
routeKey: HttpRouteKey.with('/v1/mything/{proxy+}', HttpMethod.ANY),
integration: new HttpLambdaIntegration('RootIntegration', handler),
});
Loading