IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

RBAC: Allow passing in user UID when searching for user's permissions (#97125)

Browse Source 
* allow passing in user UID instead of ID when searching for user's permissions

* fix tests
This commit is contained in:
Ieva 2024-11-28 16:36:26 +00:00 committed by GitHub
parent 2fdac80488
commit cc0ec349a4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 109 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
pkg/services/accesscontrol/accesscontrol.go
View File

@ -3,10 +3,8 @@ package accesscontrol
import (
"context"
"fmt"
"strconv"
"strings"
"github.com/grafana/authlib/claims"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
@ -90,7 +88,7 @@ type SearchOptions struct {
Action string
ActionSets []string
Scope string
TypedID string // ID of the identity (ex: user:3, service-account:4)
UserID int64
wildcards Wildcards // private field computed based on the Scope
RolePrefixes []string
}
@ -110,19 +108,6 @@ func (s *SearchOptions) Wildcards() []string {
return s.wildcards
}
func (s *SearchOptions) ComputeUserID() (int64, error) {
typ, id, err := claims.ParseTypeID(s.TypedID)
if err != nil {
return 0, err
}
if !claims.IsIdentityType(typ, claims.TypeUser, claims.TypeServiceAccount) {
return 0, fmt.Errorf("invalid type: %s", typ)
}
return strconv.ParseInt(id, 10, 64)
}
type SyncUserRolesCommand struct {
UserID int64
// name of roles the user should have

38
pkg/services/accesscontrol/acimpl/service.go
View File

@ -53,7 +53,7 @@ var OSSRolesPrefixes = []string{accesscontrol.ManagedRolePrefix, accesscontrol.E
func ProvideService(
cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService,
accessControl accesscontrol.AccessControl, actionResolver accesscontrol.ActionResolver,
accessControl accesscontrol.AccessControl, userService user.Service, actionResolver accesscontrol.ActionResolver,
features featuremgmt.FeatureToggles, tracer tracing.Tracer, zclient zanzana.Client, permRegistry permreg.PermissionRegistry,
lock *serverlock.ServerLockService,
) (*Service, error) {
@ -70,7 +70,7 @@ func ProvideService(
lock,
)
api.NewAccessControlAPI(routeRegister, accessControl, service, features).RegisterAPIEndpoints()
api.NewAccessControlAPI(routeRegister, accessControl, service, userService, features).RegisterAPIEndpoints()
if err := accesscontrol.DeclareFixedRoles(service, cfg); err != nil {
return nil, err
}
@ -526,20 +526,14 @@ func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Reque
// Limit roles to available in OSS
options.RolePrefixes = OSSRolesPrefixes
if options.TypedID != "" {
userID, err := options.ComputeUserID()
if err != nil {
s.log.Error("Failed to resolve user ID", "error", err)
return nil, err
}
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{userID: userPerms}, nil
return map[int64][]accesscontrol.Permission{options.UserID: userPerms}, nil
}
timer := prometheus.NewTimer(metrics.MAccessSearchPermissionsSummary)
@ -641,7 +635,7 @@ func (s *Service) SearchUserPermissions(ctx context.Context, orgID int64, search
timer := prometheus.NewTimer(metrics.MAccessPermissionsSummary)
defer timer.ObserveDuration()
if searchOptions.TypedID == "" {
if searchOptions.UserID <= 0 {
return nil, fmt.Errorf("expected namespaced ID to be specified")
}
@ -655,20 +649,15 @@ func (s *Service) searchUserPermissions(ctx context.Context, orgID int64, search
ctx, span := tracer.Start(ctx, "accesscontrol.acimpl.searchUserPermissions")
defer span.End()
userID, err := searchOptions.ComputeUserID()
if err != nil {
return nil, err
}
// Get permissions for user's basic roles from RAM
roleList, err := s.store.GetUsersBasicRoles(ctx, []int64{userID}, orgID)
roleList, err := s.store.GetUsersBasicRoles(ctx, []int64{searchOptions.UserID}, orgID)
if err != nil {
return nil, fmt.Errorf("could not fetch basic roles for the user: %w", err)
}
var roles []string
var ok bool
if roles, ok = roleList[userID]; !ok {
return nil, fmt.Errorf("found no basic roles for user %d in organisation %d", userID, orgID)
if roles, ok = roleList[searchOptions.UserID]; !ok {
return nil, fmt.Errorf("found no basic roles for user %d in organisation %d", searchOptions.UserID, orgID)
}
permissions := make([]accesscontrol.Permission, 0)
for _, builtin := range roles {
@ -692,13 +681,13 @@ func (s *Service) searchUserPermissions(ctx context.Context, orgID int64, search
if err != nil {
return nil, err
}
permissions = append(permissions, dbPermissions[userID]...)
permissions = append(permissions, dbPermissions[searchOptions.UserID]...)
if s.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) && len(searchOptions.ActionSets) != 0 {
permissions = s.actionResolver.ExpandActionSetsWithFilter(permissions, GetActionFilter(searchOptions))
}
key, err := accesscontrol.GetSearchPermissionCacheKey(s.log, &user.SignedInUser{UserID: userID, OrgID: orgID}, searchOptions)
key, err := accesscontrol.GetSearchPermissionCacheKey(s.log, &user.SignedInUser{UserID: searchOptions.UserID, OrgID: orgID}, searchOptions)
if err != nil {
s.log.Warn("failed to create search permission cache key", "err", err)
} else {
@ -712,14 +701,9 @@ func (s *Service) searchUserPermissionsFromCache(ctx context.Context, orgID int6
_, span := tracer.Start(ctx, "accesscontrol.acimpl.searchUserPermissionsFromCache")
defer span.End()
userID, err := searchOptions.ComputeUserID()
if err != nil {
return nil, false
}
// Create a temp signed in user object to retrieve cache key
tempUser := &user.SignedInUser{
UserID: userID,
UserID: searchOptions.UserID,
OrgID: orgID,
}

3
pkg/services/accesscontrol/acimpl/service_bench_test.go
View File

@ -8,7 +8,6 @@ import (
"github.com/stretchr/testify/require"
"github.com/grafana/authlib/claims"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log"
@ -262,7 +261,7 @@ func benchSearchUserWithAction(b *testing.B, usersCount, resourceCount int) {
for n := 0; n < b.N; n++ {
usersPermissions, err := acService.SearchUsersPermissions(context.Background(), siu,
accesscontrol.SearchOptions{Action: "resources:action2", TypedID: claims.NewTypeID(claims.TypeUser, "14")})
accesscontrol.SearchOptions{Action: "resources:action2", UserID: 14})
require.NoError(b, err)
require.Len(b, usersPermissions, 1)
for _, permissions := range usersPermissions {

21
pkg/services/accesscontrol/acimpl/service_test.go
View File

@ -8,7 +8,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/authlib/claims"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/localcache"
@ -547,7 +546,7 @@ func TestService_SearchUsersPermissions(t *testing.T) {
// only the user's basic roles and the user's stored permissions
name: "check namespacedId filter works correctly",
siuPermissions: listAllPerms,
searchOption: accesscontrol.SearchOptions{TypedID: claims.NewTypeID(claims.TypeServiceAccount, "1")},
searchOption: accesscontrol.SearchOptions{UserID: 1},
ramRoles: map[string]*accesscontrol.RoleDTO{
string(identity.RoleEditor): {Permissions: []accesscontrol.Permission{
{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
@ -619,7 +618,7 @@ func TestService_SearchUserPermissions(t *testing.T) {
name: "ram only",
searchOption: accesscontrol.SearchOptions{
ActionPrefix: "teams",
TypedID: claims.NewTypeID(claims.TypeUser, "2"),
UserID: 2,
},
ramRoles: map[string]*accesscontrol.RoleDTO{
string(identity.RoleEditor): {Permissions: []accesscontrol.Permission{
@ -644,7 +643,7 @@ func TestService_SearchUserPermissions(t *testing.T) {
name: "stored only",
searchOption: accesscontrol.SearchOptions{
ActionPrefix: "teams",
TypedID: claims.NewTypeID(claims.TypeUser, "2"),
UserID: 2,
},
storedPerms: map[int64][]accesscontrol.Permission{
1: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}},
@ -664,7 +663,7 @@ func TestService_SearchUserPermissions(t *testing.T) {
name: "ram and stored",
searchOption: accesscontrol.SearchOptions{
ActionPrefix: "teams",
TypedID: claims.NewTypeID(claims.TypeUser, "2"),
UserID: 2,
},
ramRoles: map[string]*accesscontrol.RoleDTO{
string(identity.RoleAdmin): {Permissions: []accesscontrol.Permission{
@ -694,7 +693,7 @@ func TestService_SearchUserPermissions(t *testing.T) {
name: "check action prefix filter works correctly",
searchOption: accesscontrol.SearchOptions{
ActionPrefix: "teams",
TypedID: claims.NewTypeID(claims.TypeUser, "1"),
UserID: 1,
},
ramRoles: map[string]*accesscontrol.RoleDTO{
string(identity.RoleEditor): {Permissions: []accesscontrol.Permission{
@ -715,8 +714,8 @@ func TestService_SearchUserPermissions(t *testing.T) {
{
name: "check action filter works correctly",
searchOption: accesscontrol.SearchOptions{
Action: accesscontrol.ActionTeamsRead,
TypedID: claims.NewTypeID(claims.TypeUser, "1"),
Action: accesscontrol.ActionTeamsRead,
UserID: 1,
},
ramRoles: map[string]*accesscontrol.RoleDTO{
string(identity.RoleEditor): {Permissions: []accesscontrol.Permission{
@ -737,8 +736,8 @@ func TestService_SearchUserPermissions(t *testing.T) {
{
name: "check action sets are correctly included if an action is specified",
searchOption: accesscontrol.SearchOptions{
Action: "dashboards:read",
TypedID: claims.NewTypeID(claims.TypeUser, "1"),
Action: "dashboards:read",
UserID: 1,
},
withActionSets: true,
actionSets: map[string][]string{
@ -771,7 +770,7 @@ func TestService_SearchUserPermissions(t *testing.T) {
name: "check action sets are correctly included if an action prefix is specified",
searchOption: accesscontrol.SearchOptions{
ActionPrefix: "dashboards",
TypedID: claims.NewTypeID(claims.TypeUser, "1"),
UserID: 1,
},
withActionSets: true,
actionSets: map[string][]string{

57
pkg/services/accesscontrol/api/api.go
View File

@ -1,7 +1,14 @@
package api
import (
"context"
"errors"
"fmt"
"net/http"
"strconv"
"github.com/grafana/authlib/claims"
"go.opentelemetry.io/otel"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/api/routing"
@ -10,16 +17,17 @@ 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"
"go.opentelemetry.io/otel"
"github.com/grafana/grafana/pkg/services/user"
)
var tracer = otel.Tracer("github.com/grafana/grafana/pkg/services/accesscontrol/api")
func NewAccessControlAPI(router routing.RouteRegister, accesscontrol ac.AccessControl, service ac.Service,
features featuremgmt.FeatureToggles) *AccessControlAPI {
userSvc user.Service, features featuremgmt.FeatureToggles) *AccessControlAPI {
return &AccessControlAPI{
RouteRegister: router,
Service: service,
userSvc: userSvc,
AccessControl: accesscontrol,
features: features,
}
@ -29,6 +37,7 @@ type AccessControlAPI struct {
Service ac.Service
AccessControl ac.AccessControl
RouteRegister routing.RouteRegister
userSvc user.Service
features featuremgmt.FeatureToggles
}
@ -81,7 +90,20 @@ func (api *AccessControlAPI) searchUsersPermissions(c *contextmodel.ReqContext)
ActionPrefix: c.Query("actionPrefix"),
Action: c.Query("action"),
Scope: c.Query("scope"),
TypedID: c.Query("namespacedId"),
}
// namespacedId is the typed identifier of an identity
// it is specified using user/service account IDs or UIDs (ex: user:3, service-account:4, user:adisufjf93e9sd)
if typedID := c.Query("namespacedId"); typedID != "" {
userID, err := api.ComputeUserID(ctx, c.Query("namespacedId"))
if err != nil {
if errors.Is(err, user.ErrUserNotFound) {
return response.JSON(http.StatusBadRequest, err.Error())
}
return response.JSON(http.StatusInternalServerError, err.Error())
}
searchOptions.UserID = userID
}
// Validate inputs
@ -89,7 +111,7 @@ func (api *AccessControlAPI) searchUsersPermissions(c *contextmodel.ReqContext)
return response.JSON(http.StatusBadRequest, "'action' and 'actionPrefix' are mutually exclusive")
}
if searchOptions.TypedID == "" && searchOptions.ActionPrefix == "" && searchOptions.Action == "" {
if searchOptions.UserID <= 0 && searchOptions.ActionPrefix == "" && searchOptions.Action == "" {
return response.JSON(http.StatusBadRequest, "at least one search option must be provided")
}
@ -106,3 +128,30 @@ func (api *AccessControlAPI) searchUsersPermissions(c *contextmodel.ReqContext)
return response.JSON(http.StatusOK, permsByAction)
}
func (api *AccessControlAPI) ComputeUserID(ctx context.Context, typedID string) (int64, error) {
if typedID == "" {
return -1, nil
}
typ, idStr, err := claims.ParseTypeID(typedID)
if err != nil {
return 0, err
}
if !claims.IsIdentityType(typ, claims.TypeUser, claims.TypeServiceAccount) {
return 0, fmt.Errorf("invalid type: %s", typ)
}
id, err := strconv.ParseInt(idStr, 10, 64)
if err == nil {
return id, nil
}
user, err := api.userSvc.GetByUID(ctx, &user.GetUserByUIDQuery{UID: idStr})
if err != nil {
return 0, err
}
return user.ID, nil
}

24
pkg/services/accesscontrol/api/api_test.go
View File

@ -5,6 +5,7 @@ import (
"net/http"
"testing"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/api/routing"
@ -13,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web/webtest"
)
@ -40,7 +42,7 @@ func TestAPI_getUserActions(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
acSvc := actest.FakeService{ExpectedPermissions: tt.permissions}
api := NewAccessControlAPI(routing.NewRouteRegister(), actest.FakeAccessControl{}, acSvc, featuremgmt.WithFeatures())
api := NewAccessControlAPI(routing.NewRouteRegister(), actest.FakeAccessControl{}, acSvc, &usertest.FakeUserService{}, featuremgmt.WithFeatures())
api.RegisterAPIEndpoints()
server := webtest.NewServer(t, api.RouteRegister)
@ -93,7 +95,7 @@ func TestAPI_getUserPermissions(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
acSvc := actest.FakeService{ExpectedPermissions: tt.permissions}
api := NewAccessControlAPI(routing.NewRouteRegister(), actest.FakeAccessControl{}, acSvc, featuremgmt.WithFeatures())
api := NewAccessControlAPI(routing.NewRouteRegister(), actest.FakeAccessControl{}, acSvc, &usertest.FakeUserService{}, featuremgmt.WithFeatures())
api.RegisterAPIEndpoints()
server := webtest.NewServer(t, api.RouteRegister)
@ -149,6 +151,19 @@ func TestAccessControlAPI_searchUsersPermissions(t *testing.T) {
expectedCode: http.StatusOK,
expectedOutput: map[int64]map[string][]string{2: {"users:read": {"users:*"}}},
},
{
desc: "Should resolve UID based identifier to the corresponding ID",
filters: "?namespacedId=user:user_2_uid",
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 fail if cannot resolve UID based identifier",
filters: "?namespacedId=user:non_existent_uid",
permissions: map[int64][]ac.Permission{2: {{Action: "users:read", Scope: "users:*"}}},
expectedCode: http.StatusBadRequest,
},
{
desc: "Should reduce permissions",
filters: "?namespacedId=service-account:2",
@ -174,7 +189,10 @@ func TestAccessControlAPI_searchUsersPermissions(t *testing.T) {
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))
mockUserSvc := usertest.NewMockService(t)
mockUserSvc.On("GetByUID", mock.Anything, &user.GetUserByUIDQuery{UID: "user_2_uid"}).Return(&user.User{ID: 2}, nil).Maybe()
mockUserSvc.On("GetByUID", mock.Anything, &user.GetUserByUIDQuery{UID: "non_existent_uid"}).Return(nil, user.ErrUserNotFound).Maybe()
api := NewAccessControlAPI(routing.NewRouteRegister(), accessControl, acSvc, mockUserSvc, featuremgmt.WithFeatures(featuremgmt.FlagAccessControlOnCall))
api.RegisterAPIEndpoints()
server := webtest.NewServer(t, api.RouteRegister)

34
pkg/services/accesscontrol/database/database.go
View File

@ -175,15 +175,6 @@ func (s *AccessControlStore) SearchUsersPermissions(ctx context.Context, orgID i
}
dbPerms := make([]UserRBACPermission, 0)
userID := int64(-1)
if options.TypedID != "" {
var err error
userID, err = options.ComputeUserID()
if err != nil {
return nil, err
}
}
if err := s.sql.WithDbSession(ctx, func(sess *db.Session) error {
roleNameFilterJoin := ""
if len(options.RolePrefixes) > 0 {
@ -193,28 +184,25 @@ func (s *AccessControlStore) SearchUsersPermissions(ctx context.Context, orgID i
params := []any{}
direct := userAssignsSQL
if userID >= 0 {
direct += " WHERE ur.user_id = ?"
params = append(params, userID)
}
team := teamAssignsSQL
if userID >= 0 {
team += " WHERE tm.user_id = ?"
params = append(params, userID)
}
basic := basicRoleAssignsSQL
if userID >= 0 {
if options.UserID > 0 {
direct += " WHERE ur.user_id = ?"
params = append(params, options.UserID)
team += " WHERE tm.user_id = ?"
params = append(params, options.UserID)
basic += " WHERE ou.user_id = ?"
params = append(params, userID)
params = append(params, options.UserID)
}
grafanaAdmin := fmt.Sprintf(grafanaAdminAssignsSQL, s.sql.Quote("user"))
params = append(params, accesscontrol.RoleGrafanaAdmin)
if userID >= 0 {
if options.UserID > 0 {
grafanaAdmin += " AND sa.user_id = ?"
params = append(params, userID)
params = append(params, options.UserID)
}
// Find permissions

3
pkg/services/accesscontrol/database/database_test.go
View File

@ -9,7 +9,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/authlib/claims"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/tracing"
@ -625,7 +624,7 @@ func TestIntegrationAccessControlStore_SearchUsersPermissions(t *testing.T) {
},
options: accesscontrol.SearchOptions{
ActionPrefix: "teams:",
TypedID: claims.NewTypeID(claims.TypeUser, "1"),
UserID: 1,
},
wantPerm: map[int64][]accesscontrol.Permission{
1: {{Action: "teams:read", Scope: "teams:id:1"}, {Action: "teams:read", Scope: "teams:id:10"},