Integrate redirect package with OAuth2 Proxy

oauthproxy_test.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"bufio"
 	"context"
 	"crypto"
 	"encoding/base64"
@@ -11,7 +10,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"os"
 	"regexp"
 	"strings"
 	"testing"
@@ -19,7 +17,6 @@ import (
 
 	"github.com/coreos/go-oidc"
 	"github.com/mbland/hmacauth"
-	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/cookies"
@@ -28,10 +25,7 @@ import (
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/upstream"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/validation"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/providers"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -63,304 +57,6 @@ func TestRobotsTxt(t *testing.T) {
 	assert.Equal(t, "User-agent: *\nDisallow: /\n", rw.Body.String())
 }
 
-func TestIsValidRedirect(t *testing.T) {
-	opts := baseTestOptions()
-	// Should match domains that are exactly foo.bar and any subdomain of bar.foo
-	opts.WhitelistDomains = []string{
-		"foo.bar",
-		".bar.foo",
-		"port.bar:8080",
-		".sub.port.bar:8080",
-		"anyport.bar:*",
-		".sub.anyport.bar:*",
-	}
-	err := validation.Validate(opts)
-	assert.NoError(t, err)
-
-	proxy, err := NewOAuthProxy(opts, func(string) bool { return true })
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	testCases := []struct {
-		Desc, Redirect string
-		ExpectedResult bool
-	}{
-		{
-			Desc:           "noRD",
-			Redirect:       "",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "singleSlash",
-			Redirect:       "/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "doubleSlash",
-			Redirect:       "//redirect",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "validHTTP",
-			Redirect:       "http://foo.bar/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "validHTTPS",
-			Redirect:       "https://foo.bar/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "invalidHTTPSubdomain",
-			Redirect:       "http://baz.foo.bar/redirect",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "invalidHTTPSSubdomain",
-			Redirect:       "https://baz.foo.bar/redirect",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "validHTTPSubdomain",
-			Redirect:       "http://baz.bar.foo/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "validHTTPSSubdomain",
-			Redirect:       "https://baz.bar.foo/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "validHTTPDomain",
-			Redirect:       "http://bar.foo/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "invalidHTTP1",
-			Redirect:       "http://foo.bar.evil.corp/redirect",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "invalidHTTPS1",
-			Redirect:       "https://foo.bar.evil.corp/redirect",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "invalidHTTP2",
-			Redirect:       "http://evil.corp/redirect?rd=foo.bar",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "invalidHTTPS2",
-			Redirect:       "https://evil.corp/redirect?rd=foo.bar",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "invalidPort",
-			Redirect:       "https://evil.corp:3838/redirect",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "invalidEmptyPort",
-			Redirect:       "http://foo.bar:3838/redirect",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "invalidEmptyPortSubdomain",
-			Redirect:       "http://baz.bar.foo:3838/redirect",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "validSpecificPort",
-			Redirect:       "http://port.bar:8080/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "invalidSpecificPort",
-			Redirect:       "http://port.bar:3838/redirect",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "validSpecificPortSubdomain",
-			Redirect:       "http://foo.sub.port.bar:8080/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "invalidSpecificPortSubdomain",
-			Redirect:       "http://foo.sub.port.bar:3838/redirect",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "validAnyPort1",
-			Redirect:       "http://anyport.bar:8080/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "validAnyPort2",
-			Redirect:       "http://anyport.bar:8081/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "validAnyPortSubdomain1",
-			Redirect:       "http://a.sub.anyport.bar:8080/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "validAnyPortSubdomain2",
-			Redirect:       "http://a.sub.anyport.bar:8081/redirect",
-			ExpectedResult: true,
-		},
-		{
-			Desc:           "openRedirect1",
-			Redirect:       "/\\evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectSpace1",
-			Redirect:       "/ /evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectSpace2",
-			Redirect:       "/ \\evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectTab1",
-			Redirect:       "/\t/evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectTab2",
-			Redirect:       "/\t\\evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectVerticalTab1",
-			Redirect:       "/\v/evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectVerticalTab2",
-			Redirect:       "/\v\\evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectNewLine1",
-			Redirect:       "/\n/evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectNewLine2",
-			Redirect:       "/\n\\evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectCarriageReturn1",
-			Redirect:       "/\r/evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectCarriageReturn2",
-			Redirect:       "/\r\\evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectTripleTab",
-			Redirect:       "/\t\t/\t/evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectTripleTab2",
-			Redirect:       "/\t\t\\\t/evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectQuadTab1",
-			Redirect:       "/\t\t/\t\t\\evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectQuadTab2",
-			Redirect:       "/\t\t\\\t\t/evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectPeriod1",
-			Redirect:       "/./\\evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectPeriod2",
-			Redirect:       "/./../../\\evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectDoubleTab",
-			Redirect:       "/\t/\t\\evil.com",
-			ExpectedResult: false,
-		},
-		{
-			Desc:           "openRedirectPartialSubdomain",
-			Redirect:       "http://evilbar.foo",
-			ExpectedResult: false,
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.Desc, func(t *testing.T) {
-			result := proxy.IsValidRedirect(tc.Redirect)
-
-			if result != tc.ExpectedResult {
-				t.Errorf("expected %t got %t", tc.ExpectedResult, result)
-			}
-		})
-	}
-}
-
-var _ = Describe("OpenRedirect Tests", func() {
-	var proxy *OAuthProxy
-
-	BeforeEach(func() {
-		opts := baseTestOptions()
-		// Should match domains that are exactly foo.bar and any subdomain of bar.foo
-		opts.WhitelistDomains = []string{
-			"foo.bar",
-			".bar.foo",
-			"port.bar:8080",
-			".sub.port.bar:8080",
-			"anyport.bar:*",
-			".sub.anyport.bar:*",
-			"www.whitelisteddomain.tld",
-		}
-		Expect(validation.Validate(opts)).To(Succeed())
-
-		var err error
-		proxy, err = NewOAuthProxy(opts, func(string) bool { return true })
-		Expect(err).ToNot(HaveOccurred())
-	})
-
-	file, err := os.Open("./testdata/openredirects.txt")
-	Expect(err).ToNot(HaveOccurred())
-	defer func() {
-		Expect(file.Close()).To(Succeed())
-	}()
-
-	scanner := bufio.NewScanner(file)
-	for scanner.Scan() {
-		rd := scanner.Text()
-		It(rd, func() {
-			rdUnescaped, err := url.QueryUnescape(rd)
-			Expect(err).ToNot(HaveOccurred())
-
-			Expect(proxy.IsValidRedirect(rdUnescaped)).To(BeFalse(), "Expected redirect not to be valid")
-		})
-	}
-
-	Expect(scanner.Err()).ToNot(HaveOccurred())
-})
-
 type TestProvider struct {
 	*providers.ProviderData
 	EmailAddress   string
@@ -1770,165 +1466,6 @@ func TestRequestSignature(t *testing.T) {
 	}
 }
 
-func Test_getAppRedirect(t *testing.T) {
-	opts := baseTestOptions()
-	opts.WhitelistDomains = append(opts.WhitelistDomains, ".example.com", ".example.com:8443")
-	err := validation.Validate(opts)
-	assert.NoError(t, err)
-	require.NotEmpty(t, opts.ProxyPrefix)
-	proxy, err := NewOAuthProxy(opts, func(s string) bool { return false })
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	tests := []struct {
-		name             string
-		url              string
-		headers          map[string]string
-		reverseProxy     bool
-		expectedRedirect string
-	}{
-		{
-			name:             "request outside of ProxyPrefix redirects to original URL",
-			url:              "/foo/bar",
-			headers:          nil,
-			reverseProxy:     false,
-			expectedRedirect: "/foo/bar",
-		},
-		{
-			name:             "request with query preserves query",
-			url:              "/foo?bar",
-			headers:          nil,
-			reverseProxy:     false,
-			expectedRedirect: "/foo?bar",
-		},
-		{
-			name:             "request under ProxyPrefix redirects to root",
-			url:              proxy.ProxyPrefix + "/foo/bar",
-			headers:          nil,
-			reverseProxy:     false,
-			expectedRedirect: "/",
-		},
-		{
-			name: "proxied request outside of ProxyPrefix redirects to proxied URL",
-			url:  "https://oauth.example.com/foo/bar",
-			headers: map[string]string{
-				"X-Forwarded-Proto": "https",
-				"X-Forwarded-Host":  "a-service.example.com",
-				"X-Forwarded-Uri":   "/foo/bar",
-			},
-			reverseProxy:     true,
-			expectedRedirect: "https://a-service.example.com/foo/bar",
-		},
-		{
-			name: "non-proxied request with spoofed proxy headers wouldn't redirect",
-			url:  "https://oauth.example.com/foo?bar",
-			headers: map[string]string{
-				"X-Forwarded-Proto": "https",
-				"X-Forwarded-Host":  "a-service.example.com",
-				"X-Forwarded-Uri":   "/foo/bar",
-			},
-			reverseProxy:     false,
-			expectedRedirect: "/foo?bar",
-		},
-		{
-			name: "proxied request under ProxyPrefix redirects to root",
-			url:  "https://oauth.example.com" + proxy.ProxyPrefix + "/foo/bar",
-			headers: map[string]string{
-				"X-Forwarded-Proto": "https",
-				"X-Forwarded-Host":  "a-service.example.com",
-				"X-Forwarded-Uri":   proxy.ProxyPrefix + "/foo/bar",
-			},
-			reverseProxy:     true,
-			expectedRedirect: "https://a-service.example.com/",
-		},
-		{
-			name: "proxied request with port under ProxyPrefix redirects to root",
-			url:  "https://oauth.example.com" + proxy.ProxyPrefix + "/foo/bar",
-			headers: map[string]string{
-				"X-Forwarded-Proto": "https",
-				"X-Forwarded-Host":  "a-service.example.com:8443",
-				"X-Forwarded-Uri":   proxy.ProxyPrefix + "/foo/bar",
-			},
-			reverseProxy:     true,
-			expectedRedirect: "https://a-service.example.com:8443/",
-		},
-		{
-			name: "proxied request with missing uri header would still redirect to desired redirect",
-			url:  "https://oauth.example.com/foo?bar",
-			headers: map[string]string{
-				"X-Forwarded-Proto": "https",
-				"X-Forwarded-Host":  "a-service.example.com",
-			},
-			reverseProxy:     true,
-			expectedRedirect: "https://a-service.example.com/foo?bar",
-		},
-		{
-			name:             "request with headers proxy not being set (and reverse proxy enabled) would still redirect to desired redirect",
-			url:              "https://oauth.example.com/foo?bar",
-			headers:          nil,
-			reverseProxy:     true,
-			expectedRedirect: "/foo?bar",
-		},
-		{
-			name: "proxied request with X-Auth-Request-Redirect being set outside of ProxyPrefix redirects to proxied URL",
-			url:  "https://oauth.example.com/foo/bar",
-			headers: map[string]string{
-				"X-Auth-Request-Redirect": "https://a-service.example.com/foo/bar",
-			},
-			reverseProxy:     true,
-			expectedRedirect: "https://a-service.example.com/foo/bar",
-		},
-		{
-			name:             "proxied request with rd query string redirects to proxied URL",
-			url:              "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fbar",
-			headers:          nil,
-			reverseProxy:     false,
-			expectedRedirect: "https://a-service.example.com/foo/bar",
-		},
-		{
-			name: "proxied request with rd query string and all headers set (and reverse proxy not enabled) redirects to proxied URL on rd query string",
-			url:  "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fjazz",
-			headers: map[string]string{
-				"X-Auth-Request-Redirect": "https://a-service.example.com/foo/baz",
-				"X-Forwarded-Proto":       "http",
-				"X-Forwarded-Host":        "another-service.example.com",
-				"X-Forwarded-Uri":         "/seasons/greetings",
-			},
-			reverseProxy:     false,
-			expectedRedirect: "https://a-service.example.com/foo/jazz",
-		},
-		{
-			name: "proxied request with rd query string and some headers set redirects to proxied URL on rd query string",
-			url:  "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fbaz",
-			headers: map[string]string{
-				"X-Forwarded-Proto": "https",
-				"X-Forwarded-Host":  "another-service.example.com",
-				"X-Forwarded-Uri":   "/seasons/greetings",
-			},
-			reverseProxy:     true,
-			expectedRedirect: "https://a-service.example.com/foo/baz",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			req, _ := http.NewRequest("GET", tt.url, nil)
-			for header, value := range tt.headers {
-				if value != "" {
-					req.Header.Add(header, value)
-				}
-			}
-			req = middleware.AddRequestScope(req, &middleware.RequestScope{
-				ReverseProxy: tt.reverseProxy,
-			})
-			redirect, err := proxy.getAppRedirect(req)
-
-			assert.NoError(t, err)
-			assert.Equal(t, tt.expectedRedirect, redirect)
-		})
-	}
-}
-
 type ajaxRequestTest struct {
 	opts  *options.Options
 	proxy *OAuthProxy