Skip to content

Commit

Permalink
SQLStore: Make addOrgUser private (#59142)
Browse files Browse the repository at this point in the history
make addOrgUser private
  • Loading branch information
zserge committed Nov 22, 2022
1 parent de99ce1 commit b875ca0
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 25 deletions.
2 changes: 1 addition & 1 deletion pkg/api/org_invite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestOrgInvitesAPIEndpointAccess(t *testing.T) {
userService.ExpectedUser = &user.User{ID: 2}
sc.hs.userService = userService
setInitCtxSignedInViewer(sc.initCtx)
setupOrgUsersDBForAccessControlTests(t, sc.db)
setupOrgUsersDBForAccessControlTests(t, sc.db, sc.hs.orgService)
setAccessControlPermissions(sc.acmock, test.permissions, sc.initCtx.OrgID)

input := strings.NewReader(test.input)
Expand Down
22 changes: 12 additions & 10 deletions pkg/api/org_users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ var (
// setupOrgUsersDBForAccessControlTests creates three users placed in two orgs
// Org1: testServerAdminViewer, testEditorOrg1
// Org2: testServerAdminViewer, testAdminOrg2
func setupOrgUsersDBForAccessControlTests(t *testing.T, db *sqlstore.SQLStore) {
func setupOrgUsersDBForAccessControlTests(t *testing.T, db *sqlstore.SQLStore, orgService org.Service) {
t.Helper()

var err error
Expand All @@ -346,9 +346,9 @@ func setupOrgUsersDBForAccessControlTests(t *testing.T, db *sqlstore.SQLStore) {
_, err = db.CreateOrgWithMember(testAdminOrg2.OrgName, testServerAdminViewer.UserID)
require.NoError(t, err)

err = db.AddOrgUser(context.Background(), &models.AddOrgUserCommand{LoginOrEmail: testAdminOrg2.Login, Role: testAdminOrg2.OrgRole, OrgId: testAdminOrg2.OrgID, UserId: testAdminOrg2.UserID})
err = orgService.AddOrgUser(context.Background(), &org.AddOrgUserCommand{LoginOrEmail: testAdminOrg2.Login, Role: testAdminOrg2.OrgRole, OrgID: testAdminOrg2.OrgID, UserID: testAdminOrg2.UserID})
require.NoError(t, err)
err = db.AddOrgUser(context.Background(), &models.AddOrgUserCommand{LoginOrEmail: testEditorOrg1.Login, Role: testEditorOrg1.OrgRole, OrgId: testEditorOrg1.OrgID, UserId: testEditorOrg1.UserID})
err = orgService.AddOrgUser(context.Background(), &org.AddOrgUserCommand{LoginOrEmail: testEditorOrg1.Login, Role: testEditorOrg1.OrgRole, OrgID: testEditorOrg1.OrgID, UserID: testEditorOrg1.UserID})
require.NoError(t, err)
}

Expand Down Expand Up @@ -398,7 +398,7 @@ func TestGetOrgUsersAPIEndpoint_AccessControlMetadata(t *testing.T) {
hs.orgService, err = orgimpl.ProvideService(hs.SQLStore, cfg, quotatest.New(false, nil))
require.NoError(t, err)
})
setupOrgUsersDBForAccessControlTests(t, sc.db)
setupOrgUsersDBForAccessControlTests(t, sc.db, sc.hs.orgService)
setInitCtxSignedInUser(sc.initCtx, tc.user)

// Perform test
Expand Down Expand Up @@ -506,7 +506,7 @@ func TestGetOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
require.NoError(t, err)
})
setInitCtxSignedInUser(sc.initCtx, tc.user)
setupOrgUsersDBForAccessControlTests(t, sc.db)
setupOrgUsersDBForAccessControlTests(t, sc.db, sc.hs.orgService)

// Perform test
response := callAPI(sc.server, http.MethodGet, fmt.Sprintf(url, tc.targetOrg), nil, t)
Expand Down Expand Up @@ -606,12 +606,14 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
cfg.RBACEnabled = tc.enableAccessControl
var err error
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
hs.orgService, err = orgimpl.ProvideService(hs.SQLStore, cfg, quotatest.New(false, nil))
require.NoError(t, err)
hs.userService, err = userimpl.ProvideService(
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), quotatest.New(false, nil))
hs.SQLStore, hs.orgService, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), quotatest.New(false, nil))
require.NoError(t, err)
})

setupOrgUsersDBForAccessControlTests(t, sc.db)
setupOrgUsersDBForAccessControlTests(t, sc.db, sc.hs.orgService)
setInitCtxSignedInUser(sc.initCtx, tc.user)

// Perform request
Expand Down Expand Up @@ -731,7 +733,7 @@ func TestOrgUsersAPIEndpointWithSetPerms_AccessControl(t *testing.T) {
require.NoError(t, err)
})
setInitCtxSignedInViewer(sc.initCtx)
setupOrgUsersDBForAccessControlTests(t, sc.db)
setupOrgUsersDBForAccessControlTests(t, sc.db, sc.hs.orgService)
setAccessControlPermissions(sc.acmock, test.permissions, sc.initCtx.OrgID)

input := strings.NewReader(test.input)
Expand Down Expand Up @@ -852,7 +854,7 @@ func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
hs.orgService, err = orgimpl.ProvideService(hs.SQLStore, cfg, quotaService)
require.NoError(t, err)
})
setupOrgUsersDBForAccessControlTests(t, sc.db)
setupOrgUsersDBForAccessControlTests(t, sc.db, sc.hs.orgService)
setInitCtxSignedInUser(sc.initCtx, tc.user)

// Perform request
Expand Down Expand Up @@ -982,7 +984,7 @@ func TestDeleteOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
hs.orgService, err = orgimpl.ProvideService(hs.SQLStore, cfg, quotaService)
require.NoError(t, err)
})
setupOrgUsersDBForAccessControlTests(t, sc.db)
setupOrgUsersDBForAccessControlTests(t, sc.db, sc.hs.orgService)
setInitCtxSignedInUser(sc.initCtx, tc.user)

response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(url, tc.targetOrg, tc.targetUserId), nil, t)
Expand Down
4 changes: 2 additions & 2 deletions pkg/services/sqlstore/org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestIntegrationAccountDataAccess(t *testing.T) {
Role: org.RoleViewer,
}

err := sqlStore.AddOrgUser(context.Background(), &cmd)
err := sqlStore.addOrgUser(context.Background(), &cmd)
t.Run("Should have been saved without error", func(t *testing.T) {
require.NoError(t, err)
})
Expand Down Expand Up @@ -142,7 +142,7 @@ func TestIntegrationAccountDataAccess(t *testing.T) {
Role: org.RoleViewer,
}

err = sqlStore.AddOrgUser(context.Background(), &orgUserCmd)
err = sqlStore.addOrgUser(context.Background(), &orgUserCmd)
require.NoError(t, err)

dash1 := insertTestDashboard(t, sqlStore, "1 test dash", ac1.OrgID, 0, false, "prod", "webapp")
Expand Down
2 changes: 1 addition & 1 deletion pkg/services/sqlstore/org_users.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/grafana/grafana/pkg/services/user"
)

func (ss *SQLStore) AddOrgUser(ctx context.Context, cmd *models.AddOrgUserCommand) error {
func (ss *SQLStore) addOrgUser(ctx context.Context, cmd *models.AddOrgUserCommand) error {
return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
// check if user exists
var usr user.User
Expand Down
4 changes: 2 additions & 2 deletions pkg/services/sqlstore/org_users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ func TestSQLStore_AddOrgUser(t *testing.T) {
require.Equal(t, int64(-1), sa.OrgID)

// assign the sa to the org but without the override. should fail
err = store.AddOrgUser(context.Background(), &models.AddOrgUserCommand{
err = store.addOrgUser(context.Background(), &models.AddOrgUserCommand{
Role: "Viewer",
OrgId: orgID,
UserId: sa.ID,
})
require.Error(t, err)

// assign the sa to the org with the override. should succeed
err = store.AddOrgUser(context.Background(), &models.AddOrgUserCommand{
err = store.addOrgUser(context.Background(), &models.AddOrgUserCommand{
Role: "Viewer",
OrgId: orgID,
UserId: sa.ID,
Expand Down
6 changes: 3 additions & 3 deletions pkg/services/sqlstore/stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func populateDB(t *testing.T, sqlStore *SQLStore) {
UserId: users[1].ID,
Role: org.RoleEditor,
}
err := sqlStore.AddOrgUser(context.Background(), cmd)
err := sqlStore.addOrgUser(context.Background(), cmd)
require.NoError(t, err)

// add 3rd user as viewer
Expand All @@ -95,7 +95,7 @@ func populateDB(t *testing.T, sqlStore *SQLStore) {
UserId: users[2].ID,
Role: org.RoleViewer,
}
err = sqlStore.AddOrgUser(context.Background(), cmd)
err = sqlStore.addOrgUser(context.Background(), cmd)
require.NoError(t, err)

// add 1st user as admin
Expand All @@ -104,7 +104,7 @@ func populateDB(t *testing.T, sqlStore *SQLStore) {
UserId: users[0].ID,
Role: org.RoleAdmin,
}
err = sqlStore.AddOrgUser(context.Background(), cmd)
err = sqlStore.addOrgUser(context.Background(), cmd)
require.NoError(t, err)

// force renewal of user stats
Expand Down
17 changes: 11 additions & 6 deletions pkg/services/user/userimpl/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgimpl"
"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/user"
Expand All @@ -25,6 +27,9 @@ func TestIntegrationUserDataAccess(t *testing.T) {
}

ss := db.InitTestDB(t)
quotaService := quotaimpl.ProvideService(ss, ss.Cfg)
orgService, err := orgimpl.ProvideService(ss, ss.Cfg, quotaService)
require.NoError(t, err)
userStore := ProvideStore(ss, setting.NewCfg())
usr := &user.SignedInUser{
OrgID: 1,
Expand Down Expand Up @@ -265,9 +270,9 @@ func TestIntegrationUserDataAccess(t *testing.T) {
IsDisabled: false,
}
})
err := ss.AddOrgUser(context.Background(), &models.AddOrgUserCommand{
err := orgService.AddOrgUser(context.Background(), &org.AddOrgUserCommand{
LoginOrEmail: users[1].Login, Role: org.RoleViewer,
OrgId: users[0].OrgID, UserId: users[1].ID,
OrgID: users[0].OrgID, UserID: users[1].ID,
})
require.Nil(t, err)

Expand Down Expand Up @@ -402,9 +407,9 @@ func TestIntegrationUserDataAccess(t *testing.T) {
}
})

err = ss.AddOrgUser(context.Background(), &models.AddOrgUserCommand{
err = orgService.AddOrgUser(context.Background(), &org.AddOrgUserCommand{
LoginOrEmail: users[1].Login, Role: org.RoleViewer,
OrgId: users[0].OrgID, UserId: users[1].ID,
OrgID: users[0].OrgID, UserID: users[1].ID,
})
require.Nil(t, err)

Expand Down Expand Up @@ -441,9 +446,9 @@ func TestIntegrationUserDataAccess(t *testing.T) {
IsDisabled: false,
}
})
err = ss.AddOrgUser(context.Background(), &models.AddOrgUserCommand{
err = orgService.AddOrgUser(context.Background(), &org.AddOrgUserCommand{
LoginOrEmail: users[1].Login, Role: org.RoleViewer,
OrgId: users[0].OrgID, UserId: users[1].ID,
OrgID: users[0].OrgID, UserID: users[1].ID,
})
require.Nil(t, err)

Expand Down

0 comments on commit b875ca0

Please sign in to comment.