Extract search users functions into a service (#39002)

* Extract search users to a new service

* Fix wire provider

* Fix common_test and remove RouteRegister

* Remove old endpoints

* Fix test

* Add indexes to dashboards and orgs tables

* Fix lint
This commit is contained in:
Selene 2021-09-29 12:51:49 +02:00 committed by GitHub
parent 0ecf13e5a3
commit 02702eb82d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 148 additions and 74 deletions

View File

@ -168,8 +168,8 @@ func (hs *HTTPServer) registerRoutes() {
// users (admin permission required) // users (admin permission required)
apiRoute.Group("/users", func(usersRoute routing.RouteRegister) { apiRoute.Group("/users", func(usersRoute routing.RouteRegister) {
userIDScope := ac.Scope("global", "users", ac.Parameter(":id")) userIDScope := ac.Scope("global", "users", ac.Parameter(":id"))
usersRoute.Get("/", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, ac.ScopeGlobalUsersAll)), routing.Wrap(SearchUsers)) usersRoute.Get("/", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, ac.ScopeGlobalUsersAll)), routing.Wrap(hs.searchUsersService.SearchUsers))
usersRoute.Get("/search", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, ac.ScopeGlobalUsersAll)), routing.Wrap(SearchUsersWithPaging)) usersRoute.Get("/search", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, ac.ScopeGlobalUsersAll)), routing.Wrap(hs.searchUsersService.SearchUsersWithPaging))
usersRoute.Get("/:id", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, userIDScope)), routing.Wrap(GetUserByID)) usersRoute.Get("/:id", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, userIDScope)), routing.Wrap(GetUserByID))
usersRoute.Get("/:id/teams", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersTeamRead, userIDScope)), routing.Wrap(GetUserTeams)) usersRoute.Get("/:id/teams", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersTeamRead, userIDScope)), routing.Wrap(GetUserTeams))
usersRoute.Get("/:id/orgs", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, userIDScope)), routing.Wrap(GetUserOrgList)) usersRoute.Get("/:id/orgs", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, userIDScope)), routing.Wrap(GetUserOrgList))

View File

@ -8,6 +8,8 @@ import (
"path/filepath" "path/filepath"
"testing" "testing"
"github.com/grafana/grafana/pkg/services/searchusers"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"gopkg.in/macaron.v1" "gopkg.in/macaron.v1"
@ -216,13 +218,15 @@ func setupAccessControlScenarioContext(t *testing.T, cfg *setting.Cfg, url strin
cfg.FeatureToggles["accesscontrol"] = true cfg.FeatureToggles["accesscontrol"] = true
cfg.Quota.Enabled = false cfg.Quota.Enabled = false
bus := bus.GetBus()
hs := &HTTPServer{ hs := &HTTPServer{
Cfg: cfg, Cfg: cfg,
Bus: bus.GetBus(), Bus: bus,
Live: newTestLive(t), Live: newTestLive(t),
QuotaService: &quota.QuotaService{Cfg: cfg}, QuotaService: &quota.QuotaService{Cfg: cfg},
RouteRegister: routing.NewRouteRegister(), RouteRegister: routing.NewRouteRegister(),
AccessControl: accesscontrolmock.New().WithPermissions(permissions), AccessControl: accesscontrolmock.New().WithPermissions(permissions),
searchUsersService: searchusers.ProvideUsersService(bus),
} }
sc := setupScenarioContext(t, url) sc := setupScenarioContext(t, url)

View File

@ -13,6 +13,8 @@ import (
"strings" "strings"
"sync" "sync"
"github.com/grafana/grafana/pkg/services/searchusers"
"github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/api/routing"
httpstatic "github.com/grafana/grafana/pkg/api/static" httpstatic "github.com/grafana/grafana/pkg/api/static"
"github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/bus"
@ -107,6 +109,7 @@ type HTTPServer struct {
cleanUpService *cleanup.CleanUpService cleanUpService *cleanup.CleanUpService
tracingService *tracing.TracingService tracingService *tracing.TracingService
internalMetricsSvc *metrics.InternalMetricsService internalMetricsSvc *metrics.InternalMetricsService
searchUsersService searchusers.Service
} }
type ServerOptions struct { type ServerOptions struct {
@ -130,7 +133,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
notificationService *notifications.NotificationService, tracingService *tracing.TracingService, notificationService *notifications.NotificationService, tracingService *tracing.TracingService,
internalMetricsSvc *metrics.InternalMetricsService, quotaService *quota.QuotaService, internalMetricsSvc *metrics.InternalMetricsService, quotaService *quota.QuotaService,
socialService social.Service, oauthTokenService oauthtoken.OAuthTokenService, socialService social.Service, oauthTokenService oauthtoken.OAuthTokenService,
encryptionService encryption.Service) (*HTTPServer, error) { encryptionService encryption.Service, searchUsersService searchusers.Service) (*HTTPServer, error) {
macaron.Env = cfg.Env macaron.Env = cfg.Env
m := macaron.New() m := macaron.New()
@ -177,6 +180,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
SocialService: socialService, SocialService: socialService,
OAuthTokenService: oauthTokenService, OAuthTokenService: oauthTokenService,
EncryptionService: encryptionService, EncryptionService: encryptionService,
searchUsersService: searchUsersService,
} }
if hs.Listener != nil { if hs.Listener != nil {
hs.log.Debug("Using provided listener") hs.log.Debug("Using provided listener")

View File

@ -8,6 +8,9 @@ import (
"testing" "testing"
"time" "time"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/services/searchusers"
"github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/components/simplejson"
@ -137,11 +140,12 @@ func setupOrgUsersAPIcontext(t *testing.T, role models.RoleType) (*scenarioConte
db := sqlstore.InitTestDB(t) db := sqlstore.InitTestDB(t)
hs := &HTTPServer{ hs := &HTTPServer{
Cfg: cfg, Cfg: cfg,
QuotaService: &quota.QuotaService{Cfg: cfg}, QuotaService: &quota.QuotaService{Cfg: cfg},
RouteRegister: routing.NewRouteRegister(), RouteRegister: routing.NewRouteRegister(),
AccessControl: accesscontrolmock.New().WithDisabled(), AccessControl: accesscontrolmock.New().WithDisabled(),
SQLStore: db, SQLStore: db,
searchUsersService: searchusers.ProvideUsersService(bus.New()),
} }
sc := setupScenarioContext(t, "/api/org/users/lookup") sc := setupScenarioContext(t, "/api/org/users/lookup")

View File

@ -258,61 +258,6 @@ func redirectToChangePassword(c *models.ReqContext) {
c.Redirect("/profile/password", 302) c.Redirect("/profile/password", 302)
} }
// GET /api/users
func SearchUsers(c *models.ReqContext) response.Response {
query, err := searchUser(c)
if err != nil {
return response.Error(500, "Failed to fetch users", err)
}
return response.JSON(200, query.Result.Users)
}
// GET /api/users/search
func SearchUsersWithPaging(c *models.ReqContext) response.Response {
query, err := searchUser(c)
if err != nil {
return response.Error(500, "Failed to fetch users", err)
}
return response.JSON(200, query.Result)
}
func searchUser(c *models.ReqContext) (*models.SearchUsersQuery, error) {
perPage := c.QueryInt("perpage")
if perPage <= 0 {
perPage = 1000
}
page := c.QueryInt("page")
if page < 1 {
page = 1
}
searchQuery := c.Query("query")
filter := c.Query("filter")
query := &models.SearchUsersQuery{Query: searchQuery, Filter: models.SearchUsersFilter(filter), Page: page, Limit: perPage}
if err := bus.Dispatch(query); err != nil {
return nil, err
}
for _, user := range query.Result.Users {
user.AvatarUrl = dtos.GetGravatarUrl(user.Email)
user.AuthLabels = make([]string, 0)
if user.AuthModule != nil && len(user.AuthModule) > 0 {
for _, authModule := range user.AuthModule {
user.AuthLabels = append(user.AuthLabels, GetAuthProviderLabel(authModule))
}
}
}
query.Result.Page = page
query.Result.PerPage = perPage
return query, nil
}
func SetHelpFlag(c *models.ReqContext) response.Response { func SetHelpFlag(c *models.ReqContext) response.Response {
flag := c.ParamsInt64(":id") flag := c.ParamsInt64(":id")

View File

@ -6,6 +6,8 @@ import (
"testing" "testing"
"time" "time"
"github.com/grafana/grafana/pkg/services/searchusers"
"github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/components/simplejson"
@ -134,7 +136,8 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) {
return nil return nil
}) })
sc.handlerFunc = SearchUsers searchUsersService := searchusers.ProvideUsersService(bus.GetBus())
sc.handlerFunc = searchUsersService.SearchUsers
sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
assert.Equal(t, 1000, sentLimit) assert.Equal(t, 1000, sentLimit)
@ -157,7 +160,8 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) {
return nil return nil
}) })
sc.handlerFunc = SearchUsers searchUsersService := searchusers.ProvideUsersService(bus.GetBus())
sc.handlerFunc = searchUsersService.SearchUsers
sc.fakeReqWithParams("GET", sc.url, map[string]string{"perpage": "10", "page": "2"}).exec() sc.fakeReqWithParams("GET", sc.url, map[string]string{"perpage": "10", "page": "2"}).exec()
assert.Equal(t, 10, sentLimit) assert.Equal(t, 10, sentLimit)
@ -176,7 +180,8 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) {
return nil return nil
}) })
sc.handlerFunc = SearchUsersWithPaging searchUsersService := searchusers.ProvideUsersService(bus.GetBus())
sc.handlerFunc = searchUsersService.SearchUsersWithPaging
sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
assert.Equal(t, 1000, sentLimit) assert.Equal(t, 1000, sentLimit)
@ -201,7 +206,8 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) {
return nil return nil
}) })
sc.handlerFunc = SearchUsersWithPaging searchUsersService := searchusers.ProvideUsersService(bus.GetBus())
sc.handlerFunc = searchUsersService.SearchUsersWithPaging
sc.fakeReqWithParams("GET", sc.url, map[string]string{"perpage": "10", "page": "2"}).exec() sc.fakeReqWithParams("GET", sc.url, map[string]string{"perpage": "10", "page": "2"}).exec()
assert.Equal(t, 10, sentLimit) assert.Equal(t, 10, sentLimit)

View File

@ -5,7 +5,6 @@ package server
import ( import (
"github.com/google/wire" "github.com/google/wire"
sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient"
"github.com/grafana/grafana/pkg/api" "github.com/grafana/grafana/pkg/api"
"github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/api/routing"

View File

@ -18,6 +18,7 @@ import (
"github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/login/authinfoservice" "github.com/grafana/grafana/pkg/services/login/authinfoservice"
"github.com/grafana/grafana/pkg/services/provisioning" "github.com/grafana/grafana/pkg/services/provisioning"
"github.com/grafana/grafana/pkg/services/searchusers"
"github.com/grafana/grafana/pkg/services/sqlstore/migrations" "github.com/grafana/grafana/pkg/services/sqlstore/migrations"
"github.com/grafana/grafana/pkg/services/validations" "github.com/grafana/grafana/pkg/services/validations"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
@ -48,6 +49,8 @@ var wireExtsBasicSet = wire.NewSet(
wire.Bind(new(login.UserProtectionService), new(*authinfoservice.OSSUserProtectionImpl)), wire.Bind(new(login.UserProtectionService), new(*authinfoservice.OSSUserProtectionImpl)),
ossencryption.ProvideService, ossencryption.ProvideService,
wire.Bind(new(encryption.Service), new(*ossencryption.Service)), wire.Bind(new(encryption.Service), new(*ossencryption.Service)),
searchusers.ProvideUsersService,
wire.Bind(new(searchusers.Service), new(*searchusers.OSSService)),
) )
var wireExtsSet = wire.NewSet( var wireExtsSet = wire.NewSet(

View File

@ -0,0 +1,95 @@
package searchusers
import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/models"
)
type Service interface {
SearchUsers(c *models.ReqContext) response.Response
SearchUsersWithPaging(c *models.ReqContext) response.Response
}
type OSSService struct {
bus bus.Bus
}
func ProvideUsersService(bus bus.Bus) *OSSService {
return &OSSService{bus: bus}
}
func (s *OSSService) SearchUsers(c *models.ReqContext) response.Response {
query, err := s.SearchUser(c)
if err != nil {
return response.Error(500, "Failed to fetch users", err)
}
return response.JSON(200, query.Result.Users)
}
func (s *OSSService) SearchUsersWithPaging(c *models.ReqContext) response.Response {
query, err := s.SearchUser(c)
if err != nil {
return response.Error(500, "Failed to fetch users", err)
}
return response.JSON(200, query.Result)
}
func (s *OSSService) SearchUser(c *models.ReqContext) (*models.SearchUsersQuery, error) {
perPage := c.QueryInt("perpage")
if perPage <= 0 {
perPage = 1000
}
page := c.QueryInt("page")
if page < 1 {
page = 1
}
searchQuery := c.Query("query")
filter := c.Query("filter")
query := &models.SearchUsersQuery{Query: searchQuery, Filter: models.SearchUsersFilter(filter), Page: page, Limit: perPage}
if err := s.bus.Dispatch(query); err != nil {
return nil, err
}
for _, user := range query.Result.Users {
user.AvatarUrl = dtos.GetGravatarUrl(user.Email)
user.AuthLabels = make([]string, 0)
if user.AuthModule != nil && len(user.AuthModule) > 0 {
for _, authModule := range user.AuthModule {
user.AuthLabels = append(user.AuthLabels, GetAuthProviderLabel(authModule))
}
}
}
query.Result.Page = page
query.Result.PerPage = perPage
return query, nil
}
func GetAuthProviderLabel(authModule string) string {
switch authModule {
case "oauth_github":
return "GitHub"
case "oauth_google":
return "Google"
case "oauth_azuread":
return "AzureAD"
case "oauth_gitlab":
return "GitLab"
case "oauth_grafana_com", "oauth_grafananet":
return "grafana.com"
case "auth.saml":
return "SAML"
case "ldap", "":
return "LDAP"
default:
return "OAuth"
}
}

View File

@ -20,6 +20,10 @@ func addDashboardAclMigrations(mg *Migrator) {
{Cols: []string{"dashboard_id"}}, {Cols: []string{"dashboard_id"}},
{Cols: []string{"dashboard_id", "user_id"}, Type: UniqueIndex}, {Cols: []string{"dashboard_id", "user_id"}, Type: UniqueIndex},
{Cols: []string{"dashboard_id", "team_id"}, Type: UniqueIndex}, {Cols: []string{"dashboard_id", "team_id"}, Type: UniqueIndex},
{Cols: []string{"user_id"}},
{Cols: []string{"team_id"}},
{Cols: []string{"org_id", "role"}},
{Cols: []string{"permission"}},
}, },
} }
@ -29,6 +33,10 @@ func addDashboardAclMigrations(mg *Migrator) {
mg.AddMigration("add index dashboard_acl_dashboard_id", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[0])) mg.AddMigration("add index dashboard_acl_dashboard_id", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[0]))
mg.AddMigration("add unique index dashboard_acl_dashboard_id_user_id", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[1])) mg.AddMigration("add unique index dashboard_acl_dashboard_id_user_id", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[1]))
mg.AddMigration("add unique index dashboard_acl_dashboard_id_team_id", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[2])) mg.AddMigration("add unique index dashboard_acl_dashboard_id_team_id", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[2]))
mg.AddMigration("add index dashboard_acl_user_id", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[3]))
mg.AddMigration("add index dashboard_acl_team_id", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[4]))
mg.AddMigration("add index dashboard_acl_org_id_role", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[5]))
mg.AddMigration("add index dashboard_permission", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[6]))
const rawSQL = ` const rawSQL = `
INSERT INTO dashboard_acl INSERT INTO dashboard_acl

View File

@ -225,4 +225,9 @@ func addDashboardMigration(mg *Migrator) {
mg.AddMigration("delete stars for deleted dashboards", NewRawSQLMigration( mg.AddMigration("delete stars for deleted dashboards", NewRawSQLMigration(
"DELETE FROM star WHERE dashboard_id NOT IN (SELECT id FROM dashboard)")) "DELETE FROM star WHERE dashboard_id NOT IN (SELECT id FROM dashboard)"))
mg.AddMigration("Add index for dashboard_is_folder", NewAddIndexMigration(dashboardV2, &Index{
Cols: []string{"is_folder"},
Type: IndexType,
}))
} }

View File

@ -41,6 +41,7 @@ func addOrgMigrations(mg *Migrator) {
Indices: []*Index{ Indices: []*Index{
{Cols: []string{"org_id"}}, {Cols: []string{"org_id"}},
{Cols: []string{"org_id", "user_id"}, Type: UniqueIndex}, {Cols: []string{"org_id", "user_id"}, Type: UniqueIndex},
{Cols: []string{"user_id"}},
}, },
} }