Fix backwards compat on "domain" config + remove "domains" config

Fixes #48

internal/config.go

@@ -31,7 +31,7 @@ type Config struct {
 	CookieName     string               `long:"cookie-name" env:"COOKIE_NAME" default:"_forward_auth" description:"Cookie Name"`
 	CSRFCookieName string               `long:"csrf-cookie-name" env:"CSRF_COOKIE_NAME" default:"_forward_auth_csrf" description:"CSRF Cookie Name"`
 	DefaultAction  string               `long:"default-action" env:"DEFAULT_ACTION" default:"auth" choice:"auth" choice:"allow" description:"Default action"`
-	Domains        []string             `long:"domain" env:"DOMAIN" description:"Only allow given email domains, can be set multiple times"`
+	Domains        CommaSeparatedList   `long:"domain" env:"DOMAIN" description:"Only allow given email domains, can be set multiple times"`
 	LifetimeString int                  `long:"lifetime" env:"LIFETIME" default:"43200" description:"Lifetime in seconds"`
 	Path           string               `long:"url-path" env:"URL_PATH" default:"/_oauth" description:"Callback URL Path"`
 	SecretString   string               `long:"secret" env:"SECRET" description:"Secret used for signing (required)" json:"-"`
@@ -45,13 +45,12 @@ type Config struct {
 	Lifetime time.Duration
 
 	// Legacy
-	CookieDomainsLegacy CookieDomains      `long:"cookie-domains" env:"COOKIE_DOMAINS" description:"DEPRECATED - Use \"cookie-domain\""`
+	CookieDomainsLegacy CookieDomains `long:"cookie-domains" env:"COOKIE_DOMAINS" description:"DEPRECATED - Use \"cookie-domain\""`
-	CookieSecretLegacy  string             `long:"cookie-secret" env:"COOKIE_SECRET" description:"DEPRECATED - Use \"secret\""  json:"-"`
+	CookieSecretLegacy  string        `long:"cookie-secret" env:"COOKIE_SECRET" description:"DEPRECATED - Use \"secret\""  json:"-"`
-	CookieSecureLegacy  string             `long:"cookie-secure" env:"COOKIE_SECURE" description:"DEPRECATED - Use \"insecure-cookie\""`
+	CookieSecureLegacy  string        `long:"cookie-secure" env:"COOKIE_SECURE" description:"DEPRECATED - Use \"insecure-cookie\""`
-	DomainsLegacy       CommaSeparatedList `long:"domains" env:"DOMAINS" description:"DEPRECATED - Use \"domain\""`
+	ClientIdLegacy      string        `long:"client-id" env:"CLIENT_ID" group:"DEPs" description:"DEPRECATED - Use \"providers.google.client-id\""`
-	ClientIdLegacy      string             `long:"client-id" env:"CLIENT_ID" group:"DEPs" description:"DEPRECATED - Use \"providers.google.client-id\""`
+	ClientSecretLegacy  string        `long:"client-secret" env:"CLIENT_SECRET" description:"DEPRECATED - Use \"providers.google.client-id\""  json:"-"`
-	ClientSecretLegacy  string             `long:"client-secret" env:"CLIENT_SECRET" description:"DEPRECATED - Use \"providers.google.client-id\""  json:"-"`
+	PromptLegacy        string        `long:"prompt" env:"PROMPT" description:"DEPRECATED - Use \"providers.google.prompt\""`
-	PromptLegacy        string             `long:"prompt" env:"PROMPT" description:"DEPRECATED - Use \"providers.google.prompt\""`
 }
 
 func NewGlobalConfig() Config {
@@ -125,10 +124,6 @@ func NewConfig(args []string) (Config, error) {
 		fmt.Println("cookie-domains config option is deprecated, please use cookie-domain")
 		c.CookieDomains = append(c.CookieDomains, c.CookieDomainsLegacy...)
 	}
-	if len(c.DomainsLegacy) > 0 {
-		fmt.Println("domains config option is deprecated, please use domain")
-		c.Domains = append(c.Domains, c.DomainsLegacy...)
-	}
 
 	// Transformations
 	if len(c.Path) > 0 && c.Path[0] != '/' {

internal/config_test.go

@@ -120,7 +120,6 @@ func TestConfigParseRuleError(t *testing.T) {
 	assert.Equal(map[string]*Rule{}, c.Rules)
 }
 
-
 func TestConfigFlagBackwardsCompatability(t *testing.T) {
 	assert := assert.New(t)
 	c, err := NewConfig([]string{
@@ -132,7 +131,7 @@ func TestConfigFlagBackwardsCompatability(t *testing.T) {
 		"--cookie-secure=false",
 		"--cookie-domains=test1.com,example.org",
 		"--cookie-domain=another1.net",
-		"--domains=test2.com,example.org",
+		"--domain=test2.com,example.org",
 		"--domain=another2.net",
 		"--whitelist=test3.com,example.org",
 		"--whitelist=another3.net",
@@ -147,7 +146,7 @@ func TestConfigFlagBackwardsCompatability(t *testing.T) {
 	}
 	assert.Equal(expected1, c.CookieDomains, "should read legacy comma separated list cookie-domains")
 
-	expected2 := []string{"another2.net", "test2.com", "example.org"}
+	expected2 := CommaSeparatedList{"test2.com", "example.org", "another2.net"}
 	assert.Equal(expected2, c.Domains, "should read legacy comma separated list domains")
 
 	expected3 := CommaSeparatedList{"test3.com", "example.org", "another3.net"}
@@ -222,6 +221,70 @@ func TestConfigParseEnvironment(t *testing.T) {
 
 	assert.Equal("env_cookie_name", c.CookieName, "variable should be read from environment")
 	assert.Equal("env_client_id", c.Providers.Google.ClientId, "namespace variable should be read from environment")
+
+	os.Unsetenv("COOKIE_NAME")
+	os.Unsetenv("PROVIDERS_GOOGLE_CLIENT_ID")
+}
+
+func TestConfigParseEnvironmentBackwardsCompatability(t *testing.T) {
+	assert := assert.New(t)
+	vars := map[string]string{
+		"CLIENT_ID":      "clientid",
+		"CLIENT_SECRET":  "verysecret",
+		"PROMPT":         "prompt",
+		"COOKIE_SECRET":  "veryverysecret",
+		"LIFETIME":       "200",
+		"COOKIE_SECURE":  "false",
+		"COOKIE_DOMAINS": "test1.com,example.org",
+		"COOKIE_DOMAIN":  "another1.net",
+		"DOMAIN":        "test2.com,example.org",
+		"WHITELIST":      "test3.com,example.org",
+	}
+	for k, v := range vars {
+		os.Setenv(k, v)
+	}
+	c, err := NewConfig([]string{})
+	require.Nil(t, err)
+
+	// The following used to be passed as comma separated list
+	expected1 := []CookieDomain{
+		*NewCookieDomain("another1.net"),
+		*NewCookieDomain("test1.com"),
+		*NewCookieDomain("example.org"),
+	}
+	assert.Equal(expected1, c.CookieDomains, "should read legacy comma separated list cookie-domains")
+
+	expected2 := CommaSeparatedList{"test2.com", "example.org"}
+	assert.Equal(expected2, c.Domains, "should read legacy comma separated list domains")
+
+	expected3 := CommaSeparatedList{"test3.com", "example.org"}
+	assert.Equal(expected3, c.Whitelist, "should read legacy comma separated list whitelist")
+
+	// Name changed
+	assert.Equal([]byte("veryverysecret"), c.Secret)
+
+	// Google provider params used to be top level
+	assert.Equal("clientid", c.ClientIdLegacy)
+	assert.Equal("clientid", c.Providers.Google.ClientId, "--client-id should set providers.google.client-id")
+	assert.Equal("verysecret", c.ClientSecretLegacy)
+	assert.Equal("verysecret", c.Providers.Google.ClientSecret, "--client-secret should set providers.google.client-secret")
+	assert.Equal("prompt", c.PromptLegacy)
+	assert.Equal("prompt", c.Providers.Google.Prompt, "--prompt should set providers.google.promot")
+
+	// "cookie-secure" used to be a standard go bool flag that could take
+	// true, TRUE, 1, false, FALSE, 0 etc. values.
+	// Here we're checking that format is still suppoted
+	assert.Equal("false", c.CookieSecureLegacy)
+	assert.True(c.InsecureCookie, "--cookie-secure=false should set insecure-cookie true")
+
+	c, err = NewConfig([]string{"--cookie-secure=TRUE"})
+	assert.Nil(err)
+	assert.Equal("TRUE", c.CookieSecureLegacy)
+	assert.False(c.InsecureCookie, "--cookie-secure=TRUE should set insecure-cookie false")
+
+	for k := range vars {
+		os.Unsetenv(k)
+	}
 }
 
 func TestConfigTransformation(t *testing.T) {