Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cognito): custom attributes in userpoolidentityprovider appear without the prefix (custom:) #32009

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

fix(cognito): custom attributes in userpoolidentityprovider appear without the prefix (custom:) #32009

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
adf483d
fix for cognito custom attributes
Balaji-JBC Nov 4, 2024
dbf8dbe
Merge branch 'main' into fix-26820
Balaji-JBC Nov 4, 2024
cd28ac8
update identitypool alpha integration test
Balaji-JBC Nov 4, 2024
506aa02
Merge branch 'main' into fix-26820
Balaji-JBC Nov 4, 2024
30138fb
Merge branch 'main' into fix-26820
Balaji-JBC Nov 9, 2024
add0766
keep backward compatibility on custom attribute key
Balaji-JBC Nov 10, 2024
c1bf08d
Update integ tests
Nov 12, 2024
97d34b3
Update integ tests
Balaji-JBC Dec 22, 2024
4ad76ff
Update integ tests
Balaji-JBC Dec 22, 2024
74626b0
Merge branch 'main' into fix-26820
Balaji-JBC Dec 22, 2024
1d73b74
Merge branch 'main' into fix-26820
Balaji-JBC Dec 24, 2024
b98507c
Merge branch 'main' into fix-26820
Balaji-JBC Feb 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...nito/test/integ.user-pool-idp.amazon.js.snapshot/integ-user-pool-idp-amazon.template.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"AttributeMapping": {
"given_name": "name",
"email": "email",
"userId": "user_id"
"custom:userId": "user_id"
},
"ProviderDetails": {
"client_id": "amzn-client-id",
Expand Down
2 changes: 1 addition & 1 deletion ...nito/test/integ.user-pool-idp.google.js.snapshot/integ-user-pool-idp-google.template.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
"email": "email",
"email_verified": "email_verified",
"gender": "gender",
"names": "names"
"custom:names": "names"
},
"ProviderDetails": {
"client_id": "google-client-id",
Expand Down
3 changes: 3 additions & 0 deletions ...user-pool-idp.saml.js.snapshot/integ-user-pool-identity-provider-saml-stack.template.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@
"samlProvider6C4CC492": {
"Type": "AWS::Cognito::UserPoolIdentityProvider",
"Properties": {
"AttributeMapping": {
"custom:department": "department"
},
"ProviderDetails": {
"IDPSignout": false,
"MetadataURL": "https://fujifish.github.io/samling/public/metadata.xml",
Expand Down
7 changes: 6 additions & 1 deletion packages/@aws-cdk-testing/framework-integ/test/aws-cognito/test/integ.user-pool-idp.saml.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { App, CfnOutput, RemovalPolicy, Stack } from 'aws-cdk-lib';
import { IntegTest } from '@aws-cdk/integ-tests-alpha';
import { Construct } from 'constructs';
import { UserPoolClientIdentityProvider, UserPool, UserPoolIdentityProviderSaml, UserPoolIdentityProviderSamlMetadata, SigningAlgorithm } from 'aws-cdk-lib/aws-cognito';
import { UserPoolClientIdentityProvider, UserPool, UserPoolIdentityProviderSaml, UserPoolIdentityProviderSamlMetadata, SigningAlgorithm, ProviderAttribute } from 'aws-cdk-lib/aws-cognito';

class TestStack extends Stack {
constructor(scope: Construct, id: string) {
Expand All @@ -17,6 +17,11 @@ class TestStack extends Stack {
metadata: UserPoolIdentityProviderSamlMetadata.url('https://fujifish.github.io/samling/public/metadata.xml'),
encryptedResponses: true,
requestSigningAlgorithm: SigningAlgorithm.RSA_SHA256,
attributeMapping: {
custom: {
department: ProviderAttribute.other('department'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we've fixed the prefix issue, these integration tests don't properly set up the custom attributes because the UserPool does not define them (I verified this by deploying the integ.user-pool-idp.amazon.ts test template).

A couple of asks (once addressed I'm happy to approve):

  1. Could we update the UserPool in the integration tests to create the corresponding custom attributes?

For example in this test:

    const userpool = new UserPool(this, 'pool', {
      removalPolicy: RemovalPolicy.DESTROY,
      customAttributes: {
        department: new StringAttribute({ minLen: 5, maxLen: 40, mutable: false }),
      }
    });
  1. Could we update the README example here as well to create the uniqueId custom attribute in the UserPool?
  2. Could we update the integ.user-pool-idp-*.ts tests to use the new IntegTest framework? Example:
import { IntegTest } from '@aws-cdk/integ-tests-alpha'; //add this to imports

const app = new App();
const stack = new Stack(app, 'integ-user-pool-idp-google');
.
.
.

new IntegTest(app, 'integ-user-pool-identity-provider-google-test', {
  testCases: [stack],
});

Thank you! Appreciate your work on this.

},
},
});

const client = userpool.addClient('client');
Expand Down
4 changes: 2 additions & 2 deletions ...o-identitypool-alpha/test/integ.identitypool.js.snapshot/integ-identitypool.template.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
"family_name": "family_name",
"email": "email",
"gender": "gender",
"names": "names"
"custom:names": "names"
},
"ProviderDetails": {
"client_id": "google-client-id",
Expand Down Expand Up @@ -148,7 +148,7 @@
"AttributeMapping": {
"given_name": "name",
"email": "email",
"userId": "user_id"
"custom:userId": "user_id"
},
"ProviderDetails": {
"client_id": "amzn-client-id",
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-cognito/lib/user-pool-idps/private/user-pool-idp-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export abstract class UserPoolIdentityProviderBase extends Resource implements I
}, mapping);
if (this.props.attributeMapping.custom) {
mapping = Object.entries(this.props.attributeMapping.custom).reduce((agg, [k, v]) => {
return { ...agg, [k]: v.attributeName };
return { ...agg, [`custom:${k}`]: v.attributeName };
}, mapping);
}
if (Object.keys(mapping).length === 0) { return undefined; }
Expand Down
8 changes: 4 additions & 4 deletions packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/amazon.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ describe('UserPoolIdentityProvider', () => {
// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolIdentityProvider', {
AttributeMapping: {
given_name: 'name',
address: 'amzn-address',
customAttr1: 'email',
customAttr2: 'amzn-custom-attr',
'given_name': 'name',
'address': 'amzn-address',
'custom:customAttr1': 'email',
'custom:customAttr2': 'amzn-custom-attr',
},
});
});
Expand Down
8 changes: 4 additions & 4 deletions packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/apple.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ describe('UserPoolIdentityProvider', () => {
// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolIdentityProvider', {
AttributeMapping: {
family_name: 'lastName',
given_name: 'firstName',
customAttr1: 'email',
customAttr2: 'sub',
'family_name': 'lastName',
'given_name': 'firstName',
'custom:customAttr1': 'email',
'custom:customAttr2': 'sub',
},
});
});
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/base.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ describe('UserPoolIdentityProvider', () => {

// THEN
expect(idp.mapping).toStrictEqual({
'custom-attr-1': 'email',
'custom-attr-2': 'name',
'custom:custom-attr-1': 'email',
'custom:custom-attr-2': 'name',
});
});

Expand Down
8 changes: 4 additions & 4 deletions packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/facebook.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ describe('UserPoolIdentityProvider', () => {
// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolIdentityProvider', {
AttributeMapping: {
given_name: 'name',
address: 'fb-address',
customAttr1: 'email',
customAttr2: 'fb-custom-attr',
'given_name': 'name',
'address': 'fb-address',
'custom:customAttr1': 'email',
'custom:customAttr2': 'fb-custom-attr',
},
});
});
Expand Down
8 changes: 4 additions & 4 deletions packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/google.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ describe('UserPoolIdentityProvider', () => {
// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolIdentityProvider', {
AttributeMapping: {
given_name: 'name',
address: 'google-address',
customAttr1: 'email',
customAttr2: 'google-custom-attr',
'given_name': 'name',
'address': 'google-address',
'custom:customAttr1': 'email',
'custom:customAttr2': 'google-custom-attr',
},
});
});
Expand Down
8 changes: 4 additions & 4 deletions packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/oidc.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ describe('UserPoolIdentityProvider', () => {
// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolIdentityProvider', {
AttributeMapping: {
family_name: 'family_name',
given_name: 'given_name',
customAttr1: 'email',
customAttr2: 'sub',
'family_name': 'family_name',
'given_name': 'given_name',
'custom:customAttr1': 'email',
'custom:customAttr2': 'sub',
},
});
});
Expand Down
8 changes: 4 additions & 4 deletions packages/aws-cdk-lib/aws-cognito/test/user-pool-idps/saml.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,10 @@ describe('UserPoolIdentityProvider', () => {
// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPoolIdentityProvider', {
AttributeMapping: {
family_name: 'family_name',
given_name: 'given_name',
customAttr1: 'email',
customAttr2: 'sub',
'family_name': 'family_name',
'given_name': 'given_name',
'custom:customAttr1': 'email',
'custom:customAttr2': 'sub',
},
});
});
Expand Down