feat(cognito): allow mutable attributes for requiredAttributes #7754

arnulfojr · 2020-05-02T19:55:03Z

I've taken the liberty to implement a preview, refer to #7752

Any feedback is welcome!

BREAKING CHANGE: requiredAttributes on UserPool construct is now replaced with standardAttributes with a slightly modified signature.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation · 2020-05-02T20:34:03Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 8ceffaa
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-05-02T20:38:58Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: d89a22a
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-05-02T20:40:34Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 412978c
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-05-02T21:14:48Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: c8cf596
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-05-03T23:01:59Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 2f29756
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

nija-at

Is this a feature that Cognito supports? I couldn't find any documentation that states that standard attributes can be marked mutable or immutable and Cognito on the AWS console doesn't show these options either.

nija-at

Thanks for the explanation on the GH issue, and taking the effort in submitting a PR.

nija-at · 2020-05-15T08:43:09Z

packages/@aws-cdk/aws-cognito/README.md

  requiredAttributes: {
-    fullname: true,
-    address: true,
+    fullname: new StandardAttribute(),
+    address: StandardAttribute.asMutable(),


The new user experience can be made more slick and easy to use.

Something like this -

standardAttributes: { fullname: { required: true, mutable: false, }, address: { required: false, mutable: true, }, }

We should mark requiredAttributes as deprecated and recommend the use of standardAttributes.

:) that's a way better experience than my proposal, I'll do the changes over the weekend

When you said we should mark requiredAttributes as deprecated you mean we should keep the property the same and simply set it as deprecated?
Can we set it as deprecated but point to the StandardAttribute interface instead? I know it breaks the interface but just wondering what exactly you meant there

I meant to only mark it as deprecated and not change the interface

Pull request has been modified.

aws-cdk-automation · 2020-05-21T20:53:24Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 6a348ea
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-05-21T20:56:46Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 4c56f4b
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-05-21T21:21:45Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 9e8c45f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-05-21T21:30:33Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: c2b6161
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-05-21T21:38:04Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: fac3355
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

nija-at

Thanks for making these changes. The API looks much better now.

nija-at · 2020-05-27T08:39:36Z

packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts

+   * Whether the user's postal address is a required attribute.
+   * @default - Attribute is not required


Suggested change

* Whether the user's postal address is a required attribute.

* @default - Attribute is not required

* The user's postal address.

* @default - see the defaults under `StandardAttribute`

Apply similar changes to the other attributes.

nija-at · 2020-05-27T08:42:22Z

packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts

+   * @default false
+   */
+  readonly mutable?: boolean;


To clarify: standard attributes in Cognito are immutable by default?

nija-at · 2020-05-27T08:46:24Z

packages/@aws-cdk/aws-cognito/lib/user-pool.ts

+const StandardAttributeMap: Record<
+keyof StandardAttributes,
+StandardAttribute
+> = {


Reduce a few line breaks to make the code a bit easier to read -

Suggested change

const StandardAttributeMap: Record<

keyof StandardAttributes,

StandardAttribute

> = {

const StandardAttributeMap: Record<keyof StandardAttributes, StandardAttribute> = {

nija-at · 2020-05-27T08:57:42Z

packages/@aws-cdk/aws-cognito/lib/user-pool.ts

   * The set of attributes that are required for every user in the user pool.
   * Read more on attributes here - https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html
   *
-   * @default - No attributes are required.
+   * @deprecated use {@link standardAttributes}
+   * @default - No required attributes.
   */
  readonly requiredAttributes?: RequiredAttributes;


I was a bit hesitant in the beginning but given that aws-cognito is marked experimental, let's entirely remove the requiredAttributes and RequiredAttributes and call this out as a breaking change.

nija-at · 2020-05-27T09:01:09Z

packages/@aws-cdk/aws-cognito/lib/user-pool.ts

+            standardAttributes[attrName as keyof StandardAttributes]!.required ||
+            standardAttributes[attrName as keyof StandardAttributes]!.mutable,


@ericzbeard tells me that from his experience, the combination of required and mutable (or maybe a different combination) is invalid, and the fact that it's invalid is not clear until the attribute change is attempted. And even at this point, the error message is generic/vague.

Perhaps we should add some validations to prevent users from doing this?

@ericzbeard - can you corroborate?

I'm not totally clear on what this PR is doing. We should make it so that all of the regular attributes like given_name, etc are mutable by default, since the failure when using an IDP has no error message and is very hard to troubleshoot. (Is that the idea here?) What's not allowed is a required custom attribute.

Ah I see. We're not dealing with custom attributes in this PR, so I guess that doesn't apply here.

nija-at · 2020-05-27T09:05:25Z

packages/@aws-cdk/aws-cognito/lib/user-pool.ts

+      const stdAttributes = Object.keys(standardAttributes)
+        .filter(
+          attrName =>
+            !!standardAttributes[attrName as keyof StandardAttributes],
+        ).filter(
+          attrName =>
+            standardAttributes[attrName as keyof StandardAttributes]!.required ||
+            standardAttributes[attrName as keyof StandardAttributes]!.mutable,
+        )
+        .map(attrName => ({
+          name: StandardAttributeMap[attrName as keyof StandardAttributes],
+          ...(standardAttributes[attrName as keyof StandardAttributes] || {}),
+        }));


You may be able to avoid the repeated attrName as keyof StandardAttributes if you use Object.entries() instead of Object.keys()

Pull request has been modified.

rather than taking boolean feat: allow mutable attributes for requiredAttributes

…th standardAttributes

The PR aws#8403 changed the "IAM stack" to use the default environment and forgot to update the expected output (which now does not contain a token for the URL suffix). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Pull request has been modified.

aws-cdk-automation · 2020-06-07T16:55:23Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: bb15b99
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-07T17:09:19Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 812375f
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-07T17:59:08Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 2fb20ac
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-08T10:37:25Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3442a35
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-08T10:40:40Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 49ec34e
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts

aws-cdk-automation · 2020-06-08T11:22:17Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: b305224
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-08T11:35:56Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: daa9318
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-06-08T11:36:01Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2020-06-08T11:36:25Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: daa9318
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-08T12:23:52Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3eb4fcb
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-06-08T12:24:14Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

arnulfojr force-pushed the master branch from 8ceffaa to c8cf596 Compare May 2, 2020 20:31

arnulfojr force-pushed the master branch from c8cf596 to 2f29756 Compare May 3, 2020 22:17

SomayaB changed the title ~~feat: allow mutable attributes for requiredAttributes~~ feat(cognito): allow mutable attributes for requiredAttributes May 5, 2020

SomayaB assigned nija-at May 5, 2020

nija-at reviewed May 12, 2020

View reviewed changes

nija-at mentioned this pull request May 12, 2020

Allow mutable requiredAttributes in Cognito User Pools #7752

Closed

2 tasks

nija-at previously requested changes May 15, 2020

View reviewed changes

arnulfojr force-pushed the master branch 2 times, most recently from c2b6161 to fac3355 Compare May 21, 2020 20:49

nija-at previously requested changes May 27, 2020

View reviewed changes

arnulfojr added 7 commits June 1, 2020 15:30

BREAKING CHANGE: requiredAttributes require an instance of IAttribute

b45ec85

rather than taking boolean feat: allow mutable attributes for requiredAttributes

Updated README

b0cffae

Introduced StandardAttribute

dfc0207

simplified StandardAttribute as simple object

e299ec7

Retouches in comments

1ced138

Reinclude RequiredAttributes interface

35e238f

BREAKING CHANGE: Replaced UserPool property for requiredAttributes wi…

93be392

…th standardAttributes

arnulfojr force-pushed the master branch from 155f113 to 93be392 Compare June 1, 2020 13:33

Elad Ben-Israel and others added 6 commits June 7, 2020 13:21

rename "embedded" to "nested" and move outdir resolution to app

94cc827

hoist runtimeInfo collection to assembly

711d861

update cx schema

2bb8e62

remove "experimental" annotation from Assembly

7715885

Addressing PR comments

bb15b99

Merge branch 'master' of github.com:aws/aws-cdk

812375f

Fix linting

2fb20ac

eladb force-pushed the master branch from 603c608 to 6407535 Compare June 7, 2020 18:03

eladb self-assigned this Jun 8, 2020

Elad Ben-Israel added 3 commits June 8, 2020 13:32

Merge remote-tracking branch 'origin/master'

3442a35

revert core changes

49ec34e

remove extrenous file

b305224

nija-at reviewed Jun 8, 2020

View reviewed changes

Apply suggestions from code review

daa9318

nija-at approved these changes Jun 8, 2020

View reviewed changes

Merge branch 'master' into master

3eb4fcb

mergify bot merged commit 1fabd98 into aws:master Jun 8, 2020

		* Whether the user's postal address is a required attribute.
		* @default - Attribute is not required

		standardAttributes[attrName as keyof StandardAttributes]!.required \|\|
		standardAttributes[attrName as keyof StandardAttributes]!.mutable,

feat(cognito): allow mutable attributes for requiredAttributes #7754

feat(cognito): allow mutable attributes for requiredAttributes #7754

Conversation

arnulfojr commented May 2, 2020 • edited by nija-at Loading

aws-cdk-automation commented May 2, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented May 2, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented May 2, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented May 2, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented May 3, 2020

AWS CodeBuild CI Report

nija-at left a comment

Choose a reason for hiding this comment

nija-at left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arnulfojr May 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-cdk-automation commented May 21, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented May 21, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented May 21, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented May 21, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented May 21, 2020

AWS CodeBuild CI Report

nija-at left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-cdk-automation commented Jun 7, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 7, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 7, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 8, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 8, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 8, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 8, 2020

AWS CodeBuild CI Report

mergify bot commented Jun 8, 2020

aws-cdk-automation commented Jun 8, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 8, 2020

AWS CodeBuild CI Report

mergify bot commented Jun 8, 2020

arnulfojr commented May 2, 2020 •

edited by nija-at

Loading

arnulfojr May 21, 2020 •

edited

Loading