RBAC: Add userLogin filter to the permission search endpoint (#81137)

* RBAC: Search add user login filter

* Switch to a userService resolving instead

* Remove unused error

* Fallback to use the cache

* account for userID filter

* Account for the error

* snake case

* Add test cases

* Add api tests

* Fix return on error

* Re-order imports
This commit is contained in:
Gabriel MABILLE
2024-01-26 09:43:16 +01:00
committed by GitHub
parent 2e352ba4d6
commit 722b78f3e0
11 changed files with 246 additions and 30 deletions
+2 -1
View File
@@ -46,6 +46,7 @@ import (
"github.com/grafana/grafana/pkg/services/team/teamimpl"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/userimpl"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web"
"github.com/grafana/grafana/pkg/web/webtest"
@@ -439,7 +440,7 @@ func setupServer(b testing.TB, sc benchScenario, features featuremgmt.FeatureTog
license := licensingtest.NewFakeLicensing()
license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe()
acSvc := acimpl.ProvideOSSService(sc.cfg, acdb.ProvideService(sc.db), localcache.ProvideService(), features)
acSvc := acimpl.ProvideOSSService(sc.cfg, acdb.ProvideService(sc.db), localcache.ProvideService(), usertest.NewUserServiceFake(), features)
quotaSrv := quotatest.New(false, nil)
@@ -71,7 +71,7 @@ func initializeConflictResolver(cmd *utils.ContextCommandLine, f Formatter, ctx
if err != nil {
return nil, fmt.Errorf("%v: %w", "failed to get feature management service", err)
}
acService, err := acimpl.ProvideService(cfg, s, routing, nil, nil, featMgmt)
acService, err := acimpl.ProvideService(cfg, s, routing, nil, nil, nil, featMgmt)
if err != nil {
return nil, fmt.Errorf("%v: %w", "failed to get access control", err)
}
@@ -31,6 +31,7 @@ import (
"github.com/grafana/grafana/pkg/services/hooks"
"github.com/grafana/grafana/pkg/services/kmsproviders/osskmsproviders"
"github.com/grafana/grafana/pkg/services/licensing"
"github.com/grafana/grafana/pkg/services/org/orgimpl"
"github.com/grafana/grafana/pkg/services/pluginsintegration/config"
"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore"
"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
@@ -40,6 +41,8 @@ import (
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/migrations"
"github.com/grafana/grafana/pkg/services/supportbundles/bundleregistry"
"github.com/grafana/grafana/pkg/services/team/teamimpl"
"github.com/grafana/grafana/pkg/services/user/userimpl"
"github.com/grafana/grafana/pkg/setting"
)
@@ -73,11 +76,23 @@ func apiBuilderServices(cfg *setting.Cfg, pluginID string) (
kvStore := kvstore.ProvideService(sqlStore)
featureToggles := featuremgmt.ProvideToggles(featureManager)
acimplService, err := acimpl.ProvideService(cfg, sqlStore, routeRegisterImpl, cacheService, accessControl, featureToggles)
bundleRegistry := bundleregistry.ProvideService()
quota := quotaimpl.ProvideService(sqlStore, cfg)
orgService, err := orgimpl.ProvideService(sqlStore, cfg, quota)
if err != nil {
return nil, nil, nil, nil, err
}
teamService := teamimpl.ProvideService(sqlStore, cfg)
userService, err := userimpl.ProvideService(sqlStore, orgService, cfg, teamService, cacheService, quota, bundleRegistry)
if err != nil {
return nil, nil, nil, nil, err
}
acimplService, err := acimpl.ProvideService(cfg, sqlStore, routeRegisterImpl, cacheService, accessControl, userService, featureToggles)
if err != nil {
return nil, nil, nil, nil, err
}
bundleRegistry := bundleregistry.ProvideService()
usageStats, err := service.ProvideService(cfg, kvStore, routeRegisterImpl, tracingService, accessControl, acimplService, bundleRegistry)
if err != nil {
return nil, nil, nil, nil, err
@@ -57,6 +57,7 @@ type SearchOptions struct {
ActionPrefix string // Needed for the PoC v1, it's probably going to be removed.
Action string
Scope string
UserLogin string // Login for which to return information, if none is specified information is returned for all users.
UserID int64 // ID for the user for which to return information, if none is specified information is returned for all users.
wildcards Wildcards // private field computed based on the Scope
}
@@ -76,6 +77,19 @@ func (s *SearchOptions) Wildcards() []string {
return s.wildcards
}
func (s *SearchOptions) ResolveUserLogin(ctx context.Context, userSvc user.Service) error {
if s.UserLogin == "" {
return nil
}
// Resolve userLogin -> userID
dbUsr, err := userSvc.GetByLogin(ctx, &user.GetUserByLoginQuery{LoginOrEmail: s.UserLogin})
if err != nil {
return err
}
s.UserID = dbUsr.ID
return nil
}
type SyncUserRolesCommand struct {
UserID int64
// name of roles the user should have
+42 -16
View File
@@ -42,8 +42,8 @@ var SharedWithMeFolderPermission = accesscontrol.Permission{
}
func ProvideService(cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService,
accessControl accesscontrol.AccessControl, features featuremgmt.FeatureToggles) (*Service, error) {
service := ProvideOSSService(cfg, database.ProvideService(db), cache, features)
accessControl accesscontrol.AccessControl, userSvc user.Service, features featuremgmt.FeatureToggles) (*Service, error) {
service := ProvideOSSService(cfg, database.ProvideService(db), cache, userSvc, features)
api.NewAccessControlAPI(routeRegister, accessControl, service, features).RegisterAPIEndpoints()
if err := accesscontrol.DeclareFixedRoles(service, cfg); err != nil {
@@ -63,14 +63,15 @@ func ProvideService(cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegis
return service, nil
}
func ProvideOSSService(cfg *setting.Cfg, store store, cache *localcache.CacheService, features featuremgmt.FeatureToggles) *Service {
func ProvideOSSService(cfg *setting.Cfg, store store, cache *localcache.CacheService, userSvc user.Service, features featuremgmt.FeatureToggles) *Service {
s := &Service{
cfg: cfg,
store: store,
log: log.New("accesscontrol.service"),
cache: cache,
roles: accesscontrol.BuildBasicRoleDefinitions(),
cfg: cfg,
features: features,
log: log.New("accesscontrol.service"),
roles: accesscontrol.BuildBasicRoleDefinitions(),
store: store,
userSvc: userSvc,
}
return s
@@ -88,13 +89,14 @@ type store interface {
// Service is the service implementing role based access control.
type Service struct {
log log.Logger
cfg *setting.Cfg
store store
cache *localcache.CacheService
cfg *setting.Cfg
features featuremgmt.FeatureToggles
log log.Logger
registrations accesscontrol.RegistrationList
roles map[string]*accesscontrol.RoleDTO
features featuremgmt.FeatureToggles
store store
userSvc user.Service
}
func (s *Service) GetUsageStats(_ context.Context) map[string]any {
@@ -243,8 +245,25 @@ func (s *Service) DeclarePluginRoles(ctx context.Context, ID, name string, regs
}
// SearchUsersPermissions returns all users' permissions filtered by action prefixes
func (s *Service) SearchUsersPermissions(ctx context.Context, user identity.Requester,
func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Requester,
options accesscontrol.SearchOptions) (map[int64][]accesscontrol.Permission, error) {
if options.UserLogin != "" {
// Resolve userLogin -> userID
if err := options.ResolveUserLogin(ctx, s.userSvc); err != nil {
return nil, err
}
options.UserLogin = ""
}
if options.UserID > 0 {
// Reroute to the user specific implementation of search permissions
// because it leverages the user permission cache.
userPerms, err := s.SearchUserPermissions(ctx, usr.GetOrgID(), options)
if err != nil {
return nil, err
}
return map[int64][]accesscontrol.Permission{options.UserID: userPerms}, nil
}
timer := prometheus.NewTimer(metrics.MAccessSearchPermissionsSummary)
defer timer.ObserveDuration()
@@ -258,20 +277,20 @@ func (s *Service) SearchUsersPermissions(ctx context.Context, user identity.Requ
}
}
usersRoles, err := s.store.GetUsersBasicRoles(ctx, nil, user.GetOrgID())
usersRoles, err := s.store.GetUsersBasicRoles(ctx, nil, usr.GetOrgID())
if err != nil {
return nil, err
}
// Get managed permissions (DB)
usersPermissions, err := s.store.SearchUsersPermissions(ctx, user.GetOrgID(), options)
usersPermissions, err := s.store.SearchUsersPermissions(ctx, usr.GetOrgID(), options)
if err != nil {
return nil, err
}
// helper to filter out permissions the signed in users cannot see
canView := func() func(userID int64) bool {
siuPermissions := user.GetPermissions()
siuPermissions := usr.GetPermissions()
if len(siuPermissions) == 0 {
return func(_ int64) bool { return false }
}
@@ -329,8 +348,15 @@ func (s *Service) SearchUserPermissions(ctx context.Context, orgID int64, search
timer := prometheus.NewTimer(metrics.MAccessPermissionsSummary)
defer timer.ObserveDuration()
if searchOptions.UserLogin != "" {
// Resolve userLogin -> userID
if err := searchOptions.ResolveUserLogin(ctx, s.userSvc); err != nil {
return nil, err
}
}
if searchOptions.UserID == 0 {
return nil, fmt.Errorf("expected user ID to be specified")
return nil, fmt.Errorf("expected user ID or login to be specified")
}
if permissions, success := s.searchUserPermissionsFromCache(orgID, searchOptions); success {
@@ -19,6 +19,7 @@ import (
"github.com/grafana/grafana/pkg/services/licensing"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/setting"
)
@@ -27,12 +28,14 @@ func setupTestEnv(t testing.TB) *Service {
cfg := setting.NewCfg()
ac := &Service{
cache: localcache.ProvideService(),
cfg: cfg,
features: featuremgmt.WithFeatures(),
log: log.New("accesscontrol"),
registrations: accesscontrol.RegistrationList{},
store: database.ProvideService(db.InitTestDB(t)),
roles: accesscontrol.BuildBasicRoleDefinitions(),
features: featuremgmt.WithFeatures(),
store: database.ProvideService(db.InitTestDB(t)),
userSvc: usertest.NewUserServiceFake(),
}
require.NoError(t, ac.RegisterFixedRoles(context.Background()))
return ac
@@ -57,6 +60,7 @@ func TestUsageMetrics(t *testing.T) {
cfg,
database.ProvideService(db.InitTestDB(t)),
localcache.ProvideService(),
usertest.NewUserServiceFake(),
featuremgmt.WithFeatures(),
)
assert.Equal(t, tt.expectedValue, s.GetUsageStats(context.Background())["stats.oss.accesscontrol.enabled.count"])
@@ -525,11 +529,77 @@ func TestService_SearchUsersPermissions(t *testing.T) {
},
},
},
{
// This test is not exactly representative as normally the store would return
// only the user's basic roles and the user's stored permissions
name: "check userID filter works correctly",
siuPermissions: listAllPerms,
searchOption: accesscontrol.SearchOptions{UserID: 1},
ramRoles: map[string]*accesscontrol.RoleDTO{
string(roletype.RoleEditor): {Permissions: []accesscontrol.Permission{
{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
}},
string(roletype.RoleAdmin): {Permissions: []accesscontrol.Permission{
{Action: accesscontrol.ActionTeamsWrite, Scope: "teams:*"},
}},
accesscontrol.RoleGrafanaAdmin: {Permissions: []accesscontrol.Permission{
{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:*"},
}},
},
storedPerms: map[int64][]accesscontrol.Permission{
1: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}},
2: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"},
{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:id:1"}},
},
storedRoles: map[int64][]string{
1: {string(roletype.RoleEditor)},
2: {string(roletype.RoleAdmin), accesscontrol.RoleGrafanaAdmin},
},
want: map[int64][]accesscontrol.Permission{
1: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}, {Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"}},
},
},
{
// This test is not exactly representative as normally the store would return
// only the user's basic roles and the user's stored permissions
name: "check userLogin filter works correctly",
siuPermissions: listAllPerms,
searchOption: accesscontrol.SearchOptions{UserLogin: "testUser"},
ramRoles: map[string]*accesscontrol.RoleDTO{
string(roletype.RoleEditor): {Permissions: []accesscontrol.Permission{
{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
}},
string(roletype.RoleAdmin): {Permissions: []accesscontrol.Permission{
{Action: accesscontrol.ActionTeamsWrite, Scope: "teams:*"},
}},
accesscontrol.RoleGrafanaAdmin: {Permissions: []accesscontrol.Permission{
{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:*"},
}},
},
storedPerms: map[int64][]accesscontrol.Permission{
1: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}},
2: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"},
{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:id:1"}},
},
storedRoles: map[int64][]string{
1: {string(roletype.RoleEditor)},
2: {string(roletype.RoleAdmin), accesscontrol.RoleGrafanaAdmin},
},
want: map[int64][]accesscontrol.Permission{
2: {{Action: accesscontrol.ActionTeamsWrite, Scope: "teams:*"},
{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"},
{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:id:1"},
{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:*"}},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ac := setupTestEnv(t)
// Resolve user login to id 2
ac.userSvc = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 2}}
ac.roles = tt.ramRoles
ac.store = actest.FakeStore{
ExpectedUsersPermissions: tt.storedPerms,
+18 -4
View File
@@ -69,17 +69,31 @@ func (api *AccessControlAPI) getUserPermissions(c *contextmodel.ReqContext) resp
return response.JSON(http.StatusOK, ac.GroupScopesByAction(permissions))
}
// GET /api/access-control/users/permissions
// GET /api/access-control/users/permissions/search
func (api *AccessControlAPI) searchUsersPermissions(c *contextmodel.ReqContext) response.Response {
userIDString := c.Query("userId")
userID, err := strconv.ParseInt(userIDString, 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "user ID is invalid", err)
}
searchOptions := ac.SearchOptions{
UserLogin: c.Query("userLogin"),
ActionPrefix: c.Query("actionPrefix"),
Action: c.Query("action"),
Scope: c.Query("scope"),
}
searchOptions.UserID = userID
// Validate inputs
if (searchOptions.ActionPrefix != "") == (searchOptions.Action != "") {
return response.JSON(http.StatusBadRequest, "provide one of 'action' or 'actionPrefix'")
if (searchOptions.ActionPrefix != "") && (searchOptions.Action != "") {
return response.JSON(http.StatusBadRequest, "'action' and 'actionPrefix' are mutually exclusive")
}
if (searchOptions.UserLogin != "") && (searchOptions.UserID > 0) {
return response.JSON(http.StatusBadRequest, "'userId' and 'userLogin' are mutually exclusive")
}
if searchOptions.UserID <= 0 && searchOptions.UserLogin == "" &&
searchOptions.ActionPrefix == "" && searchOptions.Action == "" {
return response.JSON(http.StatusBadRequest, "at least one search option must be provided")
}
// Compute metadata
@@ -101,7 +115,7 @@ func (api *AccessControlAPI) searchUserPermissions(c *contextmodel.ReqContext) r
userIDString := web.Params(c.Req)[":userID"]
userID, err := strconv.ParseInt(userIDString, 10, 64)
if err != nil {
response.Error(http.StatusBadRequest, "user ID is invalid", err)
return response.Error(http.StatusBadRequest, "user ID is invalid", err)
}
searchOptions := ac.SearchOptions{
@@ -118,3 +118,77 @@ func TestAPI_getUserPermissions(t *testing.T) {
})
}
}
func TestAccessControlAPI_searchUsersPermissions(t *testing.T) {
type testCase struct {
desc string
permissions map[int64][]ac.Permission
filters string
expectedOutput map[int64]map[string][]string
expectedCode int
}
tests := []testCase{
{
desc: "Should reject if no filter is provided",
expectedCode: http.StatusBadRequest,
},
{
desc: "Should reject if conflicting action filters are provided",
filters: "?actionPrefix=grafana-test-app&action=grafana-test-app.projects:read",
expectedCode: http.StatusBadRequest,
},
{
desc: "Should reject if conflicting user filters are provided",
filters: "?userLogin=admin&userId=2",
expectedCode: http.StatusBadRequest,
},
{
desc: "Should reject if invalid userID is provided",
filters: "?userId=invalid",
expectedCode: http.StatusBadRequest,
},
{
desc: "Should work with valid filter provided",
filters: "?userId=2",
permissions: map[int64][]ac.Permission{2: {{Action: "users:read", Scope: "users:*"}}},
expectedCode: http.StatusOK,
expectedOutput: map[int64]map[string][]string{2: {"users:read": {"users:*"}}},
},
{
desc: "Should reduce permissions",
filters: "?userId=2",
permissions: map[int64][]ac.Permission{2: {{Action: "users:read", Scope: "users:id:1"}, {Action: "users:read", Scope: "users:*"}}},
expectedCode: http.StatusOK,
expectedOutput: map[int64]map[string][]string{2: {"users:read": {"users:*"}}},
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
acSvc := actest.FakeService{ExpectedUsersPermissions: tt.permissions}
accessControl := actest.FakeAccessControl{ExpectedEvaluate: true} // Always allow access to the endpoint
api := NewAccessControlAPI(routing.NewRouteRegister(), accessControl, acSvc, featuremgmt.WithFeatures(featuremgmt.FlagAccessControlOnCall))
api.RegisterAPIEndpoints()
server := webtest.NewServer(t, api.RouteRegister)
url := "/api/access-control/users/permissions/search" + tt.filters
req := server.NewGetRequest(url)
webtest.RequestWithSignedInUser(req, &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{},
})
res, err := server.Send(req)
defer func() { require.NoError(t, res.Body.Close()) }()
require.NoError(t, err)
require.Equal(t, tt.expectedCode, res.StatusCode)
if tt.expectedCode == http.StatusOK {
var output map[int64]map[string][]string
err := json.NewDecoder(res.Body).Decode(&output)
require.NoError(t, err)
require.Equal(t, tt.expectedOutput, output)
}
})
}
}
@@ -83,7 +83,7 @@ func setupTestEnv(t *testing.T) *TestEnv {
cache: localcache.New(cacheExpirationTime, cacheCleanupInterval),
cfg: cfg,
accessControl: acimpl.ProvideAccessControl(cfg),
acService: acimpl.ProvideOSSService(cfg, env.AcStore, localcache.New(0, 0), fmgt),
acService: acimpl.ProvideOSSService(cfg, env.AcStore, localcache.New(0, 0), env.UserService, fmgt),
memstore: storage.NewMemoryStore(),
sqlstore: env.OAuthStore,
logger: log.New("oauthserver.test"),
@@ -54,8 +54,6 @@ func NewTestMigrationStore(t testing.TB, sqlStore *sqlstore.SQLStore, cfg *setti
quotaService := &quotatest.FakeQuotaService{}
ac := acimpl.ProvideAccessControl(cfg)
routeRegister := routing.ProvideRegister()
acSvc, err := acimpl.ProvideService(cfg, sqlStore, routing.ProvideRegister(), cache, ac, features)
require.NoError(t, err)
license := licensingtest.NewFakeLicensing()
license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe()
@@ -65,6 +63,9 @@ func NewTestMigrationStore(t testing.TB, sqlStore *sqlstore.SQLStore, cfg *setti
userSvc, err := userimpl.ProvideService(sqlStore, orgService, cfg, teamSvc, cache, quotaService, bundleregistry.ProvideService())
require.NoError(t, err)
acSvc, err := acimpl.ProvideService(cfg, sqlStore, routing.ProvideRegister(), cache, ac, userSvc, features)
require.NoError(t, err)
dashboardStore, err := database.ProvideDashboardStore(sqlStore, sqlStore.Cfg, features, tagimpl.ProvideService(sqlStore), quotaService)
require.NoError(t, err)
folderService := folderimpl.ProvideService(ac, bus, cfg, dashboardStore, folderStore, sqlStore, features, nil)
@@ -20,6 +20,7 @@ import (
"github.com/grafana/grafana/pkg/services/secrets/kvstore"
sa "github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/setting"
)
@@ -43,7 +44,7 @@ func setupTestEnv(t *testing.T) *TestEnv {
}
logger := log.New("extsvcaccounts.test")
env.S = &ExtSvcAccountsService{
acSvc: acimpl.ProvideOSSService(cfg, env.AcStore, localcache.New(0, 0), fmgt),
acSvc: acimpl.ProvideOSSService(cfg, env.AcStore, localcache.New(0, 0), usertest.NewUserServiceFake(), fmgt),
features: fmgt,
logger: logger,
metrics: newMetrics(nil, env.SaSvc, logger),