fix(cognito): re-adding threat protection capabilities and clarifying feature plans

[IkeNefcy]

Issue

Closes #33393

Reason for this change

This is to solve the issue, but also to help fix errors introduced by #32367

When Feature Flags was introduced, a bad assumption was made. This assumption is in the comments on #32367

If the advanced security mode is enabled with Essentials or Plus feature plan, CloudFormation will fail with following error:

Resource handler returned message: "The following features need to be disabled for the ESSENTIALS pricing tier configured: Threat Protection (Service: CognitoIdenti
tyProvider, Status Code: 400, Request ID: xx)"

We cannot validate advancedSecurityMode is off when featurePlan is not specified (defaults to Essentials) because existing user pools are set to Lite feature plan for backward compatibility and CDK cannot determine what the actual feature plan is.

Unfortunately, this is not what this error means, and oddly the readme entry they added actually has the correct text.
This text indicates the author thought that in order to use Threat Protection / advanced security mode, you needed to be on LITE mode. but this doesn't make sense, that's the lowest tier..

As confirmed by the issue this PR is closing #33393, you actually need to be on PLUS tier to be able to use Threat Protection.

Also the text "Advanced Security Mode is deprecated in favor of user pool feature plans" which is in multiple places is not correct. All feature plans are, is a flag indicating a price plan. Each price plan enables more features as you go up.

This means Advanced Security Mode isn't actually gone, all they did was rename it to Threat Protection. In fact, CFN has not changed the Advanced Security Mode key name in their interpreter language. Which is the correct call since this would be a very large breaking change if they did.
This is confirmed by the docs for CFN https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cognito-userpool-userpooladdons.html#cfn-cognito-userpool-userpooladdons-advancedsecuritymode
and the docs for L1
https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_cognito.CfnUserPool.UserPoolAddOnsProperty.html

So deprecating Advanced Security Mode is okay, only because a name change to Threat Protection would make more sense.

Description of changes

I've added props for Threat protection. There are 2 keys that can be edited, the mode of the standard user and the mode of a custom user, thus

standardThreatProtectionMode, customThreatProtectionMode

I've allowed advancedSecurityMode to be usable with the deprecation flag, but updated the guidance to talk about threat protection.
Also I have disallowed using advancedSecurityMode and either ThreatProtectionMode key at the same time

    if (
      props.advancedSecurityMode &&
      (props.standardThreatProtectionMode || props.customThreatProtectionMode)
    ) {
      throw new ValidationError('you cannot set Threat Protection and Advanced Security Mode at the same time. Advanced Security Mode is deprecated and should be replaced with Threat Protection instead.', this);
    }

And when I was testing this, I tested an empty cognito user pool, and I got this CFN error

Resource handler returned message: "1 validation error detected: Value null at 'userPoolAddOns.advancedSecurityMode' failed to satisfy constraint: Member must not be null 

Unsure if this is new behavior from CFN, but this is indicating to me that there needs to be a default setting.

So since advancedSecurityMode can't actually be anything but OFF without featurePlan being PLUS, and you can only get to PLUS by specifying the featurePlan key (no key = ESSENTIALS by default, or LITE for backwards compatibility), I am setting a new default value

    const standardThreatProtectionMode = props.standardThreatProtectionMode ?
      props.standardThreatProtectionMode :
      StandardThreatProtectionMode.NO_ENFORCEMENT;

 . . . 

    const chosenSecurityMode = props.advancedSecurityMode ? props.advancedSecurityMode : standardThreatProtectionMode;

Basically if advancedSecurityMode is not specified, then it falls to standardThreatProtectionMode, where if this is not specified, it's set to StandardThreatProtectionMode.NO_ENFORCEMENT. (OFF)

I updated the enum keys to match the UI when using StandardThreatProtectionMode or CustomThreatProtectionMode

export enum StandardThreatProtectionMode {
  /** Cognito automatically takes preventative actions in response to different levels of risk that you configure for your user pool */
  FULL_FUNCTION = 'ENFORCED',
  /** Cognito gathers metrics on detected risks, but doesn't take automatic action */
  AUDIT_ONLY = 'AUDIT',
  /** Cognito doesn't gather metrics on detected risks or automatically take preventative actions */
  NO_ENFORCEMENT = 'OFF',
}

The keys will make more sense to someone who is creating a user pool with CDK, but still map to the required CFN values since CFN is keeping the current behavior.

Description of how you validated changes

I updated the tests created from the previous PR and I added some more to help.

test('advanced security defaults when no option provided', () => {
  // GIVEN
  const stack = new Stack();

  // WHEN
  new UserPool(stack, 'Pool', {});

  // THEN
  Template.fromStack(stack).hasResourceProperties('AWS::Cognito::UserPool', {
    UserPoolAddOns: {
      AdvancedSecurityAdditionalFlows: {},
      AdvancedSecurityMode: 'OFF',
    },
  });
});

test.each([
  [FeaturePlan.ESSENTIALS, AdvancedSecurityMode.AUDIT],
  [FeaturePlan.ESSENTIALS, AdvancedSecurityMode.ENFORCED],
  [FeaturePlan.LITE, AdvancedSecurityMode.AUDIT],
  [FeaturePlan.LITE, AdvancedSecurityMode.ENFORCED],
])('throws when feature plan is %s and advanced security mode is %s', (featurePlan, advancedSecurityMode) => {
  // GIVEN
  const stack = new Stack();

  // WHEN
  expect(() => {
    new UserPool(stack, 'Pool', { featurePlan, advancedSecurityMode });
  }).toThrow('you cannot enable Advanced Security when feature plan is not Plus.');
});

test.each([
  [FeaturePlan.ESSENTIALS, StandardThreatProtectionMode.AUDIT_ONLY],
  [FeaturePlan.ESSENTIALS, StandardThreatProtectionMode.FULL_FUNCTION],
  [FeaturePlan.LITE, StandardThreatProtectionMode.AUDIT_ONLY],
  [FeaturePlan.LITE, StandardThreatProtectionMode.FULL_FUNCTION],
])('throws when feature plan is %s and standard threat protection mode is %s', (featurePlan, standardThreatProtectionMode) => {
  // GIVEN
  const stack = new Stack();

  // WHEN
  expect(() => {
    new UserPool(stack, 'Pool', { featurePlan, standardThreatProtectionMode });
  }).toThrow('you cannot enable Threat Protection when feature plan is not Plus.');
});

test.each([
  [FeaturePlan.ESSENTIALS, CustomThreatProtectionMode.AUDIT_ONLY],
  [FeaturePlan.ESSENTIALS, CustomThreatProtectionMode.FULL_FUNCTION],
  [FeaturePlan.LITE, CustomThreatProtectionMode.AUDIT_ONLY],
  [FeaturePlan.LITE, CustomThreatProtectionMode.FULL_FUNCTION],
])('throws when feature plan is %s and custom threat protection mode is %s', (featurePlan, customThreatProtectionMode) => {
  // GIVEN
  const stack = new Stack();

  // WHEN
  expect(() => {
    new UserPool(stack, 'Pool', { featurePlan, customThreatProtectionMode });
  }).toThrow('you cannot enable Threat Protection when feature plan is not Plus.');
});

test('throws when deprecated property AdvancedSecurityMode and StandardThreatProtectionMode are specified at the same time.', () => {
  // GIVEN
  const stack = new Stack();

  // WHEN
  expect(() => {
    new UserPool(stack, 'Pool', {
      featurePlan: FeaturePlan.PLUS,
      advancedSecurityMode: AdvancedSecurityMode.AUDIT,
      standardThreatProtectionMode: StandardThreatProtectionMode.AUDIT_ONLY,
    });
  }).toThrow('you cannot set Threat Protection and Advanced Security Mode at the same time. Advanced Security Mode is deprecated and should be replaced with Threat Protection instead.');
});

test('throws when deprecated property AdvancedSecurityMode and CustomThreatProtectionMode are specified at the same time.', () => {
  // GIVEN
  const stack = new Stack();

  // WHEN
  expect(() => {
    new UserPool(stack, 'Pool', {
      featurePlan: FeaturePlan.PLUS,
      advancedSecurityMode: AdvancedSecurityMode.AUDIT,
      customThreatProtectionMode: CustomThreatProtectionMode.AUDIT_ONLY,
    });
  }).toThrow('you cannot set Threat Protection and Advanced Security Mode at the same time. Advanced Security Mode is deprecated and should be replaced with Threat Protection instead.');
});

I've updated a few integs but I also wrote my own for this.

import { IntegTest } from '@aws-cdk/integ-tests-alpha';
import { App, RemovalPolicy, Stack } from 'aws-cdk-lib';
import { UserPool, FeaturePlan, StandardThreatProtectionMode, CustomThreatProtectionMode } from 'aws-cdk-lib/aws-cognito';

const app = new App();
const stack = new Stack(app, 'integ-user-pool-threat-protection');

new UserPool(stack, 'userpool-standard-threat-protection', {
  featurePlan: FeaturePlan.PLUS,
  standardThreatProtectionMode: StandardThreatProtectionMode.FULL_FUNCTION,
  removalPolicy: RemovalPolicy.DESTROY,
});

new UserPool(stack, 'userpool-custom-threat-protection', {
  featurePlan: FeaturePlan.PLUS,
  customThreatProtectionMode: CustomThreatProtectionMode.FULL_FUNCTION,
  removalPolicy: RemovalPolicy.DESTROY,
});

new IntegTest(app, 'IntegTest', { testCases: [stack] });

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.21%. Comparing base (f9252ab) to head (6e783b6).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #33565      +/-   ##
==========================================
+ Coverage   82.20%   82.21%   +0.01%     
==========================================
  Files         119      119              
  Lines        6872     6876       +4     
  Branches     1162     1162              
==========================================
+ Hits         5649     5653       +4     
  Misses       1120     1120              
  Partials      103      103              

Flag	Coverage Δ
suite.unit	82.21% <ø> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	∅ <ø> (∅)
packages/aws-cdk-lib/core	82.21% <100.00%> (+0.01%)	⬆️

[IkeNefcy]

I know the flags are copied from the issue I used, but tbh this should be p1 bug

[GavinZZ]

Thank you for the fix! Left some comments.

Pull request has been modified.

[IkeNefcy]

doing one more quick change to reorganize..

[GavinZZ]

LGTM, thank you for the contributions and addressing my questions and comments!

[mergify]

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

[mergify]

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]

AWS CodeBuild CI Report

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

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

[mergify]

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

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.