From 0f919671e79f5130f8d63a52361beef4b0ae3609 Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Wed, 6 Jul 2022 11:34:36 +0200 Subject: [PATCH] Service accounts: Add service account to teams (#51536) * Revert "Serviceaccounts: #48995 Do not display service accounts assigned to team (#48995)" This reverts commit cbf71fbd7fc444cf298ff39e5777ba24fe5a4210. * fix: test to not include more actions than necessary * adding service accounts to teams - backend and frontend changes * also support SA addition through the old team membership endpoints * fix tests * tests * serviceaccounts permission tests * serviceaccounts permission service tests run * added back test that was removed by accident * lint * refactor: add testoptionsTeams * fix a bug * service account picker change * explicitly set SA managed permissions to false for dash and folders * lint * allow team creator to list service accounts Co-authored-by: IevaVasiljeva --- .../rbac-fixed-basic-role-definitions.md | 2 +- pkg/api/accesscontrol.go | 3 +- .../database/resource_permissions.go | 68 +++++---- pkg/services/accesscontrol/models.go | 29 ++-- .../ossaccesscontrol/permissions_services.go | 21 +-- .../accesscontrol/resourcepermissions/api.go | 59 ++++---- .../resourcepermissions/api_test.go | 134 +++++++++++++++--- .../resourcepermissions/service.go | 2 +- .../resourcepermissions/service_test.go | 16 ++- pkg/services/sqlstore/team.go | 18 ++- pkg/services/sqlstore/team_test.go | 94 ++++++++---- .../AccessControl/AddPermission.tsx | 9 ++ .../components/AccessControl/Permissions.tsx | 21 ++- .../core/components/AccessControl/types.ts | 3 + .../Select/ServiceAccountPicker.tsx | 69 +++++++++ 15 files changed, 404 insertions(+), 144 deletions(-) create mode 100644 public/app/core/components/Select/ServiceAccountPicker.tsx diff --git a/docs/sources/administration/roles-and-permissions/access-control/rbac-fixed-basic-role-definitions.md b/docs/sources/administration/roles-and-permissions/access-control/rbac-fixed-basic-role-definitions.md index b69f9c702b0..28e3437b11f 100644 --- a/docs/sources/administration/roles-and-permissions/access-control/rbac-fixed-basic-role-definitions.md +++ b/docs/sources/administration/roles-and-permissions/access-control/rbac-fixed-basic-role-definitions.md @@ -74,7 +74,7 @@ The following tables list permissions associated with basic and fixed roles. | `fixed:settings:reader` | `settings:read` | Read Grafana instance settings. | | `fixed:settings:writer` | All permissions from `fixed:settings:reader` and
`settings:write` | Read and update Grafana instance settings. | | `fixed:stats:reader` | `server.stats:read` | Read Grafana instance statistics. | -| `fixed:teams:creator` | `teams:create`
`org.users:read` | Create a team and list organization users (required to manage the created team). | +| `fixed:teams:creator` | `teams:create`
`org.users:read`
`serviceaccounts:read` | Create a team and list organization users and service accounts (required to manage the created team). | | `fixed:teams:writer` | `teams:create`
`teams:delete`
`teams:read`
`teams:write`
`teams.permissions:read`
`teams.permissions:write` | Create, read, update and delete teams and manage team memberships. | | `fixed:users:reader` | `users:read`
`users.quotas:read`
`users.authtoken:read`
` | Read all users and their information, such as team memberships, authentication tokens, and quotas. | | `fixed:users:writer` | All permissions from `fixed:users:reader` and
`users:write`
`users:create`
`users:delete`
`users:enable`
`users:disable`
`users.password:write`
`users.permissions:write`
`users:logout`
`users.authtoken:write`
`users.quotas:write` | Read and update all attributes and settings for all users in Grafana: update user information, read user information, create or enable or disable a user, make a user a Grafana administrator, sign out a user, update a user’s authentication token, or update quotas for all users. | diff --git a/pkg/api/accesscontrol.go b/pkg/api/accesscontrol.go index f834d307068..b750ab71410 100644 --- a/pkg/api/accesscontrol.go +++ b/pkg/api/accesscontrol.go @@ -223,11 +223,12 @@ func (hs *HTTPServer) declareFixedRoles() error { Role: ac.RoleDTO{ Name: "fixed:teams:creator", DisplayName: "Team creator", - Description: "Create teams and read organisation users (required to manage the created teams).", + Description: "Create teams and read organisation users and service accounts (required to manage the created teams).", Group: "Teams", Permissions: []ac.Permission{ {Action: ac.ActionTeamsCreate}, {Action: ac.ActionOrgUsersRead, Scope: ac.ScopeUsersAll}, + {Action: serviceaccounts.ActionRead, Scope: serviceaccounts.ScopeAll}, }, }, Grants: teamCreatorGrants, diff --git a/pkg/services/accesscontrol/database/resource_permissions.go b/pkg/services/accesscontrol/database/resource_permissions.go index ab34e1c63f3..0a39bb2663a 100644 --- a/pkg/services/accesscontrol/database/resource_permissions.go +++ b/pkg/services/accesscontrol/database/resource_permissions.go @@ -9,23 +9,25 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions/types" + "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/sqlstore" ) type flatResourcePermission struct { - ID int64 `xorm:"id"` - RoleName string - Action string - Scope string - UserId int64 - UserLogin string - UserEmail string - TeamId int64 - TeamEmail string - Team string - BuiltInRole string - Created time.Time - Updated time.Time + ID int64 `xorm:"id"` + RoleName string + Action string + Scope string + UserId int64 + UserLogin string + UserEmail string + UserIsServiceAccount bool + TeamId int64 + TeamEmail string + Team string + BuiltInRole string + Created time.Time + Updated time.Time } func (p *flatResourcePermission) IsManaged(scope string) bool { @@ -292,6 +294,7 @@ func (s *AccessControlStore) getResourcePermissions(sess *sqlstore.DBSession, or ur.user_id AS user_id, u.login AS user_login, u.email AS user_email, + u.is_service_account AS user_is_service_account, 0 AS team_id, '' AS team, '' AS team_email, @@ -302,6 +305,7 @@ func (s *AccessControlStore) getResourcePermissions(sess *sqlstore.DBSession, or 0 AS user_id, '' AS user_login, '' AS user_email, + false AS user_is_service_account, tr.team_id AS team_id, t.name AS team, t.email AS team_email, @@ -312,6 +316,7 @@ func (s *AccessControlStore) getResourcePermissions(sess *sqlstore.DBSession, or 0 AS user_id, '' AS user_login, '' AS user_email, + false AS user_is_service_account, 0 as team_id, '' AS team, '' AS team_email, @@ -370,8 +375,13 @@ func (s *AccessControlStore) getResourcePermissions(sess *sqlstore.DBSession, or if err != nil { return nil, err } - user := userSelect + userFrom + where + " AND " + userFilter.Where + serviceAccountFilter, err := accesscontrol.Filter(query.User, "u.id", "serviceaccounts:id:", serviceaccounts.ActionRead) + if err != nil { + return nil, err + } + user := userSelect + userFrom + where + " AND (" + userFilter.Where + " OR " + serviceAccountFilter.Where + ") " args = append(args, userFilter.Args...) + args = append(args, serviceAccountFilter.Args...) teamFilter, err := accesscontrol.Filter(query.User, "t.id", "teams:id:", accesscontrol.ActionTeamsRead) if err != nil { @@ -457,20 +467,21 @@ func flatPermissionsToResourcePermission(scope string, permissions []flatResourc first := permissions[0] return &accesscontrol.ResourcePermission{ - ID: first.ID, - RoleName: first.RoleName, - Actions: actions, - Scope: first.Scope, - UserId: first.UserId, - UserLogin: first.UserLogin, - UserEmail: first.UserEmail, - TeamId: first.TeamId, - TeamEmail: first.TeamEmail, - Team: first.Team, - BuiltInRole: first.BuiltInRole, - Created: first.Created, - Updated: first.Updated, - IsManaged: first.IsManaged(scope), + ID: first.ID, + RoleName: first.RoleName, + Actions: actions, + Scope: first.Scope, + UserId: first.UserId, + UserLogin: first.UserLogin, + UserEmail: first.UserEmail, + UserIsServiceAccount: first.UserIsServiceAccount, + TeamId: first.TeamId, + TeamEmail: first.TeamEmail, + Team: first.Team, + BuiltInRole: first.BuiltInRole, + Created: first.Created, + Updated: first.Updated, + IsManaged: first.IsManaged(scope), } } @@ -581,6 +592,7 @@ func (s *AccessControlStore) getResourcePermissionsByIds(sess *sqlstore.DBSessio ur.user_id AS user_id, u.login AS user_login, u.email AS user_email, + u.is_service_account AS user_is_service_account, tr.team_id AS team_id, t.name AS team, t.email AS team_email, diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index 0317ea73b52..85ef4f1188b 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -222,20 +222,21 @@ type ScopeParams struct { // ResourcePermission is structure that holds all actions that either a team / user / builtin-role // can perform against specific resource. type ResourcePermission struct { - ID int64 - RoleName string - Actions []string - Scope string - UserId int64 - UserLogin string - UserEmail string - TeamId int64 - TeamEmail string - Team string - BuiltInRole string - IsManaged bool - Created time.Time - Updated time.Time + ID int64 + RoleName string + Actions []string + Scope string + UserId int64 + UserLogin string + UserEmail string + UserIsServiceAccount bool + TeamId int64 + TeamEmail string + Team string + BuiltInRole string + IsManaged bool + Created time.Time + Updated time.Time } func (p *ResourcePermission) Contains(targetActions []string) bool { diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index a271c3bbd6f..de8da4c50b4 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -58,9 +58,10 @@ func ProvideTeamPermissions( return nil }, Assignments: resourcepermissions.Assignments{ - Users: true, - Teams: false, - BuiltInRoles: false, + Users: true, + Teams: false, + BuiltInRoles: false, + ServiceAccounts: true, }, PermissionsToActions: map[string][]string{ "Member": TeamMemberActions, @@ -149,9 +150,10 @@ func ProvideDashboardPermissions( return []string{}, nil }, Assignments: resourcepermissions.Assignments{ - Users: true, - Teams: true, - BuiltInRoles: true, + Users: true, + Teams: true, + BuiltInRoles: true, + ServiceAccounts: false, }, PermissionsToActions: map[string][]string{ "View": DashboardViewActions, @@ -206,9 +208,10 @@ func ProvideFolderPermissions( return nil }, Assignments: resourcepermissions.Assignments{ - Users: true, - Teams: true, - BuiltInRoles: true, + Users: true, + Teams: true, + BuiltInRoles: true, + ServiceAccounts: false, }, PermissionsToActions: map[string][]string{ "View": append(DashboardViewActions, FolderViewActions...), diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index a1892130cfd..c4d8d963ef8 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api.go +++ b/pkg/services/accesscontrol/resourcepermissions/api.go @@ -40,7 +40,7 @@ func (a *api) registerEndpoints() { scope := accesscontrol.Scope(a.service.options.Resource, a.service.options.ResourceAttribute, accesscontrol.Parameter(":resourceID")) r.Get("/description", auth(disable, accesscontrol.EvalPermission(actionRead)), routing.Wrap(a.getDescription)) r.Get("/:resourceID", inheritanceSolver, auth(disable, accesscontrol.EvalPermission(actionRead, scope)), routing.Wrap(a.getPermissions)) - if a.service.options.Assignments.Users { + if a.service.options.Assignments.Users || a.service.options.Assignments.ServiceAccounts { r.Post("/:resourceID/users/:userID", inheritanceSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setUserPermission)) } if a.service.options.Assignments.Teams { @@ -53,9 +53,10 @@ func (a *api) registerEndpoints() { } type Assignments struct { - Users bool `json:"users"` - Teams bool `json:"teams"` - BuiltInRoles bool `json:"builtInRoles"` + Users bool `json:"users"` + Teams bool `json:"teams"` + BuiltInRoles bool `json:"builtInRoles"` + ServiceAccounts bool `json:"serviceAccounts"` } type Description struct { @@ -71,18 +72,19 @@ func (a *api) getDescription(c *models.ReqContext) response.Response { } type resourcePermissionDTO struct { - ID int64 `json:"id"` - RoleName string `json:"roleName"` - IsManaged bool `json:"isManaged"` - UserID int64 `json:"userId,omitempty"` - UserLogin string `json:"userLogin,omitempty"` - UserAvatarUrl string `json:"userAvatarUrl,omitempty"` - Team string `json:"team,omitempty"` - TeamID int64 `json:"teamId,omitempty"` - TeamAvatarUrl string `json:"teamAvatarUrl,omitempty"` - BuiltInRole string `json:"builtInRole,omitempty"` - Actions []string `json:"actions"` - Permission string `json:"permission"` + ID int64 `json:"id"` + RoleName string `json:"roleName"` + IsManaged bool `json:"isManaged"` + UserID int64 `json:"userId,omitempty"` + UserLogin string `json:"userLogin,omitempty"` + UserAvatarUrl string `json:"userAvatarUrl,omitempty"` + UserIsServiceAccount bool `json:"userIsServiceAccount,omitempty"` + Team string `json:"team,omitempty"` + TeamID int64 `json:"teamId,omitempty"` + TeamAvatarUrl string `json:"teamAvatarUrl,omitempty"` + BuiltInRole string `json:"builtInRole,omitempty"` + Actions []string `json:"actions"` + Permission string `json:"permission"` } func (a *api) getPermissions(c *models.ReqContext) response.Response { @@ -110,18 +112,19 @@ func (a *api) getPermissions(c *models.ReqContext) response.Response { } dto = append(dto, resourcePermissionDTO{ - ID: p.ID, - RoleName: p.RoleName, - UserID: p.UserId, - UserLogin: p.UserLogin, - UserAvatarUrl: dtos.GetGravatarUrl(p.UserEmail), - Team: p.Team, - TeamID: p.TeamId, - TeamAvatarUrl: teamAvatarUrl, - BuiltInRole: p.BuiltInRole, - Actions: p.Actions, - Permission: permission, - IsManaged: p.IsManaged, + ID: p.ID, + RoleName: p.RoleName, + UserID: p.UserId, + UserLogin: p.UserLogin, + UserAvatarUrl: dtos.GetGravatarUrl(p.UserEmail), + UserIsServiceAccount: p.UserIsServiceAccount, + Team: p.Team, + TeamID: p.TeamId, + TeamAvatarUrl: teamAvatarUrl, + BuiltInRole: p.BuiltInRole, + Actions: p.Actions, + Permission: permission, + IsManaged: p.IsManaged, }) } } diff --git a/pkg/services/accesscontrol/resourcepermissions/api_test.go b/pkg/services/accesscontrol/resourcepermissions/api_test.go index 2f32ae41b81..2178aa8d0fe 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_test.go @@ -18,6 +18,7 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/contexthandler/ctxkey" + "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" @@ -159,12 +160,12 @@ func TestApi_getPermissions(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - service, sql := setupTestEnvironment(t, tt.permissions, testOptions) + service, sql := setupTestEnvironment(t, tt.permissions, testOptionsDashboards) server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service) seedPermissions(t, tt.resourceID, sql, service) - permissions, recorder := getPermission(t, server, testOptions.Resource, tt.resourceID) + permissions, recorder := getPermission(t, server, testOptionsDashboards.Resource, tt.resourceID) assert.Equal(t, tt.expectedStatus, recorder.Code) if tt.expectedStatus == http.StatusOK { @@ -236,14 +237,14 @@ func TestApi_setBuiltinRolePermission(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - service, _ := setupTestEnvironment(t, tt.permissions, testOptions) + service, _ := setupTestEnvironment(t, tt.permissions, testOptionsDashboards) server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service) - recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "builtInRoles", tt.builtInRole) + recorder := setPermission(t, server, testOptionsDashboards.Resource, tt.resourceID, tt.permission, "builtInRoles", tt.builtInRole) assert.Equal(t, tt.expectedStatus, recorder.Code) if tt.expectedStatus == http.StatusOK { - permissions, _ := getPermission(t, server, testOptions.Resource, tt.resourceID) + permissions, _ := getPermission(t, server, testOptionsDashboards.Resource, tt.resourceID) require.Len(t, permissions, 1) assert.Equal(t, tt.permission, permissions[0].Permission) assert.Equal(t, tt.builtInRole, permissions[0].BuiltInRole) @@ -314,19 +315,19 @@ func TestApi_setTeamPermission(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - service, sql := setupTestEnvironment(t, tt.permissions, testOptions) + service, sql := setupTestEnvironment(t, tt.permissions, testOptionsDashboards) server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service) // seed team _, err := sql.CreateTeam("test", "test@test.com", 1) require.NoError(t, err) - recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "teams", strconv.Itoa(int(tt.teamID))) + recorder := setPermission(t, server, testOptionsDashboards.Resource, tt.resourceID, tt.permission, "teams", strconv.Itoa(int(tt.teamID))) assert.Equal(t, tt.expectedStatus, recorder.Code) assert.Equal(t, tt.expectedStatus, recorder.Code) if tt.expectedStatus == http.StatusOK { - permissions, _ := getPermission(t, server, testOptions.Resource, tt.resourceID) + permissions, _ := getPermission(t, server, testOptionsDashboards.Resource, tt.resourceID) require.Len(t, permissions, 1) assert.Equal(t, tt.permission, permissions[0].Permission) assert.Equal(t, tt.teamID, permissions[0].TeamID) @@ -397,19 +398,18 @@ func TestApi_setUserPermission(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - service, sql := setupTestEnvironment(t, tt.permissions, testOptions) + service, sql := setupTestEnvironment(t, tt.permissions, testOptionsDashboards) server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service) // seed user _, err := sql.CreateUser(context.Background(), user.CreateUserCommand{Login: "test", OrgID: 1}) require.NoError(t, err) - recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "users", strconv.Itoa(int(tt.userID))) + recorder := setPermission(t, server, testOptionsDashboards.Resource, tt.resourceID, tt.permission, "users", strconv.Itoa(int(tt.userID))) assert.Equal(t, tt.expectedStatus, recorder.Code) - assert.Equal(t, tt.expectedStatus, recorder.Code) if tt.expectedStatus == http.StatusOK { - permissions, _ := getPermission(t, server, testOptions.Resource, tt.resourceID) + permissions, _ := getPermission(t, server, testOptionsDashboards.Resource, tt.resourceID) require.Len(t, permissions, 1) assert.Equal(t, tt.permission, permissions[0].Permission) assert.Equal(t, tt.userID, permissions[0].UserID) @@ -418,6 +418,92 @@ func TestApi_setUserPermission(t *testing.T) { } } +type setServiceAccountPermissionTestCase struct { + desc string + serviceaccountID int64 + resourceID string + expectedStatus int + permission string + permissions []accesscontrol.Permission +} + +func TestApi_setServiceAccountPermission(t *testing.T) { + tests := []setServiceAccountPermissionTestCase{ + { + desc: "should set Edit permission for serviceaccount 1", + serviceaccountID: 1, + resourceID: "1", + expectedStatus: 200, + permission: "Edit", + permissions: []accesscontrol.Permission{ + {Action: "teams.permissions:read", Scope: "teams:id:1"}, + {Action: "teams.permissions:write", Scope: "teams:id:1"}, + {Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll}, + {Action: serviceaccounts.ActionRead, Scope: serviceaccounts.ScopeAll}, + }, + }, + { + desc: "should set View permission for serviceaccount 1", + serviceaccountID: 1, + resourceID: "1", + expectedStatus: 200, + permission: "View", + permissions: []accesscontrol.Permission{ + {Action: "teams.permissions:read", Scope: "teams:id:1"}, + {Action: "teams.permissions:write", Scope: "teams:id:1"}, + {Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll}, + {Action: serviceaccounts.ActionRead, Scope: serviceaccounts.ScopeAll}, + }, + }, + { + desc: "should set return http 400 when serviceaccount does not exist", + serviceaccountID: 2, + resourceID: "1", + expectedStatus: http.StatusBadRequest, + permission: "View", + permissions: []accesscontrol.Permission{ + {Action: "teams.permissions:read", Scope: "teams:id:1"}, + {Action: "teams.permissions:write", Scope: "teams:id:1"}, + {Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll}, + {Action: serviceaccounts.ActionRead, Scope: serviceaccounts.ScopeAll}, + }, + }, + { + desc: "should return http 403 when missing permissions", + serviceaccountID: 1, + resourceID: "1", + expectedStatus: http.StatusForbidden, + permission: "View", + permissions: []accesscontrol.Permission{ + {Action: "teams.permissions:read", Scope: "teams:id:1"}, + {Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + service, sql := setupTestEnvironment(t, tt.permissions, testOptionsTeams) + server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service) + + // seed serviceaccount + _, err := sql.CreateUser(context.Background(), user.CreateUserCommand{Login: "test", OrgID: 1, IsServiceAccount: true}) + require.NoError(t, err) + + recorder := setPermission(t, server, testOptionsTeams.Resource, tt.resourceID, tt.permission, "users", strconv.Itoa(int(tt.serviceaccountID))) + assert.Equal(t, tt.expectedStatus, recorder.Code) + + if tt.expectedStatus == http.StatusOK { + permissions, _ := getPermission(t, server, testOptionsTeams.Resource, tt.resourceID) + require.Len(t, permissions, 1) + assert.Equal(t, tt.permission, permissions[0].Permission) + assert.Equal(t, tt.serviceaccountID, permissions[0].UserID) + assert.Equal(t, true, permissions[0].UserIsServiceAccount) + } + }) + } +} + func setupTestServer(t *testing.T, user *models.SignedInUser, service *Service) *web.Mux { server := web.New() server.UseMiddleware(web.Renderer(path.Join(setting.StaticRootPath, "views"), "[[", "]]")) @@ -444,13 +530,14 @@ func contextProvider(tc *testContext) web.Handler { } } -var testOptions = Options{ +var testOptionsDashboards = Options{ Resource: "dashboards", ResourceAttribute: "id", Assignments: Assignments{ - Users: true, - Teams: true, - BuiltInRoles: true, + Users: true, + Teams: true, + BuiltInRoles: true, + ServiceAccounts: true, }, PermissionsToActions: map[string][]string{ "View": {"dashboards:read"}, @@ -458,6 +545,21 @@ var testOptions = Options{ }, } +var testOptionsTeams = Options{ + Resource: "teams", + ResourceAttribute: "id", + Assignments: Assignments{ + Users: true, + Teams: true, + BuiltInRoles: true, + ServiceAccounts: true, + }, + PermissionsToActions: map[string][]string{ + "View": {"teams:read"}, + "Edit": {"teams:read", "teams:write", "teams:delete"}, + }, +} + func getPermission(t *testing.T, server *web.Mux, resource, resourceID string) ([]resourcePermissionDTO, *httptest.ResponseRecorder) { req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/api/access-control/%s/%s", resource, resourceID), nil) require.NoError(t, err) diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index d8ae4903963..2722dba149a 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -275,7 +275,7 @@ func (s *Service) validateResource(ctx context.Context, orgID int64, resourceID } func (s *Service) validateUser(ctx context.Context, orgID, userID int64) error { - if !s.options.Assignments.Users { + if !(s.options.Assignments.Users || s.options.Assignments.ServiceAccounts) { return ErrInvalidAssignment } diff --git a/pkg/services/accesscontrol/resourcepermissions/service_test.go b/pkg/services/accesscontrol/resourcepermissions/service_test.go index 8775502ab99..be8a18c5db8 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/service_test.go @@ -38,7 +38,7 @@ func TestService_SetUserPermission(t *testing.T) { t.Run(tt.desc, func(t *testing.T) { service, sql := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{ Resource: "dashboards", - Assignments: Assignments{Users: true}, + Assignments: Assignments{Users: true, ServiceAccounts: true}, PermissionsToActions: nil, }) @@ -159,9 +159,10 @@ func TestService_SetPermissions(t *testing.T) { options: Options{ Resource: "dashboards", Assignments: Assignments{ - Users: true, - Teams: true, - BuiltInRoles: true, + Users: true, + Teams: true, + BuiltInRoles: true, + ServiceAccounts: true, }, PermissionsToActions: map[string][]string{ "View": {"dashboards:read"}, @@ -178,9 +179,10 @@ func TestService_SetPermissions(t *testing.T) { options: Options{ Resource: "dashboards", Assignments: Assignments{ - Users: true, - Teams: true, - BuiltInRoles: true, + Users: true, + Teams: true, + BuiltInRoles: true, + ServiceAccounts: true, }, PermissionsToActions: map[string][]string{ "View": {"dashboards:read"}, diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index b303b031a51..dce533502c5 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/models" ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/serviceaccounts" ) type TeamStore interface { @@ -528,17 +529,25 @@ func (ss *SQLStore) GetTeamMembers(ctx context.Context, query *models.GetTeamMem // If the signed in user is not set no member will be returned if !ac.IsDisabled(ss.Cfg) { sqlID := fmt.Sprintf("%s.%s", ss.engine.Dialect().Quote("user"), ss.engine.Dialect().Quote("id")) + var filter ac.SQLFilter *acFilter, err = ac.Filter(query.SignedInUser, sqlID, "users:id:", ac.ActionOrgUsersRead) if err != nil { return err } + + filter, err = ac.Filter(query.SignedInUser, sqlID, "serviceaccounts:id:", serviceaccounts.ActionRead) + if err != nil { + return err + } + acFilter.Where = fmt.Sprintf("(%s OR %s)", acFilter.Where, filter.Where) + acFilter.Args = append(acFilter.Args, filter.Args...) } return ss.getTeamMembers(ctx, query, acFilter) } // getTeamMembers return a list of members for the specified team -func (ss *SQLStore) getTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery, acUserFilter *ac.SQLFilter) error { +func (ss *SQLStore) getTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery, acFilter *ac.SQLFilter) error { return ss.WithDbSession(ctx, func(dbSess *DBSession) error { query.Result = make([]*models.TeamMemberDTO, 0) sess := dbSess.Table("team_member") @@ -546,11 +555,8 @@ func (ss *SQLStore) getTeamMembers(ctx context.Context, query *models.GetTeamMem fmt.Sprintf("team_member.user_id=%s.%s", ss.Dialect.Quote("user"), ss.Dialect.Quote("id")), ) - // explicitly check for serviceaccounts - sess.Where(fmt.Sprintf("%s.is_service_account=?", ss.Dialect.Quote("user")), ss.Dialect.BooleanStr(false)) - - if acUserFilter != nil { - sess.Where(acUserFilter.Where, acUserFilter.Args...) + if acFilter != nil { + sess.Where(acFilter.Where, acFilter.Args...) } // Join with only most recent auth module diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index 845b0fddf3d..2726ad0e2ef 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -24,9 +24,8 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { OrgId: 1, Permissions: map[int64]map[string][]string{ 1: { - ac.ActionTeamsRead: []string{ac.ScopeTeamsAll}, - ac.ActionOrgUsersRead: []string{ac.ScopeUsersAll}, - serviceaccounts.ActionRead: []string{serviceaccounts.ScopeAll}, + ac.ActionTeamsRead: []string{ac.ScopeTeamsAll}, + ac.ActionOrgUsersRead: []string{ac.ScopeUsersAll}, }, }, } @@ -373,9 +372,21 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { require.EqualValues(t, getTeamQuery.Result.MemberCount, 2) }) - t.Run("Should be able to exclude service accounts from teamembers", func(t *testing.T) { + t.Run("Should be able to add service accounts to teams, list it as a team member and remove it from team", func(t *testing.T) { sqlStore = InitTestDB(t) setup() + + signedInUser := &models.SignedInUser{ + Login: "loginuser0", + OrgId: testOrgID, + Permissions: map[int64]map[string][]string{ + testOrgID: { + ac.ActionTeamsRead: []string{ac.ScopeTeamsAll}, + ac.ActionOrgUsersRead: []string{ac.ScopeUsersAll}, + }, + }, + } + userCmd = user.CreateUserCommand{ Email: fmt.Sprint("sa", 1, "@test.com"), Name: fmt.Sprint("sa", 1), @@ -385,24 +396,23 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { serviceAccount, err := sqlStore.CreateUser(context.Background(), userCmd) require.NoError(t, err) - groupId := team2.Id - // add service account to team - err = sqlStore.AddTeamMember(serviceAccount.ID, testOrgID, groupId, false, 0) + teamId := team1.Id + err = sqlStore.AddTeamMember(serviceAccount.ID, testOrgID, teamId, false, 0) require.NoError(t, err) - // add user to team - err = sqlStore.AddTeamMember(userIds[0], testOrgID, groupId, false, 0) + getTeamMembersQuery := &models.GetTeamMembersQuery{OrgId: testOrgID, TeamId: teamId, SignedInUser: signedInUser} + err = sqlStore.GetTeamMembers(context.Background(), getTeamMembersQuery) + require.NoError(t, err) + require.EqualValues(t, 1, len(getTeamMembersQuery.Result)) + require.EqualValues(t, serviceAccount.ID, getTeamMembersQuery.Result[0].UserId) + + removeTeamMemberCmd := &models.RemoveTeamMemberCommand{OrgId: testOrgID, TeamId: teamId, UserId: serviceAccount.ID} + err = sqlStore.RemoveTeamMember(context.Background(), removeTeamMemberCmd) require.NoError(t, err) - teamMembersQuery := &models.GetTeamMembersQuery{ - OrgId: testOrgID, - SignedInUser: testUser, - TeamId: groupId, - } - err = sqlStore.GetTeamMembers(context.Background(), teamMembersQuery) + err = sqlStore.GetTeamMembers(context.Background(), getTeamMembersQuery) require.NoError(t, err) - // should not receive service account from query - require.Equal(t, len(teamMembersQuery.Result), 1) + require.EqualValues(t, 0, len(getTeamMembersQuery.Result)) }) }) }) @@ -489,7 +499,7 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { t.Skip("skipping integration test") } testOrgID := int64(2) - userIds := make([]int64, 4) + userIds := make([]int64, 5) // Seed 2 teams with 2 members setup := func(store *SQLStore) { @@ -498,12 +508,15 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { team2, errCreateTeam := store.CreateTeam("group2 name", "test2@example.org", testOrgID) require.NoError(t, errCreateTeam) - for i := 0; i < 4; i++ { + for i := 0; i < 5; i++ { userCmd := user.CreateUserCommand{ Email: fmt.Sprint("user", i, "@example.org"), Name: fmt.Sprint("user", i), Login: fmt.Sprint("loginuser", i), } + if i >= 3 { + userCmd.IsServiceAccount = true + } user, errCreateUser := store.CreateUser(context.Background(), userCmd) require.NoError(t, errCreateUser) userIds[i] = user.ID @@ -517,6 +530,8 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { require.NoError(t, errAddMember) errAddMember = store.AddTeamMember(userIds[3], testOrgID, team2.Id, false, 0) require.NoError(t, errAddMember) + errAddMember = store.AddTeamMember(userIds[4], testOrgID, team2.Id, false, 0) + require.NoError(t, errAddMember) } store := InitTestDB(t, InitTestDBOpt{}) @@ -534,11 +549,14 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { query: &models.GetTeamMembersQuery{ OrgId: testOrgID, SignedInUser: &models.SignedInUser{ - OrgId: testOrgID, - Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: {ac.ScopeUsersAll}}}, + OrgId: testOrgID, + Permissions: map[int64]map[string][]string{testOrgID: { + ac.ActionOrgUsersRead: {ac.ScopeUsersAll}, + serviceaccounts.ActionRead: {serviceaccounts.ScopeAll}, + }}, }, }, - expectedNumUsers: 4, + expectedNumUsers: 5, }, { desc: "should return no team members", @@ -558,11 +576,15 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { OrgId: testOrgID, SignedInUser: &models.SignedInUser{ OrgId: testOrgID, - Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: { - ac.Scope("users", "id", fmt.Sprintf("%d", userIds[0])), - ac.Scope("users", "id", fmt.Sprintf("%d", userIds[2])), - ac.Scope("users", "id", fmt.Sprintf("%d", userIds[3])), - }}}, + Permissions: map[int64]map[string][]string{testOrgID: { + ac.ActionOrgUsersRead: { + ac.Scope("users", "id", fmt.Sprintf("%d", userIds[0])), + ac.Scope("users", "id", fmt.Sprintf("%d", userIds[2])), + }, + serviceaccounts.ActionRead: { + ac.Scope("serviceaccounts", "id", fmt.Sprintf("%d", userIds[3])), + }, + }}, }, }, expectedNumUsers: 3, @@ -576,10 +598,20 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { if !hasWildcardScope(tt.query.SignedInUser, ac.ActionOrgUsersRead) { for _, member := range tt.query.Result { - assert.Contains(t, - tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgId][ac.ActionOrgUsersRead], - ac.Scope("users", "id", fmt.Sprintf("%d", member.UserId)), - ) + hasPermission := false + for _, scope := range tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgId][ac.ActionOrgUsersRead] { + if scope == ac.Scope("users", "id", fmt.Sprintf("%d", member.UserId)) { + hasPermission = true + break + } + } + for _, scope := range tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgId][serviceaccounts.ActionRead] { + if scope == ac.Scope("serviceaccounts", "id", fmt.Sprintf("%d", member.UserId)) { + hasPermission = true + break + } + } + assert.True(t, hasPermission) } } }) diff --git a/public/app/core/components/AccessControl/AddPermission.tsx b/public/app/core/components/AccessControl/AddPermission.tsx index 8c17862ec7a..9785b75776a 100644 --- a/public/app/core/components/AccessControl/AddPermission.tsx +++ b/public/app/core/components/AccessControl/AddPermission.tsx @@ -2,6 +2,7 @@ import React, { useEffect, useMemo, useState } from 'react'; import { Button, Form, HorizontalGroup, Select } from '@grafana/ui'; import { CloseButton } from 'app/core/components/CloseButton/CloseButton'; +import { ServiceAccountPicker } from 'app/core/components/Select/ServiceAccountPicker'; import { TeamPicker } from 'app/core/components/Select/TeamPicker'; import { UserPicker } from 'app/core/components/Select/UserPicker'; import { OrgRole } from 'app/types/acl'; @@ -28,6 +29,9 @@ export const AddPermission = ({ title = 'Add Permission For', permissions, assig if (assignments.users) { options.push({ value: PermissionTarget.User, label: 'User' }); } + if (assignments.serviceAccounts) { + options.push({ value: PermissionTarget.ServiceAccount, label: 'Service Account' }); + } if (assignments.teams) { options.push({ value: PermissionTarget.Team, label: 'Team' }); } @@ -46,6 +50,7 @@ export const AddPermission = ({ title = 'Add Permission For', permissions, assig const isValid = () => (target === PermissionTarget.Team && teamId > 0) || (target === PermissionTarget.User && userId > 0) || + (target === PermissionTarget.ServiceAccount && userId > 0) || (PermissionTarget.BuiltInRole && OrgRole.hasOwnProperty(builtInRole)); return ( @@ -72,6 +77,10 @@ export const AddPermission = ({ title = 'Add Permission For', permissions, assig setUserId(u.value || 0)} className={'width-20'} /> )} + {target === PermissionTarget.ServiceAccount && ( + setUserId(s.value?.id || 0)} className={'width-20'} /> + )} + {target === PermissionTarget.Team && ( setTeamId(t.value?.id || 0)} className={'width-20'} /> )} diff --git a/public/app/core/components/AccessControl/Permissions.tsx b/public/app/core/components/AccessControl/Permissions.tsx index 39446dedd47..0e8b9955ef1 100644 --- a/public/app/core/components/AccessControl/Permissions.tsx +++ b/public/app/core/components/AccessControl/Permissions.tsx @@ -16,6 +16,7 @@ const INITIAL_DESCRIPTION: Description = { assignments: { teams: false, users: false, + serviceAccounts: false, builtInRoles: false, }, }; @@ -57,7 +58,7 @@ export const Permissions = ({ const onAdd = (state: SetPermission) => { let promise: Promise | null = null; - if (state.target === PermissionTarget.User) { + if (state.target === PermissionTarget.User || state.target === PermissionTarget.ServiceAccount) { promise = setUserPermission(resource, resourceId, state.userId!, state.permission); } else if (state.target === PermissionTarget.Team) { promise = setTeamPermission(resource, resourceId, state.teamId!, state.permission); @@ -109,7 +110,15 @@ export const Permissions = ({ const users = useMemo( () => sortBy( - items.filter((i) => i.userId), + items.filter((i) => i.userId && !i.userIsServiceAccount), + ['userLogin'] + ), + [items] + ); + const serviceAccounts = useMemo( + () => + sortBy( + items.filter((i) => i.userId && i.userIsServiceAccount), ['userLogin'] ), [items] @@ -161,6 +170,14 @@ export const Permissions = ({ onRemove={onRemove} canSet={canSetPermissions} /> + ) => void; + className?: string; + inputId?: string; + autoFocus?: boolean; +} + +export function ServiceAccountPicker({ onSelected, className, inputId, autoFocus }: Props) { + const [loadingServiceAccounts, setLoadingServiceAccounts] = useState(false); + + // For whatever reason the autoFocus prop doesn't seem to work + // with AsyncSelect, hence this workaround. Maybe fixed in a later version? + useEffect(() => { + if (autoFocus && inputId) { + document.getElementById(inputId)?.focus(); + } + }, [autoFocus, inputId]); + + let search = (query?: string) => { + setLoadingServiceAccounts(true); + + if (isNil(query)) { + query = ''; + } + + return getBackendSrv() + .get(`/api/serviceaccounts/search?query=${query}`) + .then((result: { serviceAccounts: ServiceAccount[] }) => { + const serviceAccounts: Array> = result.serviceAccounts.map((serviceAccount) => { + return { + id: serviceAccount.id, + value: serviceAccount, + label: serviceAccount.login, + imgUrl: serviceAccount.avatarUrl, + login: serviceAccount.login, + }; + }); + setLoadingServiceAccounts(false); + return serviceAccounts; + }); + }; + + const debouncedSearch = debounce(search, 300, { + leading: true, + trailing: true, + }); + + return ( + + ); +}