From 7ee183d788a71015fb5aeafeaee1fa8001cc44ad Mon Sep 17 00:00:00 2001 From: Pahud Hsieh Date: Fri, 13 Sep 2024 15:45:43 -0400 Subject: [PATCH] fix(cognito): deprecate privateKey and add privateKeyValue as typed SecureValue (#31409) ### Issue # (if applicable) Closes https://github.com/aws/aws-cdk/issues/31378 ### Reason for this change 1. `privateKey` was typed `string` which should be `SecureValue` just as [clientSecretValue](https://github.com/aws/aws-cdk/blob/1e203753519e10e19ef0db87e1382377b609bcaa/packages/aws-cdk-lib/aws-cognito/lib/user-pool-idps/google.ts#L28) in Google IdP. This PR deprecates `privateKey` and adds `privateKeyValue` with correct type. 2. `apple.ts` was named by mistake and it won't be unit tested. This PR renames it to `apple.test.ts` so it would be covered. Figured an existing test was failed, just fixed that failed one as well. ### Description of changes - Add `privateKeyValue` property of type SecretValue to UserPoolIdentityProviderAppleProps - Deprecate the existing `privateKey` string property - Implement logic to ensure exactly one of `privateKey` or `privateKeyValue` is provided - Update UserPoolIdentityProviderApple to use the new `privateKeyValue` when available - Rename apple.ts test file to apple.test.ts for consistency - Add new test case to verify mutually exclusive properties Users must now provide either `privateKey` or `privateKeyValue`, but not both. This change enhances security by allowing the use of SecretValue for the Apple IDP private key. ### Description of how you validated changes ### 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) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-cognito/lib/user-pool-idps/apple.ts | 19 +++++++- .../{apple.ts => apple.test.ts} | 45 +++++++++++++++++-- 2 files changed, 59 insertions(+), 5 deletions(-) rename packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/{apple.ts => apple.test.ts} (68%) diff --git a/packages/aws-cdk-lib/aws-cognito/lib/user-pool-idps/apple.ts b/packages/aws-cdk-lib/aws-cognito/lib/user-pool-idps/apple.ts index 86d94c78726f7..c78bbe6d00484 100644 --- a/packages/aws-cdk-lib/aws-cognito/lib/user-pool-idps/apple.ts +++ b/packages/aws-cdk-lib/aws-cognito/lib/user-pool-idps/apple.ts @@ -2,6 +2,7 @@ import { Construct } from 'constructs'; import { UserPoolIdentityProviderProps } from './base'; import { CfnUserPoolIdentityProvider } from '../cognito.generated'; import { UserPoolIdentityProviderBase } from './private/user-pool-idp-base'; +import { SecretValue } from '../../../core'; /** * Properties to initialize UserPoolAppleIdentityProvider @@ -22,8 +23,16 @@ export interface UserPoolIdentityProviderAppleProps extends UserPoolIdentityProv readonly keyId: string; /** * The privateKey content for Apple APIs to authenticate the client. + * + * @deprecated use privateKeyValue + * @default none */ - readonly privateKey: string; + readonly privateKey?: string; + /** + * The privateKey content for Apple APIs to authenticate the client. + * @default none + */ + readonly privateKeyValue?: SecretValue; /** * The list of apple permissions to obtain for getting access to the apple profile * @see https://developer.apple.com/documentation/sign_in_with_apple/clientconfigi/3230955-scope @@ -44,6 +53,12 @@ export class UserPoolIdentityProviderApple extends UserPoolIdentityProviderBase const scopes = props.scopes ?? ['name']; + // Exactly one of the properties must be configured + if ((!props.privateKey && !props.privateKeyValue) || + (props.privateKey && props.privateKeyValue)) { + throw new Error('Exactly one of "privateKey" or "privateKeyValue" must be configured.'); + } + const resource = new CfnUserPoolIdentityProvider(this, 'Resource', { userPoolId: props.userPool.userPoolId, providerName: 'SignInWithApple', // must be 'SignInWithApple' when the type is 'SignInWithApple' @@ -52,7 +67,7 @@ export class UserPoolIdentityProviderApple extends UserPoolIdentityProviderBase client_id: props.clientId, team_id: props.teamId, key_id: props.keyId, - private_key: props.privateKey, + private_key: props.privateKeyValue ? props.privateKeyValue.unsafeUnwrap() : props.privateKey, authorize_scopes: scopes.join(' '), }, attributeMapping: super.configureAttributeMapping(), diff --git a/packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/apple.ts b/packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/apple.test.ts similarity index 68% rename from packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/apple.ts rename to packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/apple.test.ts index c38e5b16ee73b..a299953d1cc8f 100644 --- a/packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/apple.ts +++ b/packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/apple.test.ts @@ -1,5 +1,5 @@ import { Template } from '../../../assertions'; -import { Stack } from '../../../core'; +import { Stack, SecretValue } from '../../../core'; import { ProviderAttribute, UserPool, UserPoolIdentityProviderApple } from '../../lib'; describe('UserPoolIdentityProvider', () => { @@ -102,12 +102,51 @@ describe('UserPoolIdentityProvider', () => { // THEN Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolIdentityProvider', { AttributeMapping: { - family_name: 'firstName', - given_name: 'lastName', + family_name: 'lastName', + given_name: 'firstName', customAttr1: 'email', customAttr2: 'sub', }, }); }); + + // cannot assign both privateKey and privateKeyValue + test('cannot assign both privateKey and privateKeyValue', () => { + // GIVEN + const stack = new Stack(); + const pool = new UserPool(stack, 'userpool'); + + expect(() => { + new UserPoolIdentityProviderApple(stack, 'userpoolidp', { + userPool: pool, + clientId: 'com.amzn.cdk', + teamId: 'CDKTEAMCDK', + keyId: 'XXXXXXXXXX', + privateKey: 'PRIV_KEY_CDK', + privateKeyValue: SecretValue.secretsManager('dummyId'), + }); + }).toThrow('Exactly one of "privateKey" or "privateKeyValue" must be configured.'); + }); + + // should support privateKeyValue + test('should support privateKeyValue', () => { + // GIVEN + const stack = new Stack(); + const pool = new UserPool(stack, 'userpool'); + + new UserPoolIdentityProviderApple(stack, 'userpoolidp', { + userPool: pool, + clientId: 'com.amzn.cdk', + teamId: 'CDKTEAMCDK', + keyId: 'XXXXXXXXXX', + privateKeyValue: SecretValue.secretsManager('dummyId'), + }); + + Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolIdentityProvider', { + ProviderDetails: { + private_key: '{{resolve:secretsmanager:dummyId:SecretString:::}}', + }, + }); + }); }); });