From 53f8088316b23cff53459e3273a708da46fcd1fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20Gyllensw=C3=A4rd?= Date: Thu, 7 Nov 2019 11:24:54 +0100 Subject: [PATCH] Auth Proxy: replace ini setting ldap_sync_ttl with sync_ttl (#20191) * Renamed ttl config in code to be more consistent with behaviour * Introduced new setting `sync_ttl` in .ini file * Keeping the old setting `ldap_sync_ttl` in the .ini file as fallback and compatibility. --- conf/defaults.ini | 1 + pkg/middleware/auth_proxy/auth_proxy.go | 2 +- pkg/setting/setting.go | 14 ++++++-- pkg/setting/setting_test.go | 44 +++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index d111b3d24ef..47b847b2a27 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -435,6 +435,7 @@ header_name = X-WEBAUTH-USER header_property = username auto_sign_up = true ldap_sync_ttl = 60 +sync_ttl = 60 whitelist = headers = diff --git a/pkg/middleware/auth_proxy/auth_proxy.go b/pkg/middleware/auth_proxy/auth_proxy.go index 13f2141809d..f81c7a951ab 100644 --- a/pkg/middleware/auth_proxy/auth_proxy.go +++ b/pkg/middleware/auth_proxy/auth_proxy.go @@ -92,7 +92,7 @@ func New(options *Options) *AuthProxy { headerType: setting.AuthProxyHeaderProperty, headers: setting.AuthProxyHeaders, whitelistIP: setting.AuthProxyWhitelist, - cacheTTL: setting.AuthProxyLDAPSyncTtl, + cacheTTL: setting.AuthProxySyncTtl, LDAPAllowSignup: setting.LDAPAllowSignup, AuthProxyAutoSignUp: setting.AuthProxyAutoSignUp, } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index eb333465556..54bd4dc6d87 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -147,7 +147,7 @@ var ( AuthProxyHeaderName string AuthProxyHeaderProperty string AuthProxyAutoSignUp bool - AuthProxyLDAPSyncTtl int + AuthProxySyncTtl int AuthProxyWhitelist string AuthProxyHeaders map[string]string @@ -854,7 +854,17 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { return err } AuthProxyAutoSignUp = authProxy.Key("auto_sign_up").MustBool(true) - AuthProxyLDAPSyncTtl = authProxy.Key("ldap_sync_ttl").MustInt() + + ldapSyncVal := authProxy.Key("ldap_sync_ttl").MustInt() + syncVal := authProxy.Key("sync_ttl").MustInt() + + if ldapSyncVal != 60 { + AuthProxySyncTtl = ldapSyncVal + cfg.Logger.Warn("[Deprecated] the configuration setting 'ldap_sync_ttl' is deprecated, please use 'sync_ttl' instead") + } else { + AuthProxySyncTtl = syncVal + } + AuthProxyWhitelist, err = valueAsString(authProxy, "whitelist", "") if err != nil { return err diff --git a/pkg/setting/setting_test.go b/pkg/setting/setting_test.go index 5e0bcd0ae65..95f6e2b7d91 100644 --- a/pkg/setting/setting_test.go +++ b/pkg/setting/setting_test.go @@ -227,6 +227,50 @@ func TestLoadingSettings(t *testing.T) { So(cfg.RendererCallbackUrl, ShouldEqual, "http://myserver/renderer/") }) + + Convey("Only sync_ttl should return the value sync_ttl", func() { + cfg := NewCfg() + err := cfg.Load(&CommandLineArgs{ + HomePath: "../../", + Args: []string{"cfg:auth.proxy.sync_ttl=2"}, + }) + So(err, ShouldBeNil) + + So(AuthProxySyncTtl, ShouldEqual, 2) + }) + + Convey("Only ldap_sync_ttl should return the value ldap_sync_ttl", func() { + cfg := NewCfg() + err := cfg.Load(&CommandLineArgs{ + HomePath: "../../", + Args: []string{"cfg:auth.proxy.ldap_sync_ttl=5"}, + }) + So(err, ShouldBeNil) + + So(AuthProxySyncTtl, ShouldEqual, 5) + }) + + Convey("ldap_sync should override ldap_sync_ttl that is default value", func() { + cfg := NewCfg() + err := cfg.Load(&CommandLineArgs{ + HomePath: "../../", + Args: []string{"cfg:auth.proxy.sync_ttl=5"}, + }) + So(err, ShouldBeNil) + + So(AuthProxySyncTtl, ShouldEqual, 5) + }) + + Convey("ldap_sync should not override ldap_sync_ttl that is different from default value", func() { + cfg := NewCfg() + err := cfg.Load(&CommandLineArgs{ + HomePath: "../../", + Args: []string{"cfg:auth.proxy.ldap_sync_ttl=12", "cfg:auth.proxy.sync_ttl=5"}, + }) + So(err, ShouldBeNil) + + So(AuthProxySyncTtl, ShouldEqual, 12) + }) }) Convey("Test reading string values from .ini file", t, func() {