RBAC: remove simple RBAC disabled checks (#71137)

* remove simple RBAC disabled checks

* fixing tests

* remove old AC tests
This commit is contained in:
Ieva
2023-07-10 15:14:21 +03:00
committed by GitHub
parent 8f2f6d63eb
commit a65cb4d808
16 changed files with 64 additions and 720 deletions
+1 -11
View File
@@ -80,13 +80,8 @@ type Service struct {
}
func (s *Service) GetUsageStats(_ context.Context) map[string]interface{} {
enabled := 0
if !accesscontrol.IsDisabled(s.cfg) {
enabled = 1
}
return map[string]interface{}{
"stats.oss.accesscontrol.enabled.count": enabled,
"stats.oss.accesscontrol.enabled.count": 1,
}
}
@@ -165,11 +160,6 @@ func (s *Service) DeleteUserPermissions(ctx context.Context, orgID int64, userID
// DeclareFixedRoles allow the caller to declare, to the service, fixed roles and their assignments
// to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin"
func (s *Service) DeclareFixedRoles(registrations ...accesscontrol.RoleRegistration) error {
// If accesscontrol is disabled no need to register roles
if accesscontrol.IsDisabled(s.cfg) {
return nil
}
for _, r := range registrations {
err := accesscontrol.ValidateFixedRole(r.Role)
if err != nil {
@@ -45,11 +45,6 @@ func TestUsageMetrics(t *testing.T) {
enabled bool
expectedValue int
}{
{
name: "Expecting metric with value 0",
enabled: false,
expectedValue: 0,
},
{
name: "Expecting metric with value 1",
enabled: true,
-4
View File
@@ -239,10 +239,6 @@ func UseGlobalOrg(c *contextmodel.ReqContext) (int64, error) {
func LoadPermissionsMiddleware(service Service) web.Handler {
return func(c *contextmodel.ReqContext) {
if service.IsDisabled() {
return
}
permissions, err := service.GetUserPermissions(c.Req.Context(), c.SignedInUser,
Options{ReloadCache: false})
if err != nil {
+6 -8
View File
@@ -34,19 +34,17 @@ func (ss *sqlxStore) GetAPIKeys(ctx context.Context, query *apikey.GetApiKeysQue
where = append(where, "service_account_id IS NULL")
if !accesscontrol.IsDisabled(ss.cfg) {
filter, err := accesscontrol.Filter(query.User, "id", "apikeys:id:", accesscontrol.ActionAPIKeyRead)
if err != nil {
return nil, err
}
where = append(where, filter.Where)
args = append(args, filter.Args...)
filter, err := accesscontrol.Filter(query.User, "id", "apikeys:id:", accesscontrol.ActionAPIKeyRead)
if err != nil {
return nil, err
}
where = append(where, filter.Where)
args = append(args, filter.Args...)
ws := fmt.Sprint(strings.Join(where[:], " AND "))
qr := fmt.Sprintf(`SELECT * FROM api_key WHERE %s ORDER BY name ASC LIMIT 100`, ws)
keys := make([]*apikey.APIKey, 0)
err := ss.sess.Select(ctx, &keys, qr, args...)
err = ss.sess.Select(ctx, &keys, qr, args...)
return keys, err
}
+4 -6
View File
@@ -39,13 +39,11 @@ func (ss *sqlStore) GetAPIKeys(ctx context.Context, query *apikey.GetApiKeysQuer
sess = sess.Where("service_account_id IS NULL")
if !accesscontrol.IsDisabled(ss.cfg) {
filter, err := accesscontrol.Filter(query.User, "id", "apikeys:id:", accesscontrol.ActionAPIKeyRead)
if err != nil {
return err
}
sess.And(filter.Where, filter.Args...)
filter, err := accesscontrol.Filter(query.User, "id", "apikeys:id:", accesscontrol.ActionAPIKeyRead)
if err != nil {
return err
}
sess.And(filter.Where, filter.Args...)
res = make([]*apikey.APIKey, 0)
return sess.Find(&res)
@@ -26,7 +26,7 @@ type PermissionsSync struct {
}
func (s *PermissionsSync) SyncPermissionsHook(ctx context.Context, identity *authn.Identity, _ *authn.Request) error {
if s.ac.IsDisabled() || !identity.ClientParams.SyncPermissions {
if !identity.ClientParams.SyncPermissions {
return nil
}
@@ -17,56 +17,40 @@ func TestPermissionsSync_SyncPermission(t *testing.T) {
type testCase struct {
name string
identity *authn.Identity
rbacDisabled bool
expectedPermissions []accesscontrol.Permission
}
testCases := []testCase{
{
name: "enriches the identity successfully when SyncPermissions is true",
identity: &authn.Identity{ID: "user:2", OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}},
rbacDisabled: false,
name: "enriches the identity successfully when SyncPermissions is true",
identity: &authn.Identity{ID: "user:2", OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}},
expectedPermissions: []accesscontrol.Permission{
{Action: accesscontrol.ActionUsersRead},
},
},
{
name: "does not load the permissions when SyncPermissions is false",
identity: &authn.Identity{ID: "user:2", OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}},
rbacDisabled: false,
name: "does not load the permissions when SyncPermissions is false",
identity: &authn.Identity{ID: "user:2", OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}},
expectedPermissions: []accesscontrol.Permission{
{Action: accesscontrol.ActionUsersRead},
},
},
{
name: "does not load the permissions when RBAC is disabled",
rbacDisabled: true,
identity: &authn.Identity{ID: "user:2", OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}},
expectedPermissions: []accesscontrol.Permission{},
},
}
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
s := setupTestEnv(tt.rbacDisabled)
s := setupTestEnv()
err := s.SyncPermissionsHook(context.Background(), tt.identity, &authn.Request{})
require.NoError(t, err)
if !tt.rbacDisabled {
assert.Equal(t, 1, len(tt.identity.Permissions))
assert.Equal(t, accesscontrol.GroupScopesByAction(tt.expectedPermissions), tt.identity.Permissions[tt.identity.OrgID])
} else {
assert.Equal(t, 0, len(tt.identity.Permissions))
}
assert.Equal(t, 1, len(tt.identity.Permissions))
assert.Equal(t, accesscontrol.GroupScopesByAction(tt.expectedPermissions), tt.identity.Permissions[tt.identity.OrgID])
})
}
}
func setupTestEnv(rbacDisabled bool) *PermissionsSync {
func setupTestEnv() *PermissionsSync {
acMock := &acmock.Mock{
IsDisabledFunc: func() bool {
return rbacDisabled
},
GetUserPermissionsFunc: func(ctx context.Context, siu *user.SignedInUser, o accesscontrol.Options) ([]accesscontrol.Permission, error) {
return []accesscontrol.Permission{
{Action: accesscontrol.ActionUsersRead},
+13 -26
View File
@@ -250,7 +250,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
return nil, folder.ErrBadRequest.Errorf("missing signed in user")
}
if !s.accessControl.IsDisabled() && s.features.IsEnabled(featuremgmt.FlagNestedFolders) && cmd.ParentUID != "" {
if s.features.IsEnabled(featuremgmt.FlagNestedFolders) && cmd.ParentUID != "" {
// Check that the user is allowed to create a subfolder in this folder
evaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.ParentUID))
hasAccess, evalErr := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator)
@@ -549,32 +549,19 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
}
// Check that the user is allowed to move the folder to the destination folder
if !s.accessControl.IsDisabled() {
var evaluator accesscontrol.Evaluator
if cmd.NewParentUID != "" {
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.NewParentUID))
} else {
// Evaluate folder creation permission when moving folder to the root level
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)
}
hasAccess, evalErr := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator)
if evalErr != nil {
return nil, evalErr
}
if !hasAccess {
return nil, dashboards.ErrFolderAccessDenied
}
var evaluator accesscontrol.Evaluator
if cmd.NewParentUID != "" {
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.NewParentUID))
} else {
g, err := guardian.NewByUID(ctx, cmd.UID, cmd.OrgID, cmd.SignedInUser)
if err != nil {
return nil, err
}
if canSave, err := g.CanSave(); err != nil || !canSave {
if err != nil {
return nil, toFolderError(err)
}
return nil, dashboards.ErrFolderAccessDenied
}
// Evaluate folder creation permission when moving folder to the root level
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)
}
hasAccess, evalErr := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator)
if evalErr != nil {
return nil, evalErr
}
if !hasAccess {
return nil, dashboards.ErrFolderAccessDenied
}
// here we get the folder, we need to get the height of current folder
@@ -26,7 +26,7 @@ var userAdmin = &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleAdmin,
var userAdminRBAC = &user.SignedInUser{UserID: 2, OrgID: 1, OrgRole: org.RoleAdmin, Login: "testAdminUserRBAC", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsPublicWrite: {dashboards.ScopeDashboardsAll}}}}
var userViewer = &user.SignedInUser{UserID: 3, OrgID: 1, OrgRole: org.RoleViewer, Login: "testViewerUser"}
var userViewerRBAC = &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleViewer, Login: "testViewerUserRBAC", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}}}}
var anonymousUser *user.SignedInUser
var anonymousUser = &user.SignedInUser{IsAnonymous: true}
type JsonErrResponse struct {
Error string `json:"error"`
@@ -84,7 +84,7 @@ type testContext struct {
func contextProvider(tc *testContext) web.Handler {
return func(c *web.Context) {
signedIn := tc.user != nil
signedIn := tc.user != nil && !tc.user.IsAnonymous
reqCtx := &contextmodel.ReqContext{
Context: c,
SignedInUser: tc.user,
@@ -189,7 +189,6 @@ func TestAPIQueryPublicDashboard(t *testing.T) {
setup := func(enabled bool) (*web.Mux, *publicdashboards.FakePublicDashboardService) {
service := publicdashboards.NewFakePublicDashboardService(t)
cfg := setting.NewCfg()
cfg.RBACEnabled = false
testServer := setupTestServer(
t,