RBAC: Fix authorize in org (#81552)

* RBAC: Fix authorize in org

* Implement option 2

* Fix typo

* Fix alerting test

* Add test to cover the not member case
This commit is contained in:
Gabriel MABILLE 2024-02-01 12:37:01 +01:00 committed by GitHub
parent 0f1ba3a9fe
commit 3df0611f81
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 80 additions and 11 deletions

View File

@ -39,13 +39,19 @@ func (a *AccessControl) Evaluate(ctx context.Context, user identity.Requester, e
}
namespace, identifier := user.GetNamespacedID()
if len(user.GetPermissions()) == 0 {
// If the user is in no organization, then the evaluation must happen based on the user's global permissions
permissions := user.GetPermissions()
if user.GetOrgID() == accesscontrol.NoOrgID {
permissions = user.GetGlobalPermissions()
}
if len(permissions) == 0 {
a.log.Debug("No permissions set for entity", "namespace", namespace, "id", identifier, "orgID", user.GetOrgID(), "login", user.GetLogin())
return false, nil
}
// Test evaluation without scope resolver first, this will prevent 403 for wildcard scopes when resource does not exist
if evaluator.Evaluate(user.GetPermissions()) {
if evaluator.Evaluate(permissions) {
return true, nil
}
@ -57,7 +63,7 @@ func (a *AccessControl) Evaluate(ctx context.Context, user identity.Requester, e
return false, err
}
return resolvedEvaluator.Evaluate(user.GetPermissions()), nil
return resolvedEvaluator.Evaluate(permissions), nil
}
func (a *AccessControl) RegisterScopeAttributeResolver(prefix string, resolver accesscontrol.ScopeAttributeResolver) {

View File

@ -202,6 +202,24 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
teamService: &teamtest.FakeService{},
expectedStatus: http.StatusForbidden,
},
{
name: "should fetch global user permissions when user is not a member of the target org",
orgIDGetter: func(c *contextmodel.ReqContext) (int64, error) {
return 2, nil
},
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
acService: &actest.FakeService{
ExpectedPermissions: []accesscontrol.Permission{{Action: "users:read", Scope: "users:*"}},
},
userCache: &usertest.FakeUserService{
GetSignedInUserFn: func(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) {
return &user.SignedInUser{UserID: 1, OrgID: -1, Permissions: map[int64]map[string][]string{}}, nil
},
},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:write": {"users:*"}}}},
expectedStatus: http.StatusOK,
},
}
// Run test cases

View File

@ -260,23 +260,32 @@ func makeTmpUser(ctx context.Context, service Service, cache userCache,
tmpUser.OrgName = queryResult.OrgName
tmpUser.OrgRole = queryResult.OrgRole
if teamService != nil {
teamIDs, err := teamService.GetTeamIDsByUser(ctx, &team.GetTeamIDsByUserQuery{OrgID: targetOrgID, UserID: tmpUser.UserID})
if err != nil {
return nil, err
// Only fetch the team membership is the user is a member of the organization
if queryResult.OrgID == targetOrgID {
if teamService != nil {
teamIDs, err := teamService.GetTeamIDsByUser(ctx, &team.GetTeamIDsByUserQuery{OrgID: targetOrgID, UserID: tmpUser.UserID})
if err != nil {
return nil, err
}
tmpUser.Teams = teamIDs
}
tmpUser.Teams = teamIDs
}
}
}
if tmpUser.Permissions[targetOrgID] == nil || len(tmpUser.Permissions[targetOrgID]) == 0 {
// If the user is not a member of the organization
// evaluation must happen based on global permissions.
evaluationOrg := targetOrgID
if tmpUser.OrgID == NoOrgID {
evaluationOrg = GlobalOrgID
}
if tmpUser.Permissions[evaluationOrg] == nil || len(tmpUser.Permissions[evaluationOrg]) == 0 {
permissions, err := service.GetUserPermissions(ctx, tmpUser, Options{})
if err != nil {
return nil, err
}
tmpUser.Permissions[targetOrgID] = GroupScopesByAction(permissions)
tmpUser.Permissions[evaluationOrg] = GroupScopesByAction(permissions)
}
return tmpUser, nil

View File

@ -330,6 +330,7 @@ func (cmd *SaveExternalServiceRoleCommand) Validate() error {
const (
GlobalOrgID = 0
NoOrgID = int64(-1)
GeneralFolderUID = "general"
RoleGrafanaAdmin = "Grafana Admin"

View File

@ -40,6 +40,8 @@ type Requester interface {
GetOrgRole() roletype.RoleType
// GetPermissions returns the permissions of the active entity.
GetPermissions() map[string][]string
// GetGlobalPermissions returns the permissions of the active entity that are available across all organizations.
GetGlobalPermissions() map[string][]string
// DEPRECATED: GetTeams returns the teams the entity is a member of.
// Retrieve the teams from the team service instead of using this method.
GetTeams() []int64

View File

@ -31,6 +31,7 @@ const (
const (
AnonymousNamespaceID = NamespaceAnonymous + ":0"
GlobalOrgID = int64(0)
)
var _ identity.Requester = (*Identity)(nil)
@ -164,6 +165,19 @@ func (i *Identity) GetPermissions() map[string][]string {
return i.Permissions[i.GetOrgID()]
}
// GetGlobalPermissions returns the permissions of the active entity that are available across all organizations
func (i *Identity) GetGlobalPermissions() map[string][]string {
if i.Permissions == nil {
return make(map[string][]string)
}
if i.Permissions[GlobalOrgID] == nil {
return make(map[string][]string)
}
return i.Permissions[GlobalOrgID]
}
func (i *Identity) GetTeams() []int64 {
return i.Teams
}

View File

@ -8,6 +8,10 @@ import (
"github.com/grafana/grafana/pkg/services/auth/identity"
)
const (
GlobalOrgID = int64(0)
)
type SignedInUser struct {
UserID int64 `xorm:"user_id"`
OrgID int64 `xorm:"org_id"`
@ -159,6 +163,19 @@ func (u *SignedInUser) GetPermissions() map[string][]string {
return u.Permissions[u.GetOrgID()]
}
// GetGlobalPermissions returns the permissions of the active entity that are available across all organizations
func (u *SignedInUser) GetGlobalPermissions() map[string][]string {
if u.Permissions == nil {
return make(map[string][]string)
}
if u.Permissions[GlobalOrgID] == nil {
return make(map[string][]string)
}
return u.Permissions[GlobalOrgID]
}
// DEPRECATED: GetTeams returns the teams the entity is a member of
// Retrieve the teams from the team service instead of using this method.
func (u *SignedInUser) GetTeams() []int64 {

View File

@ -11,6 +11,7 @@ import (
"github.com/grafana/grafana-plugin-sdk-go/data"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/models/roletype"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions"
"github.com/grafana/grafana/pkg/services/datasources"
@ -92,9 +93,10 @@ func TestBacktesting(t *testing.T) {
t.Run("if user does not have permissions", func(t *testing.T) {
testUserId := createUser(t, env.SQLStore, user.CreateUserCommand{
DefaultOrgRole: "",
DefaultOrgRole: string(roletype.RoleNone),
Password: "test",
Login: "test",
OrgID: 1,
})
testUserApiCli := newAlertingApiClient(grafanaListedAddr, "test", "test")