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 700aaf98114..ccb0ffebc7c 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 @@ -78,7 +78,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`
`serviceaccounts:read` | Create a team and list organization users and service accounts (required to manage the created team). | +| `fixed:teams:creator` | `teams:create`
`org.users:read` | Create a team and list organization users (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 799f83e22fd..e7716d7e2b2 100644 --- a/pkg/api/accesscontrol.go +++ b/pkg/api/accesscontrol.go @@ -230,12 +230,11 @@ func (hs *HTTPServer) declareFixedRoles() error { Role: ac.RoleDTO{ Name: "fixed:teams:creator", DisplayName: "Team creator", - Description: "Create teams and read organisation users and service accounts (required to manage the created teams).", + Description: "Create teams and read organisation users (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 38b6a8e474a..69034277678 100644 --- a/pkg/services/accesscontrol/database/resource_permissions.go +++ b/pkg/services/accesscontrol/database/resource_permissions.go @@ -9,26 +9,24 @@ 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" "github.com/grafana/grafana/pkg/services/user" ) type flatResourcePermission struct { - 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 + 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 } func (p *flatResourcePermission) IsManaged(scope string) bool { @@ -295,7 +293,6 @@ 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, @@ -306,7 +303,6 @@ 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, @@ -317,7 +313,6 @@ 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, @@ -376,13 +371,8 @@ func (s *AccessControlStore) getResourcePermissions(sess *sqlstore.DBSession, or if err != nil { return nil, err } - 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 + ") " + user := userSelect + userFrom + where + " AND " + userFilter.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 { @@ -468,21 +458,20 @@ 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, - 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), + 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), } } @@ -593,7 +582,6 @@ 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 85ef4f1188b..0317ea73b52 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -222,21 +222,20 @@ 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 - UserIsServiceAccount bool - 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 + 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 8bc62f0e590..d33ab1a4c39 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -59,10 +59,9 @@ func ProvideTeamPermissions( return nil }, Assignments: resourcepermissions.Assignments{ - Users: true, - Teams: false, - BuiltInRoles: false, - ServiceAccounts: true, + Users: true, + Teams: false, + BuiltInRoles: false, }, PermissionsToActions: map[string][]string{ "Member": TeamMemberActions, @@ -151,10 +150,9 @@ func ProvideDashboardPermissions( return []string{}, nil }, Assignments: resourcepermissions.Assignments{ - Users: true, - Teams: true, - BuiltInRoles: true, - ServiceAccounts: false, + Users: true, + Teams: true, + BuiltInRoles: true, }, PermissionsToActions: map[string][]string{ "View": DashboardViewActions, @@ -209,10 +207,9 @@ func ProvideFolderPermissions( return nil }, Assignments: resourcepermissions.Assignments{ - Users: true, - Teams: true, - BuiltInRoles: true, - ServiceAccounts: false, + Users: true, + Teams: true, + BuiltInRoles: true, }, PermissionsToActions: map[string][]string{ "View": append(DashboardViewActions, FolderViewActions...), @@ -283,10 +280,9 @@ func ProvideServiceAccountPermissions( return err }, Assignments: resourcepermissions.Assignments{ - Users: true, - Teams: true, - BuiltInRoles: false, - ServiceAccounts: false, + Users: true, + Teams: true, + BuiltInRoles: false, }, PermissionsToActions: map[string][]string{ "Edit": {serviceaccounts.ActionRead, serviceaccounts.ActionWrite}, diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index c4d8d963ef8..a1892130cfd 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 || a.service.options.Assignments.ServiceAccounts { + if a.service.options.Assignments.Users { r.Post("/:resourceID/users/:userID", inheritanceSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setUserPermission)) } if a.service.options.Assignments.Teams { @@ -53,10 +53,9 @@ func (a *api) registerEndpoints() { } type Assignments struct { - Users bool `json:"users"` - Teams bool `json:"teams"` - BuiltInRoles bool `json:"builtInRoles"` - ServiceAccounts bool `json:"serviceAccounts"` + Users bool `json:"users"` + Teams bool `json:"teams"` + BuiltInRoles bool `json:"builtInRoles"` } type Description struct { @@ -72,19 +71,18 @@ 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"` - 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"` + 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"` } func (a *api) getPermissions(c *models.ReqContext) response.Response { @@ -112,19 +110,18 @@ 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), - UserIsServiceAccount: p.UserIsServiceAccount, - 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), + 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 2178aa8d0fe..2f32ae41b81 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_test.go @@ -18,7 +18,6 @@ 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" @@ -160,12 +159,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, testOptionsDashboards) + service, sql := setupTestEnvironment(t, tt.permissions, testOptions) 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, testOptionsDashboards.Resource, tt.resourceID) + permissions, recorder := getPermission(t, server, testOptions.Resource, tt.resourceID) assert.Equal(t, tt.expectedStatus, recorder.Code) if tt.expectedStatus == http.StatusOK { @@ -237,14 +236,14 @@ func TestApi_setBuiltinRolePermission(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - service, _ := setupTestEnvironment(t, tt.permissions, testOptionsDashboards) + service, _ := setupTestEnvironment(t, tt.permissions, testOptions) server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service) - recorder := setPermission(t, server, testOptionsDashboards.Resource, tt.resourceID, tt.permission, "builtInRoles", tt.builtInRole) + recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "builtInRoles", tt.builtInRole) assert.Equal(t, tt.expectedStatus, recorder.Code) if tt.expectedStatus == http.StatusOK { - permissions, _ := getPermission(t, server, testOptionsDashboards.Resource, tt.resourceID) + permissions, _ := getPermission(t, server, testOptions.Resource, tt.resourceID) require.Len(t, permissions, 1) assert.Equal(t, tt.permission, permissions[0].Permission) assert.Equal(t, tt.builtInRole, permissions[0].BuiltInRole) @@ -315,19 +314,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, testOptionsDashboards) + service, sql := setupTestEnvironment(t, tt.permissions, testOptions) 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, testOptionsDashboards.Resource, tt.resourceID, tt.permission, "teams", strconv.Itoa(int(tt.teamID))) + recorder := setPermission(t, server, testOptions.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, testOptionsDashboards.Resource, tt.resourceID) + permissions, _ := getPermission(t, server, testOptions.Resource, tt.resourceID) require.Len(t, permissions, 1) assert.Equal(t, tt.permission, permissions[0].Permission) assert.Equal(t, tt.teamID, permissions[0].TeamID) @@ -398,18 +397,19 @@ func TestApi_setUserPermission(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - service, sql := setupTestEnvironment(t, tt.permissions, testOptionsDashboards) + service, sql := setupTestEnvironment(t, tt.permissions, testOptions) 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, testOptionsDashboards.Resource, tt.resourceID, tt.permission, "users", strconv.Itoa(int(tt.userID))) + recorder := setPermission(t, server, testOptions.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, testOptionsDashboards.Resource, tt.resourceID) + permissions, _ := getPermission(t, server, testOptions.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,92 +418,6 @@ 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"), "[[", "]]")) @@ -530,14 +444,13 @@ func contextProvider(tc *testContext) web.Handler { } } -var testOptionsDashboards = Options{ +var testOptions = Options{ Resource: "dashboards", ResourceAttribute: "id", Assignments: Assignments{ - Users: true, - Teams: true, - BuiltInRoles: true, - ServiceAccounts: true, + Users: true, + Teams: true, + BuiltInRoles: true, }, PermissionsToActions: map[string][]string{ "View": {"dashboards:read"}, @@ -545,21 +458,6 @@ var testOptionsDashboards = 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 2722dba149a..d8ae4903963 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 || s.options.Assignments.ServiceAccounts) { + if !s.options.Assignments.Users { return ErrInvalidAssignment } diff --git a/pkg/services/accesscontrol/resourcepermissions/service_test.go b/pkg/services/accesscontrol/resourcepermissions/service_test.go index be8a18c5db8..8775502ab99 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, ServiceAccounts: true}, + Assignments: Assignments{Users: true}, PermissionsToActions: nil, }) @@ -159,10 +159,9 @@ func TestService_SetPermissions(t *testing.T) { options: Options{ Resource: "dashboards", Assignments: Assignments{ - Users: true, - Teams: true, - BuiltInRoles: true, - ServiceAccounts: true, + Users: true, + Teams: true, + BuiltInRoles: true, }, PermissionsToActions: map[string][]string{ "View": {"dashboards:read"}, @@ -179,10 +178,9 @@ func TestService_SetPermissions(t *testing.T) { options: Options{ Resource: "dashboards", Assignments: Assignments{ - Users: true, - Teams: true, - BuiltInRoles: true, - ServiceAccounts: true, + Users: true, + Teams: true, + BuiltInRoles: true, }, PermissionsToActions: map[string][]string{ "View": {"dashboards:read"}, diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index 7bbe322cc2b..8e136eabdd5 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -177,10 +177,12 @@ func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) { assert.Equal(t, tc.body["name"], sa.Name) assert.Equal(t, tc.wantID, sa.Login) tempUser := &models.SignedInUser{ - OrgId: 1, + OrgId: 1, + UserId: 1, Permissions: map[int64]map[string][]string{ 1: { - serviceaccounts.ActionRead: []string{serviceaccounts.ScopeAll}, + serviceaccounts.ActionRead: []string{serviceaccounts.ScopeAll}, + accesscontrol.ActionOrgUsersRead: []string{accesscontrol.ScopeUsersAll}, }, }, } diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index dce533502c5..b303b031a51 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -9,7 +9,6 @@ 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 { @@ -529,25 +528,17 @@ 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, acFilter *ac.SQLFilter) error { +func (ss *SQLStore) getTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery, acUserFilter *ac.SQLFilter) error { return ss.WithDbSession(ctx, func(dbSess *DBSession) error { query.Result = make([]*models.TeamMemberDTO, 0) sess := dbSess.Table("team_member") @@ -555,8 +546,11 @@ 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")), ) - if acFilter != nil { - sess.Where(acFilter.Where, acFilter.Args...) + // 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...) } // Join with only most recent auth module diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index b84383c3359..04c3c80dfc6 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -24,8 +24,9 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { OrgId: 1, Permissions: map[int64]map[string][]string{ 1: { - ac.ActionTeamsRead: []string{ac.ScopeTeamsAll}, - ac.ActionOrgUsersRead: []string{ac.ScopeUsersAll}, + ac.ActionTeamsRead: []string{ac.ScopeTeamsAll}, + ac.ActionOrgUsersRead: []string{ac.ScopeUsersAll}, + serviceaccounts.ActionRead: []string{serviceaccounts.ScopeAll}, }, }, } @@ -372,21 +373,9 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { require.EqualValues(t, getTeamQuery.Result.MemberCount, 2) }) - 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) { + t.Run("Should be able to exclude service accounts from teamembers", 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), @@ -396,23 +385,24 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { serviceAccount, err := sqlStore.CreateUser(context.Background(), userCmd) require.NoError(t, err) - teamId := team1.Id - err = sqlStore.AddTeamMember(serviceAccount.ID, testOrgID, teamId, false, 0) + groupId := team2.Id + // add service account to team + err = sqlStore.AddTeamMember(serviceAccount.ID, testOrgID, groupId, false, 0) require.NoError(t, err) - 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) + // add user to team + err = sqlStore.AddTeamMember(userIds[0], testOrgID, groupId, false, 0) require.NoError(t, err) - err = sqlStore.GetTeamMembers(context.Background(), getTeamMembersQuery) + teamMembersQuery := &models.GetTeamMembersQuery{ + OrgId: testOrgID, + SignedInUser: testUser, + TeamId: groupId, + } + err = sqlStore.GetTeamMembers(context.Background(), teamMembersQuery) require.NoError(t, err) - require.EqualValues(t, 0, len(getTeamMembersQuery.Result)) + // should not receive service account from query + require.Equal(t, len(teamMembersQuery.Result), 1) }) }) }) @@ -499,7 +489,7 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { t.Skip("skipping integration test") } testOrgID := int64(2) - userIds := make([]int64, 5) + userIds := make([]int64, 4) // Seed 2 teams with 2 members setup := func(store *SQLStore) { @@ -508,15 +498,12 @@ 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 < 5; i++ { + for i := 0; i < 4; 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 @@ -530,8 +517,6 @@ 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{}) @@ -549,14 +534,11 @@ 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}, - serviceaccounts.ActionRead: {serviceaccounts.ScopeAll}, - }}, + OrgId: testOrgID, + Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: {ac.ScopeUsersAll}}}, }, }, - expectedNumUsers: 5, + expectedNumUsers: 4, }, { desc: "should return no team members", @@ -576,15 +558,11 @@ 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])), - }, - serviceaccounts.ActionRead: { - ac.Scope("serviceaccounts", "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])), + ac.Scope("users", "id", fmt.Sprintf("%d", userIds[3])), + }}}, }, }, expectedNumUsers: 3, @@ -598,20 +576,10 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { if !hasWildcardScope(tt.query.SignedInUser, ac.ActionOrgUsersRead) { for _, member := range tt.query.Result { - 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) + assert.Contains(t, + tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgId][ac.ActionOrgUsersRead], + ac.Scope("users", "id", fmt.Sprintf("%d", member.UserId)), + ) } } }) diff --git a/public/app/core/components/AccessControl/AddPermission.tsx b/public/app/core/components/AccessControl/AddPermission.tsx index 9785b75776a..8c17862ec7a 100644 --- a/public/app/core/components/AccessControl/AddPermission.tsx +++ b/public/app/core/components/AccessControl/AddPermission.tsx @@ -2,7 +2,6 @@ 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'; @@ -29,9 +28,6 @@ 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' }); } @@ -50,7 +46,6 @@ 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 ( @@ -77,10 +72,6 @@ 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 0e8b9955ef1..39446dedd47 100644 --- a/public/app/core/components/AccessControl/Permissions.tsx +++ b/public/app/core/components/AccessControl/Permissions.tsx @@ -16,7 +16,6 @@ const INITIAL_DESCRIPTION: Description = { assignments: { teams: false, users: false, - serviceAccounts: false, builtInRoles: false, }, }; @@ -58,7 +57,7 @@ export const Permissions = ({ const onAdd = (state: SetPermission) => { let promise: Promise | null = null; - if (state.target === PermissionTarget.User || state.target === PermissionTarget.ServiceAccount) { + if (state.target === PermissionTarget.User) { promise = setUserPermission(resource, resourceId, state.userId!, state.permission); } else if (state.target === PermissionTarget.Team) { promise = setTeamPermission(resource, resourceId, state.teamId!, state.permission); @@ -110,15 +109,7 @@ export const Permissions = ({ const users = useMemo( () => sortBy( - items.filter((i) => i.userId && !i.userIsServiceAccount), - ['userLogin'] - ), - [items] - ); - const serviceAccounts = useMemo( - () => - sortBy( - items.filter((i) => i.userId && i.userIsServiceAccount), + items.filter((i) => i.userId), ['userLogin'] ), [items] @@ -170,14 +161,6 @@ 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 ( - - ); -}