From 6322fce725ff3fcce2edc4e534b2052da466a3e2 Mon Sep 17 00:00:00 2001 From: Jo Date: Wed, 8 Feb 2023 09:32:59 +0100 Subject: [PATCH] LDAP: Move to single package cluster (#63035) * move multildap to ldap package * move LDAP api and tests to ldap package * register background service * lint --- .github/CODEOWNERS | 1 - pkg/api/api.go | 5 - pkg/api/http_server.go | 5 +- pkg/login/ldap_login.go | 2 +- pkg/login/ldap_login_test.go | 2 +- .../backgroundsvcs/background_services.go | 3 +- pkg/server/wire.go | 2 + pkg/services/authn/clients/ldap.go | 2 +- pkg/services/authn/clients/ldap_test.go | 2 +- .../contexthandler/authproxy/authproxy.go | 2 +- .../authproxy/authproxy_test.go | 2 +- .../ldap/api/service.go} | 87 ++- .../ldap/api/service_test.go} | 542 +++++++++++------- .../{ => ldap}/multildap/multidap_mock.go | 0 .../{ => ldap}/multildap/multildap.go | 0 .../{ => ldap}/multildap/multildap_test.go | 0 16 files changed, 394 insertions(+), 263 deletions(-) rename pkg/{api/ldap_debug.go => services/ldap/api/service.go} (77%) rename pkg/{api/ldap_debug_test.go => services/ldap/api/service_test.go} (52%) rename pkg/services/{ => ldap}/multildap/multidap_mock.go (100%) rename pkg/services/{ => ldap}/multildap/multildap.go (100%) rename pkg/services/{ => ldap}/multildap/multildap_test.go (100%) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 8c445c73f6f..0f591315db7 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -523,7 +523,6 @@ lerna.json @grafana/frontend-ops /pkg/services/guardian/ @grafana/grafana-authnz-team /pkg/services/ldap/ @grafana/grafana-authnz-team /pkg/services/login/ @grafana/grafana-authnz-team -/pkg/services/multildap/ @grafana/grafana-authnz-team /pkg/services/oauthtoken/ @grafana/grafana-authnz-team /pkg/services/teamguardian/ @grafana/grafana-authnz-team /pkg/services/serviceaccounts/ @grafana/grafana-authnz-team diff --git a/pkg/api/api.go b/pkg/api/api.go index 91fb8a2007e..22032c04f4b 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -655,11 +655,6 @@ func (hs *HTTPServer) registerRoutes() { adminRoute.Post("/provisioning/datasources/reload", authorize(reqGrafanaAdmin, ac.EvalPermission(ActionProvisioningReload, ScopeProvisionersDatasources)), routing.Wrap(hs.AdminProvisioningReloadDatasources)) adminRoute.Post("/provisioning/notifications/reload", authorize(reqGrafanaAdmin, ac.EvalPermission(ActionProvisioningReload, ScopeProvisionersNotifications)), routing.Wrap(hs.AdminProvisioningReloadNotifications)) adminRoute.Post("/provisioning/alerting/reload", authorize(reqGrafanaAdmin, ac.EvalPermission(ActionProvisioningReload, ScopeProvisionersAlertRules)), routing.Wrap(hs.AdminProvisioningReloadAlerting)) - - adminRoute.Post("/ldap/reload", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPConfigReload)), routing.Wrap(hs.ReloadLDAPCfg)) - adminRoute.Post("/ldap/sync/:id", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPUsersSync)), routing.Wrap(hs.PostSyncUserWithLDAP)) - adminRoute.Get("/ldap/:username", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPUsersRead)), routing.Wrap(hs.GetUserFromLDAP)) - adminRoute.Get("/ldap/status", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPStatusRead)), routing.Wrap(hs.GetLDAPStatus)) }, reqSignedIn) // Administering users diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 213dce093e8..e8810d4786a 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -56,7 +56,6 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/hooks" - "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/libraryelements" "github.com/grafana/grafana/pkg/services/librarypanels" "github.com/grafana/grafana/pkg/services/licensing" @@ -174,7 +173,6 @@ type HTTPServer struct { grafanaUpdateChecker *updatechecker.GrafanaService pluginsUpdateChecker *updatechecker.PluginsService searchUsersService searchusers.Service - ldapGroups ldap.Groups teamGuardian teamguardian.TeamGuardian queryDataService *query.Service serviceAccountsService serviceaccounts.Service @@ -241,7 +239,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi encryptionService encryption.Internal, grafanaUpdateChecker *updatechecker.GrafanaService, pluginsUpdateChecker *updatechecker.PluginsService, searchUsersService searchusers.Service, dataSourcesService datasources.DataSourceService, queryDataService *query.Service, - ldapGroups ldap.Groups, teamGuardian teamguardian.TeamGuardian, serviceaccountsService serviceaccounts.Service, + teamGuardian teamguardian.TeamGuardian, serviceaccountsService serviceaccounts.Service, authInfoService login.AuthInfoService, storageService store.StorageService, httpEntityStore httpentitystore.HTTPEntityStore, notificationService *notifications.NotificationService, dashboardService dashboards.DashboardService, dashboardProvisioningService dashboards.DashboardProvisioningService, folderService folder.Service, @@ -325,7 +323,6 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi httpEntityStore: httpEntityStore, DataSourcesService: dataSourcesService, searchUsersService: searchUsersService, - ldapGroups: ldapGroups, teamGuardian: teamGuardian, queryDataService: queryDataService, serviceAccountsService: serviceaccountsService, diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go index a2f0b4f3321..8196f4517e8 100644 --- a/pkg/login/ldap_login.go +++ b/pkg/login/ldap_login.go @@ -7,8 +7,8 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ldap" + "github.com/grafana/grafana/pkg/services/ldap/multildap" "github.com/grafana/grafana/pkg/services/login" - "github.com/grafana/grafana/pkg/services/multildap" "github.com/grafana/grafana/pkg/setting" ) diff --git a/pkg/login/ldap_login_test.go b/pkg/login/ldap_login_test.go index ac4dc47205c..23294abd037 100644 --- a/pkg/login/ldap_login_test.go +++ b/pkg/login/ldap_login_test.go @@ -9,9 +9,9 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/services/ldap" + "github.com/grafana/grafana/pkg/services/ldap/multildap" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/logintest" - "github.com/grafana/grafana/pkg/services/multildap" "github.com/grafana/grafana/pkg/setting" ) diff --git a/pkg/server/backgroundsvcs/background_services.go b/pkg/server/backgroundsvcs/background_services.go index 68ed1e3f41f..08c6460f492 100644 --- a/pkg/server/backgroundsvcs/background_services.go +++ b/pkg/server/backgroundsvcs/background_services.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/services/dashboardsnapshots" "github.com/grafana/grafana/pkg/services/grpcserver" "github.com/grafana/grafana/pkg/services/guardian" + ldapapi "github.com/grafana/grafana/pkg/services/ldap/api" "github.com/grafana/grafana/pkg/services/live" "github.com/grafana/grafana/pkg/services/live/pushhttp" "github.com/grafana/grafana/pkg/services/login/authinfoservice" @@ -53,7 +54,7 @@ func ProvideBackgroundServiceRegistry( _ dashboardsnapshots.Service, _ *alerting.AlertNotificationService, _ serviceaccounts.Service, _ *guardian.Provider, _ *plugindashboardsservice.DashboardUpdater, _ *sanitizer.Provider, - _ *grpcserver.HealthService, _ entity.EntityStoreServer, _ *grpcserver.ReflectionService, + _ *grpcserver.HealthService, _ entity.EntityStoreServer, _ *grpcserver.ReflectionService, _ *ldapapi.Service, ) *BackgroundServiceRegistry { return NewBackgroundServiceRegistry( httpServer, diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 6e3cfdd25bd..c5e8654ecb7 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -68,6 +68,7 @@ import ( "github.com/grafana/grafana/pkg/services/grpcserver/interceptors" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/hooks" + ldapapi "github.com/grafana/grafana/pkg/services/ldap/api" "github.com/grafana/grafana/pkg/services/libraryelements" "github.com/grafana/grafana/pkg/services/librarypanels" "github.com/grafana/grafana/pkg/services/live" @@ -248,6 +249,7 @@ var wireBasicSet = wire.NewSet( tracing.ProvideService, metrics.ProvideService, testdatasource.ProvideService, + ldapapi.ProvideService, opentsdb.ProvideService, social.ProvideService, influxdb.ProvideService, diff --git a/pkg/services/authn/clients/ldap.go b/pkg/services/authn/clients/ldap.go index 2b41d35d74f..a142a546ab7 100644 --- a/pkg/services/authn/clients/ldap.go +++ b/pkg/services/authn/clients/ldap.go @@ -5,8 +5,8 @@ import ( "errors" "github.com/grafana/grafana/pkg/services/authn" + "github.com/grafana/grafana/pkg/services/ldap/multildap" "github.com/grafana/grafana/pkg/services/login" - "github.com/grafana/grafana/pkg/services/multildap" "github.com/grafana/grafana/pkg/setting" ) diff --git a/pkg/services/authn/clients/ldap_test.go b/pkg/services/authn/clients/ldap_test.go index cdcaee93192..1eec720a597 100644 --- a/pkg/services/authn/clients/ldap_test.go +++ b/pkg/services/authn/clients/ldap_test.go @@ -8,8 +8,8 @@ import ( "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/ldap" + "github.com/grafana/grafana/pkg/services/ldap/multildap" "github.com/grafana/grafana/pkg/services/login" - "github.com/grafana/grafana/pkg/services/multildap" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/setting" ) diff --git a/pkg/services/contexthandler/authproxy/authproxy.go b/pkg/services/contexthandler/authproxy/authproxy.go index 6a6fbda21b7..95a340c7cc8 100644 --- a/pkg/services/contexthandler/authproxy/authproxy.go +++ b/pkg/services/contexthandler/authproxy/authproxy.go @@ -18,8 +18,8 @@ import ( "github.com/grafana/grafana/pkg/infra/remotecache" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/ldap" + "github.com/grafana/grafana/pkg/services/ldap/multildap" "github.com/grafana/grafana/pkg/services/login" - "github.com/grafana/grafana/pkg/services/multildap" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" diff --git a/pkg/services/contexthandler/authproxy/authproxy_test.go b/pkg/services/contexthandler/authproxy/authproxy_test.go index a646a01e68f..109f1c878f5 100644 --- a/pkg/services/contexthandler/authproxy/authproxy_test.go +++ b/pkg/services/contexthandler/authproxy/authproxy_test.go @@ -13,8 +13,8 @@ import ( "github.com/grafana/grafana/pkg/infra/remotecache" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/ldap" + "github.com/grafana/grafana/pkg/services/ldap/multildap" "github.com/grafana/grafana/pkg/services/login/loginservice" - "github.com/grafana/grafana/pkg/services/multildap" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" diff --git a/pkg/api/ldap_debug.go b/pkg/services/ldap/api/service.go similarity index 77% rename from pkg/api/ldap_debug.go rename to pkg/services/ldap/api/service.go index a6907ad7b93..1ebbb02c223 100644 --- a/pkg/api/ldap_debug.go +++ b/pkg/services/ldap/api/service.go @@ -9,23 +9,64 @@ import ( "strings" "github.com/grafana/grafana/pkg/api/response" + "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/middleware" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/auth" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/ldap" + "github.com/grafana/grafana/pkg/services/ldap/multildap" "github.com/grafana/grafana/pkg/services/login" - "github.com/grafana/grafana/pkg/services/multildap" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" ) +type Service struct { + cfg *setting.Cfg + userService user.Service + authInfoService login.AuthInfoService + ldapGroupsService ldap.Groups + loginService login.Service + orgService org.Service + sessionService auth.UserTokenService + log log.Logger +} + +func ProvideService(cfg *setting.Cfg, router routing.RouteRegister, accessControl ac.AccessControl, + userService user.Service, authInfoService login.AuthInfoService, ldapGroupsService ldap.Groups, + loginService login.Service, orgService org.Service, sessionService auth.UserTokenService) *Service { + s := &Service{ + cfg: cfg, + userService: userService, + authInfoService: authInfoService, + ldapGroupsService: ldapGroupsService, + loginService: loginService, + orgService: orgService, + sessionService: sessionService, + log: log.New("ldap.api"), + } + + authorize := ac.Middleware(accessControl) + reqGrafanaAdmin := middleware.ReqGrafanaAdmin + + router.Group("/api/admin", func(adminRoute routing.RouteRegister) { + adminRoute.Post("/ldap/reload", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPConfigReload)), routing.Wrap(s.ReloadLDAPCfg)) + adminRoute.Post("/ldap/sync/:id", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPUsersSync)), routing.Wrap(s.PostSyncUserWithLDAP)) + adminRoute.Get("/ldap/:username", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPUsersRead)), routing.Wrap(s.GetUserFromLDAP)) + adminRoute.Get("/ldap/status", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPStatusRead)), routing.Wrap(s.GetLDAPStatus)) + }, middleware.ReqSignedIn) + + return s +} + var ( getLDAPConfig = multildap.GetConfig newLDAP = multildap.New - ldapLogger = log.New("LDAP.debug") - errOrganizationNotFound = func(orgId int64) error { return fmt.Errorf("unable to find organization with ID '%d'", orgId) } @@ -117,7 +158,7 @@ func (user *LDAPUserDTO) FetchOrgs(ctx context.Context, orga org.Service) error // 401: unauthorisedError // 403: forbiddenError // 500: internalServerError -func (hs *HTTPServer) ReloadLDAPCfg(c *contextmodel.ReqContext) response.Response { +func (s *Service) ReloadLDAPCfg(c *contextmodel.ReqContext) response.Response { if !ldap.IsEnabled() { return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } @@ -143,12 +184,12 @@ func (hs *HTTPServer) ReloadLDAPCfg(c *contextmodel.ReqContext) response.Respons // 401: unauthorisedError // 403: forbiddenError // 500: internalServerError -func (hs *HTTPServer) GetLDAPStatus(c *contextmodel.ReqContext) response.Response { +func (s *Service) GetLDAPStatus(c *contextmodel.ReqContext) response.Response { if !ldap.IsEnabled() { return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } - ldapConfig, err := getLDAPConfig(hs.Cfg) + ldapConfig, err := getLDAPConfig(s.cfg) if err != nil { return response.Error(http.StatusBadRequest, "Failed to obtain the LDAP configuration. Please verify the configuration and try again", err) } @@ -196,12 +237,12 @@ func (hs *HTTPServer) GetLDAPStatus(c *contextmodel.ReqContext) response.Respons // 401: unauthorisedError // 403: forbiddenError // 500: internalServerError -func (hs *HTTPServer) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response.Response { +func (s *Service) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response.Response { if !ldap.IsEnabled() { return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } - ldapConfig, err := getLDAPConfig(hs.Cfg) + ldapConfig, err := getLDAPConfig(s.cfg) if err != nil { return response.Error(http.StatusBadRequest, "Failed to obtain the LDAP configuration. Please verify the configuration and try again", err) } @@ -213,7 +254,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response. query := user.GetUserByIDQuery{ID: userId} - usr, err := hs.userService.GetByID(c.Req.Context(), &query) + usr, err := s.userService.GetByID(c.Req.Context(), &query) if err != nil { // validate the userId exists if errors.Is(err, user.ErrUserNotFound) { return response.Error(404, user.ErrUserNotFound.Error(), nil) @@ -223,7 +264,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response. } authModuleQuery := &login.GetAuthInfoQuery{UserId: usr.ID, AuthModule: login.LDAPAuthModule} - if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), authModuleQuery); err != nil { // validate the userId comes from LDAP + if err := s.authInfoService.GetAuthInfo(c.Req.Context(), authModuleQuery); err != nil { // validate the userId comes from LDAP if errors.Is(err, user.ErrUserNotFound) { return response.Error(404, user.ErrUserNotFound.Error(), nil) } @@ -235,19 +276,19 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response. userInfo, _, err := ldapServer.User(usr.Login) if err != nil { if errors.Is(err, multildap.ErrDidNotFindUser) { // User was not in the LDAP server - we need to take action: - if hs.Cfg.AdminUser == usr.Login { // User is *the* Grafana Admin. We cannot disable it. + if s.cfg.AdminUser == usr.Login { // User is *the* Grafana Admin. We cannot disable it. errMsg := fmt.Sprintf(`Refusing to sync grafana super admin "%s" - it would be disabled`, usr.Login) - ldapLogger.Error(errMsg) + s.log.Error(errMsg) return response.Error(http.StatusBadRequest, errMsg, err) } // Since the user was not in the LDAP server. Let's disable it. - err := hs.Login.DisableExternalUser(c.Req.Context(), usr.Login) + err := s.loginService.DisableExternalUser(c.Req.Context(), usr.Login) if err != nil { return response.Error(http.StatusInternalServerError, "Failed to disable the user", err) } - err = hs.AuthTokenService.RevokeAllUserTokens(c.Req.Context(), userId) + err = s.sessionService.RevokeAllUserTokens(c.Req.Context(), userId) if err != nil { return response.Error(http.StatusInternalServerError, "Failed to remove session tokens for the user", err) } @@ -255,14 +296,14 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response. return response.Error(http.StatusBadRequest, "User not found in LDAP. Disabled the user without updating information", nil) // should this be a success? } - ldapLogger.Debug("Failed to sync the user with LDAP", "err", err) + s.log.Debug("Failed to sync the user with LDAP", "err", err) return response.Error(http.StatusBadRequest, "Something went wrong while finding the user in LDAP", err) } upsertCmd := &login.UpsertUserCommand{ ReqContext: c, ExternalUser: userInfo, - SignupAllowed: hs.Cfg.LDAPAllowSignup, + SignupAllowed: s.cfg.LDAPAllowSignup, UserLookupParams: login.UserLookupParams{ UserID: &usr.ID, // Upsert by ID only Email: nil, @@ -270,7 +311,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response. }, } - err = hs.Login.UpsertUser(c.Req.Context(), upsertCmd) + err = s.loginService.UpsertUser(c.Req.Context(), upsertCmd) if err != nil { return response.Error(http.StatusInternalServerError, "Failed to update the user", err) } @@ -292,12 +333,12 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response. // 401: unauthorisedError // 403: forbiddenError // 500: internalServerError -func (hs *HTTPServer) GetUserFromLDAP(c *contextmodel.ReqContext) response.Response { +func (s *Service) GetUserFromLDAP(c *contextmodel.ReqContext) response.Response { if !ldap.IsEnabled() { return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } - ldapConfig, err := getLDAPConfig(hs.Cfg) + ldapConfig, err := getLDAPConfig(s.cfg) if err != nil { return response.Error(http.StatusBadRequest, "Failed to obtain the LDAP configuration", err) } @@ -315,7 +356,7 @@ func (hs *HTTPServer) GetUserFromLDAP(c *contextmodel.ReqContext) response.Respo return response.Error(http.StatusNotFound, "No user was found in the LDAP server(s) with that username", err) } - ldapLogger.Debug("user found", "user", user) + s.log.Debug("user found", "user", user) name, surname := splitName(user.Name) @@ -354,12 +395,12 @@ func (hs *HTTPServer) GetUserFromLDAP(c *contextmodel.ReqContext) response.Respo u.OrgRoles = append(u.OrgRoles, LDAPRoleDTO{GroupDN: userGroup}) } - ldapLogger.Debug("mapping org roles", "orgsRoles", u.OrgRoles) - if err := u.FetchOrgs(c.Req.Context(), hs.orgService); err != nil { + s.log.Debug("mapping org roles", "orgsRoles", u.OrgRoles) + if err := u.FetchOrgs(c.Req.Context(), s.orgService); err != nil { return response.Error(http.StatusBadRequest, "An organization was not found - Please verify your LDAP configuration", err) } - u.Teams, err = hs.ldapGroups.GetTeams(user.Groups, orgIDs) + u.Teams, err = s.ldapGroupsService.GetTeams(user.Groups, orgIDs) if err != nil { return response.Error(http.StatusBadRequest, "Unable to find the teams for this user", err) } diff --git a/pkg/api/ldap_debug_test.go b/pkg/services/ldap/api/service_test.go similarity index 52% rename from pkg/api/ldap_debug_test.go rename to pkg/services/ldap/api/service_test.go index 5e4e51fd750..ceb183c95ec 100644 --- a/pkg/api/ldap_debug_test.go +++ b/pkg/services/ldap/api/service_test.go @@ -3,25 +3,22 @@ package api import ( "encoding/json" "errors" + "io" "net/http" - "net/http/httptest" - "path/filepath" + "os" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" - "github.com/grafana/grafana/pkg/infra/db/dbtest" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/auth/authtest" - contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/ldap" + "github.com/grafana/grafana/pkg/services/ldap/multildap" "github.com/grafana/grafana/pkg/services/login" - "github.com/grafana/grafana/pkg/services/login/loginservice" "github.com/grafana/grafana/pkg/services/login/logintest" - "github.com/grafana/grafana/pkg/services/multildap" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org/orgtest" "github.com/grafana/grafana/pkg/services/user" @@ -31,12 +28,12 @@ import ( ) type LDAPMock struct { - Results []*login.ExternalUserInfo + Results []*login.ExternalUserInfo + UserSearchResult *login.ExternalUserInfo + UserSearchConfig ldap.ServerConfig + UserSearchError error } -var userSearchResult *login.ExternalUserInfo -var userSearchConfig ldap.ServerConfig -var userSearchError error var pingResult []*multildap.ServerStatus var pingError error @@ -54,59 +51,76 @@ func (m *LDAPMock) Users(logins []string) ([]*login.ExternalUserInfo, error) { } func (m *LDAPMock) User(login string) (*login.ExternalUserInfo, ldap.ServerConfig, error) { - return userSearchResult, userSearchConfig, userSearchError + return m.UserSearchResult, m.UserSearchConfig, m.UserSearchError } -// *** -// GetUserFromLDAP tests -// *** - -func getUserFromLDAPContext(t *testing.T, requestURL string, searchOrgRst []*org.OrgDTO) *scenarioContext { +func setupAPITest(t *testing.T, opts ...func(a *Service)) (*Service, *webtest.Server) { t.Helper() + router := routing.NewRouteRegister() + cfg := setting.NewCfg() + + a := ProvideService(cfg, + router, + acimpl.ProvideAccessControl(cfg), + usertest.NewUserServiceFake(), + &logintest.AuthInfoServiceFake{}, + ldap.ProvideGroupsService(), + &logintest.LoginServiceFake{}, + &orgtest.FakeOrgService{}, + authtest.NewFakeUserAuthTokenService(), + ) + + for _, o := range opts { + o(a) + } - sc := setupScenarioContext(t, requestURL) - - origLDAP := setting.LDAPEnabled - setting.LDAPEnabled = true - t.Cleanup(func() { setting.LDAPEnabled = origLDAP }) - - hs := &HTTPServer{Cfg: setting.NewCfg(), ldapGroups: ldap.ProvideGroupsService(), orgService: &orgtest.FakeOrgService{ExpectedOrgs: searchOrgRst}} - - sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { - sc.context = c - return hs.GetUserFromLDAP(c) - }) - - sc.m.Get("/api/admin/ldap/:username", sc.defaultHandler) - - sc.resp = httptest.NewRecorder() - req, _ := http.NewRequest(http.MethodGet, requestURL, nil) - sc.req = req - sc.exec() + server := webtest.NewServer(t, router) - return sc + return a, server } func TestGetUserFromLDAPAPIEndpoint_UserNotFound(t *testing.T) { + setting.LDAPEnabled = true + defer func() { setting.LDAPEnabled = false }() + getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { return &ldap.Config{}, nil } newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} + return &LDAPMock{ + UserSearchResult: nil, + } } - userSearchResult = nil + _, server := setupAPITest(t, func(a *Service) { + a.orgService = &orgtest.FakeOrgService{ + ExpectedOrgs: []*org.OrgDTO{}, + } + }) - sc := getUserFromLDAPContext(t, "/api/admin/ldap/user-that-does-not-exist", []*org.OrgDTO{}) + req := server.NewGetRequest("/api/admin/ldap/user-that-does-not-exist") + webtest.RequestWithSignedInUser(req, &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: {"ldap.user:read": {"*"}}}, + }) - 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()) + res, err := server.Send(req) + defer func() { require.NoError(t, res.Body.Close()) }() + require.NoError(t, err) + + assert.Equal(t, http.StatusNotFound, res.StatusCode) + bodyBytes, _ := io.ReadAll(res.Body) + assert.JSONEq(t, "{\"message\":\"No user was found in the LDAP server(s) with that username\"}", string(bodyBytes)) } func TestGetUserFromLDAPAPIEndpoint_OrgNotfound(t *testing.T) { + setting.LDAPEnabled = true + defer func() { setting.LDAPEnabled = false }() + isAdmin := true - userSearchResult = &login.ExternalUserInfo{ + userSearchResult := &login.ExternalUserInfo{ Name: "John Doe", Email: "john.doe@example.com", Login: "johndoe", @@ -115,7 +129,7 @@ func TestGetUserFromLDAPAPIEndpoint_OrgNotfound(t *testing.T) { IsGrafanaAdmin: &isAdmin, } - userSearchConfig = ldap.ServerConfig{ + userSearchConfig := ldap.ServerConfig{ Attr: ldap.AttributeMap{ Name: "ldap-name", Surname: "ldap-surname", @@ -136,32 +150,54 @@ func TestGetUserFromLDAPAPIEndpoint_OrgNotfound(t *testing.T) { }, } + newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { + return &LDAPMock{ + UserSearchResult: userSearchResult, + UserSearchConfig: userSearchConfig, + } + } + mockOrgSearchResult := []*org.OrgDTO{ {ID: 1, Name: "Main Org."}, } + _, server := setupAPITest(t, func(a *Service) { + a.orgService = &orgtest.FakeOrgService{ + ExpectedOrgs: mockOrgSearchResult, + } + }) + getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { return &ldap.Config{}, nil } - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} - } + req := server.NewGetRequest("/api/admin/ldap/johndoe") + webtest.RequestWithSignedInUser(req, &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: {"ldap.user:read": {"*"}}}, + }) - sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe", mockOrgSearchResult) + res, err := server.Send(req) + defer func() { require.NoError(t, res.Body.Close()) }() + require.NoError(t, err) - require.Equal(t, http.StatusBadRequest, sc.resp.Code) + assert.Equal(t, http.StatusBadRequest, res.StatusCode) + bodyBytes, _ := io.ReadAll(res.Body) - var res map[string]interface{} - err := json.Unmarshal(sc.resp.Body.Bytes(), &res) + var resMap map[string]interface{} + err = json.Unmarshal(bodyBytes, &resMap) assert.NoError(t, err) - assert.Equal(t, "unable to find organization with ID '2'", res["error"]) - assert.Equal(t, "An organization was not found - Please verify your LDAP configuration", res["message"]) + assert.Equal(t, "unable to find organization with ID '2'", resMap["error"]) + assert.Equal(t, "An organization was not found - Please verify your LDAP configuration", resMap["message"]) } func TestGetUserFromLDAPAPIEndpoint(t *testing.T) { + setting.LDAPEnabled = true + defer func() { setting.LDAPEnabled = false }() + isAdmin := true - userSearchResult = &login.ExternalUserInfo{ + userSearchResult := &login.ExternalUserInfo{ Name: "John Doe", Email: "john.doe@example.com", Login: "johndoe", @@ -170,7 +206,7 @@ func TestGetUserFromLDAPAPIEndpoint(t *testing.T) { IsGrafanaAdmin: &isAdmin, } - userSearchConfig = ldap.ServerConfig{ + userSearchConfig := ldap.ServerConfig{ Attr: ldap.AttributeMap{ Name: "ldap-name", Surname: "ldap-surname", @@ -200,12 +236,30 @@ func TestGetUserFromLDAPAPIEndpoint(t *testing.T) { } newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} + return &LDAPMock{ + UserSearchResult: userSearchResult, + UserSearchConfig: userSearchConfig, + } } - sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe", mockOrgSearchResult) + _, server := setupAPITest(t, func(a *Service) { + a.orgService = &orgtest.FakeOrgService{ + ExpectedOrgs: mockOrgSearchResult, + } + }) + + req := server.NewGetRequest("/api/admin/ldap/johndoe") + webtest.RequestWithSignedInUser(req, &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: {"ldap.user:read": {"*"}}}, + }) - assert.Equal(t, sc.resp.Code, http.StatusOK) + res, err := server.Send(req) + defer func() { require.NoError(t, res.Body.Close()) }() + require.NoError(t, err) + + assert.Equal(t, http.StatusOK, res.StatusCode) expected := ` { @@ -231,12 +285,16 @@ func TestGetUserFromLDAPAPIEndpoint(t *testing.T) { } ` - assert.JSONEq(t, expected, sc.resp.Body.String()) + bodyBytes, _ := io.ReadAll(res.Body) + assert.JSONEq(t, expected, string(bodyBytes)) } func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) { + setting.LDAPEnabled = true + defer func() { setting.LDAPEnabled = false }() + isAdmin := true - userSearchResult = &login.ExternalUserInfo{ + userSearchResult := &login.ExternalUserInfo{ Name: "John Doe", Email: "john.doe@example.com", Login: "johndoe", @@ -245,7 +303,7 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) { IsGrafanaAdmin: &isAdmin, } - userSearchConfig = ldap.ServerConfig{ + userSearchConfig := ldap.ServerConfig{ Attr: ldap.AttributeMap{ Name: "ldap-name", Surname: "ldap-surname", @@ -270,12 +328,30 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) { } newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} + return &LDAPMock{ + UserSearchResult: userSearchResult, + UserSearchConfig: userSearchConfig, + } } - sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe", mockOrgSearchResult) + _, server := setupAPITest(t, func(a *Service) { + a.orgService = &orgtest.FakeOrgService{ + ExpectedOrgs: mockOrgSearchResult, + } + }) - require.Equal(t, sc.resp.Code, http.StatusOK) + req := server.NewGetRequest("/api/admin/ldap/johndoe") + webtest.RequestWithSignedInUser(req, &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: {"ldap.user:read": {"*"}}}, + }) + + res, err := server.Send(req) + defer func() { require.NoError(t, res.Body.Close()) }() + require.NoError(t, err) + + assert.Equal(t, http.StatusOK, res.StatusCode) expected := ` { @@ -300,41 +376,14 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) { } ` - assert.JSONEq(t, expected, sc.resp.Body.String()) + bodyBytes, _ := io.ReadAll(res.Body) + assert.JSONEq(t, expected, string(bodyBytes)) } -// *** -// GetLDAPStatus tests -// *** - -func getLDAPStatusContext(t *testing.T) *scenarioContext { - t.Helper() - - requestURL := "/api/admin/ldap/status" - sc := setupScenarioContext(t, requestURL) - - ldap := setting.LDAPEnabled +func TestGetLDAPStatusAPIEndpoint(t *testing.T) { setting.LDAPEnabled = true - t.Cleanup(func() { setting.LDAPEnabled = ldap }) - - hs := &HTTPServer{Cfg: setting.NewCfg()} - - sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { - sc.context = c - return hs.GetLDAPStatus(c) - }) - - sc.m.Get("/api/admin/ldap/status", sc.defaultHandler) - - sc.resp = httptest.NewRecorder() - req, _ := http.NewRequest(http.MethodGet, requestURL, nil) - sc.req = req - sc.exec() - - return sc -} + defer func() { setting.LDAPEnabled = false }() -func TestGetLDAPStatusAPIEndpoint(t *testing.T) { pingResult = []*multildap.ServerStatus{ {Host: "10.0.0.3", Port: 361, Available: true, Error: nil}, {Host: "10.0.0.3", Port: 362, Available: true, Error: nil}, @@ -349,9 +398,21 @@ func TestGetLDAPStatusAPIEndpoint(t *testing.T) { return &LDAPMock{} } - sc := getLDAPStatusContext(t) + _, server := setupAPITest(t, func(a *Service) { + }) + + req := server.NewGetRequest("/api/admin/ldap/status") + webtest.RequestWithSignedInUser(req, &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: {"ldap.status:read": {}}}, + }) + + res, err := server.Send(req) + defer func() { require.NoError(t, res.Body.Close()) }() + require.NoError(t, err) - require.Equal(t, http.StatusOK, sc.resp.Code) + assert.Equal(t, http.StatusOK, res.StatusCode) expected := ` [ @@ -360,70 +421,44 @@ func TestGetLDAPStatusAPIEndpoint(t *testing.T) { { "host": "10.0.0.5", "port": 361, "available": false, "error": "something is awfully wrong" } ] ` - assert.JSONEq(t, expected, sc.resp.Body.String()) -} -// *** -// PostSyncUserWithLDAP tests -// *** + bodyBytes, _ := io.ReadAll(res.Body) + assert.JSONEq(t, expected, string(bodyBytes)) +} -func postSyncUserWithLDAPContext(t *testing.T, requestURL string, preHook func(*testing.T, *scenarioContext), userService user.Service) *scenarioContext { - t.Helper() +func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) { + setting.LDAPEnabled = true + defer func() { setting.LDAPEnabled = false }() - sc := setupScenarioContext(t, requestURL) - sc.authInfoService = &logintest.AuthInfoServiceFake{} + userServiceMock := usertest.NewUserServiceFake() + userServiceMock.ExpectedUser = &user.User{Login: "ldap-daniel", ID: 34} - ldap := setting.LDAPEnabled - t.Cleanup(func() { - setting.LDAPEnabled = ldap - }) - setting.LDAPEnabled = true + getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { + return &ldap.Config{}, nil + } - hs := &HTTPServer{ - Cfg: sc.cfg, - AuthTokenService: authtest.NewFakeUserAuthTokenService(), - Login: loginservice.LoginServiceMock{}, - authInfoService: sc.authInfoService, - userService: userService, + newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { + return &LDAPMock{UserSearchResult: &login.ExternalUserInfo{ + Login: "ldap-daniel", + }} } - sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { - sc.context = c - return hs.PostSyncUserWithLDAP(c) + _, server := setupAPITest(t, func(a *Service) { + a.userService = userServiceMock }) - sc.m.Post("/api/admin/ldap/sync/:id", sc.defaultHandler) + req := server.NewPostRequest("/api/admin/ldap/sync/34", nil) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: {"ldap.user:sync": {}}}, + }) - sc.resp = httptest.NewRecorder() - req, err := http.NewRequest(http.MethodPost, requestURL, nil) + res, err := server.Send(req) + defer func() { require.NoError(t, res.Body.Close()) }() require.NoError(t, err) - preHook(t, sc) - - sc.req = req - sc.exec() - - return sc -} - -func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) { - userServiceMock := usertest.NewUserServiceFake() - userServiceMock.ExpectedUser = &user.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 - } - - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} - } - - userSearchResult = &login.ExternalUserInfo{ - Login: "ldap-daniel", - } - }, userServiceMock) - - assert.Equal(t, http.StatusOK, sc.resp.Code) + assert.Equal(t, http.StatusOK, res.StatusCode) expected := ` { @@ -431,23 +466,40 @@ func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) { } ` - assert.JSONEq(t, expected, sc.resp.Body.String()) + bodyBytes, _ := io.ReadAll(res.Body) + assert.JSONEq(t, expected, string(bodyBytes)) } func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) { + setting.LDAPEnabled = true + defer func() { setting.LDAPEnabled = false }() userServiceMock := usertest.NewUserServiceFake() userServiceMock.ExpectedError = user.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 - } - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} - } - }, userServiceMock) + getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { + return &ldap.Config{}, nil + } + + newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { + return &LDAPMock{} + } - assert.Equal(t, http.StatusNotFound, sc.resp.Code) + _, server := setupAPITest(t, func(a *Service) { + a.userService = userServiceMock + }) + + req := server.NewPostRequest("/api/admin/ldap/sync/34", nil) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: {"ldap.user:sync": {}}}, + }) + + res, err := server.Send(req) + defer func() { require.NoError(t, res.Body.Close()) }() + require.NoError(t, err) + + assert.Equal(t, http.StatusNotFound, res.StatusCode) expected := ` { @@ -455,52 +507,83 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) { } ` - assert.JSONEq(t, expected, sc.resp.Body.String()) + bodyBytes, _ := io.ReadAll(res.Body) + assert.JSONEq(t, expected, string(bodyBytes)) } func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) { + setting.LDAPEnabled = true + defer func() { setting.LDAPEnabled = false }() + userServiceMock := usertest.NewUserServiceFake() userServiceMock.ExpectedUser = &user.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 - } - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} - } + getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { + return &ldap.Config{}, nil + } + + newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { + return &LDAPMock{UserSearchError: multildap.ErrDidNotFindUser} + } + + _, server := setupAPITest(t, func(a *Service) { + a.userService = userServiceMock + a.cfg.AdminUser = "ldap-daniel" + }) - userSearchError = multildap.ErrDidNotFindUser + req := server.NewPostRequest("/api/admin/ldap/sync/34", nil) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: {"ldap.user:sync": {}}}, + }) + + res, err := server.Send(req) + defer func() { require.NoError(t, res.Body.Close()) }() + require.NoError(t, err) - sc.cfg.AdminUser = "ldap-daniel" - }, userServiceMock) - assert.Equal(t, http.StatusBadRequest, sc.resp.Code) + assert.Equal(t, http.StatusBadRequest, res.StatusCode) - var res map[string]interface{} - err := json.Unmarshal(sc.resp.Body.Bytes(), &res) + bodyBytes, _ := io.ReadAll(res.Body) + var resMap map[string]interface{} + err = json.Unmarshal(bodyBytes, &resMap) assert.NoError(t, err) - assert.Equal(t, "did not find a user", res["error"]) - assert.Equal(t, "Refusing to sync grafana super admin \"ldap-daniel\" - it would be disabled", res["message"]) + assert.Equal(t, "did not find a user", resMap["error"]) + assert.Equal(t, "Refusing to sync grafana super admin \"ldap-daniel\" - it would be disabled", resMap["message"]) } func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) { + setting.LDAPEnabled = true + defer func() { setting.LDAPEnabled = false }() + userServiceMock := usertest.NewUserServiceFake() userServiceMock.ExpectedUser = &user.User{Login: "ldap-daniel", ID: 34} - sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34", func(t *testing.T, sc *scenarioContext) { - sc.authInfoService.ExpectedExternalUser = &login.ExternalUserInfo{IsDisabled: true, UserId: 34} - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { - return &ldap.Config{}, nil - } - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} - } + getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { + return &ldap.Config{}, nil + } - userSearchResult = nil - userSearchError = multildap.ErrDidNotFindUser - }, userServiceMock) + newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { + return &LDAPMock{UserSearchError: multildap.ErrDidNotFindUser} + } + + _, server := setupAPITest(t, func(a *Service) { + a.userService = userServiceMock + a.authInfoService = &logintest.AuthInfoServiceFake{ExpectedExternalUser: &login.ExternalUserInfo{IsDisabled: true, UserId: 34}} + }) + + req := server.NewPostRequest("/api/admin/ldap/sync/34", nil) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: {"ldap.user:sync": {}}}, + }) - assert.Equal(t, http.StatusBadRequest, sc.resp.Code) + res, err := server.Send(req) + defer func() { require.NoError(t, res.Body.Close()) }() + require.NoError(t, err) + + assert.Equal(t, http.StatusBadRequest, res.StatusCode) expected := ` { @@ -508,14 +591,46 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) { } ` - assert.JSONEq(t, expected, sc.resp.Body.String()) + bodyBytes, _ := io.ReadAll(res.Body) + assert.JSONEq(t, expected, string(bodyBytes)) } -// *** -// Access control tests for ldap endpoints -// *** - func TestLDAP_AccessControl(t *testing.T) { + setting.LDAPEnabled = true + + f, errC := os.CreateTemp("", "ldap.toml") + require.NoError(t, errC) + + _, errF := f.WriteString( + `[[servers]] +host = "127.0.0.1" +port = 389 +search_filter = "(cn=%s)" +search_base_dns = ["dc=grafana,dc=org"]`) + require.NoError(t, errF) + + setting.LDAPConfigFile = f.Name() + + errF = f.Close() + require.NoError(t, errF) + + defer func() { + setting.LDAPEnabled = false + setting.LDAPConfigFile = "" + }() + + getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { + return &ldap.Config{}, nil + } + + newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { + return &LDAPMock{ + UserSearchResult: &login.ExternalUserInfo{ + Login: "ldap-daniel", + }, + } + } + type testCase struct { desc string method string @@ -600,41 +715,22 @@ func TestLDAP_AccessControl(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - enabled := setting.LDAPEnabled - configFile := setting.LDAPConfigFile - - t.Cleanup(func() { - setting.LDAPEnabled = enabled - setting.LDAPConfigFile = configFile - }) - - setting.LDAPEnabled = true - path, err := filepath.Abs("../../conf/ldap.toml") - assert.NoError(t, err) - setting.LDAPConfigFile = path - - server := SetupAPITestServer(t, func(hs *HTTPServer) { - cfg := setting.NewCfg() - cfg.LDAPEnabled = true - hs.Cfg = cfg - hs.SQLStore = dbtest.NewFakeDB() - hs.orgService = orgtest.NewOrgServiceFake() - hs.userService = &usertest.FakeUserService{ExpectedUser: &user.User{}} - hs.ldapGroups = &ldap.OSSGroups{} - hs.Login = &loginservice.LoginServiceMock{} - hs.authInfoService = &logintest.AuthInfoServiceFake{} + _, server := setupAPITest(t, func(a *Service) { + a.userService = &usertest.FakeUserService{ExpectedUser: &user.User{Login: "ldap-daniel", ID: 1}} }) // Add minimal setup to pass handler - userSearchResult = &login.ExternalUserInfo{} - userSearchError = nil - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} - } - - res, err := server.Send(webtest.RequestWithSignedInUser(server.NewRequest(tt.method, tt.url, nil), userWithPermissions(1, tt.permissions))) + res, err := server.Send( + webtest.RequestWithSignedInUser(server.NewRequest(tt.method, tt.url, nil), + userWithPermissions(1, tt.permissions))) require.NoError(t, err) - assert.Equal(t, tt.expectedCode, res.StatusCode) + + bodyBytes, _ := io.ReadAll(res.Body) + assert.Equal(t, tt.expectedCode, res.StatusCode, string(bodyBytes)) require.NoError(t, res.Body.Close()) }) } } + +func userWithPermissions(orgID int64, permissions []accesscontrol.Permission) *user.SignedInUser { + return &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(permissions)}} +} diff --git a/pkg/services/multildap/multidap_mock.go b/pkg/services/ldap/multildap/multidap_mock.go similarity index 100% rename from pkg/services/multildap/multidap_mock.go rename to pkg/services/ldap/multildap/multidap_mock.go diff --git a/pkg/services/multildap/multildap.go b/pkg/services/ldap/multildap/multildap.go similarity index 100% rename from pkg/services/multildap/multildap.go rename to pkg/services/ldap/multildap/multildap.go diff --git a/pkg/services/multildap/multildap_test.go b/pkg/services/ldap/multildap/multildap_test.go similarity index 100% rename from pkg/services/multildap/multildap_test.go rename to pkg/services/ldap/multildap/multildap_test.go