Add validation for IAM members. (#6897) (#13203)

Signed-off-by: Modular Magician <magic-modules@google.com>

Signed-off-by: Modular Magician <magic-modules@google.com>

.changelog/6897.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+iam: Added plan-time validation for IAM members
+```

google/resource_google_project_iam_binding_test.go

@@ -2,6 +2,7 @@ package google
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -23,6 +24,7 @@ func TestAccProjectIamBinding_basic(t *testing.T) {
 	org := getTestOrgFromEnv(t)
 	pid := fmt.Sprintf("tf-test-%d", randInt(t))
 	role := "roles/compute.instanceAdmin"
+	member := "user:admin@hashicorptest.com"
 	vcrTest(t, resource.TestCase{
 		PreCheck:  func() { testAccPreCheck(t) },
 		Providers: testAccProviders,
@@ -36,7 +38,7 @@ func TestAccProjectIamBinding_basic(t *testing.T) {
 			},
 			// Apply an IAM binding
 			{
-				Config: testAccProjectAssociateBindingBasic(pid, pname, org, role),
+				Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, member),
 			},
 			projectIamBindingImportStep("google_project_iam_binding.acceptance", pid, role),
 		},
@@ -51,6 +53,7 @@ func TestAccProjectIamBinding_multiple(t *testing.T) {
 	pid := fmt.Sprintf("tf-test-%d", randInt(t))
 	role := "roles/compute.instanceAdmin"
 	role2 := "roles/viewer"
+	member := "user:admin@hashicorptest.com"
 
 	vcrTest(t, resource.TestCase{
 		PreCheck:  func() { testAccPreCheck(t) },
@@ -65,7 +68,7 @@ func TestAccProjectIamBinding_multiple(t *testing.T) {
 			},
 			// Apply an IAM binding
 			{
-				Config: testAccProjectAssociateBindingBasic(pid, pname, org, role),
+				Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, member),
 			},
 			// Apply another IAM binding
 			{
@@ -116,6 +119,7 @@ func TestAccProjectIamBinding_update(t *testing.T) {
 	org := getTestOrgFromEnv(t)
 	pid := fmt.Sprintf("tf-test-%d", randInt(t))
 	role := "roles/compute.instanceAdmin"
+	member := "user:admin@hashicorptest.com"
 
 	vcrTest(t, resource.TestCase{
 		PreCheck:  func() { testAccPreCheck(t) },
@@ -130,7 +134,7 @@ func TestAccProjectIamBinding_update(t *testing.T) {
 			},
 			// Apply an IAM binding
 			{
-				Config: testAccProjectAssociateBindingBasic(pid, pname, org, role),
+				Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, member),
 			},
 			projectIamBindingImportStep("google_project_iam_binding.acceptance", pid, role),
 
@@ -248,7 +252,29 @@ func TestAccProjectIamBinding_withCondition(t *testing.T) {
 	})
 }
 
-func testAccProjectAssociateBindingBasic(pid, name, org, role string) string {
+// Test that an IAM binding with invalid members returns an error.
+func TestAccProjectIamBinding_invalidMembers(t *testing.T) {
+	t.Parallel()
+
+	org := getTestOrgFromEnv(t)
+	pid := fmt.Sprintf("tf-test-%d", randInt(t))
+	role := "roles/compute.instanceAdmin"
+	vcrTest(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccProjectAssociateBindingBasic(pid, pname, org, role, "admin@hashicorptest.com"),
+				ExpectError: regexp.MustCompile("invalid value for members\\.0 \\(IAM members must have one of the values outlined here: https://cloud.google.com/billing/docs/reference/rest/v1/Policy#Binding\\)"),
+			},
+			{
+				Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, "user:admin@hashicorptest.com"),
+			},
+		},
+	})
+}
+
+func testAccProjectAssociateBindingBasic(pid, name, org, role, member string) string {
 	return fmt.Sprintf(`
 resource "google_project" "acceptance" {
   project_id = "%s"
@@ -258,10 +284,10 @@ resource "google_project" "acceptance" {
 
 resource "google_project_iam_binding" "acceptance" {
   project = google_project.acceptance.project_id
-  members = ["user:admin@hashicorptest.com"]
+  members = ["%s"]
   role    = "%s"
 }
-`, pid, name, org, role)
+`, pid, name, org, member, role)
 }
 
 func testAccProjectAssociateBindingMultiple(pid, name, org, role, role2 string) string {

google/resource_google_project_iam_member_test.go

@@ -2,6 +2,7 @@ package google
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -167,6 +168,28 @@ func TestAccProjectIamMember_withCondition(t *testing.T) {
 	})
 }
 
+func TestAccProjectIamMember_invalidMembers(t *testing.T) {
+	t.Parallel()
+
+	org := getTestOrgFromEnv(t)
+	pid := fmt.Sprintf("tf-test-%d", randInt(t))
+	role := "roles/compute.instanceAdmin"
+
+	vcrTest(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccProjectAssociateMemberBasic(pid, pname, org, role, "admin@hashicorptest.com"),
+				ExpectError: regexp.MustCompile("invalid value for member \\(IAM members must have one of the values outlined here: https://cloud.google.com/billing/docs/reference/rest/v1/Policy#Binding\\)"),
+			},
+			{
+				Config: testAccProjectAssociateMemberBasic(pid, pname, org, role, "user:admin@hashicorptest.com"),
+			},
+		},
+	})
+}
+
 func testAccProjectAssociateMemberBasic(pid, name, org, role, member string) string {
 	return fmt.Sprintf(`
 resource "google_project" "acceptance" {

google/resource_google_project_iam_policy_test.go

@@ -3,6 +3,7 @@ package google
 import (
 	"encoding/json"
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -16,6 +17,7 @@ func TestAccProjectIamPolicy_basic(t *testing.T) {
 
 	org := getTestOrgFromEnv(t)
 	pid := fmt.Sprintf("tf-test-%d", randInt(t))
+	member := "user:evanbrown@google.com"
 	vcrTest(t, resource.TestCase{
 		PreCheck:  func() { testAccPreCheck(t) },
 		Providers: testAccProviders,
@@ -30,7 +32,7 @@ func TestAccProjectIamPolicy_basic(t *testing.T) {
 			// Apply an IAM policy from a data source. The application
 			// merges policies, so we validate the expected state.
 			{
-				Config: testAccProjectAssociatePolicyBasic(pid, pname, org),
+				Config: testAccProjectAssociatePolicyBasic(pid, pname, org, member),
 			},
 			{
 				ResourceName: "google_project_iam_policy.acceptance",
@@ -156,6 +158,28 @@ func TestAccProjectIamPolicy_withCondition(t *testing.T) {
 	})
 }
 
+// Test that an IAM policy with invalid members returns errors.
+func TestAccProjectIamPolicy_invalidMembers(t *testing.T) {
+	t.Parallel()
+
+	org := getTestOrgFromEnv(t)
+	pid := fmt.Sprintf("tf-test-%d", randInt(t))
+
+	vcrTest(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccProjectAssociatePolicyBasic(pid, pname, org, "admin@hashicorptest.com"),
+				ExpectError: regexp.MustCompile("invalid value for bindings\\.1\\.members\\.0 \\(IAM members must have one of the values outlined here: https://cloud.google.com/billing/docs/reference/rest/v1/Policy#Binding\\)"),
+			},
+			{
+				Config: testAccProjectAssociatePolicyBasic(pid, pname, org, "user:admin@hashicorptest.com"),
+			},
+		},
+	})
+}
+
 func getStatePrimaryResource(s *terraform.State, res, expectedID string) (*terraform.InstanceState, error) {
 	// Get the project resource
 	resource, ok := s.RootModule().Resources[res]
@@ -228,7 +252,7 @@ func testAccProjectExistingPolicy(t *testing.T, pid string) resource.TestCheckFu
 	}
 }
 
-func testAccProjectAssociatePolicyBasic(pid, name, org string) string {
+func testAccProjectAssociatePolicyBasic(pid, name, org, member string) string {
 	return fmt.Sprintf(`
 resource "google_project" "acceptance" {
   project_id = "%s"
@@ -245,7 +269,7 @@ data "google_iam_policy" "admin" {
   binding {
     role = "roles/storage.objectViewer"
     members = [
-      "user:evanbrown@google.com",
+      "%s",
     ]
   }
   binding {
@@ -256,7 +280,7 @@ data "google_iam_policy" "admin" {
     ]
   }
 }
-`, pid, name, org)
+`, pid, name, org, member)
 }
 
 func testAccProjectAssociatePolicyAuditConfigBasic(pid, name, org string) string {

google/resource_iam_binding.go

@@ -4,12 +4,10 @@ import (
 	"errors"
 	"fmt"
 	"log"
-	"regexp"
 	"strings"
 
 	"github.com/davecgh/go-spew/spew"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 	"google.golang.org/api/cloudresourcemanager/v1"
 )
 
@@ -25,7 +23,7 @@ var iamBindingSchema = map[string]*schema.Schema{
 		Elem: &schema.Schema{
 			Type:             schema.TypeString,
 			DiffSuppressFunc: caseDiffSuppress,
-			ValidateFunc:     validation.StringDoesNotMatch(regexp.MustCompile("^deleted:"), "Terraform does not support IAM bindings for deleted principals"),
+			ValidateFunc:     validateIAMMember,
 		},
 		Set: func(v interface{}) int {
 			return schema.HashString(strings.ToLower(v.(string)))

google/resource_iam_member.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/davecgh/go-spew/spew"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 	"google.golang.org/api/cloudresourcemanager/v1"
 )
 
@@ -21,6 +20,26 @@ func iamMemberCaseDiffSuppress(k, old, new string, d *schema.ResourceData) bool
 	return caseDiffSuppress(k, old, new, d)
 }
 
+func validateIAMMember(i interface{}, k string) ([]string, []error) {
+	v, ok := i.(string)
+	if !ok {
+		return nil, []error{fmt.Errorf("expected type of %s to be string", k)}
+	}
+
+	if matched, err := regexp.MatchString("^deleted", v); err != nil {
+		return nil, []error{fmt.Errorf("error validating %s: %v", k, err)}
+	} else if matched {
+		return nil, []error{fmt.Errorf("invalid value for %s (Terraform does not support IAM members for deleted principals)", k)}
+	}
+
+	if matched, err := regexp.MatchString("(.+:.+|allUsers|allAuthenticatedUsers)", v); err != nil {
+		return nil, []error{fmt.Errorf("error validating %s: %v", k, err)}
+	} else if !matched {
+		return nil, []error{fmt.Errorf("invalid value for %s (IAM members must have one of the values outlined here: https://cloud.google.com/billing/docs/reference/rest/v1/Policy#Binding)", k)}
+	}
+	return nil, nil
+}
+
 var IamMemberBaseSchema = map[string]*schema.Schema{
 	"role": {
 		Type:     schema.TypeString,
@@ -32,7 +51,7 @@ var IamMemberBaseSchema = map[string]*schema.Schema{
 		Required:         true,
 		ForceNew:         true,
 		DiffSuppressFunc: iamMemberCaseDiffSuppress,
-		ValidateFunc:     validation.StringDoesNotMatch(regexp.MustCompile("^deleted:"), "Terraform does not support IAM members for deleted principals"),
+		ValidateFunc:     validateIAMMember,
 	},
 	"condition": {
 		Type:     schema.TypeList,

google/resource_iam_policy.go

@@ -179,9 +179,15 @@ func unmarshalIamPolicy(policyData string) (*cloudresourcemanager.Policy, error)
 }
 
 func validateIamPolicy(i interface{}, k string) (s []string, es []error) {
-	_, err := unmarshalIamPolicy(i.(string))
-	if err != nil {
+	if policy, err := unmarshalIamPolicy(i.(string)); err != nil {
 		es = append(es, err)
+	} else {
+		for i, binding := range policy.Bindings {
+			for j, member := range binding.Members {
+				_, memberErrors := validateIAMMember(member, fmt.Sprintf("bindings.%d.members.%d", i, j))
+				es = append(es, memberErrors...)
+			}
+		}
 	}
 	return
 }

google/resource_storage_bucket_iam_test.go

@@ -38,6 +38,10 @@ func TestAccStorageBucketIamPolicy(t *testing.T) {
 				ImportState:       true,
 				ImportStateVerify: true,
 			},
+			{
+				// Test IAM Policy with member 'allUsers'
+				Config: testAccStorageBucketIamPolicy_allUsers(bucket),
+			},
 		},
 	})
 }
@@ -118,3 +122,27 @@ resource "google_storage_bucket_iam_policy" "bucket-binding" {
 }
 `, bucket, account, serviceAcct)
 }
+
+func testAccStorageBucketIamPolicy_allUsers(bucket string) string {
+	return fmt.Sprintf(`
+resource "google_storage_bucket" "bucket" {
+  name     = "%s"
+  location = "US"
+}
+
+data "google_iam_policy" "foo-policy" {
+  binding {
+    role = "roles/storage.objectViewer"
+    members = [
+      "allUsers",
+      "allAuthenticatedUsers",
+    ]
+  }
+}
+
+resource "google_storage_bucket_iam_policy" "bucket-binding" {
+  bucket      = google_storage_bucket.bucket.name
+  policy_data = data.google_iam_policy.foo-policy.policy_data
+}
+`, bucket)
+}