Plugins: Register management endpoints only when external managed is also false (#51802)

* Only define plugin install endpoints when catalog enabled

* add external check

pkg/api/api.go

@@ -328,7 +328,7 @@ func (hs *HTTPServer) registerRoutes() {
 		apiRoute.Any("/plugins/:pluginId/resources/*", hs.CallResource)
 		apiRoute.Get("/plugins/errors", routing.Wrap(hs.GetPluginErrorsList))
 
-		if hs.Cfg.PluginAdminEnabled {
+		if hs.Cfg.PluginAdminEnabled && !hs.Cfg.PluginAdminExternalManageEnabled {
 			apiRoute.Group("/plugins", func(pluginRoute routing.RouteRegister) {
 				pluginRoute.Post("/:pluginId/install", routing.Wrap(hs.InstallPlugin))
 				pluginRoute.Post("/:pluginId/uninstall", routing.Wrap(hs.UninstallPlugin))

pkg/api/fakes.go

@@ -6,6 +6,28 @@ import (
 	"github.com/grafana/grafana/pkg/plugins"
 )
 
+type fakePluginManager struct {
+	plugins map[string]fakePlugin
+}
+
+type fakePlugin struct {
+	pluginID string
+	version  string
+}
+
+func (pm *fakePluginManager) Add(_ context.Context, pluginID, version string) error {
+	pm.plugins[pluginID] = fakePlugin{
+		pluginID: pluginID,
+		version:  version,
+	}
+	return nil
+}
+
+func (pm *fakePluginManager) Remove(_ context.Context, pluginID string) error {
+	delete(pm.plugins, pluginID)
+	return nil
+}
+
 type fakePluginStore struct {
 	plugins.Store
 

pkg/api/plugins_test.go

@@ -4,25 +4,96 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/infra/log/logtest"
-
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/infra/log/logtest"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/web/webtest"
 )
 
+func Test_PluginsInstallAndUninstall(t *testing.T) {
+	type tc struct {
+		pluginAdminEnabled               bool
+		pluginAdminExternalManageEnabled bool
+		expectedHTTPStatus               int
+		expectedHTTPBody                 string
+	}
+	tcs := []tc{
+		{pluginAdminEnabled: true, pluginAdminExternalManageEnabled: true, expectedHTTPStatus: 404, expectedHTTPBody: "404 page not found\n"},
+		{pluginAdminEnabled: true, pluginAdminExternalManageEnabled: false, expectedHTTPStatus: 200, expectedHTTPBody: ""},
+		{pluginAdminEnabled: false, pluginAdminExternalManageEnabled: true, expectedHTTPStatus: 404, expectedHTTPBody: "404 page not found\n"},
+		{pluginAdminEnabled: false, pluginAdminExternalManageEnabled: false, expectedHTTPStatus: 404, expectedHTTPBody: "404 page not found\n"},
+	}
+
+	testName := func(action string, testCase tc) string {
+		return fmt.Sprintf("%s request returns %d when adminEnabled: %t and externalEnabled: %t",
+			action, testCase.expectedHTTPStatus, testCase.pluginAdminEnabled, testCase.pluginAdminExternalManageEnabled)
+	}
+
+	pm := &fakePluginManager{
+		plugins: make(map[string]fakePlugin),
+	}
+	for _, tc := range tcs {
+		srv := SetupAPITestServer(t, func(hs *HTTPServer) {
+			hs.Cfg = &setting.Cfg{
+				PluginAdminEnabled:               tc.pluginAdminEnabled,
+				PluginAdminExternalManageEnabled: tc.pluginAdminExternalManageEnabled,
+			}
+			hs.pluginManager = pm
+		})
+
+		t.Run(testName("Install", tc), func(t *testing.T) {
+			req := srv.NewPostRequest("/api/plugins/test/install", strings.NewReader("{ \"version\": \"1.0.2\" }"))
+			webtest.RequestWithSignedInUser(req, &models.SignedInUser{UserId: 1, OrgId: 1, OrgRole: models.ROLE_EDITOR, IsGrafanaAdmin: true})
+			resp, err := srv.SendJSON(req)
+			require.NoError(t, err)
+
+			body := new(strings.Builder)
+			_, err = io.Copy(body, resp.Body)
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedHTTPBody, body.String())
+			require.NoError(t, resp.Body.Close())
+			require.Equal(t, tc.expectedHTTPStatus, resp.StatusCode)
+
+			if tc.expectedHTTPStatus == 200 {
+				require.Equal(t, fakePlugin{pluginID: "test", version: "1.0.2"}, pm.plugins["test"])
+			}
+		})
+
+		t.Run(testName("Uninstall", tc), func(t *testing.T) {
+			req := srv.NewPostRequest("/api/plugins/test/uninstall", strings.NewReader("{}"))
+			webtest.RequestWithSignedInUser(req, &models.SignedInUser{UserId: 1, OrgId: 1, OrgRole: models.ROLE_VIEWER, IsGrafanaAdmin: true})
+			resp, err := srv.SendJSON(req)
+			require.NoError(t, err)
+
+			body := new(strings.Builder)
+			_, err = io.Copy(body, resp.Body)
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedHTTPBody, body.String())
+			require.NoError(t, resp.Body.Close())
+			require.Equal(t, tc.expectedHTTPStatus, resp.StatusCode)
+
+			if tc.expectedHTTPStatus == 200 {
+				require.Empty(t, pm.plugins)
+			}
+		})
+	}
+}
+
 func Test_GetPluginAssets(t *testing.T) {
 	pluginID := "test-plugin"
 	pluginDir := "."

pkg/tests/testinfra/testinfra.go

@@ -267,6 +267,12 @@ func CreateGrafDir(t *testing.T, opts ...GrafanaOpts) (string, string) {
 			_, err = anonSect.NewKey("plugin_admin_enabled", "true")
 			require.NoError(t, err)
 		}
+		if o.PluginAdminExternalManageEnabled {
+			anonSect, err := cfg.NewSection("plugins")
+			require.NoError(t, err)
+			_, err = anonSect.NewKey("plugin_admin_external_manage_enabled", "true")
+			require.NoError(t, err)
+		}
 		if o.ViewersCanEdit {
 			usersSection, err := cfg.NewSection("users")
 			require.NoError(t, err)
@@ -322,6 +328,7 @@ type GrafanaOpts struct {
 	CatalogAppEnabled                     bool
 	ViewersCanEdit                        bool
 	PluginAdminEnabled                    bool
+	PluginAdminExternalManageEnabled      bool
 	AppModeProduction                     bool
 	DisableLegacyAlerting                 bool
 	EnableUnifiedAlerting                 bool