Skip to content

Commit

Permalink
fix(cognito): deprecate privateKey and add privateKeyValue as typed S…
Browse files Browse the repository at this point in the history
…ecureValue (#31409)

### Issue # (if applicable)

Closes #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*
  • Loading branch information
pahud authored Sep 13, 2024
1 parent ff02cca commit 7ee183d
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 5 deletions.
19 changes: 17 additions & 2 deletions packages/aws-cdk-lib/aws-cognito/lib/user-pool-idps/apple.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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'
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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:::}}',
},
});
});
});
});

0 comments on commit 7ee183d

Please sign in to comment.