Add support for Cognito user pool advanced security mode #7361

tyrjola · 2019-01-28T14:17:59Z

Changes proposed in this pull request:

Add 'user_pool_add_ons' argument with option to enable 'advanced_security_mode'

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCognitoUserPool_withAdvancedSecurityMode'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSCognitoUserPool_withAdvancedSecurityMode -timeout 120m
=== RUN   TestAccAWSCognitoUserPool_withAdvancedSecurityMode
=== PAUSE TestAccAWSCognitoUserPool_withAdvancedSecurityMode
=== CONT  TestAccAWSCognitoUserPool_withAdvancedSecurityMode
--- PASS: TestAccAWSCognitoUserPool_withAdvancedSecurityMode (26.03s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	26.063s

bflad

Hi @tyrjola 👋 Thanks for submitting this! I left some initial feedback below. Please let us know if you have any questions or do not have time to implement the items.

bflad · 2019-02-12T05:43:28Z

aws/resource_aws_cognito_user_pool.go

+						"advanced_security_mode": {
+							Type:         schema.TypeString,
+							Required:     true,
+							ValidateFunc: validateCognitoUserPoolAdvancedSecurityMode,


Instead of implementing a custom validation function, we can use the below. 👍

Suggested change

ValidateFunc: validateCognitoUserPoolAdvancedSecurityMode,

ValidateFunc: validation.StringInSlice([]string{

cognitoidentityprovider.AdvancedSecurityModeTypeAudit,

cognitoidentityprovider.AdvancedSecurityModeTypeEnforced,

cognitoidentityprovider.AdvancedSecurityModeTypeOff,

}, false),

bflad · 2019-02-12T05:49:31Z

aws/resource_aws_cognito_user_pool.go

+		configs := v.([]interface{})
+		config, ok := configs[0].(map[string]interface{})
+
+		if ok && config != nil {


In Go, the second return value from a type assertion (the configs[0].(map[string]interface{}) above) is a boolean whether it was successful. The config will always be map[string]interface{} if ok is true.

bflad · 2019-02-12T05:52:58Z

aws/resource_aws_cognito_user_pool.go

@@ -710,6 +739,12 @@ func resourceAwsCognitoUserPoolRead(d *schema.ResourceData, meta interface{}) er
 		d.Set("username_attributes", flattenStringList(resp.UserPool.UsernameAttributes))
 	}

+	if resp.UserPool.UserPoolAddOns != nil && resp.UserPool.UserPoolAddOns.AdvancedSecurityMode != nil {


Since flattenCognitoUserPoolUserPoolAddOns already has a nil check for the passed in *cognitoidentityprovider.UserPoolAddOnsType, we should prefer to remove this conditional so Terraform is always set (even with an empty list) for drift detection.

bflad · 2019-02-12T05:54:04Z

aws/resource_aws_cognito_user_pool_test.go

@@ -736,6 +761,28 @@ resource "aws_cognito_user_pool" "pool" {
 }`, name)
 }

+func testAccAWSCognitoUserPoolConfig_withAdvancedSecurityMode(name string) string {


Nit: Instead of two separate test configurations, the advanced security mode could be passed in as a second parameter.

bflad · 2019-02-12T05:54:53Z

aws/structure.go

+	config := make(map[string]interface{})
+
+	if s == nil {
+		return nil


We should return an empty list here:

Suggested change

return nil

return []map[string]interface{}{}

bflad · 2019-02-12T05:57:20Z

website/docs/r/cognito_user_pool.markdown

@@ -41,6 +41,7 @@ The following arguments are supported:
 * `sms_verification_message` - (Optional) A string representing the SMS verification message.
 * `tags` - (Optional) A mapping of tags to assign to the User Pool.
 * `username_attributes` - (Optional) Specifies whether email addresses or phone numbers can be specified as usernames when a user signs up. Conflicts with `alias_attributes`.
+* `user_pool_add_ons` - (Optional) The configuration for [user pool add-ons](#user-pool-add-ons).


Nit: Might be nice to include some details about this argument before linking to its nested attributes, e.g.

Suggested change

* `user_pool_add_ons` - (Optional) The configuration for [user pool add-ons](#user-pool-add-ons).

* `user_pool_add_ons` - (Optional) Configuration block for [user pool add-ons](#user-pool-add-ons).

aws/resource_aws_cognito_user_pool_test.go

tyrjola · 2019-02-12T13:15:53Z

Hi @bflad, and thank you for the review and improvement suggestions!

I fixed/refactored the PR accordingly, please let me know if it looks ok to you.

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCognitoUserPool_withAdvancedSecurityMode'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSCognitoUserPool_withAdvancedSecurityMode -timeout 120m
=== RUN   TestAccAWSCognitoUserPool_withAdvancedSecurityMode
=== PAUSE TestAccAWSCognitoUserPool_withAdvancedSecurityMode
=== CONT  TestAccAWSCognitoUserPool_withAdvancedSecurityMode
--- PASS: TestAccAWSCognitoUserPool_withAdvancedSecurityMode (36.05s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	36.071s

bflad

Thanks for the updates, @tyrjola! Looks great. 🚀

Output from acceptance testing:

--- PASS: TestAccAWSCognitoUserPool_importBasic (10.15s)
--- PASS: TestAccAWSCognitoUserPool_basic (14.10s)
--- PASS: TestAccAWSCognitoUserPool_withEmailConfiguration (17.01s)
--- PASS: TestAccAWSCognitoUserPool_withVerificationMessageTemplate (18.15s)
--- PASS: TestAccAWSCognitoUserPool_withPasswordPolicy (18.37s)
--- PASS: TestAccAWSCognitoUserPool_withDeviceConfiguration (18.51s)
--- PASS: TestAccAWSCognitoUserPool_withTags (18.56s)
--- PASS: TestAccAWSCognitoUserPool_withSmsVerificationMessage (18.74s)
--- PASS: TestAccAWSCognitoUserPool_withEmailVerificationMessage (20.21s)
--- PASS: TestAccAWSCognitoUserPool_withAdvancedSecurityMode (21.39s)
--- PASS: TestAccAWSCognitoUserPool_withSchemaAttributes (23.19s)
--- PASS: TestAccAWSCognitoUserPool_withSmsConfiguration (26.54s)
--- PASS: TestAccAWSCognitoUserPool_withAliasAttributes (27.46s)
--- PASS: TestAccAWSCognitoUserPool_withSmsConfigurationUpdated (31.83s)
--- PASS: TestAccAWSCognitoUserPool_withAdminCreateUserConfiguration (36.44s)
--- PASS: TestAccAWSCognitoUserPool_update (37.20s)
--- PASS: TestAccAWSCognitoUserPool_withLambdaConfig (37.51s)

bflad · 2019-02-14T23:46:26Z

This has been released in version 1.59.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-03-31T17:59:12Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

cognito: add support for advanced security mode

5e2754b

ghost added size/L Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/cognito tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Jan 28, 2019

bflad reviewed Feb 12, 2019

View reviewed changes

bflad added enhancement Requests to existing resources that expand the functionality or scope. waiting-response Maintainers are waiting on response from community or contributor. labels Feb 12, 2019

tyrjola added 4 commits February 12, 2019 14:37

cognito: refactor advanced sec mode validation

48e4c8c

cognito: sanitize nil checks for user pool add-ons

cbcc918

cognito: refactor user pool add-on tests

e165d07

docs: update user pool add-ons description

6501c50

ghost added size/M Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Feb 12, 2019

ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Feb 12, 2019

bflad added this to the v1.59.0 milestone Feb 13, 2019

bflad approved these changes Feb 13, 2019

View reviewed changes

bflad merged commit 1860d76 into hashicorp:master Feb 13, 2019

bflad added a commit that referenced this pull request Feb 13, 2019

Update CHANGELOG for #7361

014bdae

ghost locked and limited conversation to collaborators Mar 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Cognito user pool advanced security mode #7361

Add support for Cognito user pool advanced security mode #7361

tyrjola commented Jan 28, 2019 •

edited

Loading

bflad left a comment

bflad Feb 12, 2019

bflad Feb 12, 2019

bflad Feb 12, 2019

bflad Feb 12, 2019

bflad Feb 12, 2019

bflad Feb 12, 2019

tyrjola commented Feb 12, 2019

bflad left a comment

bflad commented Feb 14, 2019

ghost commented Mar 31, 2020

-							ValidateFunc: validateCognitoUserPoolAdvancedSecurityMode,
+							ValidateFunc: validation.StringInSlice([]string{
+								cognitoidentityprovider.AdvancedSecurityModeTypeAudit,
+								cognitoidentityprovider.AdvancedSecurityModeTypeEnforced,
+								cognitoidentityprovider.AdvancedSecurityModeTypeOff,
+							}, false),

	* `user_pool_add_ons` - (Optional) The configuration for [user pool add-ons](#user-pool-add-ons).
	* `user_pool_add_ons` - (Optional) Configuration block for [user pool add-ons](#user-pool-add-ons).

Add support for Cognito user pool advanced security mode #7361

Add support for Cognito user pool advanced security mode #7361

Conversation

tyrjola commented Jan 28, 2019 • edited Loading

bflad left a comment

Choose a reason for hiding this comment

bflad Feb 12, 2019

Choose a reason for hiding this comment

bflad Feb 12, 2019

Choose a reason for hiding this comment

bflad Feb 12, 2019

Choose a reason for hiding this comment

bflad Feb 12, 2019

Choose a reason for hiding this comment

bflad Feb 12, 2019

Choose a reason for hiding this comment

bflad Feb 12, 2019

Choose a reason for hiding this comment

tyrjola commented Feb 12, 2019

bflad left a comment

Choose a reason for hiding this comment

bflad commented Feb 14, 2019

ghost commented Mar 31, 2020

tyrjola commented Jan 28, 2019 •

edited

Loading