From 14fd934b32dad88b3b08fdeaf44f26c4a23ecf29 Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sat, 7 Nov 2020 11:19:30 -0800 Subject: [PATCH 1/3] Flip `--skip-auth-strip-headers` to `true` by default --- CHANGELOG.md | 2 ++ docs/docs/configuration/overview.md | 2 +- pkg/apis/options/legacy_options.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87666b87..cbbf89bf 100644 --- 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) diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md index b63c6922..76e7400f 100644 --- 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 | diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 39352135..4dc52ae0 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -159,7 +159,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 } From 1c26539ef06b227edec7703c9c9fbdeb4a2798ad Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sat, 7 Nov 2020 12:33:37 -0800 Subject: [PATCH 2/3] Align tests to SkipAuthStripHeaders default --- pkg/apis/options/legacy_options.go | 5 +- pkg/apis/options/legacy_options_test.go | 155 ++++++++++++++++++------ 2 files changed, 119 insertions(+), 41 deletions(-) diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 4dc52ae0..f9e55499 100644 --- 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(), diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index a00061a3..2e50edcc 100644 --- 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, }, }), ) From 7d6ff03d1395065caed136bc6fb0396963d798a5 Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sat, 7 Nov 2020 12:34:27 -0800 Subject: [PATCH 3/3] Fix X-Auth-Request-Preferred-Username in response headers --- pkg/apis/options/legacy_options.go | 44 +++++++++++++++++++----------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index f9e55499..88784e9b 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -203,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 { @@ -331,7 +334,7 @@ func getPreferredUsernameHeader() Header { } } -func getXAuthRequestHeaders(passAccessToken bool) []Header { +func getXAuthRequestHeaders() []Header { headers := []Header{ { Name: "X-Auth-Request-User", @@ -353,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{ @@ -366,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", + }, + }, + }, + } +}