From 82a88cc83f1c6a9d6759d1b4936de0643a4cada9 Mon Sep 17 00:00:00 2001 From: Alexander Zobnin Date: Mon, 4 Mar 2024 15:29:13 +0300 Subject: [PATCH] Access control: Extend GetUserPermissions() to query permissions in org (#83392) * Access control: Extend GetUserPermissions() to query permissions in specific org * Use db query to fetch permissions in org * refactor * refactor * use conditional join * minor refactor * Add test cases * Search permissions correctly in OSS vs Enterprise * Get permissions from memory * Refactor * remove unused func * Add tests for GetUserPermissionsInOrg * fix linter --- pkg/api/org_users.go | 11 ++-- pkg/api/org_users_test.go | 1 + pkg/services/accesscontrol/accesscontrol.go | 3 + pkg/services/accesscontrol/acimpl/service.go | 49 +++++++++++++++- .../accesscontrol/acimpl/service_test.go | 56 +++++++++++++++++++ pkg/services/accesscontrol/actest/fake.go | 4 ++ .../accesscontrol/database/database.go | 34 +++++++---- .../accesscontrol/database/database_test.go | 18 ++++++ pkg/services/accesscontrol/mock/mock.go | 12 ++++ 9 files changed, 171 insertions(+), 17 deletions(-) diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index dec12f2ae25..9e1403467e8 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -323,12 +323,13 @@ func (hs *HTTPServer) searchOrgUsersHelper(c *contextmodel.ReqContext, query *or // Get accesscontrol metadata and IPD labels for users in the target org accessControlMetadata := map[string]accesscontrol.Metadata{} - if c.QueryBool("accesscontrol") && c.SignedInUser.Permissions != nil { - // TODO https://github.com/grafana/identity-access-team/issues/268 - user access control service for fetching permissions from another organization - permissions, ok := c.SignedInUser.Permissions[query.OrgID] - if ok { - accessControlMetadata = accesscontrol.GetResourcesMetadata(c.Req.Context(), permissions, "users:id:", userIDs) + if c.QueryBool("accesscontrol") { + permissionsList, err := hs.accesscontrolService.GetUserPermissionsInOrg(c.Req.Context(), c.SignedInUser, query.OrgID) + permissions := accesscontrol.GroupScopesByAction(permissionsList) + if err != nil { + return nil, err } + accessControlMetadata = accesscontrol.GetResourcesMetadata(c.Req.Context(), permissions, "users:id:", userIDs) } for i := range filteredUsers { diff --git a/pkg/api/org_users_test.go b/pkg/api/org_users_test.go index d4f9951bcf6..e1820ca6158 100644 --- a/pkg/api/org_users_test.go +++ b/pkg/api/org_users_test.go @@ -368,6 +368,7 @@ func TestGetOrgUsersAPIEndpoint_AccessControlMetadata(t *testing.T) { } hs.authInfoService = &authinfotest.FakeService{} hs.userService = &usertest.FakeUserService{ExpectedSignedInUser: userWithPermissions(1, tt.permissions)} + hs.accesscontrolService = actest.FakeService{ExpectedPermissions: tt.permissions} }) url := "/api/orgs/1/users" diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 8098cdc1404..616d876a205 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -26,6 +26,8 @@ type Service interface { registry.ProvidesUsageStats // GetUserPermissions returns user permissions with only action and scope fields set. GetUserPermissions(ctx context.Context, user identity.Requester, options Options) ([]Permission, error) + // GetUserPermissionsInOrg return user permission in a specific organization + GetUserPermissionsInOrg(ctx context.Context, user identity.Requester, orgID int64) ([]Permission, error) // SearchUsersPermissions returns all users' permissions filtered by an action prefix SearchUsersPermissions(ctx context.Context, user identity.Requester, options SearchOptions) (map[int64][]Permission, error) // ClearUserPermissionCache removes the permission cache entry for the given user @@ -75,6 +77,7 @@ type SearchOptions struct { Scope string NamespacedID string // ID of the identity (ex: user:3, service-account:4) wildcards Wildcards // private field computed based on the Scope + RolePrefixes []string } // Wildcards computes the wildcard scopes that include the scope diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index 529d17d6ea6..5cd05fd0941 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -23,6 +23,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol/migrator" "github.com/grafana/grafana/pkg/services/accesscontrol/pluginutils" "github.com/grafana/grafana/pkg/services/auth/identity" + "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" @@ -41,6 +42,8 @@ var SharedWithMeFolderPermission = accesscontrol.Permission{ Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.SharedWithMeFolderUID), } +var OSSRolesPrefixes = []string{accesscontrol.ManagedRolePrefix, accesscontrol.ExternalServiceRolePrefix} + func ProvideService(cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService, accessControl accesscontrol.AccessControl, features featuremgmt.FeatureToggles) (*Service, error) { service := ProvideOSSService(cfg, database.ProvideService(db), cache, features) @@ -125,7 +128,7 @@ func (s *Service) getUserPermissions(ctx context.Context, user identity.Requeste UserID: userID, Roles: accesscontrol.GetOrgRoles(user), TeamIDs: user.GetTeams(), - RolePrefixes: []string{accesscontrol.ManagedRolePrefix, accesscontrol.ExternalServiceRolePrefix}, + RolePrefixes: OSSRolesPrefixes, }) if err != nil { return nil, err @@ -158,6 +161,48 @@ func (s *Service) getCachedUserPermissions(ctx context.Context, user identity.Re return permissions, nil } +func (s *Service) GetUserPermissionsInOrg(ctx context.Context, user identity.Requester, orgID int64) ([]accesscontrol.Permission, error) { + permissions := make([]accesscontrol.Permission, 0) + + if s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) { + permissions = append(permissions, SharedWithMeFolderPermission) + } + + namespace, id := user.GetNamespacedID() + userID, err := identity.UserIdentifier(namespace, id) + if err != nil { + return nil, err + } + + // Get permissions for user's basic roles from RAM + roleList, err := s.store.GetUsersBasicRoles(ctx, []int64{userID}, orgID) + if err != nil { + return nil, fmt.Errorf("could not fetch basic roles for the user: %w", err) + } + var roles []string + var ok bool + if roles, ok = roleList[userID]; !ok { + return nil, fmt.Errorf("found no basic roles for user %d in organisation %d", userID, orgID) + } + for _, builtin := range roles { + if basicRole, ok := s.roles[builtin]; ok { + permissions = append(permissions, basicRole.Permissions...) + } + } + + dbPermissions, err := s.store.SearchUsersPermissions(ctx, orgID, accesscontrol.SearchOptions{ + NamespacedID: authn.NamespacedID(namespace, userID), + // Query only basic, managed and plugin roles in OSS + RolePrefixes: OSSRolesPrefixes, + }) + if err != nil { + return nil, err + } + + userPermissions := dbPermissions[userID] + return append(permissions, userPermissions...), nil +} + func (s *Service) ClearUserPermissionCache(user identity.Requester) { s.cache.Delete(permissionCacheKey(user)) } @@ -237,6 +282,8 @@ func (s *Service) DeclarePluginRoles(ctx context.Context, ID, name string, regs // SearchUsersPermissions returns all users' permissions filtered by action prefixes func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Requester, options accesscontrol.SearchOptions) (map[int64][]accesscontrol.Permission, error) { + // Limit roles to available in OSS + options.RolePrefixes = OSSRolesPrefixes if options.NamespacedID != "" { userID, err := options.ComputeUserID() if err != nil { diff --git a/pkg/services/accesscontrol/acimpl/service_test.go b/pkg/services/accesscontrol/acimpl/service_test.go index a36ed4cdfed..88ea293cf72 100644 --- a/pkg/services/accesscontrol/acimpl/service_test.go +++ b/pkg/services/accesscontrol/acimpl/service_test.go @@ -938,3 +938,59 @@ func TestService_DeleteExternalServiceRole(t *testing.T) { }) } } + +func TestService_GetUserPermissionsInOrg(t *testing.T) { + tests := []struct { + name string + orgID int64 + ramRoles map[string]*accesscontrol.RoleDTO // BasicRole => RBAC BasicRole + storedPerms map[int64][]accesscontrol.Permission // UserID => Permissions + storedRoles map[int64][]string // UserID => Roles + want []accesscontrol.Permission + }{ + { + name: "should get correct permissions from another org", + orgID: 2, + ramRoles: map[string]*accesscontrol.RoleDTO{ + string(roletype.RoleEditor): {Permissions: []accesscontrol.Permission{}}, + string(roletype.RoleAdmin): {Permissions: []accesscontrol.Permission{ + {Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"}, + }}, + }, + storedPerms: map[int64][]accesscontrol.Permission{ + 1: { + {Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}, + {Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:id:1"}, + }, + 2: { + {Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:2"}, + }, + }, + storedRoles: map[int64][]string{ + 1: {string(roletype.RoleAdmin)}, + 2: {string(roletype.RoleEditor)}, + }, + want: []accesscontrol.Permission{ + {Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:2"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + ac := setupTestEnv(t) + + ac.roles = tt.ramRoles + ac.store = actest.FakeStore{ + ExpectedUsersPermissions: tt.storedPerms, + ExpectedUsersRoles: tt.storedRoles, + } + user := &user.SignedInUser{OrgID: 1, UserID: 2} + + got, err := ac.GetUserPermissionsInOrg(ctx, user, 2) + require.Nil(t, err) + + assert.ElementsMatch(t, got, tt.want) + }) + } +} diff --git a/pkg/services/accesscontrol/actest/fake.go b/pkg/services/accesscontrol/actest/fake.go index 12e4330fee8..21698bd410a 100644 --- a/pkg/services/accesscontrol/actest/fake.go +++ b/pkg/services/accesscontrol/actest/fake.go @@ -27,6 +27,10 @@ func (f FakeService) GetUserPermissions(ctx context.Context, user identity.Reque return f.ExpectedPermissions, f.ExpectedErr } +func (f FakeService) GetUserPermissionsInOrg(ctx context.Context, user identity.Requester, orgID int64) ([]accesscontrol.Permission, error) { + return f.ExpectedPermissions, f.ExpectedErr +} + func (f FakeService) SearchUsersPermissions(ctx context.Context, user identity.Requester, options accesscontrol.SearchOptions) (map[int64][]accesscontrol.Permission, error) { return f.ExpectedUsersPermissions, f.ExpectedErr } diff --git a/pkg/services/accesscontrol/database/database.go b/pkg/services/accesscontrol/database/database.go index 6e61421ee2a..fa91e954908 100644 --- a/pkg/services/accesscontrol/database/database.go +++ b/pkg/services/accesscontrol/database/database.go @@ -36,8 +36,8 @@ func (s *AccessControlStore) GetUserPermissions(ctx context.Context, query acces ` + filter if len(query.RolePrefixes) > 0 { - q += " WHERE ( " + strings.Repeat("role.name LIKE ? OR ", len(query.RolePrefixes)) - q = q[:len(q)-4] + " )" // remove last " OR " + q += " WHERE ( " + strings.Repeat("role.name LIKE ? OR ", len(query.RolePrefixes)-1) + q += "role.name LIKE ? )" for i := range query.RolePrefixes { params = append(params, query.RolePrefixes[i]+"%") } @@ -53,7 +53,7 @@ func (s *AccessControlStore) GetUserPermissions(ctx context.Context, query acces return result, err } -// SearchUsersPermissions returns the list of user permissions indexed by UserID +// SearchUsersPermissions returns the list of user permissions in specific organization indexed by UserID func (s *AccessControlStore) SearchUsersPermissions(ctx context.Context, orgID int64, options accesscontrol.SearchOptions) (map[int64][]accesscontrol.Permission, error) { type UserRBACPermission struct { UserID int64 `xorm:"user_id"` @@ -61,7 +61,13 @@ func (s *AccessControlStore) SearchUsersPermissions(ctx context.Context, orgID i Scope string `xorm:"scope"` } dbPerms := make([]UserRBACPermission, 0) + if err := s.sql.WithDbSession(ctx, func(sess *db.Session) error { + roleNameFilterJoin := "" + if len(options.RolePrefixes) > 0 { + roleNameFilterJoin = "INNER JOIN role AS r on up.role_id = r.id" + } + // Find permissions q := ` SELECT @@ -69,21 +75,21 @@ func (s *AccessControlStore) SearchUsersPermissions(ctx context.Context, orgID i action, scope FROM ( - SELECT ur.user_id, ur.org_id, p.action, p.scope + SELECT ur.user_id, ur.org_id, p.action, p.scope, ur.role_id FROM permission AS p INNER JOIN user_role AS ur on ur.role_id = p.role_id UNION ALL - SELECT tm.user_id, tr.org_id, p.action, p.scope + SELECT tm.user_id, tr.org_id, p.action, p.scope, tr.role_id FROM permission AS p INNER JOIN team_role AS tr ON tr.role_id = p.role_id INNER JOIN team_member AS tm ON tm.team_id = tr.team_id UNION ALL - SELECT ou.user_id, ou.org_id, p.action, p.scope + SELECT ou.user_id, ou.org_id, p.action, p.scope, br.role_id FROM permission AS p INNER JOIN builtin_role AS br ON br.role_id = p.role_id INNER JOIN org_user AS ou ON ou.role = br.role UNION ALL - SELECT sa.user_id, br.org_id, p.action, p.scope + SELECT sa.user_id, br.org_id, p.action, p.scope, br.role_id FROM permission AS p INNER JOIN builtin_role AS br ON br.role_id = p.role_id INNER JOIN ( @@ -91,8 +97,8 @@ func (s *AccessControlStore) SearchUsersPermissions(ctx context.Context, orgID i FROM ` + s.sql.GetDialect().Quote("user") + ` AS u WHERE u.is_admin ) AS sa ON 1 = 1 WHERE br.role = ? - ) AS up - WHERE (org_id = ? OR org_id = ?) + ) AS up ` + roleNameFilterJoin + ` + WHERE (up.org_id = ? OR up.org_id = ?) ` params := []any{accesscontrol.RoleGrafanaAdmin, accesscontrol.GlobalOrgID, orgID} @@ -121,9 +127,15 @@ func (s *AccessControlStore) SearchUsersPermissions(ctx context.Context, orgID i q += ` AND user_id = ?` params = append(params, userID) } + if len(options.RolePrefixes) > 0 { + q += " AND ( " + strings.Repeat("r.name LIKE ? OR ", len(options.RolePrefixes)-1) + q += "r.name LIKE ? )" + for _, prefix := range options.RolePrefixes { + params = append(params, prefix+"%") + } + } - return sess.SQL(q, params...). - Find(&dbPerms) + return sess.SQL(q, params...).Find(&dbPerms) }); err != nil { return nil, err } diff --git a/pkg/services/accesscontrol/database/database_test.go b/pkg/services/accesscontrol/database/database_test.go index 4e4f19b236a..5cbfae4633d 100644 --- a/pkg/services/accesscontrol/database/database_test.go +++ b/pkg/services/accesscontrol/database/database_test.go @@ -629,6 +629,24 @@ func TestIntegrationAccessControlStore_SearchUsersPermissions(t *testing.T) { options: accesscontrol.SearchOptions{Action: "teams:read", Scope: "teams:id:1"}, wantPerm: map[int64][]accesscontrol.Permission{1: {{Action: "teams:read", Scope: "teams:id:1"}}}, }, + { + name: "user assignment by role prefixes", + users: []testUser{{orgRole: org.RoleAdmin, isAdmin: false}}, + permCmds: []rs.SetResourcePermissionsCommand{ + {User: accesscontrol.User{ID: 1, IsExternal: false}, SetResourcePermissionCommand: readTeamPerm("1")}, + }, + options: accesscontrol.SearchOptions{RolePrefixes: []string{accesscontrol.ManagedRolePrefix}}, + wantPerm: map[int64][]accesscontrol.Permission{1: {{Action: "teams:read", Scope: "teams:id:1"}}}, + }, + { + name: "filter out permissions by role prefix", + users: []testUser{{orgRole: org.RoleAdmin, isAdmin: false}}, + permCmds: []rs.SetResourcePermissionsCommand{ + {User: accesscontrol.User{ID: 1, IsExternal: false}, SetResourcePermissionCommand: readTeamPerm("1")}, + }, + options: accesscontrol.SearchOptions{RolePrefixes: []string{accesscontrol.BasicRolePrefix}}, + wantPerm: map[int64][]accesscontrol.Permission{}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/services/accesscontrol/mock/mock.go b/pkg/services/accesscontrol/mock/mock.go index ef66d74c301..48fd43fc992 100644 --- a/pkg/services/accesscontrol/mock/mock.go +++ b/pkg/services/accesscontrol/mock/mock.go @@ -21,6 +21,7 @@ type fullAccessControl interface { type Calls struct { Evaluate []interface{} GetUserPermissions []interface{} + GetUserPermissionsInOrg []interface{} ClearUserPermissionCache []interface{} DeclareFixedRoles []interface{} DeclarePluginRoles []interface{} @@ -47,6 +48,7 @@ type Mock struct { // Override functions EvaluateFunc func(context.Context, identity.Requester, accesscontrol.Evaluator) (bool, error) GetUserPermissionsFunc func(context.Context, identity.Requester, accesscontrol.Options) ([]accesscontrol.Permission, error) + GetUserPermissionsInOrgFunc func(context.Context, identity.Requester, int64) ([]accesscontrol.Permission, error) ClearUserPermissionCacheFunc func(identity.Requester) DeclareFixedRolesFunc func(...accesscontrol.RoleRegistration) error DeclarePluginRolesFunc func(context.Context, string, string, []plugins.RoleRegistration) error @@ -140,6 +142,16 @@ func (m *Mock) GetUserPermissions(ctx context.Context, user identity.Requester, return m.permissions, nil } +func (m *Mock) GetUserPermissionsInOrg(ctx context.Context, user identity.Requester, orgID int64) ([]accesscontrol.Permission, error) { + m.Calls.GetUserPermissionsInOrg = append(m.Calls.GetUserPermissionsInOrg, []interface{}{ctx, user, orgID}) + // Use override if provided + if m.GetUserPermissionsInOrgFunc != nil { + return m.GetUserPermissionsInOrgFunc(ctx, user, orgID) + } + // Otherwise return the Permissions list + return m.permissions, nil +} + func (m *Mock) ClearUserPermissionCache(user identity.Requester) { m.Calls.ClearUserPermissionCache = append(m.Calls.ClearUserPermissionCache, []interface{}{user}) // Use override if provided