From d91c9045b2ca027947c94ff8b93adb80f8ca8434 Mon Sep 17 00:00:00 2001 From: Sam Stephens Date: Sat, 30 Jul 2022 16:22:15 +1200 Subject: [PATCH] fix(cognito-identitypool): providerUrl causes error when mappingKey is not provided and it is a token (#21191) This property is for use when the identityProvider is a Token. By default identityProvider is used as the key in the role mapping hash, but Cloudformation only allows concrete strings to be used as hash keys. In particular this feature is a requirement to allow a previously defined CDK UserPool to be used as an identityProvider. closes #19222 Please note that the integ test results will need updating. I attempted to run the tests, and received the error ``` Error: ENOENT: no such file or directory, open '/home/sam/aws-cdk/packages/aws-cdk/lib/init-templates/v1/info.json' ERROR integ.identitypool 0.535s Command exited with status 1 ``` I've used `npm` to update to the latest CDK CLI. I appear to not be the only person facing this issue; see https://github.com/aws/aws-cdk/pull/21056#issuecomment-1178879318 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-cognito-identitypool/README.md | 21 ++++- .../lib/identitypool-role-attachment.ts | 20 ++++- .../integ-identitypool.template.json | 14 +++ .../test/identitypool.test.ts | 88 +++++++++++++++++++ .../test/integ.identitypool.ts | 13 +++ 5 files changed, 154 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito-identitypool/README.md b/packages/@aws-cdk/aws-cognito-identitypool/README.md index e1be0d70ec29c..f41791b0e2e34 100644 --- a/packages/@aws-cdk/aws-cognito-identitypool/README.md +++ b/packages/@aws-cdk/aws-cognito-identitypool/README.md @@ -270,7 +270,7 @@ new IdentityPool(this, 'myidentitypool', { }); ``` -Using a rule-based approach to role mapping allows roles to be assigned based on custom claims passed from the identity provider: +Using a rule-based approach to role mapping allows roles to be assigned based on custom claims passed from the identity provider: ```ts import { IdentityPoolProviderUrl, RoleMappingMatchType } from '@aws-cdk/aws-cognito-identitypool'; @@ -349,6 +349,25 @@ new IdentityPool(this, 'myidentitypool', { }); ``` +If a provider URL is a CDK Token, as it will be if you are trying to use a previously defined Cognito User Pool, you will need to also provide a mappingKey. +This is because by default, the key in the Cloudformation role mapping hash is the providerUrl, and Cloudformation map keys must be concrete strings, they +cannot be references. For example: + +```ts +import { UserPool } from '@aws-cdk/aws-cognito'; +import { IdentityPoolProviderUrl } from '@aws-cdk/aws-cognito-identitypool'; + +declare const userPool : UserPool; +new IdentityPool(this, 'myidentitypool', { + identityPoolName: 'myidentitypool', + roleMappings: [{ + mappingKey: 'cognito', + providerUrl: IdentityPoolProviderUrl.userPool(userPool.userPoolProviderUrl), + useToken: true, + }], +}); +``` + See [here](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cognito-identitypoolroleattachment-rolemapping.html#cfn-cognito-identitypoolroleattachment-rolemapping-identityprovider) for more information. ### Authentication Flow diff --git a/packages/@aws-cdk/aws-cognito-identitypool/lib/identitypool-role-attachment.ts b/packages/@aws-cdk/aws-cognito-identitypool/lib/identitypool-role-attachment.ts index b3811d99de5f3..961ef576a1f7a 100644 --- a/packages/@aws-cdk/aws-cognito-identitypool/lib/identitypool-role-attachment.ts +++ b/packages/@aws-cdk/aws-cognito-identitypool/lib/identitypool-role-attachment.ts @@ -7,6 +7,7 @@ import { import { Resource, IResource, + Token, } from '@aws-cdk/core'; import { Construct, @@ -65,6 +66,12 @@ export interface IdentityPoolRoleMapping { */ readonly providerUrl: IdentityPoolProviderUrl; + /** + * The key used for the role mapping in the role mapping hash. Required if the providerUrl is a token. + * @default - the provided providerUrl + */ + readonly mappingKey?: string; + /** * If true then mapped roles must be passed through the cognito:roles or cognito:preferred_role claims from identity provider. * @see https://docs.aws.amazon.com/cognito/latest/developerguide/role-based-access-control.html#using-tokens-to-assign-roles-to-users @@ -176,6 +183,17 @@ export class IdentityPoolRoleAttachment extends Resource implements IIdentityPoo ): { [name:string]: CfnIdentityPoolRoleAttachment.RoleMappingProperty } | undefined { if (!props || !props.length) return undefined; return props.reduce((acc, prop) => { + let mappingKey; + if (prop.mappingKey) { + mappingKey = prop.mappingKey; + } else { + const providerUrl = prop.providerUrl.value; + if (Token.isUnresolved(providerUrl)) { + throw new Error('mappingKey must be provided when providerUrl.value is a token'); + } + mappingKey = providerUrl; + } + let roleMapping: any = { ambiguousRoleResolution: prop.resolveAmbiguousRoles ? 'AuthenticatedRole' : 'Deny', type: prop.useToken ? 'Token' : 'Rules', @@ -196,7 +214,7 @@ export class IdentityPoolRoleAttachment extends Resource implements IIdentityPoo }), }; }; - acc[prop.providerUrl.value] = roleMapping; + acc[mappingKey] = roleMapping; return acc; }, {} as { [name:string]: CfnIdentityPoolRoleAttachment.RoleMappingProperty }); } diff --git a/packages/@aws-cdk/aws-cognito-identitypool/test/identitypool.integ.snapshot/integ-identitypool.template.json b/packages/@aws-cdk/aws-cognito-identitypool/test/identitypool.integ.snapshot/integ-identitypool.template.json index b57d320cb0c8a..c7d7a40dbae36 100644 --- a/packages/@aws-cdk/aws-cognito-identitypool/test/identitypool.integ.snapshot/integ-identitypool.template.json +++ b/packages/@aws-cdk/aws-cognito-identitypool/test/identitypool.integ.snapshot/integ-identitypool.template.json @@ -399,6 +399,20 @@ "Arn" ] } + }, + "RoleMappings": { + "www.amazon.com": { + "AmbiguousRoleResolution": "Deny", + "IdentityProvider": "www.amazon.com", + "Type":"Token" + }, + "theKey":{ + "AmbiguousRoleResolution": "Deny", + "IdentityProvider": { + "Fn::ImportValue": "ProviderUrl" + }, + "Type":"Token" + } } }, "DependsOn": [ diff --git a/packages/@aws-cdk/aws-cognito-identitypool/test/identitypool.test.ts b/packages/@aws-cdk/aws-cognito-identitypool/test/identitypool.test.ts index 903c3915a0e11..f600278fe9de8 100644 --- a/packages/@aws-cdk/aws-cognito-identitypool/test/identitypool.test.ts +++ b/packages/@aws-cdk/aws-cognito-identitypool/test/identitypool.test.ts @@ -18,6 +18,7 @@ import { PolicyDocument, } from '@aws-cdk/aws-iam'; import { + Fn, Stack, } from '@aws-cdk/core'; import { @@ -397,6 +398,93 @@ describe('identity pool', () => { }); describe('role mappings', () => { + test('mappingKey respected when identity provider is not a token', () => { + const stack = new Stack(); + new IdentityPool(stack, 'TestIdentityPoolRoleMappingToken', { + roleMappings: [{ + mappingKey: 'amazon', + providerUrl: IdentityPoolProviderUrl.AMAZON, + useToken: true, + }], + }); + Template.fromStack(stack).hasResourceProperties('AWS::Cognito::IdentityPoolRoleAttachment', { + IdentityPoolId: { + Ref: 'TestIdentityPoolRoleMappingToken0B11D690', + }, + RoleMappings: { + amazon: { + AmbiguousRoleResolution: 'Deny', + IdentityProvider: 'www.amazon.com', + Type: 'Token', + }, + }, + Roles: { + authenticated: { + 'Fn::GetAtt': [ + 'TestIdentityPoolRoleMappingTokenAuthenticatedRoleD99CE043', + 'Arn', + ], + }, + unauthenticated: { + 'Fn::GetAtt': [ + 'TestIdentityPoolRoleMappingTokenUnauthenticatedRole1D86D800', + 'Arn', + ], + }, + }, + }); + }); + + test('mappingKey required when identity provider is not a token', () => { + const stack = new Stack(); + const providerUrl = Fn.importValue('ProviderUrl'); + expect(() => new IdentityPool(stack, 'TestIdentityPoolRoleMappingErrors', { + roleMappings: [{ + providerUrl: IdentityPoolProviderUrl.userPool(providerUrl), + useToken: true, + }], + })).toThrowError('mappingKey must be provided when providerUrl.value is a token'); + }); + + test('mappingKey respected when identity provider is a token', () => { + const stack = new Stack(); + const providerUrl = Fn.importValue('ProviderUrl'); + new IdentityPool(stack, 'TestIdentityPoolRoleMappingToken', { + roleMappings: [{ + mappingKey: 'theKey', + providerUrl: IdentityPoolProviderUrl.userPool(providerUrl), + useToken: true, + }], + }); + Template.fromStack(stack).hasResourceProperties('AWS::Cognito::IdentityPoolRoleAttachment', { + IdentityPoolId: { + Ref: 'TestIdentityPoolRoleMappingToken0B11D690', + }, + RoleMappings: { + theKey: { + AmbiguousRoleResolution: 'Deny', + IdentityProvider: { + 'Fn::ImportValue': 'ProviderUrl', + }, + Type: 'Token', + }, + }, + Roles: { + authenticated: { + 'Fn::GetAtt': [ + 'TestIdentityPoolRoleMappingTokenAuthenticatedRoleD99CE043', + 'Arn', + ], + }, + unauthenticated: { + 'Fn::GetAtt': [ + 'TestIdentityPoolRoleMappingTokenUnauthenticatedRole1D86D800', + 'Arn', + ], + }, + }, + }); + }); test('using token', () => { const stack = new Stack(); new IdentityPool(stack, 'TestIdentityPoolRoleMappingToken', { diff --git a/packages/@aws-cdk/aws-cognito-identitypool/test/integ.identitypool.ts b/packages/@aws-cdk/aws-cognito-identitypool/test/integ.identitypool.ts index 5fc7ee1027084..58811bd94f5e8 100644 --- a/packages/@aws-cdk/aws-cognito-identitypool/test/integ.identitypool.ts +++ b/packages/@aws-cdk/aws-cognito-identitypool/test/integ.identitypool.ts @@ -10,10 +10,12 @@ import { } from '@aws-cdk/aws-iam'; import { App, + Fn, Stack, } from '@aws-cdk/core'; import { IdentityPool, + IdentityPoolProviderUrl, } from '../lib/identitypool'; import { UserPoolAuthenticationProvider, @@ -56,6 +58,17 @@ const idPool = new IdentityPool(stack, 'identitypool', { amazon: { appId: 'amzn1.application.12312k3j234j13rjiwuenf' }, google: { clientId: '12345678012.apps.googleusercontent.com' }, }, + roleMappings: [ + { + providerUrl: IdentityPoolProviderUrl.AMAZON, + useToken: true, + }, + { + mappingKey: 'theKey', + providerUrl: IdentityPoolProviderUrl.userPool(Fn.importValue('ProviderUrl')), + useToken: true, + }, + ], allowClassicFlow: true, identityPoolName: 'my-id-pool', });