Merge pull request #904 from grnhse/default-strip-request-headers

Flip `--skip-auth-strip-headers` to `true` by default
2025-01-24 05:26:55 +02:00 · 2020-11-08 12:04:47 +00:00 · 2020-11-08 12:04:47 +00:00 · 289f4f3c5f
commit 289f4f3c5f
parent 5cba642938 7d6ff03d13
4 changed files with 151 additions and 59 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -4,6 +4,7 @@

 ## Important Notes

+- [#826](https://github.com/oauth2-proxy/oauth2-proxy/pull/826) `skip-auth-strip-headers` now applies to all requests, not just those where authentication would be skipped.
 - [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) `--skip-auth-route` is (almost) backwards compatible with `--skip-auth-regex`
  - We are marking `--skip-auth-regex` as DEPRECATED and will remove it in the next major version.
  - If your regex contains an `=` and you want it for all methods, you will need to add a leading `=` (this is the area where `--skip-auth-regex` doesn't port perfectly)
@ -32,6 +33,7 @@

 ## Changes since v6.1.1

+- [#904](https://github.com/oauth2-proxy/oauth2-proxy/pull/904) Set `skip-auth-strip-headers` to `true` by default (@NickMeves)
 - [#826](https://github.com/oauth2-proxy/oauth2-proxy/pull/826) Integrate new header injectors into project (@JoelSpeed)
 - [#898](https://github.com/oauth2-proxy/oauth2-proxy/pull/898) Migrate documentation to Docusaurus (@JoelSpeed)
 - [#754](https://github.com/oauth2-proxy/oauth2-proxy/pull/754) Azure token refresh (@codablock)
--- a/docs/docs/configuration/overview.md
+++ b/docs/docs/configuration/overview.md
@ -116,7 +116,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/
 | `--skip-auth-preflight` | bool | will skip authentication for OPTIONS requests | false |
 | `--skip-auth-regex` | string \| list | (DEPRECATED for `--skip-auth-route`) bypass authentication for requests paths that match (may be given multiple times) | |
 | `--skip-auth-route` | string \| list | bypass authentication for requests that match the method & path. Format: method=path_regex OR path_regex alone for all methods | |
-| `--skip-auth-strip-headers` | bool | strips `X-Forwarded-*` style authentication headers & `Authorization` header if they would be set by oauth2-proxy for allowlisted requests (`--skip-auth-route`, `--skip-auth-regex`, `--skip-auth-preflight`, `--trusted-ip`) | false |
+| `--skip-auth-strip-headers` | bool | strips `X-Forwarded-*` style authentication headers & `Authorization` header if they would be set by oauth2-proxy | true |
 | `--skip-jwt-bearer-tokens` | bool | will skip requests that have verified JWT bearer tokens (the token must have [`aud`](https://en.wikipedia.org/wiki/JSON_Web_Token#Standard_fields) that matches this client id or one of the extras from `extra-jwt-issuers`) | false |
 | `--skip-oidc-discovery` | bool | bypass OIDC endpoint discovery. `--login-url`, `--redeem-url` and `--oidc-jwks-url` must be configured in this case | false |
 | `--skip-provider-button` | bool | will skip sign-in-page to directly reach the next step: oauth/start | false |
--- a/pkg/apis/options/legacy_options.go
+++ b/pkg/apis/options/legacy_options.go
@ -31,8 +31,9 @@ func NewLegacyOptions() *LegacyOptions {
 		},

 		LegacyHeaders: LegacyHeaders{
-			PassBasicAuth:   true,
-			PassUserHeaders: true,
+			PassBasicAuth:        true,
+			PassUserHeaders:      true,
+			SkipAuthStripHeaders: true,
 		},

 		Options: *NewOptions(),
@ -159,7 +160,7 @@ func legacyHeadersFlagSet() *pflag.FlagSet {

 	flagSet.Bool("prefer-email-to-user", false, "Prefer to use the Email address as the Username when passing information to upstream. Will only use Username if Email is unavailable, eg. htaccess authentication. Used in conjunction with -pass-basic-auth and -pass-user-headers")
 	flagSet.String("basic-auth-password", "", "the password to set when passing the HTTP Basic Auth header")
-	flagSet.Bool("skip-auth-strip-headers", false, "strips X-Forwarded-* style authentication headers & Authorization header if they would be set by oauth2-proxy for request paths in --skip-auth-regex")
+	flagSet.Bool("skip-auth-strip-headers", true, "strips X-Forwarded-* style authentication headers & Authorization header if they would be set by oauth2-proxy")

 	return flagSet
 }
@ -202,7 +203,10 @@ func (l *LegacyHeaders) getResponseHeaders() []Header {
 	responseHeaders := []Header{}

 	if l.SetXAuthRequest {
-		responseHeaders = append(responseHeaders, getXAuthRequestHeaders(l.PassAccessToken)...)
+		responseHeaders = append(responseHeaders, getXAuthRequestHeaders()...)
+		if l.PassAccessToken {
+			responseHeaders = append(responseHeaders, getXAuthRequestAccessTokenHeader())
+		}
 	}

 	if l.SetBasicAuth {
@ -330,7 +334,7 @@ func getPreferredUsernameHeader() Header {
 	}
 }

-func getXAuthRequestHeaders(passAccessToken bool) []Header {
+func getXAuthRequestHeaders() []Header {
 	headers := []Header{
 		{
 			Name: "X-Auth-Request-User",
@ -352,7 +356,16 @@ func getXAuthRequestHeaders(passAccessToken bool) []Header {
 				},
 			},
 		},
-		getPreferredUsernameHeader(),
+		{
+			Name: "X-Auth-Request-Preferred-Username",
+			Values: []HeaderValue{
+				{
+					ClaimSource: &ClaimSource{
+						Claim: "preferred_username",
+					},
+				},
+			},
+		},
 		{
 			Name: "X-Auth-Request-Groups",
 			Values: []HeaderValue{
@ -365,18 +378,18 @@ func getXAuthRequestHeaders(passAccessToken bool) []Header {
 		},
 	}

-	if passAccessToken {
-		headers = append(headers, Header{
-			Name: "X-Auth-Request-Access-Token",
-			Values: []HeaderValue{
-				{
-					ClaimSource: &ClaimSource{
-						Claim: "access_token",
-					},
-				},
-			},
-		})
-	}
-
 	return headers
 }
+
+func getXAuthRequestAccessTokenHeader() Header {
+	return Header{
+		Name: "X-Auth-Request-Access-Token",
+		Values: []HeaderValue{
+			{
+				ClaimSource: &ClaimSource{
+					Claim: "access_token",
+				},
+			},
+		},
+	}
+}
--- a/pkg/apis/options/legacy_options_test.go
+++ b/pkg/apis/options/legacy_options_test.go
@ -61,7 +61,7 @@ var _ = Describe("Legacy Options", func() {
 			opts.InjectRequestHeaders = []Header{
 				{
 					Name:                 "X-Forwarded-Groups",
-					PreserveRequestValue: true,
+					PreserveRequestValue: false,
 					Values: []HeaderValue{
 						{
 							ClaimSource: &ClaimSource{
@ -72,7 +72,7 @@ var _ = Describe("Legacy Options", func() {
 				},
 				{
 					Name:                 "X-Forwarded-User",
-					PreserveRequestValue: true,
+					PreserveRequestValue: false,
 					Values: []HeaderValue{
 						{
 							ClaimSource: &ClaimSource{
@ -83,7 +83,7 @@ var _ = Describe("Legacy Options", func() {
 				},
 				{
 					Name:                 "X-Forwarded-Email",
-					PreserveRequestValue: true,
+					PreserveRequestValue: false,
 					Values: []HeaderValue{
 						{
 							ClaimSource: &ClaimSource{
@ -94,7 +94,7 @@ var _ = Describe("Legacy Options", func() {
 				},
 				{
 					Name:                 "X-Forwarded-Preferred-Username",
-					PreserveRequestValue: true,
+					PreserveRequestValue: false,
 					Values: []HeaderValue{
 						{
 							ClaimSource: &ClaimSource{
@ -277,7 +277,7 @@ var _ = Describe("Legacy Options", func() {

 		xForwardedUser := Header{
 			Name:                 "X-Forwarded-User",
-			PreserveRequestValue: true,
+			PreserveRequestValue: false,
 			Values: []HeaderValue{
 				{
 					ClaimSource: &ClaimSource{
@ -289,7 +289,7 @@ var _ = Describe("Legacy Options", func() {

 		xForwardedEmail := Header{
 			Name:                 "X-Forwarded-Email",
-			PreserveRequestValue: true,
+			PreserveRequestValue: false,
 			Values: []HeaderValue{
 				{
 					ClaimSource: &ClaimSource{
@ -301,7 +301,7 @@ var _ = Describe("Legacy Options", func() {

 		xForwardedGroups := Header{
 			Name:                 "X-Forwarded-Groups",
-			PreserveRequestValue: true,
+			PreserveRequestValue: false,
 			Values: []HeaderValue{
 				{
 					ClaimSource: &ClaimSource{
@ -313,7 +313,7 @@ var _ = Describe("Legacy Options", func() {

 		xForwardedPreferredUsername := Header{
 			Name:                 "X-Forwarded-Preferred-Username",
-			PreserveRequestValue: true,
+			PreserveRequestValue: false,
 			Values: []HeaderValue{
 				{
 					ClaimSource: &ClaimSource{
@ -325,7 +325,7 @@ var _ = Describe("Legacy Options", func() {

 		basicAuthHeader := Header{
 			Name:                 "Authorization",
-			PreserveRequestValue: true,
+			PreserveRequestValue: false,
 			Values: []HeaderValue{
 				{
 					ClaimSource: &ClaimSource{
@ -340,7 +340,7 @@ var _ = Describe("Legacy Options", func() {

 		xForwardedUserWithEmail := Header{
 			Name:                 "X-Forwarded-User",
-			PreserveRequestValue: true,
+			PreserveRequestValue: false,
 			Values: []HeaderValue{
 				{
 					ClaimSource: &ClaimSource{
@ -350,9 +350,21 @@ var _ = Describe("Legacy Options", func() {
 			},
 		}

+		xForwardedAccessToken := Header{
+			Name:                 "X-Forwarded-Access-Token",
+			PreserveRequestValue: false,
+			Values: []HeaderValue{
+				{
+					ClaimSource: &ClaimSource{
+						Claim: "access_token",
+					},
+				},
+			},
+		}
+
 		basicAuthHeaderWithEmail := Header{
 			Name:                 "Authorization",
-			PreserveRequestValue: true,
+			PreserveRequestValue: false,
 			Values: []HeaderValue{
 				{
 					ClaimSource: &ClaimSource{
@ -401,13 +413,13 @@ var _ = Describe("Legacy Options", func() {
 			},
 		}

-		xForwardedAccessToken := Header{
-			Name:                 "X-Forwarded-Access-Token",
-			PreserveRequestValue: true,
+		xAuthRequestPreferredUsername := Header{
+			Name:                 "X-Auth-Request-Preferred-Username",
+			PreserveRequestValue: false,
 			Values: []HeaderValue{
 				{
 					ClaimSource: &ClaimSource{
-						Claim: "access_token",
+						Claim: "preferred_username",
 					},
 				},
 			},
@ -427,7 +439,7 @@ var _ = Describe("Legacy Options", func() {

 		authorizationHeader := Header{
 			Name:                 "Authorization",
-			PreserveRequestValue: true,
+			PreserveRequestValue: false,
 			Values: []HeaderValue{
 				{
 					ClaimSource: &ClaimSource{
@ -457,7 +469,7 @@ var _ = Describe("Legacy Options", func() {

 					PreferEmailToUser:    false,
 					BasicAuthPassword:    "",
-					SkipAuthStripHeaders: false,
+					SkipAuthStripHeaders: true,
 				},
 				expectedRequestHeaders:  []Header{},
 				expectedResponseHeaders: []Header{},
@ -475,7 +487,7 @@ var _ = Describe("Legacy Options", func() {

 					PreferEmailToUser:    false,
 					BasicAuthPassword:    basicAuthSecret,
-					SkipAuthStripHeaders: false,
+					SkipAuthStripHeaders: true,
 				},
 				expectedRequestHeaders: []Header{
 					xForwardedUser,
@ -485,10 +497,10 @@ var _ = Describe("Legacy Options", func() {
 					basicAuthHeader,
 				},
 				expectedResponseHeaders: []Header{
-					withPreserveRequestValue(basicAuthHeader, false),
+					basicAuthHeader,
 				},
 			}),
-			Entry("with basic auth enabled and skipAuthStripHeaders", legacyHeadersTableInput{
+			Entry("with basic auth enabled and skipAuthStripHeaders disabled", legacyHeadersTableInput{
 				legacyHeaders: &LegacyHeaders{
 					PassBasicAuth:     true,
 					PassAccessToken:   false,
@ -501,17 +513,17 @@ var _ = Describe("Legacy Options", func() {

 					PreferEmailToUser:    false,
 					BasicAuthPassword:    basicAuthSecret,
-					SkipAuthStripHeaders: true,
+					SkipAuthStripHeaders: false,
 				},
 				expectedRequestHeaders: []Header{
-					withPreserveRequestValue(xForwardedUser, false),
-					withPreserveRequestValue(xForwardedEmail, false),
-					withPreserveRequestValue(xForwardedGroups, false),
-					withPreserveRequestValue(xForwardedPreferredUsername, false),
-					withPreserveRequestValue(basicAuthHeader, false),
+					withPreserveRequestValue(xForwardedUser, true),
+					withPreserveRequestValue(xForwardedEmail, true),
+					withPreserveRequestValue(xForwardedGroups, true),
+					withPreserveRequestValue(xForwardedPreferredUsername, true),
+					withPreserveRequestValue(basicAuthHeader, true),
 				},
 				expectedResponseHeaders: []Header{
-					withPreserveRequestValue(basicAuthHeader, false),
+					basicAuthHeader,
 				},
 			}),
 			Entry("with basic auth enabled and preferEmailToUser", legacyHeadersTableInput{
@ -527,7 +539,7 @@ var _ = Describe("Legacy Options", func() {

 					PreferEmailToUser:    true,
 					BasicAuthPassword:    basicAuthSecret,
-					SkipAuthStripHeaders: false,
+					SkipAuthStripHeaders: true,
 				},
 				expectedRequestHeaders: []Header{
 					xForwardedUserWithEmail,
@ -536,7 +548,7 @@ var _ = Describe("Legacy Options", func() {
 					basicAuthHeaderWithEmail,
 				},
 				expectedResponseHeaders: []Header{
-					withPreserveRequestValue(basicAuthHeaderWithEmail, false),
+					basicAuthHeaderWithEmail,
 				},
 			}),
 			Entry("with basic auth enabled and passUserHeaders", legacyHeadersTableInput{
@ -552,7 +564,7 @@ var _ = Describe("Legacy Options", func() {

 					PreferEmailToUser:    false,
 					BasicAuthPassword:    basicAuthSecret,
-					SkipAuthStripHeaders: false,
+					SkipAuthStripHeaders: true,
 				},
 				expectedRequestHeaders: []Header{
 					xForwardedUser,
@ -562,7 +574,7 @@ var _ = Describe("Legacy Options", func() {
 					basicAuthHeader,
 				},
 				expectedResponseHeaders: []Header{
-					withPreserveRequestValue(basicAuthHeader, false),
+					basicAuthHeader,
 				},
 			}),
 			Entry("with passUserHeaders", legacyHeadersTableInput{
@ -578,7 +590,7 @@ var _ = Describe("Legacy Options", func() {

 					PreferEmailToUser:    false,
 					BasicAuthPassword:    "",
-					SkipAuthStripHeaders: false,
+					SkipAuthStripHeaders: true,
 				},
 				expectedRequestHeaders: []Header{
 					xForwardedUser,
@ -588,6 +600,29 @@ var _ = Describe("Legacy Options", func() {
 				},
 				expectedResponseHeaders: []Header{},
 			}),
+			Entry("with passUserHeaders and SkipAuthStripHeaders disabled", legacyHeadersTableInput{
+				legacyHeaders: &LegacyHeaders{
+					PassBasicAuth:     false,
+					PassAccessToken:   false,
+					PassUserHeaders:   true,
+					PassAuthorization: false,
+
+					SetBasicAuth:     false,
+					SetXAuthRequest:  false,
+					SetAuthorization: false,
+
+					PreferEmailToUser:    false,
+					BasicAuthPassword:    "",
+					SkipAuthStripHeaders: false,
+				},
+				expectedRequestHeaders: []Header{
+					withPreserveRequestValue(xForwardedUser, true),
+					withPreserveRequestValue(xForwardedEmail, true),
+					withPreserveRequestValue(xForwardedGroups, true),
+					withPreserveRequestValue(xForwardedPreferredUsername, true),
+				},
+				expectedResponseHeaders: []Header{},
+			}),
 			Entry("with setXAuthRequest", legacyHeadersTableInput{
 				legacyHeaders: &LegacyHeaders{
 					PassBasicAuth:     false,
@ -601,14 +636,14 @@ var _ = Describe("Legacy Options", func() {

 					PreferEmailToUser:    false,
 					BasicAuthPassword:    "",
-					SkipAuthStripHeaders: false,
+					SkipAuthStripHeaders: true,
 				},
 				expectedRequestHeaders: []Header{},
 				expectedResponseHeaders: []Header{
 					xAuthRequestUser,
 					xAuthRequestEmail,
 					xAuthRequestGroups,
-					withPreserveRequestValue(xForwardedPreferredUsername, false),
+					xAuthRequestPreferredUsername,
 				},
 			}),
 			Entry("with passAccessToken", legacyHeadersTableInput{
@ -624,7 +659,7 @@ var _ = Describe("Legacy Options", func() {

 					PreferEmailToUser:    false,
 					BasicAuthPassword:    "",
-					SkipAuthStripHeaders: false,
+					SkipAuthStripHeaders: true,
 				},
 				expectedRequestHeaders: []Header{
 					xForwardedAccessToken,
@ -644,7 +679,7 @@ var _ = Describe("Legacy Options", func() {

 					PreferEmailToUser:    false,
 					BasicAuthPassword:    "",
-					SkipAuthStripHeaders: false,
+					SkipAuthStripHeaders: true,
 				},
 				expectedRequestHeaders: []Header{
 					xForwardedAccessToken,
@ -653,11 +688,53 @@ var _ = Describe("Legacy Options", func() {
 					xAuthRequestUser,
 					xAuthRequestEmail,
 					xAuthRequestGroups,
-					withPreserveRequestValue(xForwardedPreferredUsername, false),
+					xAuthRequestPreferredUsername,
 					xAuthRequestAccessToken,
 				},
 			}),
+			Entry("with passAcessToken and SkipAuthStripHeaders disabled", legacyHeadersTableInput{
+				legacyHeaders: &LegacyHeaders{
+					PassBasicAuth:     false,
+					PassAccessToken:   true,
+					PassUserHeaders:   false,
+					PassAuthorization: false,
+
+					SetBasicAuth:     false,
+					SetXAuthRequest:  false,
+					SetAuthorization: false,
+
+					PreferEmailToUser:    false,
+					BasicAuthPassword:    "",
+					SkipAuthStripHeaders: false,
+				},
+				expectedRequestHeaders: []Header{
+					withPreserveRequestValue(xForwardedAccessToken, true),
+				},
+				expectedResponseHeaders: []Header{},
+			}),
 			Entry("with authorization headers", legacyHeadersTableInput{
+				legacyHeaders: &LegacyHeaders{
+					PassBasicAuth:     false,
+					PassAccessToken:   false,
+					PassUserHeaders:   false,
+					PassAuthorization: true,
+
+					SetBasicAuth:     false,
+					SetXAuthRequest:  false,
+					SetAuthorization: true,
+
+					PreferEmailToUser:    false,
+					BasicAuthPassword:    "",
+					SkipAuthStripHeaders: true,
+				},
+				expectedRequestHeaders: []Header{
+					authorizationHeader,
+				},
+				expectedResponseHeaders: []Header{
+					authorizationHeader,
+				},
+			}),
+			Entry("with authorization headers and SkipAuthStripHeaders disabled", legacyHeadersTableInput{
 				legacyHeaders: &LegacyHeaders{
 					PassBasicAuth:     false,
 					PassAccessToken:   false,
@ -673,10 +750,10 @@ var _ = Describe("Legacy Options", func() {
 					SkipAuthStripHeaders: false,
 				},
 				expectedRequestHeaders: []Header{
-					authorizationHeader,
+					withPreserveRequestValue(authorizationHeader, true),
 				},
 				expectedResponseHeaders: []Header{
-					withPreserveRequestValue(authorizationHeader, false),
+					authorizationHeader,
 				},
 			}),
 		)