Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for password_policy.temporary_password_validity_days #10890

Merged
merged 3 commits into from
Jan 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions aws/resource_aws_cognito_user_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ func resourceAwsCognitoUserPool() *schema.Resource {
},
},
"unused_account_validity_days": {
Type: schema.TypeInt,
Optional: true,
Default: 7,
ValidateFunc: validation.IntBetween(0, 90),
Type: schema.TypeInt,
Optional: true,
Deprecated: "Use password_policy.temporary_password_validity_days instead",
ValidateFunc: validation.IntBetween(0, 90),
ConflictsWith: []string{"password_policy.0.temporary_password_validity_days"},
},
},
},
Expand Down Expand Up @@ -295,6 +296,12 @@ func resourceAwsCognitoUserPool() *schema.Resource {
Type: schema.TypeBool,
Optional: true,
},
"temporary_password_validity_days": {
Type: schema.TypeInt,
Optional: true,
ValidateFunc: validation.IntBetween(0, 365),
ConflictsWith: []string{"admin_create_user_config.0.unused_account_validity_days"},
},
},
},
},
Expand Down Expand Up @@ -948,6 +955,11 @@ func resourceAwsCognitoUserPoolUpdate(d *schema.ResourceData, meta interface{})
log.Printf("[DEBUG] Received %s, retrying UpdateUserPool", err)
return resource.RetryableError(err)
}
if isAWSErr(err, cognitoidentityprovider.ErrCodeInvalidParameterException, "Please use TemporaryPasswordValidityDays in PasswordPolicy instead of UnusedAccountValidityDays") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if isAWSErr(err, cognitoidentityprovider.ErrCodeInvalidParameterException, "Please use TemporaryPasswordValidityDays in PasswordPolicy instead of UnusedAccountValidityDays") {
if isAWSErr(err, cognitoidentityprovider.ErrCodeInvalidParameterException, "UnusedAccountValidityDays has been deprecated, set TemporaryPasswordValidityDays in PasswordPolicy instead.") {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have time, I think it would be a good idea to add a test that checks the error condition, but I know this is a blocker for folks who've been following the PR, so I don't want to delay it any longer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change is not working as this is the exact error message from AWS API I need to act on.

Copy link
Contributor Author

@michalschott michalschott Jan 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think the test I've added should do the work. Since we're retrying on error without deprecated field being send, the plan will still show that a change will be required - reason why I have used nonemptyplan.

log.Printf("[DEBUG] Received %s, retrying UpdateUserPool without UnusedAccountValidityDays", err)
params.AdminCreateUserConfig.UnusedAccountValidityDays = nil
return resource.RetryableError(err)
}

return resource.NonRetryableError(err)
})
Expand Down
58 changes: 46 additions & 12 deletions aws/resource_aws_cognito_user_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,21 @@ func TestAccAWSCognitoUserPool_withAdminCreateUserConfiguration(t *testing.T) {
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAWSCognitoUserPoolConfig_withAdminCreateUserConfigurationUpdatedError(name),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "admin_create_user_config.0.unused_account_validity_days", "6"),
resource.TestCheckResourceAttr(resourceName, "admin_create_user_config.0.allow_admin_create_user_only", "false"),
resource.TestCheckResourceAttr(resourceName, "admin_create_user_config.0.invite_message_template.0.email_message", "Your username is {username} and constant password is {####}. "),
resource.TestCheckResourceAttr(resourceName, "admin_create_user_config.0.invite_message_template.0.email_subject", "Foo{####}BaBaz"),
resource.TestCheckResourceAttr(resourceName, "admin_create_user_config.0.invite_message_template.0.sms_message", "Your username is {username} and constant password is {####}."),
),
ExpectNonEmptyPlan: true,
},
{
Config: testAccAWSCognitoUserPoolConfig_withAdminCreateUserConfigurationUpdated(name),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "admin_create_user_config.0.unused_account_validity_days", "7"),
resource.TestCheckResourceAttr(resourceName, "admin_create_user_config.0.unused_account_validity_days", "6"),
resource.TestCheckResourceAttr(resourceName, "admin_create_user_config.0.allow_admin_create_user_only", "false"),
resource.TestCheckResourceAttr(resourceName, "admin_create_user_config.0.invite_message_template.0.email_message", "Your username is {username} and constant password is {####}. "),
resource.TestCheckResourceAttr(resourceName, "admin_create_user_config.0.invite_message_template.0.email_subject", "Foo{####}BaBaz"),
Expand Down Expand Up @@ -484,6 +495,7 @@ func TestAccAWSCognitoUserPool_withPasswordPolicy(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "password_policy.0.require_numbers", "false"),
resource.TestCheckResourceAttr(resourceName, "password_policy.0.require_symbols", "true"),
resource.TestCheckResourceAttr(resourceName, "password_policy.0.require_uppercase", "false"),
resource.TestCheckResourceAttr(resourceName, "password_policy.0.temporary_password_validity_days", "7"),
),
},
{
Expand All @@ -500,6 +512,7 @@ func TestAccAWSCognitoUserPool_withPasswordPolicy(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "password_policy.0.require_numbers", "true"),
resource.TestCheckResourceAttr(resourceName, "password_policy.0.require_symbols", "false"),
resource.TestCheckResourceAttr(resourceName, "password_policy.0.require_uppercase", "true"),
resource.TestCheckResourceAttr(resourceName, "password_policy.0.temporary_password_validity_days", "14"),
),
},
},
Expand Down Expand Up @@ -876,7 +889,7 @@ resource "aws_cognito_user_pool" "test" {
`, name)
}

func testAccAWSCognitoUserPoolConfig_withAdminCreateUserConfigurationUpdated(name string) string {
func testAccAWSCognitoUserPoolConfig_withAdminCreateUserConfigurationUpdatedError(name string) string {
return fmt.Sprintf(`
resource "aws_cognito_user_pool" "test" {
name = "terraform-test-pool-%s"
Expand All @@ -895,6 +908,25 @@ resource "aws_cognito_user_pool" "test" {
`, name)
}

func testAccAWSCognitoUserPoolConfig_withAdminCreateUserConfigurationUpdated(name string) string {
return fmt.Sprintf(`
resource "aws_cognito_user_pool" "test" {
name = "terraform-test-pool-%s"

admin_create_user_config {
allow_admin_create_user_only = false
unused_account_validity_days = 6
aeschright marked this conversation as resolved.
Show resolved Hide resolved

invite_message_template {
email_message = "Your username is {username} and constant password is {####}. "
email_subject = "Foo{####}BaBaz"
sms_message = "Your username is {username} and constant password is {####}."
}
}
}
`, name)
}

func testAccAWSCognitoUserPoolConfig_withAdvancedSecurityMode(name string, mode string) string {
return fmt.Sprintf(`
resource "aws_cognito_user_pool" "test" {
Expand Down Expand Up @@ -1086,11 +1118,12 @@ resource "aws_cognito_user_pool" "test" {
name = "terraform-test-pool-%s"

password_policy {
minimum_length = 7
require_lowercase = true
require_numbers = false
require_symbols = true
require_uppercase = false
minimum_length = 7
require_lowercase = true
require_numbers = false
require_symbols = true
require_uppercase = false
temporary_password_validity_days = 7
}
}
`, name)
Expand All @@ -1102,11 +1135,12 @@ resource "aws_cognito_user_pool" "test" {
name = "terraform-test-pool-%s"

password_policy {
minimum_length = 9
require_lowercase = false
require_numbers = true
require_symbols = false
require_uppercase = true
minimum_length = 9
require_lowercase = false
require_numbers = true
require_symbols = false
require_uppercase = true
temporary_password_validity_days = 14
}
}
`, name)
Expand Down
8 changes: 8 additions & 0 deletions aws/structure.go
Original file line number Diff line number Diff line change
Expand Up @@ -2721,6 +2721,10 @@ func expandCognitoUserPoolPasswordPolicy(config map[string]interface{}) *cognito
configs.RequireUppercase = aws.Bool(v.(bool))
}

if v, ok := config["temporary_password_validity_days"]; ok {
configs.TemporaryPasswordValidityDays = aws.Int64(int64(v.(int)))
}

return configs
}

Expand Down Expand Up @@ -2993,6 +2997,10 @@ func flattenCognitoUserPoolPasswordPolicy(s *cognitoidentityprovider.PasswordPol
m["require_uppercase"] = *s.RequireUppercase
}

if s.TemporaryPasswordValidityDays != nil {
m["temporary_password_validity_days"] = *s.TemporaryPasswordValidityDays
}

if len(m) > 0 {
return []map[string]interface{}{m}
}
Expand Down
3 changes: 2 additions & 1 deletion website/docs/r/cognito_user_pool.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ The following arguments are supported:

* `allow_admin_create_user_only` (Optional) - Set to True if only the administrator is allowed to create user profiles. Set to False if users can sign themselves up via an app.
* `invite_message_template` (Optional) - The [invite message template structure](#invite-message-template).
* `unused_account_validity_days` (Optional) - The user account expiration limit, in days, after which the account is no longer usable.
* `unused_account_validity_days` (Optional) - **DEPRECATED** Use password_policy.temporary_password_validity_days instead - The user account expiration limit, in days, after which the account is no longer usable.

##### Invite Message template

Expand Down Expand Up @@ -87,6 +87,7 @@ The following arguments are supported:
* `require_numbers` (Optional) - Whether you have required users to use at least one number in their password.
* `require_symbols` (Optional) - Whether you have required users to use at least one symbol in their password.
* `require_uppercase` (Optional) - Whether you have required users to use at least one uppercase letter in their password.
* `temporary_password_validity_days` (Optional) - In the password policy you have set, refers to the number of days a temporary password is valid. If the user does not sign-in during this time, their password will need to be reset by an administrator.

#### Schema Attributes

Expand Down