Skip to content

N
nexpie-grafana-theme
Commit c298c7f3 by Marcus Efraimsson Committed by GitHub
Options

Merge pull request #11034 from grafana/10701_permissions 

Only allow override permissions that is higher
parents f4d9536c f44e4765
Showing with 250 additions and 127 deletions
pkg/api/dashboard_permission.go
......@@ -18,13 +18,13 @@ func GetDashboardPermissionList(c *middleware.Context) Response {
return rsp
}
guardian := guardian.New(dashId, c.OrgId, c.SignedInUser)
g := guardian.New(dashId, c.OrgId, c.SignedInUser)
if canAdmin, err := guardian.CanAdmin(); err != nil || !canAdmin {
if canAdmin, err := g.CanAdmin(); err != nil || !canAdmin {
return dashboardGuardianResponse(err)
}
acl, err := guardian.GetAcl()
acl, err := g.GetAcl()
if err != nil {
return ApiError(500, "Failed to get dashboard permissions", err)
}
......@@ -46,8 +46,8 @@ func UpdateDashboardPermissions(c *middleware.Context, apiCmd dtos.UpdateDashboa
return rsp
}
guardian := guardian.New(dashId, c.OrgId, c.SignedInUser)
if canAdmin, err := guardian.CanAdmin(); err != nil || !canAdmin {
g := guardian.New(dashId, c.OrgId, c.SignedInUser)
if canAdmin, err := g.CanAdmin(); err != nil || !canAdmin {
return dashboardGuardianResponse(err)
}
......@@ -67,8 +67,13 @@ func UpdateDashboardPermissions(c *middleware.Context, apiCmd dtos.UpdateDashboa
})
}
if okToUpdate, err := guardian.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, cmd.Items); err != nil || !okToUpdate {
if okToUpdate, err := g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, cmd.Items); err != nil || !okToUpdate {
if err != nil {
if err == guardian.ErrGuardianPermissionExists ||
err == guardian.ErrGuardianOverride {
return ApiError(400, err.Error(), err)
}
return ApiError(500, "Error while checking dashboard permissions", err)
}
......
pkg/api/folder_permission.go
......@@ -19,13 +19,13 @@ func GetFolderPermissionList(c *middleware.Context) Response {
return toFolderError(err)
}
guardian := guardian.New(folder.Id, c.OrgId, c.SignedInUser)
g := guardian.New(folder.Id, c.OrgId, c.SignedInUser)
if canAdmin, err := guardian.CanAdmin(); err != nil || !canAdmin {
if canAdmin, err := g.CanAdmin(); err != nil || !canAdmin {
return toFolderError(m.ErrFolderAccessDenied)
}
acl, err := guardian.GetAcl()
acl, err := g.GetAcl()
if err != nil {
return ApiError(500, "Failed to get folder permissions", err)
}
......@@ -50,8 +50,8 @@ func UpdateFolderPermissions(c *middleware.Context, apiCmd dtos.UpdateDashboardA
return toFolderError(err)
}
guardian := guardian.New(folder.Id, c.OrgId, c.SignedInUser)
canAdmin, err := guardian.CanAdmin()
g := guardian.New(folder.Id, c.OrgId, c.SignedInUser)
canAdmin, err := g.CanAdmin()
if err != nil {
return toFolderError(err)
}
......@@ -76,8 +76,13 @@ func UpdateFolderPermissions(c *middleware.Context, apiCmd dtos.UpdateDashboardA
})
}
if okToUpdate, err := guardian.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, cmd.Items); err != nil || !okToUpdate {
if okToUpdate, err := g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, cmd.Items); err != nil || !okToUpdate {
if err != nil {
if err == guardian.ErrGuardianPermissionExists ||
err == guardian.ErrGuardianOverride {
return ApiError(400, err.Error(), err)
}
return ApiError(500, "Error while checking folder permissions", err)
}
......
pkg/api/folder_permission_test.go
......@@ -5,6 +5,7 @@ import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/middleware"
m "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/dashboards"
......@@ -15,6 +16,35 @@ import (
func TestFolderPermissionApiEndpoint(t *testing.T) {
Convey("Folder permissions test", t, func() {
Convey("Given folder not exists", func() {
mock := &fakeFolderService{
GetFolderByUidError: m.ErrFolderNotFound,
}
origNewFolderService := dashboards.NewFolderService
mockFolderService(mock)
loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", m.ROLE_EDITOR, func(sc *scenarioContext) {
callGetFolderPermissions(sc)
So(sc.resp.Code, ShouldEqual, 404)
})
cmd := dtos.UpdateDashboardAclCommand{
Items: []dtos.DashboardAclUpdateItem{
{UserId: 1000, Permission: m.PERMISSION_ADMIN},
},
}
updateFolderPermissionScenario("When calling POST on", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", cmd, func(sc *scenarioContext) {
callUpdateFolderPermissions(sc)
So(sc.resp.Code, ShouldEqual, 404)
})
Reset(func() {
dashboards.NewFolderService = origNewFolderService
})
})
Convey("Given user has no admin permissions", func() {
origNewGuardian := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanAdminValue: false})
......@@ -54,7 +84,17 @@ func TestFolderPermissionApiEndpoint(t *testing.T) {
Convey("Given user has admin permissions and permissions to update", func() {
origNewGuardian := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanAdminValue: true, CheckPermissionBeforeUpdateValue: true})
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
CanAdminValue: true,
CheckPermissionBeforeUpdateValue: true,
GetAclValue: []*m.DashboardAclInfoDTO{
{OrgId: 1, DashboardId: 1, UserId: 2, Permission: m.PERMISSION_VIEW},
{OrgId: 1, DashboardId: 1, UserId: 3, Permission: m.PERMISSION_EDIT},
{OrgId: 1, DashboardId: 1, UserId: 4, Permission: m.PERMISSION_ADMIN},
{OrgId: 1, DashboardId: 1, TeamId: 1, Permission: m.PERMISSION_VIEW},
{OrgId: 1, DashboardId: 1, TeamId: 2, Permission: m.PERMISSION_ADMIN},
},
})
mock := &fakeFolderService{
GetFolderByUidResult: &m.Folder{
......@@ -70,6 +110,11 @@ func TestFolderPermissionApiEndpoint(t *testing.T) {
loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", m.ROLE_ADMIN, func(sc *scenarioContext) {
callGetFolderPermissions(sc)
So(sc.resp.Code, ShouldEqual, 200)
respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
So(err, ShouldBeNil)
So(len(respJSON.MustArray()), ShouldEqual, 5)
So(respJSON.GetIndex(0).Get("userId").MustInt(), ShouldEqual, 2)
So(respJSON.GetIndex(0).Get("permission").MustInt(), ShouldEqual, m.PERMISSION_VIEW)
})
cmd := dtos.UpdateDashboardAclCommand{
......@@ -88,6 +133,78 @@ func TestFolderPermissionApiEndpoint(t *testing.T) {
dashboards.NewFolderService = origNewFolderService
})
})
Convey("When trying to update permissions with duplicate permissions", func() {
origNewGuardian := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
CanAdminValue: true,
CheckPermissionBeforeUpdateValue: false,
CheckPermissionBeforeUpdateError: guardian.ErrGuardianPermissionExists,
})
mock := &fakeFolderService{
GetFolderByUidResult: &m.Folder{
Id: 1,
Uid: "uid",
Title: "Folder",
},
}
origNewFolderService := dashboards.NewFolderService
mockFolderService(mock)
cmd := dtos.UpdateDashboardAclCommand{
Items: []dtos.DashboardAclUpdateItem{
{UserId: 1000, Permission: m.PERMISSION_ADMIN},
},
}
updateFolderPermissionScenario("When calling POST on", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", cmd, func(sc *scenarioContext) {
callUpdateFolderPermissions(sc)
So(sc.resp.Code, ShouldEqual, 400)
})
Reset(func() {
guardian.New = origNewGuardian
dashboards.NewFolderService = origNewFolderService
})
})
Convey("When trying to override inherited permissions with lower presedence", func() {
origNewGuardian := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
CanAdminValue: true,
CheckPermissionBeforeUpdateValue: false,
CheckPermissionBeforeUpdateError: guardian.ErrGuardianOverride},
)
mock := &fakeFolderService{
GetFolderByUidResult: &m.Folder{
Id: 1,
Uid: "uid",
Title: "Folder",
},
}
origNewFolderService := dashboards.NewFolderService
mockFolderService(mock)
cmd := dtos.UpdateDashboardAclCommand{
Items: []dtos.DashboardAclUpdateItem{
{UserId: 1000, Permission: m.PERMISSION_ADMIN},
},
}
updateFolderPermissionScenario("When calling POST on", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", cmd, func(sc *scenarioContext) {
callUpdateFolderPermissions(sc)
So(sc.resp.Code, ShouldEqual, 400)
})
Reset(func() {
guardian.New = origNewGuardian
dashboards.NewFolderService = origNewFolderService
})
})
})
}
......
pkg/models/dashboard_acl.go
......@@ -68,6 +68,27 @@ type DashboardAclInfoDTO struct {
Url string `json:"url"`
}
func (dto *DashboardAclInfoDTO) hasSameRoleAs(other *DashboardAclInfoDTO) bool {
if dto.Role == nil || other.Role == nil {
return false
}
return dto.UserId <= 0 && dto.TeamId <= 0 && dto.UserId == other.UserId && dto.TeamId == other.TeamId && *dto.Role == *other.Role
}
func (dto *DashboardAclInfoDTO) hasSameUserAs(other *DashboardAclInfoDTO) bool {
return dto.UserId > 0 && dto.UserId == other.UserId
}
func (dto *DashboardAclInfoDTO) hasSameTeamAs(other *DashboardAclInfoDTO) bool {
return dto.TeamId > 0 && dto.TeamId == other.TeamId
}
// IsDuplicateOf returns true if other item has same role, same user or same team
func (dto *DashboardAclInfoDTO) IsDuplicateOf(other *DashboardAclInfoDTO) bool {
return dto.hasSameRoleAs(other) || dto.hasSameUserAs(other) || dto.hasSameTeamAs(other)
}
//
// COMMANDS
//
......
pkg/services/guardian/guardian.go
package guardian
import (
"errors"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/log"
m "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/setting"
)
var (
ErrGuardianPermissionExists = errors.New("Permission already exists")
ErrGuardianOverride = errors.New("You can only override a permission to be higher")
)
// DashboardGuardian to be used for guard against operations without access on dashboard and acl
type DashboardGuardian interface {
CanSave() (bool, error)
......@@ -119,14 +126,51 @@ func (g *dashboardGuardianImpl) checkAcl(permission m.PermissionType, acl []*m.D
}
func (g *dashboardGuardianImpl) CheckPermissionBeforeUpdate(permission m.PermissionType, updatePermissions []*m.DashboardAcl) (bool, error) {
if g.user.OrgRole == m.ROLE_ADMIN {
return true, nil
}
acl := []*m.DashboardAclInfoDTO{}
adminRole := m.ROLE_ADMIN
everyoneWithAdminRole := &m.DashboardAclInfoDTO{DashboardId: g.dashId, UserId: 0, TeamId: 0, Role: &adminRole, Permission: m.PERMISSION_ADMIN}
// validate that duplicate permissions don't exists
for _, p := range updatePermissions {
acl = append(acl, &m.DashboardAclInfoDTO{UserId: p.UserId, TeamId: p.TeamId, Role: p.Role, Permission: p.Permission})
aclItem := &m.DashboardAclInfoDTO{DashboardId: p.DashboardId, UserId: p.UserId, TeamId: p.TeamId, Role: p.Role, Permission: p.Permission}
if aclItem.IsDuplicateOf(everyoneWithAdminRole) {
return false, ErrGuardianPermissionExists
}
for _, a := range acl {
if a.IsDuplicateOf(aclItem) {
return false, ErrGuardianPermissionExists
}
}
acl = append(acl, aclItem)
}
existingPermissions, err := g.GetAcl()
if err != nil {
return false, err
}
// validate overridden permissions to be higher
for _, a := range acl {
for _, existingPerm := range existingPermissions {
// handle default permissions
if existingPerm.DashboardId == -1 {
existingPerm.DashboardId = g.dashId
}
if a.DashboardId == existingPerm.DashboardId {
continue
}
if a.IsDuplicateOf(existingPerm) && a.Permission <= existingPerm.Permission {
return false, ErrGuardianOverride
}
}
}
if g.user.OrgRole == m.ROLE_ADMIN {
return true, nil
}
return g.checkAcl(permission, acl)
......@@ -143,6 +187,13 @@ func (g *dashboardGuardianImpl) GetAcl() ([]*m.DashboardAclInfoDTO, error) {
return nil, err
}
for _, a := range query.Result {
// handle default permissions
if a.DashboardId == -1 {
a.DashboardId = g.dashId
}
}
g.acl = query.Result
return g.acl, nil
}
......@@ -169,6 +220,8 @@ type FakeDashboardGuardian struct {
CanAdminValue bool
HasPermissionValue bool
CheckPermissionBeforeUpdateValue bool
CheckPermissionBeforeUpdateError error
GetAclValue []*m.DashboardAclInfoDTO
}
func (g *FakeDashboardGuardian) CanSave() (bool, error) {
......@@ -192,11 +245,11 @@ func (g *FakeDashboardGuardian) HasPermission(permission m.PermissionType) (bool
}
func (g *FakeDashboardGuardian) CheckPermissionBeforeUpdate(permission m.PermissionType, updatePermissions []*m.DashboardAcl) (bool, error) {
return g.CheckPermissionBeforeUpdateValue, nil
return g.CheckPermissionBeforeUpdateValue, g.CheckPermissionBeforeUpdateError
}
func (g *FakeDashboardGuardian) GetAcl() ([]*m.DashboardAclInfoDTO, error) {
return nil, nil
return g.GetAclValue, nil
}
func MockDashboardGuardian(mock *FakeDashboardGuardian) {
......
public/app/core/components/Permissions/AddPermissions.jest.tsx
......@@ -17,7 +17,7 @@ describe('AddPermissions', () => {
])
);
backendSrv.post = jest.fn();
backendSrv.post = jest.fn(() => Promise.resolve({}));
store = RootStore.create(
{},
......
public/app/core/components/Permissions/AddPermissions.tsx
......@@ -135,14 +135,6 @@ class AddPermissions extends Component<IProps, any> {
</div>
</div>
</form>
{permissions.error ? (
<div className="gf-form width-17">
<span ng-if="ctrl.error" className="text-error p-l-1">
<i className="fa fa-warning" />
{permissions.error}
</span>
</div>
) : null}
</div>
);
}
......
public/app/stores/PermissionsStore/PermissionsStore.jest.ts
import { PermissionsStore, aclTypeValues } from './PermissionsStore';
import { PermissionsStore } from './PermissionsStore';
import { backendSrv } from 'test/mocks/common';
describe('PermissionsStore', () => {
let store;
beforeEach(() => {
beforeEach(async () => {
backendSrv.get.mockReturnValue(
Promise.resolve([
{ id: 2, dashboardId: 1, role: 'Viewer', permission: 1, permissionName: 'View' },
......@@ -20,7 +20,7 @@ describe('PermissionsStore', () => {
])
);
backendSrv.post = jest.fn();
backendSrv.post = jest.fn(() => Promise.resolve({}));
store = PermissionsStore.create(
{
......@@ -32,14 +32,14 @@ describe('PermissionsStore', () => {
}
);
return store.load(1, false, false);
await store.load(1, false, false);
});
it('should save update on permission change', () => {
it('should save update on permission change', async () => {
expect(store.items[0].permission).toBe(1);
expect(store.items[0].permissionName).toBe('View');
store.updatePermissionOnIndex(0, 2, 'Edit');
await store.updatePermissionOnIndex(0, 2, 'Edit');
expect(store.items[0].permission).toBe(2);
expect(store.items[0].permissionName).toBe('Edit');
......@@ -47,69 +47,18 @@ describe('PermissionsStore', () => {
expect(backendSrv.post.mock.calls[0][0]).toBe('/api/dashboards/id/1/permissions');
});
it('should save removed permissions automatically', () => {
it('should save removed permissions automatically', async () => {
expect(store.items.length).toBe(3);
store.removeStoreItem(2);
await store.removeStoreItem(2);
expect(store.items.length).toBe(2);
expect(backendSrv.post.mock.calls.length).toBe(1);
expect(backendSrv.post.mock.calls[0][0]).toBe('/api/dashboards/id/1/permissions');
});
describe('when duplicate team permissions are added', () => {
beforeEach(() => {
const newItem = {
teamId: 10,
team: 'tester-team',
permission: 1,
dashboardId: 1,
};
store.resetNewType();
store.newItem.setTeam(newItem.teamId, newItem.team);
store.newItem.setPermission(newItem.permission);
store.addStoreItem();
store.newItem.setTeam(newItem.teamId, newItem.team);
store.newItem.setPermission(newItem.permission);
store.addStoreItem();
});
it('should return a validation error', () => {
expect(store.items.length).toBe(4);
expect(store.error).toBe('This permission exists already.');
expect(backendSrv.post.mock.calls.length).toBe(1);
});
});
describe('when duplicate user permissions are added', () => {
beforeEach(() => {
expect(store.items.length).toBe(3);
const newItem = {
userId: 10,
userLogin: 'tester1',
permission: 1,
dashboardId: 1,
};
store.setNewType(aclTypeValues.USER.value);
store.newItem.setUser(newItem.userId, newItem.userLogin);
store.newItem.setPermission(newItem.permission);
store.addStoreItem();
store.setNewType(aclTypeValues.USER.value);
store.newItem.setUser(newItem.userId, newItem.userLogin);
store.newItem.setPermission(newItem.permission);
store.addStoreItem();
});
it('should return a validation error', () => {
expect(store.items.length).toBe(4);
expect(store.error).toBe('This permission exists already.');
expect(backendSrv.post.mock.calls.length).toBe(1);
});
});
describe('when one inherited and one not inherited team permission are added', () => {
beforeEach(() => {
beforeEach(async () => {
const overridingItemForChildDashboard = {
team: 'MyTestTeam',
dashboardId: 1,
......@@ -120,11 +69,7 @@ describe('PermissionsStore', () => {
store.resetNewType();
store.newItem.setTeam(overridingItemForChildDashboard.teamId, overridingItemForChildDashboard.team);
store.newItem.setPermission(overridingItemForChildDashboard.permission);
store.addStoreItem();
});
it('should allowing overriding the inherited permission and not throw a validation error', () => {
expect(store.error).toBe(null);
await store.addStoreItem();
});
it('should add new overriding permission', () => {
......
public/app/stores/PermissionsStore/PermissionsStore.ts
import { types, getEnv, flow } from 'mobx-state-tree';
import { PermissionsStoreItem } from './PermissionsStoreItem';
const duplicateError = 'This permission exists already.';
export const permissionOptions = [
{ value: 1, label: 'View', description: 'Can view dashboards.' },
{ value: 2, label: 'Edit', description: 'Can add, edit and delete dashboards.' },
......@@ -75,7 +73,6 @@ export const PermissionsStore = types
isFolder: types.maybe(types.boolean),
dashboardId: types.maybe(types.number),
items: types.optional(types.array(PermissionsStoreItem), []),
error: types.maybe(types.string),
originalItems: types.optional(types.array(PermissionsStoreItem), []),
newType: types.optional(types.string, defaultNewType),
newItem: types.maybe(NewPermissionsItem),
......@@ -88,7 +85,6 @@ export const PermissionsStore = types
return isDuplicate(it, item);
});
if (dupe) {
self.error = duplicateError;
return false;
}
......@@ -96,8 +92,7 @@ export const PermissionsStore = types
},
}))
.actions(self => {
const resetNewType = () => {
self.error = null;
const resetNewTypeInternal = () => {
self.newItem = NewPermissionsItem.create();
};
......@@ -115,11 +110,9 @@ export const PermissionsStore = types
self.items = items;
self.originalItems = items;
self.fetching = false;
self.error = null;
}),
addStoreItem: flow(function* addStoreItem() {
self.error = null;
let item = {
type: self.newItem.type,
permission: self.newItem.permission,
......@@ -147,19 +140,21 @@ export const PermissionsStore = types
throw Error('Unknown type: ' + self.newItem.type);
}
if (!self.isValid(item)) {
return undefined;
}
const updatedItems = self.items.peek();
const newItem = prepareItem(item, self.dashboardId, self.isFolder, self.isInRoot);
updatedItems.push(newItem);
self.items.push(prepareItem(item, self.dashboardId, self.isFolder, self.isInRoot));
resetNewType();
return updateItems(self);
try {
yield updateItems(self, updatedItems);
self.items.push(newItem);
resetNewTypeInternal();
} catch {}
yield Promise.resolve();
}),
removeStoreItem: flow(function* removeStoreItem(idx: number) {
self.error = null;
self.items.splice(idx, 1);
return updateItems(self);
yield updateItems(self, self.items.peek());
}),
updatePermissionOnIndex: flow(function* updatePermissionOnIndex(
......@@ -167,9 +162,8 @@ export const PermissionsStore = types
permission: number,
permissionName: string
) {
self.error = null;
self.items[idx].updatePermission(permission, permissionName);
return updateItems(self);
yield updateItems(self, self.items.peek());
}),
setNewType(newType: string) {
......@@ -177,7 +171,7 @@ export const PermissionsStore = types
},
resetNewType() {
resetNewType();
resetNewTypeInternal();
},
toggleAddPermissions() {
......@@ -190,12 +184,10 @@ export const PermissionsStore = types
};
});
const updateItems = self => {
self.error = null;
const updateItems = (self, items) => {
const backendSrv = getEnv(self).backendSrv;
const updated = [];
for (let item of self.items) {
for (let item of items) {
if (item.inherited) {
continue;
}
......@@ -208,16 +200,9 @@ const updateItems = self => {
});
}
let res;
try {
res = backendSrv.post(`/api/dashboards/id/${self.dashboardId}/permissions`, {
items: updated,
});
} catch (error) {
self.error = error;
}
return res;
return backendSrv.post(`/api/dashboards/id/${self.dashboardId}/permissions`, {
items: updated,
});
};
const prepareServerResponse = (response, dashboardId: number, isFolder: boolean, isInRoot: boolean) => {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment