diff --git a/pkg/services/accesscontrol/database/resource_permissions.go b/pkg/services/accesscontrol/database/resource_permissions.go index ff2c3b39b93..7625ab408df 100644 --- a/pkg/services/accesscontrol/database/resource_permissions.go +++ b/pkg/services/accesscontrol/database/resource_permissions.go @@ -361,14 +361,14 @@ func (s *AccessControlStore) getResourcesPermissions(sess *sqlstore.DBSession, o initialLength := len(args) - userFilter, err := accesscontrol.Filter(context.Background(), "u.id", "users", accesscontrol.ActionOrgUsersRead, query.User) + userFilter, err := accesscontrol.Filter(query.User, "u.id", "users", accesscontrol.ActionOrgUsersRead) if err != nil { return nil, err } user := userSelect + userFrom + where + " AND " + userFilter.Where args = append(args, userFilter.Args...) - teamFilter, err := accesscontrol.Filter(context.Background(), "t.id", "teams", accesscontrol.ActionTeamsRead, query.User) + teamFilter, err := accesscontrol.Filter(query.User, "t.id", "teams", accesscontrol.ActionTeamsRead) if err != nil { return nil, err } diff --git a/pkg/services/accesscontrol/filter.go b/pkg/services/accesscontrol/filter.go index 635350982fa..98435d96d3a 100644 --- a/pkg/services/accesscontrol/filter.go +++ b/pkg/services/accesscontrol/filter.go @@ -1,7 +1,6 @@ package accesscontrol import ( - "context" "errors" "strconv" "strings" @@ -26,10 +25,14 @@ var ( allowAllQuery = SQLFilter{" 1 = 1", nil} ) +type SQLFilter struct { + Where string + Args []interface{} +} + // Filter creates a where clause to restrict the view of a query based on a users permissions -// Scopes for a certain action will be compared against prefix:id:sqlID where prefix is the scope prefix and sqlID -// is the id to generate scope from e.g. user.id -func Filter(ctx context.Context, sqlID, prefix, action string, user *models.SignedInUser) (SQLFilter, error) { +// Scopes that exists for all actions will be parsed and compared against the supplied sqlID +func Filter(user *models.SignedInUser, sqlID, prefix string, actions ...string) (SQLFilter, error) { if _, ok := sqlIDAcceptList[sqlID]; !ok { return denyQuery, errors.New("sqlID is not in the accept list") } @@ -37,26 +40,33 @@ func Filter(ctx context.Context, sqlID, prefix, action string, user *models.Sign return denyQuery, errors.New("missing permissions") } - var hasWildcard bool - var ids []interface{} - for _, scope := range user.Permissions[user.OrgId][action] { - if strings.HasPrefix(scope, prefix) || scope == "*" { - if id := strings.TrimPrefix(scope, prefix); id == "*" || id == ":*" || id == ":id:*" { - hasWildcard = true - break - } - if id, err := parseScopeID(scope); err == nil { - ids = append(ids, id) - } + wildcards := 0 + result := make(map[int64]int) + for _, a := range actions { + ids, hasWildcard := parseScopes(prefix, user.Permissions[user.OrgId][a]) + if hasWildcard { + wildcards += 1 + continue + } + if len(ids) == 0 { + return denyQuery, nil + } + for _, id := range ids { + result[id] += 1 } } - if hasWildcard { + // return early if every action has wildcard scope + if wildcards == len(actions) { return allowAllQuery, nil } - if len(ids) == 0 { - return denyQuery, nil + var ids []interface{} + for id, count := range result { + // if an id exist for every action include it in the filter + if count+wildcards == len(actions) { + ids = append(ids, id) + } } query := strings.Builder{} @@ -70,6 +80,20 @@ func Filter(ctx context.Context, sqlID, prefix, action string, user *models.Sign return SQLFilter{query.String(), ids}, nil } +func parseScopes(prefix string, scopes []string) (ids []int64, hasWildcard bool) { + for _, scope := range scopes { + if strings.HasPrefix(scope, prefix) || scope == "*" { + if id := strings.TrimPrefix(scope, prefix); id == "*" || id == ":*" || id == ":id:*" { + return nil, true + } + if id, err := parseScopeID(scope); err == nil { + ids = append(ids, id) + } + } + } + return ids, false +} + func parseScopeID(scope string) (int64, error) { return strconv.ParseInt(scope[strings.LastIndex(scope, ":")+1:], 10, 64) } diff --git a/pkg/services/accesscontrol/filter_bench_test.go b/pkg/services/accesscontrol/filter_bench_test.go index 126eaf4adb4..4fbb816a1f2 100644 --- a/pkg/services/accesscontrol/filter_bench_test.go +++ b/pkg/services/accesscontrol/filter_bench_test.go @@ -32,11 +32,10 @@ func benchmarkFilter(b *testing.B, numDs, numPermissions int) { for i := 0; i < b.N; i++ { baseSql := `SELECT data_source.* FROM data_source WHERE` acFilter, err := accesscontrol.Filter( - context.Background(), + &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(permissions)}}, "data_source.id", "datasources", "datasources:read", - &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(permissions)}}, ) require.NoError(b, err) diff --git a/pkg/services/accesscontrol/filter_test.go b/pkg/services/accesscontrol/filter_test.go index 2dcfbb65eed..cca758ca68c 100644 --- a/pkg/services/accesscontrol/filter_test.go +++ b/pkg/services/accesscontrol/filter_test.go @@ -14,9 +14,12 @@ import ( ) type filterDatasourcesTestCase struct { - desc string - sqlID string - permissions []*accesscontrol.Permission + desc string + sqlID string + prefix string + actions []string + permissions map[string][]string + expectedDataSources []string expectErr bool } @@ -24,63 +27,95 @@ type filterDatasourcesTestCase struct { func TestFilter_Datasources(t *testing.T) { tests := []filterDatasourcesTestCase{ { - desc: "expect all data sources to be returned", - sqlID: "data_source.id", - permissions: []*accesscontrol.Permission{ - {Action: "datasources:read", Scope: "datasources:*"}, + desc: "expect all data sources to be returned", + sqlID: "data_source.id", + prefix: "datasources", + actions: []string{"datasources:read"}, + permissions: map[string][]string{ + "datasources:read": {"datasources:*"}, }, expectedDataSources: []string{"ds:1", "ds:2", "ds:3", "ds:4", "ds:5", "ds:6", "ds:7", "ds:8", "ds:9", "ds:10"}, }, { - desc: "expect all data sources for wildcard id scope to be returned", - sqlID: "data_source.id", - permissions: []*accesscontrol.Permission{ - {Action: "datasources:read", Scope: "datasources:id:*"}, + desc: "expect all data sources for wildcard id scope to be returned", + sqlID: "data_source.id", + prefix: "datasources", + actions: []string{"datasources:read"}, + permissions: map[string][]string{ + "datasources:read": {"datasources:id:*"}, }, expectedDataSources: []string{"ds:1", "ds:2", "ds:3", "ds:4", "ds:5", "ds:6", "ds:7", "ds:8", "ds:9", "ds:10"}, }, { - desc: "expect all data sources for wildcard scope to be returned", - sqlID: "data_source.id", - permissions: []*accesscontrol.Permission{ - {Action: "datasources:read", Scope: "*"}, + desc: "expect all data sources for wildcard scope to be returned", + sqlID: "data_source.id", + prefix: "datasources", + actions: []string{"datasources:read"}, + permissions: map[string][]string{ + "datasources:read": {"*"}, }, expectedDataSources: []string{"ds:1", "ds:2", "ds:3", "ds:4", "ds:5", "ds:6", "ds:7", "ds:8", "ds:9", "ds:10"}, }, { desc: "expect no data sources to be returned", sqlID: "data_source.id", - permissions: []*accesscontrol.Permission{}, + prefix: "datasources", + actions: []string{"datasources:read"}, + permissions: map[string][]string{}, expectedDataSources: []string{}, }, { - desc: "expect data sources with id 3, 7 and 8 to be returned", - sqlID: "data_source.id", - permissions: []*accesscontrol.Permission{ - {Action: "datasources:read", Scope: "datasources:id:3"}, - {Action: "datasources:read", Scope: "datasources:id:7"}, - {Action: "datasources:read", Scope: "datasources:id:8"}, + desc: "expect data sources with id 3, 7 and 8 to be returned", + sqlID: "data_source.id", + prefix: "datasources", + actions: []string{"datasources:read"}, + permissions: map[string][]string{ + "datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"}, }, expectedDataSources: []string{"ds:3", "ds:7", "ds:8"}, }, { - desc: "expect no data sources to be returned for malformed scope", - sqlID: "data_source.id", - permissions: []*accesscontrol.Permission{ - {Action: "datasources:read", Scope: "datasources:id:1*"}, + desc: "expect no data sources to be returned for malformed scope", + sqlID: "data_source.id", + prefix: "datasources", + actions: []string{"datasources:read"}, + permissions: map[string][]string{ + "datasources:read": {"datasources:id:1*"}, }, - expectedDataSources: []string{}, }, { - desc: "expect error if sqlID is not in the accept list", - sqlID: "other.id", - permissions: []*accesscontrol.Permission{ - {Action: "datasources:read", Scope: "datasources:id:3"}, - {Action: "datasources:read", Scope: "datasources:id:7"}, - {Action: "datasources:read", Scope: "datasources:id:8"}, + desc: "expect error if sqlID is not in the accept list", + sqlID: "other.id", + prefix: "datasources", + actions: []string{"datasources:read"}, + permissions: map[string][]string{ + "datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"}, + }, + expectErr: true, + }, + { + desc: "expect data sources that users has several actions for", + sqlID: "data_source.id", + prefix: "datasources", + actions: []string{"datasources:read", "datasources:write"}, + permissions: map[string][]string{ + "datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"}, + "datasources:write": {"datasources:id:3", "datasources:id:8"}, + }, + expectedDataSources: []string{"ds:3", "ds:8"}, + expectErr: false, + }, + { + desc: "expect data sources that users has several actions for", + sqlID: "data_source.id", + prefix: "datasources", + actions: []string{"datasources:read", "datasources:write"}, + permissions: map[string][]string{ + "datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"}, + "datasources:write": {"datasources:*", "datasources:id:8"}, }, expectedDataSources: []string{"ds:3", "ds:7", "ds:8"}, - expectErr: true, + expectErr: false, }, } @@ -105,11 +140,13 @@ func TestFilter_Datasources(t *testing.T) { baseSql := `SELECT data_source.* FROM data_source WHERE` acFilter, err := accesscontrol.Filter( - context.Background(), + &models.SignedInUser{ + OrgId: 1, + Permissions: map[int64]map[string][]string{1: tt.permissions}, + }, tt.sqlID, - "datasources", - "datasources:read", - &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, + tt.prefix, + tt.actions..., ) if !tt.expectErr { diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index 9e186528376..3259614442e 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -241,11 +241,6 @@ type SetResourcePermissionCommand struct { Permission string } -type SQLFilter struct { - Where string - Args []interface{} -} - const ( GlobalOrgID = 0 // Permission actions diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/database.go index 623157b3b06..c451eae27cc 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/pkg/services/serviceaccounts/database/database.go @@ -317,7 +317,7 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts(ctx context.Context, s.sqlStore.Dialect.BooleanStr(true))) if s.sqlStore.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { - acFilter, err := accesscontrol.Filter(ctx, "org_user.user_id", "serviceaccounts", "serviceaccounts:read", query.User) + acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "serviceaccounts", serviceaccounts.ActionRead) if err != nil { return err } diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index e967fe62f52..55923cfb24b 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -119,7 +119,7 @@ func (ss *SQLStore) GetOrgUsers(ctx context.Context, query *models.GetOrgUsersQu whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %t", x.Dialect().Quote("user"), query.IsServiceAccount)) if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) && query.User != nil { - acFilter, err := accesscontrol.Filter(ctx, "org_user.user_id", "users", "org.users:read", query.User) + acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users", accesscontrol.ActionOrgUsersRead) if err != nil { return err } @@ -184,7 +184,7 @@ func (ss *SQLStore) SearchOrgUsers(ctx context.Context, query *models.SearchOrgU whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %t", x.Dialect().Quote("user"), query.IsServiceAccount)) if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { - acFilter, err := accesscontrol.Filter(ctx, "org_user.user_id", "users", "org.users:read", query.User) + acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users", accesscontrol.ActionOrgUsersRead) if err != nil { return err } diff --git a/pkg/services/sqlstore/permissions/dashboard.go b/pkg/services/sqlstore/permissions/dashboard.go index 0b511bfc888..ffd8c7a2baf 100644 --- a/pkg/services/sqlstore/permissions/dashboard.go +++ b/pkg/services/sqlstore/permissions/dashboard.go @@ -1,7 +1,6 @@ package permissions import ( - "context" "strings" "github.com/grafana/grafana/pkg/models" @@ -83,30 +82,32 @@ type AccessControlDashboardPermissionFilter struct { } func (f AccessControlDashboardPermissionFilter) Where() (string, []interface{}) { - folderAction := dashboards.ActionFoldersRead - dashboardAction := accesscontrol.ActionDashboardsRead + folderActions := []string{dashboards.ActionFoldersRead} + dashboardActions := []string{accesscontrol.ActionDashboardsRead} if f.PermissionLevel == models.PERMISSION_EDIT { - folderAction = accesscontrol.ActionDashboardsCreate - dashboardAction = accesscontrol.ActionDashboardsWrite + folderActions = append(folderActions, accesscontrol.ActionDashboardsCreate) + dashboardActions = append(dashboardActions, accesscontrol.ActionDashboardsWrite) } + var args []interface{} builder := strings.Builder{} - builder.WriteString("(((") - dashFilter, _ := accesscontrol.Filter(context.Background(), "dashboard.id", "dashboards", dashboardAction, f.User) + dashFilter, _ := accesscontrol.Filter(f.User, "dashboard.id", "dashboards", dashboardActions...) builder.WriteString(dashFilter.Where) + args = append(args, dashFilter.Args...) builder.WriteString(" OR ") - dashFolderFilter, _ := accesscontrol.Filter(context.Background(), "dashboard.folder_id", "folders", dashboardAction, f.User) + dashFolderFilter, _ := accesscontrol.Filter(f.User, "dashboard.folder_id", "folders", dashboardActions...) builder.WriteString(dashFolderFilter.Where) - builder.WriteString(") AND NOT dashboard.is_folder) OR (") + args = append(args, dashFolderFilter.Args...) - folderFilter, _ := accesscontrol.Filter(context.Background(), "dashboard.id", "folders", folderAction, f.User) + folderFilter, _ := accesscontrol.Filter(f.User, "dashboard.id", "folders", folderActions...) builder.WriteString(folderFilter.Where) builder.WriteString(" AND dashboard.is_folder))") + args = append(args, folderFilter.Args...) - return builder.String(), append(dashFilter.Args, append(dashFolderFilter.Args, folderFilter.Args...)...) + return builder.String(), args } diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index 8fad2e6b0c4..541b48d5484 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -229,7 +229,7 @@ func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQu err error ) if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { - acFilter, err = ac.Filter(ctx, "team.id", "teams", ac.ActionTeamsRead, query.SignedInUser) + acFilter, err = ac.Filter(query.SignedInUser, "team.id", "teams", ac.ActionTeamsRead) if err != nil { return err } @@ -528,10 +528,8 @@ func (ss *SQLStore) GetTeamMembers(ctx context.Context, query *models.GetTeamMem // Note we assume that checking SignedInUser is allowed to see team members for this team has already been performed // If the signed in user is not set no member will be returned if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { - *acFilter, err = ac.Filter(ctx, - fmt.Sprintf("%s.%s", x.Dialect().Quote("user"), x.Dialect().Quote("id")), - "users", ac.ActionOrgUsersRead, query.SignedInUser, - ) + sqlID := fmt.Sprintf("%s.%s", x.Dialect().Quote("user"), x.Dialect().Quote("id")) + *acFilter, err = ac.Filter(query.SignedInUser, sqlID, "users", ac.ActionOrgUsersRead) if err != nil { return err }