Skip to content

Commit

Permalink
fix(aws-cognito): make callbackUrls required when auth flow is explic…
Browse files Browse the repository at this point in the history
…itly set
  • Loading branch information
markmansur committed Dec 3, 2023
1 parent 03a5ecd commit c5dc506
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 13 deletions.
13 changes: 7 additions & 6 deletions packages/aws-cdk-lib/aws-cognito/lib/user-pool-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,13 @@ export interface OAuthSettings {
* OAuth flows that are allowed with this client.
* @see - the 'Allowed OAuth Flows' section at https://docs.aws.amazon.com/cognito/latest/developerguide/cognito-user-pools-app-idp-settings.html
* @default {authorizationCodeGrant:true,implicitCodeGrant:true}
* @requires at least one callback if explicity set.
*/
readonly flows?: OAuthFlows;

/**
* List of allowed redirect URLs for the identity providers.
* @default - ['https://example.com'] if either authorizationCodeGrant or implicitCodeGrant flows are enabled, no callback URLs otherwise.
* @default - ['https://example.com'] if `flows` isn't set.
*/
readonly callbackUrls?: string[];

Expand Down Expand Up @@ -391,11 +392,11 @@ export class UserPoolClient extends Resource implements IUserPoolClient {
};

let callbackUrls: string[] | undefined = props.oAuth?.callbackUrls;
if (this.oAuthFlows.authorizationCodeGrant || this.oAuthFlows.implicitCodeGrant) {
if (callbackUrls === undefined) {
callbackUrls = ['https://example.com'];
} else if (callbackUrls.length === 0) {
throw new Error('callbackUrl must not be empty when codeGrant or implicitGrant OAuth flows are enabled.');
if (!props.oAuth && callbackUrls === undefined) {
callbackUrls = ['https://example.com'];
} else if (props.oAuth?.flows?.authorizationCodeGrant || props.oAuth?.flows?.implicitCodeGrant) {
if (callbackUrls === undefined || callbackUrls.length === 0) {
throw new Error('callbackUrls must not be empty when codeGrant or implicitGrant OAuth flows are enabled.');
}
}

Expand Down
66 changes: 59 additions & 7 deletions packages/aws-cdk-lib/aws-cognito/test/user-pool-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ describe('User Pool Client', () => {
authorizationCodeGrant: true,
implicitCodeGrant: true,
},
callbackUrls: ['https://example.com'],
scopes: [OAuthScope.PHONE],
},
});
Expand Down Expand Up @@ -377,16 +378,35 @@ describe('User Pool Client', () => {
pool.addClient('Client2', {
oAuth: {
flows: {
authorizationCodeGrant: true,
clientCredentials: true,
},
callbackUrls: ['https://aws.com'],
},
});

pool.addClient('Client3', {
oAuth: {
flows: {
authorizationCodeGrant: true,
},
callbackUrls: ['https://aws.com'],
},
});

pool.addClient('Client4', {
oAuth: {
flows: {
implicitCodeGrant: true,
},
callbackUrls: ['https://aws.com'],
},
});

pool.addClient('Client5');

pool.addClient('Client6', {
oAuth: {
callbackUrls: ['https://aws.com'],
},
});

Expand All @@ -396,14 +416,29 @@ describe('User Pool Client', () => {
});

Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolClient', {
AllowedOAuthFlows: ['implicit'],
CallbackURLs: ['https://example.com'],
AllowedOAuthFlows: ['client_credentials'],
CallbackURLs: ['https://aws.com'],
});

Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolClient', {
AllowedOAuthFlows: ['code'],
CallbackURLs: ['https://aws.com'],
});

Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolClient', {
AllowedOAuthFlows: ['implicit'],
CallbackURLs: ['https://aws.com'],
});

Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolClient', {
AllowedOAuthFlows: ['implicit', 'code'],
CallbackURLs: ['https://example.com'],
});

Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolClient', {
AllowedOAuthFlows: ['implicit', 'code'],
CallbackURLs: ['https://aws.com'],
});
});

test('callbackUrls are not rendered if OAuth is disabled ', () => {
Expand All @@ -423,7 +458,7 @@ describe('User Pool Client', () => {
});
});

test('fails when callbackUrls is empty for codeGrant or implicitGrant', () => {
test('fails when callbackUrls is empty/undefined for codeGrant or implicitGrant', () => {
const stack = new Stack();
const pool = new UserPool(stack, 'Pool');

Expand All @@ -432,21 +467,36 @@ describe('User Pool Client', () => {
flows: { implicitCodeGrant: true },
callbackUrls: [],
},
})).toThrow(/callbackUrl must not be empty/);
})).toThrow(/callbackUrls must not be empty/);

expect(() => pool.addClient('Client3', {
expect(() => pool.addClient('Client2', {
oAuth: {
flows: { authorizationCodeGrant: true },
callbackUrls: [],
},
})).toThrow(/callbackUrl must not be empty/);
})).toThrow(/callbackUrls must not be empty/);

expect(() => pool.addClient('Client3', {
oAuth: {
flows: { implicitCodeGrant: true },
},
})).toThrow(/callbackUrls must not be empty/);

expect(() => pool.addClient('Client4', {
oAuth: {
flows: { authorizationCodeGrant: true },
},
})).toThrow(/callbackUrls must not be empty/);

expect(() => pool.addClient('Client5', {
oAuth: {
flows: { clientCredentials: true },
callbackUrls: [],
},
})).not.toThrow();

expect(() => pool.addClient('Client6', {
})).not.toThrow();
});

test('logoutUrls can be set', () => {
Expand Down Expand Up @@ -474,6 +524,7 @@ describe('User Pool Client', () => {
authorizationCodeGrant: true,
clientCredentials: true,
},
callbackUrls: ['https://example.com'],
scopes: [OAuthScope.PHONE],
},
})).toThrow(/clientCredentials OAuth flow cannot be selected/);
Expand All @@ -484,6 +535,7 @@ describe('User Pool Client', () => {
implicitCodeGrant: true,
clientCredentials: true,
},
callbackUrls: ['https://example.com'],
scopes: [OAuthScope.PHONE],
},
})).toThrow(/clientCredentials OAuth flow cannot be selected/);
Expand Down

0 comments on commit c5dc506

Please sign in to comment.