c2325ec · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

on master help sign in

Merge pull request #1226 from oauth2-proxy/app-redirect

c2325ecNick Meves5 years ago

CHANGELOG.md+1

compare with master

unifiedsplit

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

9## Changes since v7.1.3

11+- [#1226](https://github.com/oauth2-proxy/oauth2-proxy/pull/1226) Move app redirection logic to its own package (@JoelSpeed)

11- [#1128](https://github.com/oauth2-proxy/oauth2-proxy/pull/1128) Use gorilla mux for OAuth Proxy routing (@JoelSpeed)

12- [#1128](https://github.com/oauth2-proxy/oauth2-proxy/pull/1128) Use gorilla mux for OAuth Proxy routing (@JoelSpeed)

12- [#1238](https://github.com/oauth2-proxy/oauth2-proxy/pull/1238) Added ADFS provider (@samirachoadi)

13- [#1238](https://github.com/oauth2-proxy/oauth2-proxy/pull/1238) Added ADFS provider (@samirachoadi)

13- [#1227](https://github.com/oauth2-proxy/oauth2-proxy/pull/1227) Fix Refresh Session not working for multiple cookies (@rishi1111)

14- [#1227](https://github.com/oauth2-proxy/oauth2-proxy/pull/1227) Fix Refresh Session not working for multiple cookies (@rishi1111)

oauthproxy.go+24−215

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

22 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

23 sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"

24 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/app/pagewriter"

25+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/app/redirect"

25 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/authentication/basic"

26 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/authentication/basic"

26 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/cookies"

27 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/cookies"

27 proxyhttp "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/http"

28 proxyhttp "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/http"

@@ -55,10 +56,6 @@ var (

56 // ErrAccessDenied means the user should receive a 401 Unauthorized response

57 // ErrAccessDenied means the user should receive a 401 Unauthorized response

57 ErrAccessDenied = errors.New("access denied")

58 ErrAccessDenied = errors.New("access denied")

58−

59− // Used to check final redirects are not susceptible to open redirects.

60− // Matches //, /\ and both of these with whitespace in between (eg / / or / \).

61− invalidRedirectRegex = regexp.MustCompile(`[/\\](?:[\s\v]*|\.{1,2})[/\\]`)

62)

59)

64// allowedRoute manages method + path based allowlists

61// allowedRoute manages method + path based allowlists

@@ -87,13 +84,15 @@ type OAuthProxy struct {

87 realClientIPParser ipapi.RealClientIPParser

84 realClientIPParser ipapi.RealClientIPParser

88 trustedIPs *ip.NetSet

85 trustedIPs *ip.NetSet

90− sessionChain alice.Chain

87+ sessionChain alice.Chain

91− headersChain alice.Chain

88+ headersChain alice.Chain

92− preAuthChain alice.Chain

89+ preAuthChain alice.Chain

93− pageWriter pagewriter.Writer

90+ pageWriter pagewriter.Writer

94− server proxyhttp.Server

91+ server proxyhttp.Server

95− upstreamProxy http.Handler

92+ upstreamProxy http.Handler

96− serveMux *mux.Router

93+ serveMux *mux.Router

94+ redirectValidator redirect.Validator

95+ appDirector redirect.AppDirector

97}

96}

99// NewOAuthProxy creates a new instance of OAuthProxy from the options provided

98// NewOAuthProxy creates a new instance of OAuthProxy from the options provided

@@ -176,6 +175,12 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr

176 return nil, fmt.Errorf("could not build headers chain: %v", err)

175 return nil, fmt.Errorf("could not build headers chain: %v", err)

177 }

176 }

178

177

178+ redirectValidator := redirect.NewValidator(opts.WhitelistDomains)

179+ appDirector := redirect.NewAppDirector(redirect.AppDirectorOpts{

180+ ProxyPrefix: opts.ProxyPrefix,

181+ Validator: redirectValidator,

182+ })

183+

179 p := &OAuthProxy{

184 p := &OAuthProxy{

180 CookieOptions: &opts.Cookie,

185 CookieOptions: &opts.Cookie,

181 Validator: validator,

186 Validator: validator,

@@ -200,6 +205,8 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr

200 preAuthChain: preAuthChain,

205 preAuthChain: preAuthChain,

201 pageWriter: pageWriter,

206 pageWriter: pageWriter,

202 upstreamProxy: upstreamProxy,

207 upstreamProxy: upstreamProxy,

208+ redirectValidator: redirectValidator,

209+ appDirector: appDirector,

203 }

210 }

204 p.buildServeMux(opts.ProxyPrefix)

211 p.buildServeMux(opts.ProxyPrefix)

205

212

@@ -465,59 +472,13 @@ func (p *OAuthProxy) SaveSession(rw http.ResponseWriter, req *http.Request, s *s

465 return p.sessionStore.Save(rw, req, s)

472 return p.sessionStore.Save(rw, req, s)

466}

473}

467

474

468−// IsValidRedirect checks whether the redirect URL is whitelisted

469−func (p *OAuthProxy) IsValidRedirect(redirect string) bool {

470− switch {

471− case redirect == "":

472− // The user didn't specify a redirect, should fallback to `/`

473− return false

474− case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !invalidRedirectRegex.MatchString(redirect):

475− return true

476− case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):

477− redirectURL, err := url.Parse(redirect)

478− if err != nil {

479− logger.Printf("Rejecting invalid redirect %q: scheme unsupported or missing", redirect)

480− return false

481− }

482− redirectHostname := redirectURL.Hostname()

483−

484− for _, allowedDomain := range p.whitelistDomains {

485− allowedHost, allowedPort := splitHostPort(allowedDomain)

486− if allowedHost == "" {

487− continue

488− }

489−

490− if redirectHostname == strings.TrimPrefix(allowedHost, ".") ||

491− (strings.HasPrefix(allowedHost, ".") &&

492− strings.HasSuffix(redirectHostname, allowedHost)) {

493− // the domain names match, now validate the ports

494− // if the whitelisted domain's port is '*', allow all ports

495− // if the whitelisted domain contains a specific port, only allow that port

496− // if the whitelisted domain doesn't contain a port at all, only allow empty redirect ports ie http and https

497− redirectPort := redirectURL.Port()

498− if allowedPort == "*" ||

499− allowedPort == redirectPort ||

500− (allowedPort == "" && redirectPort == "") {

501− return true

502− }

503− }

504− }

505−

506− logger.Printf("Rejecting invalid redirect %q: domain / port not in whitelist", redirect)

507− return false

508− default:

509− logger.Printf("Rejecting invalid redirect %q: not an absolute or relative URL", redirect)

510− return false

511− }

512−}

513−

514func (p *OAuthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {

475func (p *OAuthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {

515 p.serveMux.ServeHTTP(rw, req)

476 p.serveMux.ServeHTTP(rw, req)

516}

477}

517

478

518// ErrorPage writes an error response

479// ErrorPage writes an error response

519func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, req *http.Request, code int, appError string, messages ...interface{}) {

480func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, req *http.Request, code int, appError string, messages ...interface{}) {

520− redirectURL, err := p.getAppRedirect(req)

481+ redirectURL, err := p.appDirector.GetRedirect(req)

521 if err != nil {

482 if err != nil {

522 logger.Errorf("Error obtaining redirect: %v", err)

483 logger.Errorf("Error obtaining redirect: %v", err)

523 }

484 }

@@ -582,7 +543,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code

582 }

543 }

583 rw.WriteHeader(code)

544 rw.WriteHeader(code)

584

545

585− redirectURL, err := p.getAppRedirect(req)

546+ redirectURL, err := p.appDirector.GetRedirect(req)

586 if err != nil {

547 if err != nil {

587 logger.Errorf("Error obtaining redirect: %v", err)

548 logger.Errorf("Error obtaining redirect: %v", err)

588 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

549 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

@@ -617,7 +578,7 @@ func (p *OAuthProxy) ManualSignIn(req *http.Request) (string, bool) {

617

578

618// SignIn serves a page prompting users to sign in

579// SignIn serves a page prompting users to sign in

619func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) {

580func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) {

620− redirect, err := p.getAppRedirect(req)

581+ redirect, err := p.appDirector.GetRedirect(req)

621 if err != nil {

582 if err != nil {

622 logger.Errorf("Error obtaining redirect: %v", err)

583 logger.Errorf("Error obtaining redirect: %v", err)

623 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

584 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

@@ -681,7 +642,7 @@ func (p *OAuthProxy) UserInfo(rw http.ResponseWriter, req *http.Request) {

681

642

682// SignOut sends a response to clear the authentication cookie

643// SignOut sends a response to clear the authentication cookie

683func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) {

644func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) {

684− redirect, err := p.getAppRedirect(req)

645+ redirect, err := p.appDirector.GetRedirect(req)

685 if err != nil {

646 if err != nil {

686 logger.Errorf("Error obtaining redirect: %v", err)

647 logger.Errorf("Error obtaining redirect: %v", err)

687 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

648 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

@@ -707,7 +668,7 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {

707 return

668 return

708 }

669 }

709

670

710− appRedirect, err := p.getAppRedirect(req)

671+ appRedirect, err := p.appDirector.GetRedirect(req)

711 if err != nil {

672 if err != nil {

712 logger.Errorf("Error obtaining application redirect: %v", err)

673 logger.Errorf("Error obtaining application redirect: %v", err)

713 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

674 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

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

790 csrf.SetSessionNonce(session)

751 csrf.SetSessionNonce(session)

791 p.provider.ValidateSession(req.Context(), session)

752 p.provider.ValidateSession(req.Context(), session)

792

753

793− if !p.IsValidRedirect(appRedirect) {

754+ if !p.redirectValidator.IsValidRedirect(appRedirect) {

794 appRedirect = "/"

755 appRedirect = "/"

795 }

756 }

796

757

@@ -947,158 +908,6 @@ func (p *OAuthProxy) getOAuthRedirectURI(req *http.Request) string {

947 return rd.String()

908 return rd.String()

948}

909}

949

910

950−// getAppRedirect determines the full URL or URI path to redirect clients to

951−// once authenticated with the OAuthProxy

952−// Strategy priority (first legal result is used):

953−// - `rd` querysting parameter

954−// - `X-Auth-Request-Redirect` header

955−// - `X-Forwarded-(Proto|Host|Uri)` headers (when ReverseProxy mode is enabled)

956−// - `X-Forwarded-(Proto|Host)` if `Uri` has the ProxyPath (i.e. /oauth2/*)

957−// - `X-Forwarded-Uri` direct URI path (when ReverseProxy mode is enabled)

958−// - `req.URL.RequestURI` if not under the ProxyPath (i.e. /oauth2/*)

959−// - `/`

960−func (p *OAuthProxy) getAppRedirect(req *http.Request) (string, error) {

961− err := req.ParseForm()

962− if err != nil {

963− return "", err

964− }

965−

966− // These redirect getter functions are strategies ordered by priority

967− // for figuring out the redirect URL.

968− type redirectGetter func(req *http.Request) string

969− for _, rdGetter := range []redirectGetter{

970− p.getRdQuerystringRedirect,

971− p.getXAuthRequestRedirect,

972− p.getXForwardedHeadersRedirect,

973− p.getURIRedirect,

974− } {

975− redirect := rdGetter(req)

976− // Call `p.IsValidRedirect` again here a final time to be safe

977− if redirect != "" && p.IsValidRedirect(redirect) {

978− return redirect, nil

979− }

980− }

981−

982− return "/", nil

983−}

984−

985−func isForwardedRequest(req *http.Request) bool {

986− return requestutil.IsProxied(req) &&

987− req.Host != requestutil.GetRequestHost(req)

988−}

989−

990−func (p *OAuthProxy) hasProxyPrefix(path string) bool {

991− return strings.HasPrefix(path, fmt.Sprintf("%s/", p.ProxyPrefix))

992−}

993−

994−func (p *OAuthProxy) validateRedirect(redirect string, errorFormat string) string {

995− if p.IsValidRedirect(redirect) {

996− return redirect

997− }

998− if redirect != "" {

999− logger.Errorf(errorFormat, redirect)

1000− }

1001− return ""

1002−}

1003−

1004−// getRdQuerystringRedirect handles this getAppRedirect strategy:

1005−// - `rd` querysting parameter

1006−func (p *OAuthProxy) getRdQuerystringRedirect(req *http.Request) string {

1007− return p.validateRedirect(

1008− req.Form.Get("rd"),

1009− "Invalid redirect provided in rd querystring parameter: %s",

1010− )

1011−}

1012−

1013−// getXAuthRequestRedirect handles this getAppRedirect strategy:

1014−// - `X-Auth-Request-Redirect` Header

1015−func (p *OAuthProxy) getXAuthRequestRedirect(req *http.Request) string {

1016− return p.validateRedirect(

1017− req.Header.Get("X-Auth-Request-Redirect"),

1018− "Invalid redirect provided in X-Auth-Request-Redirect header: %s",

1019− )

1020−}

1021−

1022−// getXForwardedHeadersRedirect handles these getAppRedirect strategies:

1023−// - `X-Forwarded-(Proto|Host|Uri)` headers (when ReverseProxy mode is enabled)

1024−// - `X-Forwarded-(Proto|Host)` if `Uri` has the ProxyPath (i.e. /oauth2/*)

1025−func (p *OAuthProxy) getXForwardedHeadersRedirect(req *http.Request) string {

1026− if !isForwardedRequest(req) {

1027− return ""

1028− }

1029−

1030− uri := requestutil.GetRequestURI(req)

1031− if p.hasProxyPrefix(uri) {

1032− uri = "/"

1033− }

1034−

1035− redirect := fmt.Sprintf(

1036− "%s://%s%s",

1037− requestutil.GetRequestProto(req),

1038− requestutil.GetRequestHost(req),

1039− uri,

1040− )

1041−

1042− return p.validateRedirect(redirect,

1043− "Invalid redirect generated from X-Forwarded-* headers: %s")

1044−}

1045−

1046−// getURIRedirect handles these getAppRedirect strategies:

1047−// - `X-Forwarded-Uri` direct URI path (when ReverseProxy mode is enabled)

1048−// - `req.URL.RequestURI` if not under the ProxyPath (i.e. /oauth2/*)

1049−// - `/`

1050−func (p *OAuthProxy) getURIRedirect(req *http.Request) string {

1051− redirect := p.validateRedirect(

1052− requestutil.GetRequestURI(req),

1053− "Invalid redirect generated from X-Forwarded-Uri header: %s",

1054− )

1055− if redirect == "" {

1056− redirect = req.URL.RequestURI()

1057− }

1058−

1059− if p.hasProxyPrefix(redirect) {

1060− return "/"

1061− }

1062− return redirect

1063−}

1064−

1065−// splitHostPort separates host and port. If the port is not valid, it returns

1066−// the entire input as host, and it doesn't check the validity of the host.

1067−// Unlike net.SplitHostPort, but per RFC 3986, it requires ports to be numeric.

1068−// *** taken from net/url, modified validOptionalPort() to accept ":*"

1069−func splitHostPort(hostport string) (host, port string) {

1070− host = hostport

1071−

1072− colon := strings.LastIndexByte(host, ':')

1073− if colon != -1 && validOptionalPort(host[colon:]) {

1074− host, port = host[:colon], host[colon+1:]

1075− }

1076−

1077− if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") {

1078− host = host[1 : len(host)-1]

1079− }

1080−

1081− return

1082−}

1083−

1084−// validOptionalPort reports whether port is either an empty string

1085−// or matches /^:\d*$/

1086−// *** taken from net/url, modified to accept ":*"

1087−func validOptionalPort(port string) bool {

1088− if port == "" || port == ":*" {

1089− return true

1090− }

1091− if port[0] != ':' {

1092− return false

1093− }

1094− for _, b := range port[1:] {

1095− if b < '0' || b > '9' {

1096− return false

1097− }

1098− }

1099− return true

1100−}

1101−

1102// getAuthenticatedSession checks whether a user is authenticated and returns a session object and nil error if so

911// getAuthenticatedSession checks whether a user is authenticated and returns a session object and nil error if so

1103// Returns:

912// Returns:

1104// - `nil, ErrNeedsLogin` if user needs to login.

913// - `nil, ErrNeedsLogin` if user needs to login.

oauthproxy_test.go−463

@@ -1,7 +1,6 @@

1package main

3import (

4− "bufio"

5 "context"

4 "context"

6 "crypto"

5 "crypto"

7 "encoding/base64"

6 "encoding/base64"

@@ -11,7 +10,6 @@ import (

11 "net/http"

10 "net/http"

12 "net/http/httptest"

11 "net/http/httptest"

13 "net/url"

12 "net/url"

14− "os"

15 "regexp"

13 "regexp"

16 "strings"

14 "strings"

17 "testing"

15 "testing"

@@ -19,7 +17,6 @@ import (

20 "github.com/coreos/go-oidc"

18 "github.com/coreos/go-oidc"

21 "github.com/mbland/hmacauth"

19 "github.com/mbland/hmacauth"

22− "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"

23 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

20 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

24 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"

21 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"

25 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/cookies"

22 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/cookies"

@@ -28,10 +25,7 @@ import (

28 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/upstream"

25 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/upstream"

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

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

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

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

31− . "github.com/onsi/ginkgo"

32− . "github.com/onsi/gomega"

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

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

34− "github.com/stretchr/testify/require"

35)

29)

37const (

31const (

@@ -63,304 +57,6 @@ func TestRobotsTxt(t *testing.T) {

63 assert.Equal(t, "User-agent: *\nDisallow: /\n", rw.Body.String())

57 assert.Equal(t, "User-agent: *\nDisallow: /\n", rw.Body.String())

64}

58}

66−func TestIsValidRedirect(t *testing.T) {

67− opts := baseTestOptions()

68− // Should match domains that are exactly foo.bar and any subdomain of bar.foo

69− opts.WhitelistDomains = []string{

70− "foo.bar",

71− ".bar.foo",

72− "port.bar:8080",

73− ".sub.port.bar:8080",

74− "anyport.bar:*",

75− ".sub.anyport.bar:*",

76− }

77− err := validation.Validate(opts)

78− assert.NoError(t, err)

79−

80− proxy, err := NewOAuthProxy(opts, func(string) bool { return true })

81− if err != nil {

82− t.Fatal(err)

83− }

84−

85− testCases := []struct {

86− Desc, Redirect string

87− ExpectedResult bool

88− }{

89− {

90− Desc: "noRD",

91− Redirect: "",

92− ExpectedResult: false,

93− },

94− {

95− Desc: "singleSlash",

96− Redirect: "/redirect",

97− ExpectedResult: true,

98− },

99− {

100− Desc: "doubleSlash",

101− Redirect: "//redirect",

102− ExpectedResult: false,

103− },

104− {

105− Desc: "validHTTP",

106− Redirect: "http://foo.bar/redirect",

107− ExpectedResult: true,

108− },

109− {

110− Desc: "validHTTPS",

111− Redirect: "https://foo.bar/redirect",

112− ExpectedResult: true,

113− },

114− {

115− Desc: "invalidHTTPSubdomain",

116− Redirect: "http://baz.foo.bar/redirect",

117− ExpectedResult: false,

118− },

119− {

120− Desc: "invalidHTTPSSubdomain",

121− Redirect: "https://baz.foo.bar/redirect",

122− ExpectedResult: false,

123− },

124− {

125− Desc: "validHTTPSubdomain",

126− Redirect: "http://baz.bar.foo/redirect",

127− ExpectedResult: true,

128− },

129− {

130− Desc: "validHTTPSSubdomain",

131− Redirect: "https://baz.bar.foo/redirect",

132− ExpectedResult: true,

133− },

134− {

135− Desc: "validHTTPDomain",

136− Redirect: "http://bar.foo/redirect",

137− ExpectedResult: true,

138− },

139− {

140− Desc: "invalidHTTP1",

141− Redirect: "http://foo.bar.evil.corp/redirect",

142− ExpectedResult: false,

143− },

144− {

145− Desc: "invalidHTTPS1",

146− Redirect: "https://foo.bar.evil.corp/redirect",

147− ExpectedResult: false,

148− },

149− {

150− Desc: "invalidHTTP2",

151− Redirect: "http://evil.corp/redirect?rd=foo.bar",

152− ExpectedResult: false,

153− },

154− {

155− Desc: "invalidHTTPS2",

156− Redirect: "https://evil.corp/redirect?rd=foo.bar",

157− ExpectedResult: false,

158− },

159− {

160− Desc: "invalidPort",

161− Redirect: "https://evil.corp:3838/redirect",

162− ExpectedResult: false,

163− },

164− {

165− Desc: "invalidEmptyPort",

166− Redirect: "http://foo.bar:3838/redirect",

167− ExpectedResult: false,

168− },

169− {

170− Desc: "invalidEmptyPortSubdomain",

171− Redirect: "http://baz.bar.foo:3838/redirect",

172− ExpectedResult: false,

173− },

174− {

175− Desc: "validSpecificPort",

176− Redirect: "http://port.bar:8080/redirect",

177− ExpectedResult: true,

178− },

179− {

180− Desc: "invalidSpecificPort",

181− Redirect: "http://port.bar:3838/redirect",

182− ExpectedResult: false,

183− },

184− {

185− Desc: "validSpecificPortSubdomain",

186− Redirect: "http://foo.sub.port.bar:8080/redirect",

187− ExpectedResult: true,

188− },

189− {

190− Desc: "invalidSpecificPortSubdomain",

191− Redirect: "http://foo.sub.port.bar:3838/redirect",

192− ExpectedResult: false,

193− },

194− {

195− Desc: "validAnyPort1",

196− Redirect: "http://anyport.bar:8080/redirect",

197− ExpectedResult: true,

198− },

199− {

200− Desc: "validAnyPort2",

201− Redirect: "http://anyport.bar:8081/redirect",

202− ExpectedResult: true,

203− },

204− {

205− Desc: "validAnyPortSubdomain1",

206− Redirect: "http://a.sub.anyport.bar:8080/redirect",

207− ExpectedResult: true,

208− },

209− {

210− Desc: "validAnyPortSubdomain2",

211− Redirect: "http://a.sub.anyport.bar:8081/redirect",

212− ExpectedResult: true,

213− },

214− {

215− Desc: "openRedirect1",

216− Redirect: "/\\evil.com",

217− ExpectedResult: false,

218− },

219− {

220− Desc: "openRedirectSpace1",

221− Redirect: "/ /evil.com",

222− ExpectedResult: false,

223− },

224− {

225− Desc: "openRedirectSpace2",

226− Redirect: "/ \\evil.com",

227− ExpectedResult: false,

228− },

229− {

230− Desc: "openRedirectTab1",

231− Redirect: "/\t/evil.com",

232− ExpectedResult: false,

233− },

234− {

235− Desc: "openRedirectTab2",

236− Redirect: "/\t\\evil.com",

237− ExpectedResult: false,

238− },

239− {

240− Desc: "openRedirectVerticalTab1",

241− Redirect: "/\v/evil.com",

242− ExpectedResult: false,

243− },

244− {

245− Desc: "openRedirectVerticalTab2",

246− Redirect: "/\v\\evil.com",

247− ExpectedResult: false,

248− },

249− {

250− Desc: "openRedirectNewLine1",

251− Redirect: "/\n/evil.com",

252− ExpectedResult: false,

253− },

254− {

255− Desc: "openRedirectNewLine2",

256− Redirect: "/\n\\evil.com",

257− ExpectedResult: false,

258− },

259− {

260− Desc: "openRedirectCarriageReturn1",

261− Redirect: "/\r/evil.com",

262− ExpectedResult: false,

263− },

264− {

265− Desc: "openRedirectCarriageReturn2",

266− Redirect: "/\r\\evil.com",

267− ExpectedResult: false,

268− },

269− {

270− Desc: "openRedirectTripleTab",

271− Redirect: "/\t\t/\t/evil.com",

272− ExpectedResult: false,

273− },

274− {

275− Desc: "openRedirectTripleTab2",

276− Redirect: "/\t\t\\\t/evil.com",

277− ExpectedResult: false,

278− },

279− {

280− Desc: "openRedirectQuadTab1",

281− Redirect: "/\t\t/\t\t\\evil.com",

282− ExpectedResult: false,

283− },

284− {

285− Desc: "openRedirectQuadTab2",

286− Redirect: "/\t\t\\\t\t/evil.com",

287− ExpectedResult: false,

288− },

289− {

290− Desc: "openRedirectPeriod1",

291− Redirect: "/./\\evil.com",

292− ExpectedResult: false,

293− },

294− {

295− Desc: "openRedirectPeriod2",

296− Redirect: "/./../../\\evil.com",

297− ExpectedResult: false,

298− },

299− {

300− Desc: "openRedirectDoubleTab",

301− Redirect: "/\t/\t\\evil.com",

302− ExpectedResult: false,

303− },

304− {

305− Desc: "openRedirectPartialSubdomain",

306− Redirect: "http://evilbar.foo",

307− ExpectedResult: false,

308− },

309− }

310−

311− for _, tc := range testCases {

312− t.Run(tc.Desc, func(t *testing.T) {

313− result := proxy.IsValidRedirect(tc.Redirect)

314−

315− if result != tc.ExpectedResult {

316− t.Errorf("expected %t got %t", tc.ExpectedResult, result)

317− }

318− })

319− }

320−}

321−

322−var _ = Describe("OpenRedirect Tests", func() {

323− var proxy *OAuthProxy

324−

325− BeforeEach(func() {

326− opts := baseTestOptions()

327− // Should match domains that are exactly foo.bar and any subdomain of bar.foo

328− opts.WhitelistDomains = []string{

329− "foo.bar",

330− ".bar.foo",

331− "port.bar:8080",

332− ".sub.port.bar:8080",

333− "anyport.bar:*",

334− ".sub.anyport.bar:*",

335− "www.whitelisteddomain.tld",

336− }

337− Expect(validation.Validate(opts)).To(Succeed())

338−

339− var err error

340− proxy, err = NewOAuthProxy(opts, func(string) bool { return true })

341− Expect(err).ToNot(HaveOccurred())

342− })

343−

344− file, err := os.Open("./testdata/openredirects.txt")

345− Expect(err).ToNot(HaveOccurred())

346− defer func() {

347− Expect(file.Close()).To(Succeed())

348− }()

349−

350− scanner := bufio.NewScanner(file)

351− for scanner.Scan() {

352− rd := scanner.Text()

353− It(rd, func() {

354− rdUnescaped, err := url.QueryUnescape(rd)

355− Expect(err).ToNot(HaveOccurred())

356−

357− Expect(proxy.IsValidRedirect(rdUnescaped)).To(BeFalse(), "Expected redirect not to be valid")

358− })

359− }

360−

361− Expect(scanner.Err()).ToNot(HaveOccurred())

362−})

363−

364type TestProvider struct {

60type TestProvider struct {

365 *providers.ProviderData

61 *providers.ProviderData

366 EmailAddress string

62 EmailAddress string

@@ -1770,165 +1466,6 @@ func TestRequestSignature(t *testing.T) {

1770 }

1466 }

1771}

1467}

1772

1468

1773−func Test_getAppRedirect(t *testing.T) {

1774− opts := baseTestOptions()

1775− opts.WhitelistDomains = append(opts.WhitelistDomains, ".example.com", ".example.com:8443")

1776− err := validation.Validate(opts)

1777− assert.NoError(t, err)

1778− require.NotEmpty(t, opts.ProxyPrefix)

1779− proxy, err := NewOAuthProxy(opts, func(s string) bool { return false })

1780− if err != nil {

1781− t.Fatal(err)

1782− }

1783−

1784− tests := []struct {

1785− name string

1786− url string

1787− headers map[string]string

1788− reverseProxy bool

1789− expectedRedirect string

1790− }{

1791− {

1792− name: "request outside of ProxyPrefix redirects to original URL",

1793− url: "/foo/bar",

1794− headers: nil,

1795− reverseProxy: false,

1796− expectedRedirect: "/foo/bar",

1797− },

1798− {

1799− name: "request with query preserves query",

1800− url: "/foo?bar",

1801− headers: nil,

1802− reverseProxy: false,

1803− expectedRedirect: "/foo?bar",

1804− },

1805− {

1806− name: "request under ProxyPrefix redirects to root",

1807− url: proxy.ProxyPrefix + "/foo/bar",

1808− headers: nil,

1809− reverseProxy: false,

1810− expectedRedirect: "/",

1811− },

1812− {

1813− name: "proxied request outside of ProxyPrefix redirects to proxied URL",

1814− url: "https://oauth.example.com/foo/bar",

1815− headers: map[string]string{

1816− "X-Forwarded-Proto": "https",

1817− "X-Forwarded-Host": "a-service.example.com",

1818− "X-Forwarded-Uri": "/foo/bar",

1819− },

1820− reverseProxy: true,

1821− expectedRedirect: "https://a-service.example.com/foo/bar",

1822− },

1823− {

1824− name: "non-proxied request with spoofed proxy headers wouldn't redirect",

1825− url: "https://oauth.example.com/foo?bar",

1826− headers: map[string]string{

1827− "X-Forwarded-Proto": "https",

1828− "X-Forwarded-Host": "a-service.example.com",

1829− "X-Forwarded-Uri": "/foo/bar",

1830− },

1831− reverseProxy: false,

1832− expectedRedirect: "/foo?bar",

1833− },

1834− {

1835− name: "proxied request under ProxyPrefix redirects to root",

1836− url: "https://oauth.example.com" + proxy.ProxyPrefix + "/foo/bar",

1837− headers: map[string]string{

1838− "X-Forwarded-Proto": "https",

1839− "X-Forwarded-Host": "a-service.example.com",

1840− "X-Forwarded-Uri": proxy.ProxyPrefix + "/foo/bar",

1841− },

1842− reverseProxy: true,

1843− expectedRedirect: "https://a-service.example.com/",

1844− },

1845− {

1846− name: "proxied request with port under ProxyPrefix redirects to root",

1847− url: "https://oauth.example.com" + proxy.ProxyPrefix + "/foo/bar",

1848− headers: map[string]string{

1849− "X-Forwarded-Proto": "https",

1850− "X-Forwarded-Host": "a-service.example.com:8443",

1851− "X-Forwarded-Uri": proxy.ProxyPrefix + "/foo/bar",

1852− },

1853− reverseProxy: true,

1854− expectedRedirect: "https://a-service.example.com:8443/",

1855− },

1856− {

1857− name: "proxied request with missing uri header would still redirect to desired redirect",

1858− url: "https://oauth.example.com/foo?bar",

1859− headers: map[string]string{

1860− "X-Forwarded-Proto": "https",

1861− "X-Forwarded-Host": "a-service.example.com",

1862− },

1863− reverseProxy: true,

1864− expectedRedirect: "https://a-service.example.com/foo?bar",

1865− },

1866− {

1867− name: "request with headers proxy not being set (and reverse proxy enabled) would still redirect to desired redirect",

1868− url: "https://oauth.example.com/foo?bar",

1869− headers: nil,

1870− reverseProxy: true,

1871− expectedRedirect: "/foo?bar",

1872− },

1873− {

1874− name: "proxied request with X-Auth-Request-Redirect being set outside of ProxyPrefix redirects to proxied URL",

1875− url: "https://oauth.example.com/foo/bar",

1876− headers: map[string]string{

1877− "X-Auth-Request-Redirect": "https://a-service.example.com/foo/bar",

1878− },

1879− reverseProxy: true,

1880− expectedRedirect: "https://a-service.example.com/foo/bar",

1881− },

1882− {

1883− name: "proxied request with rd query string redirects to proxied URL",

1884− url: "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fbar",

1885− headers: nil,

1886− reverseProxy: false,

1887− expectedRedirect: "https://a-service.example.com/foo/bar",

1888− },

1889− {

1890− name: "proxied request with rd query string and all headers set (and reverse proxy not enabled) redirects to proxied URL on rd query string",

1891− url: "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fjazz",

1892− headers: map[string]string{

1893− "X-Auth-Request-Redirect": "https://a-service.example.com/foo/baz",

1894− "X-Forwarded-Proto": "http",

1895− "X-Forwarded-Host": "another-service.example.com",

1896− "X-Forwarded-Uri": "/seasons/greetings",

1897− },

1898− reverseProxy: false,

1899− expectedRedirect: "https://a-service.example.com/foo/jazz",

1900− },

1901− {

1902− name: "proxied request with rd query string and some headers set redirects to proxied URL on rd query string",

1903− url: "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fbaz",

1904− headers: map[string]string{

1905− "X-Forwarded-Proto": "https",

1906− "X-Forwarded-Host": "another-service.example.com",

1907− "X-Forwarded-Uri": "/seasons/greetings",

1908− },

1909− reverseProxy: true,

1910− expectedRedirect: "https://a-service.example.com/foo/baz",

1911− },

1912− }

1913− for _, tt := range tests {

1914− t.Run(tt.name, func(t *testing.T) {

1915− req, _ := http.NewRequest("GET", tt.url, nil)

1916− for header, value := range tt.headers {

1917− if value != "" {

1918− req.Header.Add(header, value)

1919− }

1920− }

1921− req = middleware.AddRequestScope(req, &middleware.RequestScope{

1922− ReverseProxy: tt.reverseProxy,

1923− })

1924− redirect, err := proxy.getAppRedirect(req)

1925−

1926− assert.NoError(t, err)

1927− assert.Equal(t, tt.expectedRedirect, redirect)

1928− })

1929− }

1930−}

1931−

1932type ajaxRequestTest struct {

1469type ajaxRequestTest struct {

1933 opts *options.Options

1470 opts *options.Options

1934 proxy *OAuthProxy

1471 proxy *OAuthProxy

pkg/app/redirect/director.gonew file+96

@@ -0,0 +1,96 @@

1+package redirect

3+import (

4+ "fmt"

5+ "net/http"

6+ "strings"

8+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"

9+)

10+

11+// AppDirector is responsible for determining where OAuth2 Proxy should redirect

12+// a users request to after the user has authenticated with the identity provider.

13+type AppDirector interface {

14+ GetRedirect(req *http.Request) (string, error)

15+}

16+

17+// AppDirectorOpts are the requirements for constructing a new AppDirector.

18+type AppDirectorOpts struct {

19+ ProxyPrefix string

20+ Validator Validator

21+}

22+

23+// NewAppDirector constructs a new AppDirector for getting the application

24+// redirect URL.

25+func NewAppDirector(opts AppDirectorOpts) AppDirector {

26+ prefix := opts.ProxyPrefix

27+ if !strings.HasSuffix(prefix, "/") {

28+ prefix = fmt.Sprintf("%s/", prefix)

29+ }

30+

31+ return &appDirector{

32+ proxyPrefix: prefix,

33+ validator: opts.Validator,

34+ }

35+}

36+

37+// appDirector implements the AppDirector interface.

38+type appDirector struct {

39+ proxyPrefix string

40+ validator Validator

41+}

42+

43+// GetRedirect determines the full URL or URI path to redirect clients to once

44+// authenticated with the OAuthProxy.

45+// Strategy priority (first legal result is used):

46+// - `rd` querysting parameter

47+// - `X-Auth-Request-Redirect` header

48+// - `X-Forwarded-(Proto|Host|Uri)` headers (when ReverseProxy mode is enabled)

49+// - `X-Forwarded-(Proto|Host)` if `Uri` has the ProxyPath (i.e. /oauth2/*)

50+// - `X-Forwarded-Uri` direct URI path (when ReverseProxy mode is enabled)

51+// - `req.URL.RequestURI` if not under the ProxyPath (i.e. /oauth2/*)

52+// - `/`

53+func (a *appDirector) GetRedirect(req *http.Request) (string, error) {

54+ err := req.ParseForm()

55+ if err != nil {

56+ return "", err

57+ }

58+

59+ // These redirect getter functions are strategies ordered by priority

60+ // for figuring out the redirect URL.

61+ for _, rdGetter := range []redirectGetter{

62+ a.getRdQuerystringRedirect,

63+ a.getXAuthRequestRedirect,

64+ a.getXForwardedHeadersRedirect,

65+ a.getURIRedirect,

66+ } {

67+ redirect := rdGetter(req)

68+ // Call `p.IsValidRedirect` again here a final time to be safe

69+ if redirect != "" && a.validator.IsValidRedirect(redirect) {

70+ return redirect, nil

71+ }

72+ }

73+

74+ return "/", nil

75+}

76+

77+// validateRedirect checks that the redirect is valid.

78+// When an invalid, non-empty redirect is found, an error will be logged using

79+// the provided format.

80+func (a *appDirector) validateRedirect(redirect string, errorFormat string) string {

81+ if a.validator.IsValidRedirect(redirect) {

82+ return redirect

83+ }

84+ if redirect != "" {

85+ logger.Errorf(errorFormat, redirect)

86+ }

87+ return ""

88+}

89+

90+// hasProxyPrefix determines whether the obtained path would be a request to

91+// one of OAuth2 Proxy's own endpoints, eg. th callback URL.

92+// Redirects to these endpoints should not be allowed as they will create

93+// redirection loops.

94+func (a *appDirector) hasProxyPrefix(path string) bool {

95+ return strings.HasPrefix(path, a.proxyPrefix)

96+}

pkg/app/redirect/director_test.gonew file+177

@@ -0,0 +1,177 @@

1+package redirect

3+import (

4+ "net/http"

6+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"

7+ . "github.com/onsi/ginkgo"

8+ . "github.com/onsi/ginkgo/extensions/table"

9+ . "github.com/onsi/gomega"

10+)

11+

12+const testProxyPrefix = "/oauth2"

13+

14+var _ = Describe("Director Suite", func() {

15+ type getRedirectTableInput struct {

16+ requestURL string

17+ headers map[string]string

18+ reverseProxy bool

19+ validator Validator

20+ expectedRedirect string

21+ }

22+

23+ DescribeTable("GetRedirect",

24+ func(in getRedirectTableInput) {

25+ appDirector := NewAppDirector(AppDirectorOpts{

26+ ProxyPrefix: testProxyPrefix,

27+ Validator: in.validator,

28+ })

29+

30+ req, _ := http.NewRequest("GET", in.requestURL, nil)

31+ for header, value := range in.headers {

32+ if value != "" {

33+ req.Header.Add(header, value)

34+ }

35+ }

36+ req = middleware.AddRequestScope(req, &middleware.RequestScope{

37+ ReverseProxy: in.reverseProxy,

38+ })

39+

40+ redirect, err := appDirector.GetRedirect(req)

41+ Expect(err).ToNot(HaveOccurred())

42+ Expect(redirect).To(Equal(in.expectedRedirect))

43+ },

44+ Entry("Request outside of the proxy prefix, redirects to original request", getRedirectTableInput{

45+ requestURL: "/foo/bar",

46+ headers: nil,

47+ reverseProxy: false,

48+ validator: testValidator(true),

49+ expectedRedirect: "/foo/bar",

50+ }),

51+ Entry("Request with query, preserves the query", getRedirectTableInput{

52+ requestURL: "/foo?bar",

53+ headers: nil,

54+ reverseProxy: false,

55+ validator: testValidator(true),

56+ expectedRedirect: "/foo?bar",

57+ }),

58+ Entry("Request under the proxy prefix, redirects to root", getRedirectTableInput{

59+ requestURL: testProxyPrefix + "/foo/bar",

60+ headers: nil,

61+ reverseProxy: false,

62+ validator: testValidator(true),

63+ expectedRedirect: "/",

64+ }),

65+ Entry("Proxied request with headers, outside of ProxyPrefix, redirects to proxied URL", getRedirectTableInput{

66+ requestURL: "https://oauth.example.com/foo/bar",

67+ headers: map[string]string{

68+ "X-Forwarded-Proto": "https",

69+ "X-Forwarded-Host": "a-service.example.com",

70+ "X-Forwarded-Uri": "/foo/bar",

71+ },

72+ reverseProxy: true,

73+ validator: testValidator(true),

74+ expectedRedirect: "https://a-service.example.com/foo/bar",

75+ }),

76+ Entry("Non-proxied request with spoofed headers, wouldn't redirect", getRedirectTableInput{

77+ requestURL: "https://oauth.example.com/foo?bar",

78+ headers: map[string]string{

79+ "X-Forwarded-Proto": "https",

80+ "X-Forwarded-Host": "a-service.example.com",

81+ "X-Forwarded-Uri": "/foo/bar",

82+ },

83+ reverseProxy: false,

84+ validator: testValidator(true),

85+ expectedRedirect: "/foo?bar",

86+ }),

87+ Entry("Proxied request with headers, under ProxyPrefix, redirects to root", getRedirectTableInput{

88+ requestURL: "https://oauth.example.com" + testProxyPrefix + "/foo/bar",

89+ headers: map[string]string{

90+ "X-Forwarded-Proto": "https",

91+ "X-Forwarded-Host": "a-service.example.com",

92+ "X-Forwarded-Uri": testProxyPrefix + "/foo/bar",

93+ },

94+ reverseProxy: true,

95+ validator: testValidator(true),

96+ expectedRedirect: "https://a-service.example.com/",

97+ }),

98+ Entry("Proxied request with port, under ProxyPrefix, redirects to root", getRedirectTableInput{

99+ requestURL: "https://oauth.example.com" + testProxyPrefix + "/foo/bar",

100+ headers: map[string]string{

101+ "X-Forwarded-Proto": "https",

102+ "X-Forwarded-Host": "a-service.example.com:8443",

103+ "X-Forwarded-Uri": testProxyPrefix + "/foo/bar",

104+ },

105+ reverseProxy: true,

106+ validator: testValidator(true),

107+ expectedRedirect: "https://a-service.example.com:8443/",

108+ }),

109+ Entry("Proxied request with headers, missing URI header, redirects to the desired redirect URL", getRedirectTableInput{

110+ requestURL: "https://oauth.example.com/foo?bar",

111+ headers: map[string]string{

112+ "X-Forwarded-Proto": "https",

113+ "X-Forwarded-Host": "a-service.example.com",

114+ },

115+ reverseProxy: true,

116+ validator: testValidator(true),

117+ expectedRedirect: "https://a-service.example.com/foo?bar",

118+ }),

119+ Entry("Proxied request without headers, with reverse proxy enabled, redirects to the desired URL", getRedirectTableInput{

120+ requestURL: "https://oauth.example.com/foo?bar",

121+ headers: nil,

122+ reverseProxy: true,

123+ validator: testValidator(true),

124+ expectedRedirect: "/foo?bar",

125+ }),

126+ Entry("Proxied request with X-Auth-Request-Redirect, outside of ProxyPrefix, redirects to proxied URL", getRedirectTableInput{

127+ requestURL: "https://oauth.example.com/foo/bar",

128+ headers: map[string]string{

129+ "X-Auth-Request-Redirect": "https://a-service.example.com/foo/bar",

130+ },

131+ reverseProxy: true,

132+ validator: testValidator(true),

133+ expectedRedirect: "https://a-service.example.com/foo/bar",

134+ }),

135+ Entry("Proxied request with RD parameter, outside of ProxyPrefix, redirects to proxied URL", getRedirectTableInput{

136+ requestURL: "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fbar",

137+ headers: nil,

138+ reverseProxy: false,

139+ validator: testValidator(true),

140+ expectedRedirect: "https://a-service.example.com/foo/bar",

141+ }),

142+ Entry("Proxied request with RD parameter and all headers set, reverse proxy disabled, redirects to proxied URL based on the RD parameter", getRedirectTableInput{

143+ requestURL: "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fjazz",

144+ headers: map[string]string{

145+ "X-Auth-Request-Redirect": "https://a-service.example.com/foo/baz",

146+ "X-Forwarded-Proto": "http",

147+ "X-Forwarded-Host": "another-service.example.com",

148+ "X-Forwarded-Uri": "/seasons/greetings",

149+ },

150+ reverseProxy: false,

151+ validator: testValidator(true),

152+ expectedRedirect: "https://a-service.example.com/foo/jazz",

153+ }),

154+ Entry("Proxied request with RD parameter and some headers set, reverse proxy enabled, redirects to proxied URL based on the RD parameter", getRedirectTableInput{

155+ requestURL: "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fjazz",

156+ headers: map[string]string{

157+ "X-Forwarded-Proto": "http",

158+ "X-Forwarded-Host": "another-service.example.com",

159+ "X-Forwarded-Uri": "/seasons/greetings",

160+ },

161+ reverseProxy: true,

162+ validator: testValidator(true),

163+ expectedRedirect: "https://a-service.example.com/foo/jazz",

164+ }),

165+ Entry("Proxied request with invalid RD parameter and some headers set, reverse proxy enabled, redirects to proxied URL based on the headers", getRedirectTableInput{

166+ requestURL: "https://oauth.example.com/foo/bar?rd=http%3A%2F%2Fanother%2Dservice%2Eexample%2Ecom%2Ffoo%2Fjazz",

167+ headers: map[string]string{

168+ "X-Forwarded-Proto": "https",

169+ "X-Forwarded-Host": "a-service.example.com",

170+ "X-Forwarded-Uri": "/foo/bar",

171+ },

172+ reverseProxy: true,

173+ validator: testValidator(false, "https://a-service.example.com/foo/bar"),

174+ expectedRedirect: "https://a-service.example.com/foo/bar",

175+ }),

176+ )

177+})

pkg/app/redirect/getters.gonew file+73

@@ -0,0 +1,73 @@

1+package redirect

3+import (

4+ "fmt"

5+ "net/http"

7+ requestutil "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests/util"

8+)

10+// redirectGetter represents a method to allow the proxy to determine a redirect

11+// based on the original request.

12+type redirectGetter func(req *http.Request) string

13+

14+// getRdQuerystringRedirect handles this getAppRedirect strategy:

15+// - `rd` querysting parameter

16+func (a *appDirector) getRdQuerystringRedirect(req *http.Request) string {

17+ return a.validateRedirect(

18+ req.Form.Get("rd"),

19+ "Invalid redirect provided in rd querystring parameter: %s",

20+ )

21+}

22+

23+// getXAuthRequestRedirect handles this getAppRedirect strategy:

24+// - `X-Auth-Request-Redirect` Header

25+func (a *appDirector) getXAuthRequestRedirect(req *http.Request) string {

26+ return a.validateRedirect(

27+ req.Header.Get("X-Auth-Request-Redirect"),

28+ "Invalid redirect provided in X-Auth-Request-Redirect header: %s",

29+ )

30+}

31+

32+// getXForwardedHeadersRedirect handles these getAppRedirect strategies:

33+// - `X-Forwarded-(Proto|Host|Uri)` headers (when ReverseProxy mode is enabled)

34+// - `X-Forwarded-(Proto|Host)` if `Uri` has the ProxyPath (i.e. /oauth2/*)

35+func (a *appDirector) getXForwardedHeadersRedirect(req *http.Request) string {

36+ if !requestutil.IsForwardedRequest(req) {

37+ return ""

38+ }

39+

40+ uri := requestutil.GetRequestURI(req)

41+ if a.hasProxyPrefix(uri) {

42+ uri = "/"

43+ }

44+

45+ redirect := fmt.Sprintf(

46+ "%s://%s%s",

47+ requestutil.GetRequestProto(req),

48+ requestutil.GetRequestHost(req),

49+ uri,

50+ )

51+

52+ return a.validateRedirect(redirect,

53+ "Invalid redirect generated from X-Forwarded-* headers: %s")

54+}

55+

56+// getURIRedirect handles these getAppRedirect strategies:

57+// - `X-Forwarded-Uri` direct URI path (when ReverseProxy mode is enabled)

58+// - `req.URL.RequestURI` if not under the ProxyPath (i.e. /oauth2/*)

59+// - `/`

60+func (a *appDirector) getURIRedirect(req *http.Request) string {

61+ redirect := a.validateRedirect(

62+ requestutil.GetRequestURI(req),

63+ "Invalid redirect generated from X-Forwarded-Uri header: %s",

64+ )

65+ if redirect == "" {

66+ redirect = req.URL.RequestURI()

67+ }

68+

69+ if a.hasProxyPrefix(redirect) {

70+ return "/"

71+ }

72+ return redirect

73+}

pkg/app/redirect/pagewriter_suite_test.gonew file+39

@@ -0,0 +1,39 @@

1+package redirect

3+import (

4+ "testing"

6+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"

7+ . "github.com/onsi/ginkgo"

8+ . "github.com/onsi/gomega"

9+)

10+

11+func TestOptionsSuite(t *testing.T) {

12+ logger.SetOutput(GinkgoWriter)

13+ logger.SetErrOutput(GinkgoWriter)

14+

15+ RegisterFailHandler(Fail)

16+ RunSpecs(t, "Redirect Suite")

17+}

18+

19+// testValidator creates a mock validator that will always return the given result.

20+func testValidator(result bool, allowedRedirects ...string) Validator {

21+ return &mockValidator{result: result, allowedRedirects: allowedRedirects}

22+}

23+

24+// mockValidator implements the Validator interface for use in testing.

25+type mockValidator struct {

26+ result bool

27+ allowedRedirects []string

28+}

29+

30+// IsValidRedirect implements the Validator interface.

31+func (m *mockValidator) IsValidRedirect(redirect string) bool {

32+ for _, allowed := range m.allowedRedirects {

33+ if redirect == allowed {

34+ return true

35+ }

36+ }

37+

38+ return m.result

39+}

pkg/app/redirect/validator.gonew file+120

@@ -0,0 +1,120 @@

1+package redirect

3+import (

4+ "net/url"

5+ "regexp"

6+ "strings"

8+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"

9+)

10+

11+var (

12+ // Used to check final redirects are not susceptible to open redirects.

13+ // Matches //, /\ and both of these with whitespace in between (eg / / or / \).

14+ invalidRedirectRegex = regexp.MustCompile(`[/\\](?:[\s\v]*|\.{1,2})[/\\]`)

15+)

16+

17+// Validator is an interface to allow validation of application

18+// redirect URLs.

19+// As these values are determined from the request, they must go

20+// through thorough checks to ensure the safety of the end user.

21+type Validator interface {

22+ IsValidRedirect(redirect string) bool

23+}

24+

25+// NewValidator constructs a new redirect validator.

26+func NewValidator(allowedDomains []string) Validator {

27+ return &validator{

28+ allowedDomains: allowedDomains,

29+ }

30+}

31+

32+// validator implements the Validator interface to allow validation

33+// of redirect URLs.

34+type validator struct {

35+ allowedDomains []string

36+}

37+

38+// IsValidRedirect checks whether the redirect URL is safe and allowed.

39+func (v *validator) IsValidRedirect(redirect string) bool {

40+ switch {

41+ case redirect == "":

42+ // The user didn't specify a redirect.

43+ // In this case, we expect the proxt to fallback to `/`

44+ return false

45+ case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !invalidRedirectRegex.MatchString(redirect):

46+ return true

47+ case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):

48+ redirectURL, err := url.Parse(redirect)

49+ if err != nil {

50+ logger.Printf("Rejecting invalid redirect %q: scheme unsupported or missing", redirect)

51+ return false

52+ }

53+ redirectHostname := redirectURL.Hostname()

54+

55+ for _, allowedDomain := range v.allowedDomains {

56+ allowedHost, allowedPort := splitHostPort(allowedDomain)

57+ if allowedHost == "" {

58+ continue

59+ }

60+

61+ if redirectHostname == strings.TrimPrefix(allowedHost, ".") ||

62+ (strings.HasPrefix(allowedHost, ".") &&

63+ strings.HasSuffix(redirectHostname, allowedHost)) {

64+ // the domain names match, now validate the ports

65+ // if the whitelisted domain's port is '*', allow all ports

66+ // if the whitelisted domain contains a specific port, only allow that port

67+ // if the whitelisted domain doesn't contain a port at all, only allow empty redirect ports ie http and https

68+ redirectPort := redirectURL.Port()

69+ if allowedPort == "*" ||

70+ allowedPort == redirectPort ||

71+ (allowedPort == "" && redirectPort == "") {

72+ return true

73+ }

74+ }

75+ }

76+

77+ logger.Printf("Rejecting invalid redirect %q: domain / port not in whitelist", redirect)

78+ return false

79+ default:

80+ logger.Printf("Rejecting invalid redirect %q: not an absolute or relative URL", redirect)

81+ return false

82+ }

83+}

84+

85+// splitHostPort separates host and port. If the port is not valid, it returns

86+// the entire input as host, and it doesn't check the validity of the host.

87+// Unlike net.SplitHostPort, but per RFC 3986, it requires ports to be numeric.

88+// *** taken from net/url, modified validOptionalPort() to accept ":*"

89+func splitHostPort(hostport string) (host, port string) {

90+ host = hostport

91+

92+ colon := strings.LastIndexByte(host, ':')

93+ if colon != -1 && validOptionalPort(host[colon:]) {

94+ host, port = host[:colon], host[colon+1:]

95+ }

96+

97+ if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") {

98+ host = host[1 : len(host)-1]

99+ }

100+

101+ return

102+}

103+

104+// validOptionalPort reports whether port is either an empty string

105+// or matches /^:\d*$/

106+// *** taken from net/url, modified to accept ":*"

107+func validOptionalPort(port string) bool {

108+ if port == "" || port == ":*" {

109+ return true

110+ }

111+ if port[0] != ':' {

112+ return false

113+ }

114+ for _, b := range port[1:] {

115+ if b < '0' || b > '9' {

116+ return false

117+ }

118+ }

119+ return true

120+}

pkg/app/redirect/validator_test.gonew file+143

@@ -0,0 +1,143 @@

1+package redirect

3+import (

4+ "bufio"

5+ "net/url"

6+ "os"

8+ . "github.com/onsi/ginkgo"

9+ . "github.com/onsi/ginkgo/extensions/table"

10+ . "github.com/onsi/gomega"

11+)

12+

13+var _ = Describe("Validator suite", func() {

14+ var testAllowedDomains []string

15+

16+ BeforeEach(func() {

17+ testAllowedDomains = []string{

18+ "foo.bar",

19+ ".bar.foo",

20+ "port.bar:8080",

21+ ".sub.port.bar:8080",

22+ "anyport.bar:*",

23+ ".sub.anyport.bar:*",

24+ "www.whitelisteddomain.tld",

25+ }

26+ })

27+

28+ Context("OpenRedirect List", func() {

29+ file, err := os.Open("../../../testdata/openredirects.txt")

30+ Expect(err).ToNot(HaveOccurred())

31+ defer func() {

32+ Expect(file.Close()).To(Succeed())

33+ }()

34+

35+ scanner := bufio.NewScanner(file)

36+ for scanner.Scan() {

37+ rd := scanner.Text()

38+ It(rd, func() {

39+ rdUnescaped, err := url.QueryUnescape(rd)

40+ Expect(err).ToNot(HaveOccurred())

41+

42+ validator := NewValidator(testAllowedDomains)

43+ Expect(validator.IsValidRedirect(rdUnescaped)).To(BeFalse(), "Expected redirect not to be valid")

44+ })

45+ }

46+

47+ Expect(scanner.Err()).ToNot(HaveOccurred())

48+ })

49+

50+ Context("Validator", func() {

51+ DescribeTable("IsValidRedirect",

52+ func(testRedirect string, expected bool) {

53+ validator := NewValidator(testAllowedDomains)

54+ Expect(validator.IsValidRedirect(testRedirect)).To(Equal(expected))

55+ },

56+ Entry("No Redirect", "", false),

57+ Entry("Single Slash", "/redirect", true),

58+ Entry("Double Slash", "//redirect", false),

59+ Entry("Valid HTTP", "http://foo.bar/redirect", true),

60+ Entry("Valid HTTPS", "https://foo.bar/redirect", true),

61+ Entry("Invalid HTTP subdomain", "http://baz.foo.bar/redirect", false),

62+ Entry("Invalid HTTPS subdomain", "https://baz.foo.bar/redirect", false),

63+ Entry("Valid HTTP subdomain", "http://baz.bar.foo/redirect", true),

64+ Entry("Valid HTTPS subdomain", "https://baz.bar.foo/redirect", true),

65+ Entry("Valid HTTP Domain", "http://bar.foo/redirect", true), // Is this correct, do we want to match the root domain?

66+ Entry("Invalid HTTP Similar Domain", "http://foo.bar.evil.corp/redirect", false),

67+ Entry("Invalid HTTPS Similar Domain", "https://foo.bar.evil.corp/redirect", false),

68+ Entry("Invalid HTTP RD Parameter", "http://evil.corp/redirect?rd=foo.bar", false),

69+ Entry("Invalid HTTPS RD Parameter", "https://evil.corp/redirect?rd=foo.bar", false),

70+ Entry("Invalid Port and Domain", "https://evil.corp:3838/redirect", false),

71+ Entry("Invalid Port on Allowed Domain", "http://foo.bar:3838/redirect", false),

72+ Entry("Invalid Port on Allowed Subdomain", "http://baz.bar.foo:3838/redirect", false),

73+ Entry("Valid Specified Port and Domain", "http://port.bar:8080/redirect", true),

74+ Entry("Invalid Specified Port and Domain", "http://port.bar:3838/redirect", false),

75+ Entry("Valid Specified Port and Subdomain", "http://foo.sub.port.bar:8080/redirect", true),

76+ Entry("Invalid Specified Port andSubdomain", "http://foo.subport.bar:3838/redirect", false),

77+ Entry("Valid Any Port, Specified Domain", "http://anyport.bar:8080/redirect", true),

78+ Entry("Valid Different Any Port, Specified Domain", "http://anyport.bar:8081/redirect", true),

79+ Entry("Valid Any Port, Specified Subdomain", "http://a.sub.anyport.bar:8080/redirect", true),

80+ Entry("Valid Different Any Port, Specified Subdomain", "http://a.sub.anyport.bar:8081/redirect", true),

81+ Entry("Escape Double Slash", "/\\evil.com", false),

82+ Entry("Space Single Slash", "/ /evil.com", false),

83+ Entry("Space Double Slash", "/ \\evil.com", false),

84+ Entry("Tab Single Slash", "/\t/evil.com", false),

85+ Entry("Tab Double Slash", "/\t\\evil.com", false),

86+ Entry("Vertical Tab Single Slash", "/\v/evil.com", false),

87+ Entry("Vertiacl Tab Double Slash", "/\v\\evil.com", false),

88+ Entry("New Line Single Slash", "/\n/evil.com", false),

89+ Entry("New Line Double Slash", "/\n\\evil.com", false),

90+ Entry("Carriage Return Single Slash", "/\r/evil.com", false),

91+ Entry("Carriage Return Double Slash", "/\r\\evil.com", false),

92+ Entry("Double Tab", "/\t/\t\\evil.com", false),

93+ Entry("Triple Tab 1", "/\t\t/\t/evil.com", false),

94+ Entry("Triple Tab 2", "/\t\t\\\t/evil.com", false),

95+ Entry("Quad Tab 1", "/\t\t/\t\t\\evil.com", false),

96+ Entry("Quad Tab 2", "/\t\t\\\t\t/evil.com", false),

97+ Entry("Relative Path", "/./\\evil.com", false),

98+ Entry("Relative Subpath", "/./../../\\evil.com", false),

99+ Entry("Partial Subdomain", "evilbar.foo", false),

100+ )

101+ })

102+

103+ Context("SplitHostPort", func() {

104+ type splitHostPortTableInput struct {

105+ hostport string

106+ expectedHost string

107+ expectedPort string

108+ }

109+

110+ DescribeTable("Should split the host and port",

111+ func(in splitHostPortTableInput) {

112+ host, port := splitHostPort(in.hostport)

113+ Expect(host).To(Equal(in.expectedHost))

114+ Expect(port).To(Equal(in.expectedPort))

115+ },

116+ Entry("when no port is specified", splitHostPortTableInput{

117+ hostport: "foo.bar",

118+ expectedHost: "foo.bar",

119+ expectedPort: "",

120+ }),

121+ Entry("with a valid port specified", splitHostPortTableInput{

122+ hostport: "foo.bar:8080",

123+ expectedHost: "foo.bar",

124+ expectedPort: "8080",

125+ }),

126+ Entry("with an invalid port specified", splitHostPortTableInput{

127+ hostport: "foo.bar:808a",

128+ expectedHost: "foo.bar:808a",

129+ expectedPort: "",

130+ }),

131+ Entry("with a wildcard port specified", splitHostPortTableInput{

132+ hostport: "foo.bar:*",

133+ expectedHost: "foo.bar",

134+ expectedPort: "*",

135+ }),

136+ Entry("when the host is specified with brackets", splitHostPortTableInput{

137+ hostport: "[foo.bar]",

138+ expectedHost: "foo.bar",

139+ expectedPort: "",

140+ }),

141+ )

142+ })

143+})

pkg/requests/util/util.go+5

@@ -52,3 +52,8 @@ func IsProxied(req *http.Request) bool {

52 }

53 return scope.ReverseProxy

54}

55+

56+func IsForwardedRequest(req *http.Request) bool {

57+ return IsProxied(req) &&

58+ req.Host != GetRequestHost(req)

59+}