From f69654fcd5dd20b264e38af988a4a69673de76bb Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Wed, 30 May 2018 13:13:29 +0200 Subject: [PATCH 1/4] Restrict Explore UI to Editor and Admin roles Access is restricted via not showing in the following places: * hide from sidemenu * hide from panel header menu * disable keybinding `x` Also adds a `roles` property to reactContainer routes that will be checked if `roles` is set, and on failure redirects to `/`. --- pkg/api/index.go | 2 +- public/app/core/services/keybindingSrv.ts | 33 ++++++++++--------- .../app/features/panel/metrics_panel_ctrl.ts | 4 ++- public/app/routes/ReactContainer.tsx | 20 +++++++++-- public/app/routes/routes.ts | 1 + 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/pkg/api/index.go b/pkg/api/index.go index f082f03b5f6..acf0c30c907 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -128,7 +128,7 @@ func setIndexViewData(c *m.ReqContext) (*dtos.IndexViewData, error) { Children: dashboardChildNavs, }) - if setting.ExploreEnabled { + if setting.ExploreEnabled && (c.OrgRole == m.ROLE_ADMIN || c.OrgRole == m.ROLE_EDITOR) { data.NavTree = append(data.NavTree, &dtos.NavLink{ Text: "Explore", Id: "explore", diff --git a/public/app/core/services/keybindingSrv.ts b/public/app/core/services/keybindingSrv.ts index 25d00ab37f1..b1021c90adc 100644 --- a/public/app/core/services/keybindingSrv.ts +++ b/public/app/core/services/keybindingSrv.ts @@ -14,7 +14,7 @@ export class KeybindingSrv { timepickerOpen = false; /** @ngInject */ - constructor(private $rootScope, private $location, private datasourceSrv, private timeSrv) { + constructor(private $rootScope, private $location, private datasourceSrv, private timeSrv, private contextSrv) { // clear out all shortcuts on route change $rootScope.$on('$routeChangeSuccess', () => { Mousetrap.reset(); @@ -177,21 +177,24 @@ export class KeybindingSrv { } }); - this.bind('x', async () => { - if (dashboard.meta.focusPanelId) { - const panel = dashboard.getPanelById(dashboard.meta.focusPanelId); - const datasource = await this.datasourceSrv.get(panel.datasource); - if (datasource && datasource.supportsExplore) { - const range = this.timeSrv.timeRangeForUrl(); - const state = { - ...datasource.getExploreState(panel), - range, - }; - const exploreState = encodePathComponent(JSON.stringify(state)); - this.$location.url(`/explore/${exploreState}`); + // jump to explore if permissions allow + if (this.contextSrv.isEditor) { + this.bind('x', async () => { + if (dashboard.meta.focusPanelId) { + const panel = dashboard.getPanelById(dashboard.meta.focusPanelId); + const datasource = await this.datasourceSrv.get(panel.datasource); + if (datasource && datasource.supportsExplore) { + const range = this.timeSrv.timeRangeForUrl(); + const state = { + ...datasource.getExploreState(panel), + range, + }; + const exploreState = encodePathComponent(JSON.stringify(state)); + this.$location.url(`/explore/${exploreState}`); + } } - } - }); + }); + } // delete panel this.bind('p r', () => { diff --git a/public/app/features/panel/metrics_panel_ctrl.ts b/public/app/features/panel/metrics_panel_ctrl.ts index 3c48119ba3a..cf1b2cd49bc 100644 --- a/public/app/features/panel/metrics_panel_ctrl.ts +++ b/public/app/features/panel/metrics_panel_ctrl.ts @@ -16,6 +16,7 @@ class MetricsPanelCtrl extends PanelCtrl { datasourceName: any; $q: any; $timeout: any; + contextSrv: any; datasourceSrv: any; timeSrv: any; templateSrv: any; @@ -37,6 +38,7 @@ class MetricsPanelCtrl extends PanelCtrl { // make metrics tab the default this.editorTabIndex = 1; this.$q = $injector.get('$q'); + this.contextSrv = $injector.get('contextSrv'); this.datasourceSrv = $injector.get('datasourceSrv'); this.timeSrv = $injector.get('timeSrv'); this.templateSrv = $injector.get('templateSrv'); @@ -312,7 +314,7 @@ class MetricsPanelCtrl extends PanelCtrl { getAdditionalMenuItems() { const items = []; - if (this.datasource && this.datasource.supportsExplore) { + if (this.contextSrv.isEditor && this.datasource && this.datasource.supportsExplore) { items.push({ text: 'Explore', click: 'ctrl.explore();', diff --git a/public/app/routes/ReactContainer.tsx b/public/app/routes/ReactContainer.tsx index db6938cc878..b161a5e7a87 100644 --- a/public/app/routes/ReactContainer.tsx +++ b/public/app/routes/ReactContainer.tsx @@ -6,6 +6,7 @@ import coreModule from 'app/core/core_module'; import { store } from 'app/stores/store'; import { BackendSrv } from 'app/core/services/backend_srv'; import { DatasourceSrv } from 'app/features/plugins/datasource_srv'; +import { ContextSrv } from 'app/core/services/context_srv'; function WrapInProvider(store, Component, props) { return ( @@ -16,16 +17,31 @@ function WrapInProvider(store, Component, props) { } /** @ngInject */ -export function reactContainer($route, $location, backendSrv: BackendSrv, datasourceSrv: DatasourceSrv) { +export function reactContainer( + $route, + $location, + backendSrv: BackendSrv, + datasourceSrv: DatasourceSrv, + contextSrv: ContextSrv +) { return { restrict: 'E', template: '', link(scope, elem) { - let component = $route.current.locals.component; + // Check permissions for this component + const { roles } = $route.current.locals; + if (roles && roles.length) { + if (!roles.some(r => contextSrv.hasRole(r))) { + $location.url('/'); + } + } + + let { component } = $route.current.locals; // Dynamic imports return whole module, need to extract default export if (component.default) { component = component.default; } + const props = { backendSrv: backendSrv, datasourceSrv: datasourceSrv, diff --git a/public/app/routes/routes.ts b/public/app/routes/routes.ts index b10084d1941..568b3438b38 100644 --- a/public/app/routes/routes.ts +++ b/public/app/routes/routes.ts @@ -113,6 +113,7 @@ export function setupAngularRoutes($routeProvider, $locationProvider) { .when('/explore/:initial?', { template: '', resolve: { + roles: () => ['Editor', 'Admin'], component: () => import(/* webpackChunkName: "explore" */ 'app/containers/Explore/Wrapper'), }, }) From 7224ca6c622547124fcab828919872fde93efca6 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Wed, 30 May 2018 13:24:09 +0200 Subject: [PATCH 2/4] Fix panel menu test --- public/app/features/panel/specs/metrics_panel_ctrl.jest.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/public/app/features/panel/specs/metrics_panel_ctrl.jest.ts b/public/app/features/panel/specs/metrics_panel_ctrl.jest.ts index f2e5199b57d..79564e2a123 100644 --- a/public/app/features/panel/specs/metrics_panel_ctrl.jest.ts +++ b/public/app/features/panel/specs/metrics_panel_ctrl.jest.ts @@ -24,8 +24,9 @@ describe('MetricsPanelCtrl', () => { }); }); - describe('and has datasource set that supports explore', () => { + describe('and has datasource set that supports explore and user has powers', () => { beforeEach(() => { + ctrl.contextSrv = { isEditor: true }; ctrl.datasource = { supportsExplore: true }; additionalItems = ctrl.getAdditionalMenuItems(); }); From 827fb7e8de3bf075a2af13f8f6940abbee5eb584 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Wed, 30 May 2018 15:24:47 +0200 Subject: [PATCH 3/4] Fix karma tests that rely on MetricsPanelCtrl --- public/app/features/panel/metrics_panel_ctrl.ts | 4 ++-- public/test/specs/helpers.ts | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/public/app/features/panel/metrics_panel_ctrl.ts b/public/app/features/panel/metrics_panel_ctrl.ts index cf1b2cd49bc..cbda8c874db 100644 --- a/public/app/features/panel/metrics_panel_ctrl.ts +++ b/public/app/features/panel/metrics_panel_ctrl.ts @@ -1,9 +1,9 @@ -import config from 'app/core/config'; import $ from 'jquery'; import _ from 'lodash'; + +import config from 'app/core/config'; import kbn from 'app/core/utils/kbn'; import { PanelCtrl } from 'app/features/panel/panel_ctrl'; - import * as rangeUtil from 'app/core/utils/rangeutil'; import * as dateMath from 'app/core/utils/datemath'; import { encodePathComponent } from 'app/core/utils/location_util'; diff --git a/public/test/specs/helpers.ts b/public/test/specs/helpers.ts index 276d9867ec4..dd8bd39846e 100644 --- a/public/test/specs/helpers.ts +++ b/public/test/specs/helpers.ts @@ -11,6 +11,7 @@ export function ControllerTestContext() { this.$element = {}; this.$sanitize = {}; this.annotationsSrv = {}; + this.contextSrv = {}; this.timeSrv = new TimeSrvStub(); this.templateSrv = new TemplateSrvStub(); this.datasourceSrv = { @@ -27,6 +28,7 @@ export function ControllerTestContext() { this.providePhase = function(mocks) { return angularMocks.module(function($provide) { + $provide.value('contextSrv', self.contextSrv); $provide.value('datasourceSrv', self.datasourceSrv); $provide.value('annotationsSrv', self.annotationsSrv); $provide.value('timeSrv', self.timeSrv); From 0c45ee63a9bf360ded82e3a229fd0a142187c797 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Thu, 31 May 2018 11:26:24 +0200 Subject: [PATCH 4/4] Guard /explore by editor role on the backend --- pkg/api/api.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/api/api.go b/pkg/api/api.go index 01189f7a81e..c205e7d3e2f 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -77,6 +77,9 @@ func (hs *HTTPServer) registerRoutes() { r.Get("/dashboards/", reqSignedIn, Index) r.Get("/dashboards/*", reqSignedIn, Index) + r.Get("/explore/", reqEditorRole, Index) + r.Get("/explore/*", reqEditorRole, Index) + r.Get("/playlists/", reqSignedIn, Index) r.Get("/playlists/*", reqSignedIn, Index) r.Get("/alerting/", reqSignedIn, Index)