public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[v11.1.x] RBAC: Fix an issue with server admins not being able to manage users in orgs that they don't belong to (#92273)

Browse Source 
* RBAC: Fix an issue with server admins not being able to manage users in orgs that they don't belong to (#92024)

* look at global perms if user is not a part of the target org

* use constant

* update tests

(cherry picked from commit 41ac5b5ae7)

* fix tests

---------

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>
This commit is contained in:
grafana-delivery-bot[bot]
2024-08-22 11:24:08 +01:00
committed by GitHub
co-authored by Ieva
parent 783216e4b9
commit da500a56b3
2 changed files with 43 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/accesscontrol/authorize_in_org_test.go
+39 -62
View File
@@ -1,7 +1,6 @@
package accesscontrol_test
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
@@ -11,7 +10,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/authn/authntest"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
@@ -19,7 +18,6 @@ import (
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/services/team/teamtest"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/web"
)
@@ -34,8 +32,8 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
orgIDGetter accesscontrol.OrgIDGetter
evaluator accesscontrol.Evaluator
accessControl accesscontrol.AccessControl
acService accesscontrol.Service
userCache user.Service
userIdentities []*authn.Identity
authnErrors []error
ctxSignedInUser *user.SignedInUser
teamService team.Service
expectedStatus int
@@ -45,7 +43,6 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
targetOrgId: accesscontrol.GlobalOrgID,
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
userCache: &usertest.FakeUserService{},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}},
targerOrgPermissions: []accesscontrol.Permission{{Action: "users:read", Scope: "users:*"}},
teamService: &teamtest.FakeService{},
@@ -57,7 +54,6 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
targerOrgPermissions: []accesscontrol.Permission{{Action: "users:read", Scope: "users:*"}},
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
userCache: &usertest.FakeUserService{},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}},
teamService: &teamtest.FakeService{},
expectedStatus: http.StatusOK,
@@ -68,7 +64,6 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
targerOrgPermissions: []accesscontrol.Permission{},
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
userCache: &usertest.FakeUserService{},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{}},
teamService: &teamtest.FakeService{},
expectedStatus: http.StatusForbidden,
@@ -79,7 +74,6 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
targerOrgPermissions: []accesscontrol.Permission{{Action: "users:read", Scope: "users:*"}},
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
userCache: &usertest.FakeUserService{},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}},
teamService: &teamtest.FakeService{},
expectedStatus: http.StatusOK,
@@ -90,33 +84,10 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
targerOrgPermissions: []accesscontrol.Permission{},
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
userCache: &usertest.FakeUserService{},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}},
teamService: &teamtest.FakeService{},
expectedStatus: http.StatusForbidden,
},
{
name: "should return 403 when user org ID doesn't match and user does not exist in org 2",
targetOrgId: 2,
targerOrgPermissions: []accesscontrol.Permission{},
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
userCache: &usertest.FakeUserService{ExpectedError: fmt.Errorf("user not found")},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}},
teamService: &teamtest.FakeService{},
expectedStatus: http.StatusForbidden,
},
{
name: "should return 403 early when api key org ID doesn't match",
targetOrgId: 2,
targerOrgPermissions: []accesscontrol.Permission{},
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
userCache: &usertest.FakeUserService{},
ctxSignedInUser: &user.SignedInUser{ApiKeyID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}},
teamService: &teamtest.FakeService{},
expectedStatus: http.StatusForbidden,
},
{
name: "should fetch user permissions when org ID doesn't match",
targetOrgId: 2,
@@ -124,13 +95,8 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
teamService: &teamtest.FakeService{},
userCache: &usertest.FakeUserService{
GetSignedInUserFn: func(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) {
return &user.SignedInUser{UserID: 1, OrgID: 2, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}}, nil
},
},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:write": {"users:*"}}}},
expectedStatus: http.StatusOK,
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:write": {"users:*"}}}},
expectedStatus: http.StatusOK,
},
{
name: "fails to fetch user permissions when org ID doesn't match",
@@ -139,16 +105,9 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
teamService: &teamtest.FakeService{},
acService: &actest.FakeService{
ExpectedErr: fmt.Errorf("failed to get user permissions"),
},
userCache: &usertest.FakeUserService{
GetSignedInUserFn: func(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) {
return &user.SignedInUser{UserID: 1, OrgID: 2, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}}, nil
},
},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}},
expectedStatus: http.StatusForbidden,
authnErrors: []error{fmt.Errorf("failed to get user permissions")},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}},
expectedStatus: http.StatusForbidden,
},
{
name: "unable to get target org",
@@ -157,24 +116,35 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
},
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
userCache: &usertest.FakeUserService{},
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}},
teamService: &teamtest.FakeService{},
expectedStatus: http.StatusForbidden,
},
{
name: "should fetch global user permissions when user is not a member of the target org",
targetOrgId: 2,
targerOrgPermissions: []accesscontrol.Permission{{Action: "users:read", Scope: "users:*"}},
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
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
},
},
name: "should fetch global user permissions when user is not a member of the target org",
targetOrgId: 2,
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:write": {"users:*"}}}},
expectedStatus: http.StatusOK,
userIdentities: []*authn.Identity{
{ID: identity.MustParseNamespaceID("user:1"), OrgID: -1, Permissions: map[int64]map[string][]string{}},
{ID: identity.MustParseNamespaceID("user:1"), OrgID: accesscontrol.GlobalOrgID, Permissions: map[int64]map[string][]string{accesscontrol.GlobalOrgID: {"users:read": {"users:*"}}}},
},
authnErrors: []error{nil, nil},
expectedStatus: http.StatusOK,
},
{
name: "should fail if user is not a member of the target org and doesn't have the right permissions globally",
targetOrgId: 2,
evaluator: accesscontrol.EvalPermission("users:read", "users:*"),
accessControl: ac,
ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:write": {"users:*"}}}},
userIdentities: []*authn.Identity{
{ID: identity.MustParseNamespaceID("user:1"), OrgID: -1, Permissions: map[int64]map[string][]string{}},
{ID: identity.MustParseNamespaceID("user:1"), OrgID: accesscontrol.GlobalOrgID, Permissions: map[int64]map[string][]string{accesscontrol.GlobalOrgID: {"folders:read": {"folders:*"}}}},
},
authnErrors: []error{nil, nil},
expectedStatus: http.StatusForbidden,
},
}
@@ -190,9 +160,16 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
Permissions: map[int64]map[string][]string{},
}
expectedIdentity.Permissions[tc.targetOrgId] = accesscontrol.GroupScopesByAction(tc.targerOrgPermissions)
var expectedErr error
if len(tc.authnErrors) > 0 {
expectedErr = tc.authnErrors[0]
}
authnService := &authntest.FakeService{
ExpectedIdentity: expectedIdentity,
ExpectedIdentity: expectedIdentity,
ExpectedIdentities: tc.userIdentities,
ExpectedErr: expectedErr,
ExpectedErrs: tc.authnErrors,
}
var orgIDGetter accesscontrol.OrgIDGetter
pkg/services/accesscontrol/middleware.go
+4
View File
@@ -195,6 +195,10 @@ func AuthorizeInOrgMiddleware(ac AccessControl, authnService authn.Service) func
var orgUser identity.Requester = c.SignedInUser
if targetOrgID != c.SignedInUser.GetOrgID() {
orgUser, err = authnService.ResolveIdentity(c.Req.Context(), targetOrgID, c.SignedInUser.GetID())
if err == nil && orgUser.GetOrgID() == NoOrgID {
// User is not a member of the target org, so only their global permissions are relevant
orgUser, err = authnService.ResolveIdentity(c.Req.Context(), GlobalOrgID, c.SignedInUser.GetID())
}
if err != nil {
deny(c, nil, fmt.Errorf("failed to authenticate user in target org: %w", err))
return