From e56b2cae00249c38a6254ab60dde1c9fbab6a37c Mon Sep 17 00:00:00 2001 From: Jo Date: Wed, 12 Jul 2023 12:28:04 +0200 Subject: [PATCH] MESA: Allow using synced permissions (#71377) * wip * cover authorize in org behavior * revert export * fix org tests * change permissions nit --- pkg/api/org_test.go | 2 +- pkg/api/quota_test.go | 11 +- .../accesscontrol/authorize_in_org_test.go | 188 ++++++++++++++++++ pkg/services/accesscontrol/middleware.go | 23 ++- 4 files changed, 212 insertions(+), 12 deletions(-) create mode 100644 pkg/services/accesscontrol/authorize_in_org_test.go diff --git a/pkg/api/org_test.go b/pkg/api/org_test.go index fa417c1aa13..54c7056c06d 100644 --- a/pkg/api/org_test.go +++ b/pkg/api/org_test.go @@ -253,7 +253,7 @@ func TestAPIEndpoint_GetOrg(t *testing.T) { hs.accesscontrolService = &actest.FakeService{ExpectedPermissions: tt.permissions} }) verify := func(path string) { - req := webtest.RequestWithSignedInUser(server.NewGetRequest(path), userWithPermissions(2, nil)) + req := webtest.RequestWithSignedInUser(server.NewGetRequest(path), userWithPermissions(2, tt.permissions)) res, err := server.Send(req) require.NoError(t, err) assert.Equal(t, tt.expectedCode, res.StatusCode) diff --git a/pkg/api/quota_test.go b/pkg/api/quota_test.go index 18653fdbb21..b5ea62293f0 100644 --- a/pkg/api/quota_test.go +++ b/pkg/api/quota_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/usertest" "github.com/grafana/grafana/pkg/setting" @@ -62,6 +63,7 @@ func TestAPIEndpoint_GetOrgQuotas(t *testing.T) { cfg.Quota.Enabled = true server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = cfg + hs.accesscontrolService = &actest.FakeService{ExpectedPermissions: []accesscontrol.Permission{}} hs.userService = &usertest.FakeUserService{ ExpectedSignedInUser: &user.SignedInUser{OrgID: 2}, } @@ -77,7 +79,7 @@ func TestAPIEndpoint_GetOrgQuotas(t *testing.T) { t.Run("AccessControl prevents viewing another org quotas with correct permissions in another org", func(t *testing.T) { // Set correct permissions in org 1 and empty permissions in org 2 user := userWithPermissions(1, []accesscontrol.Permission{{Action: accesscontrol.ActionOrgsQuotasRead}}) - user.Permissions[2] = map[string][]string{} + user.Permissions[2] = nil req := webtest.RequestWithSignedInUser(server.NewGetRequest(fmt.Sprintf(getOrgsQuotasURL, 2)), user) res, err := server.Send(req) require.NoError(t, err) @@ -107,8 +109,10 @@ func TestAPIEndpoint_PutOrgQuotas(t *testing.T) { Org: 5, }, } + fakeACService := &actest.FakeService{} server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = cfg + hs.accesscontrolService = fakeACService hs.userService = &usertest.FakeUserService{ ExpectedSignedInUser: &user.SignedInUser{OrgID: 2}, } @@ -118,6 +122,7 @@ func TestAPIEndpoint_PutOrgQuotas(t *testing.T) { t.Run("AccessControl allows updating another org quotas with correct permissions", func(t *testing.T) { user := userWithPermissions(2, []accesscontrol.Permission{{Action: accesscontrol.ActionOrgsQuotasWrite}}) user.OrgID = 1 + fakeACService.ExpectedPermissions = []accesscontrol.Permission{{Action: accesscontrol.ActionOrgsQuotasWrite}} req := webtest.RequestWithSignedInUser(server.NewRequest(http.MethodPut, fmt.Sprintf(putOrgsQuotasURL, 2, "org_user"), input), user) response, err := server.SendJSON(req) require.NoError(t, err) @@ -128,7 +133,8 @@ func TestAPIEndpoint_PutOrgQuotas(t *testing.T) { input = strings.NewReader(testUpdateOrgQuotaCmd) t.Run("AccessControl prevents updating another org quotas with correct permissions in another org", func(t *testing.T) { user := userWithPermissions(1, []accesscontrol.Permission{{Action: accesscontrol.ActionOrgsQuotasWrite}}) - user.Permissions[2] = map[string][]string{} + user.Permissions[2] = nil + fakeACService.ExpectedPermissions = []accesscontrol.Permission{} req := webtest.RequestWithSignedInUser(server.NewRequest(http.MethodPut, fmt.Sprintf(putOrgsQuotasURL, 2, "org_user"), input), user) response, err := server.SendJSON(req) require.NoError(t, err) @@ -139,6 +145,7 @@ func TestAPIEndpoint_PutOrgQuotas(t *testing.T) { input = strings.NewReader(testUpdateOrgQuotaCmd) t.Run("AccessControl prevents updating another org quotas with incorrect permissions", func(t *testing.T) { user := userWithPermissions(2, []accesscontrol.Permission{{Action: "orgs:invalid"}}) + fakeACService.ExpectedPermissions = []accesscontrol.Permission{} req := webtest.RequestWithSignedInUser(server.NewRequest(http.MethodPut, fmt.Sprintf(putOrgsQuotasURL, 2, "org_user"), input), user) response, err := server.SendJSON(req) require.NoError(t, err) diff --git a/pkg/services/accesscontrol/authorize_in_org_test.go b/pkg/services/accesscontrol/authorize_in_org_test.go new file mode 100644 index 00000000000..4893e8b88ee --- /dev/null +++ b/pkg/services/accesscontrol/authorize_in_org_test.go @@ -0,0 +1,188 @@ +package accesscontrol_test + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "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/user" + "github.com/grafana/grafana/pkg/services/user/usertest" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/web" +) + +func TestAuthorizeInOrgMiddleware(t *testing.T) { + cfg := setting.NewCfg() + ac := acimpl.ProvideAccessControl(cfg) + + // Define test cases + testCases := []struct { + name string + orgIDGetter accesscontrol.OrgIDGetter + evaluator accesscontrol.Evaluator + accessControl accesscontrol.AccessControl + acService accesscontrol.Service + userCache user.Service + ctxSignedInUser *user.SignedInUser + expectedStatus int + }{ + { + name: "should authorize user with global org ID - no fetch", + orgIDGetter: func(c *contextmodel.ReqContext) (int64, error) { + return accesscontrol.GlobalOrgID, nil + }, + evaluator: accesscontrol.EvalPermission("users:read", "users:*"), + accessControl: ac, + acService: &actest.FakeService{}, + userCache: &usertest.FakeUserService{}, + ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}}, + expectedStatus: http.StatusOK, + }, + { + name: "should authorize user with non-global org ID - no fetch", + orgIDGetter: func(c *contextmodel.ReqContext) (int64, error) { + return 1, nil + }, + evaluator: accesscontrol.EvalPermission("users:read", "users:*"), + accessControl: ac, + acService: &actest.FakeService{}, + userCache: &usertest.FakeUserService{}, + ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}}, + expectedStatus: http.StatusOK, + }, + { + name: "should return 403 when user has no permissions for the org", + orgIDGetter: func(c *contextmodel.ReqContext) (int64, error) { + return 1, nil + }, + evaluator: accesscontrol.EvalPermission("users:read", "users:*"), + accessControl: ac, + userCache: &usertest.FakeUserService{}, + ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{}}, + acService: &actest.FakeService{}, + expectedStatus: http.StatusForbidden, + }, + { + name: "should return 403 when user org ID doesn't match and user does not exist in org 2", + orgIDGetter: func(c *contextmodel.ReqContext) (int64, error) { + return 2, nil + }, + evaluator: accesscontrol.EvalPermission("users:read", "users:*"), + accessControl: ac, + userCache: &usertest.FakeUserService{ExpectedError: fmt.Errorf("user not found")}, + acService: &actest.FakeService{}, + ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}}, + expectedStatus: http.StatusForbidden, + }, + { + name: "should fetch user permissions when they are empty", + orgIDGetter: func(c *contextmodel.ReqContext) (int64, error) { + return 1, 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: nil}, nil + }, + }, + ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: nil}, + expectedStatus: http.StatusOK, + }, + { + name: "should fetch user permissions when org ID doesn't match", + 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: 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, + }, + { + name: "fails to fetch user permissions when org ID doesn't match", + orgIDGetter: func(c *contextmodel.ReqContext) (int64, error) { + return 2, nil + }, + evaluator: accesscontrol.EvalPermission("users:read", "users:*"), + accessControl: ac, + 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, + }, + { + name: "unable to get target org", + orgIDGetter: func(c *contextmodel.ReqContext) (int64, error) { + return 0, fmt.Errorf("unable to get target org") + }, + evaluator: accesscontrol.EvalPermission("users:read", "users:*"), + accessControl: ac, + acService: &actest.FakeService{}, + userCache: &usertest.FakeUserService{}, + ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}}, + expectedStatus: http.StatusForbidden, + }, + } + + // Run test cases + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create test context + req := httptest.NewRequest(http.MethodGet, "/api/endpoint", nil) + + service := tc.acService + + // Create middleware + middleware := accesscontrol.AuthorizeInOrgMiddleware( + tc.accessControl, + service, + tc.userCache, + )(tc.orgIDGetter, tc.evaluator) + + // Create test server + server := web.New() + server.Use(contextProvider(func(c *contextmodel.ReqContext) { + c.SignedInUser = tc.ctxSignedInUser + c.IsSignedIn = true + })) + server.Use(middleware) + + server.Get("/api/endpoint", func(c *contextmodel.ReqContext) { + c.Resp.WriteHeader(http.StatusOK) + }) + + // Perform request + recorder := httptest.NewRecorder() + server.ServeHTTP(recorder, req) + + // Check response + assert.Equal(t, tc.expectedStatus, recorder.Code, fmt.Sprintf("expected body: %s", recorder.Body.String())) + }) + } +} diff --git a/pkg/services/accesscontrol/middleware.go b/pkg/services/accesscontrol/middleware.go index dba479ce47b..520c68c5e65 100644 --- a/pkg/services/accesscontrol/middleware.go +++ b/pkg/services/accesscontrol/middleware.go @@ -179,19 +179,16 @@ type userCache interface { func AuthorizeInOrgMiddleware(ac AccessControl, service Service, cache userCache) func(OrgIDGetter, Evaluator) web.Handler { return func(getTargetOrg OrgIDGetter, evaluator Evaluator) web.Handler { return func(c *contextmodel.ReqContext) { - // using a copy of the user not to modify the signedInUser, yet perform the permission evaluation in another org + // We need to copy the user here because we're going to mutate it userCopy := *(c.SignedInUser) - orgID, err := getTargetOrg(c) + targetOrgID, err := getTargetOrg(c) if err != nil { deny(c, nil, fmt.Errorf("failed to get target org: %w", err)) return } - if orgID == GlobalOrgID { - userCopy.OrgID = orgID - userCopy.OrgName = "" - userCopy.OrgRole = "" - } else { - query := user.GetSignedInUserQuery{UserID: c.UserID, OrgID: orgID} + + if targetOrgID != GlobalOrgID && userCopy.Permissions[targetOrgID] == nil { + query := user.GetSignedInUserQuery{UserID: c.UserID, OrgID: targetOrgID} queryResult, err := cache.GetSignedInUserWithCacheCtx(c.Req.Context(), &query) if err != nil { deny(c, nil, fmt.Errorf("failed to authenticate user in target org: %w", err)) @@ -207,12 +204,20 @@ func AuthorizeInOrgMiddleware(ac AccessControl, service Service, cache userCache if err != nil { deny(c, nil, fmt.Errorf("failed to authenticate user in target org: %w", err)) } + + // guard against nil map + if userCopy.Permissions == nil { + userCopy.Permissions = make(map[int64]map[string][]string) + } userCopy.Permissions[userCopy.OrgID] = GroupScopesByAction(permissions) } authorize(c, ac, &userCopy, evaluator) - // Set the sign-ed in user permissions in that org + // guard against nil map + if c.SignedInUser.Permissions == nil { + c.SignedInUser.Permissions = make(map[int64]map[string][]string) + } c.SignedInUser.Permissions[userCopy.OrgID] = userCopy.Permissions[userCopy.OrgID] } }