Log IsValidRedirect violations and do a final safety call

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 ## Important Notes
 
+- [#964](https://github.com/oauth2-proxy/oauth2-proxy/pull/964) Redirect URL generation will attempt secondary strategies
+  in the priority chain if any fail the `IsValidRedirect` security check. Previously any failures fell back to `/`.
 - [#953](https://github.com/oauth2-proxy/oauth2-proxy/pull/953) Keycloak will now use `--profile-url` if set for the userinfo endpoint
   instead of `--validate-url`. `--validate-url` will still work for backwards compatibility.
 - [#957](https://github.com/oauth2-proxy/oauth2-proxy/pull/957) To use X-Forwarded-{Proto,Host,Uri} on redirect detection, `--reverse-proxy` must be `true`.
@@ -36,6 +38,11 @@
 
 ## Breaking Changes
 
+- [#964](https://github.com/oauth2-proxy/oauth2-proxy/pull/964) `--reverse-proxy` must be true to trust `X-Forwarded-*` headers as canonical.
+  These are used throughout the application in redirect URLs, cookie domains and host logging logic. These are the headers:
+  - `X-Forwarded-Proto` instead of `req.URL.Scheme`
+  - `X-Forwarded-Host` instead of `req.Host`
+  - `X-Forwarded-Uri` instead of `req.URL.RequestURI()`
 - [#953](https://github.com/oauth2-proxy/oauth2-proxy/pull/953) In config files & envvar configs, `keycloak_group` is now the plural `keycloak_groups`.
   Flag configs are still `--keycloak-group` but it can be passed multiple times.
 - [#911](https://github.com/oauth2-proxy/oauth2-proxy/pull/911) Specifying a non-existent provider will cause OAuth2-Proxy to fail on startup instead of defaulting to "google".
@@ -60,6 +67,7 @@
 ## Changes since v6.1.1
 
 - [#995](https://github.com/oauth2-proxy/oauth2-proxy/pull/995) Add Security Policy (@JoelSpeed)
+- [#964](https://github.com/oauth2-proxy/oauth2-proxy/pull/964) Require `--reverse-proxy` true to trust `X-Forwareded-*` type headers (@NickMeves)
 - [#970](https://github.com/oauth2-proxy/oauth2-proxy/pull/970) Fix joined cookie name for those containing underline in the suffix (@peppered)
 - [#953](https://github.com/oauth2-proxy/oauth2-proxy/pull/953) Migrate Keycloak to EnrichSession & support multiple groups for authorization (@NickMeves)
 - [#957](https://github.com/oauth2-proxy/oauth2-proxy/pull/957) Use X-Forwarded-{Proto,Host,Uri} on redirect as last resort (@linuxgemini)

oauthproxy.go

@@ -955,7 +955,9 @@ func (p *OAuthProxy) getAppRedirect(req *http.Request) (string, error) {
 		p.getXForwardedHeadersRedirect,
 		p.getURIRedirect,
 	} {
-		if redirect := rdGetter(req); redirect != "" {
+		redirect := rdGetter(req)
+		// Call `p.IsValidRedirect` again here a final time to be safe
+		if redirect != "" && p.IsValidRedirect(redirect) {
 			return redirect, nil
 		}
 	}
@@ -972,24 +974,32 @@ func (p *OAuthProxy) hasProxyPrefix(path string) bool {
 	return strings.HasPrefix(path, fmt.Sprintf("%s/", p.ProxyPrefix))
 }
 
-// getRdQuerystringRedirect handles this getAppRedirect strategy:
+func (p *OAuthProxy) validateRedirect(redirect string, errorFormat string) string {
-// - `rd` querysting parameter
-func (p *OAuthProxy) getRdQuerystringRedirect(req *http.Request) string {
-	redirect := req.Form.Get("rd")
 	if p.IsValidRedirect(redirect) {
 		return redirect
 	}
+	if redirect != "" {
+		logger.Errorf(errorFormat, redirect)
+	}
 	return ""
 }
 
+// getRdQuerystringRedirect handles this getAppRedirect strategy:
+// - `rd` querysting parameter
+func (p *OAuthProxy) getRdQuerystringRedirect(req *http.Request) string {
+	return p.validateRedirect(
+		req.Form.Get("rd"),
+		"Invalid redirect provided in rd querystring parameter: %s",
+	)
+}
+
 // getXAuthRequestRedirect handles this getAppRedirect strategy:
 // - `X-Auth-Request-Redirect` Header
 func (p *OAuthProxy) getXAuthRequestRedirect(req *http.Request) string {
-	redirect := req.Header.Get("X-Auth-Request-Redirect")
+	return p.validateRedirect(
-	if p.IsValidRedirect(redirect) {
+		req.Header.Get("X-Auth-Request-Redirect"),
-		return redirect
+		"Invalid redirect provided in X-Auth-Request-Redirect header: %s",
-	}
+	)
-	return ""
 }
 
 // getXForwardedHeadersRedirect handles these getAppRedirect strategies:
@@ -1012,10 +1022,8 @@ func (p *OAuthProxy) getXForwardedHeadersRedirect(req *http.Request) string {
 		uri,
 	)
 
-	if p.IsValidRedirect(redirect) {
+	return p.validateRedirect(redirect,
-		return redirect
+		"Invalid redirect generated from X-Forwarded-* headers: %s")
-	}
-	return ""
 }
 
 // getURIRedirect handles these getAppRedirect strategies:
@@ -1023,8 +1031,11 @@ func (p *OAuthProxy) getXForwardedHeadersRedirect(req *http.Request) string {
 // - `req.URL.RequestURI` if not under the ProxyPath (i.e. /oauth2/*)
 // - `/`
 func (p *OAuthProxy) getURIRedirect(req *http.Request) string {
-	redirect := requestutil.GetRequestURI(req)
+	redirect := p.validateRedirect(
-	if !p.IsValidRedirect(redirect) {
+		requestutil.GetRequestURI(req),
+		"Invalid redirect generated from X-Forwarded-Uri header: %s",
+	)
+	if redirect == "" {
 		redirect = req.URL.RequestURI()
 	}