From 1324186f875ded57e6c30c53df9fb4087b756d42 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Fri, 15 Dec 2023 16:37:39 +0100 Subject: [PATCH] Chore: Evaluate if an app is disabled for API requests (#79564) --- pkg/api/api.go | 12 +++--- pkg/api/plugin_checks.go | 43 ++++++++++++++++++ pkg/api/plugin_checks_test.go | 72 +++++++++++++++++++++++++++++++ pkg/api/plugin_dashboards_test.go | 7 +++ 4 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 pkg/api/plugin_checks.go create mode 100644 pkg/api/plugin_checks_test.go diff --git a/pkg/api/api.go b/pkg/api/api.go index 93db84e57bb..f3a27c5ceda 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -386,12 +386,12 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Get("/plugins", routing.Wrap(hs.GetPluginList)) apiRoute.Get("/plugins/:pluginId/settings", routing.Wrap(hs.GetPluginSettingByID)) // RBAC check performed in handler for App Plugins apiRoute.Get("/plugins/:pluginId/markdown/:name", routing.Wrap(hs.GetPluginMarkdown)) - apiRoute.Get("/plugins/:pluginId/health", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), routing.Wrap(hs.CheckHealth)) - apiRoute.Any("/plugins/:pluginId/resources", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, pluginIDScope)), hs.CallResource) - apiRoute.Any("/plugins/:pluginId/resources/*", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, pluginIDScope)), hs.CallResource) + apiRoute.Get("/plugins/:pluginId/health", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), checkAppEnabled(hs.pluginStore, hs.PluginSettings), routing.Wrap(hs.CheckHealth)) + apiRoute.Any("/plugins/:pluginId/resources", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, pluginIDScope)), checkAppEnabled(hs.pluginStore, hs.PluginSettings), hs.CallResource) + apiRoute.Any("/plugins/:pluginId/resources/*", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, pluginIDScope)), checkAppEnabled(hs.pluginStore, hs.PluginSettings), hs.CallResource) apiRoute.Get("/plugins/errors", routing.Wrap(hs.GetPluginErrorsList)) - apiRoute.Any("/plugin-proxy/:pluginId/*", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, pluginIDScope)), hs.ProxyPluginRequest) - apiRoute.Any("/plugin-proxy/:pluginId", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, pluginIDScope)), hs.ProxyPluginRequest) + apiRoute.Any("/plugin-proxy/:pluginId/*", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, pluginIDScope)), checkAppEnabled(hs.pluginStore, hs.PluginSettings), hs.ProxyPluginRequest) + apiRoute.Any("/plugin-proxy/:pluginId", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, pluginIDScope)), checkAppEnabled(hs.pluginStore, hs.PluginSettings), hs.ProxyPluginRequest) if hs.Cfg.PluginAdminEnabled && (hs.Features.IsEnabledGlobally(featuremgmt.FlagManagedPluginsInstall) || !hs.Cfg.PluginAdminExternalManageEnabled) { apiRoute.Group("/plugins", func(pluginRoute routing.RouteRegister) { @@ -401,7 +401,7 @@ func (hs *HTTPServer) registerRoutes() { } apiRoute.Group("/plugins", func(pluginRoute routing.RouteRegister) { - pluginRoute.Get("/:pluginId/dashboards/", reqOrgAdmin, routing.Wrap(hs.GetPluginDashboards)) + pluginRoute.Get("/:pluginId/dashboards/", reqOrgAdmin, checkAppEnabled(hs.pluginStore, hs.PluginSettings), routing.Wrap(hs.GetPluginDashboards)) pluginRoute.Post("/:pluginId/settings", authorize(ac.EvalPermission(pluginaccesscontrol.ActionWrite, pluginIDScope)), routing.Wrap(hs.UpdatePluginSetting)) pluginRoute.Get("/:pluginId/metrics", reqOrgAdmin, routing.Wrap(hs.CollectPluginMetrics)) }) diff --git a/pkg/api/plugin_checks.go b/pkg/api/plugin_checks.go new file mode 100644 index 00000000000..9a7deb87630 --- /dev/null +++ b/pkg/api/plugin_checks.go @@ -0,0 +1,43 @@ +package api + +import ( + "errors" + "net/http" + + contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings" + "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" + "github.com/grafana/grafana/pkg/web" +) + +func checkAppEnabled(pluginStore pluginstore.Store, pluginSettings pluginsettings.Service) func(c *contextmodel.ReqContext) { + return func(c *contextmodel.ReqContext) { + pluginID := web.Params(c.Req)[":pluginId"] + p, exists := pluginStore.Plugin(c.Req.Context(), pluginID) + if !exists { + c.JsonApiErr(http.StatusNotFound, "Plugin not found", nil) + return + } + if !p.IsApp() { + return + } + + ps, err := pluginSettings.GetPluginSettingByPluginID(c.Req.Context(), &pluginsettings.GetByPluginIDArgs{ + OrgID: c.OrgID, + PluginID: pluginID, + }) + if err != nil { + if errors.Is(err, pluginsettings.ErrPluginSettingNotFound) { + c.JsonApiErr(http.StatusNotFound, "Plugin not found", nil) + return + } + c.JsonApiErr(http.StatusInternalServerError, "Failed to get plugin settings", err) + return + } + + if !ps.Enabled { + c.JsonApiErr(http.StatusNotFound, "Plugin is not enabled", nil) + return + } + } +} diff --git a/pkg/api/plugin_checks_test.go b/pkg/api/plugin_checks_test.go new file mode 100644 index 00000000000..fffa04453f3 --- /dev/null +++ b/pkg/api/plugin_checks_test.go @@ -0,0 +1,72 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/grafana/grafana/pkg/plugins" + contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings" + "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/web" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHTTPServer_CheckEnabled(t *testing.T) { + tests := []struct { + name string + pluginID string + expectedCode int + }{ + { + name: "should return a 404 if the plugin doesn't exist", + pluginID: "missing", + expectedCode: 404, + }, + { + name: "should not set an error code if the plugin is not an app", + pluginID: "mysql", + expectedCode: 0, // unset + }, + { + name: "should not set an error code if the plugin is enabled", + pluginID: "grafana-test-app", + expectedCode: 0, // unset + }, + { + name: "should return a 404 if the plugin is disabled", + pluginID: "grafana-test-app_disabled", + expectedCode: 404, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hs := &HTTPServer{} + hs.pluginStore = &pluginstore.FakePluginStore{ + PluginList: []pluginstore.Plugin{ + {JSONData: plugins.JSONData{ID: "mysql"}}, + {JSONData: plugins.JSONData{Type: plugins.TypeApp, ID: "grafana-test-app"}}, + {JSONData: plugins.JSONData{Type: plugins.TypeApp, ID: "grafana-test-app_disabled"}}, + }, + } + hs.PluginSettings = &pluginsettings.FakePluginSettings{Plugins: map[string]*pluginsettings.DTO{ + "grafana-test-app": {ID: 0, OrgID: 1, PluginID: "grafana-test-app", PluginVersion: "1.0.0", Enabled: true}, + "grafana-test-app_disabled": {ID: 0, OrgID: 1, PluginID: "grafana-test-app_disabled", PluginVersion: "1.0.0", Enabled: false}, + }} + httpReq, err := http.NewRequest(http.MethodGet, "", nil) + httpReq = web.SetURLParams(httpReq, map[string]string{":pluginId": tt.pluginID}) + require.NoError(t, err) + + responseWriter := web.NewResponseWriter("GET", httptest.NewRecorder()) + c := &contextmodel.ReqContext{ + Context: &web.Context{Req: httpReq, Resp: responseWriter}, + SignedInUser: &user.SignedInUser{OrgID: 1}, + } + checkAppEnabled(hs.pluginStore, hs.PluginSettings)(c) + assert.Equal(t, tt.expectedCode, c.Resp.Status()) + }) + } +} diff --git a/pkg/api/plugin_dashboards_test.go b/pkg/api/plugin_dashboards_test.go index ae3709ba39c..b815b42428c 100644 --- a/pkg/api/plugin_dashboards_test.go +++ b/pkg/api/plugin_dashboards_test.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/plugindashboards" + "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/web/webtest" @@ -44,6 +45,12 @@ func TestGetPluginDashboards(t *testing.T) { s := SetupAPITestServer(t, func(hs *HTTPServer) { hs.pluginDashboardService = pluginDashboardService hs.QuotaService = quotatest.New(false, nil) + hs.pluginStore = &pluginstore.FakePluginStore{ + PluginList: []pluginstore.Plugin{ + {JSONData: plugins.JSONData{ID: existingPluginID}}, + {JSONData: plugins.JSONData{ID: "boom"}}, + }, + } }) t.Run("Not signed in should return 404 Not Found", func(t *testing.T) {