diff --git a/pkg/api/api.go b/pkg/api/api.go index 2036b617b2d..943a6156ebd 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -132,6 +132,14 @@ func (hs *HTTPServer) registerRoutes() { r.Get("/plugins/:id/", middleware.CanAdminPlugins(hs.Cfg), hs.Index) r.Get("/plugins/:id/edit", middleware.CanAdminPlugins(hs.Cfg), hs.Index) // deprecated r.Get("/plugins/:id/page/:page", middleware.CanAdminPlugins(hs.Cfg), hs.Index) + + r.Get("/connections/your-connections/datasources", authorize(reqOrgAdmin, datasources.ConfigurationPageAccess), hs.Index) + r.Get("/connections/your-connections/datasources/new", authorize(reqOrgAdmin, datasources.NewPageAccess), hs.Index) + r.Get("/connections/your-connections/datasources/edit/*", authorize(reqOrgAdmin, datasources.EditPageAccess), hs.Index) + r.Get("/connections/connect-data", middleware.CanAdminPlugins(hs.Cfg), hs.Index) + r.Get("/connections/connect-data/datasources/:id", middleware.CanAdminPlugins(hs.Cfg), hs.Index) + r.Get("/connections/connect-data/datasources/:id/page/:page", middleware.CanAdminPlugins(hs.Cfg), hs.Index) + // App Root Page appPluginIDScope := plugins.ScopeProvider.GetResourceScope(ac.Parameter(":id")) r.Get("/a/:id/*", authorize(reqSignedIn, ac.EvalPermission(plugins.ActionAppAccess, appPluginIDScope)), hs.Index) diff --git a/pkg/services/datasources/accesscontrol.go b/pkg/services/datasources/accesscontrol.go index 4b16ab1479c..aae4773e18a 100644 --- a/pkg/services/datasources/accesscontrol.go +++ b/pkg/services/datasources/accesscontrol.go @@ -24,12 +24,15 @@ var ( var ( // ConfigurationPageAccess is used to protect the "Configure > Data sources" tab access - ConfigurationPageAccess = accesscontrol.EvalAll( - accesscontrol.EvalPermission(ActionRead), - accesscontrol.EvalAny( - accesscontrol.EvalPermission(ActionCreate), - accesscontrol.EvalPermission(ActionDelete), - accesscontrol.EvalPermission(ActionWrite), + ConfigurationPageAccess = accesscontrol.EvalAny( + accesscontrol.EvalPermission(accesscontrol.ActionDatasourcesExplore), + accesscontrol.EvalAll( + accesscontrol.EvalPermission(ActionRead), + accesscontrol.EvalAny( + accesscontrol.EvalPermission(ActionCreate), + accesscontrol.EvalPermission(ActionDelete), + accesscontrol.EvalPermission(ActionWrite), + ), ), ) diff --git a/pkg/services/navtree/navtreeimpl/applinks_test.go b/pkg/services/navtree/navtreeimpl/applinks_test.go index f0ed89967ba..ba32c38c9b4 100644 --- a/pkg/services/navtree/navtreeimpl/applinks_test.go +++ b/pkg/services/navtree/navtreeimpl/applinks_test.go @@ -25,6 +25,7 @@ func TestAddAppLinks(t *testing.T) { reqCtx := &models.ReqContext{SignedInUser: &user.SignedInUser{}, Context: &web.Context{Req: httpReq}} permissions := []ac.Permission{ {Action: plugins.ActionAppAccess, Scope: "*"}, + {Action: plugins.ActionInstall, Scope: "*"}, } testApp1 := plugins.PluginDTO{ @@ -290,18 +291,20 @@ func TestAddAppLinks(t *testing.T) { treeRoot.AddSection(service.buildDataConnectionsNavLink(reqCtx)) connectionsNode := treeRoot.FindById("connections") require.Equal(t, "Connections", connectionsNode.Text) - require.Equal(t, "Connect data", connectionsNode.Children[1].Text) - require.Equal(t, "connections-connect-data", connectionsNode.Children[1].Id) // Original "Connect data" page - require.Equal(t, "", connectionsNode.Children[1].PluginID) + + connectDataNode := connectionsNode.Children[0] + require.Equal(t, "Connect data", connectDataNode.Text) + require.Equal(t, "connections-connect-data", connectDataNode.Id) // Original "Connect data" page + require.Equal(t, "", connectDataNode.PluginID) err := service.addAppLinks(&treeRoot, reqCtx) // Check if the standalone plugin page appears under the section where we registered it require.NoError(t, err) require.Equal(t, "Connections", connectionsNode.Text) - require.Equal(t, "Connect data", connectionsNode.Children[1].Text) - require.Equal(t, "standalone-plugin-page-/connections/connect-data", connectionsNode.Children[1].Id) // Overridden "Connect data" page - require.Equal(t, "test-app3", connectionsNode.Children[1].PluginID) + require.Equal(t, "Connect data", connectDataNode.Text) + require.Equal(t, "standalone-plugin-page-/connections/connect-data", connectDataNode.Id) // Overridden "Connect data" page + require.Equal(t, "test-app3", connectDataNode.PluginID) // Check if the standalone plugin page does not appear under the app section anymore // (Also checking if the Default Page got removed) diff --git a/pkg/services/navtree/navtreeimpl/navtree.go b/pkg/services/navtree/navtreeimpl/navtree.go index bef8730c2a7..73df6e82de1 100644 --- a/pkg/services/navtree/navtreeimpl/navtree.go +++ b/pkg/services/navtree/navtreeimpl/navtree.go @@ -12,6 +12,7 @@ import ( ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/navtree" "github.com/grafana/grafana/pkg/services/org" @@ -155,7 +156,9 @@ func (s *ServiceImpl) GetNavTree(c *models.ReqContext, hasEditPerm bool, prefs * } if s.features.IsEnabled(featuremgmt.FlagDataConnectionsConsole) { - treeRoot.AddSection(s.buildDataConnectionsNavLink(c)) + if connectionsSection := s.buildDataConnectionsNavLink(c); connectionsSection != nil { + treeRoot.AddSection(connectionsSection) + } } if s.features.IsEnabled(featuremgmt.FlagLivePipeline) { @@ -558,45 +561,55 @@ func (s *ServiceImpl) buildAlertNavLinks(c *models.ReqContext, hasEditPerm bool) } func (s *ServiceImpl) buildDataConnectionsNavLink(c *models.ReqContext) *navtree.NavLink { + hasAccess := ac.HasAccess(s.accessControl, c) + var children []*navtree.NavLink var navLink *navtree.NavLink baseUrl := s.cfg.AppSubURL + "/connections" - // Your connections - children = append(children, &navtree.NavLink{ - Id: "connections-your-connections", - Text: "Your connections", - SubTitle: "Manage your existing connections", - Url: baseUrl + "/your-connections", - // Datasources - Children: []*navtree.NavLink{{ - Id: "connections-your-connections-datasources", - Text: "Data sources", - SubTitle: "View and manage your connected data source connections", - Url: baseUrl + "/your-connections/datasources", - }}, - }) - - // Connect data - children = append(children, &navtree.NavLink{ - Id: "connections-connect-data", - Text: "Connect data", - SubTitle: "Browse and create new connections", - Url: s.cfg.AppSubURL + "/connections/connect-data", - Children: []*navtree.NavLink{}, - }) - - // Connections (main) - navLink = &navtree.NavLink{ - Text: "Connections", - Icon: "adjust-circle", - Id: "connections", - Url: baseUrl, - Children: children, - Section: navtree.NavSectionCore, - SortWeight: navtree.WeightDataConnections, + if hasAccess(ac.ReqOrgAdmin, datasources.ConfigurationPageAccess) { + // Your connections + children = append(children, &navtree.NavLink{ + Id: "connections-your-connections", + Text: "Your connections", + SubTitle: "Manage your existing connections", + Url: baseUrl + "/your-connections", + // Datasources + Children: []*navtree.NavLink{{ + Id: "connections-your-connections-datasources", + Text: "Data sources", + SubTitle: "View and manage your connected data source connections", + Url: baseUrl + "/your-connections/datasources", + }}, + }) } - return navLink + // Connect data + // FIXME: while we don't have a permissions for listing plugins the legacy check has to stay as a default + if plugins.ReqCanAdminPlugins(s.cfg)(c) || hasAccess(plugins.ReqCanAdminPlugins(s.cfg), plugins.AdminAccessEvaluator) { + children = append(children, &navtree.NavLink{ + Id: "connections-connect-data", + Text: "Connect data", + SubTitle: "Browse and create new connections", + Url: s.cfg.AppSubURL + "/connections/connect-data", + Children: []*navtree.NavLink{}, + }) + } + + if len(children) > 0 { + // Connections (main) + navLink = &navtree.NavLink{ + Text: "Connections", + Icon: "adjust-circle", + Id: "connections", + Url: baseUrl, + Children: children, + Section: navtree.NavSectionCore, + SortWeight: navtree.WeightDataConnections, + } + + return navLink + } + return nil } diff --git a/public/app/features/datasources/components/DataSourcesList.test.tsx b/public/app/features/datasources/components/DataSourcesList.test.tsx index bb6cf117858..36db8204a1d 100644 --- a/public/app/features/datasources/components/DataSourcesList.test.tsx +++ b/public/app/features/datasources/components/DataSourcesList.test.tsx @@ -2,15 +2,12 @@ import { render, screen } from '@testing-library/react'; import React from 'react'; import { Provider } from 'react-redux'; -import { contextSrv } from 'app/core/services/context_srv'; import { configureStore } from 'app/store/configureStore'; import { getMockDataSources } from '../__mocks__'; import { DataSourcesListView } from './DataSourcesList'; -jest.mock('app/core/services/context_srv'); - const setup = () => { const store = configureStore(); @@ -21,16 +18,14 @@ const setup = () => { dataSourcesCount={3} isLoading={false} hasCreateRights={true} + hasWriteRights={true} + hasExploreRights={true} /> ); }; describe('', () => { - beforeEach(() => { - (contextSrv.hasPermission as jest.Mock) = jest.fn().mockReturnValue(true); - }); - it('should render action bar', async () => { setup(); @@ -53,12 +48,4 @@ describe('', () => { expect(await screen.findByRole('heading', { name: 'dataSource-0' })).toBeInTheDocument(); expect(await screen.findByRole('link', { name: 'dataSource-0' })).toBeInTheDocument(); }); - - it('should not render Explore button if user has no permissions', async () => { - (contextSrv.hasPermission as jest.Mock) = jest.fn().mockReturnValue(false); - setup(); - - expect(await screen.findAllByRole('link', { name: 'Build a Dashboard' })).toHaveLength(3); - expect(screen.queryAllByRole('link', { name: 'Explore' })).toHaveLength(0); - }); }); diff --git a/public/app/features/datasources/components/DataSourcesList.tsx b/public/app/features/datasources/components/DataSourcesList.tsx index cc41d384036..281d2e7dd67 100644 --- a/public/app/features/datasources/components/DataSourcesList.tsx +++ b/public/app/features/datasources/components/DataSourcesList.tsx @@ -21,6 +21,8 @@ export function DataSourcesList() { const dataSourcesCount = useSelector(({ dataSources }: StoreState) => getDataSourcesCount(dataSources)); const hasFetched = useSelector(({ dataSources }: StoreState) => dataSources.hasFetched); const hasCreateRights = contextSrv.hasPermission(AccessControlAction.DataSourcesCreate); + const hasWriteRights = contextSrv.hasPermission(AccessControlAction.DataSourcesWrite); + const hasExploreRights = contextSrv.hasPermission(AccessControlAction.DataSourcesExplore); return ( ); } @@ -37,12 +41,20 @@ export type ViewProps = { dataSourcesCount: number; isLoading: boolean; hasCreateRights: boolean; + hasWriteRights: boolean; + hasExploreRights: boolean; }; -export function DataSourcesListView({ dataSources, dataSourcesCount, isLoading, hasCreateRights }: ViewProps) { +export function DataSourcesListView({ + dataSources, + dataSourcesCount, + isLoading, + hasCreateRights, + hasWriteRights, + hasExploreRights, +}: ViewProps) { const styles = useStyles2(getStyles); const dataSourcesRoutes = useDataSourcesRoutes(); - const canExploreDataSources = contextSrv.hasPermission(AccessControlAction.DataSourcesExplore); if (isLoading) { return ; @@ -75,7 +87,7 @@ export function DataSourcesListView({ dataSources, dataSourcesCount, isLoading, const dsLink = config.appSubUrl + dataSourcesRoutes.Edit.replace(/:uid/gi, dataSource.uid); return (
  • - + {dataSource.name} @@ -91,7 +103,7 @@ export function DataSourcesListView({ dataSources, dataSourcesCount, isLoading, Build a Dashboard - {canExploreDataSources && ( + {hasExploreRights && ( { expect(await screen.findByRole('link', { name: 'Add data source' })).toBeInTheDocument(); }); - it('should disable the "Add data source" button if user has no permissions', async () => { - (contextSrv.hasPermission as jest.Mock) = jest.fn().mockReturnValue(false); + describe('when user has no permissions', () => { + beforeEach(() => { + (contextSrv.hasPermission as jest.Mock) = jest.fn().mockReturnValue(false); + }); + + it('should disable the "Add data source" button if user has no permissions', async () => { + setup({ isSortAscending: true }); + + expect(await screen.findByRole('heading', { name: 'Configuration' })).toBeInTheDocument(); + expect(await screen.findByRole('link', { name: 'Documentation' })).toBeInTheDocument(); + expect(await screen.findByRole('link', { name: 'Support' })).toBeInTheDocument(); + expect(await screen.findByRole('link', { name: 'Community' })).toBeInTheDocument(); + expect(await screen.findByRole('link', { name: 'Add data source' })).toHaveStyle('pointer-events: none'); + }); + + it('should not show the Explore button', async () => { + getDataSourcesMock.mockResolvedValue(getMockDataSources(3)); + setup({ isSortAscending: true }); + + expect(await screen.findAllByRole('link', { name: 'Build a Dashboard' })).toHaveLength(3); + expect(screen.queryAllByRole('link', { name: 'Explore' })).toHaveLength(0); + }); + + it('should not link cards to edit pages', async () => { + getDataSourcesMock.mockResolvedValue(getMockDataSources(1)); + setup({ isSortAscending: true }); + + expect(await screen.findByRole('heading', { name: 'dataSource-0' })).toBeInTheDocument(); + expect(await screen.queryByRole('link', { name: 'dataSource-0' })).toBeNull(); + }); + }); + + it('should show the Explore button', async () => { + getDataSourcesMock.mockResolvedValue(getMockDataSources(3)); setup({ isSortAscending: true }); - expect(await screen.findByRole('heading', { name: 'Configuration' })).toBeInTheDocument(); - expect(await screen.findByRole('link', { name: 'Documentation' })).toBeInTheDocument(); - expect(await screen.findByRole('link', { name: 'Support' })).toBeInTheDocument(); - expect(await screen.findByRole('link', { name: 'Community' })).toBeInTheDocument(); - expect(await screen.findByRole('link', { name: 'Add data source' })).toHaveStyle('pointer-events: none'); + expect(await screen.findAllByRole('link', { name: 'Build a Dashboard' })).toHaveLength(3); + expect(screen.queryAllByRole('link', { name: 'Explore' })).toHaveLength(3); + }); + + it('should link cards to edit pages', async () => { + getDataSourcesMock.mockResolvedValue(getMockDataSources(1)); + setup({ isSortAscending: true }); + + expect(await screen.findByRole('heading', { name: 'dataSource-0' })).toBeInTheDocument(); + expect(await screen.findByRole('link', { name: 'dataSource-0' })).toBeInTheDocument(); }); it('should render action bar and datasources', async () => {