From 9b34a56d7cb393a2931a030cc52d822267497f08 Mon Sep 17 00:00:00 2001 From: Ieva Date: Tue, 14 Jan 2025 08:33:42 +0000 Subject: [PATCH] AuthZ service: Take action sets into account when checking folder create permissions (#98751) take action sets into account when checking folder create permissions --- pkg/services/authz/rbac/service.go | 14 +++++++++++--- pkg/services/authz/rbac/store/models.go | 1 + pkg/services/authz/rbac/store/permission_query.sql | 10 ++++++++-- pkg/services/authz/rbac/store/sql_test.go | 10 ++++++++++ pkg/services/authz/rbac/store/store.go | 2 +- .../mysql--permission_query-With_action_sets.sql | 8 ++++++++ .../mysql--permission_query-admin_user.sql | 6 ++++-- .../mysql--permission_query-anonymous_user.sql | 6 ++++-- .../mysql--permission_query-user_with_teams.sql | 6 ++++-- .../mysql--permission_query-viewer_user.sql | 6 ++++-- ...postgres--permission_query-With_action_sets.sql | 8 ++++++++ .../postgres--permission_query-admin_user.sql | 6 ++++-- .../postgres--permission_query-anonymous_user.sql | 6 ++++-- .../postgres--permission_query-user_with_teams.sql | 6 ++++-- .../postgres--permission_query-viewer_user.sql | 6 ++++-- .../sqlite--permission_query-With_action_sets.sql | 8 ++++++++ .../sqlite--permission_query-admin_user.sql | 6 ++++-- .../sqlite--permission_query-anonymous_user.sql | 6 ++++-- .../sqlite--permission_query-user_with_teams.sql | 6 ++++-- .../sqlite--permission_query-viewer_user.sql | 6 ++++-- 20 files changed, 103 insertions(+), 30 deletions(-) create mode 100755 pkg/services/authz/rbac/store/testdata/mysql--permission_query-With_action_sets.sql create mode 100755 pkg/services/authz/rbac/store/testdata/postgres--permission_query-With_action_sets.sql create mode 100755 pkg/services/authz/rbac/store/testdata/sqlite--permission_query-With_action_sets.sql diff --git a/pkg/services/authz/rbac/service.go b/pkg/services/authz/rbac/service.go index f6b8b5c2fe5..8834b3e1da9 100644 --- a/pkg/services/authz/rbac/service.go +++ b/pkg/services/authz/rbac/service.go @@ -241,8 +241,15 @@ func (s *Service) getUserPermissions(ctx context.Context, ns claims.NamespaceInf ctx, span := s.tracer.Start(ctx, "authz_direct_db.service.getUserPermissions") defer span.End() + // When checking folder creation permissions, also check edit and admin action sets for folder, as the scoped folder create actions aren't stored in the DB separately + var actionSets []string + if action == "folders:create" { + actionSets = append(actionSets, "folders:edit") + actionSets = append(actionSets, "folders:admin") + } + if idType == claims.TypeAnonymous { - return s.getAnonymousPermissions(ctx, ns, action) + return s.getAnonymousPermissions(ctx, ns, action, actionSets) } userIdentifiers, err := s.GetUserIdentifiers(ctx, ns, userID) @@ -269,6 +276,7 @@ func (s *Service) getUserPermissions(ctx context.Context, ns claims.NamespaceInf userPermQuery := store.PermissionsQuery{ UserID: userIdentifiers.ID, Action: action, + ActionSets: actionSets, TeamIDs: teamIDs, Role: basicRoles.Role, IsServerAdmin: basicRoles.IsAdmin, @@ -293,7 +301,7 @@ func (s *Service) getUserPermissions(ctx context.Context, ns claims.NamespaceInf return res.(map[string]bool), nil } -func (s *Service) getAnonymousPermissions(ctx context.Context, ns claims.NamespaceInfo, action string) (map[string]bool, error) { +func (s *Service) getAnonymousPermissions(ctx context.Context, ns claims.NamespaceInfo, action string, actionSets []string) (map[string]bool, error) { ctx, span := s.tracer.Start(ctx, "authz_direct_db.service.getAnonymousPermissions") defer span.End() @@ -303,7 +311,7 @@ func (s *Service) getAnonymousPermissions(ctx context.Context, ns claims.Namespa } res, err, _ := s.sf.Do(anonPermKey+"_getAnonymousPermissions", func() (interface{}, error) { - permissions, err := s.store.GetUserPermissions(ctx, ns, store.PermissionsQuery{Action: action, Role: "Viewer"}) + permissions, err := s.store.GetUserPermissions(ctx, ns, store.PermissionsQuery{Action: action, ActionSets: actionSets, Role: "Viewer"}) if err != nil { return nil, err } diff --git a/pkg/services/authz/rbac/store/models.go b/pkg/services/authz/rbac/store/models.go index bee9cb5540b..5df60d261a7 100644 --- a/pkg/services/authz/rbac/store/models.go +++ b/pkg/services/authz/rbac/store/models.go @@ -14,6 +14,7 @@ type PermissionsQuery struct { OrgID int64 UserID int64 Action string + ActionSets []string TeamIDs []int64 Role string IsServerAdmin bool diff --git a/pkg/services/authz/rbac/store/permission_query.sql b/pkg/services/authz/rbac/store/permission_query.sql index 10e466f8074..dfa82390541 100644 --- a/pkg/services/authz/rbac/store/permission_query.sql +++ b/pkg/services/authz/rbac/store/permission_query.sql @@ -1,5 +1,11 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM {{ .Ident .PermissionTable }} as p -WHERE p.action = {{ .Arg .Query.Action }} AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM {{ .Ident .PermissionTable }} as p +WHERE + {{ if .Query.ActionSets }} + p.action IN ({{ .ArgList .Query.ActionSets }}, {{ .Arg .Query.Action }}) + {{ else }} + p.action = {{ .Arg .Query.Action }} + {{ end }} +AND p.role_id IN ( SELECT role_id FROM {{ .Ident .BuiltinRoleTable }} as br WHERE (br.role = {{ .Arg .Query.Role }} AND (br.org_id = {{ .Arg .Query.OrgID }} OR br.org_id = 0)) {{ if .Query.IsServerAdmin }} OR (br.role = 'Grafana Admin') diff --git a/pkg/services/authz/rbac/store/sql_test.go b/pkg/services/authz/rbac/store/sql_test.go index 37aca917d10..c3d9afd1f0e 100644 --- a/pkg/services/authz/rbac/store/sql_test.go +++ b/pkg/services/authz/rbac/store/sql_test.go @@ -105,6 +105,16 @@ func TestIdentityQueries(t *testing.T) { Role: "Viewer", }), }, + { + Name: "With_action_sets", + Data: getPermissions(&PermissionsQuery{ + UserID: 1, + OrgID: 1, + Action: "folders:create", + ActionSets: []string{"folders:edit", "folders:admin"}, + Role: "Viewer", + }), + }, }, sqlFolders: { { diff --git a/pkg/services/authz/rbac/store/store.go b/pkg/services/authz/rbac/store/store.go index 3e61c62f775..510619c2d39 100644 --- a/pkg/services/authz/rbac/store/store.go +++ b/pkg/services/authz/rbac/store/store.go @@ -60,7 +60,7 @@ func (s *StoreImpl) GetUserPermissions(ctx context.Context, ns claims.NamespaceI var perms []accesscontrol.Permission for res.Next() { var perm accesscontrol.Permission - if err := res.Scan(&perm.Action, &perm.Kind, &perm.Attribute, &perm.Identifier, &perm.Scope); err != nil { + if err := res.Scan(&perm.Kind, &perm.Attribute, &perm.Identifier, &perm.Scope); err != nil { return nil, err } perms = append(perms, perm) diff --git a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-With_action_sets.sql b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-With_action_sets.sql new file mode 100755 index 00000000000..0393be57d3f --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-With_action_sets.sql @@ -0,0 +1,8 @@ +SELECT p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p +WHERE + p.action IN ('folders:edit', 'folders:admin', 'folders:create') +AND p.role_id IN ( + SELECT role_id FROM `grafana`.`builtin_role` as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) + UNION + SELECT role_id FROM `grafana`.`user_role` as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) +) diff --git a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-admin_user.sql b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-admin_user.sql index a38590186d0..ba9f7064c91 100755 --- a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-admin_user.sql +++ b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-admin_user.sql @@ -1,5 +1,7 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM `grafana`.`builtin_role` as br WHERE (br.role = 'Admin' AND (br.org_id = 1 OR br.org_id = 0)) OR (br.role = 'Grafana Admin') UNION diff --git a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-anonymous_user.sql b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-anonymous_user.sql index c0a0434e9fc..99959aeef2d 100755 --- a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-anonymous_user.sql +++ b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-anonymous_user.sql @@ -1,4 +1,6 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM `grafana`.`builtin_role` as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) ) diff --git a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-user_with_teams.sql b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-user_with_teams.sql index 09c374c4000..d47a6b8226d 100755 --- a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-user_with_teams.sql +++ b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-user_with_teams.sql @@ -1,5 +1,7 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM `grafana`.`builtin_role` as br WHERE (br.role = 'None' AND (br.org_id = 1 OR br.org_id = 0)) UNION SELECT role_id FROM `grafana`.`user_role` as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) diff --git a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-viewer_user.sql b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-viewer_user.sql index 0bf68123439..15f8dde3169 100755 --- a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-viewer_user.sql +++ b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-viewer_user.sql @@ -1,5 +1,7 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM `grafana`.`builtin_role` as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) UNION SELECT role_id FROM `grafana`.`user_role` as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) diff --git a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-With_action_sets.sql b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-With_action_sets.sql new file mode 100755 index 00000000000..8c35badfb31 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-With_action_sets.sql @@ -0,0 +1,8 @@ +SELECT p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE + p.action IN ('folders:edit', 'folders:admin', 'folders:create') +AND p.role_id IN ( + SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) + UNION + SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) +) diff --git a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-admin_user.sql b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-admin_user.sql index 3e4ac0a954c..c78b0cbd2aa 100755 --- a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-admin_user.sql +++ b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-admin_user.sql @@ -1,5 +1,7 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Admin' AND (br.org_id = 1 OR br.org_id = 0)) OR (br.role = 'Grafana Admin') UNION diff --git a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-anonymous_user.sql b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-anonymous_user.sql index d8a4944a45f..20544064211 100755 --- a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-anonymous_user.sql +++ b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-anonymous_user.sql @@ -1,4 +1,6 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) ) diff --git a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-user_with_teams.sql b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-user_with_teams.sql index d7b8ea9ed0d..ae269975c83 100755 --- a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-user_with_teams.sql +++ b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-user_with_teams.sql @@ -1,5 +1,7 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'None' AND (br.org_id = 1 OR br.org_id = 0)) UNION SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) diff --git a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-viewer_user.sql b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-viewer_user.sql index 71e2412fbc1..3a78a730602 100755 --- a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-viewer_user.sql +++ b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-viewer_user.sql @@ -1,5 +1,7 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) UNION SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-With_action_sets.sql b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-With_action_sets.sql new file mode 100755 index 00000000000..8c35badfb31 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-With_action_sets.sql @@ -0,0 +1,8 @@ +SELECT p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE + p.action IN ('folders:edit', 'folders:admin', 'folders:create') +AND p.role_id IN ( + SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) + UNION + SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) +) diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-admin_user.sql b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-admin_user.sql index 3e4ac0a954c..c78b0cbd2aa 100755 --- a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-admin_user.sql +++ b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-admin_user.sql @@ -1,5 +1,7 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Admin' AND (br.org_id = 1 OR br.org_id = 0)) OR (br.role = 'Grafana Admin') UNION diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-anonymous_user.sql b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-anonymous_user.sql index d8a4944a45f..20544064211 100755 --- a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-anonymous_user.sql +++ b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-anonymous_user.sql @@ -1,4 +1,6 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) ) diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-user_with_teams.sql b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-user_with_teams.sql index d7b8ea9ed0d..ae269975c83 100755 --- a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-user_with_teams.sql +++ b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-user_with_teams.sql @@ -1,5 +1,7 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'None' AND (br.org_id = 1 OR br.org_id = 0)) UNION SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-viewer_user.sql b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-viewer_user.sql index 71e2412fbc1..3a78a730602 100755 --- a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-viewer_user.sql +++ b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-viewer_user.sql @@ -1,5 +1,7 @@ -SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p -WHERE p.action = 'folders:read' AND p.role_id IN ( +SELECT p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE + p.action = 'folders:read' +AND p.role_id IN ( SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) UNION SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0)