Skip to content

Commit

Permalink
fix(apigateway): authorizer is not attached to RestApi across projects
Browse files Browse the repository at this point in the history
When an Authorizer is defined in one node project, imported into another
where it is wired up with a RestApi, synthesis throws the error
'Authorizer must be attached to a RestApi'.

The root cause of this error is the `instanceof` check. If a user has
their project set up in a way that more than one instance of the
`@aws-cdk/aws-apigateway` module is present in their `node_modules/`
folder, even if they are of the exact same version, type checking is
bound to fail.

Switching instead to a symbol property based comparison, so that type
information survives across installations of the module.

fixes #7377
  • Loading branch information
Niranjan Jayakar committed Apr 24, 2020
1 parent fda05c3 commit 5d83a33
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 8 deletions.
17 changes: 16 additions & 1 deletion packages/@aws-cdk/aws-apigateway/lib/authorizer.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,29 @@
import { Resource } from '@aws-cdk/core';
import { Construct, Resource } from '@aws-cdk/core';
import { AuthorizationType } from './method';
import { RestApi } from './restapi';

const AUTHORIZER_SYMBOL = Symbol.for('@aws-cdk/aws-apigateway.Authorizer');

/**
* Base class for all custom authorizers
*/
export abstract class Authorizer extends Resource implements IAuthorizer {
/**
* Return whether the given object is an Authorizer.
*/
public static isAuthorizer(x: any): x is Authorizer {
return x !== null && typeof(x) === 'object' && AUTHORIZER_SYMBOL in x;
}

public readonly abstract authorizerId: string;
public readonly authorizationType?: AuthorizationType = AuthorizationType.CUSTOM;

constructor(scope: Construct, id: string) {
super(scope, id);

Object.defineProperty(this, AUTHORIZER_SYMBOL, { value: true });
}

/**
* Called when the authorizer is used from a specific REST API.
* @internal
Expand Down
44 changes: 38 additions & 6 deletions packages/@aws-cdk/aws-apigateway/lib/method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export interface MethodOptions {
* for the integration response to be correctly mapped to a response to the client.
* @see https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-method-settings-method-response.html
*/
readonly methodResponses?: MethodResponse[]
readonly methodResponses?: MethodResponse[];

/**
* The request parameters that API Gateway accepts. Specify request parameters
Expand All @@ -65,15 +65,45 @@ export interface MethodOptions {
readonly requestParameters?: { [param: string]: boolean };

/**
* The resources that are used for the response's content type. Specify request
* models as key-value pairs (string-to-string mapping), with a content type
* as the key and a Model resource name as the value
* The models which describe data structure of request payload. When
* combined with `requestValidator` or `requestValidatorOptions`, the service
* will validate the API request payload before it reaches the API's Integration (including proxies).
* Specify `requestModels` as key-value pairs, with a content type
* (e.g. `'application/json'`) as the key and an API Gateway Model as the value.
*
* @example
*
* const userModel: apigateway.Model = api.addModel('UserModel', {
* schema: {
* type: apigateway.JsonSchemaType.OBJECT
* properties: {
* userId: {
* type: apigateway.JsonSchema.STRING
* },
* name: {
* type: apigateway.JsonSchema.STRING
* }
* },
* required: ['userId']
* }
* });
* api.root.addResource('user').addMethod('POST',
* new apigateway.LambdaIntegration(userLambda), {
* requestModels: {
* 'application/json': userModel
* }
* }
* );
*
* @see https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-method-settings-method-request.html#setup-method-request-model
*/
readonly requestModels?: { [param: string]: IModel };

/**
* The ID of the associated request validator.
* Only one of `requestValidator` or `requestValidatorOptions` must be specified.
* Works together with `requestModels` or `requestParameters` to validate
* the request before it reaches integration like Lambda Proxy Integration.
* @default - No default validator
*/
readonly requestValidator?: IRequestValidator;
Expand All @@ -84,11 +114,13 @@ export interface MethodOptions {
* @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-method.html#cfn-apigateway-method-authorizationscopes
* @default - no authorization scopes
*/
readonly authorizationScopes?: string[]
readonly authorizationScopes?: string[];

/**
* Request validator options to create new validator
* Only one of `requestValidator` or `requestValidatorOptions` must be specified.
* Works together with `requestModels` or `requestParameters` to validate
* the request before it reaches integration like Lambda Proxy Integration.
* @default - No default validator
*/
readonly requestValidatorOptions?: RequestValidatorOptions;
Expand Down Expand Up @@ -153,7 +185,7 @@ export class Method extends Resource {
`which is different from what is required by the authorizer [${authorizer.authorizationType}]`);
}

if (authorizer instanceof Authorizer) {
if (Authorizer.isAuthorizer(authorizer)) {
authorizer._attachToApi(this.restApi);
}

Expand Down
35 changes: 34 additions & 1 deletion packages/@aws-cdk/aws-apigateway/test/authorizers/test.lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as iam from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
import { Duration, Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import { AuthorizationType, IdentitySource, RequestAuthorizer, RestApi, TokenAuthorizer } from '../../lib';
import { AuthorizationType, Authorizer, IdentitySource, RequestAuthorizer, RestApi, TokenAuthorizer } from '../../lib';

export = {
'default token authorizer'(test: Test) {
Expand Down Expand Up @@ -301,4 +301,37 @@ export = {

test.done();
},

'token authorizer is of type Authorizer'(test: Test) {
const stack = new Stack();

const handler = new lambda.Function(stack, 'token', {
code: lambda.Code.fromInline('foo'),
runtime: lambda.Runtime.NODEJS_12_X,
handler: 'index.handler',
});
const authorizer = new TokenAuthorizer(stack, 'authorizer', { handler });

test.ok(Authorizer.isAuthorizer(authorizer), 'TokenAuthorizer is not of type Authorizer');

test.done();
},

'request authorizer is of type Authorizer'(test: Test) {
const stack = new Stack();

const handler = new lambda.Function(stack, 'token', {
code: lambda.Code.fromInline('foo'),
runtime: lambda.Runtime.NODEJS_12_X,
handler: 'index.handler',
});
const authorizer = new RequestAuthorizer(stack, 'authorizer', {
handler,
identitySources: [ IdentitySource.header('my-header') ],
});

test.ok(Authorizer.isAuthorizer(authorizer), 'RequestAuthorizer is not of type Authorizer');

test.done();
},
};
55 changes: 55 additions & 0 deletions packages/@aws-cdk/aws-apigateway/test/test.authorizer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { Code, Function, Runtime } from '@aws-cdk/aws-lambda';
import { Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import { Authorizer, IdentitySource, RequestAuthorizer, RestApi, TokenAuthorizer } from '../lib';

export = {
'isAuthorizer correctly detects an instance of type Authorizer'(test: Test) {
class MyAuthorizer extends Authorizer {
public readonly authorizerId = 'test-authorizer-id';
public _attachToApi(_: RestApi): void {
// do nothing
}
}
const stack = new Stack();
const authorizer = new MyAuthorizer(stack, 'authorizer');

test.ok(Authorizer.isAuthorizer(authorizer), 'type Authorizer expected but is not');
test.ok(!Authorizer.isAuthorizer(stack), 'type Authorizer found, when not expected');

test.done();
},

'token authorizer is of type Authorizer'(test: Test) {
const stack = new Stack();

const handler = new Function(stack, 'token', {
code: Code.fromInline('foo'),
runtime: Runtime.NODEJS_12_X,
handler: 'index.handler',
});
const authorizer = new TokenAuthorizer(stack, 'authorizer', { handler });

test.ok(Authorizer.isAuthorizer(authorizer), 'TokenAuthorizer is not of type Authorizer');

test.done();
},

'request authorizer is of type Authorizer'(test: Test) {
const stack = new Stack();

const handler = new Function(stack, 'token', {
code: Code.fromInline('foo'),
runtime: Runtime.NODEJS_12_X,
handler: 'index.handler',
});
const authorizer = new RequestAuthorizer(stack, 'authorizer', {
handler,
identitySources: [ IdentitySource.header('my-header') ],
});

test.ok(Authorizer.isAuthorizer(authorizer), 'RequestAuthorizer is not of type Authorizer');

test.done();
},
};

0 comments on commit 5d83a33

Please sign in to comment.