Merge pull request #1032 from jfrog/GH-1031-fix-managed-user-validation

Fix user password validation

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 11.2.1 (July 22, 2024). Tested on Artifactory 7.84.17 with Terraform 1.9.2 and OpenTofu 1.7.3
+
+BUG FIXES:
+
+* resource/artifactory_user, resource/artifactory_managed_user, resource/artifactory_unmanaged_user: Fix `password` validation for interpolated value. Also improve validation logic. Issue: [#1031](https://github.com/jfrog/terraform-provider-artifactory/issues/1031) PR: [#1032](https://github.com/jfrog/terraform-provider-artifactory/pull/1032)
+
 ## 11.2.0 (July 16, 2024). Tested on Artifactory 7.84.17 with Terraform 1.9.2 and OpenTofu 1.7.3
 
 IMPROVEMENTS:

docs/resources/managed_user.md

@@ -7,10 +7,11 @@ subcategory: "User"
 
 Provides an Artifactory managed user resource. This can be used to create and maintain Artifactory users. For example, service account where password is known and managed externally.
 
-Unlike `artifactory_unmanaged_user` and `artifactory_user`, the `password` attribute is required and cannot be empty.
-Consider using a separate provider to generate and manage passwords.
+Unlike `artifactory_unmanaged_user` and `artifactory_user`, the `password` attribute is required and cannot be empty. Consider using a separate provider to generate and manage passwords.
 
-~> The password is stored in the Terraform state file. Make sure you secure it, please refer to the official [Terraform documentation](https://developer.hashicorp.com/terraform/language/state/sensitive-data).
+~>The password is stored in the Terraform state file. Make sure you secure it, please refer to the official [Terraform documentation](https://developer.hashicorp.com/terraform/language/state/sensitive-data).
+
+->Due to Terraform limitation with interpolated value, we can only validate interpolated value prior to making API requests. This means `terraform validate` or `terraform plan` will not return error if `password` does not meet `password_policy` criteria.
 
 ## Example Usage
 
@@ -62,7 +63,7 @@ Optional:
 - `digit` (Number) Minimum number of digits that the password must contain
 - `length` (Number) Minimum length of the password
 - `lowercase` (Number) Minimum number of lowercase letters that the password must contain
-- `special_char` (Number) Minimum number of special char that the password must contain. Special chars list: `!"#$%&'()*+,-./:;<=>?@[\]^_``{|}~`
+- `special_char` (Number) Minimum number of special char that the password must contain. Special chars list: ``!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~``
 - `uppercase` (Number) Minimum number of uppercase letters that the password must contain
 
 ## Import

docs/resources/user.md

@@ -7,9 +7,12 @@ subcategory: "User"
 
 Provides an Artifactory user resource. This can be used to create and manage Artifactory users.
 The password is a required field by the [Artifactory API](https://www.jfrog.com/confluence/display/JFROG/Artifactory+REST+API#ArtifactoryRESTAPI-CreateorReplaceUser), but we made it optional in this resource to accommodate the scenario where the password is not needed and will be reset by the actual user later.  
+
 When the optional attribute `password` is omitted, a random password is generated according to current Artifactory password policy.
 
-~> The generated password won't be stored in the TF state and can not be recovered. The user must reset the password to be able to log in. An admin can always generate the access key for the user as well. The password change won't trigger state drift. We don't recommend to use this resource unless there is a specific use case for it. Recommended resource is `artifactory_managed_user`.
+~>The generated password won't be stored in the TF state and can not be recovered. The user must reset the password to be able to log in. An admin can always generate the access key for the user as well. The password change won't trigger state drift. We don't recommend to use this resource unless there is a specific use case for it. Recommended resource is `artifactory_managed_user`.
+
+->Due to Terraform limitation with interpolated value, we can only validate interpolated value prior to making API requests. This means `terraform validate` or `terraform plan` will not return error if `password` does not meet `password_policy` criteria.
 
 ## Example Usage
 
@@ -65,7 +68,7 @@ Optional:
 - `digit` (Number) Minimum number of digits that the password must contain
 - `length` (Number) Minimum length of the password
 - `lowercase` (Number) Minimum number of lowercase letters that the password must contain
-- `special_char` (Number) Minimum number of special char that the password must contain. Special chars list: `!"#$%&'()*+,-./:;<=>?@[\]^_``{|}~`
+- `special_char` (Number) Minimum number of special char that the password must contain. Special chars list: ``!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~``
 - `uppercase` (Number) Minimum number of uppercase letters that the password must contain
 
 ## Import

pkg/artifactory/resource/user/resource_artifactory_managed_user_test.go

@@ -230,11 +230,11 @@ func TestAccManagedUser_password_policy(t *testing.T) {
 		password   string
 		errorRegex string
 	}{
-		{"Uppercase", "Abcde1234--", `.*Attribute password string must have at least 2 uppercase letters.*`},
-		{"Lowercase", "ABCDe1234--", `.*Attribute password string must have at least 2 lowercase letters.*`},
-		{"Special Char", "ABCDefgh12-", `.*Attribute password string must have at least 2 special characters.*`},
-		{"Digit", "ABCDEfghi1--", `.*Attribute password string must have at least 2 digits.*`},
-		{"Length", "ABcd123--", `.*Attribute password string length must be at least.*`},
+		{"Uppercase", "-A1b2c3d4e-", `.*Attribute password string must have at least 2 uppercase letters.*`},
+		{"Lowercase", "-A1B2C3D4e-", `.*Attribute password string must have at least 2 lowercase letters.*`},
+		{"Special Char", "A1B2CDefgh-", `.*Attribute password string must have at least 2 special characters.*`},
+		{"Digit", "-AfBgChDiE1-", `.*Attribute password string must have at least 2 digits.*`},
+		{"Length", "-A1B2c3d-", `.*Attribute password string length must be at least.*`},
 	}
 
 	for _, tc := range testCase {
@@ -282,6 +282,116 @@ func testAccManagedUserPasswordPolicy(password, errorRegex string) func(t *testi
 	}
 }
 
+func TestAccManagedUser_password_policy_interpolated(t *testing.T) {
+	testCase := []struct {
+		name             string
+		passwordCriteria string
+		errorRegex       string
+	}{
+		{
+			"Uppercase",
+			`length = 10
+			min_lower = 2
+			upper = false
+			min_numeric = 2
+			min_special = 2
+			override_special = "!"#$%%&'()*+,-./:;<=>?@[\]^_\x60{|}~"`,
+			`.*Attribute password string must have at least 2 uppercase letters.*`,
+		},
+		{
+			"Lowercase",
+			`length = 10
+			lower = false
+			min_upper = 2
+			min_numeric = 2
+			min_special = 2
+			override_special = "!"#$%%&'()*+,-./:;<=>?@[\]^_\x60{|}~"`,
+			`.*Attribute password string must have at least 2 lowercase letters.*`,
+		},
+		{
+			"Special Char",
+			`length = 10
+			min_lower = 2
+			min_upper = 2
+			min_numeric = 2
+			special = false
+			override_special = "!"#$%%&'()*+,-./:;<=>?@[\]^_\x60{|}~"`,
+			`.*Attribute password string must have at least 2 special characters.*`,
+		},
+		{
+			"Digit",
+			`length = 10
+			min_lower = 2
+			min_upper = 2
+			numeric = false
+			min_special = 2
+			override_special = "!"#$%%&'()*+,-./:;<=>?@[\]^_\x60{|}~"`,
+			`.*Attribute password string must have at least 2 digits.*`,
+		},
+		{
+			"Length",
+			`length = 9
+			min_lower = 2
+			min_upper = 2
+			min_numeric = 2
+			min_special = 2
+			override_special = "!"#$%%&'()*+,-./:;<=>?@[\]^_\x60{|}~"`,
+			`.*Attribute password string length must be at least.*`,
+		},
+	}
+
+	for _, tc := range testCase {
+		t.Run(tc.name, testAccManagedUserPasswordPolicyInterpolated(tc.passwordCriteria, tc.errorRegex))
+	}
+}
+
+func testAccManagedUserPasswordPolicyInterpolated(passwordCriteria, errorRegex string) func(t *testing.T) {
+	return func(t *testing.T) {
+		id, _, name := testutil.MkNames("test-", "artifactory_user")
+
+		temp := `
+		resource "random_password" "test" {
+			{{ .passwordCriteria }}
+		}
+
+		resource "artifactory_managed_user" "{{ .resourceName }}" {
+			name  = "{{ .name }}"
+			password = random_password.test.result
+			password_policy = {
+				uppercase = 2
+				lowercase = 2
+				special_char = 2
+				digit = 2
+				length = 10
+			}
+			email = "{{ .email }}"
+		}`
+
+		config := util.ExecuteTemplate("TestAccUser_password_policy", temp, map[string]string{
+			"resourceName":     name,
+			"name":             fmt.Sprintf("test-%d", id),
+			"email":            fmt.Sprintf("test-%[email protected]", id),
+			"passwordCriteria": passwordCriteria,
+		})
+
+		resource.Test(t, resource.TestCase{
+			PreCheck: func() { acctest.PreCheck(t) },
+			ExternalProviders: map[string]resource.ExternalProvider{
+				"random": {
+					Source: "hashicorp/random",
+				},
+			},
+			ProtoV6ProviderFactories: acctest.ProtoV6ProviderFactories,
+			Steps: []resource.TestStep{
+				{
+					Config:      config,
+					ExpectError: regexp.MustCompile(errorRegex),
+				},
+			},
+		})
+	}
+}
+
 func TestAccManagedUser_basic(t *testing.T) {
 	id, fqrn, name := testutil.MkNames("test-user-", "artifactory_managed_user")
 	_, _, groupName := testutil.MkNames("test-group-", "artifactory_group")

pkg/artifactory/resource/user/resource_artifactory_user_test.go

@@ -547,11 +547,11 @@ func TestAccUser_password_policy(t *testing.T) {
 		password   string
 		errorRegex string
 	}{
-		{"Uppercase", "Abcde1234--", `.*Attribute password string must have at least 2 uppercase letters.*`},
-		{"Lowercase", "ABCDe1234--", `.*Attribute password string must have at least 2 lowercase letters.*`},
-		{"Special Char", "ABCDefgh12-", `.*Attribute password string must have at least 2 special characters.*`},
-		{"Digit", "ABCDEfghi1--", `.*Attribute password string must have at least 2 digits.*`},
-		{"Length", "ABcd123--", `.*Attribute password string length must be at least.*`},
+		{"Uppercase", "-A1b2c3d4e-", `.*Attribute password string must have at least 2 uppercase letters.*`},
+		{"Lowercase", "-A1B2C3D4e-", `.*Attribute password string must have at least 2 lowercase letters.*`},
+		{"Special Char", "A1B2CDefgh-", `.*Attribute password string must have at least 2 special characters.*`},
+		{"Digit", "-AfBgChDiE1-", `.*Attribute password string must have at least 2 digits.*`},
+		{"Length", "-A1B2c3d-", `.*Attribute password string length must be at least.*`},
 	}
 
 	for _, tc := range testCase {
@@ -561,7 +561,7 @@ func TestAccUser_password_policy(t *testing.T) {
 
 func testAccUserPasswordPolicy(password, errorRegex string) func(t *testing.T) {
 	return func(t *testing.T) {
-		id, fqrn, name := testutil.MkNames("test-", "artifactory_user")
+		id, _, name := testutil.MkNames("test-", "artifactory_user")
 
 		temp := `
 			resource "artifactory_user" "{{ .resourceName }}" {
@@ -588,7 +588,116 @@ func testAccUserPasswordPolicy(password, errorRegex string) func(t *testing.T) {
 		resource.Test(t, resource.TestCase{
 			PreCheck:                 func() { acctest.PreCheck(t) },
 			ProtoV6ProviderFactories: acctest.ProtoV6ProviderFactories,
-			CheckDestroy:             testAccCheckUserDestroy(fqrn),
+			Steps: []resource.TestStep{
+				{
+					Config:      config,
+					ExpectError: regexp.MustCompile(errorRegex),
+				},
+			},
+		})
+	}
+}
+
+func TestAccUser_password_policy_interpolated(t *testing.T) {
+	testCase := []struct {
+		name             string
+		passwordCriteria string
+		errorRegex       string
+	}{
+		{
+			"Uppercase",
+			`length = 10
+			min_lower = 2
+			upper = false
+			min_numeric = 2
+			min_special = 2
+			override_special = "!"#$%%&'()*+,-./:;<=>?@[\]^_\x60{|}~"`,
+			`.*Attribute password string must have at least 2 uppercase letters.*`,
+		},
+		{
+			"Lowercase",
+			`length = 10
+			lower = false
+			min_upper = 2
+			min_numeric = 2
+			min_special = 2
+			override_special = "!"#$%%&'()*+,-./:;<=>?@[\]^_\x60{|}~"`,
+			`.*Attribute password string must have at least 2 lowercase letters.*`,
+		},
+		{
+			"Special Char",
+			`length = 10
+			min_lower = 2
+			min_upper = 2
+			min_numeric = 2
+			special = false
+			override_special = "!"#$%%&'()*+,-./:;<=>?@[\]^_\x60{|}~"`,
+			`.*Attribute password string must have at least 2 special characters.*`,
+		},
+		{
+			"Digit",
+			`length = 10
+			min_lower = 2
+			min_upper = 2
+			numeric = false
+			min_special = 2
+			override_special = "!"#$%%&'()*+,-./:;<=>?@[\]^_\x60{|}~"`,
+			`.*Attribute password string must have at least 2 digits.*`,
+		},
+		{
+			"Length",
+			`length = 9
+			min_lower = 2
+			min_upper = 2
+			min_numeric = 2
+			min_special = 2
+			override_special = "!"#$%%&'()*+,-./:;<=>?@[\]^_\x60{|}~"`,
+			`.*Attribute password string length must be at least.*`,
+		},
+	}
+
+	for _, tc := range testCase {
+		t.Run(tc.name, testAccUserPasswordPolicyInterpolated(tc.passwordCriteria, tc.errorRegex))
+	}
+}
+
+func testAccUserPasswordPolicyInterpolated(passwordCriteria, errorRegex string) func(t *testing.T) {
+	return func(t *testing.T) {
+		id, _, name := testutil.MkNames("test-", "artifactory_user")
+
+		temp := `
+		resource "random_password" "test" {
+			{{ .passwordCriteria }}
+		}
+
+		resource "artifactory_user" "{{ .resourceName }}" {
+			name  = "{{ .name }}"
+			password = random_password.test.result
+			password_policy = {
+				uppercase = 2
+				lowercase = 2
+				special_char = 2
+				digit = 2
+				length = 10
+			}
+			email = "{{ .email }}"
+		}`
+
+		config := util.ExecuteTemplate("TestAccUser_password_policy", temp, map[string]string{
+			"resourceName":     name,
+			"name":             fmt.Sprintf("test-%d", id),
+			"email":            fmt.Sprintf("test-%[email protected]", id),
+			"passwordCriteria": passwordCriteria,
+		})
+
+		resource.Test(t, resource.TestCase{
+			PreCheck: func() { acctest.PreCheck(t) },
+			ExternalProviders: map[string]resource.ExternalProvider{
+				"random": {
+					Source: "hashicorp/random",
+				},
+			},
+			ProtoV6ProviderFactories: acctest.ProtoV6ProviderFactories,
 			Steps: []resource.TestStep{
 				{
 					Config:      config,