Merge pull request #267 from buzzfeed/jusshersmith-group-validator-bug

sso_proxy: reduce amount of group validations

Author: Jusshersmith

internal/auth/providers/okta.go

@@ -199,7 +199,7 @@ func (p *OktaProvider) oktaRequest(method, endpoint string, params url.Values, t
 	if resp.StatusCode != http.StatusOK {
 		p.StatsdClient.Incr("provider.error", tags, 1.0)
 		logger.WithHTTPStatus(resp.StatusCode).WithEndpoint(stripToken(endpoint)).WithResponseBody(
-			respBody).Info()
+			respBody).Error("non-200 response returned from Okta")
 		switch resp.StatusCode {
 		case 400:
 			var response struct {

internal/pkg/options/email_domain_validator.go

@@ -9,7 +9,7 @@ import (
 )
 
 var (
-	_ Validator = &EmailDomainValidator{}
+	_ Validator = EmailDomainValidator{}
 
 	// These error message should be formatted in such a way that is appropriate
 	// for display to the end user.
@@ -28,7 +28,7 @@ type EmailDomainValidator struct {
 // - if the originally passed in list of domains consists only of "*", then all emails
 //   are considered valid based on their domain.
 // If valid, nil is returned in place of an error.
-func NewEmailDomainValidator(allowedDomains []string) *EmailDomainValidator {
+func NewEmailDomainValidator(allowedDomains []string) EmailDomainValidator {
 	emailDomains := make([]string, 0, len(allowedDomains))
 
 	for _, domain := range allowedDomains {
@@ -39,12 +39,12 @@ func NewEmailDomainValidator(allowedDomains []string) *EmailDomainValidator {
 			emailDomains = append(emailDomains, emailDomain)
 		}
 	}
-	return &EmailDomainValidator{
+	return EmailDomainValidator{
 		AllowedDomains: emailDomains,
 	}
 }
 
-func (v *EmailDomainValidator) Validate(session *sessions.SessionState) error {
+func (v EmailDomainValidator) Validate(session *sessions.SessionState) error {
 	if session.Email == "" {
 		return ErrInvalidEmailAddress
 	}
@@ -64,7 +64,7 @@ func (v *EmailDomainValidator) Validate(session *sessions.SessionState) error {
 	return nil
 }
 
-func (v *EmailDomainValidator) validate(session *sessions.SessionState) error {
+func (v EmailDomainValidator) validate(session *sessions.SessionState) error {
 	email := strings.ToLower(session.Email)
 	for _, domain := range v.AllowedDomains {
 		if strings.HasSuffix(email, domain) {

internal/proxy/oauthproxy.go

@@ -781,13 +781,23 @@ func (p *OAuthProxy) Authenticate(rw http.ResponseWriter, req *http.Request) (er
 		}
 	}
 
-	errors := options.RunValidators(p.Validators, session)
-	if len(errors) == len(p.Validators) {
-		tags = append(tags, "error:validation_failed")
-		p.StatsdClient.Incr("application_error", tags, 1.0)
-		logger.WithRemoteAddress(remoteAddr).WithUser(session.Email).Info(
-			fmt.Sprintf("permission denied: unauthorized: %q", errors))
-		return ErrUserNotAuthorized
+	// We revalidate group membership whenever the session is refreshed or revalidated
+	// just above in the call to ValidateSessionState and RefreshSession.
+	// To reduce strain on upstream identity providers we only revalidate email domains and
+	// addresses on each request here.
+	for _, v := range p.Validators {
+		_, EmailGroupValidator := v.(options.EmailGroupValidator)
+
+		if !EmailGroupValidator {
+			err := v.Validate(session)
+			if err != nil {
+				tags = append(tags, "error:validation_failed")
+				p.StatsdClient.Incr("application_error", tags, 1.0)
+				logger.WithRemoteAddress(remoteAddr).WithUser(session.Email).Info(
+					fmt.Sprintf("permission denied: unauthorized: %q", err))
+				return ErrUserNotAuthorized
+			}
+		}
 	}
 
 	logger.WithRemoteAddress(remoteAddr).WithUser(session.Email).Info(