From 40dff288cd2fd3381775f07cf04c8de5da2c0ded Mon Sep 17 00:00:00 2001 From: Will Browne Date: Wed, 6 Jul 2022 16:22:59 +0200 Subject: [PATCH] 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 | 2 +- pkg/api/fakes.go | 22 ++++++++++ pkg/api/plugins_test.go | 75 +++++++++++++++++++++++++++++++- pkg/tests/testinfra/testinfra.go | 7 +++ 4 files changed, 103 insertions(+), 3 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 760005e61d5..57fec6b43e3 100644 --- a/pkg/api/api.go +++ b/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)) diff --git a/pkg/api/fakes.go b/pkg/api/fakes.go index 23b5a89c52d..6b1eda6f2ff 100644 --- a/pkg/api/fakes.go +++ b/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 diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index 9e869285efd..8aefeac8e62 100644 --- a/pkg/api/plugins_test.go +++ b/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 := "." diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index 1213bf8bba5..cae8ee96d75 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/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