Chore: Move methods from sqlstore to user store (#56305)

* Copy sqlstore methods to suer store

* Adjust ProvideService signatures in test

* Add xorm tags and tests for search

* Remove methods from sqlstore

* fix lint in tests
This commit is contained in:
idafurjes
2022-10-05 09:34:36 +02:00
committed by GitHub
parent 580ca144fd
commit 5167c55760
15 changed files with 670 additions and 656 deletions
-101
View File
@@ -10,7 +10,6 @@ import (
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards"
dashver "github.com/grafana/grafana/pkg/services/dashboardversion"
"github.com/grafana/grafana/pkg/services/org"
@@ -129,21 +128,6 @@ func TestIntegrationAccountDataAccess(t *testing.T) {
require.Equal(t, query.Result.Login, "ac1")
})
t.Run("Can search users", func(t *testing.T) {
query := models.SearchUsersQuery{Query: "", SignedInUser: &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{
1: {accesscontrol.ActionUsersRead: {accesscontrol.ScopeGlobalUsersAll}},
},
}}
err := sqlStore.SearchUsers(context.Background(), &query)
require.NoError(t, err)
require.Len(t, query.Result.Users, 2)
require.Equal(t, query.Result.Users[0].Email, "ac1@test.com")
require.Equal(t, query.Result.Users[1].Email, "ac2@test.com")
})
t.Run("Given an added org user", func(t *testing.T) {
cmd := models.AddOrgUserCommand{
OrgId: ac1.OrgID,
@@ -359,88 +343,3 @@ func updateDashboardACL(t *testing.T, sqlStore *SQLStore, dashboardID int64, ite
})
return err
}
// This function was copied from pkg/services/dashboards/database to circumvent
// import cycles. When this org-related code is refactored into a service the
// tests can the real GetDashboardACLInfoList functions
func getDashboardACLInfoList(s *SQLStore, query *models.GetDashboardACLInfoListQuery) error {
outerErr := s.WithDbSession(context.Background(), func(dbSession *DBSession) error {
query.Result = make([]*models.DashboardACLInfoDTO, 0)
falseStr := dialect.BooleanStr(false)
if query.DashboardID == 0 {
sql := `SELECT
da.id,
da.org_id,
da.dashboard_id,
da.user_id,
da.team_id,
da.permission,
da.role,
da.created,
da.updated,
'' as user_login,
'' as user_email,
'' as team,
'' as title,
'' as slug,
'' as uid,` +
falseStr + ` AS is_folder,` +
falseStr + ` AS inherited
FROM dashboard_acl as da
WHERE da.dashboard_id = -1`
return dbSession.SQL(sql).Find(&query.Result)
}
rawSQL := `
-- get permissions for the dashboard and its parent folder
SELECT
da.id,
da.org_id,
da.dashboard_id,
da.user_id,
da.team_id,
da.permission,
da.role,
da.created,
da.updated,
u.login AS user_login,
u.email AS user_email,
ug.name AS team,
ug.email AS team_email,
d.title,
d.slug,
d.uid,
d.is_folder,
CASE WHEN (da.dashboard_id = -1 AND d.folder_id > 0) OR da.dashboard_id = d.folder_id THEN ` + dialect.BooleanStr(true) + ` ELSE ` + falseStr + ` END AS inherited
FROM dashboard as d
LEFT JOIN dashboard folder on folder.id = d.folder_id
LEFT JOIN dashboard_acl AS da ON
da.dashboard_id = d.id OR
da.dashboard_id = d.folder_id OR
(
-- include default permissions -->
da.org_id = -1 AND (
(folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR
(folder.id IS NULL AND d.has_acl = ` + falseStr + `)
)
)
LEFT JOIN ` + dialect.Quote("user") + ` AS u ON u.id = da.user_id
LEFT JOIN team ug on ug.id = da.team_id
WHERE d.org_id = ? AND d.id = ? AND da.id IS NOT NULL
ORDER BY da.id ASC
`
return dbSession.SQL(rawSQL, query.OrgID, query.DashboardID).Find(&query.Result)
})
if outerErr != nil {
return outerErr
}
for _, p := range query.Result {
p.PermissionName = p.Permission.String()
}
return nil
}
-155
View File
@@ -463,161 +463,6 @@ func getTeamSelectSQLBase(filteredUsers []string) string {
` FROM team as team `
}
func (ss *SQLStore) SearchUsers(ctx context.Context, query *models.SearchUsersQuery) error {
return ss.WithDbSession(ctx, func(dbSess *DBSession) error {
query.Result = models.SearchUserQueryResult{
Users: make([]*models.UserSearchHitDTO, 0),
}
queryWithWildcards := "%" + query.Query + "%"
whereConditions := make([]string, 0)
whereParams := make([]interface{}, 0)
sess := dbSess.Table("user").Alias("u")
whereConditions = append(whereConditions, "u.is_service_account = ?")
whereParams = append(whereParams, dialect.BooleanStr(false))
// Join with only most recent auth module
joinCondition := `(
SELECT id from user_auth
WHERE user_auth.user_id = u.id
ORDER BY user_auth.created DESC `
joinCondition = "user_auth.id=" + joinCondition + dialect.Limit(1) + ")"
sess.Join("LEFT", "user_auth", joinCondition)
if query.OrgId > 0 {
whereConditions = append(whereConditions, "org_id = ?")
whereParams = append(whereParams, query.OrgId)
}
// user only sees the users for which it has read permissions
if !ac.IsDisabled(ss.Cfg) {
acFilter, err := ac.Filter(query.SignedInUser, "u.id", "global.users:id:", ac.ActionUsersRead)
if err != nil {
return err
}
whereConditions = append(whereConditions, acFilter.Where)
whereParams = append(whereParams, acFilter.Args...)
}
if query.Query != "" {
whereConditions = append(whereConditions, "(email "+dialect.LikeStr()+" ? OR name "+dialect.LikeStr()+" ? OR login "+dialect.LikeStr()+" ?)")
whereParams = append(whereParams, queryWithWildcards, queryWithWildcards, queryWithWildcards)
}
if query.IsDisabled != nil {
whereConditions = append(whereConditions, "is_disabled = ?")
whereParams = append(whereParams, query.IsDisabled)
}
if query.AuthModule != "" {
whereConditions = append(whereConditions, `auth_module=?`)
whereParams = append(whereParams, query.AuthModule)
}
if len(whereConditions) > 0 {
sess.Where(strings.Join(whereConditions, " AND "), whereParams...)
}
for _, filter := range query.Filters {
if jc := filter.JoinCondition(); jc != nil {
sess.Join(jc.Operator, jc.Table, jc.Params)
}
if ic := filter.InCondition(); ic != nil {
sess.In(ic.Condition, ic.Params)
}
if wc := filter.WhereCondition(); wc != nil {
sess.Where(wc.Condition, wc.Params)
}
}
if query.Limit > 0 {
offset := query.Limit * (query.Page - 1)
sess.Limit(query.Limit, offset)
}
sess.Cols("u.id", "u.email", "u.name", "u.login", "u.is_admin", "u.is_disabled", "u.last_seen_at", "user_auth.auth_module")
sess.Asc("u.login", "u.email")
if err := sess.Find(&query.Result.Users); err != nil {
return err
}
// get total
user := user.User{}
countSess := dbSess.Table("user").Alias("u")
// Join with user_auth table if users filtered by auth_module
if query.AuthModule != "" {
countSess.Join("LEFT", "user_auth", joinCondition)
}
if len(whereConditions) > 0 {
countSess.Where(strings.Join(whereConditions, " AND "), whereParams...)
}
for _, filter := range query.Filters {
if jc := filter.JoinCondition(); jc != nil {
countSess.Join(jc.Operator, jc.Table, jc.Params)
}
if ic := filter.InCondition(); ic != nil {
countSess.In(ic.Condition, ic.Params)
}
if wc := filter.WhereCondition(); wc != nil {
countSess.Where(wc.Condition, wc.Params)
}
}
count, err := countSess.Count(&user)
query.Result.TotalCount = count
for _, user := range query.Result.Users {
user.LastSeenAtAge = util.GetAgeString(user.LastSeenAt)
}
return err
})
}
func (ss *SQLStore) DisableUser(ctx context.Context, cmd *models.DisableUserCommand) error {
return ss.WithDbSession(ctx, func(dbSess *DBSession) error {
usr := user.User{}
sess := dbSess.Table("user")
if has, err := sess.ID(cmd.UserId).Where(NotServiceAccountFilter(ss)).Get(&usr); err != nil {
return err
} else if !has {
return user.ErrUserNotFound
}
usr.IsDisabled = cmd.IsDisabled
sess.UseBool("is_disabled")
_, err := sess.ID(cmd.UserId).Update(&usr)
return err
})
}
func (ss *SQLStore) BatchDisableUsers(ctx context.Context, cmd *models.BatchDisableUsersCommand) error {
return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
userIds := cmd.UserIds
if len(userIds) == 0 {
return nil
}
user_id_params := strings.Repeat(",?", len(userIds)-1)
disableSQL := "UPDATE " + dialect.Quote("user") + " SET is_disabled=? WHERE Id IN (?" + user_id_params + ")"
disableParams := []interface{}{disableSQL, cmd.IsDisabled}
for _, v := range userIds {
disableParams = append(disableParams, v)
}
_, err := sess.Where(NotServiceAccountFilter(ss)).Exec(disableParams...)
return err
})
}
func (ss *SQLStore) DeleteUser(ctx context.Context, cmd *models.DeleteUserCommand) error {
return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
return deleteUserInTransaction(ss, sess, cmd)
-321
View File
@@ -6,9 +6,7 @@ import (
"testing"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -17,10 +15,6 @@ func TestIntegrationUserDataAccess(t *testing.T) {
t.Skip("skipping integration test")
}
ss := InitTestDB(t)
usr := &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{1: {"users:read": {"global.users:*"}}},
}
t.Run("Testing DB - creates and loads disabled user", func(t *testing.T) {
ss = InitTestDB(t)
@@ -91,321 +85,6 @@ func TestIntegrationUserDataAccess(t *testing.T) {
require.Equal(t, err, models.ErrOrgNotFound)
})
t.Run("Testing DB - multiple users", func(t *testing.T) {
ss = InitTestDB(t)
createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand {
return &user.CreateUserCommand{
Email: fmt.Sprint("user", i, "@test.com"),
Name: fmt.Sprint("user", i),
Login: fmt.Sprint("loginuser", i),
IsDisabled: false,
}
})
// Return the first page of users and a total count
query := models.SearchUsersQuery{Query: "", Page: 1, Limit: 3, SignedInUser: usr}
err := ss.SearchUsers(context.Background(), &query)
require.Nil(t, err)
require.Len(t, query.Result.Users, 3)
require.EqualValues(t, query.Result.TotalCount, 5)
// Return the second page of users and a total count
query = models.SearchUsersQuery{Query: "", Page: 2, Limit: 3, SignedInUser: usr}
err = ss.SearchUsers(context.Background(), &query)
require.Nil(t, err)
require.Len(t, query.Result.Users, 2)
require.EqualValues(t, query.Result.TotalCount, 5)
// Return list of users matching query on user name
query = models.SearchUsersQuery{Query: "use", Page: 1, Limit: 3, SignedInUser: usr}
err = ss.SearchUsers(context.Background(), &query)
require.Nil(t, err)
require.Len(t, query.Result.Users, 3)
require.EqualValues(t, query.Result.TotalCount, 5)
query = models.SearchUsersQuery{Query: "ser1", Page: 1, Limit: 3, SignedInUser: usr}
err = ss.SearchUsers(context.Background(), &query)
require.Nil(t, err)
require.Len(t, query.Result.Users, 1)
require.EqualValues(t, query.Result.TotalCount, 1)
query = models.SearchUsersQuery{Query: "USER1", Page: 1, Limit: 3, SignedInUser: usr}
err = ss.SearchUsers(context.Background(), &query)
require.Nil(t, err)
require.Len(t, query.Result.Users, 1)
require.EqualValues(t, query.Result.TotalCount, 1)
query = models.SearchUsersQuery{Query: "idontexist", Page: 1, Limit: 3, SignedInUser: usr}
err = ss.SearchUsers(context.Background(), &query)
require.Nil(t, err)
require.Len(t, query.Result.Users, 0)
require.EqualValues(t, query.Result.TotalCount, 0)
// Return list of users matching query on email
query = models.SearchUsersQuery{Query: "ser1@test.com", Page: 1, Limit: 3, SignedInUser: usr}
err = ss.SearchUsers(context.Background(), &query)
require.Nil(t, err)
require.Len(t, query.Result.Users, 1)
require.EqualValues(t, query.Result.TotalCount, 1)
// Return list of users matching query on login name
query = models.SearchUsersQuery{Query: "loginuser1", Page: 1, Limit: 3, SignedInUser: usr}
err = ss.SearchUsers(context.Background(), &query)
require.Nil(t, err)
require.Len(t, query.Result.Users, 1)
require.EqualValues(t, query.Result.TotalCount, 1)
})
t.Run("Testing DB - return list users based on their is_disabled flag", func(t *testing.T) {
ss = InitTestDB(t)
createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand {
return &user.CreateUserCommand{
Email: fmt.Sprint("user", i, "@test.com"),
Name: fmt.Sprint("user", i),
Login: fmt.Sprint("loginuser", i),
IsDisabled: i%2 == 0,
}
})
isDisabled := false
query := models.SearchUsersQuery{IsDisabled: &isDisabled, SignedInUser: usr}
err := ss.SearchUsers(context.Background(), &query)
require.Nil(t, err)
require.Len(t, query.Result.Users, 2)
first, third := false, false
for _, user := range query.Result.Users {
if user.Name == "user1" {
first = true
}
if user.Name == "user3" {
third = true
}
}
require.True(t, first)
require.True(t, third)
// Re-init DB
ss = InitTestDB(t)
users := createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand {
return &user.CreateUserCommand{
Email: fmt.Sprint("user", i, "@test.com"),
Name: fmt.Sprint("user", i),
Login: fmt.Sprint("loginuser", i),
IsDisabled: false,
}
})
err = ss.AddOrgUser(context.Background(), &models.AddOrgUserCommand{
LoginOrEmail: users[1].Login, Role: org.RoleViewer,
OrgId: users[0].OrgID, UserId: users[1].ID,
})
require.Nil(t, err)
err = updateDashboardACL(t, ss, 1, &models.DashboardACL{
DashboardID: 1, OrgID: users[0].OrgID, UserID: users[1].ID,
Permission: models.PERMISSION_EDIT,
})
require.Nil(t, err)
// When the user is deleted
err = ss.DeleteUser(context.Background(), &models.DeleteUserCommand{UserId: users[1].ID})
require.Nil(t, err)
query1 := &models.GetOrgUsersQuery{OrgId: users[0].OrgID, User: usr}
err = ss.GetOrgUsersForTest(context.Background(), query1)
require.Nil(t, err)
require.Len(t, query1.Result, 1)
permQuery := &models.GetDashboardACLInfoListQuery{DashboardID: 1, OrgID: users[0].OrgID}
err = getDashboardACLInfoList(ss, permQuery)
require.Nil(t, err)
require.Len(t, permQuery.Result, 0)
// A user is an org member and has been assigned permissions
// Re-init DB
ss = InitTestDB(t)
users = createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand {
return &user.CreateUserCommand{
Email: fmt.Sprint("user", i, "@test.com"),
Name: fmt.Sprint("user", i),
Login: fmt.Sprint("loginuser", i),
IsDisabled: false,
}
})
err = ss.AddOrgUser(context.Background(), &models.AddOrgUserCommand{
LoginOrEmail: users[1].Login, Role: org.RoleViewer,
OrgId: users[0].OrgID, UserId: users[1].ID,
})
require.Nil(t, err)
err = updateDashboardACL(t, ss, 1, &models.DashboardACL{
DashboardID: 1, OrgID: users[0].OrgID, UserID: users[1].ID,
Permission: models.PERMISSION_EDIT,
})
require.Nil(t, err)
ss.CacheService.Flush()
query3 := &models.GetSignedInUserQuery{OrgId: users[1].OrgID, UserId: users[1].ID}
err = ss.GetSignedInUserWithCacheCtx(context.Background(), query3)
require.Nil(t, err)
require.NotNil(t, query3.Result)
require.Equal(t, query3.OrgId, users[1].OrgID)
err = ss.SetUsingOrg(context.Background(), &models.SetUsingOrgCommand{UserId: users[1].ID, OrgId: users[0].OrgID})
require.Nil(t, err)
query4 := &models.GetSignedInUserQuery{OrgId: 0, UserId: users[1].ID}
err = ss.GetSignedInUserWithCacheCtx(context.Background(), query4)
require.Nil(t, err)
require.NotNil(t, query4.Result)
require.Equal(t, query4.Result.OrgID, users[0].OrgID)
cacheKey := newSignedInUserCacheKey(query4.Result.OrgID, query4.UserId)
_, found := ss.CacheService.Get(cacheKey)
require.True(t, found)
disableCmd := models.BatchDisableUsersCommand{
UserIds: []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID, users[4].ID},
IsDisabled: true,
}
err = ss.BatchDisableUsers(context.Background(), &disableCmd)
require.Nil(t, err)
isDisabled = true
query5 := &models.SearchUsersQuery{IsDisabled: &isDisabled, SignedInUser: usr}
err = ss.SearchUsers(context.Background(), query5)
require.Nil(t, err)
require.EqualValues(t, query5.Result.TotalCount, 5)
// the user is deleted
err = ss.DeleteUser(context.Background(), &models.DeleteUserCommand{UserId: users[1].ID})
require.Nil(t, err)
// delete connected org users and permissions
query2 := &models.GetOrgUsersQuery{OrgId: users[0].OrgID}
err = ss.GetOrgUsersForTest(context.Background(), query2)
require.Nil(t, err)
require.Len(t, query2.Result, 1)
permQuery = &models.GetDashboardACLInfoListQuery{DashboardID: 1, OrgID: users[0].OrgID}
err = getDashboardACLInfoList(ss, permQuery)
require.Nil(t, err)
require.Len(t, permQuery.Result, 0)
})
t.Run("Testing DB - return list of users that the SignedInUser has permission to read", func(t *testing.T) {
ss := InitTestDB(t)
createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand {
return &user.CreateUserCommand{
Email: fmt.Sprint("user", i, "@test.com"),
Name: fmt.Sprint("user", i),
Login: fmt.Sprint("loginuser", i),
}
})
testUser := &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{1: {"users:read": {"global.users:id:1", "global.users:id:3"}}},
}
query := models.SearchUsersQuery{SignedInUser: testUser}
err := ss.SearchUsers(context.Background(), &query)
assert.Nil(t, err)
assert.Len(t, query.Result.Users, 2)
})
ss = InitTestDB(t)
t.Run("Testing DB - enable all users", func(t *testing.T) {
users := createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand {
return &user.CreateUserCommand{
Email: fmt.Sprint("user", i, "@test.com"),
Name: fmt.Sprint("user", i),
Login: fmt.Sprint("loginuser", i),
IsDisabled: true,
}
})
disableCmd := models.BatchDisableUsersCommand{
UserIds: []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID, users[4].ID},
IsDisabled: false,
}
err := ss.BatchDisableUsers(context.Background(), &disableCmd)
require.Nil(t, err)
isDisabled := false
query := &models.SearchUsersQuery{IsDisabled: &isDisabled, SignedInUser: usr}
err = ss.SearchUsers(context.Background(), query)
require.Nil(t, err)
require.EqualValues(t, query.Result.TotalCount, 5)
})
ss = InitTestDB(t)
t.Run("Testing DB - disable only specific users", func(t *testing.T) {
users := createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand {
return &user.CreateUserCommand{
Email: fmt.Sprint("user", i, "@test.com"),
Name: fmt.Sprint("user", i),
Login: fmt.Sprint("loginuser", i),
IsDisabled: false,
}
})
userIdsToDisable := []int64{}
for i := 0; i < 3; i++ {
userIdsToDisable = append(userIdsToDisable, users[i].ID)
}
disableCmd := models.BatchDisableUsersCommand{
UserIds: userIdsToDisable,
IsDisabled: true,
}
err := ss.BatchDisableUsers(context.Background(), &disableCmd)
require.Nil(t, err)
query := models.SearchUsersQuery{SignedInUser: usr}
err = ss.SearchUsers(context.Background(), &query)
require.Nil(t, err)
require.EqualValues(t, query.Result.TotalCount, 5)
for _, user := range query.Result.Users {
shouldBeDisabled := false
// Check if user id is in the userIdsToDisable list
for _, disabledUserId := range userIdsToDisable {
if user.Id == disabledUserId {
require.True(t, user.IsDisabled)
shouldBeDisabled = true
}
}
// Otherwise user shouldn't be disabled
if !shouldBeDisabled {
require.False(t, user.IsDisabled)
}
}
})
ss = InitTestDB(t)
t.Run("Testing DB - search users", func(t *testing.T) {