aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.57k stars 3.88k forks source link

(@aws-cdk/aws-cognito): SettingAttribute 'fullname' is not consistent with documentation. #17361

Open miguelcss opened 2 years ago

miguelcss commented 2 years ago

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

peterwoodworth commented 2 years ago

@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 commented 2 years ago

Thanks for taking a look at this. Any feedback?

karthikeyan-balu commented 1 year ago

fullname is mapped to name

ashishdhingra commented 3 months ago

@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.

github-actions[bot] commented 3 months ago

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.

miguelcss commented 3 months ago

@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.