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(cognito): error when using parameter for domainPrefix #8399

Merged
merged 2 commits into from
Jun 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Token } from '@aws-cdk/core';

/**
* The set of standard attributes that can be marked as required.
*
Expand Down Expand Up @@ -200,10 +202,10 @@ export class StringAttribute implements ICustomAttribute {
private readonly mutable?: boolean;

constructor(props: StringAttributeProps = {}) {
if (props.minLen && props.minLen < 0) {
if (props.minLen && !Token.isUnresolved(props.minLen) && props.minLen < 0) {
throw new Error(`minLen cannot be less than 0 (value: ${props.minLen}).`);
}
if (props.maxLen && props.maxLen > 2048) {
if (props.maxLen && !Token.isUnresolved(props.maxLen) && props.maxLen > 2048) {
throw new Error(`maxLen cannot be greater than 2048 (value: ${props.maxLen}).`);
}
this.minLen = props?.minLen;
Expand Down
7 changes: 5 additions & 2 deletions packages/@aws-cdk/aws-cognito/lib/user-pool-domain.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ICertificate } from '@aws-cdk/aws-certificatemanager';
import { Construct, IResource, Resource, Stack } from '@aws-cdk/core';
import { Construct, IResource, Resource, Stack, Token } from '@aws-cdk/core';
import { AwsCustomResource, AwsCustomResourcePolicy, AwsSdkCall, PhysicalResourceId } from '@aws-cdk/custom-resources';
import { CfnUserPoolDomain } from './cognito.generated';
import { IUserPool } from './user-pool';
Expand Down Expand Up @@ -90,7 +90,10 @@ export class UserPoolDomain extends Resource implements IUserPoolDomain {
throw new Error('One of, and only one of, cognitoDomain or customDomain must be specified');
}

if (props.cognitoDomain?.domainPrefix && !/^[a-z0-9-]+$/.test(props.cognitoDomain.domainPrefix)) {
if (props.cognitoDomain?.domainPrefix &&
!Token.isUnresolved(props.cognitoDomain?.domainPrefix) &&
!/^[a-z0-9-]+$/.test(props.cognitoDomain.domainPrefix)) {

throw new Error('domainPrefix for cognitoDomain can contain only lowercase alphabets, numbers and hyphens');
}

Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-cognito/lib/user-pool.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { IRole, PolicyDocument, PolicyStatement, Role, ServicePrincipal } from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
import { Construct, Duration, IResource, Lazy, Resource, Stack } from '@aws-cdk/core';
import { Construct, Duration, IResource, Lazy, Resource, Stack, Token } from '@aws-cdk/core';
import { CfnUserPool } from './cognito.generated';
import { ICustomAttribute, RequiredAttributes } from './user-pool-attr';
import { UserPoolClient, UserPoolClientOptions } from './user-pool-client';
Expand Down Expand Up @@ -722,10 +722,10 @@ export class UserPool extends UserPoolBase {

if (emailStyle === VerificationEmailStyle.CODE) {
const emailMessage = props.userVerification?.emailBody ?? `The verification code to your new account is ${CODE_TEMPLATE}`;
if (emailMessage.indexOf(CODE_TEMPLATE) < 0) {
if (!Token.isUnresolved(emailMessage) && emailMessage.indexOf(CODE_TEMPLATE) < 0) {
throw new Error(`Verification email body must contain the template string '${CODE_TEMPLATE}'`);
}
if (smsMessage.indexOf(CODE_TEMPLATE) < 0) {
if (!Token.isUnresolved(smsMessage) && smsMessage.indexOf(CODE_TEMPLATE) < 0) {
throw new Error(`SMS message must contain the template string '${CODE_TEMPLATE}'`);
}
return {
Expand All @@ -737,7 +737,7 @@ export class UserPool extends UserPoolBase {
} else {
const emailMessage = props.userVerification?.emailBody ??
`Verify your account by clicking on ${VERIFY_EMAIL_TEMPLATE}`;
if (emailMessage.indexOf(VERIFY_EMAIL_TEMPLATE) < 0) {
if (!Token.isUnresolved(emailMessage) && emailMessage.indexOf(VERIFY_EMAIL_TEMPLATE) < 0) {
throw new Error(`Verification email body must contain the template string '${VERIFY_EMAIL_TEMPLATE}'`);
}
return {
Expand Down
13 changes: 13 additions & 0 deletions packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import '@aws-cdk/assert/jest';
import { CfnParameter, Stack } from '@aws-cdk/core';
import { BooleanAttribute, CustomAttributeConfig, DateTimeAttribute, ICustomAttribute, NumberAttribute, StringAttribute } from '../lib';

describe('User Pool Attributes', () => {
Expand Down Expand Up @@ -104,6 +105,18 @@ describe('User Pool Attributes', () => {
expect(() => new StringAttribute({ maxLen: 5000 }))
.toThrow(/maxLen cannot be greater than/);
});

test('validation is skipped when minLen or maxLen are tokens', () => {
const stack = new Stack();
const parameter = new CfnParameter(stack, 'Parameter', {
type: 'Number',
});

expect(() => new StringAttribute({ minLen: parameter.valueAsNumber }))
.not.toThrow();
expect(() => new StringAttribute({ maxLen: parameter.valueAsNumber }))
.not.toThrow();
});
});

describe('NumberAttribute', () => {
Expand Down
13 changes: 12 additions & 1 deletion packages/@aws-cdk/aws-cognito/test/user-pool-domain.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import '@aws-cdk/assert/jest';
import { Certificate } from '@aws-cdk/aws-certificatemanager';
import { Stack } from '@aws-cdk/core';
import { CfnParameter, Stack } from '@aws-cdk/core';
import { UserPool, UserPoolDomain } from '../lib';

describe('User Pool Client', () => {
Expand Down Expand Up @@ -92,6 +92,17 @@ describe('User Pool Client', () => {
})).toThrow(/lowercase alphabets, numbers and hyphens/);
});

test('does not fail when domainPrefix is a token', () => {
const stack = new Stack();
const pool = new UserPool(stack, 'Pool');

const parameter = new CfnParameter(stack, 'Paraeter');

expect(() => pool.addDomain('Domain', {
cognitoDomain: { domainPrefix: parameter.valueAsString },
})).not.toThrow();
});

test('custom resource is added when cloudFrontDistribution method is called', () => {
// GIVEN
const stack = new Stack();
Expand Down
21 changes: 20 additions & 1 deletion packages/@aws-cdk/aws-cognito/test/user-pool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import '@aws-cdk/assert/jest';
import { ABSENT } from '@aws-cdk/assert/lib/assertions/have-resource';
import { Role } from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
import { Construct, Duration, Stack, Tag } from '@aws-cdk/core';
import { CfnParameter, Construct, Duration, Stack, Tag } from '@aws-cdk/core';
import { Mfa, NumberAttribute, StringAttribute, UserPool, UserPoolIdentityProvider, UserPoolOperation, VerificationEmailStyle } from '../lib';

describe('User Pool', () => {
Expand Down Expand Up @@ -161,6 +161,25 @@ describe('User Pool', () => {
})).not.toThrow();
});

test('validation is skipped for email and sms messages when tokens', () => {
const stack = new Stack();
const parameter = new CfnParameter(stack, 'Parameter');

expect(() => new UserPool(stack, 'Pool1', {
userVerification: {
emailStyle: VerificationEmailStyle.CODE,
emailBody: parameter.valueAsString,
},
})).not.toThrow();

expect(() => new UserPool(stack, 'Pool2', {
userVerification: {
emailStyle: VerificationEmailStyle.CODE,
smsMessage: parameter.valueAsString,
},
})).not.toThrow();
});

test('user invitation messages are configured correctly', () => {
// GIVEN
const stack = new Stack();
Expand Down