diff --git a/pkg/api/plugin_proxy.go b/pkg/api/plugin_proxy.go index 4a81afbc34f..1a8a3fbd190 100644 --- a/pkg/api/plugin_proxy.go +++ b/pkg/api/plugin_proxy.go @@ -51,7 +51,7 @@ func (hs *HTTPServer) ProxyPluginRequest(c *contextmodel.ReqContext) { } proxyPath := getProxyPath(c) - p, err := pluginproxy.NewPluginProxy(ps, plugin.Routes, c, proxyPath, hs.Cfg, hs.SecretsService, hs.tracer, pluginProxyTransport, hs.Features) + p, err := pluginproxy.NewPluginProxy(ps, plugin.Routes, c, proxyPath, hs.Cfg, hs.SecretsService, hs.tracer, pluginProxyTransport, hs.AccessControl, hs.Features) if err != nil { c.JsonApiErr(http.StatusInternalServerError, "Failed to create plugin proxy", err) return diff --git a/pkg/api/pluginproxy/pluginproxy.go b/pkg/api/pluginproxy/pluginproxy.go index b206591a656..10c0c38b148 100644 --- a/pkg/api/pluginproxy/pluginproxy.go +++ b/pkg/api/pluginproxy/pluginproxy.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings" @@ -22,6 +23,7 @@ import ( ) type PluginProxy struct { + accessControl ac.AccessControl ps *pluginsettings.DTO pluginRoutes []*plugins.Route ctx *contextmodel.ReqContext @@ -37,8 +39,9 @@ type PluginProxy struct { // NewPluginProxy creates a plugin proxy. func NewPluginProxy(ps *pluginsettings.DTO, routes []*plugins.Route, ctx *contextmodel.ReqContext, proxyPath string, cfg *setting.Cfg, secretsService secrets.Service, tracer tracing.Tracer, - transport *http.Transport, features featuremgmt.FeatureToggles) (*PluginProxy, error) { + transport *http.Transport, accessControl ac.AccessControl, features featuremgmt.FeatureToggles) (*PluginProxy, error) { return &PluginProxy{ + accessControl: accessControl, ps: ps, pluginRoutes: routes, ctx: ctx, @@ -67,11 +70,9 @@ func (proxy *PluginProxy) HandleRequest() { continue } - if route.ReqRole.IsValid() { - if !proxy.ctx.HasUserRole(route.ReqRole) { - proxy.ctx.JsonApiErr(http.StatusForbidden, "plugin proxy route access denied", nil) - return - } + if !proxy.hasAccessToRoute(route) { + proxy.ctx.JsonApiErr(http.StatusForbidden, "plugin proxy route access denied", nil) + return } if path, exists := params["*"]; exists { @@ -120,6 +121,21 @@ func (proxy *PluginProxy) HandleRequest() { reverseProxy.ServeHTTP(proxy.ctx.Resp, proxy.ctx.Req) } +func (proxy *PluginProxy) hasAccessToRoute(route *plugins.Route) bool { + useRBAC := proxy.features.IsEnabled(proxy.ctx.Req.Context(), featuremgmt.FlagAccessControlOnCall) && route.RequiresRBACAction() + if useRBAC { + hasAccess := ac.HasAccess(proxy.accessControl, proxy.ctx)(ac.EvalPermission(route.ReqAction)) + if !hasAccess { + proxy.ctx.Logger.Debug("plugin route is covered by RBAC, user doesn't have access", "route", proxy.ctx.Req.URL.Path) + } + return hasAccess + } + if route.ReqRole.IsValid() { + return proxy.ctx.HasUserRole(route.ReqRole) + } + return true +} + func (proxy PluginProxy) director(req *http.Request) { secureJsonData, err := proxy.secretsService.DecryptJsonData(proxy.ctx.Req.Context(), proxy.ps.SecureJSONData) if err != nil { diff --git a/pkg/api/pluginproxy/pluginproxy_test.go b/pkg/api/pluginproxy/pluginproxy_test.go index 89a5467c168..9668693482a 100644 --- a/pkg/api/pluginproxy/pluginproxy_test.go +++ b/pkg/api/pluginproxy/pluginproxy_test.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" @@ -262,7 +263,8 @@ func TestPluginProxy(t *testing.T) { ps := &pluginsettings.DTO{ SecureJSONData: map[string][]byte{}, } - proxy, err := NewPluginProxy(ps, routes, ctx, "", &setting.Cfg{}, secretsService, tracing.InitializeTracerForTest(), &http.Transport{}, featuremgmt.WithFeatures()) + cfg := &setting.Cfg{} + proxy, err := NewPluginProxy(ps, routes, ctx, "", cfg, secretsService, tracing.InitializeTracerForTest(), &http.Transport{}, acimpl.ProvideAccessControl(cfg), featuremgmt.WithFeatures()) require.NoError(t, err) proxy.HandleRequest() @@ -400,7 +402,8 @@ func TestPluginProxyRoutes(t *testing.T) { ps := &pluginsettings.DTO{ SecureJSONData: map[string][]byte{}, } - proxy, err := NewPluginProxy(ps, testRoutes, ctx, tc.proxyPath, &setting.Cfg{}, secretsService, tracing.InitializeTracerForTest(), &http.Transport{}, featuremgmt.WithFeatures()) + cfg := &setting.Cfg{} + proxy, err := NewPluginProxy(ps, testRoutes, ctx, tc.proxyPath, cfg, secretsService, tracing.InitializeTracerForTest(), &http.Transport{}, acimpl.ProvideAccessControl(cfg), featuremgmt.WithFeatures()) require.NoError(t, err) proxy.HandleRequest() @@ -421,6 +424,121 @@ func TestPluginProxyRoutes(t *testing.T) { } } +func TestPluginProxyRoutesAccessControl(t *testing.T) { + routes := []*plugins.Route{ + { + Path: "settings", + Method: "GET", + URL: "http://localhost/api/settings", + ReqRole: org.RoleAdmin, // Protected by role + }, + { + Path: "projects", + Method: "GET", + URL: "http://localhost/api/projects", + ReqAction: "plugin-id.projects:read", // Protected by RBAC action + }, + } + + tcs := []struct { + proxyPath string + usrRole org.RoleType + usrPerms map[string][]string + expectedURLPath string + expectedStatus int + }{ + { + proxyPath: "/settings", + usrRole: org.RoleAdmin, + expectedURLPath: "/api/settings", + expectedStatus: http.StatusOK, + }, + { + proxyPath: "/settings", + usrRole: org.RoleViewer, + expectedURLPath: "/api/settings", + expectedStatus: http.StatusForbidden, + }, + { + proxyPath: "/projects", + usrPerms: map[string][]string{"plugin-id.projects:read": {}}, + expectedURLPath: "/api/projects", + expectedStatus: http.StatusOK, + }, + { + proxyPath: "/projects", + usrPerms: map[string][]string{}, + expectedURLPath: "/api/projects", + expectedStatus: http.StatusForbidden, + }, + } + + for _, tc := range tcs { + t.Run(fmt.Sprintf("Should enforce RBAC when proxying path %s %s", tc.proxyPath, http.StatusText(tc.expectedStatus)), func(t *testing.T) { + secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + requestHandled := false + requestURL := "" + backendServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestURL = r.URL.RequestURI() + w.WriteHeader(200) + _, _ = w.Write([]byte("I am the backend")) + requestHandled = true + })) + t.Cleanup(backendServer.Close) + + backendURL, err := url.Parse(backendServer.URL) + require.NoError(t, err) + + testRoutes := make([]*plugins.Route, len(routes)) + for i, r := range routes { + u, err := url.Parse(r.URL) + require.NoError(t, err) + u.Scheme = backendURL.Scheme + u.Host = backendURL.Host + testRoute := *r + testRoute.URL = u.String() + testRoutes[i] = &testRoute + } + + responseWriter := web.NewResponseWriter("GET", httptest.NewRecorder()) + + ctx := &contextmodel.ReqContext{ + Logger: logger.New("pluginproxy-test"), + SignedInUser: &user.SignedInUser{ + OrgID: 1, + OrgRole: tc.usrRole, + Permissions: map[int64]map[string][]string{1: tc.usrPerms}, + }, + Context: &web.Context{ + Req: httptest.NewRequest("GET", tc.proxyPath, nil), + Resp: responseWriter, + }, + } + ps := &pluginsettings.DTO{ + SecureJSONData: map[string][]byte{}, + } + cfg := &setting.Cfg{} + proxy, err := NewPluginProxy(ps, testRoutes, ctx, tc.proxyPath, cfg, secretsService, tracing.InitializeTracerForTest(), &http.Transport{}, acimpl.ProvideAccessControl(cfg), featuremgmt.WithFeatures(featuremgmt.FlagAccessControlOnCall)) + require.NoError(t, err) + proxy.HandleRequest() + + for { + if requestHandled || ctx.Resp.Written() { + break + } + } + + require.Equal(t, tc.expectedStatus, ctx.Resp.Status()) + + if tc.expectedStatus == http.StatusForbidden { + return + } + + require.Equal(t, tc.expectedURLPath, requestURL) + }) + } +} + // getPluginProxiedRequest is a helper for easier setup of tests based on global config and ReqContext. func getPluginProxiedRequest(t *testing.T, ps *pluginsettings.DTO, secretsService secrets.Service, ctx *contextmodel.ReqContext, cfg *setting.Cfg, route *plugins.Route) *http.Request { // insert dummy route if none is specified @@ -431,7 +549,7 @@ func getPluginProxiedRequest(t *testing.T, ps *pluginsettings.DTO, secretsServic ReqRole: org.RoleEditor, } } - proxy, err := NewPluginProxy(ps, []*plugins.Route{}, ctx, "", cfg, secretsService, tracing.InitializeTracerForTest(), &http.Transport{}, featuremgmt.WithFeatures()) + proxy, err := NewPluginProxy(ps, []*plugins.Route{}, ctx, "", cfg, secretsService, tracing.InitializeTracerForTest(), &http.Transport{}, acimpl.ProvideAccessControl(cfg), featuremgmt.WithFeatures()) require.NoError(t, err) req, err := http.NewRequest(http.MethodGet, "/api/plugin-proxy/grafana-simple-app/api/v4/alerts", nil) diff --git a/pkg/plugins/plugindef/plugindef.cue b/pkg/plugins/plugindef/plugindef.cue index b2d05e658e9..e2865a0f200 100644 --- a/pkg/plugins/plugindef/plugindef.cue +++ b/pkg/plugins/plugindef/plugindef.cue @@ -364,6 +364,9 @@ schemas: [{ reqSignedIn?: bool reqRole?: string + // RBAC action the user must have to access the route. i.e. plugin-id.projects:read + reqAction?: string + // For data source plugins. Route headers adds HTTP headers to the // proxied request. headers?: [...#Header] diff --git a/pkg/plugins/plugindef/plugindef_types_gen.go b/pkg/plugins/plugindef/plugindef_types_gen.go index ef0c80d25f9..d4ec3ded09d 100644 --- a/pkg/plugins/plugindef/plugindef_types_gen.go +++ b/pkg/plugins/plugindef/plugindef_types_gen.go @@ -461,7 +461,10 @@ type Route struct { // For data source plugins. The route path that is replaced by the // route URL field when proxying the call. - Path *string `json:"path,omitempty"` + Path *string `json:"path,omitempty"` + + // RBAC action the user must have to access the route. i.e. plugin-id.projects:read + ReqAction *string `json:"reqAction,omitempty"` ReqRole *string `json:"reqRole,omitempty"` ReqSignedIn *bool `json:"reqSignedIn,omitempty"` diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index acb46eec62b..3e98bf13445 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -194,6 +194,7 @@ type Route struct { Path string `json:"path"` Method string `json:"method"` ReqRole org.RoleType `json:"reqRole"` + ReqAction string `json:"reqAction"` URL string `json:"url"` URLParams []URLParam `json:"urlParams"` Headers []Header `json:"headers"` @@ -203,6 +204,10 @@ type Route struct { Body json.RawMessage `json:"body"` } +func (r *Route) RequiresRBACAction() bool { + return r.ReqAction != "" +} + // Header describes an HTTP header that is forwarded with // the proxied request for a plugin route type Header struct {