From cad2609b19dc1d0746e693dd667b66fe58dd3444 Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Wed, 9 Aug 2023 15:14:26 +0700 Subject: [PATCH 01/12] feat: introduce new resource type "service_account" in gcloud_iam provider --- plugins/providers/gcloudiam/client.go | 69 +++++----- plugins/providers/gcloudiam/config.go | 60 ++++----- .../gcloudiam/mocks/GcloudIamClient.go | 102 ++++++++++---- plugins/providers/gcloudiam/provider.go | 3 +- plugins/providers/gcloudiam/provider_test.go | 127 +++++++++++------- plugins/providers/gcloudiam/resource.go | 5 +- 6 files changed, 219 insertions(+), 147 deletions(-) diff --git a/plugins/providers/gcloudiam/client.go b/plugins/providers/gcloudiam/client.go index 8aab12d24..149d207be 100644 --- a/plugins/providers/gcloudiam/client.go +++ b/plugins/providers/gcloudiam/client.go @@ -41,42 +41,43 @@ func newIamClient(credentialsJSON []byte, resourceName string) (*iamClient, erro }, nil } -func (c *iamClient) GetRoles() ([]*Role, error) { - var roles []*Role - - ctx := context.TODO() - req := c.iamService.Roles.List() - if err := req.Pages(ctx, func(page *iam.ListRolesResponse) error { - for _, role := range page.Roles { - roles = append(roles, c.fromIamRole(role)) +func (c *iamClient) GetGrantableRoles(ctx context.Context, resourceType string) ([]*iam.Role, error) { + var fullResourceName string + if resourceType == ResourceTypeOrganization { + orgID := strings.Replace(c.resourceName, ResourceNameOrganizationPrefix, "", 1) + fullResourceName = fmt.Sprintf("//cloudresourcemanager.googleapis.com/organizations/%s", orgID) + } else if resourceType == ResourceTypeProject { + projectID := strings.Replace(c.resourceName, ResourceNameProjectPrefix, "", 1) + fullResourceName = fmt.Sprintf("//cloudresourcemanager.googleapis.com/projects/%s", projectID) + } else if resourceType == ResourceTypeServiceAccount { + projectID := strings.Replace(c.resourceName, ResourceNameProjectPrefix, "", 1) + res, err := c.iamService.Projects.ServiceAccounts.List(c.resourceName).PageSize(1).Context(ctx).Do() + if err != nil { + return nil, fmt.Errorf("getting a sample of service account: %w", err) } - return nil - }); err != nil { - return nil, err + if res.Accounts == nil || len(res.Accounts) == 0 { + return nil, fmt.Errorf("no service accounts found in project %s", projectID) + } + fullResourceName = fmt.Sprintf("//iam.googleapis.com/%s", res.Accounts[0].Name) + } else { + return nil, ErrInvalidResourceName } - if strings.HasPrefix(c.resourceName, ResourceNameProjectPrefix) { - projectRolesReq := c.iamService.Projects.Roles.List(c.resourceName) - if err := projectRolesReq.Pages(ctx, func(page *iam.ListRolesResponse) error { - for _, role := range page.Roles { - roles = append(roles, c.fromIamRole(role)) - } - return nil - }); err != nil { - return nil, err + roles := []*iam.Role{} + nextPageToken := "" + for { + req := &iam.QueryGrantableRolesRequest{ + FullResourceName: fullResourceName, + PageToken: nextPageToken, } - } else if strings.HasPrefix(c.resourceName, ResourceNameOrganizationPrefix) { - orgRolesReq := c.iamService.Organizations.Roles.List(c.resourceName) - if err := orgRolesReq.Pages(ctx, func(page *iam.ListRolesResponse) error { - for _, role := range page.Roles { - roles = append(roles, c.fromIamRole(role)) - } - return nil - }); err != nil { + res, err := c.iamService.Roles.QueryGrantableRoles(req).Context(ctx).Do() + if err != nil { return nil, err } - } else { - return nil, ErrInvalidResourceName + roles = append(roles, res.Roles...) + if res.NextPageToken == "" { + break + } } return roles, nil @@ -195,14 +196,6 @@ func (c *iamClient) setIamPolicy(ctx context.Context, policy *cloudresourcemanag return nil, ErrInvalidResourceName } -func (c *iamClient) fromIamRole(r *iam.Role) *Role { - return &Role{ - Name: r.Name, - Title: r.Title, - Description: r.Description, - } -} - func containsString(arr []string, v string) bool { for _, item := range arr { if item == v { diff --git a/plugins/providers/gcloudiam/config.go b/plugins/providers/gcloudiam/config.go index ca6942cca..2a08c63fa 100644 --- a/plugins/providers/gcloudiam/config.go +++ b/plugins/providers/gcloudiam/config.go @@ -1,6 +1,7 @@ package gcloudiam import ( + "context" "encoding/base64" "errors" "fmt" @@ -8,6 +9,7 @@ import ( "github.com/go-playground/validator/v10" "github.com/goto/guardian/domain" + "github.com/goto/guardian/utils" "github.com/mitchellh/mapstructure" ) @@ -102,12 +104,29 @@ func (c *Config) parseAndValidate() error { c.ProviderConfig.Credentials = credentials } - if len(c.ProviderConfig.Resources) != 1 { - return ErrShouldHaveOneResource + if c.ProviderConfig.Resources == nil || len(c.ProviderConfig.Resources) == 0 { + return errors.New("empty resource config") } - r := c.ProviderConfig.Resources[0] - if err := c.validateResourceConfig(r); err != nil { - validationErrors = append(validationErrors, err) + uniqueResourceTypes := make(map[string]bool) + for _, rc := range c.ProviderConfig.Resources { + if _, ok := uniqueResourceTypes[rc.Type]; ok { + validationErrors = append(validationErrors, fmt.Errorf("duplicate resource type: %q", rc.Type)) + } + uniqueResourceTypes[rc.Type] = true + + allowedResourceTypes := []string{} + if strings.HasPrefix(credentials.ResourceName, ResourceNameOrganizationPrefix) { + allowedResourceTypes = []string{ResourceTypeOrganization} + } else if strings.HasPrefix(credentials.ResourceName, ResourceNameProjectPrefix) { + allowedResourceTypes = []string{ResourceTypeProject, ResourceTypeServiceAccount} + } + if !utils.ContainsString(allowedResourceTypes, rc.Type) { + validationErrors = append(validationErrors, fmt.Errorf("invalid resource type: %q", rc.Type)) + } + + if len(rc.Roles) == 0 { + validationErrors = append(validationErrors, ErrRolesShouldNotBeEmpty) + } } if len(validationErrors) > 0 { @@ -142,37 +161,14 @@ func (c *Config) validateCredentials(value interface{}) (*Credentials, error) { return &credentials, nil } -func (c *Config) validateResourceConfig(resource *domain.ResourceConfig) error { - resourceTypeValidation := fmt.Sprintf("oneof=%s %s", ResourceTypeProject, ResourceTypeOrganization) - if err := c.validator.Var(resource.Type, resourceTypeValidation); err != nil { - return err - } - - if len(resource.Roles) == 0 { - return ErrRolesShouldNotBeEmpty - } - - return nil -} - func (c *Config) validatePermissions(resource *domain.ResourceConfig, client GcloudIamClient) error { - iamRoles, err := client.GetRoles() + iamRoles, err := client.GetGrantableRoles(context.TODO(), resource.Type) if err != nil { return err } - - var roles []*domain.Role - for _, r := range iamRoles { - roles = append(roles, &domain.Role{ - ID: r.Name, - Name: r.Title, - Description: r.Description, - }) - } - - rolesMap := make(map[string]*domain.Role) - for _, role := range roles { - rolesMap[role.ID] = role + rolesMap := make(map[string]bool) + for _, role := range iamRoles { + rolesMap[role.Name] = true } for _, ro := range resource.Roles { diff --git a/plugins/providers/gcloudiam/mocks/GcloudIamClient.go b/plugins/providers/gcloudiam/mocks/GcloudIamClient.go index ec9e0b1ee..6ff717f29 100644 --- a/plugins/providers/gcloudiam/mocks/GcloudIamClient.go +++ b/plugins/providers/gcloudiam/mocks/GcloudIamClient.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.10.0. DO NOT EDIT. +// Code generated by mockery v2.20.0. DO NOT EDIT. package mocks @@ -6,7 +6,8 @@ import ( context "context" domain "github.com/goto/guardian/domain" - gcloudiam "github.com/goto/guardian/plugins/providers/gcloudiam" + + iam "google.golang.org/api/iam/v1" mock "github.com/stretchr/testify/mock" ) @@ -24,22 +25,25 @@ func (_m *GcloudIamClient) EXPECT() *GcloudIamClient_Expecter { return &GcloudIamClient_Expecter{mock: &_m.Mock} } -// GetRoles provides a mock function with given fields: -func (_m *GcloudIamClient) GetRoles() ([]*gcloudiam.Role, error) { - ret := _m.Called() +// GetGrantableRoles provides a mock function with given fields: ctx, resourceType +func (_m *GcloudIamClient) GetGrantableRoles(ctx context.Context, resourceType string) ([]*iam.Role, error) { + ret := _m.Called(ctx, resourceType) - var r0 []*gcloudiam.Role - if rf, ok := ret.Get(0).(func() []*gcloudiam.Role); ok { - r0 = rf() + var r0 []*iam.Role + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) ([]*iam.Role, error)); ok { + return rf(ctx, resourceType) + } + if rf, ok := ret.Get(0).(func(context.Context, string) []*iam.Role); ok { + r0 = rf(ctx, resourceType) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]*gcloudiam.Role) + r0 = ret.Get(0).([]*iam.Role) } } - var r1 error - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, resourceType) } else { r1 = ret.Error(1) } @@ -47,28 +51,35 @@ func (_m *GcloudIamClient) GetRoles() ([]*gcloudiam.Role, error) { return r0, r1 } -// GcloudIamClient_GetRoles_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetRoles' -type GcloudIamClient_GetRoles_Call struct { +// GcloudIamClient_GetGrantableRoles_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetGrantableRoles' +type GcloudIamClient_GetGrantableRoles_Call struct { *mock.Call } -// GetRoles is a helper method to define mock.On call -func (_e *GcloudIamClient_Expecter) GetRoles() *GcloudIamClient_GetRoles_Call { - return &GcloudIamClient_GetRoles_Call{Call: _e.mock.On("GetRoles")} +// GetGrantableRoles is a helper method to define mock.On call +// - ctx context.Context +// - resourceType string +func (_e *GcloudIamClient_Expecter) GetGrantableRoles(ctx interface{}, resourceType interface{}) *GcloudIamClient_GetGrantableRoles_Call { + return &GcloudIamClient_GetGrantableRoles_Call{Call: _e.mock.On("GetGrantableRoles", ctx, resourceType)} } -func (_c *GcloudIamClient_GetRoles_Call) Run(run func()) *GcloudIamClient_GetRoles_Call { +func (_c *GcloudIamClient_GetGrantableRoles_Call) Run(run func(ctx context.Context, resourceType string)) *GcloudIamClient_GetGrantableRoles_Call { _c.Call.Run(func(args mock.Arguments) { - run() + run(args[0].(context.Context), args[1].(string)) }) return _c } -func (_c *GcloudIamClient_GetRoles_Call) Return(_a0 []*gcloudiam.Role, _a1 error) *GcloudIamClient_GetRoles_Call { +func (_c *GcloudIamClient_GetGrantableRoles_Call) Return(_a0 []*iam.Role, _a1 error) *GcloudIamClient_GetGrantableRoles_Call { _c.Call.Return(_a0, _a1) return _c } +func (_c *GcloudIamClient_GetGrantableRoles_Call) RunAndReturn(run func(context.Context, string) ([]*iam.Role, error)) *GcloudIamClient_GetGrantableRoles_Call { + _c.Call.Return(run) + return _c +} + // GrantAccess provides a mock function with given fields: accountType, accountID, role func (_m *GcloudIamClient) GrantAccess(accountType string, accountID string, role string) error { ret := _m.Called(accountType, accountID, role) @@ -89,9 +100,9 @@ type GcloudIamClient_GrantAccess_Call struct { } // GrantAccess is a helper method to define mock.On call -// - accountType string -// - accountID string -// - role string +// - accountType string +// - accountID string +// - role string func (_e *GcloudIamClient_Expecter) GrantAccess(accountType interface{}, accountID interface{}, role interface{}) *GcloudIamClient_GrantAccess_Call { return &GcloudIamClient_GrantAccess_Call{Call: _e.mock.On("GrantAccess", accountType, accountID, role)} } @@ -108,11 +119,20 @@ func (_c *GcloudIamClient_GrantAccess_Call) Return(_a0 error) *GcloudIamClient_G return _c } +func (_c *GcloudIamClient_GrantAccess_Call) RunAndReturn(run func(string, string, string) error) *GcloudIamClient_GrantAccess_Call { + _c.Call.Return(run) + return _c +} + // ListAccess provides a mock function with given fields: ctx, resources func (_m *GcloudIamClient) ListAccess(ctx context.Context, resources []*domain.Resource) (domain.MapResourceAccess, error) { ret := _m.Called(ctx, resources) var r0 domain.MapResourceAccess + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, []*domain.Resource) (domain.MapResourceAccess, error)); ok { + return rf(ctx, resources) + } if rf, ok := ret.Get(0).(func(context.Context, []*domain.Resource) domain.MapResourceAccess); ok { r0 = rf(ctx, resources) } else { @@ -121,7 +141,6 @@ func (_m *GcloudIamClient) ListAccess(ctx context.Context, resources []*domain.R } } - var r1 error if rf, ok := ret.Get(1).(func(context.Context, []*domain.Resource) error); ok { r1 = rf(ctx, resources) } else { @@ -137,8 +156,8 @@ type GcloudIamClient_ListAccess_Call struct { } // ListAccess is a helper method to define mock.On call -// - ctx context.Context -// - resources []*domain.Resource +// - ctx context.Context +// - resources []*domain.Resource func (_e *GcloudIamClient_Expecter) ListAccess(ctx interface{}, resources interface{}) *GcloudIamClient_ListAccess_Call { return &GcloudIamClient_ListAccess_Call{Call: _e.mock.On("ListAccess", ctx, resources)} } @@ -155,6 +174,11 @@ func (_c *GcloudIamClient_ListAccess_Call) Return(_a0 domain.MapResourceAccess, return _c } +func (_c *GcloudIamClient_ListAccess_Call) RunAndReturn(run func(context.Context, []*domain.Resource) (domain.MapResourceAccess, error)) *GcloudIamClient_ListAccess_Call { + _c.Call.Return(run) + return _c +} + // RevokeAccess provides a mock function with given fields: accountType, accountID, role func (_m *GcloudIamClient) RevokeAccess(accountType string, accountID string, role string) error { ret := _m.Called(accountType, accountID, role) @@ -175,9 +199,9 @@ type GcloudIamClient_RevokeAccess_Call struct { } // RevokeAccess is a helper method to define mock.On call -// - accountType string -// - accountID string -// - role string +// - accountType string +// - accountID string +// - role string func (_e *GcloudIamClient_Expecter) RevokeAccess(accountType interface{}, accountID interface{}, role interface{}) *GcloudIamClient_RevokeAccess_Call { return &GcloudIamClient_RevokeAccess_Call{Call: _e.mock.On("RevokeAccess", accountType, accountID, role)} } @@ -193,3 +217,23 @@ func (_c *GcloudIamClient_RevokeAccess_Call) Return(_a0 error) *GcloudIamClient_ _c.Call.Return(_a0) return _c } + +func (_c *GcloudIamClient_RevokeAccess_Call) RunAndReturn(run func(string, string, string) error) *GcloudIamClient_RevokeAccess_Call { + _c.Call.Return(run) + return _c +} + +type mockConstructorTestingTNewGcloudIamClient interface { + mock.TestingT + Cleanup(func()) +} + +// NewGcloudIamClient creates a new instance of GcloudIamClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewGcloudIamClient(t mockConstructorTestingTNewGcloudIamClient) *GcloudIamClient { + mock := &GcloudIamClient{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/plugins/providers/gcloudiam/provider.go b/plugins/providers/gcloudiam/provider.go index 5539844b8..a6040a115 100644 --- a/plugins/providers/gcloudiam/provider.go +++ b/plugins/providers/gcloudiam/provider.go @@ -9,11 +9,12 @@ import ( "github.com/goto/guardian/domain" "github.com/mitchellh/mapstructure" "golang.org/x/net/context" + "google.golang.org/api/iam/v1" ) //go:generate mockery --name=GcloudIamClient --exported --with-expecter type GcloudIamClient interface { - GetRoles() ([]*Role, error) + GetGrantableRoles(ctx context.Context, resourceType string) ([]*iam.Role, error) GrantAccess(accountType, accountID, role string) error RevokeAccess(accountType, accountID, role string) error ListAccess(ctx context.Context, resources []*domain.Resource) (domain.MapResourceAccess, error) diff --git a/plugins/providers/gcloudiam/provider_test.go b/plugins/providers/gcloudiam/provider_test.go index c37d38300..93ae4562b 100644 --- a/plugins/providers/gcloudiam/provider_test.go +++ b/plugins/providers/gcloudiam/provider_test.go @@ -10,6 +10,7 @@ import ( "github.com/goto/guardian/plugins/providers/gcloudiam/mocks" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "google.golang.org/api/iam/v1" ) func TestCreateConfig(t *testing.T) { @@ -96,9 +97,11 @@ func TestCreateConfig(t *testing.T) { } testcases := []struct { - pc *domain.ProviderConfig + name string + pc *domain.ProviderConfig }{ { + name: "empty resource config", pc: &domain.ProviderConfig{ Credentials: gcloudiam.Credentials{ ServiceAccountKey: base64.StdEncoding.EncodeToString([]byte("service-account-key-json")), @@ -107,6 +110,7 @@ func TestCreateConfig(t *testing.T) { }, }, { + name: "invalid resource type", pc: &domain.ProviderConfig{ Credentials: gcloudiam.Credentials{ ServiceAccountKey: base64.StdEncoding.EncodeToString([]byte("service-account-key-json")), @@ -120,6 +124,39 @@ func TestCreateConfig(t *testing.T) { }, }, { + name: "duplicate resource types", + pc: &domain.ProviderConfig{ + Credentials: gcloudiam.Credentials{ + ServiceAccountKey: base64.StdEncoding.EncodeToString([]byte("service-account-key-json")), + ResourceName: "projects/test-resource-name", + }, + Resources: []*domain.ResourceConfig{ + { + Type: gcloudiam.ResourceTypeProject, + }, + { + Type: gcloudiam.ResourceTypeProject, + }, + }, + }, + }, + { + name: "service_account resource type in organization-level provider", + pc: &domain.ProviderConfig{ + Credentials: gcloudiam.Credentials{ + ServiceAccountKey: base64.StdEncoding.EncodeToString([]byte("service-account-key-json")), + ResourceName: "organizations/my-organization-id", + }, + Resources: []*domain.ResourceConfig{ + { + Type: gcloudiam.ResourceTypeServiceAccount, + }, + }, + URN: providerURN, + }, + }, + { + name: "empty roles", pc: &domain.ProviderConfig{ Credentials: gcloudiam.Credentials{ ServiceAccountKey: base64.StdEncoding.EncodeToString([]byte("service-account-key-json")), @@ -152,16 +189,16 @@ func TestCreateConfig(t *testing.T) { crypto.On("Encrypt", `{"type":"service_account"}`).Return("", expectedError) - gcloudRole1 := &gcloudiam.Role{ - Name: "roles/bigquery.admin", - Title: "BigQuery Admin", - Description: "Administer all BigQuery resources and data", + gCloudRolesList := []*iam.Role{ + { + Name: "roles/bigquery.admin", + Title: "BigQuery Admin", + Description: "Administer all BigQuery resources and data", + }, } - - gCloudRolesList := []*gcloudiam.Role{} - gCloudRolesList = append(gCloudRolesList, gcloudRole1) - - client.On("GetRoles").Return(gCloudRolesList, nil).Once() + client.EXPECT(). + GetGrantableRoles(mock.AnythingOfType("*context.emptyCtx"), gcloudiam.ResourceTypeProject). + Return(gCloudRolesList, nil).Once() pc := &domain.ProviderConfig{ Resources: []*domain.ResourceConfig{ @@ -197,16 +234,16 @@ func TestCreateConfig(t *testing.T) { providerURN: client, } - gcloudRole1 := &gcloudiam.Role{ - Name: "roles/bigquery.admin", - Title: "BigQuery Admin", - Description: "Administer all BigQuery resources and data", + gCloudRolesList := []*iam.Role{ + { + Name: "roles/bigquery.admin", + Title: "BigQuery Admin", + Description: "Administer all BigQuery resources and data", + }, } - - gCloudRolesList := []*gcloudiam.Role{} - gCloudRolesList = append(gCloudRolesList, gcloudRole1) - - client.On("GetRoles").Return(gCloudRolesList, nil).Once() + client.EXPECT(). + GetGrantableRoles(mock.AnythingOfType("*context.emptyCtx"), gcloudiam.ResourceTypeProject). + Return(gCloudRolesList, nil).Once() crypto.On("Encrypt", `{"type":"service_account"}`).Return(`{"type":"service_account"}`, nil) @@ -274,22 +311,21 @@ func TestGetResources(t *testing.T) { providerURN: client, } - gcloudRole1 := &gcloudiam.Role{ - Name: "roles/bigquery.admin", - Title: "BigQuery Admin", - Description: "Administer all BigQuery resources and data", - } - - gcloudRole2 := &gcloudiam.Role{ - Name: "roles/apigateway.viewer", - Title: "ApiGateway Viewer", - Description: "Read-only access to ApiGateway and related resources", + gCloudRolesList := []*iam.Role{ + { + Name: "roles/bigquery.admin", + Title: "BigQuery Admin", + Description: "Administer all BigQuery resources and data", + }, + { + Name: "roles/apigateway.viewer", + Title: "ApiGateway Viewer", + Description: "Read-only access to ApiGateway and related resources", + }, } - gCloudRolesList := []*gcloudiam.Role{} - gCloudRolesList = append(gCloudRolesList, gcloudRole1) - gCloudRolesList = append(gCloudRolesList, gcloudRole2) - - client.On("GetRoles").Return(gCloudRolesList, nil).Once() + client.EXPECT(). + GetGrantableRoles(mock.AnythingOfType("*context.emptyCtx"), gcloudiam.ResourceTypeProject). + Return(gCloudRolesList, nil).Once() pc := &domain.ProviderConfig{ Type: domain.ProviderTypeGCloudIAM, @@ -300,7 +336,7 @@ func TestGetResources(t *testing.T) { }, Resources: []*domain.ResourceConfig{ { - Type: "project", + Type: gcloudiam.ResourceTypeProject, Roles: []*domain.Role{ { ID: "role-1", @@ -340,16 +376,17 @@ func TestGetResources(t *testing.T) { p.Clients = map[string]gcloudiam.GcloudIamClient{ providerURN: client, } - gcloudRole := &gcloudiam.Role{ - Name: "roles/organisation.admin", - Title: "Organisation Admin", - Description: "Administer all Organisation resources and data", - } - - gCloudRolesList := []*gcloudiam.Role{} - gCloudRolesList = append(gCloudRolesList, gcloudRole) - client.On("GetRoles").Return(gCloudRolesList, nil).Once() + gCloudRolesList := []*iam.Role{ + { + Name: "roles/organisation.admin", + Title: "Organisation Admin", + Description: "Administer all Organisation resources and data", + }, + } + client.EXPECT(). + GetGrantableRoles(mock.AnythingOfType("*context.emptyCtx"), gcloudiam.ResourceTypeOrganization). + Return(gCloudRolesList, nil).Once() pc := &domain.ProviderConfig{ Type: domain.ProviderTypeGCloudIAM, URN: providerURN, @@ -358,7 +395,7 @@ func TestGetResources(t *testing.T) { }, Resources: []*domain.ResourceConfig{ { - Type: "organization", + Type: gcloudiam.ResourceTypeOrganization, Roles: []*domain.Role{ { ID: "role-1", diff --git a/plugins/providers/gcloudiam/resource.go b/plugins/providers/gcloudiam/resource.go index ed3efc8cd..76f64b4c5 100644 --- a/plugins/providers/gcloudiam/resource.go +++ b/plugins/providers/gcloudiam/resource.go @@ -1,8 +1,9 @@ package gcloudiam const ( - ResourceTypeProject = "project" - ResourceTypeOrganization = "organization" + ResourceTypeProject = "project" + ResourceTypeOrganization = "organization" + ResourceTypeServiceAccount = "service_account" ) type Role struct { From a5d7fbe48a0013f82013dde6cba78b13d146f51d Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Thu, 10 Aug 2023 10:13:29 +0700 Subject: [PATCH 02/12] feat: grant and revoke access to service account --- plugins/providers/gcloudiam/client.go | 73 +++++++++ .../gcloudiam/mocks/GcloudIamClient.go | 146 ++++++++++++++++++ plugins/providers/gcloudiam/provider.go | 103 ++++++++---- plugins/providers/gcloudiam/provider_test.go | 3 + 4 files changed, 296 insertions(+), 29 deletions(-) diff --git a/plugins/providers/gcloudiam/client.go b/plugins/providers/gcloudiam/client.go index 149d207be..9237d4738 100644 --- a/plugins/providers/gcloudiam/client.go +++ b/plugins/providers/gcloudiam/client.go @@ -41,6 +41,14 @@ func newIamClient(credentialsJSON []byte, resourceName string) (*iamClient, erro }, nil } +func (c *iamClient) ListServiceAccounts(ctx context.Context) ([]*iam.ServiceAccount, error) { + res, err := c.iamService.Projects.ServiceAccounts.List(c.resourceName).Context(ctx).Do() + if err != nil { + return nil, err + } + return res.Accounts, nil +} + func (c *iamClient) GetGrantableRoles(ctx context.Context, resourceType string) ([]*iam.Role, error) { var fullResourceName string if resourceType == ResourceTypeOrganization { @@ -139,6 +147,71 @@ func (c *iamClient) RevokeAccess(accountType, accountID, role string) error { return err } +func (c *iamClient) GrantServiceAccountAccess(ctx context.Context, sa, accountType, accountID, role string) error { + policy, err := c.iamService.Projects.ServiceAccounts. + GetIamPolicy(sa).Context(ctx).Do() + if err != nil { + return fmt.Errorf("getting IAM policy of service account %q: %w", sa, err) + } + + member := fmt.Sprintf("%s:%s", accountType, accountID) + roleExists := false + for _, b := range policy.Bindings { + if b.Role == role { + if containsString(b.Members, member) { + return ErrPermissionAlreadyExists + } + b.Members = append(b.Members, member) + } + } + if !roleExists { + policy.Bindings = append(policy.Bindings, &iam.Binding{ + Role: role, + Members: []string{member}, + }) + } + + if _, err := c.iamService.Projects.ServiceAccounts. + SetIamPolicy(sa, &iam.SetIamPolicyRequest{Policy: policy}). + Context(ctx).Do(); err != nil { + return fmt.Errorf("setting IAM policy of service account %q: %w", sa, err) + } + + return nil +} + +func (c *iamClient) RevokeServiceAccountAccess(ctx context.Context, sa, accountType, accountID, role string) error { + policy, err := c.iamService.Projects.ServiceAccounts. + GetIamPolicy(sa).Context(ctx).Do() + if err != nil { + return fmt.Errorf("getting IAM policy of service account %q: %w", sa, err) + } + + member := fmt.Sprintf("%s:%s", accountType, accountID) + for _, b := range policy.Bindings { + if b.Role == role { + removeIndex := -1 + for i, m := range b.Members { + if m == member { + removeIndex = i + } + } + if removeIndex == -1 { + return ErrPermissionNotFound + } + b.Members = append(b.Members[:removeIndex], b.Members[removeIndex+1:]...) + } + } + + if _, err := c.iamService.Projects.ServiceAccounts. + SetIamPolicy(sa, &iam.SetIamPolicyRequest{Policy: policy}). + Context(ctx).Do(); err != nil { + return fmt.Errorf("setting IAM policy of service account %q: %w", sa, err) + } + + return nil +} + func (c *iamClient) ListAccess(ctx context.Context, resources []*domain.Resource) (domain.MapResourceAccess, error) { policy, err := c.getIamPolicy(ctx) if err != nil { diff --git a/plugins/providers/gcloudiam/mocks/GcloudIamClient.go b/plugins/providers/gcloudiam/mocks/GcloudIamClient.go index 6ff717f29..8d687aea3 100644 --- a/plugins/providers/gcloudiam/mocks/GcloudIamClient.go +++ b/plugins/providers/gcloudiam/mocks/GcloudIamClient.go @@ -124,6 +124,52 @@ func (_c *GcloudIamClient_GrantAccess_Call) RunAndReturn(run func(string, string return _c } +// GrantServiceAccountAccess provides a mock function with given fields: ctx, sa, accountType, accountID, roles +func (_m *GcloudIamClient) GrantServiceAccountAccess(ctx context.Context, sa string, accountType string, accountID string, roles string) error { + ret := _m.Called(ctx, sa, accountType, accountID, roles) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) error); ok { + r0 = rf(ctx, sa, accountType, accountID, roles) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// GcloudIamClient_GrantServiceAccountAccess_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GrantServiceAccountAccess' +type GcloudIamClient_GrantServiceAccountAccess_Call struct { + *mock.Call +} + +// GrantServiceAccountAccess is a helper method to define mock.On call +// - ctx context.Context +// - sa string +// - accountType string +// - accountID string +// - roles string +func (_e *GcloudIamClient_Expecter) GrantServiceAccountAccess(ctx interface{}, sa interface{}, accountType interface{}, accountID interface{}, roles interface{}) *GcloudIamClient_GrantServiceAccountAccess_Call { + return &GcloudIamClient_GrantServiceAccountAccess_Call{Call: _e.mock.On("GrantServiceAccountAccess", ctx, sa, accountType, accountID, roles)} +} + +func (_c *GcloudIamClient_GrantServiceAccountAccess_Call) Run(run func(ctx context.Context, sa string, accountType string, accountID string, roles string)) *GcloudIamClient_GrantServiceAccountAccess_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(string), args[4].(string)) + }) + return _c +} + +func (_c *GcloudIamClient_GrantServiceAccountAccess_Call) Return(_a0 error) *GcloudIamClient_GrantServiceAccountAccess_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *GcloudIamClient_GrantServiceAccountAccess_Call) RunAndReturn(run func(context.Context, string, string, string, string) error) *GcloudIamClient_GrantServiceAccountAccess_Call { + _c.Call.Return(run) + return _c +} + // ListAccess provides a mock function with given fields: ctx, resources func (_m *GcloudIamClient) ListAccess(ctx context.Context, resources []*domain.Resource) (domain.MapResourceAccess, error) { ret := _m.Called(ctx, resources) @@ -179,6 +225,60 @@ func (_c *GcloudIamClient_ListAccess_Call) RunAndReturn(run func(context.Context return _c } +// ListServiceAccounts provides a mock function with given fields: _a0 +func (_m *GcloudIamClient) ListServiceAccounts(_a0 context.Context) ([]*iam.ServiceAccount, error) { + ret := _m.Called(_a0) + + var r0 []*iam.ServiceAccount + var r1 error + if rf, ok := ret.Get(0).(func(context.Context) ([]*iam.ServiceAccount, error)); ok { + return rf(_a0) + } + if rf, ok := ret.Get(0).(func(context.Context) []*iam.ServiceAccount); ok { + r0 = rf(_a0) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*iam.ServiceAccount) + } + } + + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(_a0) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GcloudIamClient_ListServiceAccounts_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListServiceAccounts' +type GcloudIamClient_ListServiceAccounts_Call struct { + *mock.Call +} + +// ListServiceAccounts is a helper method to define mock.On call +// - _a0 context.Context +func (_e *GcloudIamClient_Expecter) ListServiceAccounts(_a0 interface{}) *GcloudIamClient_ListServiceAccounts_Call { + return &GcloudIamClient_ListServiceAccounts_Call{Call: _e.mock.On("ListServiceAccounts", _a0)} +} + +func (_c *GcloudIamClient_ListServiceAccounts_Call) Run(run func(_a0 context.Context)) *GcloudIamClient_ListServiceAccounts_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context)) + }) + return _c +} + +func (_c *GcloudIamClient_ListServiceAccounts_Call) Return(_a0 []*iam.ServiceAccount, _a1 error) *GcloudIamClient_ListServiceAccounts_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *GcloudIamClient_ListServiceAccounts_Call) RunAndReturn(run func(context.Context) ([]*iam.ServiceAccount, error)) *GcloudIamClient_ListServiceAccounts_Call { + _c.Call.Return(run) + return _c +} + // RevokeAccess provides a mock function with given fields: accountType, accountID, role func (_m *GcloudIamClient) RevokeAccess(accountType string, accountID string, role string) error { ret := _m.Called(accountType, accountID, role) @@ -223,6 +323,52 @@ func (_c *GcloudIamClient_RevokeAccess_Call) RunAndReturn(run func(string, strin return _c } +// RevokeServiceAccountAccess provides a mock function with given fields: ctx, sa, accountType, accountID, role +func (_m *GcloudIamClient) RevokeServiceAccountAccess(ctx context.Context, sa string, accountType string, accountID string, role string) error { + ret := _m.Called(ctx, sa, accountType, accountID, role) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) error); ok { + r0 = rf(ctx, sa, accountType, accountID, role) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// GcloudIamClient_RevokeServiceAccountAccess_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RevokeServiceAccountAccess' +type GcloudIamClient_RevokeServiceAccountAccess_Call struct { + *mock.Call +} + +// RevokeServiceAccountAccess is a helper method to define mock.On call +// - ctx context.Context +// - sa string +// - accountType string +// - accountID string +// - role string +func (_e *GcloudIamClient_Expecter) RevokeServiceAccountAccess(ctx interface{}, sa interface{}, accountType interface{}, accountID interface{}, role interface{}) *GcloudIamClient_RevokeServiceAccountAccess_Call { + return &GcloudIamClient_RevokeServiceAccountAccess_Call{Call: _e.mock.On("RevokeServiceAccountAccess", ctx, sa, accountType, accountID, role)} +} + +func (_c *GcloudIamClient_RevokeServiceAccountAccess_Call) Run(run func(ctx context.Context, sa string, accountType string, accountID string, role string)) *GcloudIamClient_RevokeServiceAccountAccess_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(string), args[4].(string)) + }) + return _c +} + +func (_c *GcloudIamClient_RevokeServiceAccountAccess_Call) Return(_a0 error) *GcloudIamClient_RevokeServiceAccountAccess_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *GcloudIamClient_RevokeServiceAccountAccess_Call) RunAndReturn(run func(context.Context, string, string, string, string) error) *GcloudIamClient_RevokeServiceAccountAccess_Call { + _c.Call.Return(run) + return _c +} + type mockConstructorTestingTNewGcloudIamClient interface { mock.TestingT Cleanup(func()) diff --git a/plugins/providers/gcloudiam/provider.go b/plugins/providers/gcloudiam/provider.go index a6040a115..35c55f2dd 100644 --- a/plugins/providers/gcloudiam/provider.go +++ b/plugins/providers/gcloudiam/provider.go @@ -3,7 +3,6 @@ package gcloudiam import ( "errors" "fmt" - "strings" "github.com/goto/guardian/core/provider" "github.com/goto/guardian/domain" @@ -18,6 +17,9 @@ type GcloudIamClient interface { GrantAccess(accountType, accountID, role string) error RevokeAccess(accountType, accountID, role string) error ListAccess(ctx context.Context, resources []*domain.Resource) (domain.MapResourceAccess, error) + ListServiceAccounts(context.Context) ([]*iam.ServiceAccount, error) + GrantServiceAccountAccess(ctx context.Context, sa, accountType, accountID, roles string) error + RevokeServiceAccountAccess(ctx context.Context, sa, accountType, accountID, role string) error } //go:generate mockery --name=encryptor --exported --with-expecter @@ -68,27 +70,50 @@ func (p *Provider) CreateConfig(pc *domain.ProviderConfig) error { } func (p *Provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, error) { - var creds Credentials - if err := mapstructure.Decode(pc.Credentials, &creds); err != nil { - return nil, err - } + resources := []*domain.Resource{} + + for _, rc := range pc.Resources { + switch rc.Type { + case ResourceTypeProject, ResourceTypeOrganization: + var creds Credentials + if err := mapstructure.Decode(pc.Credentials, &creds); err != nil { + return nil, err + } + resources = append(resources, &domain.Resource{ + ProviderType: pc.Type, + ProviderURN: pc.URN, + Type: rc.Type, + URN: creds.ResourceName, + Name: fmt.Sprintf("%s - GCP IAM", creds.ResourceName), + }) + + case ResourceTypeServiceAccount: + client, err := p.getIamClient(pc) + if err != nil { + return nil, fmt.Errorf("initializing iam client: %w", err) + } + + serviceAccounts, err := client.ListServiceAccounts(context.TODO()) + if err != nil { + return nil, fmt.Errorf("listing service accounts: %w", err) + } - var t string - if strings.HasPrefix(creds.ResourceName, "project") { - t = ResourceTypeProject - } else if strings.HasPrefix(creds.ResourceName, "organization") { - t = ResourceTypeOrganization + for _, sa := range serviceAccounts { + resources = append(resources, &domain.Resource{ + ProviderType: pc.Type, + ProviderURN: pc.URN, + Type: rc.Type, + URN: sa.Name, + Name: sa.DisplayName, + }) + } + + default: + return nil, ErrInvalidResourceType + } } - return []*domain.Resource{ - { - ProviderType: pc.Type, - ProviderURN: pc.URN, - Type: t, - URN: creds.ResourceName, - Name: fmt.Sprintf("%s - GCP IAM", creds.ResourceName), - }, - }, nil + return resources, nil } func (p *Provider) GrantAccess(pc *domain.ProviderConfig, a domain.Grant) error { @@ -104,20 +129,30 @@ func (p *Provider) GrantAccess(pc *domain.ProviderConfig, a domain.Grant) error return err } - if a.Resource.Type == ResourceTypeProject || a.Resource.Type == ResourceTypeOrganization { + switch a.Resource.Type { + case ResourceTypeProject, ResourceTypeOrganization: for _, p := range a.Permissions { - permission := fmt.Sprint(p) - if err := client.GrantAccess(a.AccountType, a.AccountID, permission); err != nil { + if err := client.GrantAccess(a.AccountType, a.AccountID, p); err != nil { if !errors.Is(err, ErrPermissionAlreadyExists) { return err } } } + return nil + case ResourceTypeServiceAccount: + for _, p := range a.Permissions { + if err := client.GrantServiceAccountAccess(context.TODO(), a.Resource.URN, a.AccountType, a.AccountID, p); err != nil { + if !errors.Is(err, ErrPermissionAlreadyExists) { + return err + } + } + } return nil - } - return ErrInvalidResourceType + default: + return ErrInvalidResourceType + } } func (p *Provider) RevokeAccess(pc *domain.ProviderConfig, a domain.Grant) error { @@ -131,20 +166,30 @@ func (p *Provider) RevokeAccess(pc *domain.ProviderConfig, a domain.Grant) error return err } - if a.Resource.Type == ResourceTypeProject || a.Resource.Type == ResourceTypeOrganization { + switch a.Resource.Type { + case ResourceTypeProject, ResourceTypeOrganization: for _, p := range a.Permissions { - permission := fmt.Sprint(p) - if err := client.RevokeAccess(a.AccountType, a.AccountID, permission); err != nil { + if err := client.RevokeAccess(a.AccountType, a.AccountID, p); err != nil { if !errors.Is(err, ErrPermissionNotFound) { return err } } } + return nil + case ResourceTypeServiceAccount: + for _, p := range a.Permissions { + if err := client.RevokeServiceAccountAccess(context.TODO(), a.Resource.URN, a.AccountType, a.AccountID, p); err != nil { + if !errors.Is(err, ErrPermissionNotFound) { + return err + } + } + } return nil - } - return ErrInvalidResourceType + default: + return ErrInvalidResourceType + } } func (p *Provider) GetRoles(pc *domain.ProviderConfig, resourceType string) ([]*domain.Role, error) { diff --git a/plugins/providers/gcloudiam/provider_test.go b/plugins/providers/gcloudiam/provider_test.go index 93ae4562b..77de4b857 100644 --- a/plugins/providers/gcloudiam/provider_test.go +++ b/plugins/providers/gcloudiam/provider_test.go @@ -294,6 +294,9 @@ func TestGetResources(t *testing.T) { Type: domain.ProviderTypeGCloudIAM, URN: "test-project-id", Credentials: "invalid-creds", + Resources: []*domain.ResourceConfig{ + {Type: gcloudiam.ResourceTypeProject}, + }, } actualResources, actualError := p.GetResources(pc) From 8f97f54fcaaf9af12ab6e723385f0ea4d8d34b34 Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Mon, 14 Aug 2023 10:37:34 +0700 Subject: [PATCH 03/12] test: add test cases for service account provider --- plugins/providers/gcloudiam/provider.go | 26 ++-- plugins/providers/gcloudiam/provider_test.go | 128 ++++++++++++++++++- 2 files changed, 138 insertions(+), 16 deletions(-) diff --git a/plugins/providers/gcloudiam/provider.go b/plugins/providers/gcloudiam/provider.go index 35c55f2dd..58daebffa 100644 --- a/plugins/providers/gcloudiam/provider.go +++ b/plugins/providers/gcloudiam/provider.go @@ -98,6 +98,8 @@ func (p *Provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, return nil, fmt.Errorf("listing service accounts: %w", err) } + // TODO: filter + for _, sa := range serviceAccounts { resources = append(resources, &domain.Resource{ ProviderType: pc.Type, @@ -116,7 +118,7 @@ func (p *Provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, return resources, nil } -func (p *Provider) GrantAccess(pc *domain.ProviderConfig, a domain.Grant) error { +func (p *Provider) GrantAccess(pc *domain.ProviderConfig, g domain.Grant) error { // TODO: validate provider config and appeal var creds Credentials @@ -129,10 +131,10 @@ func (p *Provider) GrantAccess(pc *domain.ProviderConfig, a domain.Grant) error return err } - switch a.Resource.Type { + switch g.Resource.Type { case ResourceTypeProject, ResourceTypeOrganization: - for _, p := range a.Permissions { - if err := client.GrantAccess(a.AccountType, a.AccountID, p); err != nil { + for _, p := range g.Permissions { + if err := client.GrantAccess(g.AccountType, g.AccountID, p); err != nil { if !errors.Is(err, ErrPermissionAlreadyExists) { return err } @@ -141,8 +143,8 @@ func (p *Provider) GrantAccess(pc *domain.ProviderConfig, a domain.Grant) error return nil case ResourceTypeServiceAccount: - for _, p := range a.Permissions { - if err := client.GrantServiceAccountAccess(context.TODO(), a.Resource.URN, a.AccountType, a.AccountID, p); err != nil { + for _, p := range g.Permissions { + if err := client.GrantServiceAccountAccess(context.TODO(), g.Resource.URN, g.AccountType, g.AccountID, p); err != nil { if !errors.Is(err, ErrPermissionAlreadyExists) { return err } @@ -155,7 +157,7 @@ func (p *Provider) GrantAccess(pc *domain.ProviderConfig, a domain.Grant) error } } -func (p *Provider) RevokeAccess(pc *domain.ProviderConfig, a domain.Grant) error { +func (p *Provider) RevokeAccess(pc *domain.ProviderConfig, g domain.Grant) error { var creds Credentials if err := mapstructure.Decode(pc.Credentials, &creds); err != nil { return err @@ -166,10 +168,10 @@ func (p *Provider) RevokeAccess(pc *domain.ProviderConfig, a domain.Grant) error return err } - switch a.Resource.Type { + switch g.Resource.Type { case ResourceTypeProject, ResourceTypeOrganization: - for _, p := range a.Permissions { - if err := client.RevokeAccess(a.AccountType, a.AccountID, p); err != nil { + for _, p := range g.Permissions { + if err := client.RevokeAccess(g.AccountType, g.AccountID, p); err != nil { if !errors.Is(err, ErrPermissionNotFound) { return err } @@ -178,8 +180,8 @@ func (p *Provider) RevokeAccess(pc *domain.ProviderConfig, a domain.Grant) error return nil case ResourceTypeServiceAccount: - for _, p := range a.Permissions { - if err := client.RevokeServiceAccountAccess(context.TODO(), a.Resource.URN, a.AccountType, a.AccountID, p); err != nil { + for _, p := range g.Permissions { + if err := client.RevokeServiceAccountAccess(context.TODO(), g.Resource.URN, g.AccountType, g.AccountID, p); err != nil { if !errors.Is(err, ErrPermissionNotFound) { return err } diff --git a/plugins/providers/gcloudiam/provider_test.go b/plugins/providers/gcloudiam/provider_test.go index 77de4b857..d0cef8ee0 100644 --- a/plugins/providers/gcloudiam/provider_test.go +++ b/plugins/providers/gcloudiam/provider_test.go @@ -314,7 +314,7 @@ func TestGetResources(t *testing.T) { providerURN: client, } - gCloudRolesList := []*iam.Role{ + projectRoles := []*iam.Role{ { Name: "roles/bigquery.admin", Title: "BigQuery Admin", @@ -326,9 +326,29 @@ func TestGetResources(t *testing.T) { Description: "Read-only access to ApiGateway and related resources", }, } + saRoles := []*iam.Role{ + { + Name: "roles/workstations.serviceAgent", + Title: "Workstations Service Agent", + Description: "Grants the Workstations Service Account access to manage resources in consumer project.", + }, + } client.EXPECT(). GetGrantableRoles(mock.AnythingOfType("*context.emptyCtx"), gcloudiam.ResourceTypeProject). - Return(gCloudRolesList, nil).Once() + Return(projectRoles, nil).Once() + client.EXPECT(). + GetGrantableRoles(mock.AnythingOfType("*context.emptyCtx"), gcloudiam.ResourceTypeServiceAccount). + Return(saRoles, nil).Once() + + expectedServiceAccounts := []*iam.ServiceAccount{ + { + Name: "sa-name", + DisplayName: "sa-display-name", + }, + } + client.EXPECT(). + ListServiceAccounts(mock.AnythingOfType("*context.emptyCtx")). + Return(expectedServiceAccounts, nil).Once() pc := &domain.ProviderConfig{ Type: domain.ProviderTypeGCloudIAM, @@ -353,6 +373,15 @@ func TestGetResources(t *testing.T) { }, }, }, + { + Type: gcloudiam.ResourceTypeServiceAccount, + Roles: []*domain.Role{ + { + ID: "role-1", + Permissions: []interface{}{"roles/workstations.serviceAgent"}, + }, + }, + }, }, } @@ -364,6 +393,13 @@ func TestGetResources(t *testing.T) { URN: "project/test-resource-name", Name: "project/test-resource-name - GCP IAM", }, + { + ProviderType: pc.Type, + ProviderURN: pc.URN, + Type: gcloudiam.ResourceTypeServiceAccount, + URN: "sa-name", + Name: "sa-display-name", + }, } actualResources, actualError := p.GetResources(pc) @@ -562,7 +598,7 @@ func TestGrantAccess(t *testing.T) { }, URN: providerURN, } - a := domain.Grant{ + g := domain.Grant{ Resource: &domain.Resource{ Type: gcloudiam.ResourceTypeProject, URN: "test-role", @@ -575,10 +611,52 @@ func TestGrantAccess(t *testing.T) { Permissions: []string{"roles/bigquery.admin"}, } - actualError := p.GrantAccess(pc, a) + actualError := p.GrantAccess(pc, g) assert.Nil(t, actualError) }) + + t.Run("successful grant access to a service account", func(t *testing.T) { + providerURN := "test-provider-urn" + crypto := new(mocks.Encryptor) + client := new(mocks.GcloudIamClient) + p := gcloudiam.NewProvider("", crypto) + p.Clients = map[string]gcloudiam.GcloudIamClient{ + providerURN: client, + } + + pc := &domain.ProviderConfig{ + URN: providerURN, + Resources: []*domain.ResourceConfig{ + { + Type: gcloudiam.ResourceTypeServiceAccount, + Roles: []*domain.Role{ + { + ID: "test-role", + Permissions: []interface{}{"test-permission"}, + }, + }, + }, + }, + } + g := domain.Grant{ + Resource: &domain.Resource{ + Type: gcloudiam.ResourceTypeServiceAccount, + URN: "sa-urn", + }, + Role: "test-role", + AccountType: "test-account-type", + AccountID: "test-account-id", + Permissions: []string{"test-permission"}, + } + + client.EXPECT(). + GrantServiceAccountAccess(mock.AnythingOfType("*context.emptyCtx"), g.Resource.URN, g.AccountType, g.AccountID, g.Permissions[0]). + Return(nil).Once() + + err := p.GrantAccess(pc, g) + assert.NoError(t, err) + }) } func TestRevokeAccess(t *testing.T) { @@ -708,6 +786,48 @@ func TestRevokeAccess(t *testing.T) { assert.Nil(t, actualError) }) + + t.Run("successful revoke access to a service account", func(t *testing.T) { + providerURN := "test-provider-urn" + crypto := new(mocks.Encryptor) + client := new(mocks.GcloudIamClient) + p := gcloudiam.NewProvider("", crypto) + p.Clients = map[string]gcloudiam.GcloudIamClient{ + providerURN: client, + } + + pc := &domain.ProviderConfig{ + URN: providerURN, + Resources: []*domain.ResourceConfig{ + { + Type: gcloudiam.ResourceTypeServiceAccount, + Roles: []*domain.Role{ + { + ID: "test-role", + Permissions: []interface{}{"test-permission"}, + }, + }, + }, + }, + } + g := domain.Grant{ + Resource: &domain.Resource{ + Type: gcloudiam.ResourceTypeServiceAccount, + URN: "sa-urn", + }, + Role: "test-role", + AccountType: "test-account-type", + AccountID: "test-account-id", + Permissions: []string{"test-permission"}, + } + + client.EXPECT(). + RevokeServiceAccountAccess(mock.AnythingOfType("*context.emptyCtx"), g.Resource.URN, g.AccountType, g.AccountID, g.Permissions[0]). + Return(nil).Once() + + err := p.RevokeAccess(pc, g) + assert.NoError(t, err) + }) } func TestGetRoles(t *testing.T) { From e6a6ab54d5db6e5d3a7054c77b89d3b54b50645c Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Mon, 14 Aug 2023 16:22:41 +0700 Subject: [PATCH 04/12] refactor: use switch case --- plugins/providers/gcloudiam/client.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/plugins/providers/gcloudiam/client.go b/plugins/providers/gcloudiam/client.go index 9237d4738..c64005a66 100644 --- a/plugins/providers/gcloudiam/client.go +++ b/plugins/providers/gcloudiam/client.go @@ -51,13 +51,16 @@ func (c *iamClient) ListServiceAccounts(ctx context.Context) ([]*iam.ServiceAcco func (c *iamClient) GetGrantableRoles(ctx context.Context, resourceType string) ([]*iam.Role, error) { var fullResourceName string - if resourceType == ResourceTypeOrganization { + switch resourceType { + case ResourceTypeOrganization: orgID := strings.Replace(c.resourceName, ResourceNameOrganizationPrefix, "", 1) fullResourceName = fmt.Sprintf("//cloudresourcemanager.googleapis.com/organizations/%s", orgID) - } else if resourceType == ResourceTypeProject { + + case ResourceTypeProject: projectID := strings.Replace(c.resourceName, ResourceNameProjectPrefix, "", 1) fullResourceName = fmt.Sprintf("//cloudresourcemanager.googleapis.com/projects/%s", projectID) - } else if resourceType == ResourceTypeServiceAccount { + + case ResourceTypeServiceAccount: projectID := strings.Replace(c.resourceName, ResourceNameProjectPrefix, "", 1) res, err := c.iamService.Projects.ServiceAccounts.List(c.resourceName).PageSize(1).Context(ctx).Do() if err != nil { @@ -67,8 +70,9 @@ func (c *iamClient) GetGrantableRoles(ctx context.Context, resourceType string) return nil, fmt.Errorf("no service accounts found in project %s", projectID) } fullResourceName = fmt.Sprintf("//iam.googleapis.com/%s", res.Accounts[0].Name) - } else { - return nil, ErrInvalidResourceName + + default: + return nil, fmt.Errorf("unknown resource type %s", resourceType) } roles := []*iam.Role{} From 8d91692a208b8ee413f5285870e4d739af8a8ffd Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Tue, 4 Jul 2023 08:30:27 +0700 Subject: [PATCH 05/12] chore: user goreleaser v1.18.2 --- .github/workflows/release.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index f8f1f8d76..99da56167 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -27,7 +27,7 @@ jobs: uses: goreleaser/goreleaser-action@v2.6.1 with: distribution: goreleaser - version: latest + version: v1.18.2 args: --rm-dist env: GITHUB_TOKEN: ${{ secrets.GO_RELEASER_TOKEN }} From d66c2886c3be03391e2a2b5bab357fab4092eae1 Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Mon, 14 Aug 2023 17:16:55 +0700 Subject: [PATCH 06/12] chore: use goreleaser v1.8.3 --- .github/workflows/release.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 99da56167..8118097a1 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -27,7 +27,7 @@ jobs: uses: goreleaser/goreleaser-action@v2.6.1 with: distribution: goreleaser - version: v1.18.2 + version: v1.8.3 args: --rm-dist env: GITHUB_TOKEN: ${{ secrets.GO_RELEASER_TOKEN }} From aaf330215c6761be98dcb1836f1502e8b470f25a Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Mon, 14 Aug 2023 17:37:14 +0700 Subject: [PATCH 07/12] chore: fix release pipeline --- .github/workflows/release.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 8118097a1..f8f1f8d76 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -27,7 +27,7 @@ jobs: uses: goreleaser/goreleaser-action@v2.6.1 with: distribution: goreleaser - version: v1.8.3 + version: latest args: --rm-dist env: GITHUB_TOKEN: ${{ secrets.GO_RELEASER_TOKEN }} From b5af010232c2bad82e0b5839706c971a20c483f1 Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Tue, 29 Aug 2023 14:13:17 +0700 Subject: [PATCH 08/12] fix: fix fetching grantable roles next page token --- plugins/providers/gcloudiam/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/gcloudiam/client.go b/plugins/providers/gcloudiam/client.go index c64005a66..fff88dc5c 100644 --- a/plugins/providers/gcloudiam/client.go +++ b/plugins/providers/gcloudiam/client.go @@ -87,7 +87,7 @@ func (c *iamClient) GetGrantableRoles(ctx context.Context, resourceType string) return nil, err } roles = append(roles, res.Roles...) - if res.NextPageToken == "" { + if nextPageToken = res.NextPageToken; nextPageToken == "" { break } } From e197d5383fbfea3d0583ad89c8656cda66b72c3b Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Tue, 29 Aug 2023 14:48:02 +0700 Subject: [PATCH 09/12] refactor: remove additional checking --- plugins/providers/gcloudiam/provider.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/plugins/providers/gcloudiam/provider.go b/plugins/providers/gcloudiam/provider.go index 58daebffa..ad7982c19 100644 --- a/plugins/providers/gcloudiam/provider.go +++ b/plugins/providers/gcloudiam/provider.go @@ -195,10 +195,6 @@ func (p *Provider) RevokeAccess(pc *domain.ProviderConfig, g domain.Grant) error } func (p *Provider) GetRoles(pc *domain.ProviderConfig, resourceType string) ([]*domain.Role, error) { - if resourceType != ResourceTypeProject && resourceType != ResourceTypeOrganization { - return nil, ErrInvalidResourceType - } - return provider.GetRoles(pc, resourceType) } From a7644896af6785f579f6e86b2abcbe8b1a9f7d22 Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Tue, 29 Aug 2023 16:29:18 +0700 Subject: [PATCH 10/12] chore: use email as service account resource name --- plugins/providers/gcloudiam/provider.go | 2 +- plugins/providers/gcloudiam/provider_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/providers/gcloudiam/provider.go b/plugins/providers/gcloudiam/provider.go index ad7982c19..9691686e0 100644 --- a/plugins/providers/gcloudiam/provider.go +++ b/plugins/providers/gcloudiam/provider.go @@ -106,7 +106,7 @@ func (p *Provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, ProviderURN: pc.URN, Type: rc.Type, URN: sa.Name, - Name: sa.DisplayName, + Name: sa.Email, }) } diff --git a/plugins/providers/gcloudiam/provider_test.go b/plugins/providers/gcloudiam/provider_test.go index d0cef8ee0..eed4dc3f0 100644 --- a/plugins/providers/gcloudiam/provider_test.go +++ b/plugins/providers/gcloudiam/provider_test.go @@ -342,8 +342,8 @@ func TestGetResources(t *testing.T) { expectedServiceAccounts := []*iam.ServiceAccount{ { - Name: "sa-name", - DisplayName: "sa-display-name", + Name: "sa-name", + Email: "sa-email", }, } client.EXPECT(). @@ -398,7 +398,7 @@ func TestGetResources(t *testing.T) { ProviderURN: pc.URN, Type: gcloudiam.ResourceTypeServiceAccount, URN: "sa-name", - Name: "sa-display-name", + Name: "sa-email", }, } From 4d2aaf28cefdabe104d64579cee50f65dcf5e9f7 Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Thu, 31 Aug 2023 15:58:50 +0700 Subject: [PATCH 11/12] test: add more unit tests for GetResources --- plugins/providers/gcloudiam/provider_test.go | 83 ++++++++++++++++---- 1 file changed, 67 insertions(+), 16 deletions(-) diff --git a/plugins/providers/gcloudiam/provider_test.go b/plugins/providers/gcloudiam/provider_test.go index eed4dc3f0..1b9b7ae33 100644 --- a/plugins/providers/gcloudiam/provider_test.go +++ b/plugins/providers/gcloudiam/provider_test.go @@ -305,15 +305,15 @@ func TestGetResources(t *testing.T) { assert.Error(t, actualError) }) - t.Run("should check for valid roles in provider config and return project resource object", func(t *testing.T) { - providerURN := "test-provider-urn" - crypto := new(mocks.Encryptor) - client := new(mocks.GcloudIamClient) - p := gcloudiam.NewProvider("", crypto) - p.Clients = map[string]gcloudiam.GcloudIamClient{ - providerURN: client, - } + providerURN := "test-provider-urn" + crypto := new(mocks.Encryptor) + client := new(mocks.GcloudIamClient) + p := gcloudiam.NewProvider("", crypto) + p.Clients = map[string]gcloudiam.GcloudIamClient{ + providerURN: client, + } + t.Run("should check for valid roles in provider config and return project resource object", func(t *testing.T) { projectRoles := []*iam.Role{ { Name: "roles/bigquery.admin", @@ -408,14 +408,6 @@ func TestGetResources(t *testing.T) { }) t.Run("should return organization resource object", func(t *testing.T) { - providerURN := "test-provider-urn" - crypto := new(mocks.Encryptor) - client := new(mocks.GcloudIamClient) - p := gcloudiam.NewProvider("", crypto) - p.Clients = map[string]gcloudiam.GcloudIamClient{ - providerURN: client, - } - gCloudRolesList := []*iam.Role{ { Name: "roles/organisation.admin", @@ -461,6 +453,65 @@ func TestGetResources(t *testing.T) { assert.Equal(t, expectedResources, actualResources) assert.Nil(t, actualError) }) + + t.Run("should return error if resource type in invalid", func(t *testing.T) { + pc := &domain.ProviderConfig{ + Type: domain.ProviderTypeGCloudIAM, + URN: providerURN, + Credentials: map[string]interface{}{ + "resource_name": "project/test-resource-name", + }, + Resources: []*domain.ResourceConfig{ + {Type: "invalid-resource-type"}, + }, + } + _, err := p.GetResources(pc) + + assert.ErrorIs(t, err, gcloudiam.ErrInvalidResourceType) + }) + + t.Run("get service accounts resources", func(t *testing.T) { + t.Run("should return error if client initialization failed", func(t *testing.T) { + pc := &domain.ProviderConfig{ + Type: domain.ProviderTypeGCloudIAM, + URN: providerURN, + Credentials: map[string]interface{}{ + "resource_name": make(chan int), + }, + Resources: []*domain.ResourceConfig{ + { + Type: gcloudiam.ResourceTypeServiceAccount, + }, + }, + } + + _, actualError := p.GetResources(pc) + + assert.Error(t, actualError) + }) + + t.Run("should return error if client returns an error", func(t *testing.T) { + expectedError := errors.New("client error") + client.On("ListServiceAccounts", mock.AnythingOfType("*context.emptyCtx")).Return(nil, expectedError).Once() + + pc := &domain.ProviderConfig{ + Type: domain.ProviderTypeGCloudIAM, + URN: providerURN, + Credentials: map[string]interface{}{ + "resource_name": "project/test-resource-name", + }, + Resources: []*domain.ResourceConfig{ + { + Type: gcloudiam.ResourceTypeServiceAccount, + }, + }, + } + + _, actualError := p.GetResources(pc) + + assert.ErrorIs(t, actualError, expectedError) + }) + }) } func TestGrantAccess(t *testing.T) { From 6fc9fe7582924d51e4e1aaedf36da671bef95078 Mon Sep 17 00:00:00 2001 From: Rahmat Hidayat Date: Thu, 31 Aug 2023 16:16:28 +0700 Subject: [PATCH 12/12] test: add more unit tests for Grant and Revoke Access --- plugins/providers/gcloudiam/provider_test.go | 230 +++++++++++++------ 1 file changed, 158 insertions(+), 72 deletions(-) diff --git a/plugins/providers/gcloudiam/provider_test.go b/plugins/providers/gcloudiam/provider_test.go index 1b9b7ae33..8d0433a0e 100644 --- a/plugins/providers/gcloudiam/provider_test.go +++ b/plugins/providers/gcloudiam/provider_test.go @@ -570,49 +570,92 @@ func TestGrantAccess(t *testing.T) { }) t.Run("should return error if there is an error in granting the access", func(t *testing.T) { - providerURN := "test-provider-urn" - expectedError := errors.New("client error") - crypto := new(mocks.Encryptor) - client := new(mocks.GcloudIamClient) - p := gcloudiam.NewProvider("", crypto) - p.Clients = map[string]gcloudiam.GcloudIamClient{ - providerURN: client, + expectedError := errors.New("client error in granting access") + testCases := []struct { + name string + resourceType string + expectedError error + setExpectationFunc func(*mocks.GcloudIamClient) + }{ + { + name: "for project", + resourceType: gcloudiam.ResourceTypeProject, + expectedError: expectedError, + setExpectationFunc: func(c *mocks.GcloudIamClient) { + c.EXPECT(). + GrantAccess(mock.Anything, mock.Anything, mock.Anything). + Return(expectedError).Once() + }, + }, + { + name: "for organization", + resourceType: gcloudiam.ResourceTypeOrganization, + expectedError: expectedError, + setExpectationFunc: func(c *mocks.GcloudIamClient) { + c.EXPECT(). + GrantAccess(mock.Anything, mock.Anything, mock.Anything). + Return(expectedError).Once() + }, + }, + { + name: "for service account", + resourceType: gcloudiam.ResourceTypeServiceAccount, + expectedError: expectedError, + setExpectationFunc: func(c *mocks.GcloudIamClient) { + c.EXPECT(). + GrantServiceAccountAccess(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(expectedError).Once() + }, + }, } - client.On("GrantAccess", mock.Anything, mock.Anything, mock.Anything).Return(expectedError).Once() - pc := &domain.ProviderConfig{ - Resources: []*domain.ResourceConfig{ - { - Type: gcloudiam.ResourceTypeProject, - Roles: []*domain.Role{ - { - ID: "role-1", - Name: "BigQuery", - Permissions: []interface{}{"roles/bigquery.admin"}, - }, + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + providerURN := "test-provider-urn" + crypto := new(mocks.Encryptor) + client := new(mocks.GcloudIamClient) + p := gcloudiam.NewProvider("", crypto) + p.Clients = map[string]gcloudiam.GcloudIamClient{ + providerURN: client, + } + + tc.setExpectationFunc(client) + + pc := &domain.ProviderConfig{ + Resources: []*domain.ResourceConfig{ { - ID: "role-2", - Name: "Api gateway", - Permissions: []interface{}{"roles/apigateway.viewer"}, + Type: tc.resourceType, + Roles: []*domain.Role{ + { + ID: "role-1", + Name: "role-name-1", + Permissions: []interface{}{"permission-1"}, + }, + { + ID: "role-2", + Name: "role-name-2", + Permissions: []interface{}{"permission-2"}, + }, + }, }, }, - }, - }, - URN: providerURN, - } - a := domain.Grant{ - Resource: &domain.Resource{ - Type: gcloudiam.ResourceTypeProject, - URN: "999", - Name: "test-role", - }, - Role: "role-1", - Permissions: []string{"roles/bigquery.admin"}, - } + URN: providerURN, + } + a := domain.Grant{ + Resource: &domain.Resource{ + Type: tc.resourceType, + URN: "999", + Name: "test-role", + }, + Role: "role-1", + Permissions: []string{"permission-1"}, + } - actualError := p.GrantAccess(pc, a) + actualError := p.GrantAccess(pc, a) - assert.EqualError(t, actualError, expectedError.Error()) + assert.EqualError(t, actualError, tc.expectedError.Error()) + }) + } }) t.Run("should return nil error if granting access is successful", func(t *testing.T) { @@ -742,49 +785,92 @@ func TestRevokeAccess(t *testing.T) { }) t.Run("should return error if there is an error in revoking the access", func(t *testing.T) { - providerURN := "test-provider-urn" - expectedError := errors.New("client error") - crypto := new(mocks.Encryptor) - client := new(mocks.GcloudIamClient) - p := gcloudiam.NewProvider("", crypto) - p.Clients = map[string]gcloudiam.GcloudIamClient{ - providerURN: client, + expectedError := errors.New("client error in revoking access") + testCases := []struct { + name string + resourceType string + expectedError error + setExpectationFunc func(*mocks.GcloudIamClient) + }{ + { + name: "for project", + resourceType: gcloudiam.ResourceTypeProject, + expectedError: expectedError, + setExpectationFunc: func(c *mocks.GcloudIamClient) { + c.EXPECT(). + RevokeAccess(mock.Anything, mock.Anything, mock.Anything). + Return(expectedError).Once() + }, + }, + { + name: "for organization", + resourceType: gcloudiam.ResourceTypeOrganization, + expectedError: expectedError, + setExpectationFunc: func(c *mocks.GcloudIamClient) { + c.EXPECT(). + RevokeAccess(mock.Anything, mock.Anything, mock.Anything). + Return(expectedError).Once() + }, + }, + { + name: "for service account", + resourceType: gcloudiam.ResourceTypeServiceAccount, + expectedError: expectedError, + setExpectationFunc: func(c *mocks.GcloudIamClient) { + c.EXPECT(). + RevokeServiceAccountAccess(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(expectedError).Once() + }, + }, } - client.On("RevokeAccess", mock.Anything, mock.Anything, mock.Anything).Return(expectedError).Once() - pc := &domain.ProviderConfig{ - Resources: []*domain.ResourceConfig{ - { - Type: gcloudiam.ResourceTypeProject, - Roles: []*domain.Role{ - { - ID: "role-1", - Name: "BigQuery", - Permissions: []interface{}{"roles/bigquery.admin"}, - }, + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + providerURN := "test-provider-urn" + crypto := new(mocks.Encryptor) + client := new(mocks.GcloudIamClient) + p := gcloudiam.NewProvider("", crypto) + p.Clients = map[string]gcloudiam.GcloudIamClient{ + providerURN: client, + } + + tc.setExpectationFunc(client) + + pc := &domain.ProviderConfig{ + Resources: []*domain.ResourceConfig{ { - ID: "role-2", - Name: "Api gateway", - Permissions: []interface{}{"roles/apigateway.viewer"}, + Type: tc.resourceType, + Roles: []*domain.Role{ + { + ID: "role-1", + Name: "role-name-1", + Permissions: []interface{}{"permission-1"}, + }, + { + ID: "role-2", + Name: "role-name-2", + Permissions: []interface{}{"permission-2"}, + }, + }, }, }, - }, - }, - URN: providerURN, - } - a := domain.Grant{ - Resource: &domain.Resource{ - Type: gcloudiam.ResourceTypeProject, - URN: "999", - Name: "test-role", - }, - Role: "role-1", - Permissions: []string{"roles/bigquery.admin"}, - } + URN: providerURN, + } + a := domain.Grant{ + Resource: &domain.Resource{ + Type: tc.resourceType, + URN: "999", + Name: "test-role", + }, + Role: "role-1", + Permissions: []string{"permission-1"}, + } - actualError := p.RevokeAccess(pc, a) + actualError := p.RevokeAccess(pc, a) - assert.EqualError(t, actualError, expectedError.Error()) + assert.EqualError(t, actualError, tc.expectedError.Error()) + }) + } }) t.Run("should return nil error if revoking access is successful", func(t *testing.T) {