From 90a773407753b4ff868dede4442bf20243dfeaec Mon Sep 17 00:00:00 2001 From: Tietew Date: Thu, 18 Apr 2024 03:59:17 +0900 Subject: [PATCH] fix(cognito-identitypool-alpha): inconvenient IdentityPoolProviderUrl.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* --- .../aws-cognito-identitypool-alpha/README.md | 14 ++++------- .../lib/identitypool.ts | 5 +++- .../test/identitypool.test.ts | 23 ++++++++++++++++--- .../test/integ.identitypool.ts | 2 +- 4 files changed, 30 insertions(+), 14 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito-identitypool-alpha/README.md b/packages/@aws-cdk/aws-cognito-identitypool-alpha/README.md index f9cf1200cd7a1..85ddb602cd9c1 100644 --- a/packages/@aws-cdk/aws-cognito-identitypool-alpha/README.md +++ b/packages/@aws-cdk/aws-cognito-identitypool-alpha/README.md @@ -329,7 +329,7 @@ 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'; @@ -337,10 +337,6 @@ 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, @@ -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, }], }); @@ -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'); ``` - diff --git a/packages/@aws-cdk/aws-cognito-identitypool-alpha/lib/identitypool.ts b/packages/@aws-cdk/aws-cognito-identitypool-alpha/lib/identitypool.ts index 872aefade730b..9e65131f5cae8 100644 --- a/packages/@aws-cdk/aws-cognito-identitypool-alpha/lib/identitypool.ts +++ b/packages/@aws-cdk/aws-cognito-identitypool-alpha/lib/identitypool.ts @@ -1,5 +1,7 @@ import { CfnIdentityPool, + UserPool, + UserPoolClient, } from 'aws-cdk-lib/aws-cognito'; import { IOpenIdConnectProvider, @@ -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); } diff --git a/packages/@aws-cdk/aws-cognito-identitypool-alpha/test/identitypool.test.ts b/packages/@aws-cdk/aws-cognito-identitypool-alpha/test/identitypool.test.ts index d751c2d2476cc..3bacbeeb104ea 100644 --- a/packages/@aws-cdk/aws-cognito-identitypool-alpha/test/identitypool.test.ts +++ b/packages/@aws-cdk/aws-cognito-identitypool-alpha/test/identitypool.test.ts @@ -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'); @@ -452,7 +452,7 @@ describe('role mappings', () => { new IdentityPool(stack, 'TestIdentityPoolRoleMappingToken', { roleMappings: [{ mappingKey: 'theKey', - providerUrl: IdentityPoolProviderUrl.userPool(providerUrl), + providerUrl: IdentityPoolProviderUrl.custom(providerUrl), useToken: true, }], }); @@ -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'), }); @@ -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: [ @@ -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', @@ -696,4 +713,4 @@ describe('role mappings', () => { }, }); }); -}); \ No newline at end of file +}); diff --git a/packages/@aws-cdk/aws-cognito-identitypool-alpha/test/integ.identitypool.ts b/packages/@aws-cdk/aws-cognito-identitypool-alpha/test/integ.identitypool.ts index 277b0cbe201da..e2d1ff3b4528c 100644 --- a/packages/@aws-cdk/aws-cognito-identitypool-alpha/test/integ.identitypool.ts +++ b/packages/@aws-cdk/aws-cognito-identitypool-alpha/test/integ.identitypool.ts @@ -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, }, ],