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
#7596)

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 861ecdd commit 1423c53
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 2 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, ResourceProps } 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;

public constructor(scope: Construct, id: string, props?: ResourceProps) {
super(scope, id, props);

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

/**
* Called when the authorizer is used from a specific REST API.
* @internal
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/lib/method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,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
21 changes: 21 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,21 @@
import { Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import { Authorizer, RestApi } 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();
},
};

0 comments on commit 1423c53

Please sign in to comment.