From 47c388108c787b39a783c57580e724ddfaa9b8be Mon Sep 17 00:00:00 2001 From: Vikash Date: Wed, 20 Apr 2022 13:41:27 +0530 Subject: [PATCH 01/17] feat: adding table, group as metabase resource and add groups-databases to resources --- mocks/MetabaseClient.go | 36 ++++++ plugins/providers/metabase/client.go | 132 +++++++++++++++++++- plugins/providers/metabase/config.go | 2 +- plugins/providers/metabase/errors.go | 2 + plugins/providers/metabase/provider.go | 82 ++++++++++-- plugins/providers/metabase/provider_test.go | 3 + plugins/providers/metabase/resource.go | 112 +++++++++++++++-- 7 files changed, 344 insertions(+), 25 deletions(-) diff --git a/mocks/MetabaseClient.go b/mocks/MetabaseClient.go index b2bc2ed16..cf3419bd1 100644 --- a/mocks/MetabaseClient.go +++ b/mocks/MetabaseClient.go @@ -58,6 +58,42 @@ func (_m *MetabaseClient) GetDatabases() ([]*metabase.Database, error) { return r0, r1 } +func (_m *MetabaseClient) GetGroups() ([]*metabase.Group, map[string][]map[string]interface{}, map[string][]map[string]interface{}, error) { + ret := _m.Called() + + var r0 []*metabase.Group + if rf, ok := ret.Get(0).(func() []*metabase.Group); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*metabase.Group) + } + } + + var r1 map[string][]map[string]interface{} + if rf, ok := ret.Get(1).(func() map[string][]map[string]interface{}); ok { + r1 = rf() + } else { + r1 = ret.Get(1).(map[string][]map[string]interface{}) + } + + var r2 map[string][]map[string]interface{} + if rf, ok := ret.Get(2).(func() map[string][]map[string]interface{}); ok { + r2 = rf() + } else { + r2 = ret.Get(2).(map[string][]map[string]interface{}) + } + + var r3 error + if rf, ok := ret.Get(3).(func() error); ok { + r3 = rf() + } else { + r3 = ret.Error(3) + } + + return r0, r1, r2, r3 +} + // GrantCollectionAccess provides a mock function with given fields: resource, user, role func (_m *MetabaseClient) GrantCollectionAccess(resource *metabase.Collection, user string, role string) error { ret := _m.Called(resource, user, role) diff --git a/plugins/providers/metabase/client.go b/plugins/providers/metabase/client.go index f2fa902eb..0ee0788f1 100644 --- a/plugins/providers/metabase/client.go +++ b/plugins/providers/metabase/client.go @@ -9,6 +9,7 @@ import ( "net/url" "reflect" "strconv" + "sync" "github.com/mitchellh/mapstructure" @@ -18,6 +19,7 @@ import ( type MetabaseClient interface { GetDatabases() ([]*Database, error) GetCollections() ([]*Collection, error) + GetGroups() ([]*Group, map[string][]map[string]interface{}, map[string][]map[string]interface{}, error) GrantDatabaseAccess(resource *Database, user, role string) error RevokeDatabaseAccess(resource *Database, user, role string) error GrantCollectionAccess(resource *Collection, user, role string) error @@ -129,7 +131,7 @@ func NewClient(config *ClientConfig) (*client, error) { } func (c *client) GetDatabases() ([]*Database, error) { - req, err := c.newRequest(http.MethodGet, "/api/database", nil) + req, err := c.newRequest(http.MethodGet, "/api/database?include=tables", nil) if err != nil { return nil, err } @@ -170,6 +172,134 @@ func (c *client) GetCollections() ([]*Collection, error) { return collection, nil } +func (c *client) GetGroups() ([]*Group, map[string][]map[string]interface{}, map[string][]map[string]interface{}, error) { + + wg := sync.WaitGroup{} + wg.Add(3) + + var groups []*Group + var err error + go c.fetchGroups(&wg, &groups, err) + + databaseResourceGroups := make(map[string][]map[string]interface{}, 0) + go c.fetchDatabasePermissions(&wg, databaseResourceGroups, err) + + collectionResourceGroups := make(map[string][]map[string]interface{}, 0) + go c.fetchCollectionPermissions(&wg, collectionResourceGroups, err) + + wg.Wait() + + groupMap := make(map[string]*Group, 0) + for _, group := range groups { + groupMap[fmt.Sprintf("group:%d", group.ID)] = group + } + + addResourceToGroup(databaseResourceGroups, groupMap, "database") + addResourceToGroup(collectionResourceGroups, groupMap, "collection") + + return groups, databaseResourceGroups, collectionResourceGroups, err +} + +func (c *client) fetchGroups(wg *sync.WaitGroup, groups *[]*Group, err error) { + defer wg.Done() + req, err := c.newRequest(http.MethodGet, "/api/permissions/group", nil) + if err != nil { + return + } + + _, err = c.do(req, &groups) + if err != nil { + return + } +} + +func (c *client) fetchDatabasePermissions(wg *sync.WaitGroup, resourceGroups map[string][]map[string]interface{}, err error) { + defer wg.Done() + + req, err := c.newRequest(http.MethodGet, "/api/permissions/graph", nil) + if err != nil { + return + } + + graphs := make(map[string]interface{}, 0) + _, err = c.do(req, &graphs) + if err != nil { + return + } + + for groupId, r := range graphs["groups"].(map[string]interface{}) { + for dbId, role := range r.(map[string]interface{}) { + if roles, ok := role.(map[string]interface{}); ok { + for _, value := range roles { + if tables, ok := value.(map[string]interface{}); ok { + for _, tables := range tables { + if tables, ok := tables.(map[string]interface{}); ok { + for tableId, _ := range tables { + addGroupToResource(resourceGroups, fmt.Sprintf("table:%s.%s", dbId, tableId), groupId, err) + } + } + } + } + } + } + addGroupToResource(resourceGroups, fmt.Sprintf("database:%s", dbId), groupId, err) + } + } +} + +func (c *client) fetchCollectionPermissions(wg *sync.WaitGroup, resourceGroups map[string][]map[string]interface{}, err error) { + defer wg.Done() + + req, err := c.newRequest(http.MethodGet, "/api/collection/graph", nil) + if err != nil { + return + } + + graphs := make(map[string]interface{}, 0) + _, err = c.do(req, &graphs) + if err != nil { + return + } + + for groupId, r := range graphs["groups"].(map[string]interface{}) { + for collectionId, role := range r.(map[string]interface{}) { + if role != "none" { + addGroupToResource(resourceGroups, collectionId, groupId, err) + } + } + } +} + +func addResourceToGroup(resourceGroups map[string][]map[string]interface{}, groupMap map[string]*Group, resourceType string) { + for resourceId, groups := range resourceGroups { + for _, groupDetails := range groups { + groupID := groupDetails["urn"].(string) + if group, ok := groupMap[groupID]; ok { + groupDetails["name"] = group.Name + if resourceType == "database" { + group.DatabaseResources = append(group.DatabaseResources, map[string]interface{}{"urn": resourceId}) + } + if resourceType == "collection" { + group.CollectionResources = append(group.CollectionResources, map[string]interface{}{"urn": resourceId}) + } + } + } + } +} + +func addGroupToResource(resourceGroups map[string][]map[string]interface{}, resourceId string, groupId string, err error) { + id, err := strconv.Atoi(groupId) + if err != nil { + return + } + if groups, ok := resourceGroups[resourceId]; ok { + groups = append(groups, map[string]interface{}{"urn": fmt.Sprintf("group:%d", id)}) + resourceGroups[resourceId] = groups + } else { + resourceGroups[resourceId] = []map[string]interface{}{{"urn": fmt.Sprintf("group:%d", id)}} + } +} + func (c *client) GrantDatabaseAccess(resource *Database, user, role string) error { access, err := c.getDatabaseAccess() if err != nil { diff --git a/plugins/providers/metabase/config.go b/plugins/providers/metabase/config.go index 9c221fe7e..fdeeec89e 100644 --- a/plugins/providers/metabase/config.go +++ b/plugins/providers/metabase/config.go @@ -136,7 +136,7 @@ func (c *Config) validateCredentials(value interface{}) (*Credentials, error) { } func (c *Config) validateResourceConfig(resource *domain.ResourceConfig) error { - resourceTypeValidation := fmt.Sprintf("oneof=%s %s", ResourceTypeCollection, ResourceTypeDatabase) + resourceTypeValidation := fmt.Sprintf("oneof=%s %s %s %s", ResourceTypeCollection, ResourceTypeDatabase, ResourceTypeTable, ResourceTypeGroup) if err := c.validator.Var(resource.Type, resourceTypeValidation); err != nil { return err } diff --git a/plugins/providers/metabase/errors.go b/plugins/providers/metabase/errors.go index dc18b8110..3bfd7a5ba 100644 --- a/plugins/providers/metabase/errors.go +++ b/plugins/providers/metabase/errors.go @@ -12,4 +12,6 @@ var ( ErrInvalidResourceType = errors.New("invalid resource type") ErrPermissionNotFound = errors.New("permission not found") ErrInvalidApiResponse = errors.New("invalid api response") + ErrInvalidDatabaseURN = errors.New("database URN is invalid") + ErrInvalidTableURN = errors.New("table URN is invalid") ) diff --git a/plugins/providers/metabase/provider.go b/plugins/providers/metabase/provider.go index 1f268299a..f975c56db 100644 --- a/plugins/providers/metabase/provider.go +++ b/plugins/providers/metabase/provider.go @@ -45,14 +45,14 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, return nil, err } - var resourceTypes []string + var resourceTypes = make(map[string]bool, 0) for _, rc := range pc.Resources { - resourceTypes = append(resourceTypes, rc.Type) + resourceTypes[rc.Type] = true } resources := []*domain.Resource{} - if containsString(resourceTypes, ResourceTypeDatabase) { + if _, ok := resourceTypes[ResourceTypeDatabase]; ok && resourceTypes[ResourceTypeDatabase] { databases, err := client.GetDatabases() if err != nil { return nil, err @@ -62,10 +62,21 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, db.ProviderType = pc.Type db.ProviderURN = pc.URN resources = append(resources, db) + + if _, ok := resourceTypes[ResourceTypeTable]; ok && resourceTypes[ResourceTypeTable] { + tables := d.Tables + for _, t := range tables { + t.Database = db + table := t.ToDomain() + table.ProviderType = pc.Type + table.ProviderURN = pc.URN + resources = append(resources, table) + } + } } } - if containsString(resourceTypes, ResourceTypeCollection) { + if _, ok := resourceTypes[ResourceTypeCollection]; ok && resourceTypes[ResourceTypeCollection] { collections, err := client.GetCollections() if err != nil { return nil, err @@ -78,6 +89,60 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, } } + groups, databaseResourceGroups, collectionResourceGroups, err := client.GetGroups() + if err != nil { + return nil, err + } + + databaseResourceMap := make(map[string]*domain.Resource, 0) + collectionResourceMap := make(map[string]*domain.Resource, 0) + for _, resource := range resources { + if resource.Type == ResourceTypeDatabase || resource.Type == ResourceTypeTable { + databaseResourceMap[resource.URN] = resource + } + if resource.Type == ResourceTypeCollection { + collectionResourceMap[resource.URN] = resource + } + } + + for _, resource := range resources { + if resource.Type == ResourceTypeDatabase || resource.Type == ResourceTypeTable { + if groups, ok := databaseResourceGroups[resource.URN]; ok { + resource.Details["groups"] = groups + } + } + if resource.Type == ResourceTypeCollection { + if groups, ok := collectionResourceGroups[resource.URN]; ok { + resource.Details["groups"] = groups + } + } + } + + if _, ok := resourceTypes[ResourceTypeGroup]; ok && resourceTypes[ResourceTypeGroup] { + for _, g := range groups { + for _, resourceMap := range g.DatabaseResources { + resourceId := resourceMap["urn"].(string) + if resource, ok := databaseResourceMap[resourceId]; ok { + resourceMap["name"] = resource.Name + resourceMap["type"] = resource.Type + } + } + + for _, resourceMap := range g.CollectionResources { + resourceId := resourceMap["urn"].(string) + if resource, ok := collectionResourceMap[resourceId]; ok { + resourceMap["name"] = resource.Name + resourceMap["type"] = resource.Type + } + } + + db := g.ToDomain() + db.ProviderType = pc.Type + db.ProviderURN = pc.URN + resources = append(resources, db) + } + } + return resources, nil } @@ -239,12 +304,3 @@ func getPermissions(resourceConfigs []*domain.ResourceConfig, a *domain.Appeal) return permissions, nil } - -func containsString(arr []string, v string) bool { - for _, item := range arr { - if item == v { - return true - } - } - return false -} diff --git a/plugins/providers/metabase/provider_test.go b/plugins/providers/metabase/provider_test.go index 84941ecd6..1398d8146 100644 --- a/plugins/providers/metabase/provider_test.go +++ b/plugins/providers/metabase/provider_test.go @@ -139,6 +139,9 @@ func TestGetResources(t *testing.T) { }, } client.On("GetDatabases").Return(expectedDatabases, nil).Once() + groups := make([]*metabase.Group, 0) + maps := make(map[string][]map[string]interface{}, 0) + client.On("GetGroups").Return(groups, maps, maps, nil).Once() expectedCollections := []*metabase.Collection{ { ID: 1, diff --git a/plugins/providers/metabase/resource.go b/plugins/providers/metabase/resource.go index 4b6e2d81e..4a33d2568 100644 --- a/plugins/providers/metabase/resource.go +++ b/plugins/providers/metabase/resource.go @@ -3,24 +3,42 @@ package metabase import ( "fmt" "strconv" + "strings" "github.com/odpf/guardian/domain" ) const ( ResourceTypeDatabase = "database" + ResourceTypeTable = "table" ResourceTypeCollection = "collection" + ResourceTypeGroup = "group" ) type Database struct { - ID int `json:"id"` - Name string `json:"name"` - CacheFieldValuesSchedule string `json:"cache_field_values_schedule"` - Timezone string `json:"timezone"` - AutoRunQueries bool `json:"auto_run_queries"` - MetadataSyncSchedule string `json:"metadata_sync_schedule"` - Engine string `json:"engine"` - NativePermissions string `json:"native_permissions"` + ID int `json:"id"` + Name string `json:"name"` + CacheFieldValuesSchedule string `json:"cache_field_values_schedule"` + Timezone string `json:"timezone"` + AutoRunQueries bool `json:"auto_run_queries"` + MetadataSyncSchedule string `json:"metadata_sync_schedule"` + Engine string `json:"engine"` + NativePermissions string `json:"native_permissions"` + Tables []Table `json:"tables"` +} + +type Table struct { + ID int `mapstructure:"id"` + Name string `json:"name"` + DbId int `mapstructure:"db_id"` + Database *domain.Resource +} + +type Group struct { + ID int `json:"id"` + Name string `json:"name"` + DatabaseResources []map[string]interface{} `json:"database_resources"` + CollectionResources []map[string]interface{} `json:"collection_resources"` } func (d *Database) FromDomain(r *domain.Resource) error { @@ -28,7 +46,11 @@ func (d *Database) FromDomain(r *domain.Resource) error { return ErrInvalidResourceType } - id, err := strconv.Atoi(r.URN) + databaseURN := strings.Split(r.URN, ":") + if len(databaseURN) != 2 { + return ErrInvalidDatabaseURN + } + id, err := strconv.Atoi(databaseURN[1]) if err != nil { return err } @@ -42,7 +64,7 @@ func (d *Database) ToDomain() *domain.Resource { return &domain.Resource{ Type: ResourceTypeDatabase, Name: d.Name, - URN: fmt.Sprintf("%v", d.ID), + URN: fmt.Sprintf("database:%v", d.ID), Details: map[string]interface{}{ "cache_field_values_schedule": d.CacheFieldValuesSchedule, "timezone": d.Timezone, @@ -54,6 +76,76 @@ func (d *Database) ToDomain() *domain.Resource { } } +func (t *Table) FromDomain(r *domain.Resource) error { + if r.Type != ResourceTypeTable { + return ErrInvalidResourceType + } + + tableURN := strings.Split(r.URN, ":") + if len(tableURN) != 2 { + return ErrInvalidTableURN + } + + tableURN = strings.Split(tableURN[1], ".") + id, err := strconv.Atoi(tableURN[1]) + if err != nil { + return err + } + + t.ID = id + t.Name = r.Name + t.DbId, err = strconv.Atoi(tableURN[1]) + if err != nil { + return err + } + return nil +} + +func (t *Table) ToDomain() *domain.Resource { + return &domain.Resource{ + Type: ResourceTypeTable, + Name: t.Name, + URN: fmt.Sprintf("table:%d.%d", t.DbId, t.ID), + Details: t.Database.Details, + } +} + +func (g *Group) FromDomain(r *domain.Resource) error { + if r.Type != ResourceTypeGroup { + return ErrInvalidResourceType + } + + groupUrn := strings.Split(r.URN, ":") + if len(groupUrn) != 2 { + return ErrInvalidDatabaseURN + } + id, err := strconv.Atoi(groupUrn[1]) + if err != nil { + return err + } + + g.ID = id + g.Name = r.Name + g.DatabaseResources = r.Details["database"].([]map[string]interface{}) + g.CollectionResources = r.Details["collection"].([]map[string]interface{}) + if err != nil { + return err + } + return nil +} + +func (g *Group) ToDomain() *domain.Resource { + return &domain.Resource{ + Type: ResourceTypeGroup, + Name: g.Name, + URN: fmt.Sprintf("group:%d", g.ID), + Details: map[string]interface{}{ + "database": g.DatabaseResources, + "collection": g.CollectionResources, + }, + } +} + type Collection struct { ID interface{} `json:"id"` Name string `json:"name"` From 61580ff8fbda8b9e42805db4ba6dd495e104e744 Mon Sep 17 00:00:00 2001 From: Vikash Date: Wed, 20 Apr 2022 21:25:06 +0530 Subject: [PATCH 02/17] fix: fix test case --- plugins/providers/metabase/provider_test.go | 56 +++++++++++++++++---- plugins/providers/metabase/resource_test.go | 4 +- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/plugins/providers/metabase/provider_test.go b/plugins/providers/metabase/provider_test.go index 1398d8146..f163316c3 100644 --- a/plugins/providers/metabase/provider_test.go +++ b/plugins/providers/metabase/provider_test.go @@ -126,22 +126,33 @@ func TestGetResources(t *testing.T) { Resources: []*domain.ResourceConfig{ { Type: metabase.ResourceTypeDatabase, + }, { + Type: metabase.ResourceTypeTable, }, { Type: metabase.ResourceTypeCollection, + }, { + Type: metabase.ResourceTypeGroup, }, }, } expectedDatabases := []*metabase.Database{ { - ID: 1, - Name: "db_1", + ID: 1, + Name: "db_1", + Tables: []metabase.Table{{ID: 2, Name: "table_1", DbId: 1}}, }, } client.On("GetDatabases").Return(expectedDatabases, nil).Once() - groups := make([]*metabase.Group, 0) - maps := make(map[string][]map[string]interface{}, 0) - client.On("GetGroups").Return(groups, maps, maps, nil).Once() + + d := []map[string]interface{}{{"urn": "database:1"}} + c := []map[string]interface{}{{"urn": "1"}} + group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} + + client.On("GetGroups").Return([]*metabase.Group{&group}, + map[string][]map[string]interface{}{"database:1": {{"urn": "group:1"}}}, + map[string][]map[string]interface{}{"1": {{"urn": "group:1"}}}, nil).Once() + expectedCollections := []*metabase.Collection{ { ID: 1, @@ -152,7 +163,7 @@ func TestGetResources(t *testing.T) { expectedResources := []*domain.Resource{ { Type: metabase.ResourceTypeDatabase, - URN: "1", + URN: "database:1", ProviderURN: providerURN, Name: "db_1", Details: map[string]interface{}{ @@ -162,6 +173,21 @@ func TestGetResources(t *testing.T) { "metadata_sync_schedule": "", "native_permissions": "", "timezone": "", + "groups": []map[string]interface{}{{"urn": "group:1"}}, + }, + }, { + Type: metabase.ResourceTypeTable, + URN: "table:1.2", + ProviderURN: providerURN, + Name: "table_1", + Details: map[string]interface{}{ + "auto_run_queries": false, + "cache_field_values_schedule": "", + "engine": "", + "groups": []map[string]interface{}{{"urn": "group:1"}}, + "metadata_sync_schedule": "", + "native_permissions": "", + "timezone": "", }, }, { @@ -169,7 +195,19 @@ func TestGetResources(t *testing.T) { URN: "1", ProviderURN: providerURN, Name: "col_1", - Details: map[string]interface{}{}, + Details: map[string]interface{}{ + "groups": []map[string]interface{}{{"urn": "group:1"}}, + }, + }, + { + Type: metabase.ResourceTypeGroup, + URN: "group:0", + ProviderURN: providerURN, + Name: "All Users", + Details: map[string]interface{}{ + "collection": []map[string]interface{}{{"name": "col_1", "type": "collection", "urn": "1"}}, + "database": []map[string]interface{}{{"name": "db_1", "type": "database", "urn": "database:1"}}, + }, }, } @@ -391,7 +429,7 @@ func TestGrantAccess(t *testing.T) { a := &domain.Appeal{ Resource: &domain.Resource{ Type: metabase.ResourceTypeDatabase, - URN: "999", + URN: "database:999", Name: "test-database", }, Role: "test-role", @@ -440,7 +478,7 @@ func TestGrantAccess(t *testing.T) { a := &domain.Appeal{ Resource: &domain.Resource{ Type: metabase.ResourceTypeDatabase, - URN: "999", + URN: "database:999", Name: "test-database", }, Role: "viewer", diff --git a/plugins/providers/metabase/resource_test.go b/plugins/providers/metabase/resource_test.go index c3fc23b57..48670de46 100644 --- a/plugins/providers/metabase/resource_test.go +++ b/plugins/providers/metabase/resource_test.go @@ -23,7 +23,7 @@ func TestDatabase(t *testing.T) { expectedResource: &domain.Resource{ Type: metabase.ResourceTypeDatabase, Name: "database 1", - URN: "1", + URN: "database:1", }, }, } @@ -69,7 +69,7 @@ func TestDatabase(t *testing.T) { } r := &domain.Resource{ - URN: "1", + URN: "database:1", Type: metabase.ResourceTypeDatabase, Name: "test-resource", } From bf17893d7ab5a1f40bd8d62e6d428319cd842cd6 Mon Sep 17 00:00:00 2001 From: Vikash Date: Thu, 21 Apr 2022 19:42:35 +0530 Subject: [PATCH 03/17] Fix test case Add permission to group Add logs --- app/server.go | 2 +- mocks/MetabaseClient.go | 2 +- plugins/providers/metabase/client.go | 58 ++++++++++------- plugins/providers/metabase/client_test.go | 14 +++-- plugins/providers/metabase/errors.go | 2 + plugins/providers/metabase/provider.go | 7 ++- plugins/providers/metabase/provider_test.go | 70 +++++++++++++-------- plugins/providers/metabase/resource.go | 14 ++++- plugins/providers/metabase/resource_test.go | 10 +-- 9 files changed, 111 insertions(+), 68 deletions(-) diff --git a/app/server.go b/app/server.go index 3fca2a6fd..c53515251 100644 --- a/app/server.go +++ b/app/server.go @@ -73,7 +73,7 @@ func RunServer(c *Config) error { providerClients := []providers.Client{ bigquery.NewProvider(domain.ProviderTypeBigQuery, crypto), - metabase.NewProvider(domain.ProviderTypeMetabase, crypto), + metabase.NewProvider(domain.ProviderTypeMetabase, crypto, logger), grafana.NewProvider(domain.ProviderTypeGrafana, crypto), tableau.NewProvider(domain.ProviderTypeTableau, crypto), gcloudiam.NewProvider(domain.ProviderTypeGCloudIAM, crypto), diff --git a/mocks/MetabaseClient.go b/mocks/MetabaseClient.go index cf3419bd1..c75007b86 100644 --- a/mocks/MetabaseClient.go +++ b/mocks/MetabaseClient.go @@ -58,7 +58,7 @@ func (_m *MetabaseClient) GetDatabases() ([]*metabase.Database, error) { return r0, r1 } -func (_m *MetabaseClient) GetGroups() ([]*metabase.Group, map[string][]map[string]interface{}, map[string][]map[string]interface{}, error) { +func (_m *MetabaseClient) GetGroups() ([]*metabase.Group, metabase.ResourceGroupDetails, metabase.ResourceGroupDetails, error) { ret := _m.Called() var r0 []*metabase.Group diff --git a/plugins/providers/metabase/client.go b/plugins/providers/metabase/client.go index 0ee0788f1..a3c7e6c1e 100644 --- a/plugins/providers/metabase/client.go +++ b/plugins/providers/metabase/client.go @@ -11,15 +11,19 @@ import ( "strconv" "sync" + "github.com/odpf/salt/log" + "github.com/mitchellh/mapstructure" "github.com/go-playground/validator/v10" ) +type ResourceGroupDetails map[string][]map[string]interface{} + type MetabaseClient interface { GetDatabases() ([]*Database, error) GetCollections() ([]*Collection, error) - GetGroups() ([]*Group, map[string][]map[string]interface{}, map[string][]map[string]interface{}, error) + GetGroups() ([]*Group, ResourceGroupDetails, ResourceGroupDetails, error) GrantDatabaseAccess(resource *Database, user, role string) error RevokeDatabaseAccess(resource *Database, user, role string) error GrantCollectionAccess(resource *Collection, user, role string) error @@ -96,9 +100,11 @@ type client struct { httpClient HTTPClient userIDs map[string]int + + logger *log.Logrus } -func NewClient(config *ClientConfig) (*client, error) { +func NewClient(config *ClientConfig, logger *log.Logrus) (*client, error) { if err := validator.New().Struct(config); err != nil { return nil, err } @@ -119,6 +125,7 @@ func NewClient(config *ClientConfig) (*client, error) { password: config.Password, httpClient: httpClient, userIDs: map[string]int{}, + logger: logger, } sessionToken, err := c.getSessionToken() @@ -155,6 +162,7 @@ func (c *client) GetDatabases() ([]*Database, error) { if err != nil { return databases, err } + c.logger.Info(fmt.Sprintf("Fetch total %d database from request: %v", len(databases), req.URL)) return databases, err } @@ -168,12 +176,11 @@ func (c *client) GetCollections() ([]*Collection, error) { if _, err := c.do(req, &collection); err != nil { return nil, err } - + c.logger.Info(fmt.Sprintf("Fetch total %d collections from request: %v", len(collection), req.URL)) return collection, nil } -func (c *client) GetGroups() ([]*Group, map[string][]map[string]interface{}, map[string][]map[string]interface{}, error) { - +func (c *client) GetGroups() ([]*Group, ResourceGroupDetails, ResourceGroupDetails, error) { wg := sync.WaitGroup{} wg.Add(3) @@ -181,10 +188,10 @@ func (c *client) GetGroups() ([]*Group, map[string][]map[string]interface{}, map var err error go c.fetchGroups(&wg, &groups, err) - databaseResourceGroups := make(map[string][]map[string]interface{}, 0) + databaseResourceGroups := make(ResourceGroupDetails, 0) go c.fetchDatabasePermissions(&wg, databaseResourceGroups, err) - collectionResourceGroups := make(map[string][]map[string]interface{}, 0) + collectionResourceGroups := make(ResourceGroupDetails, 0) go c.fetchCollectionPermissions(&wg, collectionResourceGroups, err) wg.Wait() @@ -211,9 +218,10 @@ func (c *client) fetchGroups(wg *sync.WaitGroup, groups *[]*Group, err error) { if err != nil { return } + c.logger.Info(fmt.Sprintf("Fetch total %d groups from request: %v", len(*groups), req.URL)) } -func (c *client) fetchDatabasePermissions(wg *sync.WaitGroup, resourceGroups map[string][]map[string]interface{}, err error) { +func (c *client) fetchDatabasePermissions(wg *sync.WaitGroup, resourceGroups ResourceGroupDetails, err error) { defer wg.Done() req, err := c.newRequest(http.MethodGet, "/api/permissions/graph", nil) @@ -230,24 +238,27 @@ func (c *client) fetchDatabasePermissions(wg *sync.WaitGroup, resourceGroups map for groupId, r := range graphs["groups"].(map[string]interface{}) { for dbId, role := range r.(map[string]interface{}) { if roles, ok := role.(map[string]interface{}); ok { - for _, value := range roles { + permissions := make([]string, 0) + for key, value := range roles { if tables, ok := value.(map[string]interface{}); ok { for _, tables := range tables { if tables, ok := tables.(map[string]interface{}); ok { - for tableId, _ := range tables { - addGroupToResource(resourceGroups, fmt.Sprintf("table:%s.%s", dbId, tableId), groupId, err) + for tableId, tablePermission := range tables { + addGroupToResource(resourceGroups, fmt.Sprintf("table:%s.%s", dbId, tableId), groupId, []string{tablePermission.(string)}, err) } } } + } else { + permissions = append(permissions, fmt.Sprintf("%s:%s", key, value)) } } + addGroupToResource(resourceGroups, fmt.Sprintf("database:%s", dbId), groupId, permissions, err) } - addGroupToResource(resourceGroups, fmt.Sprintf("database:%s", dbId), groupId, err) } } } -func (c *client) fetchCollectionPermissions(wg *sync.WaitGroup, resourceGroups map[string][]map[string]interface{}, err error) { +func (c *client) fetchCollectionPermissions(wg *sync.WaitGroup, resourceGroups ResourceGroupDetails, err error) { defer wg.Done() req, err := c.newRequest(http.MethodGet, "/api/collection/graph", nil) @@ -260,43 +271,43 @@ func (c *client) fetchCollectionPermissions(wg *sync.WaitGroup, resourceGroups m if err != nil { return } - + c.logger.Info(fmt.Sprintf("Fetch permissions for collections from request: %v", req.URL)) for groupId, r := range graphs["groups"].(map[string]interface{}) { - for collectionId, role := range r.(map[string]interface{}) { - if role != "none" { - addGroupToResource(resourceGroups, collectionId, groupId, err) + for collectionId, permission := range r.(map[string]interface{}) { + if permission != "none" { + addGroupToResource(resourceGroups, fmt.Sprintf("collection:%s", collectionId), groupId, []string{permission.(string)}, err) } } } } -func addResourceToGroup(resourceGroups map[string][]map[string]interface{}, groupMap map[string]*Group, resourceType string) { +func addResourceToGroup(resourceGroups ResourceGroupDetails, groupMap map[string]*Group, resourceType string) { for resourceId, groups := range resourceGroups { for _, groupDetails := range groups { groupID := groupDetails["urn"].(string) if group, ok := groupMap[groupID]; ok { groupDetails["name"] = group.Name if resourceType == "database" { - group.DatabaseResources = append(group.DatabaseResources, map[string]interface{}{"urn": resourceId}) + group.DatabaseResources = append(group.DatabaseResources, map[string]interface{}{"urn": resourceId, "permissions": groupDetails["permissions"]}) } if resourceType == "collection" { - group.CollectionResources = append(group.CollectionResources, map[string]interface{}{"urn": resourceId}) + group.CollectionResources = append(group.CollectionResources, map[string]interface{}{"urn": resourceId, "permissions": groupDetails["permissions"]}) } } } } } -func addGroupToResource(resourceGroups map[string][]map[string]interface{}, resourceId string, groupId string, err error) { +func addGroupToResource(resourceGroups ResourceGroupDetails, resourceId string, groupId string, permissions []string, err error) { id, err := strconv.Atoi(groupId) if err != nil { return } if groups, ok := resourceGroups[resourceId]; ok { - groups = append(groups, map[string]interface{}{"urn": fmt.Sprintf("group:%d", id)}) + groups = append(groups, map[string]interface{}{"urn": fmt.Sprintf("group:%d", id), "permissions": permissions}) resourceGroups[resourceId] = groups } else { - resourceGroups[resourceId] = []map[string]interface{}{{"urn": fmt.Sprintf("group:%d", id)}} + resourceGroups[resourceId] = []map[string]interface{}{{"urn": fmt.Sprintf("group:%d", id), "permissions": permissions}} } } @@ -681,6 +692,7 @@ func (c *client) newRequest(method, path string, body interface{}) (*http.Reques func (c *client) do(req *http.Request, v interface{}) (*http.Response, error) { resp, err := c.httpClient.Do(req) if err != nil { + c.logger.Error(fmt.Sprintf("Failed to execute request %v with error %v", req.URL, err)) return nil, err } defer resp.Body.Close() diff --git a/plugins/providers/metabase/client_test.go b/plugins/providers/metabase/client_test.go index 0a87c2c5b..9358d0024 100644 --- a/plugins/providers/metabase/client_test.go +++ b/plugins/providers/metabase/client_test.go @@ -4,6 +4,8 @@ import ( "errors" "testing" + "github.com/odpf/salt/log" + "github.com/odpf/guardian/mocks" "github.com/odpf/guardian/plugins/providers/metabase" "github.com/stretchr/testify/assert" @@ -13,8 +15,8 @@ import ( func TestNewClient(t *testing.T) { t.Run("should return error if config is invalid", func(t *testing.T) { invalidConfig := &metabase.ClientConfig{} - - actualClient, actualError := metabase.NewClient(invalidConfig) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + actualClient, actualError := metabase.NewClient(invalidConfig, logger) assert.Nil(t, actualClient) assert.Error(t, actualError) @@ -26,8 +28,8 @@ func TestNewClient(t *testing.T) { Password: "test-password", Host: "invalid-url", } - - actualClient, actualError := metabase.NewClient(invalidHostConfig) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + actualClient, actualError := metabase.NewClient(invalidHostConfig, logger) assert.Nil(t, actualClient) assert.Error(t, actualError) @@ -41,11 +43,11 @@ func TestNewClient(t *testing.T) { Host: "http://localhost", HTTPClient: mockHttpClient, } - + logger := log.NewLogrus(log.LogrusWithLevel("info")) expectedError := errors.New("request error") mockHttpClient.On("Do", mock.Anything).Return(nil, expectedError).Once() - actualClient, actualError := metabase.NewClient(config) + actualClient, actualError := metabase.NewClient(config, logger) mockHttpClient.AssertExpectations(t) assert.Nil(t, actualClient) diff --git a/plugins/providers/metabase/errors.go b/plugins/providers/metabase/errors.go index 3bfd7a5ba..e0964d571 100644 --- a/plugins/providers/metabase/errors.go +++ b/plugins/providers/metabase/errors.go @@ -14,4 +14,6 @@ var ( ErrInvalidApiResponse = errors.New("invalid api response") ErrInvalidDatabaseURN = errors.New("database URN is invalid") ErrInvalidTableURN = errors.New("table URN is invalid") + ErrInvalidGroupURN = errors.New("group URN is invalid") + ErrInvalidCollectionURN = errors.New("collection URN is invalid") ) diff --git a/plugins/providers/metabase/provider.go b/plugins/providers/metabase/provider.go index f975c56db..719130952 100644 --- a/plugins/providers/metabase/provider.go +++ b/plugins/providers/metabase/provider.go @@ -4,19 +4,22 @@ import ( "github.com/mitchellh/mapstructure" pv "github.com/odpf/guardian/core/provider" "github.com/odpf/guardian/domain" + "github.com/odpf/salt/log" ) type provider struct { typeName string Clients map[string]MetabaseClient crypto domain.Crypto + logger *log.Logrus } -func NewProvider(typeName string, crypto domain.Crypto) *provider { +func NewProvider(typeName string, crypto domain.Crypto, logger *log.Logrus) *provider { return &provider{ typeName: typeName, Clients: map[string]MetabaseClient{}, crypto: crypto, + logger: logger, } } @@ -262,7 +265,7 @@ func (p *provider) getClient(providerURN string, credentials Credentials) (Metab Host: credentials.Host, Username: credentials.Username, Password: credentials.Password, - }) + }, p.logger) if err != nil { return nil, err } diff --git a/plugins/providers/metabase/provider_test.go b/plugins/providers/metabase/provider_test.go index f163316c3..3c902ae93 100644 --- a/plugins/providers/metabase/provider_test.go +++ b/plugins/providers/metabase/provider_test.go @@ -4,6 +4,8 @@ import ( "errors" "testing" + "github.com/odpf/salt/log" + "github.com/mitchellh/mapstructure" "github.com/odpf/guardian/domain" "github.com/odpf/guardian/mocks" @@ -15,8 +17,9 @@ import ( func TestGetType(t *testing.T) { t.Run("should return provider type name", func(t *testing.T) { expectedTypeName := domain.ProviderTypeMetabase + logger := log.NewLogrus(log.LogrusWithLevel("info")) crypto := new(mocks.Crypto) - p := metabase.NewProvider(expectedTypeName, crypto) + p := metabase.NewProvider(expectedTypeName, crypto, logger) actualTypeName := p.GetType() @@ -27,7 +30,8 @@ func TestGetType(t *testing.T) { func TestGetResources(t *testing.T) { t.Run("should return error if credentials is invalid", func(t *testing.T) { crypto := new(mocks.Crypto) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) pc := &domain.ProviderConfig{ Credentials: "invalid-creds", @@ -41,7 +45,8 @@ func TestGetResources(t *testing.T) { t.Run("should return error if there are any on client initialization", func(t *testing.T) { crypto := new(mocks.Crypto) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) expectedError := errors.New("decrypt error") crypto.On("Decrypt", "test-password").Return("", expectedError).Once() @@ -61,7 +66,8 @@ func TestGetResources(t *testing.T) { providerURN := "test-provider-urn" crypto := new(mocks.Crypto) client := new(mocks.MetabaseClient) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, } @@ -88,7 +94,8 @@ func TestGetResources(t *testing.T) { providerURN := "test-provider-urn" crypto := new(mocks.Crypto) client := new(mocks.MetabaseClient) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, } @@ -115,7 +122,8 @@ func TestGetResources(t *testing.T) { providerURN := "test-provider-urn" crypto := new(mocks.Crypto) client := new(mocks.MetabaseClient) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, } @@ -145,13 +153,13 @@ func TestGetResources(t *testing.T) { } client.On("GetDatabases").Return(expectedDatabases, nil).Once() - d := []map[string]interface{}{{"urn": "database:1"}} - c := []map[string]interface{}{{"urn": "1"}} + d := []map[string]interface{}{{"urn": "database:1", "permissions": []string{"read", "write"}}} + c := []map[string]interface{}{{"urn": "collection:1", "permissions": []string{"read", "write"}}} group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} client.On("GetGroups").Return([]*metabase.Group{&group}, - map[string][]map[string]interface{}{"database:1": {{"urn": "group:1"}}}, - map[string][]map[string]interface{}{"1": {{"urn": "group:1"}}}, nil).Once() + map[string][]map[string]interface{}{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, + map[string][]map[string]interface{}{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() expectedCollections := []*metabase.Collection{ { @@ -173,7 +181,7 @@ func TestGetResources(t *testing.T) { "metadata_sync_schedule": "", "native_permissions": "", "timezone": "", - "groups": []map[string]interface{}{{"urn": "group:1"}}, + "groups": []map[string]interface{}{{"urn": "group:1", "permissions": []string{"read", "write"}}}, }, }, { Type: metabase.ResourceTypeTable, @@ -184,7 +192,7 @@ func TestGetResources(t *testing.T) { "auto_run_queries": false, "cache_field_values_schedule": "", "engine": "", - "groups": []map[string]interface{}{{"urn": "group:1"}}, + "groups": []map[string]interface{}{{"urn": "group:1", "permissions": []string{"read", "write"}}}, "metadata_sync_schedule": "", "native_permissions": "", "timezone": "", @@ -192,11 +200,11 @@ func TestGetResources(t *testing.T) { }, { Type: metabase.ResourceTypeCollection, - URN: "1", + URN: "collection:1", ProviderURN: providerURN, Name: "col_1", Details: map[string]interface{}{ - "groups": []map[string]interface{}{{"urn": "group:1"}}, + "groups": []map[string]interface{}{{"urn": "group:1", "permissions": []string{"write"}}}, }, }, { @@ -205,8 +213,8 @@ func TestGetResources(t *testing.T) { ProviderURN: providerURN, Name: "All Users", Details: map[string]interface{}{ - "collection": []map[string]interface{}{{"name": "col_1", "type": "collection", "urn": "1"}}, - "database": []map[string]interface{}{{"name": "db_1", "type": "database", "urn": "database:1"}}, + "collection": []map[string]interface{}{{"name": "col_1", "type": "collection", "urn": "collection:1", "permissions": []string{"read", "write"}}}, + "database": []map[string]interface{}{{"name": "db_1", "type": "database", "urn": "database:1", "permissions": []string{"read", "write"}}}, }, }, } @@ -282,7 +290,8 @@ func TestGrantAccess(t *testing.T) { for _, tc := range testcases { crypto := new(mocks.Crypto) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) providerConfig := &domain.ProviderConfig{ Resources: tc.resourceConfigs, @@ -295,7 +304,8 @@ func TestGrantAccess(t *testing.T) { t.Run("should return error if credentials is invalid", func(t *testing.T) { crypto := new(mocks.Crypto) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) pc := &domain.ProviderConfig{ Credentials: "invalid-credentials", @@ -324,7 +334,8 @@ func TestGrantAccess(t *testing.T) { t.Run("should return error if there are any on client initialization", func(t *testing.T) { crypto := new(mocks.Crypto) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) expectedError := errors.New("decrypt error") crypto.On("Decrypt", "test-password").Return("", expectedError).Once() @@ -360,7 +371,8 @@ func TestGrantAccess(t *testing.T) { t.Run("should return error if resource type in unknown", func(t *testing.T) { crypto := new(mocks.Crypto) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) expectedError := errors.New("invalid resource type") crypto.On("Decrypt", "test-password").Return("", expectedError).Once() @@ -401,7 +413,8 @@ func TestGrantAccess(t *testing.T) { expectedError := errors.New("client error") crypto := new(mocks.Crypto) client := new(mocks.MetabaseClient) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, } @@ -443,6 +456,7 @@ func TestGrantAccess(t *testing.T) { t.Run("should return nil error if granting access is successful", func(t *testing.T) { providerURN := "test-provider-urn" crypto := new(mocks.Crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) client := new(mocks.MetabaseClient) expectedDatabase := &metabase.Database{ Name: "test-database", @@ -450,7 +464,7 @@ func TestGrantAccess(t *testing.T) { } expectedUser := "test@email.com" expectedRole := metabase.DatabaseRoleViewer - p := metabase.NewProvider("", crypto) + p := metabase.NewProvider("", crypto, logger) p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, } @@ -494,12 +508,13 @@ func TestGrantAccess(t *testing.T) { }) t.Run("given collection resource", func(t *testing.T) { - t.Run("should return error if there is an error in grandting collection access", func(t *testing.T) { + t.Run("should return error if there is an error in granting collection access", func(t *testing.T) { providerURN := "test-provider-urn" expectedError := errors.New("client error") crypto := new(mocks.Crypto) client := new(mocks.MetabaseClient) - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, } @@ -527,7 +542,7 @@ func TestGrantAccess(t *testing.T) { a := &domain.Appeal{ Resource: &domain.Resource{ Type: metabase.ResourceTypeCollection, - URN: "999", + URN: "collection:999", Name: "test-collection", }, Role: "test-role", @@ -548,7 +563,8 @@ func TestGrantAccess(t *testing.T) { } expectedUser := "test@email.com" expectedRole := "viewer" - p := metabase.NewProvider("", crypto) + logger := log.NewLogrus(log.LogrusWithLevel("info")) + p := metabase.NewProvider("", crypto, logger) p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, @@ -577,7 +593,7 @@ func TestGrantAccess(t *testing.T) { a := &domain.Appeal{ Resource: &domain.Resource{ Type: metabase.ResourceTypeCollection, - URN: "999", + URN: "collection:999", Name: "test-collection", }, Role: "viewer", diff --git a/plugins/providers/metabase/resource.go b/plugins/providers/metabase/resource.go index 4a33d2568..74f33ab10 100644 --- a/plugins/providers/metabase/resource.go +++ b/plugins/providers/metabase/resource.go @@ -117,7 +117,7 @@ func (g *Group) FromDomain(r *domain.Resource) error { groupUrn := strings.Split(r.URN, ":") if len(groupUrn) != 2 { - return ErrInvalidDatabaseURN + return ErrInvalidGroupURN } id, err := strconv.Atoi(groupUrn[1]) if err != nil { @@ -159,7 +159,15 @@ func (c *Collection) FromDomain(r *domain.Resource) error { return ErrInvalidResourceType } - id, _ := strconv.Atoi(r.URN) + collectionUrn := strings.Split(r.URN, ":") + if len(collectionUrn) != 2 { + return ErrInvalidCollectionURN + } + id, err := strconv.Atoi(collectionUrn[1]) + if err != nil { + return err + } + if id == 0 { c.ID = r.URN } else { @@ -180,7 +188,7 @@ func (c *Collection) ToDomain() *domain.Resource { return &domain.Resource{ Type: ResourceTypeCollection, Name: c.Name, - URN: fmt.Sprintf("%v", c.ID), + URN: fmt.Sprintf("collection:%v", c.ID), Details: details, } } diff --git a/plugins/providers/metabase/resource_test.go b/plugins/providers/metabase/resource_test.go index 48670de46..d4ac63717 100644 --- a/plugins/providers/metabase/resource_test.go +++ b/plugins/providers/metabase/resource_test.go @@ -97,7 +97,7 @@ func TestCollection(t *testing.T) { expectedResource: &domain.Resource{ Type: metabase.ResourceTypeCollection, Name: "collection 1", - URN: "root", + URN: "collection:root", }, }, { @@ -108,7 +108,7 @@ func TestCollection(t *testing.T) { expectedResource: &domain.Resource{ Type: metabase.ResourceTypeCollection, Name: "collection 2", - URN: "1", + URN: "collection:1", }, }, } @@ -144,18 +144,18 @@ func TestCollection(t *testing.T) { }{ { expectedResource: &domain.Resource{ - URN: "non-numeric", + URN: "collection:2", Name: "test-collection", Type: metabase.ResourceTypeCollection, }, expectedCollection: &metabase.Collection{ - ID: "non-numeric", + ID: 2, Name: "test-collection", }, }, { expectedResource: &domain.Resource{ - URN: "1", + URN: "collection:1", Name: "test-collection", Type: metabase.ResourceTypeCollection, }, From 16f517b242dd330e073a6423d3454a2195dac916 Mon Sep 17 00:00:00 2001 From: Vikash Date: Thu, 21 Apr 2022 20:05:35 +0530 Subject: [PATCH 04/17] Add constants for string literal --- plugins/providers/metabase/client.go | 75 +++++++++++++++++----------- 1 file changed, 47 insertions(+), 28 deletions(-) diff --git a/plugins/providers/metabase/client.go b/plugins/providers/metabase/client.go index a3c7e6c1e..095730744 100644 --- a/plugins/providers/metabase/client.go +++ b/plugins/providers/metabase/client.go @@ -18,6 +18,25 @@ import ( "github.com/go-playground/validator/v10" ) +const ( + databaseEndpoint = "/api/database?include=tables" + collectionEndpoint = "/api/collection" + groupEndpoint = "/api/permissions/group" + databasePermissionEndpoint = "/api/permissions/graph" + collectionPermissionEndpoint = "/api/collection/graph" + + data = "data" + database = "database" + collection = "collection" + groups = "groups" + table = "table" + none = "none" + urn = "urn" + name = "name" + permissionsConst = "permissions" + groupConst = "group" +) + type ResourceGroupDetails map[string][]map[string]interface{} type MetabaseClient interface { @@ -138,7 +157,7 @@ func NewClient(config *ClientConfig, logger *log.Logrus) (*client, error) { } func (c *client) GetDatabases() ([]*Database, error) { - req, err := c.newRequest(http.MethodGet, "/api/database?include=tables", nil) + req, err := c.newRequest(http.MethodGet, databaseEndpoint, nil) if err != nil { return nil, err } @@ -153,8 +172,8 @@ func (c *client) GetDatabases() ([]*Database, error) { if v, ok := response.([]interface{}); ok { err = mapstructure.Decode(v, &databases) // this is for metabase v0.37 - } else if v, ok := response.(map[string]interface{}); ok && v["data"] != nil { - err = mapstructure.Decode(v["data"], &databases) // this is for metabase v0.42 + } else if v, ok := response.(map[string]interface{}); ok && v[data] != nil { + err = mapstructure.Decode(v[data], &databases) // this is for metabase v0.42 } else { return databases, ErrInvalidApiResponse } @@ -167,7 +186,7 @@ func (c *client) GetDatabases() ([]*Database, error) { } func (c *client) GetCollections() ([]*Collection, error) { - req, err := c.newRequest(http.MethodGet, "/api/collection", nil) + req, err := c.newRequest(http.MethodGet, collectionEndpoint, nil) if err != nil { return nil, err } @@ -201,15 +220,15 @@ func (c *client) GetGroups() ([]*Group, ResourceGroupDetails, ResourceGroupDetai groupMap[fmt.Sprintf("group:%d", group.ID)] = group } - addResourceToGroup(databaseResourceGroups, groupMap, "database") - addResourceToGroup(collectionResourceGroups, groupMap, "collection") + addResourceToGroup(databaseResourceGroups, groupMap, database) + addResourceToGroup(collectionResourceGroups, groupMap, collection) return groups, databaseResourceGroups, collectionResourceGroups, err } func (c *client) fetchGroups(wg *sync.WaitGroup, groups *[]*Group, err error) { defer wg.Done() - req, err := c.newRequest(http.MethodGet, "/api/permissions/group", nil) + req, err := c.newRequest(http.MethodGet, groupEndpoint, nil) if err != nil { return } @@ -224,7 +243,7 @@ func (c *client) fetchGroups(wg *sync.WaitGroup, groups *[]*Group, err error) { func (c *client) fetchDatabasePermissions(wg *sync.WaitGroup, resourceGroups ResourceGroupDetails, err error) { defer wg.Done() - req, err := c.newRequest(http.MethodGet, "/api/permissions/graph", nil) + req, err := c.newRequest(http.MethodGet, databasePermissionEndpoint, nil) if err != nil { return } @@ -235,7 +254,7 @@ func (c *client) fetchDatabasePermissions(wg *sync.WaitGroup, resourceGroups Res return } - for groupId, r := range graphs["groups"].(map[string]interface{}) { + for groupId, r := range graphs[groups].(map[string]interface{}) { for dbId, role := range r.(map[string]interface{}) { if roles, ok := role.(map[string]interface{}); ok { permissions := make([]string, 0) @@ -244,7 +263,7 @@ func (c *client) fetchDatabasePermissions(wg *sync.WaitGroup, resourceGroups Res for _, tables := range tables { if tables, ok := tables.(map[string]interface{}); ok { for tableId, tablePermission := range tables { - addGroupToResource(resourceGroups, fmt.Sprintf("table:%s.%s", dbId, tableId), groupId, []string{tablePermission.(string)}, err) + addGroupToResource(resourceGroups, fmt.Sprintf("%s:%s.%s", table, dbId, tableId), groupId, []string{tablePermission.(string)}, err) } } } @@ -252,7 +271,7 @@ func (c *client) fetchDatabasePermissions(wg *sync.WaitGroup, resourceGroups Res permissions = append(permissions, fmt.Sprintf("%s:%s", key, value)) } } - addGroupToResource(resourceGroups, fmt.Sprintf("database:%s", dbId), groupId, permissions, err) + addGroupToResource(resourceGroups, fmt.Sprintf("%s:%s", database, dbId), groupId, permissions, err) } } } @@ -261,7 +280,7 @@ func (c *client) fetchDatabasePermissions(wg *sync.WaitGroup, resourceGroups Res func (c *client) fetchCollectionPermissions(wg *sync.WaitGroup, resourceGroups ResourceGroupDetails, err error) { defer wg.Done() - req, err := c.newRequest(http.MethodGet, "/api/collection/graph", nil) + req, err := c.newRequest(http.MethodGet, collectionPermissionEndpoint, nil) if err != nil { return } @@ -272,10 +291,10 @@ func (c *client) fetchCollectionPermissions(wg *sync.WaitGroup, resourceGroups R return } c.logger.Info(fmt.Sprintf("Fetch permissions for collections from request: %v", req.URL)) - for groupId, r := range graphs["groups"].(map[string]interface{}) { + for groupId, r := range graphs[groups].(map[string]interface{}) { for collectionId, permission := range r.(map[string]interface{}) { - if permission != "none" { - addGroupToResource(resourceGroups, fmt.Sprintf("collection:%s", collectionId), groupId, []string{permission.(string)}, err) + if permission != none { + addGroupToResource(resourceGroups, fmt.Sprintf("%s:%s", collection, collectionId), groupId, []string{permission.(string)}, err) } } } @@ -284,14 +303,14 @@ func (c *client) fetchCollectionPermissions(wg *sync.WaitGroup, resourceGroups R func addResourceToGroup(resourceGroups ResourceGroupDetails, groupMap map[string]*Group, resourceType string) { for resourceId, groups := range resourceGroups { for _, groupDetails := range groups { - groupID := groupDetails["urn"].(string) + groupID := groupDetails[urn].(string) if group, ok := groupMap[groupID]; ok { - groupDetails["name"] = group.Name - if resourceType == "database" { - group.DatabaseResources = append(group.DatabaseResources, map[string]interface{}{"urn": resourceId, "permissions": groupDetails["permissions"]}) + groupDetails[name] = group.Name + if resourceType == database { + group.DatabaseResources = append(group.DatabaseResources, map[string]interface{}{urn: resourceId, permissionsConst: groupDetails[permissionsConst]}) } - if resourceType == "collection" { - group.CollectionResources = append(group.CollectionResources, map[string]interface{}{"urn": resourceId, "permissions": groupDetails["permissions"]}) + if resourceType == collection { + group.CollectionResources = append(group.CollectionResources, map[string]interface{}{urn: resourceId, permissionsConst: groupDetails[permissionsConst]}) } } } @@ -304,10 +323,10 @@ func addGroupToResource(resourceGroups ResourceGroupDetails, resourceId string, return } if groups, ok := resourceGroups[resourceId]; ok { - groups = append(groups, map[string]interface{}{"urn": fmt.Sprintf("group:%d", id), "permissions": permissions}) + groups = append(groups, map[string]interface{}{urn: fmt.Sprintf("%s:%d", groupConst, id), permissionsConst: permissions}) resourceGroups[resourceId] = groups } else { - resourceGroups[resourceId] = []map[string]interface{}{{"urn": fmt.Sprintf("group:%d", id), "permissions": permissions}} + resourceGroups[resourceId] = []map[string]interface{}{{urn: fmt.Sprintf("%s:%d", groupConst, id), permissionsConst: permissions}} } } @@ -516,7 +535,7 @@ func (c *client) getSessionToken() (string, error) { } func (c *client) getCollectionAccess() (*collectionGraph, error) { - req, err := c.newRequest(http.MethodGet, "/api/collection/graph", nil) + req, err := c.newRequest(http.MethodGet, collectionPermissionEndpoint, nil) if err != nil { return nil, err } @@ -530,7 +549,7 @@ func (c *client) getCollectionAccess() (*collectionGraph, error) { } func (c *client) updateCollectionAccess(access *collectionGraph) error { - req, err := c.newRequest(http.MethodPut, "/api/collection/graph", access) + req, err := c.newRequest(http.MethodPut, collectionPermissionEndpoint, access) if err != nil { return err } @@ -543,7 +562,7 @@ func (c *client) updateCollectionAccess(access *collectionGraph) error { } func (c *client) getDatabaseAccess() (*databaseGraph, error) { - req, err := c.newRequest(http.MethodGet, "/api/permissions/graph", nil) + req, err := c.newRequest(http.MethodGet, databasePermissionEndpoint, nil) if err != nil { return nil, err } @@ -557,7 +576,7 @@ func (c *client) getDatabaseAccess() (*databaseGraph, error) { } func (c *client) updateDatabaseAccess(dbGraph *databaseGraph) error { - req, err := c.newRequest(http.MethodPut, "/api/permissions/graph", dbGraph) + req, err := c.newRequest(http.MethodPut, databasePermissionEndpoint, dbGraph) if err != nil { return err } @@ -570,7 +589,7 @@ func (c *client) updateDatabaseAccess(dbGraph *databaseGraph) error { } func (c *client) createGroup(group *group) error { - req, err := c.newRequest(http.MethodPost, "/api/permissions/group", group) + req, err := c.newRequest(http.MethodPost, groupEndpoint, group) if err != nil { return err } From e79c96ee880cec8437d4c4aac18cfb45d8a164dd Mon Sep 17 00:00:00 2001 From: Vikash Date: Thu, 21 Apr 2022 20:43:39 +0530 Subject: [PATCH 05/17] Change type for database and collection in group --- plugins/providers/metabase/client.go | 4 ++-- plugins/providers/metabase/provider.go | 19 +++++++++---------- plugins/providers/metabase/provider_test.go | 8 ++++---- plugins/providers/metabase/resource.go | 19 +++++++++++++------ 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/plugins/providers/metabase/client.go b/plugins/providers/metabase/client.go index 095730744..26f51123c 100644 --- a/plugins/providers/metabase/client.go +++ b/plugins/providers/metabase/client.go @@ -307,10 +307,10 @@ func addResourceToGroup(resourceGroups ResourceGroupDetails, groupMap map[string if group, ok := groupMap[groupID]; ok { groupDetails[name] = group.Name if resourceType == database { - group.DatabaseResources = append(group.DatabaseResources, map[string]interface{}{urn: resourceId, permissionsConst: groupDetails[permissionsConst]}) + group.DatabaseResources = append(group.DatabaseResources, &GroupResource{Urn: resourceId, Permissions: groupDetails[permissionsConst].([]string)}) } if resourceType == collection { - group.CollectionResources = append(group.CollectionResources, map[string]interface{}{urn: resourceId, permissionsConst: groupDetails[permissionsConst]}) + group.CollectionResources = append(group.CollectionResources, &GroupResource{Urn: resourceId, Permissions: groupDetails[permissionsConst].([]string)}) } } } diff --git a/plugins/providers/metabase/provider.go b/plugins/providers/metabase/provider.go index 719130952..96fd59d30 100644 --- a/plugins/providers/metabase/provider.go +++ b/plugins/providers/metabase/provider.go @@ -67,8 +67,7 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, resources = append(resources, db) if _, ok := resourceTypes[ResourceTypeTable]; ok && resourceTypes[ResourceTypeTable] { - tables := d.Tables - for _, t := range tables { + for _, t := range d.Tables { t.Database = db table := t.ToDomain() table.ProviderType = pc.Type @@ -123,19 +122,19 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, if _, ok := resourceTypes[ResourceTypeGroup]; ok && resourceTypes[ResourceTypeGroup] { for _, g := range groups { - for _, resourceMap := range g.DatabaseResources { - resourceId := resourceMap["urn"].(string) + for _, groupResource := range g.DatabaseResources { + resourceId := groupResource.Urn if resource, ok := databaseResourceMap[resourceId]; ok { - resourceMap["name"] = resource.Name - resourceMap["type"] = resource.Type + groupResource.Name = resource.Name + groupResource.Type = resource.Type } } - for _, resourceMap := range g.CollectionResources { - resourceId := resourceMap["urn"].(string) + for _, groupResource := range g.CollectionResources { + resourceId := groupResource.Urn if resource, ok := collectionResourceMap[resourceId]; ok { - resourceMap["name"] = resource.Name - resourceMap["type"] = resource.Type + groupResource.Name = resource.Name + groupResource.Type = resource.Type } } diff --git a/plugins/providers/metabase/provider_test.go b/plugins/providers/metabase/provider_test.go index 3c902ae93..da984b126 100644 --- a/plugins/providers/metabase/provider_test.go +++ b/plugins/providers/metabase/provider_test.go @@ -153,8 +153,8 @@ func TestGetResources(t *testing.T) { } client.On("GetDatabases").Return(expectedDatabases, nil).Once() - d := []map[string]interface{}{{"urn": "database:1", "permissions": []string{"read", "write"}}} - c := []map[string]interface{}{{"urn": "collection:1", "permissions": []string{"read", "write"}}} + d := []*metabase.GroupResource{{Urn: "database:1", Permissions: []string{"read", "write"}}} + c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} client.On("GetGroups").Return([]*metabase.Group{&group}, @@ -213,8 +213,8 @@ func TestGetResources(t *testing.T) { ProviderURN: providerURN, Name: "All Users", Details: map[string]interface{}{ - "collection": []map[string]interface{}{{"name": "col_1", "type": "collection", "urn": "collection:1", "permissions": []string{"read", "write"}}}, - "database": []map[string]interface{}{{"name": "db_1", "type": "database", "urn": "database:1", "permissions": []string{"read", "write"}}}, + "collection": []*metabase.GroupResource{{Name: "col_1", Type: "collection", Urn: "collection:1", Permissions: []string{"read", "write"}}}, + "database": []*metabase.GroupResource{{Name: "db_1", Type: "database", Urn: "database:1", Permissions: []string{"read", "write"}}}, }, }, } diff --git a/plugins/providers/metabase/resource.go b/plugins/providers/metabase/resource.go index 74f33ab10..a60b016ca 100644 --- a/plugins/providers/metabase/resource.go +++ b/plugins/providers/metabase/resource.go @@ -34,11 +34,18 @@ type Table struct { Database *domain.Resource } +type GroupResource struct { + Name string `json:"name"` + Permissions []string `json:"permission"` + Urn string `json:"urn"` + Type string `json:"type"` +} + type Group struct { - ID int `json:"id"` - Name string `json:"name"` - DatabaseResources []map[string]interface{} `json:"database_resources"` - CollectionResources []map[string]interface{} `json:"collection_resources"` + ID int `json:"id"` + Name string `json:"name"` + DatabaseResources []*GroupResource `json:"database"` + CollectionResources []*GroupResource `json:"collection"` } func (d *Database) FromDomain(r *domain.Resource) error { @@ -126,8 +133,8 @@ func (g *Group) FromDomain(r *domain.Resource) error { g.ID = id g.Name = r.Name - g.DatabaseResources = r.Details["database"].([]map[string]interface{}) - g.CollectionResources = r.Details["collection"].([]map[string]interface{}) + g.DatabaseResources = r.Details["database"].([]*GroupResource) + g.CollectionResources = r.Details["collection"].([]*GroupResource) if err != nil { return err } From 2d93d9cc98fc715654ce6784fe6ef89384d6ff14 Mon Sep 17 00:00:00 2001 From: Vikash Date: Fri, 22 Apr 2022 11:40:18 +0530 Subject: [PATCH 06/17] Resolve feedback --- plugins/providers/metabase/provider.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/providers/metabase/provider.go b/plugins/providers/metabase/provider.go index 96fd59d30..4dbc190a2 100644 --- a/plugins/providers/metabase/provider.go +++ b/plugins/providers/metabase/provider.go @@ -55,7 +55,7 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, resources := []*domain.Resource{} - if _, ok := resourceTypes[ResourceTypeDatabase]; ok && resourceTypes[ResourceTypeDatabase] { + if _, ok := resourceTypes[ResourceTypeDatabase]; ok { databases, err := client.GetDatabases() if err != nil { return nil, err @@ -66,7 +66,7 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, db.ProviderURN = pc.URN resources = append(resources, db) - if _, ok := resourceTypes[ResourceTypeTable]; ok && resourceTypes[ResourceTypeTable] { + if _, ok := resourceTypes[ResourceTypeTable]; ok { for _, t := range d.Tables { t.Database = db table := t.ToDomain() @@ -78,7 +78,7 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, } } - if _, ok := resourceTypes[ResourceTypeCollection]; ok && resourceTypes[ResourceTypeCollection] { + if _, ok := resourceTypes[ResourceTypeCollection]; ok { collections, err := client.GetCollections() if err != nil { return nil, err From 4257dd2b9936eeba9ea21e76b3c47b0521c4fd41 Mon Sep 17 00:00:00 2001 From: Vikash Date: Tue, 10 May 2022 10:12:10 +0530 Subject: [PATCH 07/17] fix: creation of existing group users already added to a group grant access for group revoke access for group --- mocks/MetabaseClient.go | 8 +- plugins/providers/metabase/client.go | 132 +++++++++++++------- plugins/providers/metabase/provider.go | 33 ++++- plugins/providers/metabase/provider_test.go | 35 +++++- 4 files changed, 156 insertions(+), 52 deletions(-) diff --git a/mocks/MetabaseClient.go b/mocks/MetabaseClient.go index c75007b86..e5b421fe5 100644 --- a/mocks/MetabaseClient.go +++ b/mocks/MetabaseClient.go @@ -109,12 +109,12 @@ func (_m *MetabaseClient) GrantCollectionAccess(resource *metabase.Collection, u } // GrantDatabaseAccess provides a mock function with given fields: resource, user, role -func (_m *MetabaseClient) GrantDatabaseAccess(resource *metabase.Database, user string, role string) error { - ret := _m.Called(resource, user, role) +func (_m *MetabaseClient) GrantDatabaseAccess(resource *metabase.Database, user string, role string, groups map[string]*metabase.Group) error { + ret := _m.Called(resource, user, role, groups) var r0 error - if rf, ok := ret.Get(0).(func(*metabase.Database, string, string) error); ok { - r0 = rf(resource, user, role) + if rf, ok := ret.Get(0).(func(*metabase.Database, string, string, map[string]*metabase.Group) error); ok { + r0 = rf(resource, user, role, groups) } else { r0 = ret.Error(0) } diff --git a/plugins/providers/metabase/client.go b/plugins/providers/metabase/client.go index 26f51123c..7bec72a42 100644 --- a/plugins/providers/metabase/client.go +++ b/plugins/providers/metabase/client.go @@ -3,6 +3,7 @@ package metabase import ( "bytes" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -43,10 +44,12 @@ type MetabaseClient interface { GetDatabases() ([]*Database, error) GetCollections() ([]*Collection, error) GetGroups() ([]*Group, ResourceGroupDetails, ResourceGroupDetails, error) - GrantDatabaseAccess(resource *Database, user, role string) error + GrantDatabaseAccess(resource *Database, user, role string, groups map[string]*Group) error RevokeDatabaseAccess(resource *Database, user, role string) error GrantCollectionAccess(resource *Collection, user, role string) error RevokeCollectionAccess(resource *Collection, user, role string) error + GrantGroupAccess(groupID int, email string) error + RevokeGroupAccess(groupID int, email string) error } type ClientConfig struct { @@ -60,6 +63,7 @@ type user struct { ID int `json:"id"` Email string `json:"email"` MembershipID int `json:"membership_id"` + GroupIds []int `json:"group_ids"` } type group struct { @@ -77,10 +81,7 @@ type SessionResponse struct { ID string `json:"id"` } -type databasePermission struct { - Native string `json:"native,omitempty" mapstructure:"native"` - Schemas string `json:"schemas" mapstructure:"schemas"` -} +type databasePermission map[string]interface{} type databaseGraph struct { Revision int `json:"revision"` @@ -101,11 +102,11 @@ type membershipRequest struct { var ( databaseViewerPermission = databasePermission{ - Schemas: "all", + "schemas": "all", } databaseEditorPermission = databasePermission{ - Schemas: "all", - Native: "write", + "native": "write", + "schemas": "all", } ) @@ -330,7 +331,7 @@ func addGroupToResource(resourceGroups ResourceGroupDetails, resourceId string, } } -func (c *client) GrantDatabaseAccess(resource *Database, user, role string) error { +func (c *client) GrantDatabaseAccess(resource *Database, email, role string, groups map[string]*Group) error { access, err := c.getDatabaseAccess() if err != nil { return err @@ -344,17 +345,24 @@ func (c *client) GrantDatabaseAccess(resource *Database, user, role string) erro } resourceIDStr := strconv.Itoa(resource.ID) + toBrGroupName := fmt.Sprintf("%s_%v_%s", ResourceTypeDatabase, resource.ID, role) groupID := c.findDatabaseAccessGroup(access, resourceIDStr, dbPermission) if groupID == "" { - g := &group{ - Name: fmt.Sprintf("%s_%v_%s", ResourceTypeDatabase, resource.ID, role), - } - if err := c.createGroup(g); err != nil { - return err + var groupID string + if g, ok := groups[toBrGroupName]; ok { + groupID = strconv.Itoa(g.ID) + } else { + g := &group{ + Name: toBrGroupName, + } + if err := c.createGroup(g); err != nil { + return err + } + + groupID = strconv.Itoa(g.ID) } - groupID = strconv.Itoa(g.ID) databaseID := fmt.Sprintf("%v", resource.ID) access.Groups[groupID] = map[string]databasePermission{} @@ -368,11 +376,19 @@ func (c *client) GrantDatabaseAccess(resource *Database, user, role string) erro if err != nil { return err } - userID, err := c.getUserID(user) + + user, err := c.getUser(email) if err != nil { return err } - return c.addGroupMember(groupIDint, userID) + + for _, groupId := range user.GroupIds { + if groupId == groupIDint { + return nil + } + } + + return c.addGroupMember(groupIDint, user.ID) } func (c *client) RevokeDatabaseAccess(resource *Database, user, role string) error { @@ -402,7 +418,7 @@ func (c *client) RevokeDatabaseAccess(resource *Database, user, role string) err return c.removeMembership(groupIDInt, user) } -func (c *client) GrantCollectionAccess(resource *Collection, user, role string) error { +func (c *client) GrantCollectionAccess(resource *Collection, email, role string) error { access, err := c.getCollectionAccess() if err != nil { return err @@ -433,11 +449,11 @@ func (c *client) GrantCollectionAccess(resource *Collection, user, role string) if err != nil { return err } - userID, err := c.getUserID(user) + user, err := c.getUser(email) if err != nil { return err } - return c.addGroupMember(groupIDInt, userID) + return c.addGroupMember(groupIDInt, user.ID) } func (c *client) RevokeCollectionAccess(resource *Collection, user, role string) error { @@ -460,6 +476,26 @@ func (c *client) RevokeCollectionAccess(resource *Collection, user, role string) return c.removeMembership(groupIDInt, user) } +func (c *client) GrantGroupAccess(groupID int, email string) error { + user, err := c.getUser(email) + if err != nil { + return err + } + + for _, userGroupId := range user.GroupIds { + if userGroupId == groupID { + c.logger.Warn(fmt.Sprintf("User %s is already member of group %s", email, groupID)) + return nil + } + } + + return c.addGroupMember(groupID, user.ID) +} + +func (c *client) RevokeGroupAccess(groupID int, email string) error { + return c.removeMembership(groupID, email) +} + func (c *client) removeMembership(groupID int, user string) error { group, err := c.getGroup(groupID) if err != nil { @@ -480,40 +516,37 @@ func (c *client) removeMembership(groupID int, user string) error { return c.removeGroupMember(membershipID) } -func (c *client) getUserID(email string) (int, error) { - if c.userIDs[email] != 0 { - return c.userIDs[email], nil - } - - users, err := c.getUsers() +func (c *client) getUser(email string) (user, error) { + req, err := c.newRequest(http.MethodGet, fmt.Sprintf("/api/user?query=%s", email), nil) if err != nil { - return 0, err + return user{}, err } - userIDs := map[string]int{} - for _, u := range users { - userIDs[u.Email] = u.ID + var users []user + var response interface{} + if _, err := c.do(req, &response); err != nil { + return user{}, err } - c.userIDs = userIDs - if c.userIDs[email] == 0 { - return 0, ErrUserNotFound + if v, ok := response.([]interface{}); ok { + err = mapstructure.Decode(v, &users) // this is for metabase v0.37 + } else if v, ok := response.(map[string]interface{}); ok && v[data] != nil { + err = mapstructure.Decode(v[data], &users) // this is for metabase v0.42 + } else { + return user{}, ErrInvalidApiResponse } - return c.userIDs[email], nil -} -func (c *client) getUsers() ([]user, error) { - req, err := c.newRequest(http.MethodGet, "/api/user", nil) if err != nil { - return nil, err + return user{}, ErrUserNotFound } - var users []user - if _, err := c.do(req, &users); err != nil { - return nil, err + for _, u := range users { + if u.Email == email { + return u, nil + } } - return users, nil + return user{}, ErrUserNotFound } func (c *client) getSessionToken() (string, error) { @@ -731,8 +764,21 @@ func (c *client) do(req *http.Request, v interface{}) (*http.Response, error) { } } + if resp.StatusCode == http.StatusBadRequest { + byteData, _ := io.ReadAll(resp.Body) + return nil, errors.New(string(byteData)) + } + + if resp.StatusCode == http.StatusInternalServerError { + byteData, _ := io.ReadAll(resp.Body) + return nil, errors.New(string(byteData)) + } + if v != nil { - err = json.NewDecoder(resp.Body).Decode(v) + byteData, _ := io.ReadAll(resp.Body) + a := string(byteData) + fmt.Println(a) + err = json.NewDecoder(bytes.NewReader(byteData)).Decode(v) } return resp, err } diff --git a/plugins/providers/metabase/provider.go b/plugins/providers/metabase/provider.go index 4dbc190a2..194ddf4a6 100644 --- a/plugins/providers/metabase/provider.go +++ b/plugins/providers/metabase/provider.go @@ -165,6 +165,16 @@ func (p *provider) GrantAccess(pc *domain.ProviderConfig, a *domain.Appeal) erro return err } + groups, _, _, err := client.GetGroups() + if err != nil { + return err + } + + groupMap := make(map[string]*Group, 0) + for _, group := range groups { + groupMap[group.Name] = group + } + if a.Resource.Type == ResourceTypeDatabase { d := new(Database) if err := d.FromDomain(a.Resource); err != nil { @@ -172,7 +182,7 @@ func (p *provider) GrantAccess(pc *domain.ProviderConfig, a *domain.Appeal) erro } for _, p := range permissions { - if err := client.GrantDatabaseAccess(d, a.AccountID, string(p)); err != nil { + if err := client.GrantDatabaseAccess(d, a.AccountID, string(p), groupMap); err != nil { return err } } @@ -191,6 +201,16 @@ func (p *provider) GrantAccess(pc *domain.ProviderConfig, a *domain.Appeal) erro } return nil + } else if a.Resource.Type == ResourceTypeGroup { + g := new(Group) + if err := g.FromDomain(a.Resource); err != nil { + return err + } + + if err := client.GrantGroupAccess(g.ID, a.AccountID); err != nil { + return err + } + } return ErrInvalidResourceType @@ -236,6 +256,17 @@ func (p *provider) RevokeAccess(pc *domain.ProviderConfig, a *domain.Appeal) err } } + return nil + } else if a.Resource.Type == ResourceTypeGroup { + g := new(Group) + if err := g.FromDomain(a.Resource); err != nil { + return err + } + + if err := client.RevokeGroupAccess(g.ID, a.AccountID); err != nil { + return err + } + return nil } diff --git a/plugins/providers/metabase/provider_test.go b/plugins/providers/metabase/provider_test.go index da984b126..67df74ec2 100644 --- a/plugins/providers/metabase/provider_test.go +++ b/plugins/providers/metabase/provider_test.go @@ -418,7 +418,13 @@ func TestGrantAccess(t *testing.T) { p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, } - client.On("GrantDatabaseAccess", mock.Anything, mock.Anything, mock.Anything).Return(expectedError).Once() + client.On("GrantDatabaseAccess", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(expectedError).Once() + d := []*metabase.GroupResource{{Urn: "database:1", Permissions: []string{"read", "write"}}} + c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} + group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} + client.On("GetGroups").Return([]*metabase.Group{&group}, + map[string][]map[string]interface{}{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, + map[string][]map[string]interface{}{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() pc := &domain.ProviderConfig{ Credentials: metabase.Credentials{ @@ -468,7 +474,14 @@ func TestGrantAccess(t *testing.T) { p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, } - client.On("GrantDatabaseAccess", expectedDatabase, expectedUser, expectedRole).Return(nil).Once() + client.On("GrantDatabaseAccess", expectedDatabase, expectedUser, expectedRole, mock.Anything).Return(nil).Once() + + d := []*metabase.GroupResource{{Urn: "database:1", Permissions: []string{"read", "write"}}} + c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} + group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} + client.On("GetGroups").Return([]*metabase.Group{&group}, + map[string][]map[string]interface{}{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, + map[string][]map[string]interface{}{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() pc := &domain.ProviderConfig{ Credentials: metabase.Credentials{ @@ -518,7 +531,14 @@ func TestGrantAccess(t *testing.T) { p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, } - client.On("GrantCollectionAccess", mock.Anything, mock.Anything, mock.Anything).Return(expectedError).Once() + client.On("GrantCollectionAccess", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(expectedError).Once() + + d := []*metabase.GroupResource{{Urn: "database:1", Permissions: []string{"read", "write"}}} + c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} + group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} + client.On("GetGroups").Return([]*metabase.Group{&group}, + map[string][]map[string]interface{}{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, + map[string][]map[string]interface{}{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() pc := &domain.ProviderConfig{ Credentials: metabase.Credentials{ @@ -569,7 +589,14 @@ func TestGrantAccess(t *testing.T) { p.Clients = map[string]metabase.MetabaseClient{ providerURN: client, } - client.On("GrantCollectionAccess", expectedCollection, expectedUser, expectedRole).Return(nil).Once() + client.On("GrantCollectionAccess", expectedCollection, expectedUser, expectedRole, mock.Anything).Return(nil).Once() + + d := []*metabase.GroupResource{{Urn: "database:1", Permissions: []string{"read", "write"}}} + c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} + group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} + client.On("GetGroups").Return([]*metabase.Group{&group}, + map[string][]map[string]interface{}{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, + map[string][]map[string]interface{}{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() pc := &domain.ProviderConfig{ Credentials: metabase.Credentials{ From 1c1f8072a1e35af52db71954137268ca37ee55b3 Mon Sep 17 00:00:00 2001 From: Vikash Date: Tue, 10 May 2022 17:24:01 +0530 Subject: [PATCH 08/17] Add grant and revoke method for group, table --- core/provider/service.go | 2 +- mocks/MetabaseClient.go | 77 ++++++++-- plugins/providers/metabase/client.go | 149 ++++++++++++++++++-- plugins/providers/metabase/config.go | 7 +- plugins/providers/metabase/provider.go | 78 ++++++++-- plugins/providers/metabase/provider_test.go | 20 +-- plugins/providers/metabase/resource.go | 7 +- 7 files changed, 291 insertions(+), 49 deletions(-) diff --git a/core/provider/service.go b/core/provider/service.go index ca3713e80..4d9179dee 100644 --- a/core/provider/service.go +++ b/core/provider/service.go @@ -205,7 +205,7 @@ func (s *Service) ValidateAppeal(a *domain.Appeal, p *domain.Provider) error { return err } - isRoleExists := false + isRoleExists := len(roles) == 0 for _, role := range roles { if a.Role == role.ID { isRoleExists = true diff --git a/mocks/MetabaseClient.go b/mocks/MetabaseClient.go index e5b421fe5..bfdeae096 100644 --- a/mocks/MetabaseClient.go +++ b/mocks/MetabaseClient.go @@ -1,4 +1,4 @@ -// Code generated by mockery 2.9.0. DO NOT EDIT. +// Code generated by mockery v2.10.0. DO NOT EDIT. package mocks @@ -58,6 +58,7 @@ func (_m *MetabaseClient) GetDatabases() ([]*metabase.Database, error) { return r0, r1 } +// GetGroups provides a mock function with given fields: func (_m *MetabaseClient) GetGroups() ([]*metabase.Group, metabase.ResourceGroupDetails, metabase.ResourceGroupDetails, error) { ret := _m.Called() @@ -70,18 +71,22 @@ func (_m *MetabaseClient) GetGroups() ([]*metabase.Group, metabase.ResourceGroup } } - var r1 map[string][]map[string]interface{} - if rf, ok := ret.Get(1).(func() map[string][]map[string]interface{}); ok { + var r1 metabase.ResourceGroupDetails + if rf, ok := ret.Get(1).(func() metabase.ResourceGroupDetails); ok { r1 = rf() } else { - r1 = ret.Get(1).(map[string][]map[string]interface{}) + if ret.Get(1) != nil { + r1 = ret.Get(1).(metabase.ResourceGroupDetails) + } } - var r2 map[string][]map[string]interface{} - if rf, ok := ret.Get(2).(func() map[string][]map[string]interface{}); ok { + var r2 metabase.ResourceGroupDetails + if rf, ok := ret.Get(2).(func() metabase.ResourceGroupDetails); ok { r2 = rf() } else { - r2 = ret.Get(2).(map[string][]map[string]interface{}) + if ret.Get(2) != nil { + r2 = ret.Get(2).(metabase.ResourceGroupDetails) + } } var r3 error @@ -108,7 +113,7 @@ func (_m *MetabaseClient) GrantCollectionAccess(resource *metabase.Collection, u return r0 } -// GrantDatabaseAccess provides a mock function with given fields: resource, user, role +// GrantDatabaseAccess provides a mock function with given fields: resource, user, role, groups func (_m *MetabaseClient) GrantDatabaseAccess(resource *metabase.Database, user string, role string, groups map[string]*metabase.Group) error { ret := _m.Called(resource, user, role, groups) @@ -122,6 +127,34 @@ func (_m *MetabaseClient) GrantDatabaseAccess(resource *metabase.Database, user return r0 } +// GrantGroupAccess provides a mock function with given fields: groupID, email +func (_m *MetabaseClient) GrantGroupAccess(groupID int, email string) error { + ret := _m.Called(groupID, email) + + var r0 error + if rf, ok := ret.Get(0).(func(int, string) error); ok { + r0 = rf(groupID, email) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// GrantTableAccess provides a mock function with given fields: resource, user, role, groups +func (_m *MetabaseClient) GrantTableAccess(resource *metabase.Table, user string, role string, groups map[string]*metabase.Group) error { + ret := _m.Called(resource, user, role, groups) + + var r0 error + if rf, ok := ret.Get(0).(func(*metabase.Table, string, string, map[string]*metabase.Group) error); ok { + r0 = rf(resource, user, role, groups) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // RevokeCollectionAccess provides a mock function with given fields: resource, user, role func (_m *MetabaseClient) RevokeCollectionAccess(resource *metabase.Collection, user string, role string) error { ret := _m.Called(resource, user, role) @@ -149,3 +182,31 @@ func (_m *MetabaseClient) RevokeDatabaseAccess(resource *metabase.Database, user return r0 } + +// RevokeGroupAccess provides a mock function with given fields: groupID, email +func (_m *MetabaseClient) RevokeGroupAccess(groupID int, email string) error { + ret := _m.Called(groupID, email) + + var r0 error + if rf, ok := ret.Get(0).(func(int, string) error); ok { + r0 = rf(groupID, email) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// RevokeTableAccess provides a mock function with given fields: resource, user, role +func (_m *MetabaseClient) RevokeTableAccess(resource *metabase.Table, user string, role string) error { + ret := _m.Called(resource, user, role) + + var r0 error + if rf, ok := ret.Get(0).(func(*metabase.Table, string, string) error); ok { + r0 = rf(resource, user, role) + } else { + r0 = ret.Error(0) + } + + return r0 +} diff --git a/plugins/providers/metabase/client.go b/plugins/providers/metabase/client.go index 7bec72a42..2c00fb6f2 100644 --- a/plugins/providers/metabase/client.go +++ b/plugins/providers/metabase/client.go @@ -48,6 +48,8 @@ type MetabaseClient interface { RevokeDatabaseAccess(resource *Database, user, role string) error GrantCollectionAccess(resource *Collection, user, role string) error RevokeCollectionAccess(resource *Collection, user, role string) error + GrantTableAccess(resource *Table, user, role string, groups map[string]*Group) error + RevokeTableAccess(resource *Table, user, role string) error GrantGroupAccess(groupID int, email string) error RevokeGroupAccess(groupID int, email string) error } @@ -62,8 +64,8 @@ type ClientConfig struct { type user struct { ID int `json:"id"` Email string `json:"email"` - MembershipID int `json:"membership_id"` - GroupIds []int `json:"group_ids"` + MembershipID int `json:"membership_id" mapstructure:"membership_id"` + GroupIds []int `json:"group_ids" mapstructure:"group_ids"` } type group struct { @@ -342,6 +344,8 @@ func (c *client) GrantDatabaseAccess(resource *Database, email, role string, gro dbPermission = databaseViewerPermission } else if role == DatabaseRoleEditor { dbPermission = databaseEditorPermission + } else { + return ErrInvalidRole } resourceIDStr := strconv.Itoa(resource.ID) @@ -349,7 +353,6 @@ func (c *client) GrantDatabaseAccess(resource *Database, email, role string, gro groupID := c.findDatabaseAccessGroup(access, resourceIDStr, dbPermission) if groupID == "" { - var groupID string if g, ok := groups[toBrGroupName]; ok { groupID = strconv.Itoa(g.ID) } else { @@ -372,7 +375,7 @@ func (c *client) GrantDatabaseAccess(resource *Database, email, role string, gro } } - groupIDint, err := strconv.Atoi(groupID) + groupIDInt, err := strconv.Atoi(groupID) if err != nil { return err } @@ -383,12 +386,12 @@ func (c *client) GrantDatabaseAccess(resource *Database, email, role string, gro } for _, groupId := range user.GroupIds { - if groupId == groupIDint { + if groupId == groupIDInt { return nil } } - return c.addGroupMember(groupIDint, user.ID) + return c.addGroupMember(groupIDInt, user.ID) } func (c *client) RevokeDatabaseAccess(resource *Database, user, role string) error { @@ -425,11 +428,15 @@ func (c *client) GrantCollectionAccess(resource *Collection, email, role string) } resourceIDStr := fmt.Sprintf("%v", resource.ID) + if role != CollectionRoleViewer && role != CollectionRoleCurate { + return ErrInvalidRole + } + groupID := c.findCollectionAccessGroup(access, resourceIDStr, role) if groupID == "" { g := &group{ - Name: fmt.Sprintf("%s_%s_%s", ResourceTypeCollection, resource.ID, role), + Name: fmt.Sprintf("%s_%s_%s", ResourceTypeCollection, resourceIDStr, role), } if err := c.createGroup(g); err != nil { return err @@ -453,6 +460,13 @@ func (c *client) GrantCollectionAccess(resource *Collection, email, role string) if err != nil { return err } + + for _, groupId := range user.GroupIds { + if groupId == groupIDInt { + return nil + } + } + return c.addGroupMember(groupIDInt, user.ID) } @@ -476,6 +490,106 @@ func (c *client) RevokeCollectionAccess(resource *Collection, user, role string) return c.removeMembership(groupIDInt, user) } +func (c *client) GrantTableAccess(resource *Table, email, role string, groups map[string]*Group) error { + access, err := c.getDatabaseAccess() + if err != nil { + return err + } + + var dbPermission databasePermission + resourceIDStr := strconv.Itoa(resource.ID) + databaseId := resource.DbId + databaseIdStr := strconv.Itoa(databaseId) + if role == TableRoleViewer { + dbPermission = map[string]interface{}{ + "schemas": map[string]interface{}{ + "public": map[string]interface{}{ + resourceIDStr: "all", + }, + }, + } + } else { + return ErrInvalidRole + } + + toBrGroupName := fmt.Sprintf("%s_%v_%v_%s", ResourceTypeTable, databaseId, resource.ID, role) + groupID := c.findTableAccessGroup(access, databaseIdStr, dbPermission) + + if groupID == "" { + if g, ok := groups[toBrGroupName]; ok { + groupID = strconv.Itoa(g.ID) + } else { + g := &group{ + Name: toBrGroupName, + } + if err := c.createGroup(g); err != nil { + return err + } + + groupID = strconv.Itoa(g.ID) + } + + access.Groups[groupID] = map[string]databasePermission{} + access.Groups[groupID][databaseIdStr] = dbPermission + if err := c.updateDatabaseAccess(access); err != nil { + return err + } + } + + groupIDInt, err := strconv.Atoi(groupID) + if err != nil { + return err + } + + user, err := c.getUser(email) + if err != nil { + return err + } + + for _, groupId := range user.GroupIds { + if groupId == groupIDInt { + return nil + } + } + + return c.addGroupMember(groupIDInt, user.ID) +} + +func (c *client) RevokeTableAccess(resource *Table, user, role string) error { + access, err := c.getDatabaseAccess() + if err != nil { + return err + } + + var dbPermission databasePermission + resourceIDStr := strconv.Itoa(resource.ID) + databaseId := resource.DbId + databaseIdStr := strconv.Itoa(databaseId) + if role == TableRoleViewer { + dbPermission = map[string]interface{}{ + "schemas": map[string]interface{}{ + "public": map[string]interface{}{ + resourceIDStr: "all", + }, + }, + } + } else { + return ErrInvalidRole + } + + groupID := c.findTableAccessGroup(access, databaseIdStr, dbPermission) + + if groupID == "" { + return ErrPermissionNotFound + } + + groupIDInt, err := strconv.Atoi(groupID) + if err != nil { + return err + } + return c.removeMembership(groupIDInt, user) +} + func (c *client) GrantGroupAccess(groupID int, email string) error { user, err := c.getUser(email) if err != nil { @@ -484,7 +598,7 @@ func (c *client) GrantGroupAccess(groupID int, email string) error { for _, userGroupId := range user.GroupIds { if userGroupId == groupID { - c.logger.Warn(fmt.Sprintf("User %s is already member of group %s", email, groupID)) + c.logger.Warn(fmt.Sprintf("User %s is already member of group %d", email, groupID)) return nil } } @@ -716,6 +830,20 @@ func (c *client) findDatabaseAccessGroup(access *databaseGraph, resourceID strin return "" } +func (c *client) findTableAccessGroup(access *databaseGraph, databaseID string, role databasePermission) string { + expectedDatabasePermission := map[string]databasePermission{ + databaseID: role, + } + + for groupID, databasePermissions := range access.Groups { + if reflect.DeepEqual(databasePermissions, expectedDatabasePermission) { + return groupID + } + } + + return "" +} + func (c *client) newRequest(method, path string, body interface{}) (*http.Request, error) { u, err := c.baseURL.Parse(path) if err != nil { @@ -775,10 +903,7 @@ func (c *client) do(req *http.Request, v interface{}) (*http.Response, error) { } if v != nil { - byteData, _ := io.ReadAll(resp.Body) - a := string(byteData) - fmt.Println(a) - err = json.NewDecoder(bytes.NewReader(byteData)).Decode(v) + err = json.NewDecoder(resp.Body).Decode(v) } return resp, err } diff --git a/plugins/providers/metabase/config.go b/plugins/providers/metabase/config.go index fdeeec89e..85dbb4e95 100644 --- a/plugins/providers/metabase/config.go +++ b/plugins/providers/metabase/config.go @@ -11,8 +11,11 @@ import ( ) const ( - DatabaseRoleViewer = "schemas:all" - DatabaseRoleEditor = "native:write" + DatabaseRoleViewer = "schemas:all" + DatabaseRoleEditor = "native:write" + CollectionRoleViewer = "read" + CollectionRoleCurate = "write" + TableRoleViewer = "all" AccountTypeUser = "user" ) diff --git a/plugins/providers/metabase/provider.go b/plugins/providers/metabase/provider.go index 194ddf4a6..9ec962fc5 100644 --- a/plugins/providers/metabase/provider.go +++ b/plugins/providers/metabase/provider.go @@ -55,8 +55,9 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, resources := []*domain.Resource{} + var databases []*Database if _, ok := resourceTypes[ResourceTypeDatabase]; ok { - databases, err := client.GetDatabases() + databases, err = client.GetDatabases() if err != nil { return nil, err } @@ -65,15 +66,27 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, db.ProviderType = pc.Type db.ProviderURN = pc.URN resources = append(resources, db) + } + } - if _, ok := resourceTypes[ResourceTypeTable]; ok { - for _, t := range d.Tables { - t.Database = db - table := t.ToDomain() - table.ProviderType = pc.Type - table.ProviderURN = pc.URN - resources = append(resources, table) - } + if _, ok := resourceTypes[ResourceTypeTable]; ok { + if databases == nil { + databases, err = client.GetDatabases() + } + if err != nil { + return nil, err + } + for _, d := range databases { + db := d.ToDomain() + db.ProviderType = pc.Type + db.ProviderURN = pc.URN + + for _, t := range d.Tables { + t.Database = db + table := t.ToDomain() + table.ProviderType = pc.Type + table.ProviderURN = pc.URN + resources = append(resources, table) } } } @@ -148,6 +161,16 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, return resources, nil } +func (p *provider) addTable(pc *domain.ProviderConfig, resourceTypes map[string]bool, d *Database, db *domain.Resource, resources []*domain.Resource) { + for _, t := range d.Tables { + t.Database = db + table := t.ToDomain() + table.ProviderType = pc.Type + table.ProviderURN = pc.URN + resources = append(resources, table) + } +} + func (p *provider) GrantAccess(pc *domain.ProviderConfig, a *domain.Appeal) error { // TODO: validate provider config and appeal @@ -210,7 +233,19 @@ func (p *provider) GrantAccess(pc *domain.ProviderConfig, a *domain.Appeal) erro if err := client.GrantGroupAccess(g.ID, a.AccountID); err != nil { return err } + return nil + } else if a.Resource.Type == ResourceTypeTable { + t := new(Table) + if err := t.FromDomain(a.Resource); err != nil { + return err + } + for _, p := range permissions { + if err := client.GrantTableAccess(t, a.AccountID, string(p), groupMap); err != nil { + return err + } + } + return nil } return ErrInvalidResourceType @@ -267,6 +302,18 @@ func (p *provider) RevokeAccess(pc *domain.ProviderConfig, a *domain.Appeal) err return err } + return nil + } else if a.Resource.Type == ResourceTypeTable { + t := new(Table) + if err := t.FromDomain(a.Resource); err != nil { + return err + } + + for _, p := range permissions { + if err := client.RevokeTableAccess(t, a.AccountID, string(p)); err != nil { + return err + } + } return nil } @@ -315,13 +362,18 @@ func getPermissions(resourceConfigs []*domain.ResourceConfig, a *domain.Appeal) return nil, ErrInvalidResourceType } - var role *domain.Role - for _, r := range resourceConfig.Roles { - if r.ID == a.Role { + roles := resourceConfig.Roles + role := &domain.Role{} + isRoleExists := len(roles) == 0 + for _, r := range roles { + if a.Role == r.ID { + isRoleExists = true role = r + break } } - if role == nil { + + if !isRoleExists { return nil, ErrInvalidRole } diff --git a/plugins/providers/metabase/provider_test.go b/plugins/providers/metabase/provider_test.go index 67df74ec2..56853060b 100644 --- a/plugins/providers/metabase/provider_test.go +++ b/plugins/providers/metabase/provider_test.go @@ -158,8 +158,8 @@ func TestGetResources(t *testing.T) { group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} client.On("GetGroups").Return([]*metabase.Group{&group}, - map[string][]map[string]interface{}{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, - map[string][]map[string]interface{}{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() + metabase.ResourceGroupDetails{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, + metabase.ResourceGroupDetails{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() expectedCollections := []*metabase.Collection{ { @@ -423,8 +423,8 @@ func TestGrantAccess(t *testing.T) { c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} client.On("GetGroups").Return([]*metabase.Group{&group}, - map[string][]map[string]interface{}{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, - map[string][]map[string]interface{}{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() + metabase.ResourceGroupDetails{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, + metabase.ResourceGroupDetails{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() pc := &domain.ProviderConfig{ Credentials: metabase.Credentials{ @@ -480,8 +480,8 @@ func TestGrantAccess(t *testing.T) { c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} client.On("GetGroups").Return([]*metabase.Group{&group}, - map[string][]map[string]interface{}{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, - map[string][]map[string]interface{}{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() + metabase.ResourceGroupDetails{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, + metabase.ResourceGroupDetails{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() pc := &domain.ProviderConfig{ Credentials: metabase.Credentials{ @@ -537,8 +537,8 @@ func TestGrantAccess(t *testing.T) { c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} client.On("GetGroups").Return([]*metabase.Group{&group}, - map[string][]map[string]interface{}{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, - map[string][]map[string]interface{}{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() + metabase.ResourceGroupDetails{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, + metabase.ResourceGroupDetails{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() pc := &domain.ProviderConfig{ Credentials: metabase.Credentials{ @@ -595,8 +595,8 @@ func TestGrantAccess(t *testing.T) { c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} client.On("GetGroups").Return([]*metabase.Group{&group}, - map[string][]map[string]interface{}{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, - map[string][]map[string]interface{}{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() + metabase.ResourceGroupDetails{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, + metabase.ResourceGroupDetails{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() pc := &domain.ProviderConfig{ Credentials: metabase.Credentials{ diff --git a/plugins/providers/metabase/resource.go b/plugins/providers/metabase/resource.go index a60b016ca..1f424139a 100644 --- a/plugins/providers/metabase/resource.go +++ b/plugins/providers/metabase/resource.go @@ -2,6 +2,7 @@ package metabase import ( "fmt" + "github.com/mitchellh/mapstructure" "strconv" "strings" @@ -101,7 +102,7 @@ func (t *Table) FromDomain(r *domain.Resource) error { t.ID = id t.Name = r.Name - t.DbId, err = strconv.Atoi(tableURN[1]) + t.DbId, err = strconv.Atoi(tableURN[0]) if err != nil { return err } @@ -133,8 +134,8 @@ func (g *Group) FromDomain(r *domain.Resource) error { g.ID = id g.Name = r.Name - g.DatabaseResources = r.Details["database"].([]*GroupResource) - g.CollectionResources = r.Details["collection"].([]*GroupResource) + _ = mapstructure.Decode(r.Details["database"], &g.DatabaseResources) + _ = mapstructure.Decode(r.Details["collection"], &g.CollectionResources) if err != nil { return err } From f8e6386a5e7986ac89b8015216cd2a91984b9b80 Mon Sep 17 00:00:00 2001 From: Vikash Date: Tue, 10 May 2022 17:39:26 +0530 Subject: [PATCH 09/17] Fix test --- plugins/providers/metabase/provider_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/providers/metabase/provider_test.go b/plugins/providers/metabase/provider_test.go index 56853060b..0be54458a 100644 --- a/plugins/providers/metabase/provider_test.go +++ b/plugins/providers/metabase/provider_test.go @@ -192,7 +192,6 @@ func TestGetResources(t *testing.T) { "auto_run_queries": false, "cache_field_values_schedule": "", "engine": "", - "groups": []map[string]interface{}{{"urn": "group:1", "permissions": []string{"read", "write"}}}, "metadata_sync_schedule": "", "native_permissions": "", "timezone": "", From eda6c8c6b4100721a8a6c5ab277fcf76c1e6143f Mon Sep 17 00:00:00 2001 From: Vikash Date: Tue, 10 May 2022 17:43:22 +0530 Subject: [PATCH 10/17] Fix lint --- plugins/providers/metabase/provider.go | 10 ---------- plugins/providers/metabase/resource.go | 3 ++- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/plugins/providers/metabase/provider.go b/plugins/providers/metabase/provider.go index 9ec962fc5..3cb120e43 100644 --- a/plugins/providers/metabase/provider.go +++ b/plugins/providers/metabase/provider.go @@ -161,16 +161,6 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, return resources, nil } -func (p *provider) addTable(pc *domain.ProviderConfig, resourceTypes map[string]bool, d *Database, db *domain.Resource, resources []*domain.Resource) { - for _, t := range d.Tables { - t.Database = db - table := t.ToDomain() - table.ProviderType = pc.Type - table.ProviderURN = pc.URN - resources = append(resources, table) - } -} - func (p *provider) GrantAccess(pc *domain.ProviderConfig, a *domain.Appeal) error { // TODO: validate provider config and appeal diff --git a/plugins/providers/metabase/resource.go b/plugins/providers/metabase/resource.go index 1f424139a..5ac642d1a 100644 --- a/plugins/providers/metabase/resource.go +++ b/plugins/providers/metabase/resource.go @@ -2,10 +2,11 @@ package metabase import ( "fmt" - "github.com/mitchellh/mapstructure" "strconv" "strings" + "github.com/mitchellh/mapstructure" + "github.com/odpf/guardian/domain" ) From 76c6510446e48276c88335dff2570f0f2768ef34 Mon Sep 17 00:00:00 2001 From: Vikash Date: Thu, 12 May 2022 11:51:12 +0530 Subject: [PATCH 11/17] Resolve feedback and add fetch database/collection while fetching groups --- docs/docs/providers/metabase.md | 2 + plugins/providers/metabase/client.go | 4 +- plugins/providers/metabase/config.go | 4 + plugins/providers/metabase/provider.go | 114 ++++++++++++++++--------- 4 files changed, 80 insertions(+), 44 deletions(-) diff --git a/docs/docs/providers/metabase.md b/docs/docs/providers/metabase.md index 276cb9c48..2547865e0 100644 --- a/docs/docs/providers/metabase.md +++ b/docs/docs/providers/metabase.md @@ -85,7 +85,9 @@ resources: ### `MetabaseResourceType` - `database` +- `table` - `collection` +- `group` ### `MetabaseResourcePermission` diff --git a/plugins/providers/metabase/client.go b/plugins/providers/metabase/client.go index 2c00fb6f2..ba974f223 100644 --- a/plugins/providers/metabase/client.go +++ b/plugins/providers/metabase/client.go @@ -123,10 +123,10 @@ type client struct { userIDs map[string]int - logger *log.Logrus + logger log.Logger } -func NewClient(config *ClientConfig, logger *log.Logrus) (*client, error) { +func NewClient(config *ClientConfig, logger log.Logger) (*client, error) { if err := validator.New().Struct(config); err != nil { return nil, err } diff --git a/plugins/providers/metabase/config.go b/plugins/providers/metabase/config.go index 85dbb4e95..1749386b9 100644 --- a/plugins/providers/metabase/config.go +++ b/plugins/providers/metabase/config.go @@ -173,6 +173,10 @@ func (c *Config) validatePermission(resourceType string, value interface{}) (*Pe nameValidation = "oneof=schemas:all native:write" } else if resourceType == ResourceTypeCollection { nameValidation = "oneof=read write" + } else if resourceType == ResourceTypeTable { + nameValidation = "oneof=all" + } else if resourceType == ResourceTypeGroup { + nameValidation = "len=0" } if err := c.validator.Var(pc, nameValidation); err != nil { diff --git a/plugins/providers/metabase/provider.go b/plugins/providers/metabase/provider.go index 3cb120e43..2a8e43343 100644 --- a/plugins/providers/metabase/provider.go +++ b/plugins/providers/metabase/provider.go @@ -11,7 +11,7 @@ type provider struct { typeName string Clients map[string]MetabaseClient crypto domain.Crypto - logger *log.Logrus + logger log.Logger } func NewProvider(typeName string, crypto domain.Crypto, logger *log.Logrus) *provider { @@ -56,17 +56,13 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, resources := []*domain.Resource{} var databases []*Database + var collections []*Collection if _, ok := resourceTypes[ResourceTypeDatabase]; ok { databases, err = client.GetDatabases() if err != nil { return nil, err } - for _, d := range databases { - db := d.ToDomain() - db.ProviderType = pc.Type - db.ProviderURN = pc.URN - resources = append(resources, db) - } + resources = p.addDatabases(pc, databases, resources) } if _, ok := resourceTypes[ResourceTypeTable]; ok { @@ -76,32 +72,15 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, if err != nil { return nil, err } - for _, d := range databases { - db := d.ToDomain() - db.ProviderType = pc.Type - db.ProviderURN = pc.URN - - for _, t := range d.Tables { - t.Database = db - table := t.ToDomain() - table.ProviderType = pc.Type - table.ProviderURN = pc.URN - resources = append(resources, table) - } - } + resources = p.addTables(pc, databases, resources) } if _, ok := resourceTypes[ResourceTypeCollection]; ok { - collections, err := client.GetCollections() + collections, err = client.GetCollections() if err != nil { return nil, err } - for _, c := range collections { - db := c.ToDomain() - db.ProviderType = pc.Type - db.ProviderURN = pc.URN - resources = append(resources, db) - } + resources = p.addCollection(pc, collections, resources) } groups, databaseResourceGroups, collectionResourceGroups, err := client.GetGroups() @@ -109,17 +88,6 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, return nil, err } - databaseResourceMap := make(map[string]*domain.Resource, 0) - collectionResourceMap := make(map[string]*domain.Resource, 0) - for _, resource := range resources { - if resource.Type == ResourceTypeDatabase || resource.Type == ResourceTypeTable { - databaseResourceMap[resource.URN] = resource - } - if resource.Type == ResourceTypeCollection { - collectionResourceMap[resource.URN] = resource - } - } - for _, resource := range resources { if resource.Type == ResourceTypeDatabase || resource.Type == ResourceTypeTable { if groups, ok := databaseResourceGroups[resource.URN]; ok { @@ -134,6 +102,31 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, } if _, ok := resourceTypes[ResourceTypeGroup]; ok && resourceTypes[ResourceTypeGroup] { + databaseResourceMap := make(map[string]*domain.Resource, 0) + collectionResourceMap := make(map[string]*domain.Resource, 0) + + if databases == nil { + databases, err = client.GetDatabases() + if err != nil { + return nil, err + } + } + for _, database := range databases { + resource := database.ToDomain() + databaseResourceMap[resource.URN] = resource + } + + if collections == nil { + collections, err = client.GetCollections() + if err != nil { + return nil, err + } + } + for _, collection := range collections { + resource := collection.ToDomain() + collectionResourceMap[resource.URN] = resource + } + for _, g := range groups { for _, groupResource := range g.DatabaseResources { resourceId := groupResource.Urn @@ -151,16 +144,53 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, } } - db := g.ToDomain() - db.ProviderType = pc.Type - db.ProviderURN = pc.URN - resources = append(resources, db) + group := g.ToDomain() + group.ProviderType = pc.Type + group.ProviderURN = pc.URN + resources = append(resources, group) } } return resources, nil } +func (p *provider) addCollection(pc *domain.ProviderConfig, collections []*Collection, resources []*domain.Resource) []*domain.Resource { + for _, c := range collections { + db := c.ToDomain() + db.ProviderType = pc.Type + db.ProviderURN = pc.URN + resources = append(resources, db) + } + return resources +} + +func (p *provider) addDatabases(pc *domain.ProviderConfig, databases []*Database, resources []*domain.Resource) []*domain.Resource { + for _, d := range databases { + db := d.ToDomain() + db.ProviderType = pc.Type + db.ProviderURN = pc.URN + resources = append(resources, db) + } + return resources +} + +func (p *provider) addTables(pc *domain.ProviderConfig, databases []*Database, resources []*domain.Resource) []*domain.Resource { + for _, d := range databases { + db := d.ToDomain() + db.ProviderType = pc.Type + db.ProviderURN = pc.URN + + for _, t := range d.Tables { + t.Database = db + table := t.ToDomain() + table.ProviderType = pc.Type + table.ProviderURN = pc.URN + resources = append(resources, table) + } + } + return resources +} + func (p *provider) GrantAccess(pc *domain.ProviderConfig, a *domain.Appeal) error { // TODO: validate provider config and appeal From 5d01b7c9ae0ef8ddc64a5745f81a208b0b4be9cf Mon Sep 17 00:00:00 2001 From: Vikash Date: Thu, 12 May 2022 16:09:08 +0530 Subject: [PATCH 12/17] fix: custom group naming convention for metabase provider and filter groups created by guardian --- plugins/providers/metabase/client.go | 1 + plugins/providers/metabase/provider.go | 6 ++++++ plugins/providers/metabase/provider_test.go | 2 +- plugins/providers/metabase/resource.go | 2 ++ 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/plugins/providers/metabase/client.go b/plugins/providers/metabase/client.go index ba974f223..970c4d154 100644 --- a/plugins/providers/metabase/client.go +++ b/plugins/providers/metabase/client.go @@ -736,6 +736,7 @@ func (c *client) updateDatabaseAccess(dbGraph *databaseGraph) error { } func (c *client) createGroup(group *group) error { + group.Name = GuardianGroupPrefix + group.Name req, err := c.newRequest(http.MethodPost, groupEndpoint, group) if err != nil { return err diff --git a/plugins/providers/metabase/provider.go b/plugins/providers/metabase/provider.go index 2a8e43343..bedbfdbae 100644 --- a/plugins/providers/metabase/provider.go +++ b/plugins/providers/metabase/provider.go @@ -1,6 +1,8 @@ package metabase import ( + "strings" + "github.com/mitchellh/mapstructure" pv "github.com/odpf/guardian/core/provider" "github.com/odpf/guardian/domain" @@ -128,6 +130,10 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, } for _, g := range groups { + if strings.HasPrefix(g.Name, GuardianGroupPrefix) { + continue + } + for _, groupResource := range g.DatabaseResources { resourceId := groupResource.Urn if resource, ok := databaseResourceMap[resourceId]; ok { diff --git a/plugins/providers/metabase/provider_test.go b/plugins/providers/metabase/provider_test.go index 0be54458a..1c513e9c8 100644 --- a/plugins/providers/metabase/provider_test.go +++ b/plugins/providers/metabase/provider_test.go @@ -157,7 +157,7 @@ func TestGetResources(t *testing.T) { c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} - client.On("GetGroups").Return([]*metabase.Group{&group}, + client.On("GetGroups").Return([]*metabase.Group{&group, {Name: metabase.GuardianGroupPrefix + "database_1_schema:all", DatabaseResources: d, CollectionResources: c}}, metabase.ResourceGroupDetails{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, metabase.ResourceGroupDetails{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() diff --git a/plugins/providers/metabase/resource.go b/plugins/providers/metabase/resource.go index 5ac642d1a..57de8b6c7 100644 --- a/plugins/providers/metabase/resource.go +++ b/plugins/providers/metabase/resource.go @@ -15,6 +15,8 @@ const ( ResourceTypeTable = "table" ResourceTypeCollection = "collection" ResourceTypeGroup = "group" + + GuardianGroupPrefix = "_guardian_" ) type Database struct { From 1268389983b2a52e605855656d453f4b815ec14b Mon Sep 17 00:00:00 2001 From: Vikash Date: Thu, 12 May 2022 16:09:08 +0530 Subject: [PATCH 13/17] fix: custom group naming convention for metabase provider and filter groups created by guardian --- plugins/providers/metabase/client.go | 5 +++++ plugins/providers/metabase/provider.go | 6 ++++++ plugins/providers/metabase/provider_test.go | 2 +- plugins/providers/metabase/resource.go | 2 ++ 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/plugins/providers/metabase/client.go b/plugins/providers/metabase/client.go index ba974f223..fab202b4b 100644 --- a/plugins/providers/metabase/client.go +++ b/plugins/providers/metabase/client.go @@ -10,6 +10,7 @@ import ( "net/url" "reflect" "strconv" + "strings" "sync" "github.com/odpf/salt/log" @@ -308,6 +309,9 @@ func addResourceToGroup(resourceGroups ResourceGroupDetails, groupMap map[string for _, groupDetails := range groups { groupID := groupDetails[urn].(string) if group, ok := groupMap[groupID]; ok { + if strings.HasPrefix(group.Name, GuardianGroupPrefix) { + continue + } groupDetails[name] = group.Name if resourceType == database { group.DatabaseResources = append(group.DatabaseResources, &GroupResource{Urn: resourceId, Permissions: groupDetails[permissionsConst].([]string)}) @@ -736,6 +740,7 @@ func (c *client) updateDatabaseAccess(dbGraph *databaseGraph) error { } func (c *client) createGroup(group *group) error { + group.Name = GuardianGroupPrefix + group.Name req, err := c.newRequest(http.MethodPost, groupEndpoint, group) if err != nil { return err diff --git a/plugins/providers/metabase/provider.go b/plugins/providers/metabase/provider.go index 2a8e43343..bedbfdbae 100644 --- a/plugins/providers/metabase/provider.go +++ b/plugins/providers/metabase/provider.go @@ -1,6 +1,8 @@ package metabase import ( + "strings" + "github.com/mitchellh/mapstructure" pv "github.com/odpf/guardian/core/provider" "github.com/odpf/guardian/domain" @@ -128,6 +130,10 @@ func (p *provider) GetResources(pc *domain.ProviderConfig) ([]*domain.Resource, } for _, g := range groups { + if strings.HasPrefix(g.Name, GuardianGroupPrefix) { + continue + } + for _, groupResource := range g.DatabaseResources { resourceId := groupResource.Urn if resource, ok := databaseResourceMap[resourceId]; ok { diff --git a/plugins/providers/metabase/provider_test.go b/plugins/providers/metabase/provider_test.go index 0be54458a..1c513e9c8 100644 --- a/plugins/providers/metabase/provider_test.go +++ b/plugins/providers/metabase/provider_test.go @@ -157,7 +157,7 @@ func TestGetResources(t *testing.T) { c := []*metabase.GroupResource{{Urn: "collection:1", Permissions: []string{"read", "write"}}} group := metabase.Group{Name: "All Users", DatabaseResources: d, CollectionResources: c} - client.On("GetGroups").Return([]*metabase.Group{&group}, + client.On("GetGroups").Return([]*metabase.Group{&group, {Name: metabase.GuardianGroupPrefix + "database_1_schema:all", DatabaseResources: d, CollectionResources: c}}, metabase.ResourceGroupDetails{"database:1": {{"urn": "group:1", "permissions": []string{"read", "write"}}}}, metabase.ResourceGroupDetails{"collection:1": {{"urn": "group:1", "permissions": []string{"write"}}}}, nil).Once() diff --git a/plugins/providers/metabase/resource.go b/plugins/providers/metabase/resource.go index 5ac642d1a..57de8b6c7 100644 --- a/plugins/providers/metabase/resource.go +++ b/plugins/providers/metabase/resource.go @@ -15,6 +15,8 @@ const ( ResourceTypeTable = "table" ResourceTypeCollection = "collection" ResourceTypeGroup = "group" + + GuardianGroupPrefix = "_guardian_" ) type Database struct { From 38ffeedf8e41df074e9f217fea319f04ad49c346 Mon Sep 17 00:00:00 2001 From: Vikash Date: Wed, 18 May 2022 10:23:34 +0530 Subject: [PATCH 14/17] fix: update the log to use key/value pair logging --- plugins/providers/metabase/client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/providers/metabase/client.go b/plugins/providers/metabase/client.go index fab202b4b..63e1f4aca 100644 --- a/plugins/providers/metabase/client.go +++ b/plugins/providers/metabase/client.go @@ -185,7 +185,7 @@ func (c *client) GetDatabases() ([]*Database, error) { if err != nil { return databases, err } - c.logger.Info(fmt.Sprintf("Fetch total %d database from request: %v", len(databases), req.URL)) + c.logger.Info("Fetch database from request", "total", len(databases), req.URL) return databases, err } @@ -199,7 +199,7 @@ func (c *client) GetCollections() ([]*Collection, error) { if _, err := c.do(req, &collection); err != nil { return nil, err } - c.logger.Info(fmt.Sprintf("Fetch total %d collections from request: %v", len(collection), req.URL)) + c.logger.Info("Fetch collections from request", "total", len(collection), req.URL) return collection, nil } @@ -241,7 +241,7 @@ func (c *client) fetchGroups(wg *sync.WaitGroup, groups *[]*Group, err error) { if err != nil { return } - c.logger.Info(fmt.Sprintf("Fetch total %d groups from request: %v", len(*groups), req.URL)) + c.logger.Info("Fetch groups from request", "total", len(*groups), req.URL) } func (c *client) fetchDatabasePermissions(wg *sync.WaitGroup, resourceGroups ResourceGroupDetails, err error) { From 158ea994351e5c0a824d897b2dc6ec26a0819809 Mon Sep 17 00:00:00 2001 From: Vikash Date: Thu, 19 May 2022 17:14:35 +0530 Subject: [PATCH 15/17] feat: initial implementation of metabase group migration into guardian --- cmd/migration.go | 131 +++++++++++ cmd/root.go | 3 + plugins/migrations/client.go | 17 ++ plugins/migrations/metabase/client.go | 282 +++++++++++++++++++++++ plugins/migrations/metabase/http.go | 7 + plugins/migrations/metabase/migration.go | 91 ++++++++ 6 files changed, 531 insertions(+) create mode 100644 cmd/migration.go create mode 100644 plugins/migrations/client.go create mode 100644 plugins/migrations/metabase/client.go create mode 100644 plugins/migrations/metabase/http.go create mode 100644 plugins/migrations/metabase/migration.go diff --git a/cmd/migration.go b/cmd/migration.go new file mode 100644 index 000000000..73f9a20b0 --- /dev/null +++ b/cmd/migration.go @@ -0,0 +1,131 @@ +package cmd + +import ( + "fmt" + "github.com/MakeNowJust/heredoc" + guardianv1beta1 "github.com/odpf/guardian/api/proto/odpf/guardian/v1beta1" + "github.com/odpf/guardian/domain" + "github.com/odpf/guardian/internal/crypto" + "github.com/odpf/guardian/plugins/migrations/metabase" + "github.com/spf13/cobra" + "google.golang.org/protobuf/types/known/structpb" +) + +func MigrationCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "migration", + Aliases: []string{"migration"}, + Short: "Manage migration", + Long: heredoc.Doc(` + Work with migrations. + + Migrations are used to populate guardian appeals from target system`), + Example: heredoc.Doc(` + $ guardian migration metabase + `), + Annotations: map[string]string{ + "group:core": "true", + }, + } + + cmd.AddCommand(metabaseMigrationCmd()) + bindFlagsFromConfig(cmd) + + return cmd +} + +func metabaseMigrationCmd() *cobra.Command { + return &cobra.Command{ + Use: "metabase", + Short: "Metabase migration", + Long: heredoc.Doc(` + List and filter all available access policies. + `), + Example: heredoc.Doc(` + $ guardian migration metabase + `), + Annotations: map[string]string{ + "group:core": "true", + }, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + providerId := args[0] + policyId := args[1] + fmt.Println(providerId, policyId) + + say := crypto.NewAES("") + client, cancel, err := createClient(cmd) + if err != nil { + return err + } + defer cancel() + + context := cmd.Context() + providerResponse, err := client.GetProvider(context, &guardianv1beta1.GetProviderRequest{Id: providerId}) + + provider := providerResponse.GetProvider() + providerUrn := provider.Urn + fields := provider.Config.Credentials.GetStructValue().GetFields() + abc, err := say.Decrypt(fields["password"].GetStringValue()) + + providerConfig := domain.ProviderConfig{ + Type: provider.Config.Type, + URN: provider.Config.Urn, + Credentials: map[string]string{ + "username": fields["username"].GetStringValue(), + "password": abc, + "host": fields["host"].GetStringValue(), + }, + Appeal: nil, + Resources: nil, + } + + listResources, err := client.ListResources(context, &guardianv1beta1.ListResourcesRequest{ProviderUrn: providerUrn, IsDeleted: false}) + resources := make([]domain.Resource, 0) + for _, r := range listResources.GetResources() { + resources = append(resources, domain.Resource{ + ID: r.Id, + ProviderType: r.ProviderType, + ProviderURN: r.ProviderUrn, + Type: r.Type, + URN: r.Urn, + Name: r.Name, + }) + } + + listAppeals, err := client.ListAppeals(context, &guardianv1beta1.ListAppealsRequest{ProviderUrns: []string{providerUrn}, Statuses: []string{"pending"}}) + + appeals := make([]domain.Appeal, 0) + for _, a := range listAppeals.GetAppeals() { + appeals = append(appeals, domain.Appeal{ + ID: a.Id, + ResourceID: a.ResourceId, + Status: a.Status, + AccountID: a.AccountId, + AccountType: a.AccountType, + Role: a.Role, + }) + } + + migration := metabase.NewMigration(providerConfig, resources, appeals) + appealRequests, err := migration.PopulateAccess() + + for _, appealRequest := range appealRequests { + resource := appealRequest.Resource + option, _ := structpb.NewStruct(map[string]interface{}{"duration": resource.Duration}) + + appeal, _ := client.CreateAppeal(context, &guardianv1beta1.CreateAppealRequest{ + AccountId: appealRequest.AccountID, + Resources: []*guardianv1beta1.CreateAppealRequest_Resource{{Id: resource.ID, + Options: option}}, + AccountType: "", + }) + + fmt.Println(appeal) + //Todo: iterate appeal's approval and approve it via #{client.UpdateApproval} + } + + return nil + }, + } +} diff --git a/cmd/root.go b/cmd/root.go index 114778cd8..95fc57a85 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -48,6 +48,9 @@ func New(cfg *Config) *cobra.Command { cmd.AddCommand(configCommand()) cmd.AddCommand(VersionCmd()) + //Migration command + cmd.AddCommand(MigrationCmd()) + // Help topics cmdx.SetHelp(cmd) cmd.AddCommand(cmdx.SetCompletionCmd("guardian")) diff --git a/plugins/migrations/client.go b/plugins/migrations/client.go new file mode 100644 index 000000000..93bb4903d --- /dev/null +++ b/plugins/migrations/client.go @@ -0,0 +1,17 @@ +package migrations + +type ResourceRequest struct { + ID string + Duration string +} + +type AppealRequest struct { + AccountID string + User string + Resource ResourceRequest +} + +type Client interface { + GetType() string + PopulateAccess() ([]AppealRequest, error) +} diff --git a/plugins/migrations/metabase/client.go b/plugins/migrations/metabase/client.go new file mode 100644 index 000000000..582cad292 --- /dev/null +++ b/plugins/migrations/metabase/client.go @@ -0,0 +1,282 @@ +package metabase + +import ( + "bytes" + "encoding/json" + "fmt" + "github.com/go-playground/validator/v10" + "io" + "net/http" + "net/url" +) + +type ClientConfig struct { + Host string `validate:"required,url" mapstructure:"host"` + Username string `validate:"required" mapstructure:"username"` + Password string `validate:"required" mapstructure:"password"` + HTTPClient HTTPClient +} + +type user struct { + ID int `json:"id"` + Email string `json:"email"` + MembershipID int `json:"membership_id"` +} + +type group struct { + ID int `json:"id,omitempty"` + Name string `json:"name"` + Members []user `json:"members"` +} + +type member struct { + MembershipId int `json:"membership_id,omitempty"` + GroupId int `json:"group_id"` + UserId int `json:"user_id"` +} + +type SessionRequest struct { + Username string `json:"username"` + Password string `json:"password"` +} + +type SessionResponse struct { + ID string `json:"id"` +} + +type databasePermission struct { + Native string `json:"native,omitempty" mapstructure:"native"` + Schemas string `json:"schemas" mapstructure:"schemas"` +} + +type databaseGraph struct { + Revision int `json:"revision"` + // Groups is a map[group_id]map[database_id]databasePermission + Groups map[string]map[string]databasePermission `json:"groups"` +} + +type collectionGraph struct { + Revision int `json:"revision"` + // Groups is a map[group_id]map[database_id]role string + Groups map[string]map[string]string `json:"groups"` +} + +type membershipRequest struct { + GroupID int `json:"group_id"` + UserID int `json:"user_id"` +} + +var ( + databaseViewerPermission = databasePermission{ + Schemas: "all", + } + databaseEditorPermission = databasePermission{ + Schemas: "all", + Native: "write", + } +) + +type client struct { + baseURL *url.URL + + username string + password string + sessionToken string + + httpClient HTTPClient + + userIDs map[string]int +} + +func NewClient(config *ClientConfig) (*client, error) { + if err := validator.New().Struct(config); err != nil { + return nil, err + } + + baseURL, err := url.Parse(config.Host) + if err != nil { + return nil, err + } + + httpClient := config.HTTPClient + if httpClient == nil { + httpClient = &http.Client{} + } + + c := &client{ + baseURL: baseURL, + username: config.Username, + password: config.Password, + httpClient: httpClient, + userIDs: map[string]int{}, + } + + sessionToken, err := c.getSessionToken() + if err != nil { + return nil, err + } + c.sessionToken = sessionToken + + return c, nil +} + +func (c *client) getUsers() ([]user, error) { + req, err := c.newRequest(http.MethodGet, "/api/user", nil) + if err != nil { + return nil, err + } + + var users []user + if _, err := c.do(req, &users); err != nil { + return nil, err + } + + return users, nil +} + +func (c *client) getGroups() ([]group, error) { + req, err := c.newRequest(http.MethodGet, "/api/permissions/group", nil) + if err != nil { + return nil, err + } + + var groups []group + + if _, err := c.do(req, &groups); err != nil { + return nil, err + } + + return groups, nil +} + +func (c *client) getMembership() (map[string][]member, error) { + req, err := c.newRequest(http.MethodGet, "/api/permissions/membership", nil) + if err != nil { + return nil, err + } + + var members map[string][]member + + if _, err := c.do(req, &members); err != nil { + return nil, err + } + + return members, nil +} + +func (c *client) getSessionToken() (string, error) { + sessionRequest := &SessionRequest{ + Username: c.username, + Password: c.password, + } + req, err := c.newRequest(http.MethodPost, "/api/session", sessionRequest) + if err != nil { + return "", err + } + + var sessionResponse SessionResponse + if _, err := c.do(req, &sessionResponse); err != nil { + return "", err + } + + return sessionResponse.ID, nil +} + +func (c *client) getCollectionAccess() (*collectionGraph, error) { + req, err := c.newRequest(http.MethodGet, "/api/collection/graph", nil) + if err != nil { + return nil, err + } + + var graph collectionGraph + if _, err := c.do(req, &graph); err != nil { + return nil, err + } + + return &graph, nil +} + +func (c *client) getDatabaseAccess() (*databaseGraph, error) { + req, err := c.newRequest(http.MethodGet, "/api/permissions/graph", nil) + if err != nil { + return nil, err + } + + var dbGraph databaseGraph + if _, err := c.do(req, &dbGraph); err != nil { + return nil, err + } + + return &dbGraph, nil +} + +func (c *client) getGroup(id int) (*group, error) { + url := fmt.Sprintf("/api/permissions/group/%d", id) + req, err := c.newRequest(http.MethodGet, url, nil) + if err != nil { + return nil, err + } + + var group group + + if _, err := c.do(req, &group); err != nil { + return nil, err + } + + return &group, nil +} + +func (c *client) newRequest(method, path string, body interface{}) (*http.Request, error) { + u, err := c.baseURL.Parse(path) + if err != nil { + return nil, err + } + var buf io.ReadWriter + if body != nil { + buf = new(bytes.Buffer) + err := json.NewEncoder(buf).Encode(body) + if err != nil { + return nil, err + } + } + req, err := http.NewRequest(method, u.String(), buf) + if err != nil { + return nil, err + } + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + req.Header.Set("Accept", "application/json") + req.Header.Set("X-Metabase-Session", c.sessionToken) + return req, nil +} + +func (c *client) do(req *http.Request, v interface{}) (*http.Response, error) { + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusUnauthorized { + newSessionToken, err := c.getSessionToken() + if err != nil { + return nil, err + } + c.sessionToken = newSessionToken + req.Header.Set("X-Metabase-Session", c.sessionToken) + + // re-do the request + resp, err = c.httpClient.Do(req) + if err != nil { + return nil, err + } + } + + if v != nil { + //all, _ := ioutil.ReadAll(resp.Body) + //fmt.Println(string(all)) + err = json.NewDecoder(resp.Body).Decode(v) + } + return resp, err +} diff --git a/plugins/migrations/metabase/http.go b/plugins/migrations/metabase/http.go new file mode 100644 index 000000000..0cceaaca2 --- /dev/null +++ b/plugins/migrations/metabase/http.go @@ -0,0 +1,7 @@ +package metabase + +import "net/http" + +type HTTPClient interface { + Do(*http.Request) (*http.Response, error) +} diff --git a/plugins/migrations/metabase/migration.go b/plugins/migrations/metabase/migration.go new file mode 100644 index 000000000..14d754dbc --- /dev/null +++ b/plugins/migrations/metabase/migration.go @@ -0,0 +1,91 @@ +package metabase + +import ( + "fmt" + "github.com/odpf/guardian/domain" + . "github.com/odpf/guardian/plugins/migrations" + "strconv" +) + +type migration struct { + typeName string + providerConfig domain.ProviderConfig + resources []domain.Resource + pendingAppeals []domain.Appeal +} + +const typeName = "metabase" + +func NewMigration(providerConfig domain.ProviderConfig, resources []domain.Resource, pendingAppeals []domain.Appeal) *migration { + return &migration{ + typeName: typeName, + providerConfig: providerConfig, + resources: resources, + pendingAppeals: pendingAppeals, + } +} + +func (p *migration) GetType() string { + return p.typeName +} + +func (p *migration) PopulateAccess() ([]AppealRequest, error) { + resourceMap := make(map[string]domain.Resource, 0) + appealMap := make(map[string][]domain.Appeal, 0) + + for _, resource := range p.resources { + resourceMap[resource.URN] = resource + } + + for _, appeal := range p.pendingAppeals { + if m, ok := appealMap[appeal.ResourceID]; ok { + appealMap[appeal.ResourceID] = append(m, appeal) + } else { + appealMap[appeal.ResourceID] = append(make([]domain.Appeal, 0), appeal) + } + } + + credentials := p.providerConfig.Credentials.(map[string]string) + c, err := NewClient(&ClientConfig{ + Host: credentials["host"], + Username: credentials["username"], + Password: credentials["password"], + HTTPClient: nil, + }) + + userMap := make(map[int]user, 0) + groupMap := make(map[int]group, 0) + users, err := c.getUsers() + + for _, user := range users { + userMap[user.ID] = user + } + + groups, err := c.getGroups() + for _, group := range groups { + groupMap[group.ID] = group + } + + membership, err := c.getMembership() + appeals := make([]AppealRequest, 0) + for userID, members := range membership { + for _, m := range members { + userIdInt, _ := strconv.Atoi(userID) + if user, ok := userMap[userIdInt]; ok { + resourceUrn := fmt.Sprintf("group:%d", m.GroupId) + if resource, ok := resourceMap[resourceUrn]; ok { + if _, ok := appealMap[resource.ID]; !ok { + appeal := AppealRequest{ + AccountID: user.Email, + User: user.Email, + Resource: ResourceRequest{ID: resource.ID, Duration: "30d"}, + } + appeals = append(appeals, appeal) + } + } + } + + } + } + return appeals, err +} From 2dff81cb3c0f66949d98bcc31e121e248a1f09f7 Mon Sep 17 00:00:00 2001 From: Vikash Date: Mon, 23 May 2022 11:39:22 +0530 Subject: [PATCH 16/17] Update doc of Metabase resources and add `member` as role in group --- docs/docs/providers/metabase.md | 22 +++++++++++++++++++--- plugins/providers/metabase/config.go | 2 +- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/docs/providers/metabase.md b/docs/docs/providers/metabase.md index 2547865e0..3ac74e300 100644 --- a/docs/docs/providers/metabase.md +++ b/docs/docs/providers/metabase.md @@ -72,6 +72,22 @@ resources: name: Editor permissions: - write + - type: table + policy: + id: policy_id + version: 1 + roles: + - id: viewer + name: Viewer + permissions: + - all + - type: group + policy: + id: policy_id + version: 1 + roles: + - id: member + name: Member ``` ### `MetabaseCredentials` @@ -91,6 +107,6 @@ resources: ### `MetabaseResourcePermission` -| Type | Details | -| :----------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Required. `string` | Metabase permission mapping **Possible values:** - `database`: `schemas:all` \(read table\), `native:write` \(run SQL query\) **Note**: Metabase requires `schemas:all` permission for `native:write` to be able to work - `collection`: `read`, `write` | +| Type | Details | +| :----------------- |:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| Required. `string` | Metabase permission mapping **Possible values:** - `database`: `schemas:all` \(read table\), `native:write` \(run SQL query\) **Note**: Metabase requires `schemas:all` permission for `native:write` to be able to work - `collection`: `read`, `write` **Note**: Metabase table requires `all` permission to read table, no write permission on table level **Note**: Metabase group requires no specific permission to be a member of group || diff --git a/plugins/providers/metabase/config.go b/plugins/providers/metabase/config.go index 1749386b9..794564665 100644 --- a/plugins/providers/metabase/config.go +++ b/plugins/providers/metabase/config.go @@ -176,7 +176,7 @@ func (c *Config) validatePermission(resourceType string, value interface{}) (*Pe } else if resourceType == ResourceTypeTable { nameValidation = "oneof=all" } else if resourceType == ResourceTypeGroup { - nameValidation = "len=0" + nameValidation = "oneof=member" } if err := c.validator.Var(pc, nameValidation); err != nil { From 55af1a2342d580c50bba784e5ca93c1f7cb61bbf Mon Sep 17 00:00:00 2001 From: Vikash Date: Mon, 23 May 2022 17:21:25 +0530 Subject: [PATCH 17/17] Add migrate command to migrate access for a given provider --- cmd/config.go | 3 +- cmd/migration.go | 237 +++++++++++++++-------- cmd/root.go | 2 +- plugins/migrations/client.go | 13 ++ plugins/migrations/metabase/client.go | 86 +------- plugins/migrations/metabase/migration.go | 50 ++--- 6 files changed, 199 insertions(+), 192 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 8549307f3..42ea2846a 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -11,7 +11,8 @@ import ( var cliConfig *Config type Config struct { - Host string `mapstructure:"host"` + Host string `mapstructure:"host"` + EncryptionSecretKey string `mapstructure:"encryption_secret_key"` } func LoadConfig() (*Config, error) { diff --git a/cmd/migration.go b/cmd/migration.go index 73f9a20b0..76c2ca791 100644 --- a/cmd/migration.go +++ b/cmd/migration.go @@ -1,59 +1,43 @@ package cmd import ( + "context" + "errors" "fmt" + "github.com/MakeNowJust/heredoc" - guardianv1beta1 "github.com/odpf/guardian/api/proto/odpf/guardian/v1beta1" + . "github.com/odpf/guardian/api/proto/odpf/guardian/v1beta1" "github.com/odpf/guardian/domain" "github.com/odpf/guardian/internal/crypto" - "github.com/odpf/guardian/plugins/migrations/metabase" + "github.com/odpf/guardian/plugins/migrations" + mb "github.com/odpf/guardian/plugins/migrations/metabase" "github.com/spf13/cobra" "google.golang.org/protobuf/types/known/structpb" ) -func MigrationCmd() *cobra.Command { - cmd := &cobra.Command{ - Use: "migration", - Aliases: []string{"migration"}, - Short: "Manage migration", - Long: heredoc.Doc(` - Work with migrations. - - Migrations are used to populate guardian appeals from target system`), - Example: heredoc.Doc(` - $ guardian migration metabase - `), - Annotations: map[string]string{ - "group:core": "true", - }, - } - - cmd.AddCommand(metabaseMigrationCmd()) - bindFlagsFromConfig(cmd) - - return cmd -} +const ( + pending = "pending" + active = "active" +) -func metabaseMigrationCmd() *cobra.Command { - return &cobra.Command{ - Use: "metabase", - Short: "Metabase migration", +func MigrationCmd(config *Config) *cobra.Command { + cmd := &cobra.Command{ + Use: "migrate", + Short: "Guardian migration", Long: heredoc.Doc(` - List and filter all available access policies. + Migrate target system ACL into Guardian. `), Example: heredoc.Doc(` - $ guardian migration metabase + $ guardian migration `), Annotations: map[string]string{ "group:core": "true", }, - Args: cobra.ExactArgs(2), + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { providerId := args[0] - policyId := args[1] - fmt.Println(providerId, policyId) - say := crypto.NewAES("") + say := crypto.NewAES(config.EncryptionSecretKey) client, cancel, err := createClient(cmd) if err != nil { return err @@ -61,71 +45,158 @@ func metabaseMigrationCmd() *cobra.Command { defer cancel() context := cmd.Context() - providerResponse, err := client.GetProvider(context, &guardianv1beta1.GetProviderRequest{Id: providerId}) - - provider := providerResponse.GetProvider() - providerUrn := provider.Urn - fields := provider.Config.Credentials.GetStructValue().GetFields() - abc, err := say.Decrypt(fields["password"].GetStringValue()) - - providerConfig := domain.ProviderConfig{ - Type: provider.Config.Type, - URN: provider.Config.Urn, - Credentials: map[string]string{ - "username": fields["username"].GetStringValue(), - "password": abc, - "host": fields["host"].GetStringValue(), - }, - Appeal: nil, - Resources: nil, + provider, err := getProviderConfig(client, context, providerId, say) + if err != nil { + return err } - listResources, err := client.ListResources(context, &guardianv1beta1.ListResourcesRequest{ProviderUrn: providerUrn, IsDeleted: false}) - resources := make([]domain.Resource, 0) - for _, r := range listResources.GetResources() { - resources = append(resources, domain.Resource{ - ID: r.Id, - ProviderType: r.ProviderType, - ProviderURN: r.ProviderUrn, - Type: r.Type, - URN: r.Urn, - Name: r.Name, - }) + resources, err := getResources(client, context, provider) + if err != nil { + return err } - listAppeals, err := client.ListAppeals(context, &guardianv1beta1.ListAppealsRequest{ProviderUrns: []string{providerUrn}, Statuses: []string{"pending"}}) - - appeals := make([]domain.Appeal, 0) - for _, a := range listAppeals.GetAppeals() { - appeals = append(appeals, domain.Appeal{ - ID: a.Id, - ResourceID: a.ResourceId, - Status: a.Status, - AccountID: a.AccountId, - AccountType: a.AccountType, - Role: a.Role, - }) + appealResponse, appeals, err := getActiveAndPendingAppeals(client, context, provider) + if err != nil { + return err + } + + var migration migrations.Client + if provider.Type == migrations.Metabase { + migration = mb.NewMigration(provider.Config, resources, appeals) + } else { + return errors.New(fmt.Sprintf("Migration not supported for provider %s", provider.Type)) } - migration := metabase.NewMigration(providerConfig, resources, appeals) appealRequests, err := migration.PopulateAccess() + if err != nil { + return err + } + + //migrate past-run pending appeals + for _, a := range appealResponse { + if a.Status == pending { + err := approveAppeal(a, client, context) + if err != nil { + return err + } else { + fmt.Println(a.Resource.Name, a.AccountId) + } + } + } + //migrate pending appeals for _, appealRequest := range appealRequests { resource := appealRequest.Resource - option, _ := structpb.NewStruct(map[string]interface{}{"duration": resource.Duration}) + option, _ := structpb.NewStruct(map[string]interface{}{migrations.Duration: resource.Duration}) - appeal, _ := client.CreateAppeal(context, &guardianv1beta1.CreateAppealRequest{ - AccountId: appealRequest.AccountID, - Resources: []*guardianv1beta1.CreateAppealRequest_Resource{{Id: resource.ID, - Options: option}}, + accountID := appealRequest.AccountID + appeal, err := client.CreateAppeal(context, &CreateAppealRequest{ + AccountId: accountID, + Resources: []*CreateAppealRequest_Resource{ + {Id: resource.ID, Role: resource.Role, Options: option}}, AccountType: "", }) - - fmt.Println(appeal) - //Todo: iterate appeal's approval and approve it via #{client.UpdateApproval} + if err != nil { + return err + } else { + appeals := appeal.GetAppeals() + for _, appeal := range appeals { + err := approveAppeal(appeal, client, context) + if err != nil { + return err + } + } + } } return nil }, } + + bindFlagsFromConfig(cmd) + + return cmd +} + +func getActiveAndPendingAppeals(client GuardianServiceClient, context context.Context, provider *domain.Provider) ([]*Appeal, []domain.Appeal, error) { + appeals := make([]domain.Appeal, 0) + listAppeals, err := client.ListAppeals(context, &ListAppealsRequest{ProviderUrns: []string{provider.URN}, Statuses: []string{pending, active}}) + if err != nil { + return nil, appeals, err + } + + appealResponses := listAppeals.GetAppeals() + for _, a := range appealResponses { + appeals = append(appeals, domain.Appeal{ + ID: a.Id, + ResourceID: a.ResourceId, + Status: a.Status, + AccountID: a.AccountId, + AccountType: a.AccountType, + Role: a.Role, + }) + } + return appealResponses, appeals, nil +} + +func getResources(client GuardianServiceClient, context context.Context, provider *domain.Provider) ([]domain.Resource, error) { + listResources, err := client.ListResources(context, &ListResourcesRequest{ProviderUrn: provider.URN, IsDeleted: false}) + resources := make([]domain.Resource, 0) + if err != nil { + return resources, err + } + for _, r := range listResources.GetResources() { + resources = append(resources, domain.Resource{ + ID: r.Id, + ProviderType: r.ProviderType, + ProviderURN: r.ProviderUrn, + Type: r.Type, + URN: r.Urn, + Name: r.Name, + }) + } + return resources, nil +} + +func getProviderConfig(client GuardianServiceClient, context context.Context, providerId string, say *crypto.AES) (*domain.Provider, error) { + providerResponse, err := client.GetProvider(context, &GetProviderRequest{Id: providerId}) + if err != nil { + return nil, err + } + + provider := providerResponse.GetProvider() + fields := provider.Config.Credentials.GetStructValue().GetFields() + abc, err := say.Decrypt(fields[migrations.Password].GetStringValue()) + + return &domain.Provider{ + ID: providerId, + Type: provider.Type, + URN: provider.Urn, + Config: &domain.ProviderConfig{ + Type: provider.Config.Type, + URN: provider.Config.Urn, + Credentials: map[string]string{ + migrations.Username: fields[migrations.Username].GetStringValue(), + migrations.Password: abc, + migrations.Host: fields[migrations.Host].GetStringValue(), + }, + }, + }, err +} + +func approveAppeal(appeal *Appeal, client GuardianServiceClient, context context.Context) error { + approvals := appeal.Approvals + for _, approval := range approvals { + if approval.Status == pending { + _, err := client.UpdateApproval(context, &UpdateApprovalRequest{ + Id: appeal.Id, + ApprovalName: approval.Name, + Action: &UpdateApprovalRequest_Action{Action: "approve", Reason: "Metabase migration"}, + }) + if err != nil { + return err + } + } + } + return nil } diff --git a/cmd/root.go b/cmd/root.go index 95fc57a85..e27a2b7c4 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -49,7 +49,7 @@ func New(cfg *Config) *cobra.Command { cmd.AddCommand(VersionCmd()) //Migration command - cmd.AddCommand(MigrationCmd()) + cmd.AddCommand(MigrationCmd(cliConfig)) // Help topics cmdx.SetHelp(cmd) diff --git a/plugins/migrations/client.go b/plugins/migrations/client.go index 93bb4903d..03cbc012b 100644 --- a/plugins/migrations/client.go +++ b/plugins/migrations/client.go @@ -1,7 +1,20 @@ package migrations +const ( + Metabase = "metabase" + Username = "username" + Host = "host" + Password = "password" + Duration = "duration" + Group = "group" + Member = "member" + DefaultDuration = "720h" +) + type ResourceRequest struct { ID string + Name string + Role string Duration string } diff --git a/plugins/migrations/metabase/client.go b/plugins/migrations/metabase/client.go index 582cad292..4468a66ca 100644 --- a/plugins/migrations/metabase/client.go +++ b/plugins/migrations/metabase/client.go @@ -3,11 +3,11 @@ package metabase import ( "bytes" "encoding/json" - "fmt" - "github.com/go-playground/validator/v10" "io" "net/http" "net/url" + + "github.com/go-playground/validator/v10" ) type ClientConfig struct { @@ -23,12 +23,6 @@ type user struct { MembershipID int `json:"membership_id"` } -type group struct { - ID int `json:"id,omitempty"` - Name string `json:"name"` - Members []user `json:"members"` -} - type member struct { MembershipId int `json:"membership_id,omitempty"` GroupId int `json:"group_id"` @@ -49,23 +43,6 @@ type databasePermission struct { Schemas string `json:"schemas" mapstructure:"schemas"` } -type databaseGraph struct { - Revision int `json:"revision"` - // Groups is a map[group_id]map[database_id]databasePermission - Groups map[string]map[string]databasePermission `json:"groups"` -} - -type collectionGraph struct { - Revision int `json:"revision"` - // Groups is a map[group_id]map[database_id]role string - Groups map[string]map[string]string `json:"groups"` -} - -type membershipRequest struct { - GroupID int `json:"group_id"` - UserID int `json:"user_id"` -} - var ( databaseViewerPermission = databasePermission{ Schemas: "all", @@ -134,21 +111,6 @@ func (c *client) getUsers() ([]user, error) { return users, nil } -func (c *client) getGroups() ([]group, error) { - req, err := c.newRequest(http.MethodGet, "/api/permissions/group", nil) - if err != nil { - return nil, err - } - - var groups []group - - if _, err := c.do(req, &groups); err != nil { - return nil, err - } - - return groups, nil -} - func (c *client) getMembership() (map[string][]member, error) { req, err := c.newRequest(http.MethodGet, "/api/permissions/membership", nil) if err != nil { @@ -182,50 +144,6 @@ func (c *client) getSessionToken() (string, error) { return sessionResponse.ID, nil } -func (c *client) getCollectionAccess() (*collectionGraph, error) { - req, err := c.newRequest(http.MethodGet, "/api/collection/graph", nil) - if err != nil { - return nil, err - } - - var graph collectionGraph - if _, err := c.do(req, &graph); err != nil { - return nil, err - } - - return &graph, nil -} - -func (c *client) getDatabaseAccess() (*databaseGraph, error) { - req, err := c.newRequest(http.MethodGet, "/api/permissions/graph", nil) - if err != nil { - return nil, err - } - - var dbGraph databaseGraph - if _, err := c.do(req, &dbGraph); err != nil { - return nil, err - } - - return &dbGraph, nil -} - -func (c *client) getGroup(id int) (*group, error) { - url := fmt.Sprintf("/api/permissions/group/%d", id) - req, err := c.newRequest(http.MethodGet, url, nil) - if err != nil { - return nil, err - } - - var group group - - if _, err := c.do(req, &group); err != nil { - return nil, err - } - - return &group, nil -} - func (c *client) newRequest(method, path string, body interface{}) (*http.Request, error) { u, err := c.baseURL.Parse(path) if err != nil { diff --git a/plugins/migrations/metabase/migration.go b/plugins/migrations/metabase/migration.go index 14d754dbc..fea58d0f9 100644 --- a/plugins/migrations/metabase/migration.go +++ b/plugins/migrations/metabase/migration.go @@ -2,26 +2,27 @@ package metabase import ( "fmt" + "strconv" + "github.com/odpf/guardian/domain" . "github.com/odpf/guardian/plugins/migrations" - "strconv" ) type migration struct { - typeName string - providerConfig domain.ProviderConfig - resources []domain.Resource - pendingAppeals []domain.Appeal + typeName string + providerConfig *domain.ProviderConfig + resources []domain.Resource + excludedAppeals []domain.Appeal } -const typeName = "metabase" +const typeName = Metabase -func NewMigration(providerConfig domain.ProviderConfig, resources []domain.Resource, pendingAppeals []domain.Appeal) *migration { +func NewMigration(providerConfig *domain.ProviderConfig, resources []domain.Resource, excludedAppeals []domain.Appeal) *migration { return &migration{ - typeName: typeName, - providerConfig: providerConfig, - resources: resources, - pendingAppeals: pendingAppeals, + typeName: typeName, + providerConfig: providerConfig, + resources: resources, + excludedAppeals: excludedAppeals, } } @@ -37,7 +38,7 @@ func (p *migration) PopulateAccess() ([]AppealRequest, error) { resourceMap[resource.URN] = resource } - for _, appeal := range p.pendingAppeals { + for _, appeal := range p.excludedAppeals { if m, ok := appealMap[appeal.ResourceID]; ok { appealMap[appeal.ResourceID] = append(m, appeal) } else { @@ -47,44 +48,47 @@ func (p *migration) PopulateAccess() ([]AppealRequest, error) { credentials := p.providerConfig.Credentials.(map[string]string) c, err := NewClient(&ClientConfig{ - Host: credentials["host"], - Username: credentials["username"], - Password: credentials["password"], + Host: credentials[Host], + Username: credentials[Username], + Password: credentials[Password], HTTPClient: nil, }) + if err != nil { + return nil, err + } userMap := make(map[int]user, 0) - groupMap := make(map[int]group, 0) users, err := c.getUsers() + if err != nil { + return nil, err + } for _, user := range users { userMap[user.ID] = user } - groups, err := c.getGroups() - for _, group := range groups { - groupMap[group.ID] = group + membership, err := c.getMembership() + if err != nil { + return nil, err } - membership, err := c.getMembership() appeals := make([]AppealRequest, 0) for userID, members := range membership { for _, m := range members { userIdInt, _ := strconv.Atoi(userID) if user, ok := userMap[userIdInt]; ok { - resourceUrn := fmt.Sprintf("group:%d", m.GroupId) + resourceUrn := fmt.Sprintf("%s:%d", Group, m.GroupId) if resource, ok := resourceMap[resourceUrn]; ok { if _, ok := appealMap[resource.ID]; !ok { appeal := AppealRequest{ AccountID: user.Email, User: user.Email, - Resource: ResourceRequest{ID: resource.ID, Duration: "30d"}, + Resource: ResourceRequest{ID: resource.ID, Name: resource.Name, Role: Member, Duration: DefaultDuration}, } appeals = append(appeals, appeal) } } } - } } return appeals, err