Plugins: Add backend check for app page role access (grafana#78269)

* add backend check for roles

* tidy

* fix tests

* incorporate rbac

* fix linter

* apply PR feedback

* add tests

* fix logic

* add comment

* apply PR feedback

Author: wbrowne

pkg/api/api.go

@@ -61,6 +61,7 @@ func (hs *HTTPServer) registerRoutes() {
 	reqGrafanaAdmin := middleware.ReqGrafanaAdmin
 	reqEditorRole := middleware.ReqEditorRole
 	reqOrgAdmin := middleware.ReqOrgAdmin
+	reqRoleForAppRoute := middleware.RoleAppPluginAuth(hs.AccessControl, hs.pluginStore, hs.Features, hs.log)
 	reqSnapshotPublicModeOrSignedIn := middleware.SnapshotPublicModeOrSignedIn(hs.Cfg)
 	redirectFromLegacyPanelEditURL := middleware.RedirectFromLegacyPanelEditURL(hs.Cfg)
 	authorize := ac.Middleware(hs.AccessControl)
@@ -140,8 +141,8 @@ func (hs *HTTPServer) registerRoutes() {
 
 	// App Root Page
 	appPluginIDScope := pluginaccesscontrol.ScopeProvider.GetResourceScope(ac.Parameter(":id"))
-	r.Get("/a/:id/*", authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, appPluginIDScope)), hs.Index)
-	r.Get("/a/:id", authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, appPluginIDScope)), hs.Index)
+	r.Get("/a/:id/*", authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, appPluginIDScope)), reqSignedIn, reqRoleForAppRoute, hs.Index)
+	r.Get("/a/:id", authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, appPluginIDScope)), reqSignedIn, reqRoleForAppRoute, hs.Index)
 
 	r.Get("/d/:uid/:slug", reqSignedIn, redirectFromLegacyPanelEditURL, hs.Index)
 	r.Get("/d/:uid", reqSignedIn, redirectFromLegacyPanelEditURL, hs.Index)

pkg/middleware/auth.go

@@ -4,17 +4,21 @@ import (
 	"errors"
 	"net/http"
 	"net/url"
+	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
 
+	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/middleware/cookies"
 	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/auth"
 	"github.com/grafana/grafana/pkg/services/authn"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol"
+	"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/web"
 )
@@ -97,6 +101,54 @@ func CanAdminPlugins(cfg *setting.Cfg, accessControl ac.AccessControl) func(c *c
 	}
 }
 
+func RoleAppPluginAuth(accessControl ac.AccessControl, ps pluginstore.Store, features featuremgmt.FeatureToggles,
+	logger log.Logger) func(c *contextmodel.ReqContext) {
+	return func(c *contextmodel.ReqContext) {
+		pluginID := web.Params(c.Req)[":id"]
+		p, exists := ps.Plugin(c.Req.Context(), pluginID)
+		if !exists {
+			// The frontend will handle app not found appropriately
+			return
+		}
+
+		permitted := true
+		path := normalizeIncludePath(c.Req.URL.Path)
+		hasAccess := ac.HasAccess(accessControl, c)
+		for _, i := range p.Includes {
+			if i.Type != "page" {
+				continue
+			}
+
+			u, err := url.Parse(i.Path)
+			if err != nil {
+				logger.Error("failed to parse include path", "pluginId", pluginID, "include", i.Name, "err", err)
+				continue
+			}
+
+			if normalizeIncludePath(u.Path) == path {
+				useRBAC := features.IsEnabledGlobally(featuremgmt.FlagAccessControlOnCall) && i.RequiresRBACAction()
+				if useRBAC && !hasAccess(ac.EvalPermission(i.Action)) {
+					logger.Debug("Plugin include is covered by RBAC, user doesn't have access", "plugin", pluginID, "include", i.Name)
+					permitted = false
+					break
+				} else if !useRBAC && !c.HasUserRole(i.Role) {
+					permitted = false
+					break
+				}
+			}
+		}
+
+		if !permitted {
+			accessForbidden(c)
+			return
+		}
+	}
+}
+
+func normalizeIncludePath(p string) string {
+	return strings.TrimPrefix(filepath.Clean(p), "/")
+}
+
 func RoleAuth(roles ...org.RoleType) web.Handler {
 	return func(c *contextmodel.ReqContext) {
 		ok := false

pkg/middleware/auth_test.go

@@ -10,12 +10,17 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/grafana/grafana/pkg/infra/log/logtest"
 	"github.com/grafana/grafana/pkg/infra/tracing"
+	"github.com/grafana/grafana/pkg/plugins"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
 	"github.com/grafana/grafana/pkg/services/authn"
 	"github.com/grafana/grafana/pkg/services/authn/authntest"
 	"github.com/grafana/grafana/pkg/services/contexthandler"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/services/org"
+	"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/web"
 )
@@ -150,6 +155,185 @@ func TestAuth_Middleware(t *testing.T) {
 	}
 }
 
+func TestRoleAppPluginAuth(t *testing.T) {
+	t.Run("Verify user's role when requesting app route which requires role", func(t *testing.T) {
+		appSubURL := setting.AppSubUrl
+		setting.AppSubUrl = "/grafana/"
+		t.Cleanup(func() {
+			setting.AppSubUrl = appSubURL
+		})
+
+		tcs := []struct {
+			roleRequired org.RoleType
+			role         org.RoleType
+			expStatus    int
+			expBody      string
+			expLocation  string
+		}{
+			{roleRequired: org.RoleViewer, role: org.RoleAdmin, expStatus: http.StatusOK, expBody: ""},
+			{roleRequired: org.RoleAdmin, role: org.RoleAdmin, expStatus: http.StatusOK, expBody: ""},
+			{roleRequired: org.RoleAdmin, role: org.RoleViewer, expStatus: http.StatusFound, expBody: "<a href=\"/grafana/\">Found</a>.\n\n", expLocation: "/grafana/"},
+			{roleRequired: "", role: org.RoleViewer, expStatus: http.StatusOK, expBody: ""},
+			{roleRequired: org.RoleEditor, role: "", expStatus: http.StatusFound, expBody: "<a href=\"/grafana/\">Found</a>.\n\n", expLocation: "/grafana/"},
+		}
+
+		const path = "/a/test-app/test"
+		for i, tc := range tcs {
+			t.Run(fmt.Sprintf("testcase %d", i), func(t *testing.T) {
+				ps := pluginstore.NewFakePluginStore(pluginstore.Plugin{
+					JSONData: plugins.JSONData{
+						ID: "test-app",
+						Includes: []*plugins.Includes{
+							{
+								Type: "page",
+								Role: tc.roleRequired,
+								Path: path,
+							},
+						},
+					},
+				})
+
+				middlewareScenario(t, t.Name(), func(t *testing.T, sc *scenarioContext) {
+					sc.withIdentity(&authn.Identity{
+						OrgRoles: map[int64]org.RoleType{
+							0: tc.role,
+						},
+					})
+					features := featuremgmt.WithFeatures()
+					logger := &logtest.Fake{}
+					ac := &actest.FakeAccessControl{}
+
+					sc.m.Get("/a/:id/*", RoleAppPluginAuth(ac, ps, features, logger), func(c *contextmodel.ReqContext) {
+						c.JSON(http.StatusOK, map[string]interface{}{})
+					})
+					sc.fakeReq("GET", path).exec()
+					assert.Equal(t, tc.expStatus, sc.resp.Code)
+					assert.Equal(t, tc.expBody, sc.resp.Body.String())
+					assert.Equal(t, tc.expLocation, sc.resp.Header().Get("Location"))
+				})
+			})
+		}
+	})
+
+	// We return success in this case because the frontend takes care of rendering the 404 page
+	middlewareScenario(t, "Plugin is not found returns success", func(t *testing.T, sc *scenarioContext) {
+		sc.withIdentity(&authn.Identity{
+			OrgRoles: map[int64]org.RoleType{
+				0: org.RoleViewer,
+			},
+		})
+		features := featuremgmt.WithFeatures()
+		logger := &logtest.Fake{}
+		ac := &actest.FakeAccessControl{}
+		sc.m.Get("/a/:id/*", RoleAppPluginAuth(ac, &pluginstore.FakePluginStore{}, features, logger), func(c *contextmodel.ReqContext) {
+			c.JSON(http.StatusOK, map[string]interface{}{})
+		})
+		sc.fakeReq("GET", "/a/test-app/test").exec()
+		assert.Equal(t, 200, sc.resp.Code)
+		assert.Equal(t, "", sc.resp.Body.String())
+	})
+
+	// We return success in this case because the frontend takes care of rendering the right page based on its router
+	middlewareScenario(t, "Plugin page not found returns success", func(t *testing.T, sc *scenarioContext) {
+		sc.withIdentity(&authn.Identity{
+			OrgRoles: map[int64]org.RoleType{
+				0: org.RoleViewer,
+			},
+		})
+		features := featuremgmt.WithFeatures()
+		logger := &logtest.Fake{}
+		ac := &actest.FakeAccessControl{}
+		sc.m.Get("/a/:id/*", RoleAppPluginAuth(ac, pluginstore.NewFakePluginStore(pluginstore.Plugin{
+			JSONData: plugins.JSONData{
+				ID: "test-app",
+				Includes: []*plugins.Includes{
+					{
+						Type: "page",
+						Role: org.RoleViewer,
+						Path: "/a/test-app/test",
+					},
+				},
+			},
+		}), features, logger), func(c *contextmodel.ReqContext) {
+			c.JSON(http.StatusOK, map[string]interface{}{})
+		})
+		sc.fakeReq("GET", "/a/test-app/notExistingPath").exec()
+		assert.Equal(t, 200, sc.resp.Code)
+		assert.Equal(t, "", sc.resp.Body.String())
+	})
+
+	t.Run("Plugin include with RBAC", func(t *testing.T) {
+		tcs := []struct {
+			name        string
+			evalResult  bool
+			evalErr     error
+			expStatus   int
+			expBody     string
+			expLocation string
+		}{
+			{
+				name:        "Unsuccessful RBAC eval will result in a redirect",
+				evalResult:  false,
+				expStatus:   302,
+				expBody:     "<a href=\"/\">Found</a>.\n\n",
+				expLocation: "/",
+			},
+			{
+				name:        "An RBAC eval error will result in a redirect",
+				evalErr:     errors.New("eval error"),
+				expStatus:   302,
+				expBody:     "<a href=\"/\">Found</a>.\n\n",
+				expLocation: "/",
+			},
+			{
+				name:        "Successful RBAC eval will result in a successful request",
+				evalResult:  true,
+				expStatus:   200,
+				expBody:     "",
+				expLocation: "",
+			},
+		}
+
+		for _, tc := range tcs {
+			middlewareScenario(t, "Plugin include with RBAC", func(t *testing.T, sc *scenarioContext) {
+				sc.withIdentity(&authn.Identity{
+					OrgRoles: map[int64]org.RoleType{
+						0: org.RoleViewer,
+					},
+				})
+				logger := &logtest.Fake{}
+				features := featuremgmt.WithFeatures(featuremgmt.FlagAccessControlOnCall)
+				ac := &actest.FakeAccessControl{
+					ExpectedEvaluate: tc.evalResult,
+					ExpectedErr:      tc.evalErr,
+				}
+				path := "/a/test-app/test"
+				ps := pluginstore.NewFakePluginStore(pluginstore.Plugin{
+					JSONData: plugins.JSONData{
+						ID: "test-app",
+						Includes: []*plugins.Includes{
+							{
+								Type:   "page",
+								Role:   org.RoleViewer,
+								Path:   path,
+								Action: "test-app.test:read",
+							},
+						},
+					},
+				})
+
+				sc.m.Get("/a/:id/*", RoleAppPluginAuth(ac, ps, features, logger), func(c *contextmodel.ReqContext) {
+					c.JSON(http.StatusOK, map[string]interface{}{})
+				})
+				sc.fakeReq("GET", path).exec()
+				assert.Equal(t, tc.expStatus, sc.resp.Code)
+				assert.Equal(t, tc.expBody, sc.resp.Body.String())
+				assert.Equal(t, tc.expLocation, sc.resp.Header().Get("Location"))
+			})
+		}
+	})
+}
+
 func TestRemoveForceLoginparams(t *testing.T) {
 	tcs := []struct {
 		inp string

pkg/services/pluginsintegration/pluginstore/fake.go

@@ -10,6 +10,12 @@ type FakePluginStore struct {
 	PluginList []Plugin
 }
 
+func NewFakePluginStore(ps ...Plugin) *FakePluginStore {
+	return &FakePluginStore{
+		PluginList: ps,
+	}
+}
+
 func (pr *FakePluginStore) Plugin(_ context.Context, pluginID string) (Plugin, bool) {
 	for _, v := range pr.PluginList {
 		if v.ID == pluginID {