RBAC: Allow plugins to use scoped actions (#90946)

Co-authored-by: gamab <gabriel.mabille@grafana.com>
2025-02-25 18:55:37 -06:00 · 2024-07-25 09:22:42 -05:00
parent 95000f9fc8
commit c326d865c5
8 changed files with 243 additions and 83 deletions
--- a/pkg/api/pluginproxy/ds_proxy.go
+++ b/pkg/api/pluginproxy/ds_proxy.go
@@ -19,11 +19,11 @@ import (
 	glog "github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/plugins"
-	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/oauthtoken"
+	pluginac "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/util/proxyutil"
@@ -341,12 +341,12 @@ func (proxy *DataSourceProxy) hasAccessToRoute(route *plugins.Route) bool {
 	ctxLogger := logger.FromContext(proxy.ctx.Req.Context())
 	useRBAC := proxy.features.IsEnabled(proxy.ctx.Req.Context(), featuremgmt.FlagAccessControlOnCall) && route.ReqAction != ""
 	if useRBAC {
-		routeEval := accesscontrol.EvalPermission(route.ReqAction)
-		ok := routeEval.Evaluate(proxy.ctx.GetPermissions())
-		if !ok {
+		routeEval := pluginac.GetDataSourceRouteEvaluator(proxy.ds.UID, route.ReqAction)
+		hasAccess := routeEval.Evaluate(proxy.ctx.GetPermissions())
+		if !hasAccess {
 			ctxLogger.Debug("plugin route is covered by RBAC, user doesn't have access", "route", proxy.ctx.Req.URL.Path, "action", route.ReqAction, "path", route.Path, "method", route.Method)
 		}
-		return ok
+		return hasAccess
 	}
 	if route.ReqRole.IsValid() {
 		if hasUserRole := proxy.ctx.HasUserRole(route.ReqRole); !hasUserRole {
--- a/pkg/api/pluginproxy/ds_proxy_test.go
+++ b/pkg/api/pluginproxy/ds_proxy_test.go
@@ -108,9 +108,18 @@ func TestDataSourceProxy_routeRule(t *testing.T) {
 				Path: "mypath",
 				URL:  "https://example.com/api/v1/",
 			},
+			{
+				Path:      "api/rbac-home",
+				ReqAction: "datasources:read",
+			},
+			{
+				Path:      "api/rbac-restricted",
+				ReqAction: "test-app.settings:read",
+			},
 		}

 		ds := &datasources.DataSource{
+			UID: "dsUID",
 			JsonData: simplejson.NewFromAny(map[string]any{
 				"clientId":   "asd",
 				"dynamicUrl": "https://dynamic.grafana.com",
@@ -249,6 +258,51 @@ func TestDataSourceProxy_routeRule(t *testing.T) {
 				require.NoError(t, err)
 			})
 		})
+
+		t.Run("plugin route with RBAC protection user is allowed", func(t *testing.T) {
+			ctx, _ := setUp()
+			ctx.SignedInUser.OrgID = int64(1)
+			ctx.SignedInUser.OrgRole = identity.RoleNone
+			ctx.SignedInUser.Permissions = map[int64]map[string][]string{1: {"test-app.settings:read": nil}}
+			proxy, err := setupDSProxyTest(t, ctx, ds, routes, "api/rbac-restricted")
+			require.NoError(t, err)
+			err = proxy.validateRequest()
+			require.NoError(t, err)
+		})
+
+		t.Run("plugin route with RBAC protection user is not allowed", func(t *testing.T) {
+			ctx, _ := setUp()
+			ctx.SignedInUser.OrgID = int64(1)
+			ctx.SignedInUser.OrgRole = identity.RoleNone
+			ctx.SignedInUser.Permissions = map[int64]map[string][]string{1: {"test-app:read": nil}}
+			proxy, err := setupDSProxyTest(t, ctx, ds, routes, "api/rbac-restricted")
+			require.NoError(t, err)
+			err = proxy.validateRequest()
+			require.Error(t, err)
+		})
+
+		t.Run("plugin route with dynamic RBAC protection user is allowed", func(t *testing.T) {
+			ctx, _ := setUp()
+			ctx.SignedInUser.OrgID = int64(1)
+			ctx.SignedInUser.OrgRole = identity.RoleNone
+			ctx.SignedInUser.Permissions = map[int64]map[string][]string{1: {"datasources:read": {"datasources:uid:dsUID"}}}
+			proxy, err := setupDSProxyTest(t, ctx, ds, routes, "api/rbac-home")
+			require.NoError(t, err)
+			err = proxy.validateRequest()
+			require.NoError(t, err)
+		})
+
+		t.Run("plugin route with dynamic RBAC protection user is not allowed", func(t *testing.T) {
+			ctx, _ := setUp()
+			ctx.SignedInUser.OrgID = int64(1)
+			ctx.SignedInUser.OrgRole = identity.RoleNone
+			// Has access but to another app
+			ctx.SignedInUser.Permissions = map[int64]map[string][]string{1: {"datasources:read": {"datasources:uid:notTheDsUID"}}}
+			proxy, err := setupDSProxyTest(t, ctx, ds, routes, "api/rbac-home")
+			require.NoError(t, err)
+			err = proxy.validateRequest()
+			require.Error(t, err)
+		})
 	})

 	t.Run("Plugin with multiple routes for token auth", func(t *testing.T) {
@@ -1021,7 +1075,7 @@ func setupDSProxyTest(t *testing.T, ctx *contextmodel.ReqContext, ds *datasource
 	cfg := setting.NewCfg()
 	secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore())
 	secretsStore := secretskvs.NewSQLSecretsKVStore(dbtest.NewFakeDB(), secretsService, log.NewNopLogger())
-	features := featuremgmt.WithFeatures()
+	features := featuremgmt.WithFeatures(featuremgmt.FlagAccessControlOnCall)
 	dsService, err := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, features, acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()),
 		&actest.FakePermissionsService{}, quotatest.New(false, nil), &pluginstore.FakePluginStore{}, &pluginfakes.FakePluginClient{},
 		plugincontext.ProvideBaseService(cfg, pluginconfig.NewFakePluginRequestConfigProvider()))
--- a/pkg/api/pluginproxy/pluginproxy.go
+++ b/pkg/api/pluginproxy/pluginproxy.go
@@ -15,6 +15,7 @@ import (
 	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	pluginac "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol"
 	"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings"
 	"github.com/grafana/grafana/pkg/services/secrets"
 	"github.com/grafana/grafana/pkg/setting"
@@ -130,7 +131,8 @@ func (proxy *PluginProxy) HandleRequest() {
 func (proxy *PluginProxy) hasAccessToRoute(route *plugins.Route) bool {
 	useRBAC := proxy.features.IsEnabled(proxy.ctx.Req.Context(), featuremgmt.FlagAccessControlOnCall) && route.ReqAction != ""
 	if useRBAC {
-		hasAccess := ac.HasAccess(proxy.accessControl, proxy.ctx)(ac.EvalPermission(route.ReqAction))
+		routeEval := pluginac.GetPluginRouteEvaluator(proxy.ps.PluginID, route.ReqAction)
+		hasAccess := ac.HasAccess(proxy.accessControl, proxy.ctx)(routeEval)
 		if !hasAccess {
 			proxy.ctx.Logger.Debug("plugin route is covered by RBAC, user doesn't have access", "route", proxy.ctx.Req.URL.Path)
 		}
--- a/pkg/api/pluginproxy/pluginproxy_test.go
+++ b/pkg/api/pluginproxy/pluginproxy_test.go
@@ -455,7 +455,13 @@ func TestPluginProxyRoutesAccessControl(t *testing.T) {
 			Path:      "projects",
 			Method:    "GET",
 			URL:       "http://localhost/api/projects",
-			ReqAction: "plugin-id.projects:read", // Protected by RBAC action
+			ReqAction: "test-app.projects:read", // Protected by RBAC action
+		},
+		{
+			Path:      "home",
+			Method:    "GET",
+			URL:       "http://localhost/api/home",
+			ReqAction: "plugins.app:access", // Protected by RBAC action with plugin scope
 		},
 	}

@@ -480,7 +486,7 @@ func TestPluginProxyRoutesAccessControl(t *testing.T) {
 		},
 		{
 			proxyPath:       "/projects",
-			usrPerms:        map[string][]string{"plugin-id.projects:read": {}},
+			usrPerms:        map[string][]string{"test-app.projects:read": {}},
 			expectedURLPath: "/api/projects",
 			expectedStatus:  http.StatusOK,
 		},
@@ -490,6 +496,18 @@ func TestPluginProxyRoutesAccessControl(t *testing.T) {
 			expectedURLPath: "/api/projects",
 			expectedStatus:  http.StatusForbidden,
 		},
+		{
+			proxyPath:       "/home",
+			usrPerms:        map[string][]string{"plugins.app:access": {"plugins:id:not-the-test-app"}},
+			expectedURLPath: "/api/home",
+			expectedStatus:  http.StatusForbidden,
+		},
+		{
+			proxyPath:       "/home",
+			usrPerms:        map[string][]string{"plugins.app:access": {"plugins:id:test-app"}},
+			expectedURLPath: "/api/home",
+			expectedStatus:  http.StatusOK,
+		},
 	}

 	for _, tc := range tcs {
@@ -534,6 +552,7 @@ func TestPluginProxyRoutesAccessControl(t *testing.T) {
 				},
 			}
 			ps := &pluginsettings.DTO{
+				PluginID:       "test-app",
 				SecureJSONData: map[string][]byte{},
 			}
 			cfg := &setting.Cfg{}