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.

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

aws-cognito: identity provider attribute mapping mishandles custom attributes #26820

Open
diego-santacruz opened this issue Aug 20, 2023 · 4 comments · May be fixed by #32009
Open

aws-cognito: identity provider attribute mapping mishandles custom attributes #26820

diego-santacruz opened this issue Aug 20, 2023 · 4 comments · May be fixed by #32009
Labels
@aws-cdk/aws-cognito Related to Amazon Cognito bug This issue is a bug. documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p3

Comments

@diego-santacruz
Copy link

Describe the bug

I have added a custom attribute to my Cognito user pool and I wanted for an identity provider to map a claim to this custom attribute.

Following the documentation at https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_cognito.AttributeMapping.html I defined the mapping like this

{
  email: cognito.ProviderAttribute.other('email'),
  familyName: cognito.ProviderAttribute.other('family_name'),
  givenName: cognito.ProviderAttribute.other('given_name'),
  fullname: cognito.ProviderAttribute.other('name'),
  custom: {
      'idp_userid': cognito.ProviderAttribute.other('sub')
  }
}

And then pass that to the UserPoolIdentityProviderOidcProps's attributeMapping property.

However, this generates the wrong CloudFormation template, as the custom attribute is not prefixed with custom:. Although it deploys, when editing the mapping from the console, the mapping is not recognized, but if modified manually the console adds the custom: prefix.

The generated template looks like this

   "Type": "AWS::Cognito::UserPoolIdentityProvider",
   "Properties": {
    "AttributeMapping": {
     "email": "email",
     "family_name": "family_name",
     "given_name": "given_name",
     "name": "name",
     "custom:idp_userid": "sub"
    },

When it should look like

Expected Behavior

The generated template should looks like this

   "Type": "AWS::Cognito::UserPoolIdentityProvider",
   "Properties": {
    "AttributeMapping": {
     "email": "email",
     "family_name": "family_name",
     "given_name": "given_name",
     "name": "name",
     "custom:idp_userid": "sub"
    },

Current Behavior

The generated template should looks like this

   "Type": "AWS::Cognito::UserPoolIdentityProvider",
   "Properties": {
    "AttributeMapping": {
     "email": "email",
     "family_name": "family_name",
     "given_name": "given_name",
     "name": "name",
     "idp_userid": "sub"
    },

Reproduction Steps

const userPool = new cognito.UserPool(this, 'CloudUser', {
    accountRecovery: cognito.AccountRecovery.EMAIL_ONLY,
    customAttributes: {
        idp_userid: new cognito.StringAttribute({
            minLen: 0,
            maxLen: 128,
            mutable: true
        })
    },
    standardAttributes: {
        email: {
          required: true
        }
    },
    selfSignUpEnabled: true,
    signInAliases: { email: true },
    signInCaseSensitive: false,
    userPoolName: `spx-cloud-${props.stage}-mock`,
    removalPolicy: RemovalPolicy.DESTROY
});

new cognito.UserPoolIdentityProviderOidc(this, 'SsoIdP',
    {
        name: 'test',
        userPool: userPool,
        attributeMapping: {
            email: cognito.ProviderAttribute.other('email'),
            familyName: cognito.ProviderAttribute.other('family_name'),
            givenName: cognito.ProviderAttribute.other('given_name'),
            fullname: cognito.ProviderAttribute.other('name'),
            custom: {
                'idp_userid': cognito.ProviderAttribute.other('sub')
            }
        },
        clientId: 'abcde',
        clientSecret: 'password',
        issuerUrl: 'http://....',
        cognito.OidcAttributeRequestMethod.GET,
        scopes: ['openid', 'email', 'profile']
    }
);

Possible Solution

A workaround is to define the mapping as follows

{
  email: cognito.ProviderAttribute.other('email'),
  familyName: cognito.ProviderAttribute.other('family_name'),
  givenName: cognito.ProviderAttribute.other('given_name'),
  fullname: cognito.ProviderAttribute.other('name'),
  custom: {
      'custom:idp_userid': cognito.ProviderAttribute.other('sub')
  }
}

I think the problem is in https://github.com/aws/aws-cdk/blame/00a7f033f6ad19160a7350784243ecf9c71c388b/packages/aws-cdk-lib/aws-cognito/lib/user-pool-idps/private/user-pool-idp-base.ts#L33C11-L33C11

Instead of

return { ...agg, [k]: v.attributeName };

The code should probably read

return { ...agg, [k]: `custom:${v.attributeName}` };

Additional Information/Context

No response

CDK CLI Version

2.92.0

Framework Version

2.92.0

Node.js Version

v16.18.1

OS

Linux

Language

Typescript

Language Version

No response

Other information

No response

@diego-santacruz diego-santacruz added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 20, 2023
@github-actions github-actions bot added the @aws-cdk/aws-cognito Related to Amazon Cognito label Aug 20, 2023
@peterwoodworth peterwoodworth added documentation This is a problem with documentation. p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 21, 2023
@peterwoodworth
Copy link
Contributor

Think this would be a docs issue, but we could probably implement this as a feature while keeping backwards compatibility in mind without a feature flag

@diego-santacruz
Copy link
Author

diego-santacruz commented Aug 21, 2023

Well the CDK interfaces and code treats custom attributes separately, so it seems a bit silly to name an attribute as "custom:my_attr" while it sits under a "custom" property, separate from the built-in properties.

But yes, a change in the docs would certainly work (note that the doc page I linked is not the only one to change), albeit the resulting code looks a bit awkward.

@peterwoodworth
Copy link
Contributor

Yeah I agree that it would be better for us to handle this rather than just changing the docs

@psistorm
Copy link

We had this problem just today. This is still an issue and totally unexpected behavior.
I have to strengthen the position of @diego-santacruz . It's totally unexpected behavior to need to add custom: prefix when it already sits under the custom property. When creating the property with the pool it works as expected but differently to here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cognito Related to Amazon Cognito bug This issue is a bug. documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p3
Projects
None yet
4 participants