Optionally match emails against *either* whitelist or domains when both are provided (#106)

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
-			}
-		}
-	} 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

@@ -35,6 +35,7 @@ type Config struct {
 	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"`

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)