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

Chore: Remove GetSignedInUserWithCacheCtx from store interface (#53734)

Browse Source 
* Remove delete suer from store interface

* Remove get signed in user with cache ctx from store interface

* Support options when setting up access control tests

* Fix broken tests

* Fix lint

* Add user fake to middleware

* Fix middleware tests, remove usertest being initialised twice

Co-authored-by: Karl Persson <kalle.persson@grafana.com>
This commit is contained in:
idafurjes
2022-08-16 16:08:59 +02:00
committed by GitHub
co-authored by Karl Persson
parent f3085b1cac
commit fa2e74cd6e
16 changed files with 119 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/api.go
+1 -1
View File
@@ -68,7 +68,7 @@ func (hs *HTTPServer) registerRoutes() {
reqSnapshotPublicModeOrSignedIn := middleware.SnapshotPublicModeOrSignedIn(hs.Cfg)
redirectFromLegacyPanelEditURL := middleware.RedirectFromLegacyPanelEditURL(hs.Cfg)
authorize := ac.Middleware(hs.AccessControl)
authorizeInOrg := ac.AuthorizeInOrgMiddleware(hs.AccessControl, hs.SQLStore)
authorizeInOrg := ac.AuthorizeInOrgMiddleware(hs.AccessControl, hs.userService)
quota := middleware.Quota(hs.QuotaService)
r := hs.RouteRegister
pkg/api/common_test.go
+15 -5
View File
@@ -285,6 +285,8 @@ type accessControlScenarioContext struct {
// acmock is an accesscontrol mock used to fake users rights.
acmock *accesscontrolmock.Mock
usermock *usertest.FakeUserService
// db is a test database initialized with InitTestDB
db sqlstore.Store
@@ -341,16 +343,19 @@ func setupSimpleHTTPServer(features *featuremgmt.FeatureManager) *HTTPServer {
}
}
func setupHTTPServer(t *testing.T, useFakeAccessControl bool) accessControlScenarioContext {
return setupHTTPServerWithCfg(t, useFakeAccessControl, setting.NewCfg())
func setupHTTPServer(t *testing.T, useFakeAccessControl bool, options ...APITestServerOption) accessControlScenarioContext {
return setupHTTPServerWithCfg(t, useFakeAccessControl, setting.NewCfg(), options...)
}
func setupHTTPServerWithCfg(t *testing.T, useFakeAccessControl bool, cfg *setting.Cfg) accessControlScenarioContext {
func setupHTTPServerWithCfg(t *testing.T, useFakeAccessControl bool, cfg *setting.Cfg, options ...APITestServerOption) accessControlScenarioContext {
db := sqlstore.InitTestDB(t, sqlstore.InitTestDBOpt{})
return setupHTTPServerWithCfgDb(t, useFakeAccessControl, cfg, db, db, featuremgmt.WithFeatures())
return setupHTTPServerWithCfgDb(t, useFakeAccessControl, cfg, db, db, featuremgmt.WithFeatures(), options...)
}
func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl bool, cfg *setting.Cfg, db *sqlstore.SQLStore, store sqlstore.Store, features *featuremgmt.FeatureManager) accessControlScenarioContext {
func setupHTTPServerWithCfgDb(
t *testing.T, useFakeAccessControl bool, cfg *setting.Cfg, db *sqlstore.SQLStore,
store sqlstore.Store, features *featuremgmt.FeatureManager, options ...APITestServerOption,
) accessControlScenarioContext {
t.Helper()
db.Cfg.RBACEnabled = cfg.RBACEnabled
@@ -400,6 +405,10 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl bool, cfg *sett
userService: userMock,
}
for _, o := range options {
o(hs)
}
require.NoError(t, hs.declareFixedRoles())
require.NoError(t, hs.AccessControl.(accesscontrol.RoleRegistry).RegisterFixedRoles(context.Background()))
@@ -428,6 +437,7 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl bool, cfg *sett
db: db,
cfg: cfg,
dashboardsStore: dashboardsStore,
usermock: userMock,
}
}
pkg/api/org_test.go
+19 -25
View File
@@ -1,7 +1,6 @@
package api
import (
"context"
"fmt"
"net/http"
"strings"
@@ -10,10 +9,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
)
@@ -191,18 +188,17 @@ func TestAPIEndpoint_PutCurrentOrgAddress_AccessControl(t *testing.T) {
// `/api/orgs/` endpoints test
// setupOrgsDBForAccessControlTests stores users and create specified number of orgs
func setupOrgsDBForAccessControlTests(t *testing.T, db sqlstore.Store, usr user.SignedInUser, orgsCount int) {
// setupOrgsDBForAccessControlTests creates orgs up until orgID and fake user as member of org
func setupOrgsDBForAccessControlTests(t *testing.T, db sqlstore.Store, c accessControlScenarioContext, orgID int64) {
t.Helper()
_, err := db.CreateUser(context.Background(), user.CreateUserCommand{Email: usr.Email, SkipOrgSetup: true, Login: usr.Login})
require.NoError(t, err)
setInitCtxSignedInViewer(c.initCtx)
u := *c.initCtx.SignedInUser
u.OrgID = orgID
c.usermock.ExpectedSignedInUser = &u
// Create `orgsCount` orgs
for i := 1; i <= orgsCount; i++ {
_, err = db.CreateOrgWithMember(fmt.Sprintf("TestOrg%v", i), 0)
require.NoError(t, err)
err = db.AddOrgUser(context.Background(), &models.AddOrgUserCommand{LoginOrEmail: usr.Login, Role: usr.OrgRole, OrgId: int64(i), UserId: usr.UserID})
for i := 1; i <= int(orgID); i++ {
_, err := db.CreateOrgWithMember(fmt.Sprintf("TestOrg%v", i), 0)
require.NoError(t, err)
}
}
@@ -240,7 +236,7 @@ func TestAPIEndpoint_CreateOrgs_AccessControl(t *testing.T) {
sc := setupHTTPServer(t, true)
setInitCtxSignedInViewer(sc.initCtx)
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 0)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 0)
input := strings.NewReader(fmt.Sprintf(testCreateOrgCmd, 2))
t.Run("AccessControl allows creating Orgs with correct permissions", func(t *testing.T) {
@@ -263,7 +259,7 @@ func TestAPIEndpoint_DeleteOrgs_LegacyAccessControl(t *testing.T) {
sc := setupHTTPServerWithCfg(t, true, cfg)
setInitCtxSignedInViewer(sc.initCtx)
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
t.Run("Viewer cannot delete Orgs", func(t *testing.T) {
response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(deleteOrgsURL, 2), nil, t)
@@ -279,9 +275,7 @@ func TestAPIEndpoint_DeleteOrgs_LegacyAccessControl(t *testing.T) {
func TestAPIEndpoint_DeleteOrgs_AccessControl(t *testing.T) {
sc := setupHTTPServer(t, true)
setInitCtxSignedInViewer(sc.initCtx)
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
t.Run("AccessControl prevents deleting Orgs with incorrect permissions", func(t *testing.T) {
setAccessControlPermissions(sc.acmock, []accesscontrol.Permission{{Action: "orgs:invalid"}}, 2)
@@ -346,7 +340,7 @@ func TestAPIEndpoint_GetOrg_LegacyAccessControl(t *testing.T) {
setInitCtxSignedInViewer(sc.initCtx)
// Create two orgs, to fetch another one than the logged in one
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
t.Run("Viewer cannot view another Org", func(t *testing.T) {
response := callAPI(sc.server, http.MethodGet, fmt.Sprintf(getOrgsURL, 2), nil, t)
@@ -365,7 +359,7 @@ func TestAPIEndpoint_GetOrg_AccessControl(t *testing.T) {
setInitCtxSignedInViewer(sc.initCtx)
// Create two orgs, to fetch another one than the logged in one
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
t.Run("AccessControl allows viewing another org with correct permissions", func(t *testing.T) {
setAccessControlPermissions(sc.acmock, []accesscontrol.Permission{{Action: ActionOrgsRead}}, 2)
@@ -391,7 +385,7 @@ func TestAPIEndpoint_GetOrgByName_LegacyAccessControl(t *testing.T) {
setInitCtxSignedInViewer(sc.initCtx)
// Create two orgs, to fetch another one than the logged in one
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
t.Run("Viewer cannot view another Org", func(t *testing.T) {
response := callAPI(sc.server, http.MethodGet, fmt.Sprintf(getOrgsByNameURL, "TestOrg2"), nil, t)
@@ -410,7 +404,7 @@ func TestAPIEndpoint_GetOrgByName_AccessControl(t *testing.T) {
setInitCtxSignedInViewer(sc.initCtx)
// Create two orgs, to fetch another one than the logged in one
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
t.Run("AccessControl allows viewing another org with correct permissions", func(t *testing.T) {
setAccessControlPermissions(sc.acmock, []accesscontrol.Permission{{Action: ActionOrgsRead}}, accesscontrol.GlobalOrgID)
@@ -431,7 +425,7 @@ func TestAPIEndpoint_PutOrg_LegacyAccessControl(t *testing.T) {
setInitCtxSignedInViewer(sc.initCtx)
// Create two orgs, to update another one than the logged in one
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
input := strings.NewReader(testUpdateOrgNameForm)
@@ -452,7 +446,7 @@ func TestAPIEndpoint_PutOrg_AccessControl(t *testing.T) {
setInitCtxSignedInViewer(sc.initCtx)
// Create two orgs, to update another one than the logged in one
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
input := strings.NewReader(testUpdateOrgNameForm)
t.Run("AccessControl allows updating another org with correct permissions", func(t *testing.T) {
@@ -481,7 +475,7 @@ func TestAPIEndpoint_PutOrgAddress_LegacyAccessControl(t *testing.T) {
setInitCtxSignedInViewer(sc.initCtx)
// Create two orgs, to update another one than the logged in one
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
input := strings.NewReader(testUpdateOrgAddressForm)
@@ -502,7 +496,7 @@ func TestAPIEndpoint_PutOrgAddress_AccessControl(t *testing.T) {
setInitCtxSignedInViewer(sc.initCtx)
// Create two orgs, to update another one than the logged in one
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
input := strings.NewReader(testUpdateOrgAddressForm)
t.Run("AccessControl allows updating another org address with correct permissions", func(t *testing.T) {
pkg/api/org_users_test.go
+38 -17
View File
@@ -21,7 +21,7 @@ import (
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/services/user/userimpl"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
)
@@ -349,7 +349,12 @@ func TestGetOrgUsersAPIEndpoint_AccessControlMetadata(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
cfg := setting.NewCfg()
cfg.RBACEnabled = tc.enableAccessControl
sc := setupHTTPServerWithCfg(t, false, cfg)
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
hs.userService = userimpl.ProvideService(
hs.SQLStore, nil, nil, nil, nil,
nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore),
)
})
setupOrgUsersDBForAccessControlTests(t, sc.db)
setInitCtxSignedInUser(sc.initCtx, tc.user)
@@ -448,9 +453,14 @@ func TestGetOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
cfg := setting.NewCfg()
cfg.RBACEnabled = tc.enableAccessControl
sc := setupHTTPServerWithCfg(t, false, cfg)
setupOrgUsersDBForAccessControlTests(t, sc.db)
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
hs.userService = userimpl.ProvideService(
hs.SQLStore, nil, nil, nil, nil,
nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore),
)
})
setInitCtxSignedInUser(sc.initCtx, tc.user)
setupOrgUsersDBForAccessControlTests(t, sc.db)
// Perform test
response := callAPI(sc.server, http.MethodGet, fmt.Sprintf(url, tc.targetOrg), nil, t)
@@ -548,13 +558,12 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
cfg := setting.NewCfg()
cfg.RBACEnabled = tc.enableAccessControl
sc := setupHTTPServerWithCfg(t, false, cfg)
userService := usertest.NewUserServiceFake()
userService.ExpectedUser = &user.User{ID: 2}
sc.hs.userService = userService
mockStore := mockstore.NewSQLStoreMock()
mockStore.ExpectedUser = &user.User{ID: 2}
sc.hs.SQLStore = mockStore
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
hs.userService = userimpl.ProvideService(
hs.SQLStore, nil, nil, nil, nil,
nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore),
)
})
setupOrgUsersDBForAccessControlTests(t, sc.db)
setInitCtxSignedInUser(sc.initCtx, tc.user)
@@ -674,10 +683,12 @@ func TestOrgUsersAPIEndpointWithSetPerms_AccessControl(t *testing.T) {
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
sc := setupHTTPServer(t, true)
userService := usertest.NewUserServiceFake()
userService.ExpectedUser = &user.User{ID: 2}
sc.hs.userService = userService
sc := setupHTTPServer(t, true, func(hs *HTTPServer) {
hs.userService = userimpl.ProvideService(
hs.SQLStore, nil, nil, nil, nil,
nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore),
)
})
setInitCtxSignedInViewer(sc.initCtx)
setupOrgUsersDBForAccessControlTests(t, sc.db)
setAccessControlPermissions(sc.acmock, test.permissions, sc.initCtx.OrgID)
@@ -791,7 +802,12 @@ func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
cfg := setting.NewCfg()
cfg.RBACEnabled = tc.enableAccessControl
sc := setupHTTPServerWithCfg(t, false, cfg)
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
hs.userService = userimpl.ProvideService(
hs.SQLStore, nil, nil, nil, nil,
nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore),
)
})
setupOrgUsersDBForAccessControlTests(t, sc.db)
setInitCtxSignedInUser(sc.initCtx, tc.user)
@@ -913,7 +929,12 @@ func TestDeleteOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
cfg := setting.NewCfg()
cfg.RBACEnabled = tc.enableAccessControl
sc := setupHTTPServerWithCfg(t, false, cfg)
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
hs.userService = userimpl.ProvideService(
hs.SQLStore, nil, nil, nil, nil,
nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore),
)
})
setupOrgUsersDBForAccessControlTests(t, sc.db)
setInitCtxSignedInUser(sc.initCtx, tc.user)
pkg/api/quota_test.go
+1 -1
View File
@@ -38,7 +38,7 @@ func setupDBAndSettingsForAccessControlQuotaTests(t *testing.T, sc accessControl
setting.Quota = sc.hs.Cfg.Quota
// Create two orgs with the context user
setupOrgsDBForAccessControlTests(t, sc.db, *sc.initCtx.SignedInUser, 2)
setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
}
func TestAPIEndpoint_GetCurrentOrgQuotas_LegacyAccessControl(t *testing.T) {