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
This commit is contained in:
		
							parent
							
								
									e3fb25efe6
								
							
						
					
					
						commit
						8d0149ccf8
					
				|  | @ -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) | ||||
|  |  | |||
|  | @ -900,14 +900,12 @@ 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) { | ||||
| 	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 { | ||||
| 		err = p.SaveSession(rw, req, session) | ||||
|  |  | |||
|  | @ -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"}} | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue