Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(cognito): re-adding threat protection capabilities and clarifying…
… feature plans (aws#33565) ### Issue # Closes aws#33393 ### Reason for this change This is to solve the issue, but also to help fix errors introduced by [aws#32367](aws#32367) When Feature Flags was introduced, a bad assumption was made. This assumption is in the comments on [aws#32367](aws#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 aws#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 - [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
- Loading branch information