2c668a5 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

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>

commit2c668a52d4dc6fba0b13b1ec3ed5b9b828552a0c authorStephan Aßmus <stephan.assmus@sap.com> dateFri 12 Nov 2021, 19:36 (5 years ago) parent9caf8c7

3 files changed+33−3

compare with master

unifiedsplit

CHANGELOG.md+2 oauthproxy.go+5−1 oauthproxy_test.go+26−2

CHANGELOG.md+2

@@ -8,6 +8,8 @@

99## Changes since v7.2.0

1010

11+- [#1433](https://github.com/oauth2-proxy/oauth2-proxy/pull/1433) Let authentication fail when session validation fails ((@stippi2)

12+

1113# V7.2.0

1214

1315## Release Highlights

oauthproxy.go+5−1

@@ -755,7 +755,11 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {

755755 }

756756

757757 csrf.SetSessionNonce(session)

758− p.provider.ValidateSession(req.Context(), session)

758+ if !p.provider.ValidateSession(req.Context(), session) {

759+ logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Session validation failed: %s", session)

760+ p.ErrorPage(rw, req, http.StatusForbidden, "Session validation failed")

761+ return

762+ }

759763

760764 if !p.redirectValidator.IsValidRedirect(appRedirect) {

761765 appRedirect = "/"

oauthproxy_test.go+26−2

@@ -26,6 +26,7 @@ import (

2626 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/validation"

2727 "github.com/oauth2-proxy/oauth2-proxy/v7/providers"

2828 "github.com/stretchr/testify/assert"

29+ "github.com/stretchr/testify/require"

2930)

3031

3132const (

@@ -322,6 +323,7 @@ type PassAccessTokenTest struct {

322323

323324type PassAccessTokenTestOptions struct {

324325 PassAccessToken bool

326+ ValidToken bool

325327 ProxyUpstream options.Upstream

326328}

327329

@@ -385,7 +387,9 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) (*PassAccessTokenTe

385387 providerURL, _ := url.Parse(patt.providerServer.URL)

386388 const emailAddress = "michael.bland@gsa.gov"

387389

388− patt.opts.SetProvider(NewTestProvider(providerURL, emailAddress))

390+ testProvider := NewTestProvider(providerURL, emailAddress)

391+ testProvider.ValidToken = opts.ValidToken

392+ patt.opts.SetProvider(testProvider)

389393 patt.proxy, err = NewOAuthProxy(patt.opts, func(email string) bool {

390394 return email == emailAddress

391395 })

@@ -428,7 +432,11 @@ func (patTest *PassAccessTokenTest) getCallbackEndpoint() (httpCode int, cookie

428432

429433 patTest.proxy.ServeHTTP(rw, req)

430434

431− return rw.Code, rw.Header().Values("Set-Cookie")[1]

435+ if len(rw.Header().Values("Set-Cookie")) >= 2 {

436+ cookie = rw.Header().Values("Set-Cookie")[1]

437+ }

438+

439+ return rw.Code, cookie

432440}

433441

434442// getEndpointWithCookie makes a requests againt the oauthproxy with passed requestPath

@@ -470,6 +478,7 @@ func (patTest *PassAccessTokenTest) getEndpointWithCookie(cookie string, endpoin

470478func TestForwardAccessTokenUpstream(t *testing.T) {

471479 patTest, err := NewPassAccessTokenTest(PassAccessTokenTestOptions{

472480 PassAccessToken: true,

481+ ValidToken: true,

473482 })

474483 if err != nil {

475484 t.Fatal(err)

@@ -496,6 +505,7 @@ func TestForwardAccessTokenUpstream(t *testing.T) {

496505func TestStaticProxyUpstream(t *testing.T) {

497506 patTest, err := NewPassAccessTokenTest(PassAccessTokenTestOptions{

498507 PassAccessToken: true,

508+ ValidToken: true,

499509 ProxyUpstream: options.Upstream{

500510 ID: "static-proxy",

501511 Path: "/static-proxy",

@@ -526,6 +536,7 @@ func TestStaticProxyUpstream(t *testing.T) {

526536func TestDoNotForwardAccessTokenUpstream(t *testing.T) {

527537 patTest, err := NewPassAccessTokenTest(PassAccessTokenTestOptions{

528538 PassAccessToken: false,

539+ ValidToken: true,

529540 })

530541 if err != nil {

531542 t.Fatal(err)

@@ -548,6 +559,19 @@ func TestDoNotForwardAccessTokenUpstream(t *testing.T) {

548559 assert.Equal(t, "No access token found.", payload)

549560}

550561

562+func TestSessionValidationFailure(t *testing.T) {

563+ patTest, err := NewPassAccessTokenTest(PassAccessTokenTestOptions{

564+ ValidToken: false,

565+ })

566+ require.NoError(t, err)

567+ t.Cleanup(patTest.Close)

568+

569+ // An unsuccessful validation will return 403 and not set the auth cookie.

570+ code, cookie := patTest.getCallbackEndpoint()

571+ assert.Equal(t, http.StatusForbidden, code)

572+ assert.Equal(t, "", cookie)

573+}

574+

551575type SignInPageTest struct {

552576 opts *options.Options

553577 proxy *OAuthProxy