Plugins: Check installer's permissions include plugins' permissions (#78211)

* Check installer perm

* Failed eval better output

* Switch fetching json data in the repo

* Comment

* Account for feedback

* Mv single_organization config option

* Inline error check

* Starting to replace errors not to have to do the management in two places

* Continue error translation

* Cover ErrChecksumMismatch

* Refactor a bit

* Lint. Tab

* log instead of erroring out

* Nit.

* Revert change on kinds

* revert file again

* Fix tests

* Match core plugin error status code

* Skip permission check for Grafana Admin

* Use errutil templates

* Use errutil templating

* Inline

* Test templating

* revert error changes

* Remove isGrafanaAdmin skip

* Feature toggle check

* Small refactor on hasPluginRequestedPermissions

* Add test

* Imports

* Post install check

* change log messages so that they make sense

* Cover no scope case

* Inline

* Nit.

* Fix test
This commit is contained in:
Gabriel MABILLE 2023-11-24 16:02:44 +01:00 committed by GitHub
parent ab982e7bd3
commit 24a6ee4a91
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 149 additions and 0 deletions

View File

@ -21,10 +21,12 @@ import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/plugindef"
"github.com/grafana/grafana/pkg/plugins/repo"
ac "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/org"
"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol"
"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings"
@ -470,6 +472,13 @@ func (hs *HTTPServer) InstallPlugin(c *contextmodel.ReqContext) response.Respons
return response.Error(http.StatusInternalServerError, "Failed to install plugin", err)
}
if hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagExternalServiceAccounts) {
// This is a non-blocking function that verifies that the installer has
// the permissions that the plugin requests to have on Grafana.
// If we want to make this blocking, the check will have to happen before or during the installation.
hs.hasPluginRequestedPermissions(c, pluginID)
}
return response.JSON(http.StatusOK, []byte{})
}
@ -513,6 +522,47 @@ func (hs *HTTPServer) pluginMarkdown(ctx context.Context, pluginID string, name
return md.Content, nil
}
// hasPluginRequestedPermissions logs if the plugin installer does not have the permissions that the plugin requests to have on Grafana.
func (hs *HTTPServer) hasPluginRequestedPermissions(c *contextmodel.ReqContext, pluginID string) {
plugin, ok := hs.pluginStore.Plugin(c.Req.Context(), pluginID)
if !ok {
hs.log.Debug("plugin has not been installed", "pluginID", pluginID)
return
}
// No registration => Early return
if plugin.JSONData.ExternalServiceRegistration == nil || len(plugin.JSONData.ExternalServiceRegistration.Permissions) == 0 {
hs.log.Debug("plugin did not request permissions on Grafana", "pluginID", pluginID)
return
}
hs.log.Debug("check installer's permissions, plugin wants to register an external service")
evaluator := evalAllPermissions(plugin.JSONData.ExternalServiceRegistration.Permissions)
hasAccess := ac.HasGlobalAccess(hs.AccessControl, hs.accesscontrolService, c)
if hs.Cfg.RBACSingleOrganization {
// In a single organization setup, no need for a global check
hasAccess = ac.HasAccess(hs.AccessControl, c)
}
// Log a warning if the user does not have the plugin requested permissions
if !hasAccess(evaluator) {
hs.log.Warn("Plugin installer has less permission than what the plugin requires.", "Permissions", evaluator.String())
}
}
// evalAllPermissions generates an evaluator with all permissions from the input slice
func evalAllPermissions(ps []plugindef.Permission) ac.Evaluator {
res := []ac.Evaluator{}
for _, p := range ps {
if p.Scope != nil {
res = append(res, ac.EvalPermission(p.Action, *p.Scope))
continue
}
res = append(res, ac.EvalPermission(p.Action))
}
return ac.EvalAll(res...)
}
func mdFilepath(mdFilename string) (string, error) {
fileExt := filepath.Ext(mdFilename)
switch fileExt {

View File

@ -20,14 +20,17 @@ import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/infra/log"
"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/plugins/config"
"github.com/grafana/grafana/pkg/plugins/manager/fakes"
"github.com/grafana/grafana/pkg/plugins/manager/filestore"
"github.com/grafana/grafana/pkg/plugins/manager/registry"
"github.com/grafana/grafana/pkg/plugins/plugindef"
"github.com/grafana/grafana/pkg/plugins/pluginscdn"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/org"
@ -36,7 +39,9 @@ import (
"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings"
"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore"
"github.com/grafana/grafana/pkg/services/updatechecker"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web"
"github.com/grafana/grafana/pkg/web/webtest"
)
@ -637,3 +642,97 @@ func createPlugin(jd plugins.JSONData, class plugins.Class, files plugins.FS) *p
FS: files,
}
}
func TestHTTPServer_hasPluginRequestedPermissions(t *testing.T) {
newStr := func(s string) *string {
return &s
}
pluginReg := pluginstore.Plugin{
JSONData: plugins.JSONData{
ID: "grafana-test-app",
ExternalServiceRegistration: &plugindef.ExternalServiceRegistration{
Permissions: []plugindef.Permission{{Action: ac.ActionUsersRead, Scope: newStr(ac.ScopeUsersAll)}, {Action: ac.ActionUsersCreate}},
},
},
}
tests := []struct {
name string
plugin pluginstore.Plugin
orgID int64
singleOrg bool
permissions map[int64]map[string][]string
warnCount int
}{
{
name: "no warn if plugin has no registration",
plugin: pluginstore.Plugin{
JSONData: plugins.JSONData{
ID: "grafana-test-app",
},
},
warnCount: 0,
},
{
name: "warn if user does not have plugin permissions globally",
plugin: pluginReg,
orgID: 1,
permissions: map[int64]map[string][]string{
1: {ac.ActionUsersRead: {ac.ScopeUsersAll}, ac.ActionUsersCreate: {}},
},
warnCount: 1,
},
{
name: "no warn if user has plugin permissions globally",
plugin: pluginReg,
orgID: 0,
permissions: map[int64]map[string][]string{
0: {ac.ActionUsersRead: {ac.ScopeUsersAll}, ac.ActionUsersCreate: {}},
},
warnCount: 0,
},
{
name: "no warn if user has plugin permissions in single organization",
plugin: pluginReg,
singleOrg: true,
orgID: 1,
permissions: map[int64]map[string][]string{
1: {ac.ActionUsersRead: {ac.ScopeUsersAll}, ac.ActionUsersCreate: {}},
},
warnCount: 0,
},
{
name: "warn if user does not have all plugin permissions",
plugin: pluginReg,
singleOrg: true,
orgID: 1,
permissions: map[int64]map[string][]string{1: {ac.ActionUsersCreate: {}}},
warnCount: 1,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
logger := &logtest.Fake{}
hs := &HTTPServer{}
httpReq, err := http.NewRequest(http.MethodGet, "", nil)
require.NoError(t, err)
hs.Cfg = setting.NewCfg()
hs.Cfg.RBACSingleOrganization = tt.singleOrg
hs.pluginStore = &pluginstore.FakePluginStore{
PluginList: []pluginstore.Plugin{tt.plugin},
}
hs.log = logger
hs.accesscontrolService = actest.FakeService{}
hs.AccessControl = acimpl.ProvideAccessControl(hs.Cfg)
c := &contextmodel.ReqContext{
Context: &web.Context{Req: httpReq},
SignedInUser: &user.SignedInUser{OrgID: tt.orgID, Permissions: tt.permissions},
}
hs.hasPluginRequestedPermissions(c, "grafana-test-app")
assert.Equal(t, tt.warnCount, logger.WarnLogs.Calls)
})
}
}