From 2fd7031102cc35882a809118420832e840b653dd Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Fri, 2 Jul 2021 14:43:12 +0200 Subject: [PATCH] Access Control: Add fine-grained access control to explore (#35883) * add fixed role for datasource read operations * Add action for datasource explore * add authorize middleware to explore index route * add fgac support for explore navlink * update hasAccessToExplore to check if accesscontrol is enable and evalute action if it is * add getExploreRoles to evalute roles based onaccesscontrol, viewersCanEdit and default * create function to evaluate permissions or using fallback if accesscontrol is disabled * change hasAccess to prop and derive the value in mapStateToProps * add test case to ensure buttons is not rendered when user does not have access * Only hide return with changes button * remove internal links if user does not have access to explorer Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> --- .../fine-grained-access-control-references.md | 2 + .../enterprise/access-control/permissions.md | 1 + pkg/api/api.go | 7 ++- pkg/api/index.go | 6 +- pkg/services/accesscontrol/models.go | 4 +- pkg/services/accesscontrol/roles.go | 18 +++++- public/app/core/services/context_srv.ts | 3 + .../explore/ReturnToDashboardButton.test.tsx | 6 ++ .../explore/ReturnToDashboardButton.tsx | 21 +++++-- .../app/features/explore/utils/links.test.ts | 38 ++++++++++++- public/app/features/explore/utils/links.ts | 55 +++++++++++-------- public/app/routes/routes.tsx | 22 +++++++- public/app/types/accessControl.ts | 1 + 13 files changed, 150 insertions(+), 34 deletions(-) diff --git a/docs/sources/enterprise/access-control/fine-grained-access-control-references.md b/docs/sources/enterprise/access-control/fine-grained-access-control-references.md index f9104b23241..f553aec8829 100644 --- a/docs/sources/enterprise/access-control/fine-grained-access-control-references.md +++ b/docs/sources/enterprise/access-control/fine-grained-access-control-references.md @@ -25,6 +25,7 @@ Fixed roles | Permissions | Descriptions `fixed:server:admin:read` | `server.stats:read` | Read server stats `fixed:settings:admin:read` | `settings:read` | Read settings `fixed:settings:admin:edit` | All permissions from `fixed:settings:admin:read` and
`settings:write` | Update settings +`fixed:datasource:editor:read` | `datasources:explore` | Explore datasources ## Default built-in role assignments @@ -32,3 +33,4 @@ Built-in roles | Associated roles | Descriptions --- | --- | --- Grafana Admin | `fixed:permissions:admin:edit`
`fixed:permissions:admin:read`
`fixed:reporting:admin:edit`
`fixed:reporting:admin:read`
`fixed:users:admin:edit`
`fixed:users:admin:read`
`fixed:users:org:edit`
`fixed:users:org:read`
`fixed:ldap:admin:edit`
`fixed:ldap:admin:read`
`fixed:server:admin:read`
`fixed:settings:admin:read`
`fixed:settings:admin:edit` | Allows access to resources which [Grafana Server Admin]({{< relref "../../permissions/_index.md#grafana-server-admin-role" >}}) has permissions by default. Admin | `fixed:users:org:edit`
`fixed:users:org:read`
`fixed:reporting:admin:edit`
`fixed:reporting:admin:read` | Allows access to resource which [Admin]({{< relref "../../permissions/organization_roles.md" >}}) has permissions by default. +Editor | `fixed:datasource:editor:read` diff --git a/docs/sources/enterprise/access-control/permissions.md b/docs/sources/enterprise/access-control/permissions.md index fff815e3bb7..974ac635c4e 100644 --- a/docs/sources/enterprise/access-control/permissions.md +++ b/docs/sources/enterprise/access-control/permissions.md @@ -66,6 +66,7 @@ Actions | Applicable scopes | Descriptions `settings:read` | `settings:**`
`settings:auth.saml:*`
`settings:auth.saml:enabled` (property level) | Read settings `settings:write` | `settings:**`
`settings:auth.saml:*`
`settings:auth.saml:enabled` (property level) | Update settings `server.stats:read` | n/a | Read server stats +`datasources:explore` | n/a | Enable explore ## Scope definitions diff --git a/pkg/api/api.go b/pkg/api/api.go index 7e1e7602572..ac985356ab1 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -94,7 +94,12 @@ func (hs *HTTPServer) registerRoutes() { r.Get("/dashboards/*", reqSignedIn, hs.Index) r.Get("/goto/:uid", reqSignedIn, hs.redirectFromShortURL, hs.Index) - r.Get("/explore", reqSignedIn, middleware.EnsureEditorOrViewerCanEdit, hs.Index) + r.Get("/explore", authorize(func(c *models.ReqContext) { + if f, ok := reqSignedIn.(func(c *models.ReqContext)); ok { + f(c) + } + middleware.EnsureEditorOrViewerCanEdit(c) + }, accesscontrol.ActionDatasourcesExplore), hs.Index) r.Get("/playlists/", reqSignedIn, hs.Index) r.Get("/playlists/*", reqSignedIn, hs.Index) diff --git a/pkg/api/index.go b/pkg/api/index.go index 5202db9cea6..3201310639e 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -186,7 +186,11 @@ func (hs *HTTPServer) getNavTree(c *models.ReqContext, hasEditPerm bool) ([]*dto Children: dashboardChildNavs, }) - if setting.ExploreEnabled && (c.OrgRole == models.ROLE_ADMIN || c.OrgRole == models.ROLE_EDITOR || setting.ViewersCanEdit) { + canExplore := func(context *models.ReqContext) bool { + return c.OrgRole == models.ROLE_ADMIN || c.OrgRole == models.ROLE_EDITOR || setting.ViewersCanEdit + } + + if setting.ExploreEnabled && hasAccess(canExplore, ac.ActionDatasourcesExplore) { navTree = append(navTree, &dtos.NavLink{ Text: "Explore", Id: "explore", diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index 09fe491b43e..2dea4614816 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -42,7 +42,6 @@ func (p RoleDTO) Role() Role { const ( // Permission actions - // Actions // Provisioning actions ActionProvisioningReload = "provisioning:reload" @@ -86,6 +85,9 @@ const ( // Settings actions ActionSettingsRead = "settings:read" + // Datasources actions + ActionDatasourcesExplore = "datasources:explore" + // Global Scopes ScopeGlobalUsersAll = "global:users:*" diff --git a/pkg/services/accesscontrol/roles.go b/pkg/services/accesscontrol/roles.go index 6dc8c484816..104f4a72961 100644 --- a/pkg/services/accesscontrol/roles.go +++ b/pkg/services/accesscontrol/roles.go @@ -2,6 +2,16 @@ package accesscontrol import "github.com/grafana/grafana/pkg/models" +var datasourcesEditorReadRole = RoleDTO{ + Version: 1, + Name: datasourcesEditorRead, + Permissions: []Permission{ + { + Action: ActionDatasourcesExplore, + }, + }, +} + var ldapAdminReadRole = RoleDTO{ Name: ldapAdminRead, Version: 1, @@ -166,7 +176,8 @@ var provisioningAdminRole = RoleDTO{ // resource. FixedRoleGrants lists which built-in roles are // assigned which fixed roles in this list. var FixedRoles = map[string]RoleDTO{ - serverAdminRead: serverAdminReadRole, + datasourcesEditorRead: datasourcesEditorReadRole, + serverAdminRead: serverAdminReadRole, settingsAdminRead: settingsAdminReadRole, @@ -183,6 +194,8 @@ var FixedRoles = map[string]RoleDTO{ } const ( + datasourcesEditorRead = "fixed:datasources:editor:read" + serverAdminRead = "fixed:server:admin:read" settingsAdminRead = "fixed:settings:admin:read" @@ -217,6 +230,9 @@ var FixedRoleGrants = map[string][]string{ usersOrgEdit, usersOrgRead, }, + string(models.ROLE_EDITOR): { + datasourcesEditorRead, + }, } func ConcatPermissions(permissions ...[]Permission) []Permission { diff --git a/public/app/core/services/context_srv.ts b/public/app/core/services/context_srv.ts index a948572673d..5d2eec70703 100644 --- a/public/app/core/services/context_srv.ts +++ b/public/app/core/services/context_srv.ts @@ -106,6 +106,9 @@ export class ContextSrv { } hasAccessToExplore() { + if (config.featureToggles['accesscontrol']) { + return this.hasPermission(AccessControlAction.DataSourcesExplore); + } return (this.isEditor || config.viewersCanEdit) && config.exploreEnabled; } } diff --git a/public/app/features/explore/ReturnToDashboardButton.test.tsx b/public/app/features/explore/ReturnToDashboardButton.test.tsx index 4e344b5032d..7d29a84a1d8 100644 --- a/public/app/features/explore/ReturnToDashboardButton.test.tsx +++ b/public/app/features/explore/ReturnToDashboardButton.test.tsx @@ -7,6 +7,7 @@ const createProps = (propsOverride?: Partial { expect(screen.queryByTestId(/returnButton/i)).toBeNull(); }); + it('should not render return to panel with changes button if user cannot edit panel', () => { + render(); + expect(screen.getAllByTestId(/returnButton/i)).toHaveLength(1); + }); + it('should show option to return to dashboard with changes', () => { render(); const returnWithChangesButton = screen.getByTestId('returnButtonWithChanges'); diff --git a/public/app/features/explore/ReturnToDashboardButton.tsx b/public/app/features/explore/ReturnToDashboardButton.tsx index d0f440d9dad..b00cc5def0d 100644 --- a/public/app/features/explore/ReturnToDashboardButton.tsx +++ b/public/app/features/explore/ReturnToDashboardButton.tsx @@ -4,24 +4,32 @@ import { ButtonGroup, ButtonSelect, Icon, ToolbarButton, Tooltip } from '@grafan import { DataQuery, urlUtil } from '@grafana/data'; import kbn from '../../core/utils/kbn'; +import config from 'app/core/config'; import { getDashboardSrv } from '../dashboard/services/DashboardSrv'; import { StoreState } from 'app/types'; import { ExploreId } from 'app/types/explore'; import { setDashboardQueriesToUpdateOnLoad } from '../dashboard/state/reducers'; import { isSplit } from './state/selectors'; import { locationService } from '@grafana/runtime'; +import { contextSrv } from 'app/core/services/context_srv'; function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreId }) { const explore = state.explore; const splitted = isSplit(state); const { datasourceInstance, queries, originPanelId } = explore[exploreId]!; + const roles = ['Editor', 'Admin']; + if (config.viewersCanEdit) { + roles.push('Viewer'); + } + return { exploreId, datasourceInstance, queries, originPanelId, splitted, + canEdit: roles.some((r) => contextSrv.hasRole(r)), }; } @@ -37,6 +45,7 @@ export const UnconnectedReturnToDashboardButton: FC = ({ setDashboardQueriesToUpdateOnLoad, queries, splitted, + canEdit, }) => { const withOriginId = originPanelId && Number.isInteger(originPanelId); @@ -87,11 +96,13 @@ export const UnconnectedReturnToDashboardButton: FC = ({ - returnToPanel({ withChanges: true })} - /> + {canEdit && ( + returnToPanel({ withChanges: true })} + /> + )} ); }; diff --git a/public/app/features/explore/utils/links.test.ts b/public/app/features/explore/utils/links.test.ts index 30898671cc4..54334c8b605 100644 --- a/public/app/features/explore/utils/links.test.ts +++ b/public/app/features/explore/utils/links.test.ts @@ -10,6 +10,7 @@ import { TimeRange, } from '@grafana/data'; import { setLinkSrv } from '../../panel/panellinks/link_srv'; +import { setContextSrv } from '../../../core/services/context_srv'; describe('getFieldLinksForExplore', () => { it('returns correct link model for external link', () => { @@ -62,9 +63,40 @@ describe('getFieldLinksForExplore', () => { range, }); }); + + it('returns correct link model for external link when user does not have access to explore', () => { + const { field, range } = setup( + { + title: 'external', + url: 'http://regionalhost', + }, + false + ); + const links = getFieldLinksForExplore({ field, rowIndex: 0, range }); + + expect(links[0].href).toBe('http://regionalhost'); + expect(links[0].title).toBe('external'); + }); + + it('returns no internal links if when user does not have access to explore', () => { + const { field, range } = setup( + { + title: '', + url: '', + internal: { + query: { query: 'query_1' }, + datasourceUid: 'uid_1', + datasourceName: 'test_ds', + }, + }, + false + ); + const links = getFieldLinksForExplore({ field, rowIndex: 0, range }); + expect(links).toHaveLength(0); + }); }); -function setup(link: DataLink) { +function setup(link: DataLink, hasAccess = true) { setLinkSrv({ getDataLinkUIModel(link: DataLink, replaceVariables: InterpolateFunction | undefined, origin: any): LinkModel { return { @@ -82,6 +114,10 @@ function setup(link: DataLink) { }, }); + setContextSrv({ + hasAccessToExplore: () => hasAccess, + } as any); + const field: Field = { name: 'flux-dimensions', type: FieldType.string, diff --git a/public/app/features/explore/utils/links.ts b/public/app/features/explore/utils/links.ts index fc5e2818e4f..c672dc092b0 100644 --- a/public/app/features/explore/utils/links.ts +++ b/public/app/features/explore/utils/links.ts @@ -12,6 +12,7 @@ import { import { getTemplateSrv } from '@grafana/runtime'; import { SplitOpen } from 'app/types/explore'; import { getLinkSrv } from '../../panel/panellinks/link_srv'; +import { contextSrv } from 'app/core/services/context_srv'; /** * Get links from the field of a dataframe and in addition check if there is associated @@ -52,30 +53,40 @@ export const getFieldLinksForExplore = (options: { }; } - return field.config.links - ? field.config.links.map((link) => { - if (!link.internal) { - const replace: InterpolateFunction = (value, vars) => - getTemplateSrv().replace(value, { ...vars, ...scopedVars }); + if (field.config.links) { + const links = []; - const linkModel = getLinkSrv().getDataLinkUIModel(link, replace, field); - if (!linkModel.title) { - linkModel.title = getTitleFromHref(linkModel.href); - } - return linkModel; - } else { - return mapInternalLinkToExplore({ - link, - internalLink: link.internal, - scopedVars: scopedVars, - range, - field, - onClickFn: splitOpenFn, - replaceVariables: getTemplateSrv().replace.bind(getTemplateSrv()), - }); + if (!contextSrv.hasAccessToExplore()) { + links.push(...field.config.links.filter((l) => !l.internal)); + } else { + links.push(...field.config.links); + } + + return links.map((link) => { + if (!link.internal) { + const replace: InterpolateFunction = (value, vars) => + getTemplateSrv().replace(value, { ...vars, ...scopedVars }); + + const linkModel = getLinkSrv().getDataLinkUIModel(link, replace, field); + if (!linkModel.title) { + linkModel.title = getTitleFromHref(linkModel.href); } - }) - : []; + return linkModel; + } else { + return mapInternalLinkToExplore({ + link, + internalLink: link.internal, + scopedVars: scopedVars, + range, + field, + onClickFn: splitOpenFn, + replaceVariables: getTemplateSrv().replace.bind(getTemplateSrv()), + }); + } + }); + } + + return []; }; function getTitleFromHref(href: string): string { diff --git a/public/app/routes/routes.tsx b/public/app/routes/routes.tsx index 81c664a6677..aa2cd750594 100644 --- a/public/app/routes/routes.tsx +++ b/public/app/routes/routes.tsx @@ -3,11 +3,12 @@ import LdapPage from 'app/features/admin/ldap/LdapPage'; import UserAdminPage from 'app/features/admin/UserAdminPage'; import { LoginPage } from 'app/core/components/Login/LoginPage'; import config from 'app/core/config'; -import { DashboardRoutes } from 'app/types'; +import { AccessControlAction, DashboardRoutes } from 'app/types'; import { SafeDynamicImport } from '../core/components/DynamicImports/SafeDynamicImport'; import { RouteDescriptor } from '../core/navigation/types'; import { Redirect } from 'react-router-dom'; import ErrorPage from 'app/core/components/ErrorPage/ErrorPage'; +import { contextSrv } from 'app/core/services/context_srv'; export const extraRoutes: RouteDescriptor[] = []; @@ -135,7 +136,11 @@ export function getAppRoutes(): RouteDescriptor[] { { path: '/explore', pageClass: 'page-explore', - roles: () => (config.viewersCanEdit ? [] : ['Editor', 'Admin']), + roles: () => + evaluatePermission( + () => (config.viewersCanEdit ? [] : ['Editor', 'Admin']), + AccessControlAction.DataSourcesExplore + ), component: SafeDynamicImport(() => import(/* webpackChunkName: "explore" */ 'app/features/explore/Wrapper')), }, { @@ -515,3 +520,16 @@ export function getAppRoutes(): RouteDescriptor[] { // ...playlistRoutes, ]; } + +// evaluates access control permission, using fallback if access control is disabled +const evaluatePermission = (fallback: () => string[], action: AccessControlAction): string[] => { + if (!config.featureToggles['accesscontrol']) { + return fallback(); + } + if (contextSrv.hasPermission(action)) { + return []; + } else { + // Hack to reject when user does not have permission + return ['Reject']; + } +}; diff --git a/public/app/types/accessControl.ts b/public/app/types/accessControl.ts index f6af62d68d6..d5e795ed969 100644 --- a/public/app/types/accessControl.ts +++ b/public/app/types/accessControl.ts @@ -33,4 +33,5 @@ export enum AccessControlAction { LDAPUsersRead = 'ldap.user:read', LDAPUsersSync = 'ldap.user:sync', LDAPStatusRead = 'ldap.status:read', + DataSourcesExplore = 'datasources:explore', }