Aws-cdk: turn off automatic Cognito UserPool SMS role creation

[kinbald]

:question: General Issue

The Question

Hey there,

Working on Cognito module, I noticed that when you create a UserPool, a role for the SMS service (policy: sns:Publish) is created by default even when you don't specify it.
This behavior is not present in the console nor CloudFormation template. Creating this role is a problem in our environment as we don't use it.
Could you make it optional ?

Thanking you in advance

Environment

• CDK CLI Version: 1.30.0
• Module Version: [email protected]
• OS: all
• Language: Typescript

Other information

Cfn stack example :

{
   "AWSTemplateFormatVersion":"2010-09-09",
   "Resources":{
      "UserPooldev01E6BF40":{
         "Type":"AWS::Cognito::UserPool",
         "Properties":{
            "AdminCreateUserConfig":{
               "AllowAdminCreateUserOnly":false
            },
            "AutoVerifiedAttributes":[
               "email"
            ],
            "EmailVerificationMessage":"Your verification code is {####}",
            "EmailVerificationSubject":"Your verification code",
            "Policies":{
               "PasswordPolicy":{
                  "MinimumLength":8,
                  "RequireLowercase":false,
                  "RequireNumbers":false,
                  "RequireSymbols":false,
                  "RequireUppercase":false,
                  "TemporaryPasswordValidityDays":7
               }
            },
            "Schema":[
               {
                  "Name":"email",
                  "Required":true
               },
               {
                  "Name":"name",
                  "Required":true
               },
               {
                  "AttributeDataType":"String",
                  "Name":"organization",
                  "StringAttributeConstraints":{
                     "MaxLength":"256",
                     "MinLength":"1"
                  }
               }
            ],
            "UsernameAttributes":[
               "email"
            ],
            "UserPoolName":"UserPool-dev",
            "VerificationMessageTemplate":{
               "DefaultEmailOption":"CONFIRM_WITH_CODE",
               "EmailMessage":"Your verification code is {####}",
               "EmailSubject":"Your verification code",
               "SmsMessage":"The verification code to your new account is {####}"
            }
         }
      }
   }
}

[ryaeng]

@nija-at I would like to take a stab at this one. I would create the UserPool via the CDK and verify the role is not in use. I could then comment out the creation of that role in the code and try again. Thoughts?

[nija-at]

@ryaeng - is this so you can check whether a user pool can be created without a role? If so, this sounds good.

[ryaeng]

@ryaeng - is this so you can check whether a user pool can be created without a role? If so, this sounds good.

I’ll give it a shot. Can I reach out if I get stuck?

[ryaeng]

@nija-at The policy is required in order to deploy the UserPool. Looks like this is expected behavior. Is there another way this should be addressed or should we close out the issue?

https://gist.github.com/ryaeng/5beb00102d699bc1867be32c09f33fb6

[brainstorm]

I'm pulling in @dabit3 and @mmatouk from https://github.com/aws-amplify/amplify-js/issues/3495, since I believe this is another counterintuitive issue with the current Cognito deployment/backend in general.

@dabit3 @mmatouk, please take a peek at https://github.com/aws/aws-cdk/issues/6765 if you need more general context.

[ryaeng]

I'm pulling in @dabit3 and @mmatouk from aws-amplify/amplify-js#3495, since I believe this is another counterintuitive issue with the current Cognito deployment/backend in general.

@dabit3 @mmatouk, please take a peek at #6765 if you need more general context.

@brainstorm Is there anything I can do to assist?

[brainstorm]

@ryaeng I do think that the SMS role shouldn't be required for a successful deploy... if it does, it's a design issue that Cognito backend people can fix, imho? It really should be optional.

[nija-at]

@ryaeng -

The policy is required in order to deploy the UserPool. Looks like this is expected behavior. Is there another way this should be addressed or should we close out the issue?

Can you take a look at the generated CloudFormation (and its defaults) to check if there are any attributes enabled that either enable phone verification or have an SMS message encoded?

Perhaps, unsetting them or turning them off would tell Cognito not to look for an SMS role?

[ryaeng]

@nija-at What should I be looking for? I found the following comment under the smsConfiguration belonging to the user-pool library which may be of use.

       * The UserPool is very particular that it must contain an 'sns:Publish' action as an inline policy.
       * Ideally, a conditional that restricts this action to 'sms' protocol needs to be attached, but the UserPool deployment fails validation.
       * Seems like a case of being excessively strict.

[nija-at]

@nija-at The policy is required in order to deploy the UserPool. Looks like this is expected behavior. Is there another way this should be addressed or should we close out the issue?

https://gist.github.com/ryaeng/5beb00102d699bc1867be32c09f33fb6

Can someone provide an example of a UserPool configuration that does not require the SMS role and successfully creates the UserPool with Cognito?

If this is not allowed by the Cognito backend, this option does not make sense for the CDK.

[ArteMisc]

I have verified through both the console and CloudFormation that Cognito does not require an SMS role in order to create a user pool, as long as Phone verification or SMS MFA are not enabled. When using the CDK's CfnUserPool construct, the same results can be achieved.

The CloudFormation template I used to test this is the one provided in the original issue report above. If needed, I can provide some screenshots / logs as proof.

Digging into the code, it seems the SMS role is always included in the CloudFormation template simply because it doesn't check whether one is actually needed.
https://github.com/aws/aws-cdk/blob/40fa93a22ffbdf18b0563d1cef63bbf5814dcc3f/packages/%40aws-cdk/aws-cognito/lib/user-pool.ts#L766-L801

If there is a role, it is used in the returned configuration. If there is no role, a new one is created regardless of the other userpool parameters. The comment about the necessity of the sns:Publish action seems to only relate to Cognito needing the policy to be provided inline with the role as opposed to attached through a named policy.

It seems this issue may be fixed by simply adding a conditional, something along the lines of

if (props.smsRole) {
  // return unchanged
  return { ... };
}

const mfaEnabled = props.mfa === Mfa.OPTIONAL || props.mfa === Mfa.REQUIRED;
const mfaSms = !props.mfaSecondFactor || props.mfaSecondFactor.sms === true;
const phoneVerification = props.signInAliases?.phone === true;
// - maybe also needed if the schema contains the phone attribute?
const requireRole = (mfaEnabled && mfaSms) || phoneVerification;
if (!requireRole) {
  // no role needed or provided
  return undefined;
}

// generate the role
return { ... };

Any thoughts?

[nija-at]

Thanks for the detailed response @ArteMisc. This seems reasonable.

Along with this, I would also recommend introducing a flag disableAutoSmsRole which defaults to false. An error is generated when this is set and, either SMS MFA is enabled or phone verification is enabled.

This flag makes sure that the user doesn't unintentionally create an SMS role. Especially, when they don't have these permissions, this flag and its error message will make it clear what is triggering the creation of the role.

[ArteMisc]

So updating my snippet with your feedback, it would become

if (props.smsRole) {
  // return unchanged
  return { ... };
}

const mfaEnabled = props.mfa === Mfa.OPTIONAL || props.mfa === Mfa.REQUIRED;
const mfaSms = !props.mfaSecondFactor || props.mfaSecondFactor.sms === true;
const phoneVerification = props.signInAliases?.phone === true;
const requireRole = (mfaEnabled && mfaSms) || phoneVerification;
if (!requireRole) {
  // no role needed or provided
  return undefined;
}

// --- this check is added
if (!!props.disableAutoSmsRole) {
  const reasonSMS = (mfaEnabled && mfaSms) ? "SMS mfa enabled" : undefined;
  const reasonVerify = phoneVerification ? "Phone verification enabled" : undefined;
  throw new Error(`UserPool configuration requires an SMS role (reason: ${reasonSMS || reasonVerify}) but disableAutoSmsRole was set to true`);
}

// generate the role
return { ... };

If this seems alright, I could take a stab at implementing it over the weekend.

[nija-at]

Yes, something like that seems reasonable.

[joraycorn]

Any news on that ?

[litsters]

It's August now. Any progress on this? I'm in a situation like the original poster where we don't use SMS at all with our user pool. Also in a role-strict environment where I can't create IAM roles, so this is a blocker.