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

ldap debug bus removal (#45014)

Browse Source 
* ldap debug bus removal

* linter
This commit is contained in:
ying-jeanne
2022-02-09 11:45:31 +01:00
committed by GitHub
parent 089a111858
commit ef11e783f1
13 changed files with 99 additions and 139 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/admin_users_test.go
+8 -26
View File
@@ -2,7 +2,6 @@ package api
import (
"context"
"errors"
"fmt"
"testing"
@@ -13,7 +12,7 @@ import (
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/login/loginservice"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/setting"
@@ -453,8 +452,13 @@ func adminCreateUserScenario(t *testing.T, desc string, url string, routePattern
t.Cleanup(bus.ClearBusHandlers)
hs := HTTPServer{
Bus: bus.GetBus(),
Login: fakeLoginService{expected: cmd},
Bus: bus.GetBus(),
Login: loginservice.LoginServiceMock{
ExpectedUserForm: cmd,
NoExistingOrgId: nonExistingOrgID,
AlreadyExitingLogin: existingTestLogin,
GeneratedUserId: testUserID,
},
}
sc := setupScenarioContext(t, url)
@@ -471,25 +475,3 @@ func adminCreateUserScenario(t *testing.T, desc string, url string, routePattern
fn(sc)
})
}
type fakeLoginService struct {
login.Service
expected dtos.AdminCreateUserForm
}
func (s fakeLoginService) CreateUser(cmd models.CreateUserCommand) (*models.User, error) {
if cmd.OrgId == nonExistingOrgID {
return nil, models.ErrOrgNotFound
}
if cmd.Login == existingTestLogin {
return nil, models.ErrUserAlreadyExists
}
if s.expected.Login == cmd.Login && s.expected.Email == cmd.Email &&
s.expected.Password == cmd.Password && s.expected.Name == cmd.Name && s.expected.OrgId == cmd.OrgId {
return &models.User{Id: testUserID}, nil
}
return nil, errors.New("unexpected cmd")
}
pkg/api/api.go
+1 -1
View File
@@ -238,7 +238,7 @@ func (hs *HTTPServer) registerRoutes() {
apiRoute.Post("/orgs", authorizeInOrg(reqSignedIn, acmiddleware.UseGlobalOrg, ac.EvalPermission(ActionOrgsCreate)), quota("org"), routing.Wrap(hs.CreateOrg))
// search all orgs
apiRoute.Get("/orgs", authorizeInOrg(reqGrafanaAdmin, acmiddleware.UseGlobalOrg, ac.EvalPermission(ActionOrgsRead)), routing.Wrap(SearchOrgs))
apiRoute.Get("/orgs", authorizeInOrg(reqGrafanaAdmin, acmiddleware.UseGlobalOrg, ac.EvalPermission(ActionOrgsRead)), routing.Wrap(hs.SearchOrgs))
// orgs (admin routes)
apiRoute.Group("/orgs/:orgId", func(orgsRoute routing.RouteRegister) {
pkg/api/ldap_debug.go
+7 -8
View File
@@ -8,12 +8,12 @@ import (
"strconv"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/login"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/ldap"
"github.com/grafana/grafana/pkg/services/multildap"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
)
@@ -64,7 +64,7 @@ type LDAPServerDTO struct {
}
// FetchOrgs fetches the organization(s) information by executing a single query to the database. Then, populating the DTO with the information retrieved.
func (user *LDAPUserDTO) FetchOrgs(ctx context.Context) error {
func (user *LDAPUserDTO) FetchOrgs(ctx context.Context, sqlstore sqlstore.Store) error {
orgIds := []int64{}
for _, or := range user.OrgRoles {
@@ -74,7 +74,7 @@ func (user *LDAPUserDTO) FetchOrgs(ctx context.Context) error {
q := &models.SearchOrgsQuery{}
q.Ids = orgIds
if err := bus.Dispatch(ctx, q); err != nil {
if err := sqlstore.SearchOrgs(ctx, q); err != nil {
return err
}
@@ -171,7 +171,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) response.Respon
query := models.GetUserByIdQuery{Id: userId}
if err := bus.Dispatch(c.Req.Context(), &query); err != nil { // validate the userId exists
if err := hs.SQLStore.GetUserById(c.Req.Context(), &query); err != nil { // validate the userId exists
if errors.Is(err, models.ErrUserNotFound) {
return response.Error(404, models.ErrUserNotFound.Error(), nil)
}
@@ -180,8 +180,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) response.Respon
}
authModuleQuery := &models.GetAuthInfoQuery{UserId: query.Result.Id, AuthModule: models.AuthModuleLDAP}
if err := bus.Dispatch(c.Req.Context(), authModuleQuery); err != nil { // validate the userId comes from LDAP
if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), authModuleQuery); err != nil { // validate the userId comes from LDAP
if errors.Is(err, models.ErrUserNotFound) {
return response.Error(404, models.ErrUserNotFound.Error(), nil)
}
@@ -223,7 +222,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) response.Respon
SignupAllowed: hs.Cfg.LDAPAllowSignup,
}
err = bus.Dispatch(c.Req.Context(), upsertCmd)
err = hs.Login.UpsertUser(c.Req.Context(), upsertCmd)
if err != nil {
return response.Error(http.StatusInternalServerError, "Failed to update the user", err)
}
@@ -306,7 +305,7 @@ func (hs *HTTPServer) GetUserFromLDAP(c *models.ReqContext) response.Response {
u.OrgRoles = orgRoles
ldapLogger.Debug("mapping org roles", "orgsRoles", u.OrgRoles)
err = u.FetchOrgs(c.Req.Context())
err = u.FetchOrgs(c.Req.Context(), hs.SQLStore)
if err != nil {
return response.Error(http.StatusBadRequest, "An organization was not found - Please verify your LDAP configuration", err)
}
pkg/api/ldap_debug_test.go
+26 -93
View File
@@ -9,6 +9,9 @@ import (
"testing"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/login/loginservice"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/api/routing"
@@ -53,7 +56,7 @@ func (m *LDAPMock) User(login string) (*models.ExternalUserInfo, ldap.ServerConf
// GetUserFromLDAP tests
// ***
func getUserFromLDAPContext(t *testing.T, requestURL string) *scenarioContext {
func getUserFromLDAPContext(t *testing.T, requestURL string, searchOrgRst []*models.OrgDTO) *scenarioContext {
t.Helper()
sc := setupScenarioContext(t, requestURL)
@@ -62,7 +65,7 @@ func getUserFromLDAPContext(t *testing.T, requestURL string) *scenarioContext {
setting.LDAPEnabled = true
t.Cleanup(func() { setting.LDAPEnabled = origLDAP })
hs := &HTTPServer{Cfg: setting.NewCfg(), ldapGroups: ldap.ProvideGroupsService()}
hs := &HTTPServer{Cfg: setting.NewCfg(), ldapGroups: ldap.ProvideGroupsService(), SQLStore: &mockstore.SQLStoreMock{ExpectedSearchOrgList: searchOrgRst}}
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
sc.context = c
@@ -90,7 +93,7 @@ func TestGetUserFromLDAPAPIEndpoint_UserNotFound(t *testing.T) {
userSearchResult = nil
sc := getUserFromLDAPContext(t, "/api/admin/ldap/user-that-does-not-exist")
sc := getUserFromLDAPContext(t, "/api/admin/ldap/user-that-does-not-exist", []*models.OrgDTO{})
require.Equal(t, sc.resp.Code, http.StatusNotFound)
assert.JSONEq(t, "{\"message\":\"No user was found in the LDAP server(s) with that username\"}", sc.resp.Body.String())
@@ -132,11 +135,6 @@ func TestGetUserFromLDAPAPIEndpoint_OrgNotfound(t *testing.T) {
{Id: 1, Name: "Main Org."},
}
bus.AddHandler("test", func(ctx context.Context, query *models.SearchOrgsQuery) error {
query.Result = mockOrgSearchResult
return nil
})
getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) {
return &ldap.Config{}, nil
}
@@ -145,7 +143,7 @@ func TestGetUserFromLDAPAPIEndpoint_OrgNotfound(t *testing.T) {
return &LDAPMock{}
}
sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe")
sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe", mockOrgSearchResult)
require.Equal(t, http.StatusBadRequest, sc.resp.Code)
@@ -194,11 +192,6 @@ func TestGetUserFromLDAPAPIEndpoint(t *testing.T) {
{Id: 1, Name: "Main Org."},
}
bus.AddHandler("test", func(ctx context.Context, query *models.SearchOrgsQuery) error {
query.Result = mockOrgSearchResult
return nil
})
getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) {
return &ldap.Config{}, nil
}
@@ -207,7 +200,7 @@ func TestGetUserFromLDAPAPIEndpoint(t *testing.T) {
return &LDAPMock{}
}
sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe")
sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe", mockOrgSearchResult)
assert.Equal(t, sc.resp.Code, http.StatusOK)
@@ -269,11 +262,6 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) {
{Id: 1, Name: "Main Org."},
}
bus.AddHandler("test", func(ctx context.Context, query *models.SearchOrgsQuery) error {
query.Result = mockOrgSearchResult
return nil
})
getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) {
return &ldap.Config{}, nil
}
@@ -282,7 +270,7 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) {
return &LDAPMock{}
}
sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe")
sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe", mockOrgSearchResult)
require.Equal(t, sc.resp.Code, http.StatusOK)
@@ -376,7 +364,7 @@ func TestGetLDAPStatusAPIEndpoint(t *testing.T) {
// PostSyncUserWithLDAP tests
// ***
func postSyncUserWithLDAPContext(t *testing.T, requestURL string, preHook func(*testing.T, *scenarioContext)) *scenarioContext {
func postSyncUserWithLDAPContext(t *testing.T, requestURL string, preHook func(*testing.T, *scenarioContext), sqlstoremock sqlstore.Store) *scenarioContext {
t.Helper()
sc := setupScenarioContext(t, requestURL)
@@ -390,6 +378,9 @@ func postSyncUserWithLDAPContext(t *testing.T, requestURL string, preHook func(*
hs := &HTTPServer{
Cfg: sc.cfg,
AuthTokenService: auth.NewFakeUserAuthTokenService(),
SQLStore: sqlstoremock,
Login: loginservice.LoginServiceMock{},
authInfoService: &mockAuthInfoService{},
}
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
@@ -412,6 +403,8 @@ func postSyncUserWithLDAPContext(t *testing.T, requestURL string, preHook func(*
}
func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) {
sqlstoremock := mockstore.SQLStoreMock{}
sqlstoremock.ExpectedUser = &models.User{Login: "ldap-daniel", Id: 34}
sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34", func(t *testing.T, sc *scenarioContext) {
getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) {
return &ldap.Config{}, nil
@@ -424,26 +417,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) {
userSearchResult = &models.ExternalUserInfo{
Login: "ldap-daniel",
}
bus.AddHandler("test", func(ctx context.Context, cmd *models.UpsertUserCommand) error {
require.Equal(t, "ldap-daniel", cmd.ExternalUser.Login)
return nil
})
bus.AddHandler("test", func(ctx context.Context, q *models.GetUserByIdQuery) error {
require.Equal(t, q.Id, int64(34))
q.Result = &models.User{Login: "ldap-daniel", Id: 34}
return nil
})
bus.AddHandler("test", func(ctx context.Context, q *models.GetAuthInfoQuery) error {
require.Equal(t, q.UserId, int64(34))
require.Equal(t, q.AuthModule, models.AuthModuleLDAP)
return nil
})
})
}, &sqlstoremock)
assert.Equal(t, http.StatusOK, sc.resp.Code)
@@ -457,6 +431,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) {
}
func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) {
sqlstoremock := mockstore.SQLStoreMock{ExpectedError: models.ErrUserNotFound}
sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34", func(t *testing.T, sc *scenarioContext) {
getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) {
return &ldap.Config{}, nil
@@ -465,13 +440,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) {
newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP {
return &LDAPMock{}
}
bus.AddHandler("test", func(ctx context.Context, q *models.GetUserByIdQuery) error {
require.Equal(t, q.Id, int64(34))
return models.ErrUserNotFound
})
})
}, &sqlstoremock)
assert.Equal(t, http.StatusNotFound, sc.resp.Code)
@@ -485,6 +454,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) {
}
func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) {
sqlstoremock := mockstore.SQLStoreMock{ExpectedUser: &models.User{Login: "ldap-daniel", Id: 34}}
sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34", func(t *testing.T, sc *scenarioContext) {
getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) {
return &ldap.Config{}, nil
@@ -497,22 +467,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) {
userSearchError = multildap.ErrDidNotFindUser
sc.cfg.AdminUser = "ldap-daniel"
bus.AddHandler("test", func(ctx context.Context, q *models.GetUserByIdQuery) error {
require.Equal(t, q.Id, int64(34))
q.Result = &models.User{Login: "ldap-daniel", Id: 34}
return nil
})
bus.AddHandler("test", func(ctx context.Context, q *models.GetAuthInfoQuery) error {
require.Equal(t, q.UserId, int64(34))
require.Equal(t, q.AuthModule, models.AuthModuleLDAP)
return nil
})
})
}, &sqlstoremock)
assert.Equal(t, http.StatusBadRequest, sc.resp.Code)
expected := `
@@ -526,6 +481,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) {
}
func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) {
sqlstoremock := mockstore.SQLStoreMock{ExpectedUser: &models.User{Login: "ldap-daniel", Id: 34}}
sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34", func(t *testing.T, sc *scenarioContext) {
getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) {
return &ldap.Config{}, nil
@@ -537,18 +493,6 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) {
userSearchResult = nil
bus.AddHandler("test", func(ctx context.Context, cmd *models.UpsertUserCommand) error {
require.Equal(t, "ldap-daniel", cmd.ExternalUser.Login)
return nil
})
bus.AddHandler("test", func(ctx context.Context, q *models.GetUserByIdQuery) error {
require.Equal(t, q.Id, int64(34))
q.Result = &models.User{Login: "ldap-daniel", Id: 34}
return nil
})
bus.AddHandler("test", func(ctx context.Context, q *models.GetExternalUserInfoByLoginQuery) error {
assert.Equal(t, "ldap-daniel", q.LoginOrEmail)
q.Result = &models.ExternalUserInfo{IsDisabled: true, UserId: 34}
@@ -560,7 +504,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) {
assert.Equal(t, 34, cmd.UserId)
return nil
})
})
}, &sqlstoremock)
assert.Equal(t, http.StatusBadRequest, sc.resp.Code)
@@ -670,8 +614,10 @@ func TestLDAP_AccessControl(t *testing.T) {
cfg := setting.NewCfg()
cfg.LDAPEnabled = true
sc, _ := setupAccessControlScenarioContext(t, cfg, test.url, test.permissions)
sc, hs := setupAccessControlScenarioContext(t, cfg, test.url, test.permissions)
hs.SQLStore = &mockstore.SQLStoreMock{ExpectedUser: &models.User{}}
hs.authInfoService = &mockAuthInfoService{}
hs.Login = &loginservice.LoginServiceMock{}
sc.resp = httptest.NewRecorder()
sc.req, err = http.NewRequest(test.method, test.url, nil)
assert.NoError(t, err)
@@ -683,19 +629,6 @@ func TestLDAP_AccessControl(t *testing.T) {
return &LDAPMock{}
}
bus.AddHandler("test", func(ctx context.Context, q *models.GetUserByIdQuery) error {
q.Result = &models.User{}
return nil
})
bus.AddHandler("test", func(ctx context.Context, q *models.GetAuthInfoQuery) error {
return nil
})
bus.AddHandler("test", func(ctx context.Context, cmd *models.UpsertUserCommand) error {
return nil
})
sc.exec()
assert.Equal(t, test.expectedCode, sc.resp.Code)
})
pkg/api/org.go
+2 -2
View File
@@ -207,7 +207,7 @@ func (hs *HTTPServer) DeleteOrgByID(c *models.ReqContext) response.Response {
return response.Success("Organization deleted")
}
func SearchOrgs(c *models.ReqContext) response.Response {
func (hs *HTTPServer) SearchOrgs(c *models.ReqContext) response.Response {
perPage := c.QueryInt("perpage")
if perPage <= 0 {
perPage = 1000
@@ -222,7 +222,7 @@ func SearchOrgs(c *models.ReqContext) response.Response {
Limit: perPage,
}
if err := sqlstore.SearchOrgs(c.Req.Context(), &query); err != nil {
if err := hs.SQLStore.SearchOrgs(c.Req.Context(), &query); err != nil {
return response.Error(500, "Failed to search orgs", err)
}