Adding override for supportedIdentityProviders erases all userPoolClient settings

tannerabread commented 1 year ago

How did you install the Amplify CLI?

npm

If applicable, what version of Node.js are you using?

16.18..1

Amplify CLI Version

10.7.3

What operating system are you using?

Mac

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

No manual changes, but other overrides in the same file

Describe the bug

I am trying to add a SAML provider to my cognito user pool after running amplify override auth, which works great with the following code:

// override.ts
import { AmplifyAuthCognitoStackTemplate } from "@aws-amplify/cli-extensibility-helper";

export function override(resources: AmplifyAuthCognitoStackTemplate) {
  resources.addCfnResource(
    {
      type: "AWS::Cognito::UserPoolIdentityProvider",
      properties: {
        UserPoolId: { Ref: "UserPool" },
        ProviderName: "Azure",
        ProviderDetails: {
          MetadataURL:
            "https://login.microsoftonline.com/<active_directory_id>/federationmetadata/2007-06/federationmetadata.xml?appid=<app_id>",
        },
        ProviderType: "SAML",
        AttributeMapping: {
          email:
            "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress",
        },
        IdpIdentifiers: ["Azure"],
      },
    },
    "Azure"
  );
}

The problem then comes in when I try to add the new SAML provider to the User Pool Clients (regular and web) to add the new SAML option to the Hosted UI.

I have tried with the following:

// NOT CORRECT, adds new client, need to modify existing
  resources.addCfnResource(
    {
      type: "AWS::Cognito::UserPoolClient",
      properties: {
        SupportedIdentityProviders: ["Azure"],
        UserPoolId: { Ref: "UserPool" },
      },
    },
    "Add Azure to supportedIdentityProviders"
  );

  // THROWS ERROR
  // Type '{ attrClientSecret: string; attrName: string; userPoolId: string; accessTokenValidity: number; allowedOAuthFlows: string[]; allowedOAuthFlowsUserPoolClient: boolean | IResolvable; ... 21 more ...; node: ConstructNode; }' is missing the following properties from type 'CfnUserPoolClient': inspect, cfnProperties, renderProperties, _cfnProperties, and 27 more.
  resources.userPoolClientWeb = {
    supportedIdentityProviders: [
      "Azure",
      ...resources.userPoolClientWeb.supportedIdentityProviders,
    ],
    ...resources.userPoolClientWeb
  };

  // THROWS ERROR on amplify push
  // resources.userPoolClientWeb.supportedIdentityProviders is not iterable
  resources.userPoolClientWeb.supportedIdentityProviders = [
    "Azure",
    ...resources.userPoolClientWeb.supportedIdentityProviders,
  ];

  // ERASES ALL OTHER SUPPORTED IDENTITY PROVIDERS AND OTHER HOSTED UI SETTINGS
  resources.userPoolClientWeb.supportedIdentityProviders = ["Azure"];

  // ERASES ALL OTHER SUPPORTED IDENTITY PROVIDERS AND OTHER HOSTED UI SETTINGS
  const userPoolClientWeb = resources.userPoolClientWeb;
    const baseClient = Array.isArray(userPoolClientWeb.supportedIdentityProviders)
      ? userPoolClientWeb.supportedIdentityProviders
      : [userPoolClientWeb.supportedIdentityProviders];
    // resources.userPoolClient.addPropertyOverride("SupportedIdentityProviders", [
    //   ...baseClient,
    //   "Azure",
    // ]);
    resources.userPoolClientWeb.supportedIdentityProviders = [
      ...baseClient,
      "Azure",
    ];

I have logged resources.userPoolClient and it returns the following, noting the undefined in the original supportedIdentityProviders:

userPoolClientWeb: [Circular *1],
    userPoolClient: CfnUserPoolClient {
      node: [ConstructNode],
      stack: [Circular *2],
      logicalId: '${Token[AmplifyAuthCongitoStack.UserPoolClient.LogicalID.676]}',
      cfnOptions: {},
      rawOverrides: {},
      dependsOn: [Set],
      cfnResourceType: 'AWS::Cognito::UserPoolClient',
      _cfnProperties: [Object],
      attrClientSecret: '${Token[TOKEN.677]}',
      attrName: '${Token[TOKEN.678]}',
      userPoolId: '${Token[TOKEN.675]}',
      accessTokenValidity: undefined,
      allowedOAuthFlows: undefined,
      allowedOAuthFlowsUserPoolClient: undefined,
      allowedOAuthScopes: undefined,
      analyticsConfiguration: undefined,
      callbackUrLs: undefined,
      clientName: '<resource_name>_app_client',
      defaultRedirectUri: undefined,
      enablePropagateAdditionalUserContextData: undefined,
      enableTokenRevocation: undefined,
      explicitAuthFlows: undefined,
      generateSecret: '${Token[TOKEN.680]}',
      idTokenValidity: undefined,
      logoutUrLs: undefined,
      preventUserExistenceErrors: undefined,
      readAttributes: undefined,
      refreshTokenValidity: '${Token[TOKEN.679]}',
      supportedIdentityProviders: undefined,
      tokenValidityUnits: [Object],
      writeAttributes: undefined,
      [Symbol(@aws-cdk/core.DependableTrait)]: [Object]
    },

Expected behavior

I expected to be able to do one of the previous methods for overriding, either customizing directly as shown in the docs, or using the addPropertyOverride method.

This instead removes all settings other than the newly added supported provider. See the screenshots below for the comparison:

Original, before trying to override

New, after overriding

Reproduction steps

Create a new barebones React app
amplify add auth, add a provider such as google
amplify push
amplify override auth
Modify the override.ts file to match the file in the bug and create a new SAML resource
Try to override the supportedIdentityProviders field on either userPoolClient or userPoolClientWeb
Check the client in the Cognito console and notice that Hosted UI no longer works or has any of the default settings

Project Identifier

No response

Log output

``` # Put your logs below this line ```

Additional information

No response

Before submitting, please confirm:

[X] I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.
[X] I have removed any sensitive information from my code snippets and submission.

ykethan commented 1 year ago

Hey @tannerabread, was able to find a workaround by using the following.

import { AmplifyAuthCognitoStackTemplate } from "@aws-amplify/cli-extensibility-helper";

export function override(resources: AmplifyAuthCognitoStackTemplate) {
  resources.addCfnResource(
    {
      type: "AWS::Cognito::UserPoolIdentityProvider",
      properties: {
        UserPoolId: { Ref: "UserPool" },
        ProviderName: "Azure",
        ProviderDetails: {
          MetadataURL:
            "<microsoft url>",
        },
        ProviderType: "SAML",
        AttributeMapping: {
          email:
            "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress",
        },
        IdpIdentifiers: ["Azure"],
      },
    },
    "Azure"
  );

  resources.userPoolClient.supportedIdentityProviders = [
    "LoginWithAmazon",
    "Azure",
  ];
  resources.userPoolClient.logoutUrLs = [
    "http://localhost:3000/",
  ];
  resources.userPoolClient.callbackUrLs = [
    "http://localhost:3000/",
  ];

  resources.userPoolClient.allowedOAuthFlows = ["code"];
  resources.userPoolClient.allowedOAuthScopes = ["openid", "email", "profile"];
  resources.userPoolClient.allowedOAuthFlowsUserPoolClient = true;
}

Needed to push azure resource first then push the other resources next.

observed when using ...resources.userPoolClient.supportedIdentityProviders to append to resource it fails with an iterable error and observed the attribute value was undefined. Marking as bug.

igorlg commented 1 year ago

Would be good if addCfnResource returned a reference to the CfnResource created, so it could be used in addDependency calls, like so:

import { AmplifyAuthCognitoStackTemplate, AmplifyProjectInfo } from '@aws-amplify/cli-extensibility-helper';

export function override(resources: AmplifyAuthCognitoStackTemplate, amplifyProjectInfo: AmplifyProjectInfo) {
  const my_sso = resources.addCfnResource({
    type: "AWS::Cognito::UserPoolIdentityProvider",
    properties: {
      AttributeMapping: { [...] },
      ProviderName: "MySSO",
    }
  }, "new-social-provider");

  resources.userPoolClientWeb.supportedIdentityProviders = [ "MySSO" ];
  resources.userPoolClientWeb.addDependency(my_sso);
}

Today this is not possible because resources.addCfnResource returns void... 👎

aws-amplify / amplify-cli