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-cdk/aws-cognito): SettingAttribute 'fullname' is not consistent with documentation. #17361

Open
miguelcss opened this issue Nov 5, 2021 · 6 comments
Labels
@aws-cdk/aws-cognito Related to Amazon Cognito documentation This is a problem with documentation. feature-request A feature should be added or improved. p3

Comments

@miguelcss
Copy link

miguelcss commented Nov 5, 2021

What is the problem?

Using aws-cognito to construct a new pool, the SettingAttribute interface is not consistent to the Cognito documentation.
The README.md points to the cognito documentation for a list of standard attributes user-pool-settings-attributes which specifically has 'name' property. Configuring cognito pool in AWS console, users are also familiar with 'name' .

Yet using the standardAttributes: { name: {... this option is not available. Diggin on the code one finds the actual mapping here and we can see we need to use 'fullname' for 'name'. This is convoluted and not mentioned anywhere in documentation, which leads to unexpected outcomes

Note: same goes for

  profilePicture: 'picture',
  profilePage: 'profile',
  timezone: 'zoneinfo',  // arguably the worst offender

Why was this mapping created to differ from documentation? It seems odd.

I understand changing the value will now break consumers, but consider making it match Cognito documentation in future major version bump. In the meantime refer to this mapping in documentation/README.md

Reproduction Steps

 const pool = new cognito.UserPool(this, 'MyPool', {
      userPoolName: 'CoolPool',
      standardAttributes: {
        name: {
          required: true,
          mutable: true,
        },
        email: {
          required: true,
          mutable: true,
        }
      },
      (...)
    });

What did you expect to happen?

Have cognito pool with documented parameter name enabled.

What actually happened?

Error building construct:

lib/cognito-pool.ts:32:9 - error TS2322: Type '{ name: { required: true; mutable: true; }; email: { required: true; mutable: true; }; }' is not assignable to type 'StandardAttributes'.
  Object literal may only specify known properties, and 'name' does not exist in type 'StandardAttributes'.

32         name: {
           ~~~~~~~
33           required: true,
   ~~~~~~~~~~~~~~~~~~~~~~~~~
34           mutable: true,
   ~~~~~~~~~~~~~~~~~~~~~~~~
35         },
   ~~~~~~~~~

CDK CLI Version

1.129.0

Framework Version

No response

Node.js Version

v17.0.1

OS

MacOS

Language

Typescript

Language Version

TypeScript (3.9.7)

Other information

No response

@miguelcss miguelcss added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 5, 2021
@github-actions github-actions bot added the @aws-cdk/aws-cognito Related to Amazon Cognito label Nov 5, 2021
@peterwoodworth peterwoodworth added documentation This is a problem with documentation. p2 and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 5, 2021
@peterwoodworth peterwoodworth added the feature-request A feature should be added or improved. label Nov 5, 2021
@peterwoodworth
Copy link
Contributor

@nija-at can you shed some light on why the StandardAttributes properties don't match the cognito documentation?

We should make a note in our documentation somewhere to clarify that there's a mapping

@miguelcss
Copy link
Author

Thanks for taking a look at this. Any feedback?

@nija-at nija-at assigned corymhall and unassigned nija-at Dec 2, 2021
@corymhall corymhall removed their assignment Oct 18, 2022
@karthikeyan-balu
Copy link

fullname is mapped to name

@ashishdhingra
Copy link
Contributor

@miguelcss The fullname is mapped to name attribute per StandardAttributeNames in current CDK v2. Changing property names in publicly released CDK L2 construct is a breaking change. Hence, I guess we should close this issue.

@ashishdhingra ashishdhingra added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 6, 2024
Copy link

github-actions bot commented Jun 8, 2024

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jun 8, 2024
@pahud pahud added p3 and removed p2 labels Jun 11, 2024
@miguelcss
Copy link
Author

@miguelcss The fullname is mapped to name attribute per StandardAttributeNames in current CDK v2. Changing property names in publicly released CDK L2 construct is a breaking change. Hence, I guess we should close this issue.

This is noted in the original issue description.
The issue is not about changing the mapping. It is about updating documentation.

CDK cognito attribute documentation does not mention nor explain there is a mapping.

Developers need to guess that fullname is mapped to name or worst that profilePicture: 'picture', which is not in any example snippet.

The request was to add a note in the CDK cognito attribute documentation section stating that the Standard Attributes are mapped in CDK as per mapping.

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Jun 12, 2024
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 documentation This is a problem with documentation. feature-request A feature should be added or improved. p3
Projects
None yet
Development

No branches or pull requests

7 participants