Connections: Align permissions for Connections page (#60725)

* protect /connection url paths with permissions

These permissions match the original ones at /datasources and /plugins

* add Connections section to navtree only if user has permissions

This commit works only when the easystart plugin is not present.
I'll see what I can do when it is present in the next commit(s).

* update datasources page permissions

The datasources page have Explore buttons on datasource entries,
therefore it makes sense to show this page for those, who can't edit or
create datasources but have explore permissions.
This applies for the traditional Editor role.

* DataSourcesList: link to edit page only if has right to write

If the user doesn't have rights to write datasources, then it's better
to not create a link from cards to the edit page. This way they won't
see the configuration of the data sources either, which is a desirable
outcome.

Also, I moved the query for DataSourcesExplore permission out from the
DataSourcesListView component in the DataSourcesList component, next to
the other permission queries - for the sake of consistency.

* fix permissions for connect data

This way it matches the permissions of the "Plugins" page.

* fix applinks test

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)

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),
+			),
 		),
 	)
 

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)

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
 }

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}
       />
     </Provider>
   );
 };
 
 describe('<DataSourcesList>', () => {
-  beforeEach(() => {
-    (contextSrv.hasPermission as jest.Mock) = jest.fn().mockReturnValue(true);
-  });
-
   it('should render action bar', async () => {
     setup();
 
@@ -53,12 +48,4 @@ describe('<DataSourcesList>', () => {
     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);
-  });
 });

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 (
     <DataSourcesListView
@@ -28,6 +30,8 @@ export function DataSourcesList() {
       dataSourcesCount={dataSourcesCount}
       isLoading={!hasFetched}
       hasCreateRights={hasCreateRights}
+      hasWriteRights={hasWriteRights}
+      hasExploreRights={hasExploreRights}
     />
   );
 }
@@ -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 <PageLoader />;
@@ -75,7 +87,7 @@ export function DataSourcesListView({ dataSources, dataSourcesCount, isLoading,
           const dsLink = config.appSubUrl + dataSourcesRoutes.Edit.replace(/:uid/gi, dataSource.uid);
           return (
             <li key={dataSource.uid}>
-              <Card href={dsLink}>
+              <Card href={hasWriteRights ? dsLink : undefined}>
                 <Card.Heading>{dataSource.name}</Card.Heading>
                 <Card.Figure>
                   <img src={dataSource.typeLogoUrl} alt="" height="40px" width="40px" className={styles.logo} />
@@ -91,7 +103,7 @@ export function DataSourcesListView({ dataSources, dataSourcesCount, isLoading,
                   <LinkButton icon="apps" fill="outline" variant="secondary" href={config.appSubUrl + '/dashboard/new'}>
                     Build a Dashboard
                   </LinkButton>
-                  {canExploreDataSources && (
+                  {hasExploreRights && (
                     <LinkButton
                       icon="compass"
                       fill="outline"

public/app/features/datasources/pages/DataSourcesListPage.test.tsx

@@ -52,15 +52,52 @@ describe('Render', () => {
     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 () => {