Let authentication fail when session validation fails (fixes #1396) (#1433)

* Error page for session validation failure

* Fix existing tests

* Add test-case for session validation failure

* Simplify test

* Add changelog entry for PR

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ## Changes since v7.2.0
 
+- [#1433](https://github.com/oauth2-proxy/oauth2-proxy/pull/1433) Let authentication fail when session validation fails ((@stippi2)
+
 # V7.2.0
 
 ## Release Highlights

oauthproxy.go

@@ -755,7 +755,11 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
 	}
 
 	csrf.SetSessionNonce(session)
-	p.provider.ValidateSession(req.Context(), session)
+	if !p.provider.ValidateSession(req.Context(), session) {
+		logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Session validation failed: %s", session)
+		p.ErrorPage(rw, req, http.StatusForbidden, "Session validation failed")
+		return
+	}
 
 	if !p.redirectValidator.IsValidRedirect(appRedirect) {
 		appRedirect = "/"

oauthproxy_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/validation"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/providers"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -322,6 +323,7 @@ type PassAccessTokenTest struct {
 
 type PassAccessTokenTestOptions struct {
 	PassAccessToken bool
+	ValidToken      bool
 	ProxyUpstream   options.Upstream
 }
 
@@ -385,7 +387,9 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) (*PassAccessTokenTe
 	providerURL, _ := url.Parse(patt.providerServer.URL)
 	const emailAddress = "michael.bland@gsa.gov"
 
-	patt.opts.SetProvider(NewTestProvider(providerURL, emailAddress))
+	testProvider := NewTestProvider(providerURL, emailAddress)
+	testProvider.ValidToken = opts.ValidToken
+	patt.opts.SetProvider(testProvider)
 	patt.proxy, err = NewOAuthProxy(patt.opts, func(email string) bool {
 		return email == emailAddress
 	})
@@ -428,7 +432,11 @@ func (patTest *PassAccessTokenTest) getCallbackEndpoint() (httpCode int, cookie
 
 	patTest.proxy.ServeHTTP(rw, req)
 
-	return rw.Code, rw.Header().Values("Set-Cookie")[1]
+	if len(rw.Header().Values("Set-Cookie")) >= 2 {
+		cookie = rw.Header().Values("Set-Cookie")[1]
+	}
+
+	return rw.Code, cookie
 }
 
 // getEndpointWithCookie makes a requests againt the oauthproxy with passed requestPath
@@ -470,6 +478,7 @@ func (patTest *PassAccessTokenTest) getEndpointWithCookie(cookie string, endpoin
 func TestForwardAccessTokenUpstream(t *testing.T) {
 	patTest, err := NewPassAccessTokenTest(PassAccessTokenTestOptions{
 		PassAccessToken: true,
+		ValidToken:      true,
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -496,6 +505,7 @@ func TestForwardAccessTokenUpstream(t *testing.T) {
 func TestStaticProxyUpstream(t *testing.T) {
 	patTest, err := NewPassAccessTokenTest(PassAccessTokenTestOptions{
 		PassAccessToken: true,
+		ValidToken:      true,
 		ProxyUpstream: options.Upstream{
 			ID:     "static-proxy",
 			Path:   "/static-proxy",
@@ -526,6 +536,7 @@ func TestStaticProxyUpstream(t *testing.T) {
 func TestDoNotForwardAccessTokenUpstream(t *testing.T) {
 	patTest, err := NewPassAccessTokenTest(PassAccessTokenTestOptions{
 		PassAccessToken: false,
+		ValidToken:      true,
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -548,6 +559,19 @@ func TestDoNotForwardAccessTokenUpstream(t *testing.T) {
 	assert.Equal(t, "No access token found.", payload)
 }
 
+func TestSessionValidationFailure(t *testing.T) {
+	patTest, err := NewPassAccessTokenTest(PassAccessTokenTestOptions{
+		ValidToken: false,
+	})
+	require.NoError(t, err)
+	t.Cleanup(patTest.Close)
+
+	// An unsuccessful validation will return 403 and not set the auth cookie.
+	code, cookie := patTest.getCallbackEndpoint()
+	assert.Equal(t, http.StatusForbidden, code)
+	assert.Equal(t, "", cookie)
+}
+
 type SignInPageTest struct {
 	opts                 *options.Options
 	proxy                *OAuthProxy