Update whitelisting

Whitelist Google users based on their email address instead of the username part of the email address.
Plain gmail (non Google Apps) accounts don't necessarily end in '@gmail.com', and whitelisting on username alone is open to abuse.
Skip testing for a Google Apps domain (ui.Hd) if no domain is configured.
Principals will still be added as the user part of the email address.

For the Github provider, skip checking that the user is a member of an organization is none is configured.

2 files changed, 19 insertions, 6 deletions

diff --git a/server/auth/github/github.go b/server/auth/github/github.go
index 912caae..24a4bbf 100644
--- a/server/auth/github/github.go
+++ b/server/auth/github/github.go
@@ -62,12 +62,17 @@ func (c *Config) Name() string {
 
 // Valid validates the oauth token.
 func (c *Config) Valid(token *oauth2.Token) bool {
-	if len(c.whitelist) == 0 && !c.whitelist[c.Username(token)] {
+	if len(c.whitelist) > 0 && !c.whitelist[c.Username(token)] {
 		return false
 	}
 	if !token.Valid() {
 		return false
 	}
+	if c.organization == "" {
+		// There's no organization and the token is valid. Can only reach here
+		// if there's a user whitelist set and the user is in the whitelist.
+		return true
+	}
 	client := githubapi.NewClient(c.newClient(token))
 	member, _, err := client.Organizations.IsMember(c.organization, c.Username(token))
 	if err != nil {
diff --git a/server/auth/google/google.go b/server/auth/google/google.go
index 3a833ab..08a4083 100644
--- a/server/auth/google/google.go
+++ b/server/auth/google/google.go
@@ -62,7 +62,7 @@ func (c *Config) Name() string {
 
 // Valid validates the oauth token.
 func (c *Config) Valid(token *oauth2.Token) bool {
-	if len(c.whitelist) == 0 && !c.whitelist[c.Username(token)] {
+	if len(c.whitelist) > 0 && !c.whitelist[c.Email(token)] {
 		return false
 	}
 	if !token.Valid() {
@@ -78,11 +78,14 @@ func (c *Config) Valid(token *oauth2.Token) bool {
 	if err != nil {
 		return false
 	}
+	if ti.Audience != c.config.ClientID {
+		return false
+	}
 	ui, err := svc.Userinfo.Get().Do()
 	if err != nil {
 		return false
 	}
-	if ti.Audience != c.config.ClientID || ui.Hd != c.domain {
+	if c.domain != "" && ui.Hd != c.domain {
 		return false
 	}
 	return true
@@ -107,8 +110,8 @@ func (c *Config) Exchange(code string) (*oauth2.Token, error) {
 	return c.config.Exchange(oauth2.NoContext, code)
 }
 
-// Username retrieves the username portion of the user's email address.
-func (c *Config) Username(token *oauth2.Token) string {
+// Email retrieves the email address of the user.
+func (c *Config) Email(token *oauth2.Token) string {
 	svc, err := googleapi.New(c.newClient(token))
 	if err != nil {
 		return ""
@@ -117,5 +120,10 @@ func (c *Config) Username(token *oauth2.Token) string {
 	if err != nil {
 		return ""
 	}
-	return strings.Split(ui.Email, "@")[0]
+	return ui.Email
+}
+
+// Username retrieves the username portion of the user's email address.
+func (c *Config) Username(token *oauth2.Token) string {
+	return strings.Split(c.Email(token), "@")[0]
 }