public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Service accounts: Add service account to teams" (#52710)

Browse Source 
* Revert "Service accounts: Add service account to teams (#51536)"

This reverts commit 0f919671e7.

* remove unneeded line

* fix test
This commit is contained in:
Ieva
2022-07-26 09:43:29 +01:00
committed by GitHub
parent 8f4d924531
commit b3a10202d4
16 changed files with 151 additions and 410 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/accesscontrol/database/resource_permissions.go
+28 -40
View File
@@ -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,
pkg/services/accesscontrol/models.go
+14 -15
View File
@@ -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 {
pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go
+12 -16
View File
@@ -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},
pkg/services/accesscontrol/resourcepermissions/api.go
+28 -31
View File
@@ -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,
})
}
}
pkg/services/accesscontrol/resourcepermissions/api_test.go
+16 -118
View File
@@ -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)
pkg/services/accesscontrol/resourcepermissions/service.go
+1 -1
View File
@@ -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
}
pkg/services/accesscontrol/resourcepermissions/service_test.go
+7 -9
View File
@@ -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"},