diff --git a/pkg/api/api.go b/pkg/api/api.go index f3a27c5ceda..63a1a04a72f 100644 --- a/pkg/api/api.go +++ b/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) diff --git a/pkg/middleware/auth.go b/pkg/middleware/auth.go index d0bbe913713..b56c473bb62 100644 --- a/pkg/middleware/auth.go +++ b/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 diff --git a/pkg/middleware/auth_test.go b/pkg/middleware/auth_test.go index 06b20884d72..bc45f07ac9c 100644 --- a/pkg/middleware/auth_test.go +++ b/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: "Found.\n\n", expLocation: "/grafana/"}, + {roleRequired: "", role: org.RoleViewer, expStatus: http.StatusOK, expBody: ""}, + {roleRequired: org.RoleEditor, role: "", expStatus: http.StatusFound, expBody: "Found.\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: "Found.\n\n", + expLocation: "/", + }, + { + name: "An RBAC eval error will result in a redirect", + evalErr: errors.New("eval error"), + expStatus: 302, + expBody: "Found.\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 diff --git a/pkg/services/pluginsintegration/pluginstore/fake.go b/pkg/services/pluginsintegration/pluginstore/fake.go index 7c09fb15c67..ec85fb7b937 100644 --- a/pkg/services/pluginsintegration/pluginstore/fake.go +++ b/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 {