LDAP: Move LDAP globals to Config (#63255)

* structure dtos and private methods

* add basic LDAP service

* use LDAP service in ldap debug API

* lower non fatal error

* remove unused globals

* wip

* remove final globals

* fix tests to use cfg enabled

* restructure errors

* remove logger from globals

* use ldap service in authn

* use ldap service in context handler

* fix failed tests

* fix ldap middleware provides

* fix provides in auth_test.go
This commit is contained in:
Jo
2023-02-10 19:01:55 +01:00
committed by GitHub
parent 8520a8614c
commit d4cfbd9fd3
30 changed files with 664 additions and 506 deletions
+22 -23
View File
@@ -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)
+25 -24
View File
@@ -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
}