Allow override of domains and whitelist in rules (#169)

Co-authored-by: Mathieu Cantin <mcantin@petalmd.com>
Co-authored-by: Pete Shaw <lozlow@users.noreply.github.com>

README.md

@@ -321,6 +321,7 @@ All options can be supplied in any of the following ways, in the following prece
        - `action` - same usage as [`default-action`](#default-action), supported values:
            - `auth` (default)
            - `allow`
+       - `domains` - optional, same usage as [`domain`](#domain)
        - `provider` - same usage as [`default-provider`](#default-provider), supported values:
            - `google`
            - `oidc`
@@ -333,6 +334,7 @@ All options can be supplied in any of the following ways, in the following prece
            - ``Path(`path`, `/articles/{category}/{id:[0-9]+}`, ...)``
            - ``PathPrefix(`/products/`, `/articles/{category}/{id:[0-9]+}`)``
            - ``Query(`foo=bar`, `bar=baz`)``
+       - `whitelist` - optional, same usage as whitelist`](#whitelist)
 
    For example:
    ```
@@ -348,6 +350,11 @@ All options can be supplied in any of the following ways, in the following prece
    rule.oidc.action = auth
    rule.oidc.provider = oidc
    rule.oidc.rule = PathPrefix(`/github`)
+
+   # Allow jane@example.com to `/janes-eyes-only`
+   rule.two.action = allow
+   rule.two.rule = Path(`/janes-eyes-only`)
+   rule.two.whitelist = jane@example.com
    ```
 
    Note: It is possible to break your redirect flow with rules, please be careful not to create an `allow` rule that matches your redirect_uri unless you know what you're doing. This limitation is being tracked in in #101 and the behaviour will change in future releases.
@@ -361,7 +368,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 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).
+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). If you set `domains` or `whitelist` on a rule, the global configuration is ignored.
 
 ### Forwarded Headers
 

internal/auth.go

@@ -59,18 +59,28 @@ func ValidateCookie(r *http.Request, c *http.Cookie) (string, error) {
 // 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 {
+func ValidateEmail(email, ruleName string) bool {
+	// Use global config by default
+	whitelist := config.Whitelist
+	domains := config.Domains
+
+	if rule, ok := config.Rules[ruleName]; ok {
+		// Override with rule config if found
+		if len(rule.Whitelist) > 0 || len(rule.Domains) > 0 {
+			whitelist = rule.Whitelist
+			domains = rule.Domains
+		}
+	}
+
 	// Do we have any validation to perform?
-	if len(config.Whitelist) == 0 && len(config.Domains) == 0 {
+	if len(whitelist) == 0 && len(domains) == 0 {
 		return true
 	}
 
 	// Email whitelist validation
-	if len(config.Whitelist) > 0 {
-		for _, whitelist := range config.Whitelist {
-			if email == whitelist {
-				return true
-			}
+	if len(whitelist) > 0 {
+		if ValidateWhitelist(email, whitelist) {
+			return true
 		}
 
 		// If we're not matching *either*, stop here
@@ -80,18 +90,34 @@ func ValidateEmail(email string) bool {
 	}
 
 	// 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] {
-				return true
-			}
+	if len(domains) > 0 && ValidateDomains(email, domains) {
+		return true
+	}
+
+	return false
+}
+
+// ValidateWhitelist checks if the email is in whitelist
+func ValidateWhitelist(email string, whitelist CommaSeparatedList) bool {
+	for _, whitelist := range whitelist {
+		if email == whitelist {
+			return true
 		}
 	}
+	return false
+}
 
+// ValidateDomains checks if the email matches a whitelisted domain
+func ValidateDomains(email string, domains CommaSeparatedList) bool {
+	parts := strings.Split(email, "@")
+	if len(parts) < 2 {
+		return false
+	}
+	for _, domain := range domains {
+		if domain == parts[1] {
+			return true
+		}
+	}
 	return false
 }
 

internal/auth_test.go

@@ -65,57 +65,132 @@ func TestAuthValidateEmail(t *testing.T) {
 	assert := assert.New(t)
 	config, _ = NewConfig([]string{})
 
-	// Should allow any
-	v := ValidateEmail("test@test.com")
+	// Should allow any with no whitelist/domain is specified
+	v := ValidateEmail("test@test.com", "default")
 	assert.True(v, "should allow any domain if email domain is not defined")
-	v = ValidateEmail("one@two.com")
+	v = ValidateEmail("one@two.com", "default")
 	assert.True(v, "should allow any domain if email domain is not defined")
 
-	// Should block non matching domain
-	config.Domains = []string{"test.com"}
-	v = ValidateEmail("one@two.com")
-	assert.False(v, "should not allow user from another domain")
-
 	// Should allow matching domain
 	config.Domains = []string{"test.com"}
-	v = ValidateEmail("test@test.com")
+	v = ValidateEmail("one@two.com", "default")
+	assert.False(v, "should not allow user from another domain")
+	v = ValidateEmail("test@test.com", "default")
 	assert.True(v, "should allow user from allowed domain")
 
-	// Should block non whitelisted email address
-	config.Domains = []string{}
-	config.Whitelist = []string{"test@test.com"}
-	v = ValidateEmail("one@two.com")
-	assert.False(v, "should not allow user not in whitelist")
-
 	// Should allow matching whitelisted email address
 	config.Domains = []string{}
 	config.Whitelist = []string{"test@test.com"}
-	v = ValidateEmail("test@test.com")
+	v = ValidateEmail("one@two.com", "default")
+	assert.False(v, "should not allow user not in whitelist")
+	v = ValidateEmail("test@test.com", "default")
 	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")
+	v = ValidateEmail("one@two.com", "default")
 	assert.False(v, "should not allow user not in either")
+	v = ValidateEmail("test@example.com", "default")
+	assert.False(v, "should not allow user from allowed domain")
+	v = ValidateEmail("test@test.com", "default")
+	assert.True(v, "should allow user in whitelist")
 
 	// 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")
+	v = ValidateEmail("one@two.com", "default")
+	assert.False(v, "should not allow user not in either")
+	v = ValidateEmail("test@example.com", "default")
+	assert.True(v, "should allow user from allowed domain")
+	v = ValidateEmail("test@test.com", "default")
 	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")
+
+	// Rule testing
+
+	// Should use global whitelist/domain when not specified on rule
+	config.Domains = []string{"example.com"}
+	config.Whitelist = []string{"test@test.com"}
+	config.Rules = map[string]*Rule{"test": NewRule()}
+	config.MatchWhitelistOrDomain = true
+	v = ValidateEmail("one@two.com", "test")
 	assert.False(v, "should not allow user not in either")
+	v = ValidateEmail("test@example.com", "test")
+	assert.True(v, "should allow user from allowed global domain")
+	v = ValidateEmail("test@test.com", "test")
+	assert.True(v, "should allow user in global whitelist")
+
+	// Should allow matching domain in rule
+	config.Domains = []string{"testglobal.com"}
+	config.Whitelist = []string{}
+	rule := NewRule()
+	config.Rules = map[string]*Rule{"test": rule}
+	rule.Domains = []string{"testrule.com"}
+	config.MatchWhitelistOrDomain = false
+	v = ValidateEmail("one@two.com", "test")
+	assert.False(v, "should not allow user from another domain")
+	v = ValidateEmail("one@testglobal.com", "test")
+	assert.False(v, "should not allow user from global domain")
+	v = ValidateEmail("test@testrule.com", "test")
+	assert.True(v, "should allow user from allowed domain")
+
+	// Should allow matching whitelist in rule
+	config.Domains = []string{}
+	config.Whitelist = []string{"test@testglobal.com"}
+	rule = NewRule()
+	config.Rules = map[string]*Rule{"test": rule}
+	rule.Whitelist = []string{"test@testrule.com"}
+	config.MatchWhitelistOrDomain = false
+	v = ValidateEmail("one@two.com", "test")
+	assert.False(v, "should not allow user from another domain")
+	v = ValidateEmail("test@testglobal.com", "test")
+	assert.False(v, "should not allow user from global domain")
+	v = ValidateEmail("test@testrule.com", "test")
+	assert.True(v, "should allow user from allowed domain")
+
+	// Should allow only matching email address when
+	// MatchWhitelistOrDomain is disabled
+	config.Domains = []string{"exampleglobal.com"}
+	config.Whitelist = []string{"test@testglobal.com"}
+	rule = NewRule()
+	config.Rules = map[string]*Rule{"test": rule}
+	rule.Domains = []string{"examplerule.com"}
+	rule.Whitelist = []string{"test@testrule.com"}
+	config.MatchWhitelistOrDomain = false
+	v = ValidateEmail("one@two.com", "test")
+	assert.False(v, "should not allow user not in either")
+	v = ValidateEmail("test@testglobal.com", "test")
+	assert.False(v, "should not allow user in global whitelist")
+	v = ValidateEmail("test@exampleglobal.com", "test")
+	assert.False(v, "should not allow user from global domain")
+	v = ValidateEmail("test@examplerule.com", "test")
+	assert.False(v, "should not allow user from allowed domain")
+	v = ValidateEmail("test@testrule.com", "test")
+	assert.True(v, "should allow user in whitelist")
+
+	// Should allow either matching domain or email address when
+	// MatchWhitelistOrDomain is enabled
+	config.Domains = []string{"exampleglobal.com"}
+	config.Whitelist = []string{"test@testglobal.com"}
+	rule = NewRule()
+	config.Rules = map[string]*Rule{"test": rule}
+	rule.Domains = []string{"examplerule.com"}
+	rule.Whitelist = []string{"test@testrule.com"}
+	config.MatchWhitelistOrDomain = true
+	v = ValidateEmail("one@two.com", "test")
+	assert.False(v, "should not allow user not in either")
+	v = ValidateEmail("test@testglobal.com", "test")
+	assert.False(v, "should not allow user in global whitelist")
+	v = ValidateEmail("test@exampleglobal.com", "test")
+	assert.False(v, "should not allow user from global domain")
+	v = ValidateEmail("test@examplerule.com", "test")
+	assert.True(v, "should allow user from allowed domain")
+	v = ValidateEmail("test@testrule.com", "test")
+	assert.True(v, "should allow user in whitelist")
 }
 
 func TestRedirectUri(t *testing.T) {

internal/config.go

@@ -210,6 +210,14 @@ func (c *Config) parseUnknownFlag(option string, arg flags.SplitArgument, args [
 			rule.Rule = val
 		case "provider":
 			rule.Provider = val
+		case "whitelist":
+			list := CommaSeparatedList{}
+			list.UnmarshalFlag(val)
+			rule.Whitelist = list
+		case "domains":
+			list := CommaSeparatedList{}
+			list.UnmarshalFlag(val)
+			rule.Domains = list
 		default:
 			return args, fmt.Errorf("invalid route param: %v", option)
 		}
@@ -327,9 +335,11 @@ func (c *Config) setupProvider(name string) error {
 
 // Rule holds defined rules
 type Rule struct {
-	Action   string
-	Rule     string
-	Provider string
+	Action    string
+	Rule      string
+	Provider  string
+	Whitelist CommaSeparatedList
+	Domains   CommaSeparatedList
 }
 
 // NewRule creates a new rule object

internal/server.go

@@ -101,7 +101,7 @@ func (s *Server) AuthHandler(providerName, rule string) http.HandlerFunc {
 		}
 
 		// Validate user
-		valid := ValidateEmail(email)
+		valid := ValidateEmail(email, rule)
 		if !valid {
 			logger.WithField("email", email).Warn("Invalid email")
 			http.Error(w, "Not authorized", 401)