Simplify

1 files changed, 36 insertions, 34 deletions

diff --git a/server/auth/github/github.go b/server/auth/github/github.go
index 38009e1..a46876d 100644
--- a/server/auth/github/github.go
+++ b/server/auth/github/github.go
@@ -6,7 +6,6 @@ import (
 	"net/http"
 	"time"
 
-	"github.com/nsheridan/cashier/server/auth"
 	"github.com/nsheridan/cashier/server/config"
 	"github.com/nsheridan/cashier/server/metrics"
 
@@ -22,21 +21,23 @@ const (
 // Config is an implementation of `auth.Provider` for authenticating using a
 // Github account.
 type Config struct {
-	config       *oauth2.Config
-	organization string
-	whitelist    map[string]bool
+	config        *oauth2.Config
+	orgWhitelist  map[string]bool
+	userWhitelist map[string]bool
 }
 
-var _ auth.Provider = (*Config)(nil)
-
 // New creates a new Github provider from a configuration.
-func New(c *config.Auth) (*Config, error) {
+func New(c *config.Github) (*Config, error) {
 	uw := make(map[string]bool)
 	for _, u := range c.UsersWhitelist {
 		uw[u] = true
 	}
-	if c.ProviderOpts["organization"] == "" && len(uw) == 0 {
-		return nil, errors.New("either GitHub organization or users whitelist must be specified")
+	ow := make(map[string]bool)
+	for _, o := range c.OrgsWhitelist {
+		ow[o] = true
+	}
+	if len(uw) == 0 && len(ow) == 0 {
+		return nil, errors.New("either GitHub organizations or users whitelist must be specified")
 	}
 	return &Config{
 		config: &oauth2.Config{
@@ -49,8 +50,8 @@ func New(c *config.Auth) (*Config, error) {
 				string(githubapi.ScopeReadOrg),
 			},
 		},
-		organization: c.ProviderOpts["organization"],
-		whitelist:    uw,
+		orgWhitelist:  ow,
+		userWhitelist: uw,
 	}, nil
 }
 
@@ -66,34 +67,35 @@ 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)] {
-		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.
-		metrics.M.AuthValid.WithLabelValues("github").Inc()
+	if c.isUserWhitelisted(token) {
 		return true
 	}
-	client := githubapi.NewClient(c.newClient(token))
-	member, _, err := client.Organizations.IsMember(context.TODO(), c.organization, c.Username(token))
-	if err != nil {
-		return false
-	}
-	if member {
-		metrics.M.AuthValid.WithLabelValues("github").Inc()
+	if c.isMemberOfWhitelistedOrg(token) {
+		return true
 	}
-	return member
+	return false
+}
+
+func (c *Config) isUserWhitelisted(token *oauth2.Token) bool {
+	username := c.Username(token)
+	_, ok := c.userWhitelist[username]
+	return ok
 }
 
-// Revoke is a no-op revoke method. GitHub doesn't seem to allow token
-// revocation - tokens are indefinite and there are no refresh options etc.
-// Returns nil to satisfy the Provider interface.
-func (c *Config) Revoke(token *oauth2.Token) error {
-	return nil
+func (c *Config) isMemberOfWhitelistedOrg(token *oauth2.Token) bool {
+	client := githubapi.NewClient(c.newClient(token))
+	username := c.Username(token)
+	for org, _ := range c.orgWhitelist {
+		member, _, err := client.Organizations.IsMember(context.TODO(), org, username)
+		if err != nil {
+			return false
+		}
+		if member {
+			metrics.M.AuthValid.WithLabelValues("github").Inc()
+			return true
+		}
+	}
+	return false
 }
 
 // StartSession retrieves an authentication endpoint from Github.