From c52e24e6b19374a6933735217eeba38d54304f6a Mon Sep 17 00:00:00 2001 From: Omar Alashqar Date: Tue, 13 Oct 2020 17:37:55 -0400 Subject: [PATCH 1/7] case-insensitive emails whiteList --- .defaults.yml | 2 + ...handler_case_insensitive_email_domains.yml | 23 ++++++ .../handler_case_insensitive_emails.yml | 19 +++++ handlers/auth.go | 31 +++++++- handlers/handlers_test.go | 70 +++++++++++++++++++ pkg/cfg/cfg.go | 25 ++++--- 6 files changed, 160 insertions(+), 10 deletions(-) create mode 100644 config/testing/handler_case_insensitive_email_domains.yml create mode 100644 config/testing/handler_case_insensitive_emails.yml diff --git a/.defaults.yml b/.defaults.yml index 5355712d..daad3ec4 100644 --- a/.defaults.yml +++ b/.defaults.yml @@ -10,6 +10,8 @@ vouch: listen: 0.0.0.0 port: 9090 # domains: + case_insensitive_emails: false + # case_insensitive_email_domains: allowAllUsers: false publicAccess: false # whiteList: diff --git a/config/testing/handler_case_insensitive_email_domains.yml b/config/testing/handler_case_insensitive_email_domains.yml new file mode 100644 index 00000000..9cee56a7 --- /dev/null +++ b/config/testing/handler_case_insensitive_email_domains.yml @@ -0,0 +1,23 @@ +vouch: + logLevel: debug + + case_insensitive_emails: false + + case_insensitive_email_domains: + - example1.com + + whiteList: + # these should work if cases do not match with incoming email + - test@example1.com + - test@sub.example1.com + # these should not work if cases do not match with incoming email + - test@example2.com + + jwt: + secret: testingsecret + +oauth: + provider: indieauth + client_id: http://vouch.github.io + auth_url: https://indielogin.com/auth + callback_url: http://vouch.github.io:9090/auth diff --git a/config/testing/handler_case_insensitive_emails.yml b/config/testing/handler_case_insensitive_emails.yml new file mode 100644 index 00000000..b800d5db --- /dev/null +++ b/config/testing/handler_case_insensitive_emails.yml @@ -0,0 +1,19 @@ +vouch: + logLevel: debug + + case_insensitive_emails: true + + whiteList: + # these should work if cases do not match with incoming email + - test@example.com + - test@sub.example.com + - test_username + + jwt: + secret: testingsecret + +oauth: + provider: indieauth + client_id: http://vouch.github.io + auth_url: https://indielogin.com/auth + callback_url: http://vouch.github.io:9090/auth diff --git a/handlers/auth.go b/handlers/auth.go index 72668828..242bae21 100644 --- a/handlers/auth.go +++ b/handlers/auth.go @@ -14,6 +14,7 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/vouch/vouch-proxy/pkg/cfg" "github.com/vouch/vouch-proxy/pkg/cookie" @@ -105,6 +106,17 @@ func verifyUser(u interface{}) (bool, error) { user := u.(structs.User) + /* + * these steps happen in the WhiteList case (2nd case) + * + * 1. check if were verifying email or username + * 1a. if username, skip to old behaviour + * 1b. if email, continue to next step + * 2. get domain from the email passed in + * 3. match it with whitelist (lowercase both if insensitivity is enabled) + * 3a. if matched, then verify case insensitively, else verify case sensitively + */ + switch { // AllowAllUsers @@ -114,8 +126,25 @@ func verifyUser(u interface{}) (bool, error) { // WhiteList case len(cfg.Cfg.WhiteList) != 0: + // If the username is from a case insensitive domain then we should perform case insensitive checks on the whitelist + caseInsensitiveEmails := cfg.Cfg.CaseInsensitiveEmails + + if !caseInsensitiveEmails { + for _, caseInsensitiveDomain := range cfg.Cfg.CaseInsensitiveEmailDomains { + // Guarantees that + // 1) the username is an email + // 2) the username should be treated case-insensitively + if strings.HasSuffix(strings.ToLower(user.Username), "@"+strings.ToLower(caseInsensitiveDomain)) { + caseInsensitiveEmails = true + } + } + } + + // TODO: parse whiteList to make this more strict + isEmail := strings.Contains(user.Username, "@") + for _, wl := range cfg.Cfg.WhiteList { - if user.Username == wl { + if user.Username == wl || (isEmail && caseInsensitiveEmails && strings.ToLower(user.Username) == strings.ToLower(wl)) { log.Debugf("verifyUser: Success! found user.Username in WhiteList: %s", user.Username) return true, nil } diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go index 6a968940..3a61ecd8 100644 --- a/handlers/handlers_test.go +++ b/handlers/handlers_test.go @@ -111,6 +111,76 @@ func TestVerifyUserNegative(t *testing.T) { assert.False(t, ok) assert.NotNil(t, err) } +func TestVerifyUserPositiveCaseInsensitiveEmailDomains(t *testing.T) { + setUp("/config/testing/handler_case_insensitive_email_domains.yml") + users := []*(structs.User){ + // domain having mixed case + &structs.User{Username: "test@example1.com", Email: "something@else.com", Name: "Test Name"}, + &structs.User{Username: "test@EXAMPLE1.com", Email: "something@else.com", Name: "Test Name"}, + // local being mixed case + &structs.User{Username: "test@example1.com", Email: "something@else.com", Name: "Test Name"}, + &structs.User{Username: "TEST@example1.com", Email: "something@else.com", Name: "Test Name"}, + // exists in whitelist, control test + &structs.User{Username: "test@example2.com", Email: "something@else.com", Name: "Test Name"}, + } + for _, user := range users { + ok, err := verifyUser(*user) + assert.True(t, ok) + assert.Nil(t, err) + } +} + +func TestVerifyUserNegativeCaseInsensitiveEmailDomains(t *testing.T) { + setUp("/config/testing/handler_case_insensitive_email_domains.yml") + users := []*structs.User{ + // case sensitive domain having mixed case + &structs.User{Username: "TEST@example2.com", Email: "something@else.com", Name: "Test Name"}, + &structs.User{Username: "TEST@EXAMPLE2.com", Email: "something@else.com", Name: "Test Name"}, + // sub domain should not be affected by domain + &structs.User{Username: "TEST@sub.example1.com", Email: "something@else.com", Name: "Test Name"}, + } + for _, user := range users { + ok, err := verifyUser(*user) + assert.False(t, ok) + assert.NotNil(t, err) + } +} + +func TestVerifyUserPositiveCaseInsensitiveEmails(t *testing.T) { + setUp("/config/testing/handler_case_insensitive_emails.yml") + + // caps + user := &structs.User{Username: "TEST@example.com", Email: "something@else.com", Name: "Test Name"} + ok, err := verifyUser(*user) + + assert.True(t, ok) + assert.Nil(t, err) + + // caps and subdomain + user = &structs.User{Username: "TEST@sub.example.com", Email: "something@else.com", Name: "Test Name"} + ok, err = verifyUser(*user) + + assert.True(t, ok) + assert.Nil(t, err) + + // domain is mixed case + user = &structs.User{Username: "TEST@sub.EXAMPLE.com", Email: "something@else.com", Name: "Test Name"} + ok, err = verifyUser(*user) + + assert.True(t, ok) + assert.Nil(t, err) +} + +func TestVerifyUserNegativeCaseInsensitiveEmails(t *testing.T) { + setUp("/config/testing/handler_case_insensitive_emails.yml") + + // ensures username is still case sensitive + user := &structs.User{Username: "TEST_USERNAME", Email: "something@else.com", Name: "Test Name"} + ok, err := verifyUser(*user) + + assert.False(t, ok) + assert.NotNil(t, err) +} // copied from jwtmanager_test.go // it should live there but circular imports are resolved if it lives here diff --git a/pkg/cfg/cfg.go b/pkg/cfg/cfg.go index 3e8def28..936e996e 100644 --- a/pkg/cfg/cfg.go +++ b/pkg/cfg/cfg.go @@ -35,15 +35,17 @@ import ( // as well as a `envconfig` tag used by https://github.com/kelseyhightower/envconfig // though most of the time envconfig will use the struct key's name: VOUCH_PORT VOUCH_JWT_MAXAGE type Config struct { - LogLevel string `mapstructure:"logLevel"` - Listen string `mapstructure:"listen"` - Port int `mapstructure:"port"` - Domains []string `mapstructure:"domains"` - WhiteList []string `mapstructure:"whitelist"` - TeamWhiteList []string `mapstructure:"teamWhitelist"` - AllowAllUsers bool `mapstructure:"allowAllUsers"` - PublicAccess bool `mapstructure:"publicAccess"` - JWT struct { + LogLevel string `mapstructure:"logLevel"` + Listen string `mapstructure:"listen"` + Port int `mapstructure:"port"` + Domains []string `mapstructure:"domains"` + CaseInsensitiveEmails bool `mapstructure:"case_insensitive_emails"` + CaseInsensitiveEmailDomains []string `mapstructure:"case_insensitive_email_domains"` + WhiteList []string `mapstructure:"whitelist"` + TeamWhiteList []string `mapstructure:"teamWhitelist"` + AllowAllUsers bool `mapstructure:"allowAllUsers"` + PublicAccess bool `mapstructure:"publicAccess"` + JWT struct { MaxAge int `mapstructure:"maxAge"` // in minutes Issuer string `mapstructure:"issuer"` Secret string `mapstructure:"secret"` @@ -372,6 +374,11 @@ func basicTest() error { return fmt.Errorf("configuration error: either one of %s or %s needs to be set (but not both)", Branding.LCName+".domains", Branding.LCName+".allowAllUsers") } + // Email case insensitive list should be empty if the global case insensitivity flag is set + if Cfg.CaseInsensitiveEmails && len(Cfg.CaseInsensitiveEmailDomains) > 0 { + return fmt.Errorf("configuration error: either one of %s or %s needs to be set (but not both)", Branding.LCName+".case_insensitive_emails", Branding.LCName+".case_insensitive_email_domains") + } + // issue a warning if the secret is too small log.Debugf("vouch.jwt.secret is %d characters long", len(Cfg.JWT.Secret)) if len(Cfg.JWT.Secret) < minBase64Length { From a59bd2ca2cfc6836d2c19670f44bdf1dce22a3d5 Mon Sep 17 00:00:00 2001 From: Darius Asri Rezaei Date: Wed, 14 Oct 2020 12:12:22 -0400 Subject: [PATCH 2/7] add: email verification, table tests --- handlers/auth.go | 67 +++++++++++++++++++++------------------ handlers/handlers_test.go | 58 ++++++++++++++++----------------- 2 files changed, 65 insertions(+), 60 deletions(-) diff --git a/handlers/auth.go b/handlers/auth.go index 242bae21..f5243cd8 100644 --- a/handlers/auth.go +++ b/handlers/auth.go @@ -14,6 +14,7 @@ import ( "errors" "fmt" "net/http" + "regexp" "strings" "github.com/vouch/vouch-proxy/pkg/cfg" @@ -30,6 +31,17 @@ var ( errURLNotFound = errors.New("/auth could not retrieve URL from session") ) +// TODO: check go packages for this feature +// From https://golangcode.com/validate-an-email-address/ +var emailRegex = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") +// isEmailValid checks if the email provided passes the required structure and length. +func isEmailValid(e string) bool { + if len(e) < 3 && len(e) > 254 { + return false + } + return emailRegex.MatchString(e) +} + // CallbackHandler /auth // - validate info from oauth provider (Google, GitHub, OIDC, etc) // - issue jwt in the form of a cookie @@ -101,22 +113,29 @@ func CallbackHandler(w http.ResponseWriter, r *http.Request) { responses.RenderIndex(w, "/auth "+tokenstring) } +func checkIfCaseInsensitive(user *structs.User) bool { + if cfg.Cfg.CaseInsensitiveEmails { + return true + } + + lowerUsername := strings.ToLower(user.Username) + for _, caseInsensitiveDomain := range cfg.Cfg.CaseInsensitiveEmailDomains { + // Guarantees that + // 1) the username is an email + // 2) the username should be treated case-insensitively + if strings.HasSuffix(lowerUsername, "@"+strings.ToLower(caseInsensitiveDomain)) { + return true + } + } + + return false +} + // verifyUser validates that the domains match for the user func verifyUser(u interface{}) (bool, error) { - + user := u.(structs.User) - - /* - * these steps happen in the WhiteList case (2nd case) - * - * 1. check if were verifying email or username - * 1a. if username, skip to old behaviour - * 1b. if email, continue to next step - * 2. get domain from the email passed in - * 3. match it with whitelist (lowercase both if insensitivity is enabled) - * 3a. if matched, then verify case insensitively, else verify case sensitively - */ - + switch { // AllowAllUsers @@ -127,24 +146,12 @@ func verifyUser(u interface{}) (bool, error) { // WhiteList case len(cfg.Cfg.WhiteList) != 0: // If the username is from a case insensitive domain then we should perform case insensitive checks on the whitelist - caseInsensitiveEmails := cfg.Cfg.CaseInsensitiveEmails - - if !caseInsensitiveEmails { - for _, caseInsensitiveDomain := range cfg.Cfg.CaseInsensitiveEmailDomains { - // Guarantees that - // 1) the username is an email - // 2) the username should be treated case-insensitively - if strings.HasSuffix(strings.ToLower(user.Username), "@"+strings.ToLower(caseInsensitiveDomain)) { - caseInsensitiveEmails = true - } - } - } - - // TODO: parse whiteList to make this more strict - isEmail := strings.Contains(user.Username, "@") - + caseInsensitiveEmail := checkIfCaseInsensitive(&user) + for _, wl := range cfg.Cfg.WhiteList { - if user.Username == wl || (isEmail && caseInsensitiveEmails && strings.ToLower(user.Username) == strings.ToLower(wl)) { + // Case sensitivity should only apply to email-based usernames + // if user.Username == wl || (user.Username == user.Email) && caseInsensitiveEmail && strings.ToLower(user.Username) == strings.ToLower(wl)) { + if user.Username == wl || (isEmailValid(user.Username) && caseInsensitiveEmail && strings.ToLower(user.Username) == strings.ToLower(wl)) { log.Debugf("verifyUser: Success! found user.Username in WhiteList: %s", user.Username) return true, nil } diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go index 3a61ecd8..0242f15d 100644 --- a/handlers/handlers_test.go +++ b/handlers/handlers_test.go @@ -113,16 +113,13 @@ func TestVerifyUserNegative(t *testing.T) { } func TestVerifyUserPositiveCaseInsensitiveEmailDomains(t *testing.T) { setUp("/config/testing/handler_case_insensitive_email_domains.yml") - users := []*(structs.User){ + users := []*structs.User{ // domain having mixed case - &structs.User{Username: "test@example1.com", Email: "something@else.com", Name: "Test Name"}, &structs.User{Username: "test@EXAMPLE1.com", Email: "something@else.com", Name: "Test Name"}, // local being mixed case - &structs.User{Username: "test@example1.com", Email: "something@else.com", Name: "Test Name"}, &structs.User{Username: "TEST@example1.com", Email: "something@else.com", Name: "Test Name"}, - // exists in whitelist, control test - &structs.User{Username: "test@example2.com", Email: "something@else.com", Name: "Test Name"}, } + for _, user := range users { ok, err := verifyUser(*user) assert.True(t, ok) @@ -139,6 +136,7 @@ func TestVerifyUserNegativeCaseInsensitiveEmailDomains(t *testing.T) { // sub domain should not be affected by domain &structs.User{Username: "TEST@sub.example1.com", Email: "something@else.com", Name: "Test Name"}, } + for _, user := range users { ok, err := verifyUser(*user) assert.False(t, ok) @@ -146,40 +144,40 @@ func TestVerifyUserNegativeCaseInsensitiveEmailDomains(t *testing.T) { } } -func TestVerifyUserPositiveCaseInsensitiveEmails(t *testing.T) { +func TestVerifyUserPositiveCaseInsensitiveEmails(t *testing.T) { setUp("/config/testing/handler_case_insensitive_emails.yml") - // caps - user := &structs.User{Username: "TEST@example.com", Email: "something@else.com", Name: "Test Name"} - ok, err := verifyUser(*user) - - assert.True(t, ok) - assert.Nil(t, err) - - // caps and subdomain - user = &structs.User{Username: "TEST@sub.example.com", Email: "something@else.com", Name: "Test Name"} - ok, err = verifyUser(*user) - - assert.True(t, ok) - assert.Nil(t, err) - - // domain is mixed case - user = &structs.User{Username: "TEST@sub.EXAMPLE.com", Email: "something@else.com", Name: "Test Name"} - ok, err = verifyUser(*user) - - assert.True(t, ok) - assert.Nil(t, err) + tests := []struct { + name string + user *structs.User + } { + { name: "email is caps", user: &structs.User{Username: "TEST@example.com", Email: "something@else.com", Name: "Test Name" } }, + { name: "email is caps with subdomain", user: &structs.User{Username: "TEST@sub.example.com", Email: "something@else.com", Name: "Test Name" } }, + { name: "email is caps with mixed submain", user: &structs.User{Username: "TEST@sub.EXAMPLE.com", Email: "something@else.com", Name: "Test Name" } }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ok, err := verifyUser(*tt.user) + assert.True(t, ok, "Expected user to be verified") + assert.Nil(t, err, "Expected error to be nil") + }) + } } func TestVerifyUserNegativeCaseInsensitiveEmails(t *testing.T) { setUp("/config/testing/handler_case_insensitive_emails.yml") // ensures username is still case sensitive - user := &structs.User{Username: "TEST_USERNAME", Email: "something@else.com", Name: "Test Name"} - ok, err := verifyUser(*user) + users := []*structs.User{ + &structs.User{Username: "TEST_USERNAME", Email: "something@else.com", Name: "Test Name"}, + } - assert.False(t, ok) - assert.NotNil(t, err) + for _, user := range users { + ok, err := verifyUser(*user) + assert.False(t, ok) + assert.NotNil(t, err) + } } // copied from jwtmanager_test.go From aea63c95c92a4255d19e2eb3bc7d4aca386397ff Mon Sep 17 00:00:00 2001 From: Darius Asri Rezaei Date: Wed, 14 Oct 2020 13:38:30 -0400 Subject: [PATCH 3/7] fix: logic --- handlers/auth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handlers/auth.go b/handlers/auth.go index f5243cd8..a37e33fe 100644 --- a/handlers/auth.go +++ b/handlers/auth.go @@ -36,7 +36,7 @@ var ( var emailRegex = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") // isEmailValid checks if the email provided passes the required structure and length. func isEmailValid(e string) bool { - if len(e) < 3 && len(e) > 254 { + if len(e) < 3 || len(e) > 254 { return false } return emailRegex.MatchString(e) From 12160200b14c9a1835c2de998fef497d0062eedd Mon Sep 17 00:00:00 2001 From: Darius Asri Rezaei Date: Wed, 14 Oct 2020 14:26:09 -0400 Subject: [PATCH 4/7] add: table driven tests for more test cases --- handlers/handlers_test.go | 78 +++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go index 0242f15d..cf866ae3 100644 --- a/handlers/handlers_test.go +++ b/handlers/handlers_test.go @@ -113,70 +113,78 @@ func TestVerifyUserNegative(t *testing.T) { } func TestVerifyUserPositiveCaseInsensitiveEmailDomains(t *testing.T) { setUp("/config/testing/handler_case_insensitive_email_domains.yml") - users := []*structs.User{ - // domain having mixed case - &structs.User{Username: "test@EXAMPLE1.com", Email: "something@else.com", Name: "Test Name"}, - // local being mixed case - &structs.User{Username: "TEST@example1.com", Email: "something@else.com", Name: "Test Name"}, + tests := []struct { + name string + user *structs.User + }{ + {name: "case insensitive domain having caps domain", user: &structs.User{Username: "test@EXAMPLE1.com", Email: "something@else.com", Name: "Test Name"}}, + {name: "case insensitive subdomain having caps email", user: &structs.User{Username: "TEST@example1.com", Email: "something@else.com", Name: "Test Name"}}, } - for _, user := range users { - ok, err := verifyUser(*user) - assert.True(t, ok) - assert.Nil(t, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ok, err := verifyUser(*tt.user) + assert.True(t, ok) + assert.Nil(t, err) + }) } } func TestVerifyUserNegativeCaseInsensitiveEmailDomains(t *testing.T) { setUp("/config/testing/handler_case_insensitive_email_domains.yml") - users := []*structs.User{ - // case sensitive domain having mixed case - &structs.User{Username: "TEST@example2.com", Email: "something@else.com", Name: "Test Name"}, - &structs.User{Username: "TEST@EXAMPLE2.com", Email: "something@else.com", Name: "Test Name"}, - // sub domain should not be affected by domain - &structs.User{Username: "TEST@sub.example1.com", Email: "something@else.com", Name: "Test Name"}, + tests := []struct { + name string + user *structs.User + }{ + {name: "case sensitive domain having mixed case", user: &structs.User{Username: "TEST@example2.com", Email: "something@else.com", Name: "Test Name"}}, + {name: "case sensitive subdomain having mixed case", user: &structs.User{Username: "TEST@sub.example1.com", Email: "something@else.com", Name: "Test Name"}}, } - for _, user := range users { - ok, err := verifyUser(*user) - assert.False(t, ok) - assert.NotNil(t, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ok, err := verifyUser(*tt.user) + assert.False(t, ok) + assert.NotNil(t, err) + }) } } -func TestVerifyUserPositiveCaseInsensitiveEmails(t *testing.T) { +func TestVerifyUserPositiveCaseInsensitiveEmails(t *testing.T) { setUp("/config/testing/handler_case_insensitive_emails.yml") tests := []struct { name string user *structs.User - } { - { name: "email is caps", user: &structs.User{Username: "TEST@example.com", Email: "something@else.com", Name: "Test Name" } }, - { name: "email is caps with subdomain", user: &structs.User{Username: "TEST@sub.example.com", Email: "something@else.com", Name: "Test Name" } }, - { name: "email is caps with mixed submain", user: &structs.User{Username: "TEST@sub.EXAMPLE.com", Email: "something@else.com", Name: "Test Name" } }, + }{ + {name: "email is caps", user: &structs.User{Username: "TEST@example.com", Email: "something@else.com", Name: "Test Name"}}, + {name: "email is caps with subdomain", user: &structs.User{Username: "TEST@sub.example.com", Email: "something@else.com", Name: "Test Name"}}, + {name: "email is caps with mixed submain", user: &structs.User{Username: "TEST@sub.EXAMPLE.com", Email: "something@else.com", Name: "Test Name"}}, } - + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ok, err := verifyUser(*tt.user) - assert.True(t, ok, "Expected user to be verified") - assert.Nil(t, err, "Expected error to be nil") + assert.True(t, ok) + assert.Nil(t, err) }) } } func TestVerifyUserNegativeCaseInsensitiveEmails(t *testing.T) { setUp("/config/testing/handler_case_insensitive_emails.yml") - - // ensures username is still case sensitive - users := []*structs.User{ - &structs.User{Username: "TEST_USERNAME", Email: "something@else.com", Name: "Test Name"}, + tests := []struct { + name string + user *structs.User + }{ + {name: "non-email username is caps", user: &structs.User{Username: "TEST_USERNAME", Email: "something@else.com", Name: "Test Name"}}, } - for _, user := range users { - ok, err := verifyUser(*user) - assert.False(t, ok) - assert.NotNil(t, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ok, err := verifyUser(*tt.user) + assert.False(t, ok) + assert.NotNil(t, err) + }) } } From 2cbf7e515c1b9e93bfe6112d9f38d8b44323caba Mon Sep 17 00:00:00 2001 From: Darius Asri Rezaei Date: Wed, 14 Oct 2020 15:36:04 -0400 Subject: [PATCH 5/7] fix: variable names --- handlers/auth.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/handlers/auth.go b/handlers/auth.go index a37e33fe..155fad60 100644 --- a/handlers/auth.go +++ b/handlers/auth.go @@ -34,6 +34,7 @@ var ( // TODO: check go packages for this feature // From https://golangcode.com/validate-an-email-address/ var emailRegex = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") + // isEmailValid checks if the email provided passes the required structure and length. func isEmailValid(e string) bool { if len(e) < 3 || len(e) > 254 { @@ -113,11 +114,11 @@ func CallbackHandler(w http.ResponseWriter, r *http.Request) { responses.RenderIndex(w, "/auth "+tokenstring) } -func checkIfCaseInsensitive(user *structs.User) bool { +func isUsernameCaseInsensitive(user *structs.User) bool { if cfg.Cfg.CaseInsensitiveEmails { return true } - + lowerUsername := strings.ToLower(user.Username) for _, caseInsensitiveDomain := range cfg.Cfg.CaseInsensitiveEmailDomains { // Guarantees that @@ -133,9 +134,9 @@ func checkIfCaseInsensitive(user *structs.User) bool { // verifyUser validates that the domains match for the user func verifyUser(u interface{}) (bool, error) { - + user := u.(structs.User) - + switch { // AllowAllUsers @@ -146,12 +147,12 @@ func verifyUser(u interface{}) (bool, error) { // WhiteList case len(cfg.Cfg.WhiteList) != 0: // If the username is from a case insensitive domain then we should perform case insensitive checks on the whitelist - caseInsensitiveEmail := checkIfCaseInsensitive(&user) - + usernameIsCaseInsensitive := isUsernameCaseInsensitive(&user) + for _, wl := range cfg.Cfg.WhiteList { // Case sensitivity should only apply to email-based usernames // if user.Username == wl || (user.Username == user.Email) && caseInsensitiveEmail && strings.ToLower(user.Username) == strings.ToLower(wl)) { - if user.Username == wl || (isEmailValid(user.Username) && caseInsensitiveEmail && strings.ToLower(user.Username) == strings.ToLower(wl)) { + if user.Username == wl || (isEmailValid(user.Username) && usernameIsCaseInsensitive && strings.ToLower(user.Username) == strings.ToLower(wl)) { log.Debugf("verifyUser: Success! found user.Username in WhiteList: %s", user.Username) return true, nil } From 3b4138afb769e741e418cd28c6edf3d3bb2764a2 Mon Sep 17 00:00:00 2001 From: Darius Asri Rezaei Date: Mon, 26 Oct 2020 12:12:01 -0400 Subject: [PATCH 6/7] add: example config entries --- config/config.yml_example | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/config/config.yml_example b/config/config.yml_example index 95d7e5fb..42304058 100644 --- a/config/config.yml_example +++ b/config/config.yml_example @@ -32,6 +32,19 @@ vouch: - yourdomain.com - yourotherdomain.com + # case_insensitive_emails - VOUCH_CASE_INSENSITIVE_EMAILS + # Setting this to true will treat all email-based usernames returned by the + # service provider case insensitively when validating against the whitelist + case_insensitive_emails: false + + # case_insensitive_email_domains - VOUCH_CASE_INSENSITIVE_EMAIL_DOMAINS + # Any email-based username returned by the service provider that belongs to + # any of the following domains will be treated case insensitively when + # validating against the whitelist. Subdomains of a case insensitive domain + # will not be assumed case insensitive. + # Comment `case_insensitive_email_domains:` out if you set case_insensitive_emails:true + # case_insensitive_email_domains: + # Set allowAllUsers: true to use Vouch Proxy to just accept anyone who can authenticate at the configured provider - VOUCH_ALLOWALLUSERS # allowAllUsers: false # vouch.cookie.domain must be set below when enabling allowAllUsers From 59d01b409c659be92d1192c3a6df84ec2f28a977 Mon Sep 17 00:00:00 2001 From: Darius Asri Rezaei Date: Mon, 26 Oct 2020 13:30:20 -0400 Subject: [PATCH 7/7] add cfg test and cleanup code --- handlers/auth.go | 2 -- pkg/cfg/cfg.go | 4 ++-- pkg/cfg/cfg_test.go | 7 ++++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/handlers/auth.go b/handlers/auth.go index 155fad60..212bc6ff 100644 --- a/handlers/auth.go +++ b/handlers/auth.go @@ -31,7 +31,6 @@ var ( errURLNotFound = errors.New("/auth could not retrieve URL from session") ) -// TODO: check go packages for this feature // From https://golangcode.com/validate-an-email-address/ var emailRegex = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") @@ -151,7 +150,6 @@ func verifyUser(u interface{}) (bool, error) { for _, wl := range cfg.Cfg.WhiteList { // Case sensitivity should only apply to email-based usernames - // if user.Username == wl || (user.Username == user.Email) && caseInsensitiveEmail && strings.ToLower(user.Username) == strings.ToLower(wl)) { if user.Username == wl || (isEmailValid(user.Username) && usernameIsCaseInsensitive && strings.ToLower(user.Username) == strings.ToLower(wl)) { log.Debugf("verifyUser: Success! found user.Username in WhiteList: %s", user.Username) return true, nil diff --git a/pkg/cfg/cfg.go b/pkg/cfg/cfg.go index 936e996e..ecdaf32c 100644 --- a/pkg/cfg/cfg.go +++ b/pkg/cfg/cfg.go @@ -39,8 +39,8 @@ type Config struct { Listen string `mapstructure:"listen"` Port int `mapstructure:"port"` Domains []string `mapstructure:"domains"` - CaseInsensitiveEmails bool `mapstructure:"case_insensitive_emails"` - CaseInsensitiveEmailDomains []string `mapstructure:"case_insensitive_email_domains"` + CaseInsensitiveEmails bool `mapstructure:"case_insensitive_emails" envconfig:"case_insensitive_emails"` + CaseInsensitiveEmailDomains []string `mapstructure:"case_insensitive_email_domains" envconfig:"case_insensitive_email_domains"` WhiteList []string `mapstructure:"whitelist"` TeamWhiteList []string `mapstructure:"teamWhitelist"` AllowAllUsers bool `mapstructure:"allowAllUsers"` diff --git a/pkg/cfg/cfg_test.go b/pkg/cfg/cfg_test.go index d6dfb660..18eaac1b 100644 --- a/pkg/cfg/cfg_test.go +++ b/pkg/cfg/cfg_test.go @@ -105,12 +105,12 @@ func Test_configureFromEnvCfg(t *testing.T) { "VOUCH_HEADERS_CLAIMHEADER", "VOUCH_HEADERS_ACCESSTOKEN", "VOUCH_HEADERS_IDTOKEN", "VOUCH_COOKIE_NAME", "VOUCH_COOKIE_DOMAIN", "VOUCH_COOKIE_SAMESITE", "VOUCH_TESTURL", "VOUCH_SESSION_NAME", "VOUCH_SESSION_KEY"} // array of strings - saenv := []string{"VOUCH_DOMAINS", "VOUCH_WHITELIST", "VOUCH_TEAMWHITELIST", "VOUCH_HEADERS_CLAIMS", "VOUCH_TESTURLS", "VOUCH_POST_LOGOUT_REDIRECT_URIS"} + saenv := []string{"VOUCH_DOMAINS", "VOUCH_WHITELIST", "VOUCH_TEAMWHITELIST", "VOUCH_HEADERS_CLAIMS", "VOUCH_TESTURLS", "VOUCH_POST_LOGOUT_REDIRECT_URIS", "VOUCH_CASE_INSENSITIVE_EMAIL_DOMAINS"} // int ienv := []string{"VOUCH_PORT", "VOUCH_JWT_MAXAGE", "VOUCH_COOKIE_MAXAGE"} // bool benv := []string{"VOUCH_ALLOWALLUSERS", "VOUCH_PUBLICACCESS", "VOUCH_JWT_COMPRESS", "VOUCH_COOKIE_SECURE", - "VOUCH_COOKIE_HTTPONLY", "VOUCH_TESTING"} + "VOUCH_COOKIE_HTTPONLY", "VOUCH_TESTING", "VOUCH_CASE_INSENSITIVE_EMAILS"} // populate environmental variables svalue := "svalue" @@ -143,12 +143,13 @@ func Test_configureFromEnvCfg(t *testing.T) { Cfg.Cookie.SameSite, Cfg.TestURL, Cfg.Session.Name, Cfg.Session.Key, } - sacfg := [][]string{Cfg.Domains, Cfg.WhiteList, Cfg.TeamWhiteList, Cfg.Headers.Claims, Cfg.TestURLs, Cfg.LogoutRedirectURLs} + sacfg := [][]string{Cfg.Domains, Cfg.WhiteList, Cfg.TeamWhiteList, Cfg.Headers.Claims, Cfg.TestURLs, Cfg.LogoutRedirectURLs, Cfg.CaseInsensitiveEmailDomains,} icfg := []int{Cfg.Port, Cfg.JWT.MaxAge, Cfg.Cookie.MaxAge} bcfg := []bool{Cfg.AllowAllUsers, Cfg.PublicAccess, Cfg.JWT.Compress, Cfg.Cookie.Secure, Cfg.Cookie.HTTPOnly, Cfg.Testing, + Cfg.CaseInsensitiveEmails, } tests := []struct {