fix(cognito-identitypool-alpha): inconvenient IdentityPoolProviderUrl.userPool() #29025

Tietew · 2024-02-08T04:49:48Z

Reason for this change

IdentityPoolProviderUrl.userPool() requires a string url currently.
The description is "User Pool Provider Url".
It should be `${userPool.userPoolProviderName}:${userPoolClient.userPoolClientId}`.

UserPool has an attribute userPoolProviderUrl which description is "User Pool Provider Url", but confusingly, it cannot be specified to IdentityPoolProviderUrl.userPool().

The format of the identity provider identifier isn't well documented.
See SetIdentityPoolRoles API reference for example of User Pool's identity provider identifier.

Description of changes

This PR fixes IdentityPoolProviderUrl.userPool() to accept UserPool and UserPoolClient instead of a string url.
It generates a correct identifier described above.

Description of how you validated changes

Existing integration test generates an identifier as described above.
The snapshot won't be changed by this PR.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

BREAKING CHANGE: The argument of IdentityPoolProviderUrl.userPool() has been changed from url: string to userPool: UserPool, userPoolClient: UserPoolClient. If you want to specify custom identifier string, use IdentityPoolProviderUrl.custom() instead.

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

Tietew · 2024-02-08T04:52:49Z

Exemption Request
The snapshot should not be changed.

kaizencc

Couple probing questions here to start @Tietew

kaizencc · 2024-02-09T19:15:35Z

packages/@aws-cdk/aws-cognito-identitypool-alpha/lib/identitypool.ts

@@ -155,7 +157,8 @@ export class IdentityPoolProviderUrl {
  }

  /** User Pool Provider Url */
-  public static userPool(url: string): IdentityPoolProviderUrl {
+  public static userPool(userPool: UserPool, userPoolClient: UserPoolClient): IdentityPoolProviderUrl {


I know we're ok with breaking changes, but this seems like it would ruin anyone who was simply supplying a url here that was not connected to a specific UserPool or UserPoolCliet construct. I'm not sure what the difference would be if the IdentityPoolProviderType turns from USER_POOL to CUSTOM

IdentityPoolProviderType (IdentityPoolProviderUrl.type) is not used since #16190 (implemented PR).
Changing .userPool(somthing) to .custom(something) makes no differences in CloudFormation template.

kaizencc · 2024-02-09T19:15:38Z

packages/@aws-cdk/aws-cognito-identitypool-alpha/test/identitypool.test.ts

@@ -440,7 +440,7 @@ describe('role mappings', () => {
    const providerUrl = Fn.importValue('ProviderUrl');
    expect(() => new IdentityPool(stack, 'TestIdentityPoolRoleMappingErrors', {
      roleMappings: [{
-        providerUrl: IdentityPoolProviderUrl.userPool(providerUrl),
+        providerUrl: IdentityPoolProviderUrl.custom(providerUrl),


This would change the IdentityPoolProviderTyle right? How does that affect things?

Same as above. No affects in CloudFormation template.

Tietew · 2024-02-22T07:11:48Z

Updated BREAKING CHANGE in description

aws-cdk-automation · 2024-03-01T00:09:28Z

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

aws-cdk-automation · 2024-03-09T00:07:51Z

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

mergify · 2024-04-17T01:45:38Z

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

TheRealAmazonKendra · 2024-04-17T18:01:02Z

@Mergifyio update

mergify · 2024-04-17T18:01:31Z

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/request-cli-integ-test.yml without workflows permission

Pull request has been modified.

mergify · 2024-04-17T18:30:51Z

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

aws-cdk-automation · 2024-04-17T18:58:58Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: d90abe2
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2024-04-17T18:59:20Z

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

accept userPool and userPoolClient instead of string URL

b516bf1

github-actions bot added the p2 label Feb 8, 2024

aws-cdk-automation requested a review from a team February 8, 2024 04:50

github-actions bot added the valued-contributor [Pilot] contributed between 6-12 PRs to the CDK label Feb 8, 2024

aws-cdk-automation previously requested changes Feb 8, 2024

View reviewed changes

aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Feb 8, 2024

fix jsii-rosetta error

c7407c8

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Feb 8, 2024

kaizencc reviewed Feb 9, 2024

View reviewed changes

Merge branch 'main' into coginto-idpool-userpool-url

1a6f093

github-actions bot mentioned this pull request Mar 1, 2024

Monthly PR metrics report - Feb 2023 #29328

Closed

Merge branch 'main' into coginto-idpool-userpool-url

7a7acab

aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Mar 9, 2024

aws-cdk-automation closed this Mar 9, 2024

paulhcsun reopened this Apr 8, 2024

paulhcsun added pr-linter/do-not-close The PR linter will not close this PR while this label is present and removed closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. labels Apr 8, 2024

TheRealAmazonKendra previously approved these changes Apr 17, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Apr 17, 2024

Merge branch 'main' into coginto-idpool-userpool-url

f379a70

TheRealAmazonKendra approved these changes Apr 17, 2024

View reviewed changes

Merge branch 'main' into coginto-idpool-userpool-url

d90abe2

mergify bot merged commit 90a7734 into aws:main Apr 17, 2024
9 checks passed

This was referenced May 23, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#98

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#99

Open

rwlxxvii mentioned this pull request May 27, 2024

[Snyk] Upgrade aws-cdk-lib from 2.136.1 to 2.140.0 rwlxxvii/containers#124

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cognito-identitypool-alpha): inconvenient IdentityPoolProviderUrl.userPool() #29025

fix(cognito-identitypool-alpha): inconvenient IdentityPoolProviderUrl.userPool() #29025

Tietew commented Feb 8, 2024 •

edited

aws-cdk-automation left a comment

Tietew commented Feb 8, 2024 •

edited

kaizencc left a comment

kaizencc Feb 9, 2024

Tietew Feb 14, 2024

kaizencc Feb 9, 2024

Tietew Feb 14, 2024

Tietew commented Feb 22, 2024

aws-cdk-automation commented Mar 1, 2024

aws-cdk-automation commented Mar 9, 2024

mergify bot commented Apr 17, 2024

TheRealAmazonKendra commented Apr 17, 2024

mergify bot commented Apr 17, 2024

mergify bot commented Apr 17, 2024

aws-cdk-automation commented Apr 17, 2024

mergify bot commented Apr 17, 2024

fix(cognito-identitypool-alpha): inconvenient IdentityPoolProviderUrl.userPool() #29025

fix(cognito-identitypool-alpha): inconvenient IdentityPoolProviderUrl.userPool() #29025

Conversation

Tietew commented Feb 8, 2024 • edited

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Tietew commented Feb 8, 2024 • edited

kaizencc left a comment

Choose a reason for hiding this comment

kaizencc Feb 9, 2024

Choose a reason for hiding this comment

Tietew Feb 14, 2024

Choose a reason for hiding this comment

kaizencc Feb 9, 2024

Choose a reason for hiding this comment

Tietew Feb 14, 2024

Choose a reason for hiding this comment

Tietew commented Feb 22, 2024

aws-cdk-automation commented Mar 1, 2024

aws-cdk-automation commented Mar 9, 2024

mergify bot commented Apr 17, 2024

TheRealAmazonKendra commented Apr 17, 2024

mergify bot commented Apr 17, 2024

❌ Mergify doesn't have permission to update

mergify bot commented Apr 17, 2024

aws-cdk-automation commented Apr 17, 2024

AWS CodeBuild CI Report

mergify bot commented Apr 17, 2024

Tietew commented Feb 8, 2024 •

edited

Tietew commented Feb 8, 2024 •

edited