Skip to content

Commit

Permalink
fix(cognito-identitypool-alpha): inconvenient IdentityPoolProviderUrl…
Browse files Browse the repository at this point in the history
….userPool() (#29025)

### Reason for this change

`IdentityPoolProviderUrl.userPool()` requires a string `url` currently.
The description is "User Pool Provider Url".
It should be ``` `${userPool.userPoolProviderName}:${userPoolClient.userPoolClientId}` ```.

`UserPool` has an attribute `userPoolProviderUrl` which description is "User Pool Provider Url", but confusingly, it cannot be specified to `IdentityPoolProviderUrl.userPool()`.

The format of the identity provider identifier isn't well documented.
See [SetIdentityPoolRoles API reference](https://docs.aws.amazon.com/cognitoidentity/latest/APIReference/API_SetIdentityPoolRoles.html) for example of User Pool's identity provider identifier.

### Description of changes

This PR fixes `IdentityPoolProviderUrl.userPool()` to accept `UserPool` and `UserPoolClient` instead of a string `url`.
It generates a correct identifier described above.

### Description of how you validated changes

Existing integration test generates an identifier as described above.
The snapshot won't be changed by this PR.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

BREAKING CHANGE: The argument of `IdentityPoolProviderUrl.userPool()` has been changed from `url: string` to `userPool: UserPool, userPoolClient: UserPoolClient`. If you want to specify custom identifier string, use `IdentityPoolProviderUrl.custom()` instead.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Tietew authored Apr 17, 2024
1 parent 6cd1e1f commit 90a7734
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 14 deletions.
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 {
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),
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

0 comments on commit 90a7734

Please sign in to comment.