Match emails against *either* whitelist or domains when both are prov…

…ided

The previous behaviour would ignore domains if the whitelist parameter
was provided, however if both parameters are provided then matching
either is more likely the intent.

README.md

@@ -249,6 +249,16 @@ All options can be supplied in any of the following ways, in the following prece
 
    When set, users will be redirected to this URL following logout.
 
+- `match-whitelist-or-domain`
+
+   When enabled, users will be permitted if they match *either* the `whitelist` or `domain` parameters.
+
+   This will be enabled by default in v3, but is disabled by default in v2 to maintain backwards compatibility.
+
+   Default: `false`
+
+   For more details, please also read [User Restriction](#user-restriction) in the concepts section.
+
 - `url-path`
 
    Customise the path that this service uses to handle the callback following authentication.
@@ -318,7 +328,7 @@ You can restrict who can login with the following parameters:
 * `domain` - Use this to limit logins to a specific domain, e.g. test.com only
 * `whitelist` - Use this to only allow specific users to login e.g. thom@test.com only
 
-Note, if you pass `whitelist` then only this is checked and `domain` is effectively ignored.
+Note, if you pass both `whitelist` and `domain`, then the default behaviour is for only `whitelist` to be used and `domain` will be effectively ignored. You can allow users matching *either* `whitelist` or `domain` by passing the `match-whitelist-or-domain` parameter (this will be the default behaviour in v3).
 
 ### Forwarded Headers
 

internal/auth.go

@@ -56,30 +56,43 @@ func ValidateCookie(r *http.Request, c *http.Cookie) (string, error) {
 	return parts[2], nil
 }
 
-// ValidateEmail verifies that an email is permitted by the current config
+// ValidateEmail checks if the given email address matches either a whitelisted
+// email address, as defined by the "whitelist" config parameter. Or is part of
+// a permitted domain, as defined by the "domains" config parameter
 func ValidateEmail(email string) bool {
-	found := false
+	// Do we have any validation to perform?
+	if len(config.Whitelist) == 0 && len(config.Domains) == 0 {
+		return true
+	}
+
+	// Email whitelist validation
 	if len(config.Whitelist) > 0 {
 		for _, whitelist := range config.Whitelist {
 			if email == whitelist {
-				found = true
+				return true
 			}
 		}
-	} else if len(config.Domains) > 0 {
+
+		// If we're not matching *either*, stop here
+		if !config.MatchWhitelistOrDomain {
+			return false
+		}
+	}
+
+	// Domain validation
+	if len(config.Domains) > 0 {
 		parts := strings.Split(email, "@")
 		if len(parts) < 2 {
 			return false
 		}
 		for _, domain := range config.Domains {
 			if domain == parts[1] {
-				found = true
+				return true
 			}
 		}
-	} else {
-		return true
 	}
 
-	return found
+	return false
 }
 
 // Utility methods

internal/auth_test.go

@@ -93,6 +93,30 @@ func TestAuthValidateEmail(t *testing.T) {
 	config.Whitelist = []string{"test@test.com"}
 	v = ValidateEmail("test@test.com")
 	assert.True(v, "should allow user in whitelist")
+
+	// Should allow only matching email address when
+	// MatchWhitelistOrDomain is disabled
+	config.Domains = []string{"example.com"}
+	config.Whitelist = []string{"test@test.com"}
+	config.MatchWhitelistOrDomain = false
+	v = ValidateEmail("test@test.com")
+	assert.True(v, "should allow user in whitelist")
+	v = ValidateEmail("test@example.com")
+	assert.False(v, "should not allow user from valid domain")
+	v = ValidateEmail("one@two.com")
+	assert.False(v, "should not allow user not in either")
+
+	// Should allow either matching domain or email address when
+	// MatchWhitelistOrDomain is enabled
+	config.Domains = []string{"example.com"}
+	config.Whitelist = []string{"test@test.com"}
+	config.MatchWhitelistOrDomain = true
+	v = ValidateEmail("test@test.com")
+	assert.True(v, "should allow user in whitelist")
+	v = ValidateEmail("test@example.com")
+	assert.True(v, "should allow user from valid domain")
+	v = ValidateEmail("one@two.com")
+	assert.False(v, "should not allow user not in either")
 }
 
 func TestRedirectUri(t *testing.T) {

internal/config.go

@@ -24,20 +24,21 @@ type Config struct {
 	LogLevel  string `long:"log-level" env:"LOG_LEVEL" default:"warn" choice:"trace" choice:"debug" choice:"info" choice:"warn" choice:"error" choice:"fatal" choice:"panic" description:"Log level"`
 	LogFormat string `long:"log-format"  env:"LOG_FORMAT" default:"text" choice:"text" choice:"json" choice:"pretty" description:"Log format"`
 
-	AuthHost        string               `long:"auth-host" env:"AUTH_HOST" description:"Single host to use when returning from 3rd party auth"`
-	Config          func(s string) error `long:"config" env:"CONFIG" description:"Path to config file" json:"-"`
-	CookieDomains   []CookieDomain       `long:"cookie-domain" env:"COOKIE_DOMAIN" env-delim:"," description:"Domain to set auth cookie on, can be set multiple times"`
-	InsecureCookie  bool                 `long:"insecure-cookie" env:"INSECURE_COOKIE" description:"Use insecure cookies"`
-	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"`
-	DefaultProvider string               `long:"default-provider" env:"DEFAULT_PROVIDER" default:"google" choice:"google" choice:"oidc" description:"Default provider"`
-	Domains         CommaSeparatedList   `long:"domain" env:"DOMAIN" env-delim:"," description:"Only allow given email domains, can be set multiple times"`
-	LifetimeString  int                  `long:"lifetime" env:"LIFETIME" default:"43200" description:"Lifetime in seconds"`
-	LogoutRedirect  string               `long:"logout-redirect" env:"LOGOUT_REDIRECT" description:"URL to redirect to following logout"`
-	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:"-"`
-	Whitelist       CommaSeparatedList   `long:"whitelist" env:"WHITELIST" env-delim:"," description:"Only allow given email addresses, can be set multiple times"`
+	AuthHost               string               `long:"auth-host" env:"AUTH_HOST" description:"Single host to use when returning from 3rd party auth"`
+	Config                 func(s string) error `long:"config" env:"CONFIG" description:"Path to config file" json:"-"`
+	CookieDomains          []CookieDomain       `long:"cookie-domain" env:"COOKIE_DOMAIN" env-delim:"," description:"Domain to set auth cookie on, can be set multiple times"`
+	InsecureCookie         bool                 `long:"insecure-cookie" env:"INSECURE_COOKIE" description:"Use insecure cookies"`
+	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"`
+	DefaultProvider        string               `long:"default-provider" env:"DEFAULT_PROVIDER" default:"google" choice:"google" choice:"oidc" description:"Default provider"`
+	Domains                CommaSeparatedList   `long:"domain" env:"DOMAIN" env-delim:"," description:"Only allow given email domains, can be set multiple times"`
+	LifetimeString         int                  `long:"lifetime" env:"LIFETIME" default:"43200" description:"Lifetime in seconds"`
+	LogoutRedirect         string               `long:"logout-redirect" env:"LOGOUT_REDIRECT" description:"URL to redirect to following logout"`
+	MatchWhitelistOrDomain bool                 `long:"match-whitelist-or-domain" env:"MATCH_WHITELIST_OR_DOMAIN" description:"Allow users that match *either* whitelist or domain (enabled by default in v3)"`
+	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:"-"`
+	Whitelist              CommaSeparatedList   `long:"whitelist" env:"WHITELIST" env-delim:"," description:"Only allow given email addresses, can be set multiple times"`
 
 	Providers provider.Providers `group:"providers" namespace:"providers" env-namespace:"PROVIDERS"`
 	Rules     map[string]*Rule   `long:"rule.<name>.<param>" description:"Rule definitions, param can be: \"action\", \"rule\" or \"provider\""`

internal/config_test.go

@@ -34,6 +34,7 @@ func TestConfigDefaults(t *testing.T) {
 	assert.Len(c.Domains, 0)
 	assert.Equal(time.Second*time.Duration(43200), c.Lifetime)
 	assert.Equal("", c.LogoutRedirect)
+	assert.False(c.MatchWhitelistOrDomain)
 	assert.Equal("/_oauth", c.Path)
 	assert.Len(c.Whitelist, 0)