From 8d0149ccf8eeefaf5672a41d3b9ee5d943748da1 Mon Sep 17 00:00:00 2001
From: Erico Fusco <ericofusco@users.noreply.github.com>
Date: Fri, 13 Mar 2020 20:10:38 +0000
Subject: [PATCH] Fix issue with group validation called on every request
 (#435)

* Revert group validation on every request

* Fix syntax

* Remove unit tests associated with reverted change

* Update CHANGELOG
---
 CHANGELOG.md       |  1 +
 oauthproxy.go      | 12 ++++-----
 oauthproxy_test.go | 61 ----------------------------------------------
 3 files changed, 6 insertions(+), 68 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a20e907b..dae98e94 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@
 
 ## Changes since v5.0.0
 
+- [#435](https://github.comq/pusher/oauth2_proxy/pull/435) Fix issue with group validation calling google directory API on every HTTP request (@ericofusco)
 - [#400](https://github.com/pusher/oauth2_proxy/pull/400) Add `nsswitch.conf` to Docker image to allow hosts file to work (@luketainton)
 - [#385](https://github.com/pusher/oauth2_proxy/pull/385) Use the `Authorization` header instead of `access_token` for refreshing GitHub Provider sessions (@ibuclaw)
 - [#372](https://github.com/pusher/oauth2_proxy/pull/372) Allow fallback to secondary verified email address in GitHub provider (@dmnemec)
diff --git a/oauthproxy.go b/oauthproxy.go
index 586c1e63..4d995b0f 100644
--- a/oauthproxy.go
+++ b/oauthproxy.go
@@ -900,13 +900,11 @@ func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.R
 		}
 	}
 
-	if session != nil && session.Email != "" {
-		if !p.Validator(session.Email) || !p.provider.ValidateGroup(session.Email) {
-			logger.Printf(session.Email, req, logger.AuthFailure, "Invalid authentication via session: removing session %s", session)
-			session = nil
-			saveSession = false
-			clearSession = true
-		}
+	if session != nil && session.Email != "" && !p.Validator(session.Email) {
+		logger.Printf(session.Email, req, logger.AuthFailure, "Invalid authentication via session: removing session %s", session)
+		session = nil
+		saveSession = false
+		clearSession = true
 	}
 
 	if saveSession && session != nil {
diff --git a/oauthproxy_test.go b/oauthproxy_test.go
index 1a576f97..7681d947 100644
--- a/oauthproxy_test.go
+++ b/oauthproxy_test.go
@@ -379,13 +379,6 @@ func (tp *TestProvider) ValidateSessionState(session *sessions.SessionState) boo
 	return tp.ValidToken
 }
 
-func (tp *TestProvider) ValidateGroup(email string) bool {
-	if tp.GroupValidator != nil {
-		return tp.GroupValidator(email)
-	}
-	return true
-}
-
 func TestBasicAuthPassword(t *testing.T) {
 	providerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		logger.Printf("%#v", r)
@@ -1052,25 +1045,6 @@ func TestAuthOnlyEndpointUnauthorizedOnEmailValidationFailure(t *testing.T) {
 	assert.Equal(t, "unauthorized request\n", string(bodyBytes))
 }
 
-func TestAuthOnlyEndpointUnauthorizedOnProviderGroupValidationFailure(t *testing.T) {
-	test := NewAuthOnlyEndpointTest()
-	startSession := &sessions.SessionState{
-		Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: time.Now()}
-	test.SaveSession(startSession)
-	provider := &TestProvider{
-		ValidToken: true,
-		GroupValidator: func(s string) bool {
-			return false
-		},
-	}
-
-	test.proxy.provider = provider
-	test.proxy.ServeHTTP(test.rw, test.req)
-	assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
-	bodyBytes, _ := ioutil.ReadAll(test.rw.Body)
-	assert.Equal(t, "unauthorized request\n", string(bodyBytes))
-}
-
 func TestAuthOnlyEndpointSetXAuthRequestHeaders(t *testing.T) {
 	var pcTest ProcessCookieTest
 
@@ -1489,41 +1463,6 @@ func TestGetJwtSession(t *testing.T) {
 	assert.Equal(t, test.rw.Header().Get("X-Auth-Request-Email"), "john@example.com")
 }
 
-func TestJwtUnauthorizedOnGroupValidationFailure(t *testing.T) {
-	goodJwt := "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9." +
-		"eyJzdWIiOiIxMjM0NTY3ODkwIiwiYXVkIjoiaHR0cHM6Ly90ZXN0Lm15YXBwLmNvbSIsIm5hbWUiOiJKb2huIERvZSIsImVtY" +
-		"WlsIjoiam9obkBleGFtcGxlLmNvbSIsImlzcyI6Imh0dHBzOi8vaXNzdWVyLmV4YW1wbGUuY29tIiwiaWF0IjoxNTUzNjkxMj" +
-		"E1LCJleHAiOjE5MTIxNTE4MjF9." +
-		"rLVyzOnEldUq_pNkfa-WiV8TVJYWyZCaM2Am_uo8FGg11zD7l-qmz3x1seTvqpH6Y0Ty00fmv6dJnGnC8WMnPXQiodRTfhBSe" +
-		"OKZMu0HkMD2sg52zlKkbfLTO6ic5VnbVgwjjrB8am_Ta6w7kyFUaB5C1BsIrrLMldkWEhynbb8"
-
-	keyset := NoOpKeySet{}
-	verifier := oidc.NewVerifier("https://issuer.example.com", keyset,
-		&oidc.Config{ClientID: "https://test.myapp.com", SkipExpiryCheck: true})
-
-	test := NewAuthOnlyEndpointTest(func(opts *Options) {
-		opts.PassAuthorization = true
-		opts.SetAuthorization = true
-		opts.SetXAuthRequest = true
-		opts.SkipJwtBearerTokens = true
-		opts.jwtBearerVerifiers = append(opts.jwtBearerVerifiers, verifier)
-	})
-	tp, _ := test.proxy.provider.(*TestProvider)
-	// Verify ValidateGroup fails JWT authorization
-	tp.GroupValidator = func(s string) bool {
-		return false
-	}
-
-	authHeader := fmt.Sprintf("Bearer %s", goodJwt)
-	test.req.Header = map[string][]string{
-		"Authorization": {authHeader},
-	}
-	test.proxy.ServeHTTP(test.rw, test.req)
-	if test.rw.Code != http.StatusUnauthorized {
-		t.Fatalf("expected 401 got %d", test.rw.Code)
-	}
-}
-
 func TestFindJwtBearerToken(t *testing.T) {
 	p := OAuthProxy{CookieName: "oauth2", CookieDomain: "abc"}
 	getReq := &http.Request{URL: &url.URL{Scheme: "http", Host: "example.com"}}