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

Add validation for IAM members. #13203

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/6897.txt
@@ -0,0 +1,3 @@
```release-note:enhancement
iam: Added plan-time validation for IAM members
```
38 changes: 32 additions & 6 deletions google/resource_google_project_iam_binding_test.go
Expand Up @@ -2,6 +2,7 @@ package google

import (
"fmt"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
Expand All @@ -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,
Expand All @@ -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),
},
Expand All @@ -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) },
Expand All @@ -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
{
Expand Down Expand Up @@ -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) },
Expand All @@ -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),

Expand Down Expand Up @@ -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"
Expand All @@ -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 {
Expand Down
23 changes: 23 additions & 0 deletions google/resource_google_project_iam_member_test.go
Expand Up @@ -2,6 +2,7 @@ package google

import (
"fmt"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
Expand Down Expand Up @@ -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" {
Expand Down
32 changes: 28 additions & 4 deletions google/resource_google_project_iam_policy_test.go
Expand Up @@ -3,6 +3,7 @@ package google
import (
"encoding/json"
"fmt"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
Expand All @@ -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,
Expand All @@ -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",
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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"
Expand All @@ -245,7 +269,7 @@ data "google_iam_policy" "admin" {
binding {
role = "roles/storage.objectViewer"
members = [
"user:evanbrown@google.com",
"%s",
]
}
binding {
Expand All @@ -256,7 +280,7 @@ data "google_iam_policy" "admin" {
]
}
}
`, pid, name, org)
`, pid, name, org, member)
}

func testAccProjectAssociatePolicyAuditConfigBasic(pid, name, org string) string {
Expand Down
4 changes: 1 addition & 3 deletions google/resource_iam_binding.go
Expand Up @@ -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"
)

Expand All @@ -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)))
Expand Down
23 changes: 21 additions & 2 deletions google/resource_iam_member.go
Expand Up @@ -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"
)

Expand All @@ -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,
Expand All @@ -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,
Expand Down
10 changes: 8 additions & 2 deletions google/resource_iam_policy.go
Expand Up @@ -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
}
28 changes: 28 additions & 0 deletions google/resource_storage_bucket_iam_test.go
Expand Up @@ -38,6 +38,10 @@ func TestAccStorageBucketIamPolicy(t *testing.T) {
ImportState: true,
ImportStateVerify: true,
},
{
// Test IAM Policy with member 'allUsers'
Config: testAccStorageBucketIamPolicy_allUsers(bucket),
},
},
})
}
Expand Down Expand Up @@ -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)
}