Skip to content

Commit

Permalink
fix(cognito): cannot add multiple route53 targets to the same user po…
Browse files Browse the repository at this point in the history
…ol domain (#8622)

The root cause here is that calling `cloudFrontDomainName` getter on
`UserPoolDomain` results in creating a custom resource with a fixed node
id. This resulted in the error - "There is already a Construct with name
'CloudFrontDomainName' in UserPoolDomain".

Changed the logic around so that the `CustomResource` construct is only
created on first call to the getter, and is reused on subsequent calls.

fixes #8603


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Niranjan Jayakar authored Jun 25, 2020
1 parent 8914899 commit 32b54a5
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 20 deletions.
42 changes: 23 additions & 19 deletions packages/@aws-cdk/aws-cognito/lib/user-pool-domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ export class UserPoolDomain extends Resource implements IUserPoolDomain {
public readonly domainName: string;
private isCognitoDomain: boolean;

private cloudFrontCustomResource?: AwsCustomResource;

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

Expand Down Expand Up @@ -113,25 +115,27 @@ export class UserPoolDomain extends Resource implements IUserPoolDomain {
* The domain name of the CloudFront distribution associated with the user pool domain.
*/
public get cloudFrontDomainName(): string {
const sdkCall: AwsSdkCall = {
service: 'CognitoIdentityServiceProvider',
action: 'describeUserPoolDomain',
parameters: {
Domain: this.domainName,
},
physicalResourceId: PhysicalResourceId.of(this.domainName),
};
const customResource = new AwsCustomResource(this, 'CloudFrontDomainName', {
resourceType: 'Custom::UserPoolCloudFrontDomainName',
onCreate: sdkCall,
onUpdate: sdkCall,
policy: AwsCustomResourcePolicy.fromSdkCalls({
// DescribeUserPoolDomain only supports access level '*'
// https://docs.aws.amazon.com/IAM/latest/UserGuide/list_amazoncognitouserpools.html#amazoncognitouserpools-actions-as-permissions
resources: [ '*' ],
}),
});
return customResource.getResponseField('DomainDescription.CloudFrontDistribution');
if (!this.cloudFrontCustomResource) {
const sdkCall: AwsSdkCall = {
service: 'CognitoIdentityServiceProvider',
action: 'describeUserPoolDomain',
parameters: {
Domain: this.domainName,
},
physicalResourceId: PhysicalResourceId.of(this.domainName),
};
this.cloudFrontCustomResource = new AwsCustomResource(this, 'CloudFrontDomainName', {
resourceType: 'Custom::UserPoolCloudFrontDomainName',
onCreate: sdkCall,
onUpdate: sdkCall,
policy: AwsCustomResourcePolicy.fromSdkCalls({
// DescribeUserPoolDomain only supports access level '*'
// https://docs.aws.amazon.com/IAM/latest/UserGuide/list_amazoncognitouserpools.html#amazoncognitouserpools-actions-as-permissions
resources: [ '*' ],
}),
});
}
return this.cloudFrontCustomResource.getResponseField('DomainDescription.CloudFrontDistribution');
}

/**
Expand Down
17 changes: 16 additions & 1 deletion packages/@aws-cdk/aws-cognito/test/user-pool-domain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ describe('User Pool Client', () => {
})).not.toThrow();
});

test('custom resource is added when cloudFrontDistribution method is called', () => {
test('custom resource is added when cloudFrontDomainName property is used', () => {
// GIVEN
const stack = new Stack();
const pool = new UserPool(stack, 'Pool');
Expand Down Expand Up @@ -137,6 +137,21 @@ describe('User Pool Client', () => {
});
});

test('cloudFrontDomainName property can be called multiple times', () => {
const stack = new Stack();
const pool = new UserPool(stack, 'Pool');
const domain = pool.addDomain('Domain', {
cognitoDomain: {
domainPrefix: 'cognito-domain-prefix',
},
});

const cfDomainNameFirst = domain.cloudFrontDomainName;
const cfDomainNameSecond = domain.cloudFrontDomainName;

expect(cfDomainNameSecond).toEqual(cfDomainNameFirst);
});

describe('signInUrl', () => {
test('returns the expected URL', () => {
// GIVEN
Expand Down

0 comments on commit 32b54a5

Please sign in to comment.