LDAP: Added reload endpoint for LDAP config (#15470)

* 4843 - Added reload endpoint for LDAP config

closes #4843

* Refactor to make the reload work after master drifted

pkg/api/admin_ldap.go

@@ -0,0 +1,17 @@
+package api
+
+import (
+	"github.com/grafana/grafana/pkg/services/ldap"
+)
+
+func (server *HTTPServer) ReloadLdapCfg() Response {
+	if !ldap.IsEnabled() {
+		return Error(400, "LDAP is not enabled", nil)
+	}
+
+	err := ldap.ReloadConfig()
+	if err != nil {
+		return Error(500, "Failed to reload ldap config.", err)
+	}
+	return Success("Ldap config reloaded")
+}

pkg/api/api.go

@@ -393,6 +393,7 @@ func (hs *HTTPServer) registerRoutes() {
 		adminRoute.Post("/provisioning/dashboards/reload", Wrap(hs.AdminProvisioningReloadDasboards))
 		adminRoute.Post("/provisioning/datasources/reload", Wrap(hs.AdminProvisioningReloadDatasources))
 		adminRoute.Post("/provisioning/notifications/reload", Wrap(hs.AdminProvisioningReloadNotifications))
+		adminRoute.Post("/ldap/reload", Wrap(hs.ReloadLdapCfg))
 	}, reqGrafanaAdmin)
 
 	// rendering

pkg/login/ldap_login.go

@@ -3,12 +3,15 @@ package login
 import (
 	"github.com/grafana/grafana/pkg/models"
 	LDAP "github.com/grafana/grafana/pkg/services/ldap"
+	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
 var newLDAP = LDAP.New
-var readLDAPConfig = LDAP.ReadConfig
+var getLDAPConfig = LDAP.GetConfig
 var isLDAPEnabled = LDAP.IsEnabled
 
+// 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(query *models.LoginUserQuery) (bool, error) {
 	enabled := isLDAPEnabled()
 
@@ -16,7 +19,10 @@ var loginUsingLdap = func(query *models.LoginUserQuery) (bool, error) {
 		return false, nil
 	}
 
-	config := readLDAPConfig()
+	config, err := getLDAPConfig()
+	if err != nil {
+		return true, errutil.Wrap("Failed to get LDAP config", err)
+	}
 	if len(config.Servers) == 0 {
 		return true, ErrNoLDAPServers
 	}

pkg/login/ldap_login_test.go

@@ -20,12 +20,12 @@ func TestLdapLogin(t *testing.T) {
 
 			ldapLoginScenario("When login", func(sc *ldapLoginScenarioContext) {
 				sc.withLoginResult(false)
-				readLDAPConfig = func() *LDAP.Config {
+				getLDAPConfig = func() (*LDAP.Config, error) {
 					config := &LDAP.Config{
 						Servers: []*LDAP.ServerConfig{},
 					}
 
-					return config
+					return config, nil
 				}
 
 				enabled, err := loginUsingLdap(sc.loginUserQuery)
@@ -129,7 +129,7 @@ func ldapLoginScenario(desc string, fn ldapLoginScenarioFunc) {
 			ldapAuthenticatorMock: mock,
 		}
 
-		readLDAPConfig = func() *LDAP.Config {
+		getLDAPConfig = func() (*LDAP.Config, error) {
 			config := &LDAP.Config{
 				Servers: []*LDAP.ServerConfig{
 					{
@@ -138,7 +138,7 @@ func ldapLoginScenario(desc string, fn ldapLoginScenarioFunc) {
 				},
 			}
 
-			return config
+			return config, nil
 		}
 
 		newLDAP = func(server *LDAP.ServerConfig) LDAP.IAuth {
@@ -147,7 +147,7 @@ func ldapLoginScenario(desc string, fn ldapLoginScenarioFunc) {
 
 		defer func() {
 			newLDAP = LDAP.New
-			readLDAPConfig = LDAP.ReadConfig
+			getLDAPConfig = LDAP.GetConfig
 		}()
 
 		fn(sc)

pkg/middleware/auth_proxy/auth_proxy.go

@@ -22,8 +22,8 @@ const (
 )
 
 var (
-	readLDAPConfig = ldap.ReadConfig
+	getLDAPConfig = ldap.GetConfig
-	isLDAPEnabled  = ldap.IsEnabled
+	isLDAPEnabled = ldap.IsEnabled
 )
 
 // AuthProxy struct
@@ -219,7 +219,10 @@ func (auth *AuthProxy) GetUserIDViaLDAP() (int64, *Error) {
 		Username:   auth.header,
 	}
 
-	config := readLDAPConfig()
+	config, err := getLDAPConfig()
+	if err != nil {
+		return 0, newError("Failed to get LDAP config", nil)
+	}
 	if len(config.Servers) == 0 {
 		return 0, newError("No LDAP servers available", nil)
 	}

pkg/middleware/auth_proxy/auth_proxy_test.go

@@ -67,18 +67,18 @@ func TestMiddlewareContext(t *testing.T) {
 					return true
 				}
 
-				readLDAPConfig = func() *ldap.Config {
+				getLDAPConfig = func() (*ldap.Config, error) {
 					config := &ldap.Config{
 						Servers: []*ldap.ServerConfig{
 							{},
 						},
 					}
-					return config
+					return config, nil
 				}
 
 				defer func() {
 					isLDAPEnabled = ldap.IsEnabled
-					readLDAPConfig = ldap.ReadConfig
+					getLDAPConfig = ldap.GetConfig
 				}()
 
 				store := remotecache.NewFakeStore(t)
@@ -109,16 +109,16 @@ func TestMiddlewareContext(t *testing.T) {
 					return true
 				}
 
-				readLDAPConfig = func() *ldap.Config {
+				getLDAPConfig = func() (*ldap.Config, error) {
 					config := &ldap.Config{
 						Servers: []*ldap.ServerConfig{},
 					}
-					return config
+					return config, nil
 				}
 
 				defer func() {
 					isLDAPEnabled = ldap.IsEnabled
-					readLDAPConfig = ldap.ReadConfig
+					getLDAPConfig = ldap.GetConfig
 				}()
 
 				store := remotecache.NewFakeStore(t)

pkg/plugins/plugins.go

@@ -124,7 +124,7 @@ func (pm *PluginManager) Run(ctx context.Context) error {
 		}
 	}
 
-	// kil backend plugins
+	// kill backend plugins
 	for _, p := range DataSources {
 		p.Kill()
 	}

pkg/services/ldap/settings.go

@@ -2,9 +2,11 @@ package ldap
 
 import (
 	"fmt"
-	"os"
+	"sync"
 
 	"github.com/BurntSushi/toml"
+	"github.com/grafana/grafana/pkg/util/errutil"
+	"golang.org/x/xerrors"
 
 	"github.com/grafana/grafana/pkg/log"
 	m "github.com/grafana/grafana/pkg/models"
@@ -56,47 +58,72 @@ type GroupToOrgRole struct {
 var config *Config
 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
 }
 
-// ReadConfig reads the config if
+// ReloadConfig reads the config from the disc and caches it.
-// ldap is enabled otherwise it will return nil
+func ReloadConfig() error {
-func ReadConfig() *Config {
 	if IsEnabled() == false {
 		return nil
 	}
+	loadingMutex.Lock()
+	defer loadingMutex.Unlock()
+
+	var err error
+	config, err = readConfig(setting.LdapConfigFile)
+	return err
+}
+
+// GetConfig returns the LDAP config if LDAP is enabled otherwise it returns nil. It returns either cached value of
+// the config or it reads it and caches it first.
+func GetConfig() (*Config, error) {
+	if IsEnabled() == false {
+		return nil, nil
+	}
 
 	// Make it a singleton
 	if config != nil {
-		return config
+		return config, nil
 	}
 
-	config = getConfig(setting.LdapConfigFile)
+	loadingMutex.Lock()
+	defer loadingMutex.Unlock()
 
-	return config
+	var err error
+	config, err = readConfig(setting.LdapConfigFile)
+
+	return config, err
 }
-func getConfig(configFile string) *Config {
+
+func readConfig(configFile string) (*Config, error) {
 	result := &Config{}
 
 	logger.Info("Ldap enabled, reading config file", "file", configFile)
 
 	_, err := toml.DecodeFile(configFile, result)
 	if err != nil {
-		logger.Crit("Failed to load ldap config file", "error", err)
+		return nil, errutil.Wrap("Failed to load ldap config file", err)
-		os.Exit(1)
 	}
 
 	if len(result.Servers) == 0 {
-		logger.Crit("ldap enabled but no ldap servers defined in config file")
+		return nil, xerrors.New("ldap enabled but no ldap servers defined in config file")
-		os.Exit(1)
 	}
 
 	// set default org id
 	for _, server := range result.Servers {
-		assertNotEmptyCfg(server.SearchFilter, "search_filter")
+		err = assertNotEmptyCfg(server.SearchFilter, "search_filter")
-		assertNotEmptyCfg(server.SearchBaseDNs, "search_base_dns")
+		if err != nil {
+			return nil, errutil.Wrap("Failed to validate SearchFilter section", err)
+		}
+		err = assertNotEmptyCfg(server.SearchBaseDNs, "search_base_dns")
+		if err != nil {
+			return nil, errutil.Wrap("Failed to validate SearchBaseDNs section", err)
+		}
 
 		for _, groupMap := range server.Groups {
 			if groupMap.OrgId == 0 {
@@ -105,22 +132,21 @@ func getConfig(configFile string) *Config {
 		}
 	}
 
-	return result
+	return result, nil
 }
 
-func assertNotEmptyCfg(val interface{}, propName string) {
+func assertNotEmptyCfg(val interface{}, propName string) error {
 	switch v := val.(type) {
 	case string:
 		if v == "" {
-			logger.Crit("LDAP config file is missing option", "option", propName)
+			return xerrors.Errorf("LDAP config file is missing option: %v", propName)
-			os.Exit(1)
 		}
 	case []string:
 		if len(v) == 0 {
-			logger.Crit("LDAP config file is missing option", "option", propName)
+			return xerrors.Errorf("LDAP config file is missing option: %v", propName)
-			os.Exit(1)
 		}
 	default:
 		fmt.Println("unknown")
 	}
+	return nil
 }