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-identitypool-alpha): inconvenient IdentityPoolProviderUrl.userPool() #29025

Merged
merged 6 commits into from
Apr 17, 2024
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
14 changes: 5 additions & 9 deletions packages/@aws-cdk/aws-cognito-identitypool-alpha/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -329,18 +329,14 @@ new IdentityPool(this, 'myidentitypool', {
});
```

For identity providers that don't have static Urls, a custom Url or User Pool Client Url can be supplied:
For identity providers that don't have static Urls, a custom Url can be supplied:

```ts
import { IdentityPoolProviderUrl } from '@aws-cdk/aws-cognito-identitypool-alpha';

new IdentityPool(this, 'myidentitypool', {
identityPoolName: 'myidentitypool',
roleMappings: [
{
providerUrl: IdentityPoolProviderUrl.userPool('cognito-idp.my-idp-region.amazonaws.com/my-idp-region_abcdefghi:app_client_id'),
useToken: true,
},
{
providerUrl: IdentityPoolProviderUrl.custom('my-custom-provider.com'),
useToken: true,
Expand All @@ -354,15 +350,16 @@ This is because by default, the key in the Cloudformation role mapping hash is t
cannot be references. For example:

```ts
import { UserPool } from 'aws-cdk-lib/aws-cognito';
import { UserPool, UserPoolClient } from 'aws-cdk-lib/aws-cognito';
import { IdentityPoolProviderUrl } from '@aws-cdk/aws-cognito-identitypool-alpha';

declare const userPool : UserPool;
declare const userPool: UserPool;
declare const userPoolClient: UserPoolClient;
new IdentityPool(this, 'myidentitypool', {
identityPoolName: 'myidentitypool',
roleMappings: [{
mappingKey: 'cognito',
providerUrl: IdentityPoolProviderUrl.userPool(userPool.userPoolProviderUrl),
providerUrl: IdentityPoolProviderUrl.userPool(userPool, userPoolClient),
useToken: true,
}],
});
Expand Down Expand Up @@ -399,4 +396,3 @@ IdentityPool.fromIdentityPoolId(this, 'my-imported-identity-pool',
IdentityPool.fromIdentityPoolArn(this, 'my-imported-identity-pool',
'arn:aws:cognito-identity:us-east-1:123456789012:identitypool/us-east-1:dj2823ryiwuhef937');
```

Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {
CfnIdentityPool,
UserPool,
UserPoolClient,
} from 'aws-cdk-lib/aws-cognito';
import {
IOpenIdConnectProvider,
Expand Down Expand Up @@ -155,7 +157,8 @@ export class IdentityPoolProviderUrl {
}

/** User Pool Provider Url */
public static userPool(url: string): IdentityPoolProviderUrl {
public static userPool(userPool: UserPool, userPoolClient: UserPoolClient): IdentityPoolProviderUrl {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we're ok with breaking changes, but this seems like it would ruin anyone who was simply supplying a url here that was not connected to a specific UserPool or UserPoolCliet construct. I'm not sure what the difference would be if the IdentityPoolProviderType turns from USER_POOL to CUSTOM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IdentityPoolProviderType (IdentityPoolProviderUrl.type) is not used since #16190 (implemented PR).
Changing .userPool(somthing) to .custom(something) makes no differences in CloudFormation template.

const url = `${userPool.userPoolProviderName}:${userPoolClient.userPoolClientId}`;
return new IdentityPoolProviderUrl(IdentityPoolProviderType.USER_POOL, url);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ describe('role mappings', () => {
const providerUrl = Fn.importValue('ProviderUrl');
expect(() => new IdentityPool(stack, 'TestIdentityPoolRoleMappingErrors', {
roleMappings: [{
providerUrl: IdentityPoolProviderUrl.userPool(providerUrl),
providerUrl: IdentityPoolProviderUrl.custom(providerUrl),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would change the IdentityPoolProviderTyle right? How does that affect things?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. No affects in CloudFormation template.

useToken: true,
}],
})).toThrowError('mappingKey must be provided when providerUrl.value is a token');
Expand All @@ -452,7 +452,7 @@ describe('role mappings', () => {
new IdentityPool(stack, 'TestIdentityPoolRoleMappingToken', {
roleMappings: [{
mappingKey: 'theKey',
providerUrl: IdentityPoolProviderUrl.userPool(providerUrl),
providerUrl: IdentityPoolProviderUrl.custom(providerUrl),
useToken: true,
}],
});
Expand Down Expand Up @@ -532,6 +532,8 @@ describe('role mappings', () => {

test('role mapping with rules configuration', () => {
const stack = new Stack();
const pool = new UserPool(stack, 'Pool');
const client = pool.addClient('Client');
const adminRole = new Role(stack, 'adminRole', {
assumedBy: new ServicePrincipal('admin.amazonaws.com'),
});
Expand All @@ -557,6 +559,11 @@ describe('role mappings', () => {
});
const idPool = new IdentityPool(stack, 'TestIdentityPoolRoleMappingRules', {
roleMappings: [{
mappingKey: 'cognito',
providerUrl: IdentityPoolProviderUrl.userPool(pool, client),
useToken: true,
},
{
providerUrl: IdentityPoolProviderUrl.AMAZON,
resolveAmbiguousRoles: true,
rules: [
Expand Down Expand Up @@ -601,6 +608,16 @@ describe('role mappings', () => {
Ref: 'TestIdentityPoolRoleMappingRulesC8C07BC3',
},
RoleMappings: {
'cognito': {
IdentityProvider: {
'Fn::Join': ['', [
{ 'Fn::GetAtt': ['PoolD3F588B8', 'ProviderName'] },
':',
{ Ref: 'PoolClient8A3E5EB7' },
]],
},
Type: 'Token',
},
'www.amazon.com': {
AmbiguousRoleResolution: 'AuthenticatedRole',
IdentityProvider: 'www.amazon.com',
Expand Down Expand Up @@ -696,4 +713,4 @@ describe('role mappings', () => {
},
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const idPool = new IdentityPool(stack, 'identitypool', {
roleMappings: [
{
mappingKey: 'theKey',
providerUrl: IdentityPoolProviderUrl.userPool(`${userPool.userPoolProviderName}:${client.userPoolClientId}`),
providerUrl: IdentityPoolProviderUrl.userPool(userPool, client),
useToken: true,
},
],
Expand Down