diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 442c25684e8..f2f32e9f38d 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -43,6 +43,7 @@ import ( "github.com/grafana/grafana/pkg/services/folder/folderimpl" "github.com/grafana/grafana/pkg/services/folder/foldertest" "github.com/grafana/grafana/pkg/services/guardian" + "github.com/grafana/grafana/pkg/services/ldap/service" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/loginservice" @@ -218,7 +219,7 @@ func getContextHandler(t *testing.T, cfg *setting.Cfg) *contexthandler.ContextHa renderSvc := &fakeRenderService{} authJWTSvc := jwt.NewFakeJWTService() tracer := tracing.InitializeTracerForTest() - authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginservice.LoginServiceMock{}, &usertest.FakeUserService{}, sqlStore) + authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginservice.LoginServiceMock{}, &usertest.FakeUserService{}, sqlStore, service.NewLDAPFakeService()) loginService := &logintest.LoginServiceFake{} authenticator := &logintest.AuthenticatorFake{} ctxHdlr := contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, sqlStore, tracer, authProxy, loginService, nil, authenticator, usertest.NewUserServiceFake(), orgtest.NewOrgServiceFake(), nil, featuremgmt.WithFeatures(), &authntest.FakeService{}) diff --git a/pkg/login/auth.go b/pkg/login/auth.go index 0a22319f0f9..ad26e68b41e 100644 --- a/pkg/login/auth.go +++ b/pkg/login/auth.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/loginattempt" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/setting" ) var ( @@ -36,13 +37,17 @@ type AuthenticatorService struct { loginService login.Service loginAttemptService loginattempt.Service userService user.Service + cfg *setting.Cfg } -func ProvideService(store db.DB, loginService login.Service, loginAttemptService loginattempt.Service, userService user.Service) *AuthenticatorService { +func ProvideService(store db.DB, loginService login.Service, + loginAttemptService loginattempt.Service, + userService user.Service, cfg *setting.Cfg) *AuthenticatorService { a := &AuthenticatorService{ loginService: loginService, loginAttemptService: loginAttemptService, userService: userService, + cfg: cfg, } return a } @@ -73,7 +78,7 @@ func (a *AuthenticatorService) AuthenticateUser(ctx context.Context, query *logi return err } - ldapEnabled, ldapErr := loginUsingLDAP(ctx, query, a.loginService) + ldapEnabled, ldapErr := loginUsingLDAP(ctx, query, a.loginService, a.cfg) if ldapEnabled { query.AuthModule = login.LDAPAuthModule if ldapErr == nil || !errors.Is(ldapErr, ldap.ErrInvalidCredentials) { diff --git a/pkg/login/auth_test.go b/pkg/login/auth_test.go index 8720c6e0ea4..a7a064a1d48 100644 --- a/pkg/login/auth_test.go +++ b/pkg/login/auth_test.go @@ -22,7 +22,8 @@ func TestAuthenticateUser(t *testing.T) { mockLoginUsingLDAP(false, nil, sc) loginAttemptService := &loginattempttest.FakeLoginAttemptService{ExpectedValid: true} - a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}} + cfg := setting.NewCfg() + a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}, cfg: cfg} err := a.AuthenticateUser(context.Background(), &login.LoginUserQuery{ Username: "user", Password: "", @@ -35,10 +36,11 @@ func TestAuthenticateUser(t *testing.T) { }) authScenario(t, "When user authenticates with no auth provider enabled", func(sc *authScenarioContext) { + cfg := setting.NewCfg() sc.loginUserQuery.Cfg.DisableLogin = true loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true} - a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}} + a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}, cfg: cfg} err := a.AuthenticateUser(context.Background(), sc.loginUserQuery) require.EqualError(t, err, ErrNoAuthProvider.Error()) @@ -50,11 +52,12 @@ func TestAuthenticateUser(t *testing.T) { }) authScenario(t, "When a user authenticates having too many login attempts", func(sc *authScenarioContext) { + cfg := setting.NewCfg() mockLoginUsingGrafanaDB(nil, sc) mockLoginUsingLDAP(true, nil, sc) loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: false} - a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}} + a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}, cfg: cfg} err := a.AuthenticateUser(context.Background(), sc.loginUserQuery) require.EqualError(t, err, ErrTooManyLoginAttempts.Error()) @@ -66,11 +69,12 @@ func TestAuthenticateUser(t *testing.T) { }) authScenario(t, "When grafana user authenticate with valid credentials", func(sc *authScenarioContext) { + cfg := setting.NewCfg() mockLoginUsingGrafanaDB(nil, sc) mockLoginUsingLDAP(true, ErrInvalidCredentials, sc) loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true} - a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}} + a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}, cfg: cfg} err := a.AuthenticateUser(context.Background(), sc.loginUserQuery) require.NoError(t, err) @@ -82,12 +86,13 @@ func TestAuthenticateUser(t *testing.T) { }) authScenario(t, "When grafana user authenticate and unexpected error occurs", func(sc *authScenarioContext) { + cfg := setting.NewCfg() customErr := errors.New("custom") mockLoginUsingGrafanaDB(customErr, sc) mockLoginUsingLDAP(true, ErrInvalidCredentials, sc) loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true} - a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}} + a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}, cfg: cfg} err := a.AuthenticateUser(context.Background(), sc.loginUserQuery) require.EqualError(t, err, customErr.Error()) @@ -99,11 +104,12 @@ func TestAuthenticateUser(t *testing.T) { }) authScenario(t, "When a non-existing grafana user authenticate and ldap disabled", func(sc *authScenarioContext) { + cfg := setting.NewCfg() mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(false, nil, sc) loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true} - a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}} + a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}, cfg: cfg} err := a.AuthenticateUser(context.Background(), sc.loginUserQuery) require.EqualError(t, err, user.ErrUserNotFound.Error()) @@ -115,11 +121,13 @@ func TestAuthenticateUser(t *testing.T) { }) authScenario(t, "When a non-existing grafana user authenticate and invalid ldap credentials", func(sc *authScenarioContext) { + cfg := setting.NewCfg() + cfg.LDAPEnabled = true mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(true, ldap.ErrInvalidCredentials, sc) loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true} - a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}} + a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}, cfg: cfg} err := a.AuthenticateUser(context.Background(), sc.loginUserQuery) require.EqualError(t, err, ErrInvalidCredentials.Error()) @@ -131,11 +139,13 @@ func TestAuthenticateUser(t *testing.T) { }) authScenario(t, "When a non-existing grafana user authenticate and valid ldap credentials", func(sc *authScenarioContext) { + cfg := setting.NewCfg() + cfg.LDAPEnabled = true mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(true, nil, sc) loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true} - a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}} + a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}, cfg: cfg} err := a.AuthenticateUser(context.Background(), sc.loginUserQuery) require.NoError(t, err) @@ -147,12 +157,14 @@ func TestAuthenticateUser(t *testing.T) { }) authScenario(t, "When a non-existing grafana user authenticate and ldap returns unexpected error", func(sc *authScenarioContext) { + cfg := setting.NewCfg() + cfg.LDAPEnabled = true customErr := errors.New("custom") mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(true, customErr, sc) loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true} - a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}} + a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}, cfg: cfg} err := a.AuthenticateUser(context.Background(), sc.loginUserQuery) require.EqualError(t, err, customErr.Error()) @@ -164,11 +176,13 @@ func TestAuthenticateUser(t *testing.T) { }) authScenario(t, "When grafana user authenticate with invalid credentials and invalid ldap credentials", func(sc *authScenarioContext) { + cfg := setting.NewCfg() + cfg.LDAPEnabled = true mockLoginUsingGrafanaDB(ErrInvalidCredentials, sc) mockLoginUsingLDAP(true, ldap.ErrInvalidCredentials, sc) loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true} - a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}} + a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}, cfg: cfg} err := a.AuthenticateUser(context.Background(), sc.loginUserQuery) require.EqualError(t, err, ErrInvalidCredentials.Error()) @@ -195,7 +209,7 @@ func mockLoginUsingGrafanaDB(err error, sc *authScenarioContext) { } func mockLoginUsingLDAP(enabled bool, err error, sc *authScenarioContext) { - loginUsingLDAP = func(ctx context.Context, query *login.LoginUserQuery, _ login.Service) (bool, error) { + loginUsingLDAP = func(ctx context.Context, query *login.LoginUserQuery, _ login.Service, _ *setting.Cfg) (bool, error) { sc.ldapLoginWasCalled = true return enabled, err } diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go index 8196f4517e8..746ae2f7e80 100644 --- a/pkg/login/ldap_login.go +++ b/pkg/login/ldap_login.go @@ -15,9 +15,6 @@ import ( // getLDAPConfig gets LDAP config var getLDAPConfig = multildap.GetConfig -// isLDAPEnabled checks if LDAP is enabled -var isLDAPEnabled = multildap.IsEnabled - // newLDAP creates multiple LDAP instance var newLDAP = multildap.New @@ -26,10 +23,9 @@ var ldapLogger = log.New("login.ldap") // loginUsingLDAP logs in user using LDAP. It returns whether LDAP is enabled and optional error and query arg will be // populated with the logged in user if successful. -var loginUsingLDAP = func(ctx context.Context, query *login.LoginUserQuery, loginService login.Service) (bool, error) { - enabled := isLDAPEnabled() - - if !enabled { +var loginUsingLDAP = func(ctx context.Context, query *login.LoginUserQuery, + loginService login.Service, cfg *setting.Cfg) (bool, error) { + if !cfg.LDAPEnabled { return false, nil } @@ -38,7 +34,7 @@ var loginUsingLDAP = func(ctx context.Context, query *login.LoginUserQuery, logi return true, fmt.Errorf("%v: %w", "Failed to get LDAP config", err) } - externalUser, err := newLDAP(config.Servers).Login(query) + externalUser, err := newLDAP(config.Servers, cfg).Login(query) if err != nil { if errors.Is(err, ldap.ErrCouldNotFindUser) { // Ignore the error since user might not be present anyway @@ -56,7 +52,7 @@ var loginUsingLDAP = func(ctx context.Context, query *login.LoginUserQuery, logi upsert := &login.UpsertUserCommand{ ReqContext: query.ReqContext, ExternalUser: externalUser, - SignupAllowed: setting.LDAPAllowSignup, + SignupAllowed: cfg.LDAPAllowSignup, UserLookupParams: login.UserLookupParams{ Login: &externalUser.Login, Email: &externalUser.Email, diff --git a/pkg/login/ldap_login_test.go b/pkg/login/ldap_login_test.go index 23294abd037..42d0baf7ee3 100644 --- a/pkg/login/ldap_login_test.go +++ b/pkg/login/ldap_login_test.go @@ -19,7 +19,8 @@ var errTest = errors.New("test error") func TestLoginUsingLDAP(t *testing.T) { LDAPLoginScenario(t, "When LDAP enabled and no server configured", func(sc *LDAPLoginScenarioContext) { - setting.LDAPEnabled = true + cfg := setting.NewCfg() + cfg.LDAPEnabled = true sc.withLoginResult(false) getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { @@ -31,7 +32,7 @@ func TestLoginUsingLDAP(t *testing.T) { } loginService := &logintest.LoginServiceFake{} - enabled, err := loginUsingLDAP(context.Background(), sc.loginUserQuery, loginService) + enabled, err := loginUsingLDAP(context.Background(), sc.loginUserQuery, loginService, cfg) require.EqualError(t, err, errTest.Error()) assert.True(t, enabled) @@ -39,11 +40,12 @@ func TestLoginUsingLDAP(t *testing.T) { }) LDAPLoginScenario(t, "When LDAP disabled", func(sc *LDAPLoginScenarioContext) { - setting.LDAPEnabled = false + cfg := setting.NewCfg() + cfg.LDAPEnabled = false sc.withLoginResult(false) loginService := &logintest.LoginServiceFake{} - enabled, err := loginUsingLDAP(context.Background(), sc.loginUserQuery, loginService) + enabled, err := loginUsingLDAP(context.Background(), sc.loginUserQuery, loginService, cfg) require.NoError(t, err) assert.False(t, enabled) @@ -104,7 +106,7 @@ func mockLDAPAuthenticator(valid bool) *mockAuth { validLogin: valid, } - newLDAP = func(servers []*ldap.ServerConfig) multildap.IMultiLDAP { + newLDAP = func(servers []*ldap.ServerConfig, _ *setting.Cfg) multildap.IMultiLDAP { return mock } @@ -133,15 +135,6 @@ func LDAPLoginScenario(t *testing.T, desc string, fn LDAPLoginScenarioFunc) { LDAPAuthenticatorMock: mock, } - origNewLDAP := newLDAP - origGetLDAPConfig := getLDAPConfig - origLDAPEnabled := setting.LDAPEnabled - t.Cleanup(func() { - newLDAP = origNewLDAP - getLDAPConfig = origGetLDAPConfig - setting.LDAPEnabled = origLDAPEnabled - }) - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { config := &ldap.Config{ Servers: []*ldap.ServerConfig{ @@ -154,7 +147,7 @@ func LDAPLoginScenario(t *testing.T, desc string, fn LDAPLoginScenarioFunc) { return config, nil } - newLDAP = func(server []*ldap.ServerConfig) multildap.IMultiLDAP { + newLDAP = func(server []*ldap.ServerConfig, _ *setting.Cfg) multildap.IMultiLDAP { return mock } diff --git a/pkg/middleware/middleware_basic_auth_test.go b/pkg/middleware/middleware_basic_auth_test.go index 0f77df3fb4b..d6d01c5eb8f 100644 --- a/pkg/middleware/middleware_basic_auth_test.go +++ b/pkg/middleware/middleware_basic_auth_test.go @@ -67,7 +67,7 @@ func TestMiddlewareBasicAuth(t *testing.T) { sc.userService.ExpectedUser = &user.User{Password: encoded, ID: id, Salt: salt} sc.userService.ExpectedSignedInUser = &user.SignedInUser{UserID: id} - login.ProvideService(sc.mockSQLStore, &logintest.LoginServiceFake{}, nil, sc.userService) + login.ProvideService(sc.mockSQLStore, &logintest.LoginServiceFake{}, nil, sc.userService, sc.cfg) authHeader := util.GetBasicAuthHeader("myUser", password) sc.fakeReq("GET", "/").withAuthorizationHeader(authHeader).exec() diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index e8dab37c49a..d893a92b26a 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -35,6 +35,7 @@ import ( "github.com/grafana/grafana/pkg/services/contexthandler/authproxy" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/ldap/service" loginsvc "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/loginservice" "github.com/grafana/grafana/pkg/services/login/logintest" @@ -950,9 +951,15 @@ func getContextHandler(t *testing.T, cfg *setting.Cfg, mockSQLStore *dbtest.Fake renderSvc := &fakeRenderService{} authJWTSvc := jwt.NewFakeJWTService() tracer := tracing.InitializeTracerForTest() - authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginService, userService, mockSQLStore) + authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginService, + userService, mockSQLStore, &service.LDAPFakeService{ExpectedError: service.ErrUnableToCreateLDAPClient}) authenticator := &logintest.AuthenticatorFake{ExpectedUser: &user.User{}} - return contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, mockSQLStore, tracer, authProxy, loginService, apiKeyService, authenticator, userService, orgService, oauthTokenService, featuremgmt.WithFeatures(featuremgmt.FlagAccessTokenExpirationCheck), &authntest.FakeService{}) + return contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, + remoteCacheSvc, renderSvc, mockSQLStore, tracer, authProxy, + loginService, apiKeyService, authenticator, userService, orgService, + oauthTokenService, + featuremgmt.WithFeatures(featuremgmt.FlagAccessTokenExpirationCheck), + &authntest.FakeService{}) } type fakeRenderService struct { diff --git a/pkg/server/wire.go b/pkg/server/wire.go index f65d974bd3a..0bd56d37fb5 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -69,6 +69,7 @@ import ( "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/hooks" ldapapi "github.com/grafana/grafana/pkg/services/ldap/api" + ldapservice "github.com/grafana/grafana/pkg/services/ldap/service" "github.com/grafana/grafana/pkg/services/libraryelements" "github.com/grafana/grafana/pkg/services/librarypanels" "github.com/grafana/grafana/pkg/services/live" @@ -234,6 +235,8 @@ var wireBasicSet = wire.NewSet( live.ProvideService, pushhttp.ProvideService, contexthandler.ProvideService, + ldapservice.ProvideService, + wire.Bind(new(ldapservice.LDAP), new(*ldapservice.LDAPImpl)), jwt.ProvideService, wire.Bind(new(jwt.JWTService), new(*jwt.AuthService)), ngstore.ProvideDBStore, diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index f200eb0bc6c..66d40f71c55 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -5,13 +5,13 @@ import ( "net/http" "strconv" - "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/hashicorp/go-multierror" "go.opentelemetry.io/otel/attribute" "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/network" + "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/login/social" @@ -22,6 +22,7 @@ import ( "github.com/grafana/grafana/pkg/services/authn/authnimpl/sync" "github.com/grafana/grafana/pkg/services/authn/clients" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/ldap/service" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/loginattempt" "github.com/grafana/grafana/pkg/services/oauthtoken" @@ -59,6 +60,7 @@ func ProvideService( authInfoService login.AuthInfoService, renderService rendering.Service, features *featuremgmt.FeatureManager, oauthTokenService oauthtoken.OAuthTokenService, socialService social.Service, cache *remotecache.RemoteCache, + ldapService service.LDAP, ) *Service { s := &Service{ log: log.New("authn.service"), @@ -87,7 +89,7 @@ func ProvideService( var proxyClients []authn.ProxyClient var passwordClients []authn.PasswordClient if s.cfg.LDAPEnabled { - ldap := clients.ProvideLDAP(cfg) + ldap := clients.ProvideLDAP(cfg, ldapService) proxyClients = append(proxyClients, ldap) passwordClients = append(passwordClients, ldap) } diff --git a/pkg/services/authn/clients/ldap.go b/pkg/services/authn/clients/ldap.go index a142a546ab7..66b9c86e2f6 100644 --- a/pkg/services/authn/clients/ldap.go +++ b/pkg/services/authn/clients/ldap.go @@ -13,8 +13,13 @@ import ( var _ authn.ProxyClient = new(LDAP) var _ authn.PasswordClient = new(LDAP) -func ProvideLDAP(cfg *setting.Cfg) *LDAP { - return &LDAP{cfg, &ldapServiceImpl{cfg}} +type ldapService interface { + Login(query *login.LoginUserQuery) (*login.ExternalUserInfo, error) + User(username string) (*login.ExternalUserInfo, error) +} + +func ProvideLDAP(cfg *setting.Cfg, ldapService ldapService) *LDAP { + return &LDAP{cfg, ldapService} } type LDAP struct { @@ -60,35 +65,6 @@ func (c *LDAP) AuthenticatePassword(ctx context.Context, r *authn.Request, usern return identityFromLDAPInfo(r.OrgID, info, c.cfg.LDAPAllowSignup), nil } -type ldapService interface { - Login(query *login.LoginUserQuery) (*login.ExternalUserInfo, error) - User(username string) (*login.ExternalUserInfo, error) -} - -// FIXME: remove the implementation if we convert ldap to an actual service -type ldapServiceImpl struct { - cfg *setting.Cfg -} - -func (s *ldapServiceImpl) Login(query *login.LoginUserQuery) (*login.ExternalUserInfo, error) { - cfg, err := multildap.GetConfig(s.cfg) - if err != nil { - return nil, err - } - - return multildap.New(cfg.Servers).Login(query) -} - -func (s *ldapServiceImpl) User(username string) (*login.ExternalUserInfo, error) { - cfg, err := multildap.GetConfig(s.cfg) - if err != nil { - return nil, err - } - - user, _, err := multildap.New(cfg.Servers).User(username) - return user, err -} - func identityFromLDAPInfo(orgID int64, info *login.ExternalUserInfo, allowSignup bool) *authn.Identity { return &authn.Identity{ OrgID: orgID, diff --git a/pkg/services/authn/clients/ldap_test.go b/pkg/services/authn/clients/ldap_test.go index 1eec720a597..eed01d6e291 100644 --- a/pkg/services/authn/clients/ldap_test.go +++ b/pkg/services/authn/clients/ldap_test.go @@ -9,6 +9,7 @@ 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/ldap/service" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/setting" @@ -68,7 +69,7 @@ func TestLDAP_AuthenticateProxy(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - c := &LDAP{cfg: setting.NewCfg(), service: fakeLDAPService{ExpectedInfo: tt.expectedLDAPInfo, ExpectedErr: tt.expectedLDAPErr}} + c := &LDAP{cfg: setting.NewCfg(), service: &service.LDAPFakeService{ExpectedUser: tt.expectedLDAPInfo, ExpectedError: tt.expectedLDAPErr}} identity, err := c.AuthenticateProxy(context.Background(), &authn.Request{OrgID: 1}, tt.username, nil) assert.ErrorIs(t, err, tt.expectedErr) assert.EqualValues(t, tt.expectedIdentity, identity) @@ -140,7 +141,7 @@ func TestLDAP_AuthenticatePassword(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - c := &LDAP{cfg: setting.NewCfg(), service: fakeLDAPService{ExpectedInfo: tt.expectedLDAPInfo, ExpectedErr: tt.expectedLDAPErr}} + c := &LDAP{cfg: setting.NewCfg(), service: &service.LDAPFakeService{ExpectedUser: tt.expectedLDAPInfo, ExpectedError: tt.expectedLDAPErr}} identity, err := c.AuthenticatePassword(context.Background(), &authn.Request{OrgID: 1}, tt.username, tt.password) assert.ErrorIs(t, err, tt.expectedErr) @@ -152,18 +153,3 @@ func TestLDAP_AuthenticatePassword(t *testing.T) { func strPtr(s string) *string { return &s } - -var _ ldapService = new(fakeLDAPService) - -type fakeLDAPService struct { - ExpectedErr error - ExpectedInfo *login.ExternalUserInfo -} - -func (f fakeLDAPService) Login(query *login.LoginUserQuery) (*login.ExternalUserInfo, error) { - return f.ExpectedInfo, f.ExpectedErr -} - -func (f fakeLDAPService) User(username string) (*login.ExternalUserInfo, error) { - return f.ExpectedInfo, f.ExpectedErr -} diff --git a/pkg/services/contexthandler/auth_proxy_test.go b/pkg/services/contexthandler/auth_proxy_test.go index 18c1b479609..064cd08ca1f 100644 --- a/pkg/services/contexthandler/auth_proxy_test.go +++ b/pkg/services/contexthandler/auth_proxy_test.go @@ -19,6 +19,7 @@ import ( "github.com/grafana/grafana/pkg/services/contexthandler/authproxy" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/ldap/service" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/loginservice" "github.com/grafana/grafana/pkg/services/org/orgtest" @@ -110,7 +111,7 @@ func getContextHandler(t *testing.T) *ContextHandler { } orgService := orgtest.NewOrgServiceFake() - authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginService, &userService, nil) + authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginService, &userService, nil, service.NewLDAPFakeService()) authenticator := &fakeAuthenticator{} return ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, diff --git a/pkg/services/contexthandler/authproxy/authproxy.go b/pkg/services/contexthandler/authproxy/authproxy.go index 321a59f4e44..8d151b5b093 100644 --- a/pkg/services/contexthandler/authproxy/authproxy.go +++ b/pkg/services/contexthandler/authproxy/authproxy.go @@ -19,7 +19,7 @@ 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/ldap/service" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" @@ -33,21 +33,6 @@ const ( CachePrefix = "auth-proxy-sync-ttl:%s" ) -// getLDAPConfig gets LDAP config -var getLDAPConfig = ldap.GetConfig - -// isLDAPEnabled checks if LDAP is enabled -var isLDAPEnabled = func(cfg *setting.Cfg) bool { - if cfg != nil { - return cfg.LDAPEnabled - } - - return setting.LDAPEnabled -} - -// newLDAP creates multiple LDAP instance -var newLDAP = multildap.New - // supportedHeaders states the supported headers configuration fields var supportedHeaderFields = []string{"Name", "Email", "Login", "Groups", "Role"} @@ -58,11 +43,14 @@ type AuthProxy struct { loginService login.Service sqlStore db.DB userService user.Service + ldapService service.LDAP logger log.Logger } -func ProvideAuthProxy(cfg *setting.Cfg, remoteCache *remotecache.RemoteCache, loginService login.Service, userService user.Service, sqlStore db.DB) *AuthProxy { +func ProvideAuthProxy(cfg *setting.Cfg, remoteCache *remotecache.RemoteCache, + loginService login.Service, userService user.Service, + sqlStore db.DB, ldapService service.LDAP) *AuthProxy { return &AuthProxy{ cfg: cfg, remoteCache: remoteCache, @@ -70,6 +58,7 @@ func ProvideAuthProxy(cfg *setting.Cfg, remoteCache *remotecache.RemoteCache, lo sqlStore: sqlStore, userService: userService, logger: log.New("auth.proxy"), + ldapService: ldapService, } } @@ -175,7 +164,7 @@ func (auth *AuthProxy) Login(reqCtx *contextmodel.ReqContext, ignoreCache bool) } } - if isLDAPEnabled(auth.cfg) { + if auth.cfg.LDAPEnabled { id, err := auth.LoginViaLDAP(reqCtx) if err != nil { if errors.Is(err, ldap.ErrInvalidCredentials) { @@ -234,14 +223,9 @@ func (auth *AuthProxy) RemoveUserFromCache(reqCtx *contextmodel.ReqContext) erro // LoginViaLDAP logs in user via LDAP request func (auth *AuthProxy) LoginViaLDAP(reqCtx *contextmodel.ReqContext) (int64, error) { - config, err := getLDAPConfig(auth.cfg) - if err != nil { - return 0, newError("failed to get LDAP config", err) - } - header := auth.getDecodedHeader(reqCtx, auth.cfg.AuthProxyHeaderName) - mldap := newLDAP(config.Servers) - extUser, _, err := mldap.User(header) + + extUser, err := auth.ldapService.User(header) if err != nil { return 0, err } diff --git a/pkg/services/contexthandler/authproxy/authproxy_test.go b/pkg/services/contexthandler/authproxy/authproxy_test.go index 20cad6d7d8f..361d0972a5c 100644 --- a/pkg/services/contexthandler/authproxy/authproxy_test.go +++ b/pkg/services/contexthandler/authproxy/authproxy_test.go @@ -2,7 +2,6 @@ package authproxy import ( "context" - "errors" "fmt" "net/http" "strconv" @@ -13,8 +12,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/ldap/service" + "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/loginservice" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" @@ -49,7 +48,7 @@ func prepareMiddleware(t *testing.T, remoteCache *remotecache.RemoteCache, confi }, } - return ProvideAuthProxy(cfg, remoteCache, loginService, nil, nil), ctx + return ProvideAuthProxy(cfg, remoteCache, loginService, nil, nil, service.NewLDAPFakeService()), ctx } func TestMiddlewareContext(t *testing.T) { @@ -105,85 +104,41 @@ func TestMiddlewareContext(t *testing.T) { func TestMiddlewareContext_ldap(t *testing.T) { t.Run("Logs in via LDAP", func(t *testing.T) { - origIsLDAPEnabled := isLDAPEnabled - origGetLDAPConfig := getLDAPConfig - origNewLDAP := newLDAP - t.Cleanup(func() { - newLDAP = origNewLDAP - isLDAPEnabled = origIsLDAPEnabled - getLDAPConfig = origGetLDAPConfig - }) - - isLDAPEnabled = func(*setting.Cfg) bool { - return true - } - - stub := &multildap.MultiLDAPmock{ - ID: id, - } - - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { - config := &ldap.Config{ - Servers: []*ldap.ServerConfig{ - { - SearchBaseDNs: []string{"BaseDNHere"}, - }, - }, - } - return config, nil - } - - newLDAP = func(servers []*ldap.ServerConfig) multildap.IMultiLDAP { - return stub - } - cache := remotecache.NewFakeStore(t) auth, reqCtx := prepareMiddleware(t, cache, nil) + auth.cfg.LDAPEnabled = true + ldapFake := &service.LDAPFakeService{ + ExpectedUser: &login.ExternalUserInfo{UserId: id}, + } + + auth.ldapService = ldapFake gotID, err := auth.Login(reqCtx, false) require.NoError(t, err) assert.Equal(t, id, gotID) - assert.True(t, stub.UserCalled) + assert.True(t, ldapFake.UserCalled) }) t.Run("Gets nice error if LDAP is enabled, but not configured", func(t *testing.T) { const id int64 = 42 - origIsLDAPEnabled := isLDAPEnabled - origNewLDAP := newLDAP - origGetLDAPConfig := getLDAPConfig - t.Cleanup(func() { - isLDAPEnabled = origIsLDAPEnabled - newLDAP = origNewLDAP - getLDAPConfig = origGetLDAPConfig - }) - - isLDAPEnabled = func(*setting.Cfg) bool { - return true - } - - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { - return nil, errors.New("something went wrong") - } - cache := remotecache.NewFakeStore(t) auth, reqCtx := prepareMiddleware(t, cache, nil) - - stub := &multildap.MultiLDAPmock{ - ID: id, + auth.cfg.LDAPEnabled = true + ldapFake := &service.LDAPFakeService{ + ExpectedUser: nil, + ExpectedError: service.ErrUnableToCreateLDAPClient, } - newLDAP = func(servers []*ldap.ServerConfig) multildap.IMultiLDAP { - return stub - } + auth.ldapService = ldapFake gotID, err := auth.Login(reqCtx, false) require.EqualError(t, err, "failed to get the user") assert.NotEqual(t, id, gotID) - assert.False(t, stub.LoginCalled) + assert.True(t, ldapFake.UserCalled) }) } diff --git a/pkg/services/ldap/api/dtos.go b/pkg/services/ldap/api/dtos.go new file mode 100644 index 00000000000..5245ccaafc5 --- /dev/null +++ b/pkg/services/ldap/api/dtos.go @@ -0,0 +1,54 @@ +package api + +import ( + "github.com/grafana/grafana/pkg/services/ldap" + "github.com/grafana/grafana/pkg/services/org" +) + +// swagger:parameters getUserFromLDAP +type GetLDAPUserParams struct { + // in:path + // required:true + UserName string `json:"user_name"` +} + +// swagger:parameters postSyncUserWithLDAP +type SyncLDAPUserParams struct { + // in:path + // required:true + UserID int64 `json:"user_id"` +} + +// LDAPAttribute is a serializer for user attributes mapped from LDAP. Is meant to display both the serialized value and the LDAP key we received it from. +type LDAPAttribute struct { + ConfigAttributeValue string `json:"cfgAttrValue"` + LDAPAttributeValue string `json:"ldapValue"` +} + +// RoleDTO is a serializer for mapped roles from LDAP +type LDAPRoleDTO struct { + OrgId int64 `json:"orgId"` + OrgName string `json:"orgName"` + OrgRole org.RoleType `json:"orgRole"` + GroupDN string `json:"groupDN"` +} + +// LDAPUserDTO is a serializer for users mapped from LDAP +type LDAPUserDTO struct { + Name *LDAPAttribute `json:"name"` + Surname *LDAPAttribute `json:"surname"` + Email *LDAPAttribute `json:"email"` + Username *LDAPAttribute `json:"login"` + IsGrafanaAdmin *bool `json:"isGrafanaAdmin"` + IsDisabled bool `json:"isDisabled"` + OrgRoles []LDAPRoleDTO `json:"roles"` + Teams []ldap.TeamOrgGroupDTO `json:"teams"` +} + +// LDAPServerDTO is a serializer for LDAP server statuses +type LDAPServerDTO struct { + Host string `json:"host"` + Port int `json:"port"` + Available bool `json:"available"` + Error string `json:"error"` +} diff --git a/pkg/services/ldap/api/errors.go b/pkg/services/ldap/api/errors.go new file mode 100644 index 00000000000..e53a21de1ba --- /dev/null +++ b/pkg/services/ldap/api/errors.go @@ -0,0 +1,11 @@ +package api + +import "fmt" + +type OrganizationNotFoundError struct { + OrgID int64 +} + +func (e *OrganizationNotFoundError) Error() string { + return fmt.Sprintf("unable to find organization with ID '%d'", e.OrgID) +} diff --git a/pkg/services/ldap/api/service.go b/pkg/services/ldap/api/service.go index 063d9b87516..9506c9b3509 100644 --- a/pkg/services/ldap/api/service.go +++ b/pkg/services/ldap/api/service.go @@ -17,6 +17,7 @@ import ( 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/ldap/service" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/supportbundles" @@ -26,15 +27,6 @@ import ( "github.com/grafana/grafana/pkg/web" ) -var ( - getLDAPConfig = multildap.GetConfig - newLDAP = multildap.New - - errOrganizationNotFound = func(orgId int64) error { - return fmt.Errorf("unable to find organization with ID '%d'", orgId) - } -) - type Service struct { cfg *setting.Cfg userService user.Service @@ -44,11 +36,12 @@ type Service struct { orgService org.Service sessionService auth.UserTokenService log log.Logger + ldapService service.LDAP } 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, + loginService login.Service, orgService org.Service, ldapService service.LDAP, sessionService auth.UserTokenService, bundleRegistry supportbundles.Service) *Service { s := &Service{ cfg: cfg, @@ -58,6 +51,7 @@ func ProvideService(cfg *setting.Cfg, router routing.RouteRegister, accessContro loginService: loginService, orgService: orgService, sessionService: sessionService, + ldapService: ldapService, log: log.New("ldap.api"), } @@ -85,78 +79,6 @@ func ProvideService(cfg *setting.Cfg, router routing.RouteRegister, accessContro return s } -// LDAPAttribute is a serializer for user attributes mapped from LDAP. Is meant to display both the serialized value and the LDAP key we received it from. -type LDAPAttribute struct { - ConfigAttributeValue string `json:"cfgAttrValue"` - LDAPAttributeValue string `json:"ldapValue"` -} - -// RoleDTO is a serializer for mapped roles from LDAP -type LDAPRoleDTO struct { - OrgId int64 `json:"orgId"` - OrgName string `json:"orgName"` - OrgRole org.RoleType `json:"orgRole"` - GroupDN string `json:"groupDN"` -} - -// LDAPUserDTO is a serializer for users mapped from LDAP -type LDAPUserDTO struct { - Name *LDAPAttribute `json:"name"` - Surname *LDAPAttribute `json:"surname"` - Email *LDAPAttribute `json:"email"` - Username *LDAPAttribute `json:"login"` - IsGrafanaAdmin *bool `json:"isGrafanaAdmin"` - IsDisabled bool `json:"isDisabled"` - OrgRoles []LDAPRoleDTO `json:"roles"` - Teams []ldap.TeamOrgGroupDTO `json:"teams"` -} - -// LDAPServerDTO is a serializer for LDAP server statuses -type LDAPServerDTO struct { - Host string `json:"host"` - Port int `json:"port"` - Available bool `json:"available"` - Error string `json:"error"` -} - -// 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, orga org.Service) error { - orgIds := []int64{} - - for _, or := range user.OrgRoles { - orgIds = append(orgIds, or.OrgId) - } - - q := &org.SearchOrgsQuery{} - q.IDs = orgIds - - result, err := orga.Search(ctx, q) - if err != nil { - return err - } - - orgNamesById := map[int64]string{} - for _, org := range result { - orgNamesById[org.ID] = org.Name - } - - for i, orgDTO := range user.OrgRoles { - if orgDTO.OrgId < 1 { - continue - } - - orgName := orgNamesById[orgDTO.OrgId] - - if orgName != "" { - user.OrgRoles[i].OrgName = orgName - } else { - return errOrganizationNotFound(orgDTO.OrgId) - } - } - - return nil -} - // swagger:route POST /admin/ldap/reload admin_ldap reloadLDAPCfg // // Reloads the LDAP configuration. @@ -176,10 +98,10 @@ func (s *Service) ReloadLDAPCfg(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } - err := ldap.ReloadConfig(s.cfg.LDAPConfigFilePath) - if err != nil { + if err := s.ldapService.ReloadConfig(); err != nil { return response.Error(http.StatusInternalServerError, "Failed to reload LDAP config", err) } + return response.Success("LDAP config reloaded") } @@ -202,18 +124,12 @@ func (s *Service) GetLDAPStatus(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } - 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) - } - - ldap := newLDAP(ldapConfig.Servers) - - if ldap == nil { + ldapClient := s.ldapService.Client() + if ldapClient == nil { return response.Error(http.StatusInternalServerError, "Failed to find the LDAP server", nil) } - statuses, err := ldap.Ping() + statuses, err := ldapClient.Ping() if err != nil { return response.Error(http.StatusBadRequest, "Failed to connect to the LDAP server(s)", err) } @@ -255,9 +171,9 @@ func (s *Service) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response.Resp return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } - 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) + ldapClient := s.ldapService.Client() + if ldapClient == nil { + return response.Error(http.StatusInternalServerError, "Failed to find the LDAP server", nil) } userId, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64) @@ -270,10 +186,10 @@ func (s *Service) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response.Resp 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) + return response.Error(http.StatusNotFound, user.ErrUserNotFound.Error(), nil) } - return response.Error(500, "Failed to get user", err) + return response.Error(http.StatusInternalServerError, "Failed to get user", err) } authModuleQuery := &login.GetAuthInfoQuery{UserId: usr.ID, AuthModule: login.LDAPAuthModule} @@ -285,8 +201,7 @@ func (s *Service) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response.Resp return response.Error(500, "Failed to get user", err) } - ldapServer := newLDAP(ldapConfig.Servers) - userInfo, _, err := ldapServer.User(usr.Login) + userInfo, _, err := ldapClient.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 s.cfg.AdminUser == usr.Login { // User is *the* Grafana Admin. We cannot disable it. @@ -351,20 +266,14 @@ func (s *Service) GetUserFromLDAP(c *contextmodel.ReqContext) response.Response return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } - ldapConfig, err := getLDAPConfig(s.cfg) - if err != nil { - return response.Error(http.StatusBadRequest, "Failed to obtain the LDAP configuration", err) - } - - multiLDAP := newLDAP(ldapConfig.Servers) + ldapClient := s.ldapService.Client() username := web.Params(c.Req)[":username"] - if len(username) == 0 { return response.Error(http.StatusBadRequest, "Validation error. You must specify an username", nil) } - user, serverConfig, err := multiLDAP.User(username) + user, serverConfig, err := ldapClient.User(username) if user == nil || err != nil { return response.Error(http.StatusNotFound, "No user was found in the LDAP server(s) with that username", err) } @@ -409,7 +318,7 @@ func (s *Service) GetUserFromLDAP(c *contextmodel.ReqContext) response.Response } s.log.Debug("mapping org roles", "orgsRoles", u.OrgRoles) - if err := u.FetchOrgs(c.Req.Context(), s.orgService); err != nil { + 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) } @@ -435,16 +344,40 @@ func splitName(name string) (string, string) { } } -// swagger:parameters getUserFromLDAP -type GetLDAPUserParams struct { - // in:path - // required:true - UserName string `json:"user_name"` -} +// 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, orga org.Service) error { + orgIds := []int64{} -// swagger:parameters postSyncUserWithLDAP -type SyncLDAPUserParams struct { - // in:path - // required:true - UserID int64 `json:"user_id"` + for _, or := range user.OrgRoles { + orgIds = append(orgIds, or.OrgId) + } + + q := &org.SearchOrgsQuery{} + q.IDs = orgIds + + result, err := orga.Search(ctx, q) + if err != nil { + return err + } + + orgNamesById := map[int64]string{} + for _, org := range result { + orgNamesById[org.ID] = org.Name + } + + for i, orgDTO := range user.OrgRoles { + if orgDTO.OrgId < 1 { + continue + } + + orgName := orgNamesById[orgDTO.OrgId] + + if orgName != "" { + user.OrgRoles[i].OrgName = orgName + } else { + return &OrganizationNotFoundError{orgDTO.OrgId} + } + } + + return nil } diff --git a/pkg/services/ldap/api/service_test.go b/pkg/services/ldap/api/service_test.go index c79728414d0..078fb65cee4 100644 --- a/pkg/services/ldap/api/service_test.go +++ b/pkg/services/ldap/api/service_test.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/auth/authtest" "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/ldap/multildap" + "github.com/grafana/grafana/pkg/services/ldap/service" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/logintest" "github.com/grafana/grafana/pkg/services/org" @@ -69,6 +70,7 @@ func setupAPITest(t *testing.T, opts ...func(a *Service)) (*Service, *webtest.Se ldap.ProvideGroupsService(), &logintest.LoginServiceFake{}, &orgtest.FakeOrgService{}, + service.NewLDAPFakeService(), authtest.NewFakeUserAuthTokenService(), supportbundlestest.NewFakeBundleService(), ) @@ -83,20 +85,16 @@ func setupAPITest(t *testing.T, opts ...func(a *Service)) (*Service, *webtest.Se } func TestGetUserFromLDAPAPIEndpoint_UserNotFound(t *testing.T) { - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { - return &ldap.Config{}, nil - } - - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{ - UserSearchResult: nil, - } - } - _, server := setupAPITest(t, func(a *Service) { a.orgService = &orgtest.FakeOrgService{ ExpectedOrgs: []*org.OrgDTO{}, } + a.ldapService = &service.LDAPFakeService{ + ExpectedClient: &LDAPMock{ + UserSearchResult: nil, + }, + ExpectedConfig: &ldap.Config{}, + } }) req := server.NewGetRequest("/api/admin/ldap/user-that-does-not-exist") @@ -147,13 +145,6 @@ 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."}, } @@ -162,12 +153,15 @@ func TestGetUserFromLDAPAPIEndpoint_OrgNotfound(t *testing.T) { a.orgService = &orgtest.FakeOrgService{ ExpectedOrgs: mockOrgSearchResult, } + a.ldapService = &service.LDAPFakeService{ + ExpectedClient: &LDAPMock{ + UserSearchResult: userSearchResult, + UserSearchConfig: userSearchConfig, + }, + ExpectedConfig: &ldap.Config{}, + } }) - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { - return &ldap.Config{}, nil - } - req := server.NewGetRequest("/api/admin/ldap/johndoe") webtest.RequestWithSignedInUser(req, &user.SignedInUser{ OrgID: 1, @@ -225,21 +219,17 @@ func TestGetUserFromLDAPAPIEndpoint(t *testing.T) { {ID: 1, Name: "Main Org."}, } - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { - return &ldap.Config{}, nil - } - - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{ - UserSearchResult: userSearchResult, - UserSearchConfig: userSearchConfig, - } - } - _, server := setupAPITest(t, func(a *Service) { a.orgService = &orgtest.FakeOrgService{ ExpectedOrgs: mockOrgSearchResult, } + a.ldapService = &service.LDAPFakeService{ + ExpectedClient: &LDAPMock{ + UserSearchResult: userSearchResult, + UserSearchConfig: userSearchConfig, + }, + ExpectedConfig: &ldap.Config{}, + } }) req := server.NewGetRequest("/api/admin/ldap/johndoe") @@ -314,21 +304,17 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) { {ID: 1, Name: "Main Org."}, } - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { - return &ldap.Config{}, nil - } - - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{ - UserSearchResult: userSearchResult, - UserSearchConfig: userSearchConfig, - } - } - _, server := setupAPITest(t, func(a *Service) { a.orgService = &orgtest.FakeOrgService{ ExpectedOrgs: mockOrgSearchResult, } + a.ldapService = &service.LDAPFakeService{ + ExpectedClient: &LDAPMock{ + UserSearchResult: userSearchResult, + UserSearchConfig: userSearchConfig, + }, + ExpectedConfig: &ldap.Config{}, + } }) req := server.NewGetRequest("/api/admin/ldap/johndoe") @@ -378,15 +364,11 @@ func TestGetLDAPStatusAPIEndpoint(t *testing.T) { {Host: "10.0.0.5", Port: 361, Available: false, Error: errors.New("something is awfully wrong")}, } - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { - return &ldap.Config{}, nil - } - - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} - } - _, server := setupAPITest(t, func(a *Service) { + a.ldapService = &service.LDAPFakeService{ + ExpectedClient: &LDAPMock{}, + ExpectedConfig: &ldap.Config{}, + } }) req := server.NewGetRequest("/api/admin/ldap/status") @@ -418,18 +400,14 @@ func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) { userServiceMock := usertest.NewUserServiceFake() userServiceMock.ExpectedUser = &user.User{Login: "ldap-daniel", ID: 34} - 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", - }} - } - _, server := setupAPITest(t, func(a *Service) { a.userService = userServiceMock + a.ldapService = &service.LDAPFakeService{ + ExpectedClient: &LDAPMock{UserSearchResult: &login.ExternalUserInfo{ + Login: "ldap-daniel", + }}, + ExpectedConfig: &ldap.Config{}, + } }) req := server.NewPostRequest("/api/admin/ldap/sync/34", nil) @@ -459,16 +437,12 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) { userServiceMock := usertest.NewUserServiceFake() userServiceMock.ExpectedError = user.ErrUserNotFound - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { - return &ldap.Config{}, nil - } - - newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { - return &LDAPMock{} - } - _, server := setupAPITest(t, func(a *Service) { a.userService = userServiceMock + a.ldapService = &service.LDAPFakeService{ + ExpectedClient: &LDAPMock{}, + ExpectedConfig: &ldap.Config{}, + } }) req := server.NewPostRequest("/api/admin/ldap/sync/34", nil) @@ -498,17 +472,13 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) { userServiceMock := usertest.NewUserServiceFake() userServiceMock.ExpectedUser = &user.User{Login: "ldap-daniel", ID: 34} - 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" + a.ldapService = &service.LDAPFakeService{ + ExpectedClient: &LDAPMock{UserSearchError: multildap.ErrDidNotFindUser}, + ExpectedConfig: &ldap.Config{}, + } }) req := server.NewPostRequest("/api/admin/ldap/sync/34", nil) @@ -536,17 +506,13 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) { userServiceMock := usertest.NewUserServiceFake() userServiceMock.ExpectedUser = &user.User{Login: "ldap-daniel", ID: 34} - 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.authInfoService = &logintest.AuthInfoServiceFake{ExpectedExternalUser: &login.ExternalUserInfo{IsDisabled: true, UserId: 34}} + a.ldapService = &service.LDAPFakeService{ + ExpectedClient: &LDAPMock{UserSearchError: multildap.ErrDidNotFindUser}, + ExpectedConfig: &ldap.Config{}, + } }) req := server.NewPostRequest("/api/admin/ldap/sync/34", nil) @@ -589,18 +555,6 @@ search_base_dns = ["dc=grafana,dc=org"]`) errF = f.Close() require.NoError(t, errF) - 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 @@ -688,6 +642,12 @@ search_base_dns = ["dc=grafana,dc=org"]`) _, server := setupAPITest(t, func(a *Service) { a.userService = &usertest.FakeUserService{ExpectedUser: &user.User{Login: "ldap-daniel", ID: 1}} a.cfg.LDAPConfigFilePath = ldapConfigFile + a.ldapService = &service.LDAPFakeService{ + ExpectedClient: &LDAPMock{UserSearchResult: &login.ExternalUserInfo{ + Login: "ldap-daniel", + }}, + ExpectedConfig: &ldap.Config{}, + } }) // Add minimal setup to pass handler res, err := server.Send( diff --git a/pkg/services/ldap/api/support_bundle.go b/pkg/services/ldap/api/support_bundle.go index 3b41fb2342c..414001ebe26 100644 --- a/pkg/services/ldap/api/support_bundle.go +++ b/pkg/services/ldap/api/support_bundle.go @@ -9,19 +9,17 @@ import ( "github.com/BurntSushi/toml" "github.com/grafana/grafana/pkg/services/supportbundles" - "github.com/grafana/grafana/pkg/setting" ) func (s *Service) supportBundleCollector(context.Context) (*supportbundles.SupportItem, error) { bWriter := bytes.NewBuffer(nil) bWriter.WriteString("# LDAP information\n\n") - ldapConfig, err := getLDAPConfig(s.cfg) - + ldapConfig := s.ldapService.Config() if ldapConfig != nil { bWriter.WriteString("## LDAP Status\n") - ldapClient := newLDAP(ldapConfig.Servers) + ldapClient := s.ldapService.Client() ldapStatus, err := ldapClient.Ping() if err != nil { @@ -66,7 +64,7 @@ func (s *Service) supportBundleCollector(context.Context) (*supportbundles.Suppo errM := toml.NewEncoder(bWriter).Encode(ldapConfig) if errM != nil { bWriter.WriteString( - fmt.Sprintf("Unable to encode LDAP configuration \n Err: %s", err)) + fmt.Sprintf("Unable to encode LDAP configuration \n Err: %s", errM)) } bWriter.WriteString("```\n\n") @@ -77,9 +75,9 @@ func (s *Service) supportBundleCollector(context.Context) (*supportbundles.Suppo bWriter.WriteString(fmt.Sprintf("enabled = %v\n", s.cfg.LDAPEnabled)) bWriter.WriteString(fmt.Sprintf("config_file = %s\n", s.cfg.LDAPConfigFilePath)) bWriter.WriteString(fmt.Sprintf("allow_sign_up = %v\n", s.cfg.LDAPAllowSignup)) - bWriter.WriteString(fmt.Sprintf("sync_cron = %s\n", setting.LDAPSyncCron)) - bWriter.WriteString(fmt.Sprintf("active_sync_enabled = %v\n", setting.LDAPActiveSyncEnabled)) - bWriter.WriteString(fmt.Sprintf("skip_org_role_sync = %v\n", setting.LDAPSkipOrgRoleSync)) + bWriter.WriteString(fmt.Sprintf("sync_cron = %s\n", s.cfg.LDAPSyncCron)) + bWriter.WriteString(fmt.Sprintf("active_sync_enabled = %v\n", s.cfg.LDAPActiveSyncEnabled)) + bWriter.WriteString(fmt.Sprintf("skip_org_role_sync = %v\n", s.cfg.LDAPSkipOrgRoleSync)) bWriter.WriteString("```\n\n") diff --git a/pkg/services/ldap/ldap.go b/pkg/services/ldap/ldap.go index 8a65c939fcd..9d1f67150fd 100644 --- a/pkg/services/ldap/ldap.go +++ b/pkg/services/ldap/ldap.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/setting" ) // IConnection is interface for LDAP connection manipulation @@ -42,6 +43,7 @@ type IServer interface { // Server is basic struct of LDAP authorization type Server struct { + cfg *setting.Cfg Config *ServerConfig Connection IConnection log log.Logger @@ -82,9 +84,10 @@ var ( ) // New creates the new LDAP connection -func New(config *ServerConfig) IServer { +func New(config *ServerConfig, cfg *setting.Cfg) IServer { return &Server{ Config: config, + cfg: cfg, log: log.New("ldap"), } } @@ -361,9 +364,9 @@ func (server *Server) users(logins []string) ( // If there are no ldap group mappings access is true // otherwise a single group must match func (server *Server) validateGrafanaUser(user *login.ExternalUserInfo) error { - if !SkipOrgRoleSync() && len(server.Config.Groups) > 0 && + if !server.cfg.LDAPSkipOrgRoleSync && len(server.Config.Groups) > 0 && (len(user.OrgRoles) == 0 && (user.IsGrafanaAdmin == nil || !*user.IsGrafanaAdmin)) { - server.log.Error( + server.log.Warn( "User does not belong in any of the specified LDAP groups", "username", user.Login, "groups", user.Groups, @@ -446,7 +449,7 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*login.ExternalUserInf } // Skipping org role sync - if SkipOrgRoleSync() { + if server.cfg.LDAPSkipOrgRoleSync { server.log.Debug("skipping organization role mapping.") return extUser, nil } diff --git a/pkg/services/ldap/ldap_login_test.go b/pkg/services/ldap/ldap_login_test.go index 806812881f9..717256a2fdd 100644 --- a/pkg/services/ldap/ldap_login_test.go +++ b/pkg/services/ldap/ldap_login_test.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/login" + "github.com/grafana/grafana/pkg/setting" ) var defaultLogin = &login.LoginUserQuery{ @@ -29,7 +30,11 @@ func TestServer_Login_UserBind_Fail(t *testing.T) { ResultCode: 49, } } + + cfg := setting.NewCfg() + cfg.LDAPEnabled = true server := &Server{ + cfg: cfg, Config: &ServerConfig{ SearchBaseDNs: []string{"BaseDNHere"}, }, @@ -99,7 +104,12 @@ func TestServer_Login_ValidCredentials(t *testing.T) { connection.BindProvider = func(username, password string) error { return nil } + + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ Attr: AttributeMap{ Username: "username", @@ -131,7 +141,12 @@ func TestServer_Login_UnauthenticatedBind(t *testing.T) { connection.UnauthenticatedBindProvider = func() error { return nil } + + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ SearchBaseDNs: []string{"BaseDNHere"}, }, @@ -173,7 +188,12 @@ func TestServer_Login_AuthenticatedBind(t *testing.T) { return nil } + + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ BindDN: "killa", BindPassword: "gorilla", @@ -211,7 +231,12 @@ func TestServer_Login_UserWildcardBind(t *testing.T) { authBindPassword = pass return nil } + + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ BindDN: "cn=%s,ou=users,dc=grafana,dc=org", SearchBaseDNs: []string{"BaseDNHere"}, diff --git a/pkg/services/ldap/ldap_private_test.go b/pkg/services/ldap/ldap_private_test.go index 1bbcfb6c48c..7db73df0359 100644 --- a/pkg/services/ldap/ldap_private_test.go +++ b/pkg/services/ldap/ldap_private_test.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/setting" ) func TestServer_getSearchRequest(t *testing.T) { @@ -52,7 +53,11 @@ func TestServer_getSearchRequest(t *testing.T) { func TestSerializeUsers(t *testing.T) { t.Run("simple case", func(t *testing.T) { + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ Attr: AttributeMap{ Username: "username", @@ -87,7 +92,11 @@ func TestSerializeUsers(t *testing.T) { }) t.Run("without lastname", func(t *testing.T) { + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ Attr: AttributeMap{ Username: "username", @@ -120,7 +129,11 @@ func TestSerializeUsers(t *testing.T) { }) t.Run("mark user without matching group as disabled", func(t *testing.T) { + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ Groups: []*GroupToOrgRole{{ GroupDN: "foo", @@ -150,7 +163,11 @@ func TestSerializeUsers(t *testing.T) { func TestServer_validateGrafanaUser(t *testing.T) { t.Run("no group config", func(t *testing.T) { + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ Groups: []*GroupToOrgRole{}, }, @@ -166,7 +183,11 @@ func TestServer_validateGrafanaUser(t *testing.T) { }) t.Run("user in group", func(t *testing.T) { + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ Groups: []*GroupToOrgRole{ { @@ -189,7 +210,11 @@ func TestServer_validateGrafanaUser(t *testing.T) { }) t.Run("user not in group", func(t *testing.T) { + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ Groups: []*GroupToOrgRole{ { diff --git a/pkg/services/ldap/ldap_test.go b/pkg/services/ldap/ldap_test.go index 22d0b8b9b9d..9fae2f16467 100644 --- a/pkg/services/ldap/ldap_test.go +++ b/pkg/services/ldap/ldap_test.go @@ -18,7 +18,7 @@ func TestNew(t *testing.T) { result := New(&ServerConfig{ Attr: AttributeMap{}, SearchBaseDNs: []string{"BaseDNHere"}, - }) + }, &setting.Cfg{}) assert.Implements(t, (*IServer)(nil), result) } @@ -67,7 +67,11 @@ func TestServer_Users(t *testing.T) { conn.setSearchResult(&result) // Set up attribute map without surname and email + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ Attr: AttributeMap{ Username: "username", @@ -160,6 +164,7 @@ func TestServer_Users(t *testing.T) { }) server := &Server{ + cfg: setting.NewCfg(), Config: &ServerConfig{ Attr: AttributeMap{ Username: "username", @@ -206,7 +211,11 @@ func TestServer_Users(t *testing.T) { } }) + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ Attr: AttributeMap{ Username: "username", @@ -279,7 +288,11 @@ func TestServer_Users(t *testing.T) { } }) + cfg := setting.NewCfg() + cfg.LDAPEnabled = true + server := &Server{ + cfg: cfg, Config: &ServerConfig{ Attr: AttributeMap{ Username: "username", @@ -312,11 +325,7 @@ func TestServer_Users(t *testing.T) { require.True(t, res[0].IsDisabled) }) t.Run("skip org role sync", func(t *testing.T) { - backup := setting.LDAPSkipOrgRoleSync - defer func() { - setting.LDAPSkipOrgRoleSync = backup - }() - setting.LDAPSkipOrgRoleSync = true + server.cfg.LDAPSkipOrgRoleSync = true res, err := server.Users([]string{"groot"}) require.NoError(t, err) @@ -327,6 +336,7 @@ func TestServer_Users(t *testing.T) { require.False(t, res[0].IsDisabled) }) t.Run("sync org role", func(t *testing.T) { + server.cfg.LDAPSkipOrgRoleSync = false res, err := server.Users([]string{"groot"}) require.NoError(t, err) require.Len(t, res, 1) diff --git a/pkg/services/ldap/multildap/multildap.go b/pkg/services/ldap/multildap/multildap.go index 25ec86fde72..2e70a8d5978 100644 --- a/pkg/services/ldap/multildap/multildap.go +++ b/pkg/services/ldap/multildap/multildap.go @@ -3,34 +3,29 @@ package multildap import ( "errors" + "github.com/grafana/grafana/pkg/cmd/grafana-cli/logger" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/login" + "github.com/grafana/grafana/pkg/setting" ) -// logger to log -var logger = log.New("ldap") - // GetConfig gets LDAP config var GetConfig = ldap.GetConfig -// IsEnabled checks if LDAP is enabled -var IsEnabled = ldap.IsEnabled - // newLDAP return instance of the single LDAP server var newLDAP = ldap.New -// ErrInvalidCredentials is returned if username and password do not match -var ErrInvalidCredentials = ldap.ErrInvalidCredentials - -// ErrCouldNotFindUser is returned when username hasn't been found (not username+password) -var ErrCouldNotFindUser = ldap.ErrCouldNotFindUser - -// ErrNoLDAPServers is returned when there is no LDAP servers specified -var ErrNoLDAPServers = errors.New("no LDAP servers are configured") - -// ErrDidNotFindUser if request for user is unsuccessful -var ErrDidNotFindUser = errors.New("did not find a user") +var ( + // ErrInvalidCredentials is returned if username and password do not match + ErrInvalidCredentials = ldap.ErrInvalidCredentials + // ErrCouldNotFindUser is returned when username hasn't been found (not username+password) + ErrCouldNotFindUser = ldap.ErrCouldNotFindUser + // ErrNoLDAPServers is returned when there is no LDAP servers specified + ErrNoLDAPServers = errors.New("no LDAP servers are configured") + // ErrDidNotFindUser if request for user is unsuccessful + ErrDidNotFindUser = errors.New("did not find a user") +) // ServerStatus holds the LDAP server status type ServerStatus struct { @@ -59,12 +54,16 @@ type IMultiLDAP interface { // MultiLDAP is basic struct of LDAP authorization type MultiLDAP struct { configs []*ldap.ServerConfig + cfg *setting.Cfg + log log.Logger } // New creates the new LDAP auth -func New(configs []*ldap.ServerConfig) IMultiLDAP { +func New(configs []*ldap.ServerConfig, cfg *setting.Cfg) IMultiLDAP { return &MultiLDAP{ configs: configs, + cfg: cfg, + log: log.New("ldap"), } } @@ -81,7 +80,7 @@ func (multiples *MultiLDAP) Ping() ([]*ServerStatus, error) { status.Host = config.Host status.Port = config.Port - server := newLDAP(config) + server := newLDAP(config, multiples.cfg) err := server.Dial() if err == nil { @@ -109,7 +108,7 @@ func (multiples *MultiLDAP) Login(query *login.LoginUserQuery) ( ldapSilentErrors := []error{} for index, config := range multiples.configs { - server := newLDAP(config) + server := newLDAP(config, multiples.cfg) if err := server.Dial(); err != nil { logDialFailure(err, config) @@ -127,7 +126,7 @@ func (multiples *MultiLDAP) Login(query *login.LoginUserQuery) ( if err != nil { if isSilentError(err) { ldapSilentErrors = append(ldapSilentErrors, err) - logger.Debug( + multiples.log.Debug( "unable to login with LDAP - skipping server", "host", config.Host, "port", config.Port, @@ -167,7 +166,7 @@ func (multiples *MultiLDAP) User(login string) ( search := []string{login} for index, config := range multiples.configs { - server := newLDAP(config) + server := newLDAP(config, multiples.cfg) if err := server.Dial(); err != nil { logDialFailure(err, config) @@ -210,7 +209,7 @@ func (multiples *MultiLDAP) Users(logins []string) ( } for index, config := range multiples.configs { - server := newLDAP(config) + server := newLDAP(config, multiples.cfg) if err := server.Dial(); err != nil { logDialFailure(err, config) diff --git a/pkg/services/ldap/multildap/multildap_test.go b/pkg/services/ldap/multildap/multildap_test.go index e15638b4ca8..f76bdb9bab1 100644 --- a/pkg/services/ldap/multildap/multildap_test.go +++ b/pkg/services/ldap/multildap/multildap_test.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/login" + "github.com/grafana/grafana/pkg/setting" //TODO(sh0rez): remove once import cycle resolved _ "github.com/grafana/grafana/pkg/api/response" @@ -18,7 +19,7 @@ func TestMultiLDAP(t *testing.T) { t.Run("Should return error for absent config list", func(t *testing.T) { setup() - multi := New([]*ldap.ServerConfig{}) + multi := New([]*ldap.ServerConfig{}, setting.NewCfg()) _, err := multi.Ping() require.Error(t, err) @@ -34,7 +35,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {Host: "10.0.0.1", Port: 361}, - }) + }, setting.NewCfg()) statuses, err := multi.Ping() @@ -52,7 +53,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {Host: "10.0.0.1", Port: 361}, - }) + }, setting.NewCfg()) statuses, err := multi.Ping() @@ -70,7 +71,7 @@ func TestMultiLDAP(t *testing.T) { t.Run("Should return error for absent config list", func(t *testing.T) { setup() - multi := New([]*ldap.ServerConfig{}) + multi := New([]*ldap.ServerConfig{}, setting.NewCfg()) _, err := multi.Login(&login.LoginUserQuery{}) require.Error(t, err) @@ -87,7 +88,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, err := multi.Login(&login.LoginUserQuery{}) @@ -103,7 +104,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, err := multi.Login(&login.LoginUserQuery{}) require.Equal(t, 2, mock.dialCalledTimes) @@ -124,7 +125,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) result, err := multi.Login(&login.LoginUserQuery{}) require.Equal(t, 1, mock.dialCalledTimes) @@ -144,7 +145,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, err := multi.Login(&login.LoginUserQuery{}) require.Equal(t, 2, mock.dialCalledTimes) @@ -163,7 +164,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, err := multi.Login(&login.LoginUserQuery{}) require.Equal(t, 2, mock.dialCalledTimes) @@ -183,7 +184,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, err := multi.Login(&login.LoginUserQuery{}) require.Equal(t, 2, mock.dialCalledTimes) @@ -201,7 +202,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, err := multi.Login(&login.LoginUserQuery{}) require.Equal(t, 1, mock.dialCalledTimes) @@ -218,7 +219,7 @@ func TestMultiLDAP(t *testing.T) { t.Run("Should return error for absent config list", func(t *testing.T) { setup() - multi := New([]*ldap.ServerConfig{}) + multi := New([]*ldap.ServerConfig{}, setting.NewCfg()) _, _, err := multi.User("test") require.Error(t, err) @@ -235,7 +236,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, _, err := multi.User("test") @@ -250,7 +251,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, _, err := multi.User("test") require.Equal(t, 2, mock.dialCalledTimes) @@ -270,7 +271,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, _, err := multi.User("test") require.Equal(t, 1, mock.dialCalledTimes) @@ -297,7 +298,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) user, _, err := multi.User("test") require.Equal(t, 1, mock.dialCalledTimes) @@ -318,7 +319,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, _, err := multi.User("test") require.Equal(t, 2, mock.dialCalledTimes) @@ -337,7 +338,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, err := multi.Users([]string{"test"}) require.Equal(t, 2, mock.dialCalledTimes) @@ -348,7 +349,7 @@ func TestMultiLDAP(t *testing.T) { t.Run("Should return error for absent config list", func(t *testing.T) { setup() - multi := New([]*ldap.ServerConfig{}) + multi := New([]*ldap.ServerConfig{}, setting.NewCfg()) _, err := multi.Users([]string{"test"}) require.Error(t, err) @@ -365,7 +366,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, err := multi.Users([]string{"test"}) @@ -380,7 +381,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, err := multi.Users([]string{"test"}) require.Equal(t, 2, mock.dialCalledTimes) @@ -400,7 +401,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) _, err := multi.Users([]string{"test"}) require.Equal(t, 1, mock.dialCalledTimes) @@ -433,7 +434,7 @@ func TestMultiLDAP(t *testing.T) { multi := New([]*ldap.ServerConfig{ {}, {}, - }) + }, setting.NewCfg()) users, err := multi.Users([]string{"test"}) require.Equal(t, 2, mock.dialCalledTimes) @@ -511,7 +512,7 @@ func (mock *mockLDAP) Bind() error { func setup() *mockLDAP { mock := &mockLDAP{} - newLDAP = func(config *ldap.ServerConfig) ldap.IServer { + newLDAP = func(config *ldap.ServerConfig, cfg *setting.Cfg) ldap.IServer { return mock } diff --git a/pkg/services/ldap/service/fake.go b/pkg/services/ldap/service/fake.go new file mode 100644 index 00000000000..05940531038 --- /dev/null +++ b/pkg/services/ldap/service/fake.go @@ -0,0 +1,40 @@ +package service + +import ( + "github.com/grafana/grafana/pkg/services/ldap" + "github.com/grafana/grafana/pkg/services/ldap/multildap" + "github.com/grafana/grafana/pkg/services/login" +) + +type LDAPFakeService struct { + ExpectedConfig *ldap.Config + ExpectedClient multildap.IMultiLDAP + ExpectedError error + ExpectedUser *login.ExternalUserInfo + UserCalled bool +} + +func NewLDAPFakeService() *LDAPFakeService { + return &LDAPFakeService{} +} + +func (s *LDAPFakeService) ReloadConfig() error { + return s.ExpectedError +} + +func (s *LDAPFakeService) Config() *ldap.Config { + return s.ExpectedConfig +} + +func (s *LDAPFakeService) Client() multildap.IMultiLDAP { + return s.ExpectedClient +} + +func (s *LDAPFakeService) Login(query *login.LoginUserQuery) (*login.ExternalUserInfo, error) { + return s.ExpectedUser, s.ExpectedError +} + +func (s *LDAPFakeService) User(username string) (*login.ExternalUserInfo, error) { + s.UserCalled = true + return s.ExpectedUser, s.ExpectedError +} diff --git a/pkg/services/ldap/service/helpers.go b/pkg/services/ldap/service/helpers.go new file mode 100644 index 00000000000..3de1570fad2 --- /dev/null +++ b/pkg/services/ldap/service/helpers.go @@ -0,0 +1,87 @@ +package service + +import ( + "fmt" + "os" + + "github.com/BurntSushi/toml" + + "github.com/grafana/grafana/pkg/cmd/grafana-cli/logger" + "github.com/grafana/grafana/pkg/services/ldap" + "github.com/grafana/grafana/pkg/setting" +) + +const defaultTimeout = 10 + +func readConfig(configFile string) (*ldap.Config, error) { + result := &ldap.Config{} + + logger.Info("LDAP enabled, reading config file", "file", configFile) + + // nolint:gosec + // We can ignore the gosec G304 warning on this one because `filename` comes from grafana configuration file + fileBytes, err := os.ReadFile(configFile) + if err != nil { + return nil, fmt.Errorf("%v: %w", "Failed to load LDAP config file", err) + } + + // interpolate full toml string (it can contain ENV variables) + stringContent, err := setting.ExpandVar(string(fileBytes)) + if err != nil { + return nil, fmt.Errorf("%v: %w", "Failed to expand variables", err) + } + + _, err = toml.Decode(stringContent, result) + if err != nil { + return nil, fmt.Errorf("%v: %w", "Failed to load LDAP config file", err) + } + + if len(result.Servers) == 0 { + return nil, fmt.Errorf("LDAP enabled but no LDAP servers defined in config file") + } + + for _, server := range result.Servers { + // set default org id + err = assertNotEmptyCfg(server.SearchFilter, "search_filter") + if err != nil { + return nil, fmt.Errorf("%v: %w", "Failed to validate SearchFilter section", err) + } + err = assertNotEmptyCfg(server.SearchBaseDNs, "search_base_dns") + if err != nil { + return nil, fmt.Errorf("%v: %w", "Failed to validate SearchBaseDNs section", err) + } + + for _, groupMap := range server.Groups { + if groupMap.OrgRole == "" && groupMap.IsGrafanaAdmin == nil { + return nil, fmt.Errorf("LDAP group mapping: organization role or grafana admin status is required") + } + + if groupMap.OrgId == 0 { + groupMap.OrgId = 1 + } + } + + // set default timeout if unspecified + if server.Timeout == 0 { + server.Timeout = defaultTimeout + } + } + + return result, nil +} + +func assertNotEmptyCfg(val interface{}, propName string) error { + switch v := val.(type) { + case string: + if v == "" { + return fmt.Errorf("LDAP config file is missing option: %q", propName) + } + case []string: + if len(v) == 0 { + return fmt.Errorf("LDAP config file is missing option: %q", propName) + } + default: + fmt.Println("unknown") + } + return nil +} diff --git a/pkg/services/ldap/service/ldap.go b/pkg/services/ldap/service/ldap.go new file mode 100644 index 00000000000..ce7650c786f --- /dev/null +++ b/pkg/services/ldap/service/ldap.go @@ -0,0 +1,122 @@ +package service + +import ( + "errors" + "sync" + + "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/setting" +) + +var ( + ErrUnableToCreateLDAPClient = errors.New("unable to create LDAP client") + ErrLDAPNotEnabled = errors.New("LDAP not enabled") +) + +// LDAP is the interface for the LDAP service. +type LDAP interface { + ReloadConfig() error + Config() *ldap.Config + Client() multildap.IMultiLDAP + + // Login authenticates the user against the LDAP server. + Login(query *login.LoginUserQuery) (*login.ExternalUserInfo, error) + // User searches for a user in the LDAP server. + User(username string) (*login.ExternalUserInfo, error) +} + +type LDAPImpl struct { + client multildap.IMultiLDAP + cfg *setting.Cfg + ldapCfg *ldap.Config + log log.Logger + + // loadingMutex locks the reading of the config so multiple requests for reloading are sequential. + loadingMutex *sync.Mutex +} + +func ProvideService(cfg *setting.Cfg) *LDAPImpl { + s := &LDAPImpl{ + client: nil, + ldapCfg: nil, + cfg: cfg, + log: log.New("ldap.service"), + loadingMutex: &sync.Mutex{}, + } + + if !cfg.LDAPEnabled { + return s + } + + ldapCfg, err := multildap.GetConfig(s.cfg) + if err != nil { + s.log.Error("Failed to get LDAP config", "error", err) + } else { + s.ldapCfg = ldapCfg + s.client = multildap.New(s.ldapCfg.Servers, s.cfg) + } + + return s +} + +func (s *LDAPImpl) ReloadConfig() error { + if !s.cfg.LDAPEnabled { + return nil + } + + s.loadingMutex.Lock() + defer s.loadingMutex.Unlock() + + config, err := readConfig(s.cfg.LDAPConfigFilePath) + if err != nil { + return err + } + + client := multildap.New(config.Servers, s.cfg) + if client == nil { + return ErrUnableToCreateLDAPClient + } + + s.ldapCfg = config + s.client = client + + return nil +} + +func (s *LDAPImpl) Client() multildap.IMultiLDAP { + return s.client +} + +func (s *LDAPImpl) Config() *ldap.Config { + return s.ldapCfg +} + +func (s *LDAPImpl) Login(query *login.LoginUserQuery) (*login.ExternalUserInfo, error) { + if !s.cfg.LDAPEnabled { + return nil, ErrLDAPNotEnabled + } + + client := s.Client() + if client == nil { + return nil, ErrUnableToCreateLDAPClient + } + + return client.Login(query) +} + +func (s *LDAPImpl) User(username string) (*login.ExternalUserInfo, error) { + if !s.cfg.LDAPEnabled { + return nil, ErrLDAPNotEnabled + } + + client := s.Client() + if client == nil { + return nil, ErrUnableToCreateLDAPClient + } + + user, _, err := client.User(username) + return user, err +} diff --git a/pkg/services/ldap/settings.go b/pkg/services/ldap/settings.go index 5e29b9b514e..aa961752484 100644 --- a/pkg/services/ldap/settings.go +++ b/pkg/services/ldap/settings.go @@ -71,29 +71,6 @@ var logger = log.New("ldap") // loadingMutex locks the reading of the config so multiple requests for reloading are sequential. var loadingMutex = &sync.Mutex{} -// IsEnabled checks if ldap is enabled -func IsEnabled() bool { - return setting.LDAPEnabled -} - -func SkipOrgRoleSync() bool { - return setting.LDAPSkipOrgRoleSync -} - -// ReloadConfig reads the config from the disk and caches it. -func ReloadConfig(ldapConfigFilePath string) error { - if !IsEnabled() { - return nil - } - - loadingMutex.Lock() - defer loadingMutex.Unlock() - - var err error - config, err = readConfig(ldapConfigFilePath) - return err -} - // We need to define in this space so `GetConfig` fn // could be defined as singleton var config *Config @@ -105,7 +82,7 @@ func GetConfig(cfg *setting.Cfg) (*Config, error) { if !cfg.LDAPEnabled { return nil, nil } - } else if !IsEnabled() { + } else if !cfg.LDAPEnabled { return nil, nil } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index c46397f9d8f..71e989f3bf3 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -140,14 +140,6 @@ var ( RudderstackSdkUrl string RudderstackConfigUrl string - // LDAP - LDAPEnabled bool - LDAPSkipOrgRoleSync bool - LDAPConfigFile string - LDAPSyncCron string - LDAPAllowSignup bool - LDAPActiveSyncEnabled bool - // Alerting AlertingEnabled *bool ExecuteAlerts bool @@ -440,10 +432,12 @@ type Cfg struct { GenericOAuthSkipOrgRoleSync bool // LDAP - LDAPEnabled bool - LDAPSkipOrgRoleSync bool - LDAPConfigFilePath string - LDAPAllowSignup bool + LDAPEnabled bool + LDAPSkipOrgRoleSync bool + LDAPConfigFilePath string + LDAPAllowSignup bool + LDAPActiveSyncEnabled bool + LDAPSyncCron string DefaultTheme string DefaultLanguage string @@ -1206,16 +1200,12 @@ func (cfg *Cfg) readSAMLConfig() { func (cfg *Cfg) readLDAPConfig() { ldapSec := cfg.Raw.Section("auth.ldap") - LDAPConfigFile = ldapSec.Key("config_file").String() - cfg.LDAPConfigFilePath = LDAPConfigFile - LDAPSyncCron = ldapSec.Key("sync_cron").String() - LDAPEnabled = ldapSec.Key("enabled").MustBool(false) - cfg.LDAPEnabled = LDAPEnabled - LDAPSkipOrgRoleSync = ldapSec.Key("skip_org_role_sync").MustBool(false) - cfg.LDAPSkipOrgRoleSync = LDAPSkipOrgRoleSync - LDAPActiveSyncEnabled = ldapSec.Key("active_sync_enabled").MustBool(false) - LDAPAllowSignup = ldapSec.Key("allow_sign_up").MustBool(true) - cfg.LDAPAllowSignup = LDAPAllowSignup + cfg.LDAPConfigFilePath = ldapSec.Key("config_file").String() + cfg.LDAPSyncCron = ldapSec.Key("sync_cron").String() + cfg.LDAPEnabled = ldapSec.Key("enabled").MustBool(false) + cfg.LDAPSkipOrgRoleSync = ldapSec.Key("skip_org_role_sync").MustBool(false) + cfg.LDAPActiveSyncEnabled = ldapSec.Key("active_sync_enabled").MustBool(false) + cfg.LDAPAllowSignup = ldapSec.Key("allow_sign_up").MustBool(true) } func (cfg *Cfg) handleAWSConfig() {