From 2dc0d1e7ee914be1fc3d4d2fd3a95a179e00a2ac Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 29 Jul 2020 20:08:46 +0100 Subject: [PATCH 1/7] Create LegacyHeaders struct and conversion to new Headers --- pkg/apis/options/legacy_options.go | 261 ++++++++++++++++++++++++ pkg/apis/options/legacy_options_test.go | 49 +++++ pkg/apis/options/options.go | 32 +-- 3 files changed, 314 insertions(+), 28 deletions(-) diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 2fe55ddd..39352135 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -1,6 +1,7 @@ package options import ( + "encoding/base64" "fmt" "net/url" "strconv" @@ -15,6 +16,9 @@ type LegacyOptions struct { // Legacy options related to upstream servers LegacyUpstreams LegacyUpstreams `cfg:",squash"` + // Legacy options for injecting request/response headers + LegacyHeaders LegacyHeaders `cfg:",squash"` + Options Options `cfg:",squash"` } @@ -26,6 +30,11 @@ func NewLegacyOptions() *LegacyOptions { FlushInterval: time.Duration(1) * time.Second, }, + LegacyHeaders: LegacyHeaders{ + PassBasicAuth: true, + PassUserHeaders: true, + }, + Options: *NewOptions(), } } @@ -37,6 +46,7 @@ func (l *LegacyOptions) ToOptions() (*Options, error) { } l.Options.UpstreamServers = upstreams + l.Options.InjectRequestHeaders, l.Options.InjectResponseHeaders = l.LegacyHeaders.convert() return &l.Options, nil } @@ -119,3 +129,254 @@ func (l *LegacyUpstreams) convert() (Upstreams, error) { return upstreams, nil } + +type LegacyHeaders struct { + PassBasicAuth bool `flag:"pass-basic-auth" cfg:"pass_basic_auth"` + PassAccessToken bool `flag:"pass-access-token" cfg:"pass_access_token"` + PassUserHeaders bool `flag:"pass-user-headers" cfg:"pass_user_headers"` + PassAuthorization bool `flag:"pass-authorization-header" cfg:"pass_authorization_header"` + + SetBasicAuth bool `flag:"set-basic-auth" cfg:"set_basic_auth"` + SetXAuthRequest bool `flag:"set-xauthrequest" cfg:"set_xauthrequest"` + SetAuthorization bool `flag:"set-authorization-header" cfg:"set_authorization_header"` + + PreferEmailToUser bool `flag:"prefer-email-to-user" cfg:"prefer_email_to_user"` + BasicAuthPassword string `flag:"basic-auth-password" cfg:"basic_auth_password"` + SkipAuthStripHeaders bool `flag:"skip-auth-strip-headers" cfg:"skip_auth_strip_headers"` +} + +func legacyHeadersFlagSet() *pflag.FlagSet { + flagSet := pflag.NewFlagSet("headers", pflag.ExitOnError) + + flagSet.Bool("pass-basic-auth", true, "pass HTTP Basic Auth, X-Forwarded-User and X-Forwarded-Email information to upstream") + flagSet.Bool("pass-access-token", false, "pass OAuth access_token to upstream via X-Forwarded-Access-Token header") + flagSet.Bool("pass-user-headers", true, "pass X-Forwarded-User and X-Forwarded-Email information to upstream") + flagSet.Bool("pass-authorization-header", false, "pass the Authorization Header to upstream") + + flagSet.Bool("set-basic-auth", false, "set HTTP Basic Auth information in response (useful in Nginx auth_request mode)") + flagSet.Bool("set-xauthrequest", false, "set X-Auth-Request-User and X-Auth-Request-Email response headers (useful in Nginx auth_request mode)") + flagSet.Bool("set-authorization-header", false, "set Authorization response headers (useful in Nginx auth_request mode)") + + 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") + + return flagSet +} + +// convert takes the legacy request/response headers and converts them to +// the new format for InjectRequestHeaders and InjectResponseHeaders +func (l *LegacyHeaders) convert() ([]Header, []Header) { + return l.getRequestHeaders(), l.getResponseHeaders() +} + +func (l *LegacyHeaders) getRequestHeaders() []Header { + requestHeaders := []Header{} + + if l.PassBasicAuth && l.BasicAuthPassword != "" { + requestHeaders = append(requestHeaders, getBasicAuthHeader(l.PreferEmailToUser, l.BasicAuthPassword)) + } + + // In the old implementation, PassUserHeaders is a subset of PassBasicAuth + if l.PassBasicAuth || l.PassUserHeaders { + requestHeaders = append(requestHeaders, getPassUserHeaders(l.PreferEmailToUser)...) + requestHeaders = append(requestHeaders, getPreferredUsernameHeader()) + } + + if l.PassAccessToken { + requestHeaders = append(requestHeaders, getPassAccessTokenHeader()) + } + + if l.PassAuthorization { + requestHeaders = append(requestHeaders, getAuthorizationHeader()) + } + + for i := range requestHeaders { + requestHeaders[i].PreserveRequestValue = !l.SkipAuthStripHeaders + } + + return requestHeaders +} + +func (l *LegacyHeaders) getResponseHeaders() []Header { + responseHeaders := []Header{} + + if l.SetXAuthRequest { + responseHeaders = append(responseHeaders, getXAuthRequestHeaders(l.PassAccessToken)...) + } + + if l.SetBasicAuth { + responseHeaders = append(responseHeaders, getBasicAuthHeader(l.PreferEmailToUser, l.BasicAuthPassword)) + } + + if l.SetAuthorization { + responseHeaders = append(responseHeaders, getAuthorizationHeader()) + } + + return responseHeaders +} + +func getBasicAuthHeader(preferEmailToUser bool, basicAuthPassword string) Header { + claim := "user" + if preferEmailToUser { + claim = "email" + } + + return Header{ + Name: "Authorization", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: claim, + BasicAuthPassword: &SecretSource{ + Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthPassword))), + }, + }, + }, + }, + } +} + +func getPassUserHeaders(preferEmailToUser bool) []Header { + headers := []Header{ + { + Name: "X-Forwarded-Groups", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "groups", + }, + }, + }, + }, + } + + if preferEmailToUser { + return append(headers, + Header{ + Name: "X-Forwarded-User", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + }, + }, + }, + }, + ) + } + + return append(headers, + Header{ + Name: "X-Forwarded-User", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "user", + }, + }, + }, + }, + Header{ + Name: "X-Forwarded-Email", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + }, + }, + }, + }, + ) +} + +func getPassAccessTokenHeader() Header { + return Header{ + Name: "X-Forwarded-Access-Token", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "access_token", + }, + }, + }, + } +} + +func getAuthorizationHeader() Header { + return Header{ + Name: "Authorization", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "id_token", + Prefix: "Bearer ", + }, + }, + }, + } +} + +func getPreferredUsernameHeader() Header { + return Header{ + Name: "X-Forwarded-Preferred-Username", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "preferred_username", + }, + }, + }, + } +} + +func getXAuthRequestHeaders(passAccessToken bool) []Header { + headers := []Header{ + { + Name: "X-Auth-Request-User", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "user", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Email", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + }, + }, + }, + }, + getPreferredUsernameHeader(), + { + Name: "X-Auth-Request-Groups", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "groups", + }, + }, + }, + }, + } + + if passAccessToken { + headers = append(headers, Header{ + Name: "X-Auth-Request-Access-Token", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "access_token", + }, + }, + }, + }) + } + + return headers +} diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index 80ad17b9..fb250590 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -57,6 +57,55 @@ var _ = Describe("Legacy Options", func() { }, } + opts.InjectRequestHeaders = []Header{ + { + Name: "X-Forwarded-Groups", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "groups", + }, + }, + }, + }, + { + Name: "X-Forwarded-User", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "user", + }, + }, + }, + }, + { + Name: "X-Forwarded-Email", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + }, + }, + }, + }, + { + Name: "X-Forwarded-Preferred-Username", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "preferred_username", + }, + }, + }, + }, + } + + opts.InjectResponseHeaders = []Header{} + converted, err := legacyOpts.ToOptions() Expect(err).ToNot(HaveOccurred()) Expect(converted).To(Equal(opts)) diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index a79d1520..f6c13abb 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -65,22 +65,15 @@ type Options struct { // TODO(JoelSpeed): Rename when legacy config is removed UpstreamServers Upstreams `cfg:",internal"` + InjectRequestHeaders []Header `cfg:",internal"` + InjectResponseHeaders []Header `cfg:",internal"` + SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex"` SkipAuthRoutes []string `flag:"skip-auth-route" cfg:"skip_auth_routes"` - SkipAuthStripHeaders bool `flag:"skip-auth-strip-headers" cfg:"skip_auth_strip_headers"` SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"` ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"` - PassBasicAuth bool `flag:"pass-basic-auth" cfg:"pass_basic_auth"` - SetBasicAuth bool `flag:"set-basic-auth" cfg:"set_basic_auth"` - PreferEmailToUser bool `flag:"prefer-email-to-user" cfg:"prefer_email_to_user"` - BasicAuthPassword string `flag:"basic-auth-password" cfg:"basic_auth_password"` - PassAccessToken bool `flag:"pass-access-token" cfg:"pass_access_token"` SkipProviderButton bool `flag:"skip-provider-button" cfg:"skip_provider_button"` - PassUserHeaders bool `flag:"pass-user-headers" cfg:"pass_user_headers"` SSLInsecureSkipVerify bool `flag:"ssl-insecure-skip-verify" cfg:"ssl_insecure_skip_verify"` - SetXAuthRequest bool `flag:"set-xauthrequest" cfg:"set_xauthrequest"` - SetAuthorization bool `flag:"set-authorization-header" cfg:"set_authorization_header"` - PassAuthorization bool `flag:"pass-authorization-header" cfg:"pass_authorization_header"` SkipAuthPreflight bool `flag:"skip-auth-preflight" cfg:"skip_auth_preflight"` // These options allow for other providers besides Google, with @@ -151,15 +144,7 @@ func NewOptions() *Options { Cookie: cookieDefaults(), Session: sessionOptionsDefaults(), AzureTenant: "common", - SetXAuthRequest: false, SkipAuthPreflight: false, - PassBasicAuth: true, - SetBasicAuth: false, - PassUserHeaders: true, - PassAccessToken: false, - SetAuthorization: false, - PassAuthorization: false, - PreferEmailToUser: false, Prompt: "", // Change to "login" when ApprovalPrompt officially deprecated ApprovalPrompt: "force", UserIDClaim: "email", @@ -183,18 +168,8 @@ func NewFlagSet() *pflag.FlagSet { flagSet.String("tls-cert-file", "", "path to certificate file") flagSet.String("tls-key-file", "", "path to private key file") flagSet.String("redirect-url", "", "the OAuth Redirect URL. ie: \"https://internalapp.yourcompany.com/oauth2/callback\"") - flagSet.Bool("set-xauthrequest", false, "set X-Auth-Request-User and X-Auth-Request-Email response headers (useful in Nginx auth_request mode)") - flagSet.Bool("pass-basic-auth", true, "pass HTTP Basic Auth, X-Forwarded-User and X-Forwarded-Email information to upstream") - flagSet.Bool("set-basic-auth", false, "set HTTP Basic Auth information in response (useful in Nginx auth_request mode)") - 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.Bool("pass-user-headers", true, "pass X-Forwarded-User and X-Forwarded-Email information to upstream") - flagSet.String("basic-auth-password", "", "the password to set when passing the HTTP Basic Auth header") - flagSet.Bool("pass-access-token", false, "pass OAuth access_token to upstream via X-Forwarded-Access-Token header") - flagSet.Bool("pass-authorization-header", false, "pass the Authorization Header to upstream") - flagSet.Bool("set-authorization-header", false, "set Authorization response headers (useful in Nginx auth_request mode)") flagSet.StringSlice("skip-auth-regex", []string{}, "(DEPRECATED for --skip-auth-route) bypass authentication for requests path's that match (may be given multiple times)") flagSet.StringSlice("skip-auth-route", []string{}, "bypass authentication for requests that match the method & path. Format: method=path_regex OR path_regex alone for all methods") - flagSet.Bool("skip-auth-strip-headers", false, "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`)") flagSet.Bool("skip-provider-button", false, "will skip sign-in-page to directly reach the next step: oauth/start") flagSet.Bool("skip-auth-preflight", false, "will skip authentication for OPTIONS requests") flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS providers") @@ -272,6 +247,7 @@ func NewFlagSet() *pflag.FlagSet { flagSet.AddFlagSet(cookieFlagSet()) flagSet.AddFlagSet(loggingFlagSet()) flagSet.AddFlagSet(legacyUpstreamsFlagSet()) + flagSet.AddFlagSet(legacyHeadersFlagSet()) return flagSet } From d26c65ba8d3f487eb4535e246e750ae664977c56 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 23 Jul 2020 10:47:31 +0100 Subject: [PATCH 2/7] Add validation for Headers struct --- pkg/validation/common.go | 44 +++++++++++++++++++++++++++ pkg/validation/header.go | 63 +++++++++++++++++++++++++++++++++++++++ pkg/validation/options.go | 2 ++ pkg/validation/utils.go | 11 +++++++ 4 files changed, 120 insertions(+) create mode 100644 pkg/validation/common.go create mode 100644 pkg/validation/header.go create mode 100644 pkg/validation/utils.go diff --git a/pkg/validation/common.go b/pkg/validation/common.go new file mode 100644 index 00000000..ccde822d --- /dev/null +++ b/pkg/validation/common.go @@ -0,0 +1,44 @@ +package validation + +import ( + "encoding/base64" + "fmt" + "os" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" +) + +func validateSecretSource(source options.SecretSource) string { + switch { + case len(source.Value) > 0 && source.FromEnv == "" && source.FromFile == "": + return validateSecretSourceValue(source.Value) + case len(source.Value) == 0 && source.FromEnv != "" && source.FromFile == "": + return validateSecretSourceEnv(source.FromEnv) + case len(source.Value) == 0 && source.FromEnv == "" && source.FromFile != "": + return validateSecretSourceFile(source.FromFile) + default: + return "multiple values specified for secret source: specify either value, fromEnv of fromFile" + } +} + +func validateSecretSourceValue(value []byte) string { + dst := make([]byte, len(value)) + if _, err := base64.StdEncoding.Decode(dst, value); err != nil { + return fmt.Sprintf("error decoding secret value: %v", err) + } + return "" +} + +func validateSecretSourceEnv(key string) string { + if value := os.Getenv(key); value == "" { + return fmt.Sprintf("error loading secret from environent: no value for for key %q", key) + } + return "" +} + +func validateSecretSourceFile(path string) string { + if _, err := os.Stat(path); err != nil { + return fmt.Sprintf("error loadig secret from file: %v", err) + } + return "" +} diff --git a/pkg/validation/header.go b/pkg/validation/header.go new file mode 100644 index 00000000..603feaf4 --- /dev/null +++ b/pkg/validation/header.go @@ -0,0 +1,63 @@ +package validation + +import ( + "fmt" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" +) + +func validateHeaders(headers []options.Header) []string { + msgs := []string{} + names := make(map[string]struct{}) + + for _, header := range headers { + msgs = append(msgs, validateHeader(header, names)...) + } + return msgs +} + +func validateHeader(header options.Header, names map[string]struct{}) []string { + msgs := []string{} + + if header.Name == "" { + msgs = append(msgs, "header has empty name: names are required for all headers") + } + + if _, ok := names[header.Name]; ok { + msgs = append(msgs, fmt.Sprintf("multiple headers found with name %q: header names must be unique", header.Name)) + } + names[header.Name] = struct{}{} + + for _, value := range header.Values { + msgs = append(msgs, + prefixValues(fmt.Sprintf("invalid header %q: invalid values: ", header.Name), + validateHeaderValue(header.Name, value)..., + )..., + ) + } + return msgs +} + +func validateHeaderValue(name string, value options.HeaderValue) []string { + switch { + case value.SecretSource != nil && value.ClaimSource == nil: + return []string{validateSecretSource(*value.SecretSource)} + case value.SecretSource == nil && value.ClaimSource != nil: + return validateHeaderValueClaimSource(*value.ClaimSource) + default: + return []string{"header value has multiple entries: only one entry per value is allowed"} + } +} + +func validateHeaderValueClaimSource(claim options.ClaimSource) []string { + msgs := []string{} + + if claim.Claim == "" { + msgs = append(msgs, "claim should not be empty") + } + + if claim.BasicAuthPassword != nil { + msgs = append(msgs, prefixValues("invalid basicAuthPassword: ", validateSecretSource(*claim.BasicAuthPassword))...) + } + return msgs +} diff --git a/pkg/validation/options.go b/pkg/validation/options.go index b54df330..29d8f71e 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -28,6 +28,8 @@ func Validate(o *options.Options) error { msgs := validateCookie(o.Cookie) msgs = append(msgs, validateSessionCookieMinimal(o)...) msgs = append(msgs, validateRedisSessionStore(o)...) + msgs = append(msgs, prefixValues("injectRequestHeaders: ", validateHeaders(o.InjectRequestHeaders)...)...) + msgs = append(msgs, prefixValues("injectRespeonseHeaders: ", validateHeaders(o.InjectRequestHeaders)...)...) if o.SSLInsecureSkipVerify { // InsecureSkipVerify is a configurable option we allow diff --git a/pkg/validation/utils.go b/pkg/validation/utils.go new file mode 100644 index 00000000..0ef3e15f --- /dev/null +++ b/pkg/validation/utils.go @@ -0,0 +1,11 @@ +package validation + +func prefixValues(prefix string, values ...string) []string { + msgs := []string{} + for _, value := range values { + if value != "" { + msgs = append(msgs, prefix+value) + } + } + return msgs +} From 8059a812cdd88024eb37d2df6e0aed6cf5a118ca Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 29 Jul 2020 20:10:14 +0100 Subject: [PATCH 3/7] Integrate new header injectors with OAuth2 Proxy --- main.go | 30 +- oauthproxy.go | 228 ++++-------- oauthproxy_test.go | 621 ++++++++++++++++---------------- pkg/validation/options.go | 8 - pkg/validation/options_test.go | 23 -- pkg/validation/sessions.go | 25 +- pkg/validation/sessions_test.go | 101 +++++- 7 files changed, 485 insertions(+), 551 deletions(-) diff --git a/main.go b/main.go index 8cd3ee5b..151c8331 100644 --- a/main.go +++ b/main.go @@ -3,17 +3,14 @@ package main import ( "fmt" "math/rand" - "net" "os" "os/signal" "runtime" "syscall" "time" - "github.com/justinas/alice" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/middleware" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/validation" ) @@ -63,33 +60,8 @@ func main() { rand.Seed(time.Now().UnixNano()) - chain := alice.New() - - if opts.ForceHTTPS { - _, httpsPort, err := net.SplitHostPort(opts.HTTPSAddress) - if err != nil { - logger.Fatalf("FATAL: invalid HTTPS address %q: %v", opts.HTTPAddress, err) - } - chain = chain.Append(middleware.NewRedirectToHTTPS(httpsPort)) - } - - healthCheckPaths := []string{opts.PingPath} - healthCheckUserAgents := []string{opts.PingUserAgent} - if opts.GCPHealthChecks { - healthCheckPaths = append(healthCheckPaths, "/liveness_check", "/readiness_check") - healthCheckUserAgents = append(healthCheckUserAgents, "GoogleHC/1.0") - } - - // To silence logging of health checks, register the health check handler before - // the logging handler - if opts.Logging.SilencePing { - chain = chain.Append(middleware.NewHealthCheck(healthCheckPaths, healthCheckUserAgents), LoggingHandler) - } else { - chain = chain.Append(LoggingHandler, middleware.NewHealthCheck(healthCheckPaths, healthCheckUserAgents)) - } - s := &Server{ - Handler: chain.Then(oauthproxy), + Handler: oauthproxy, Opts: opts, stop: make(chan struct{}, 1), } diff --git a/oauthproxy.go b/oauthproxy.go index 4fd03a40..c349172a 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -2,7 +2,6 @@ package main import ( "context" - b64 "encoding/base64" "encoding/json" "errors" "fmt" @@ -98,7 +97,6 @@ type OAuthProxy struct { PassAuthorization bool PreferEmailToUser bool skipAuthPreflight bool - skipAuthStripHeaders bool skipJwtBearerTokens bool mainJwtBearerVerifier *oidc.IDTokenVerifier extraJwtBearerVerifiers []*oidc.IDTokenVerifier @@ -110,6 +108,8 @@ type OAuthProxy struct { AllowedGroups []string sessionChain alice.Chain + headersChain alice.Chain + preAuthChain alice.Chain } // NewOAuthProxy creates a new instance of OAuthProxy from the options provided @@ -169,7 +169,15 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr return nil, err } + preAuthChain, err := buildPreAuthChain(opts) + if err != nil { + return nil, fmt.Errorf("could not build pre-auth chain: %v", err) + } sessionChain := buildSessionChain(opts, sessionStore, basicAuthValidator) + headersChain, err := buildHeadersChain(opts) + if err != nil { + return nil, fmt.Errorf("could not build headers chain: %v", err) + } return &OAuthProxy{ CookieName: opts.Cookie.Name, @@ -201,20 +209,10 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr allowedRoutes: allowedRoutes, whitelistDomains: opts.WhitelistDomains, skipAuthPreflight: opts.SkipAuthPreflight, - skipAuthStripHeaders: opts.SkipAuthStripHeaders, skipJwtBearerTokens: opts.SkipJwtBearerTokens, mainJwtBearerVerifier: opts.GetOIDCVerifier(), extraJwtBearerVerifiers: opts.GetJWTBearerVerifiers(), realClientIPParser: opts.GetRealClientIPParser(), - SetXAuthRequest: opts.SetXAuthRequest, - PassBasicAuth: opts.PassBasicAuth, - SetBasicAuth: opts.SetBasicAuth, - PassUserHeaders: opts.PassUserHeaders, - BasicAuthPassword: opts.BasicAuthPassword, - PassAccessToken: opts.PassAccessToken, - SetAuthorization: opts.SetAuthorization, - PassAuthorization: opts.PassAuthorization, - PreferEmailToUser: opts.PreferEmailToUser, SkipProviderButton: opts.SkipProviderButton, templates: templates, trustedIPs: trustedIPs, @@ -226,12 +224,46 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr basicAuthValidator: basicAuthValidator, displayHtpasswdForm: basicAuthValidator != nil, sessionChain: sessionChain, + headersChain: headersChain, + preAuthChain: preAuthChain, }, nil } -func buildSessionChain(opts *options.Options, sessionStore sessionsapi.SessionStore, validator basic.Validator) alice.Chain { +// buildPreAuthChain constructs a chain that should process every request before +// the OAuth2 Proxy authentication logic kicks in. +// For example forcing HTTPS or health checks. +func buildPreAuthChain(opts *options.Options) (alice.Chain, error) { chain := alice.New(middleware.NewScope()) + if opts.ForceHTTPS { + _, httpsPort, err := net.SplitHostPort(opts.HTTPSAddress) + if err != nil { + return alice.Chain{}, fmt.Errorf("invalid HTTPS address %q: %v", opts.HTTPAddress, err) + } + chain = chain.Append(middleware.NewRedirectToHTTPS(httpsPort)) + } + + healthCheckPaths := []string{opts.PingPath} + healthCheckUserAgents := []string{opts.PingUserAgent} + if opts.GCPHealthChecks { + healthCheckPaths = append(healthCheckPaths, "/liveness_check", "/readiness_check") + healthCheckUserAgents = append(healthCheckUserAgents, "GoogleHC/1.0") + } + + // To silence logging of health checks, register the health check handler before + // the logging handler + if opts.Logging.SilencePing { + chain = chain.Append(middleware.NewHealthCheck(healthCheckPaths, healthCheckUserAgents), LoggingHandler) + } else { + chain = chain.Append(LoggingHandler, middleware.NewHealthCheck(healthCheckPaths, healthCheckUserAgents)) + } + + return chain, nil +} + +func buildSessionChain(opts *options.Options, sessionStore sessionsapi.SessionStore, validator basic.Validator) alice.Chain { + chain := alice.New() + if opts.SkipJwtBearerTokens { sessionLoaders := []middlewareapi.TokenToSessionLoader{} if opts.GetOIDCVerifier() != nil { @@ -264,6 +296,20 @@ func buildSessionChain(opts *options.Options, sessionStore sessionsapi.SessionSt return chain } +func buildHeadersChain(opts *options.Options) (alice.Chain, error) { + requestInjector, err := middleware.NewRequestHeaderInjector(opts.InjectRequestHeaders) + if err != nil { + return alice.Chain{}, fmt.Errorf("error constructing request header injector: %v", err) + } + + responseInjector, err := middleware.NewResponseHeaderInjector(opts.InjectResponseHeaders) + if err != nil { + return alice.Chain{}, fmt.Errorf("error constructing request header injector: %v", err) + } + + return alice.New(requestInjector, responseInjector), nil +} + func buildSignInMessage(opts *options.Options) string { var msg string if len(opts.Banner) >= 1 { @@ -685,6 +731,10 @@ func (p *OAuthProxy) IsTrustedIP(req *http.Request) bool { } func (p *OAuthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { + p.preAuthChain.Then(http.HandlerFunc(p.serveHTTP)).ServeHTTP(rw, req) +} + +func (p *OAuthProxy) serveHTTP(rw http.ResponseWriter, req *http.Request) { if req.URL.Path != p.AuthOnlyPath && strings.HasPrefix(req.URL.Path, p.ProxyPrefix) { prepareNoCache(rw) } @@ -884,15 +934,14 @@ func (p *OAuthProxy) AuthenticateOnly(rw http.ResponseWriter, req *http.Request) // we are authenticated p.addHeadersForProxying(rw, req, session) - rw.WriteHeader(http.StatusAccepted) + p.headersChain.Then(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.WriteHeader(http.StatusAccepted) + })).ServeHTTP(rw, req) } // SkipAuthProxy proxies allowlisted requests and skips authentication func (p *OAuthProxy) SkipAuthProxy(rw http.ResponseWriter, req *http.Request) { - if p.skipAuthStripHeaders { - p.stripAuthHeaders(req) - } - p.serveMux.ServeHTTP(rw, req) + p.headersChain.Then(p.serveMux).ServeHTTP(rw, req) } // Proxy proxies the user request if the user is authenticated else it prompts @@ -903,8 +952,7 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) { case nil: // we are authenticated p.addHeadersForProxying(rw, req, session) - p.serveMux.ServeHTTP(rw, req) - + p.headersChain.Then(p.serveMux).ServeHTTP(rw, req) case ErrNeedsLogin: // we need to send the user to a login screen if isAjax(req) { @@ -961,120 +1009,6 @@ func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.R // addHeadersForProxying adds the appropriate headers the request / response for proxying func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) { - if p.PassBasicAuth { - if p.PreferEmailToUser && session.Email != "" { - req.SetBasicAuth(session.Email, p.BasicAuthPassword) - req.Header["X-Forwarded-User"] = []string{session.Email} - req.Header.Del("X-Forwarded-Email") - } else { - req.SetBasicAuth(session.User, p.BasicAuthPassword) - req.Header["X-Forwarded-User"] = []string{session.User} - if session.Email != "" { - req.Header["X-Forwarded-Email"] = []string{session.Email} - } else { - req.Header.Del("X-Forwarded-Email") - } - } - if session.PreferredUsername != "" { - req.Header["X-Forwarded-Preferred-Username"] = []string{session.PreferredUsername} - } else { - req.Header.Del("X-Forwarded-Preferred-Username") - } - } - - if p.PassUserHeaders { - if p.PreferEmailToUser && session.Email != "" { - req.Header["X-Forwarded-User"] = []string{session.Email} - req.Header.Del("X-Forwarded-Email") - } else { - req.Header["X-Forwarded-User"] = []string{session.User} - if session.Email != "" { - req.Header["X-Forwarded-Email"] = []string{session.Email} - } else { - req.Header.Del("X-Forwarded-Email") - } - } - - if session.PreferredUsername != "" { - req.Header["X-Forwarded-Preferred-Username"] = []string{session.PreferredUsername} - } else { - req.Header.Del("X-Forwarded-Preferred-Username") - } - - if len(session.Groups) > 0 { - for _, group := range session.Groups { - req.Header.Add("X-Forwarded-Groups", group) - } - } else { - req.Header.Del("X-Forwarded-Groups") - } - } - - if p.SetXAuthRequest { - rw.Header().Set("X-Auth-Request-User", session.User) - if session.Email != "" { - rw.Header().Set("X-Auth-Request-Email", session.Email) - } else { - rw.Header().Del("X-Auth-Request-Email") - } - if session.PreferredUsername != "" { - rw.Header().Set("X-Auth-Request-Preferred-Username", session.PreferredUsername) - } else { - rw.Header().Del("X-Auth-Request-Preferred-Username") - } - - if p.PassAccessToken { - if session.AccessToken != "" { - rw.Header().Set("X-Auth-Request-Access-Token", session.AccessToken) - } else { - rw.Header().Del("X-Auth-Request-Access-Token") - } - } - - if len(session.Groups) > 0 { - for _, group := range session.Groups { - rw.Header().Add("X-Auth-Request-Groups", group) - } - } else { - rw.Header().Del("X-Auth-Request-Groups") - } - } - - if p.PassAccessToken { - if session.AccessToken != "" { - req.Header["X-Forwarded-Access-Token"] = []string{session.AccessToken} - } else { - req.Header.Del("X-Forwarded-Access-Token") - } - } - - if p.PassAuthorization { - if session.IDToken != "" { - req.Header["Authorization"] = []string{fmt.Sprintf("Bearer %s", session.IDToken)} - } else { - req.Header.Del("Authorization") - } - } - if p.SetBasicAuth { - switch { - case p.PreferEmailToUser && session.Email != "": - authVal := b64.StdEncoding.EncodeToString([]byte(session.Email + ":" + p.BasicAuthPassword)) - rw.Header().Set("Authorization", "Basic "+authVal) - case session.User != "": - authVal := b64.StdEncoding.EncodeToString([]byte(session.User + ":" + p.BasicAuthPassword)) - rw.Header().Set("Authorization", "Basic "+authVal) - default: - rw.Header().Del("Authorization") - } - } - if p.SetAuthorization { - if session.IDToken != "" { - rw.Header().Set("Authorization", fmt.Sprintf("Bearer %s", session.IDToken)) - } else { - rw.Header().Del("Authorization") - } - } - if session.Email == "" { rw.Header().Set("GAP-Auth", session.User) } else { @@ -1082,32 +1016,6 @@ func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Req } } -// stripAuthHeaders removes Auth headers for allowlisted routes from skipAuthRegex -func (p *OAuthProxy) stripAuthHeaders(req *http.Request) { - if p.PassBasicAuth { - req.Header.Del("X-Forwarded-User") - req.Header.Del("X-Forwarded-Groups") - req.Header.Del("X-Forwarded-Email") - req.Header.Del("X-Forwarded-Preferred-Username") - req.Header.Del("Authorization") - } - - if p.PassUserHeaders { - req.Header.Del("X-Forwarded-User") - req.Header.Del("X-Forwarded-Groups") - req.Header.Del("X-Forwarded-Email") - req.Header.Del("X-Forwarded-Preferred-Username") - } - - if p.PassAccessToken { - req.Header.Del("X-Forwarded-Access-Token") - } - - if p.PassAuthorization { - req.Header.Del("Authorization") - } -} - // isAjax checks if a request is an ajax request func isAjax(req *http.Request) bool { acceptValues := req.Header.Values("Accept") diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 46ee24b8..1736b39f 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -495,6 +495,8 @@ func TestBasicAuthPassword(t *testing.T) { t.Fatal(err) } })) + + basicAuthPassword := "This is a secure password" opts := baseTestOptions() opts.UpstreamServers = options.Upstreams{ { @@ -505,11 +507,22 @@ func TestBasicAuthPassword(t *testing.T) { } opts.Cookie.Secure = false - opts.PassBasicAuth = true - opts.SetBasicAuth = true - opts.PassUserHeaders = true - opts.PreferEmailToUser = true - opts.BasicAuthPassword = "This is a secure password" + opts.InjectRequestHeaders = []options.Header{ + { + Name: "Authorization", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "email", + BasicAuthPassword: &options.SecretSource{ + Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthPassword))), + }, + }, + }, + }, + }, + } + err := validation.Validate(opts) assert.NoError(t, err) @@ -524,148 +537,44 @@ func TestBasicAuthPassword(t *testing.T) { t.Fatal(err) } + // Save the required session rw := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/oauth2/callback?code=callback_code&state=nonce:", strings.NewReader("")) - req.AddCookie(proxy.MakeCSRFCookie(req, "nonce", proxy.CookieExpire, time.Now())) - proxy.ServeHTTP(rw, req) - if rw.Code >= 400 { - t.Fatalf("expected 3xx got %d", rw.Code) - } - cookie := rw.Header().Values("Set-Cookie")[1] - - cookieName := proxy.CookieName - var value string - keyPrefix := cookieName + "=" - - for _, field := range strings.Split(cookie, "; ") { - value = strings.TrimPrefix(field, keyPrefix) - if value != field { - break - } else { - value = "" - } - } - - req, _ = http.NewRequest("GET", "/", strings.NewReader("")) - req.AddCookie(&http.Cookie{ - Name: cookieName, - Value: value, - Path: "/", - Expires: time.Now().Add(time.Duration(24)), - HttpOnly: true, + req, _ := http.NewRequest("GET", "/", nil) + err = proxy.sessionStore.Save(rw, req, &sessions.SessionState{ + Email: emailAddress, }) - req.AddCookie(proxy.MakeCSRFCookie(req, "nonce", proxy.CookieExpire, time.Now())) + assert.NoError(t, err) + // Extract the cookie value to inject into the test request + cookie := rw.Header().Values("Set-Cookie")[0] + + req, _ = http.NewRequest("GET", "/", nil) + req.Header.Set("Cookie", cookie) rw = httptest.NewRecorder() proxy.ServeHTTP(rw, req) // The username in the basic auth credentials is expected to be equal to the email address from the // auth response, so we use the same variable here. - expectedHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(emailAddress+":"+opts.BasicAuthPassword)) + expectedHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(emailAddress+":"+basicAuthPassword)) assert.Equal(t, expectedHeader, rw.Body.String()) providerServer.Close() } -func TestBasicAuthWithEmail(t *testing.T) { - opts := baseTestOptions() - opts.PassBasicAuth = true - opts.PassUserHeaders = false - opts.PreferEmailToUser = false - opts.BasicAuthPassword = "This is a secure password" - err := validation.Validate(opts) - assert.NoError(t, err) - - const emailAddress = "john.doe@example.com" - const userName = "9fcab5c9b889a557" - - // The username in the basic auth credentials is expected to be equal to the email address from the - expectedEmailHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(emailAddress+":"+opts.BasicAuthPassword)) - expectedUserHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(userName+":"+opts.BasicAuthPassword)) - - created := time.Now() - session := &sessions.SessionState{ - User: userName, - Email: emailAddress, - AccessToken: "oauth_token", - CreatedAt: &created, - } - { - rw := httptest.NewRecorder() - req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase0", nil) - proxy, err := NewOAuthProxy(opts, func(email string) bool { - return email == emailAddress - }) - if err != nil { - t.Fatal(err) - } - proxy.addHeadersForProxying(rw, req, session) - assert.Equal(t, expectedUserHeader, req.Header["Authorization"][0]) - assert.Equal(t, userName, req.Header["X-Forwarded-User"][0]) - } - - opts.PreferEmailToUser = true - { - rw := httptest.NewRecorder() - req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase1", nil) - - proxy, err := NewOAuthProxy(opts, func(email string) bool { - return email == emailAddress - }) - if err != nil { - t.Fatal(err) - } - proxy.addHeadersForProxying(rw, req, session) - assert.Equal(t, expectedEmailHeader, req.Header["Authorization"][0]) - assert.Equal(t, emailAddress, req.Header["X-Forwarded-User"][0]) - } -} - -func TestPassUserHeadersWithEmail(t *testing.T) { - opts := baseTestOptions() - err := validation.Validate(opts) - assert.NoError(t, err) - - const emailAddress = "john.doe@example.com" - const userName = "9fcab5c9b889a557" - - created := time.Now() - session := &sessions.SessionState{ - User: userName, - Email: emailAddress, - AccessToken: "oauth_token", - CreatedAt: &created, - } - { - rw := httptest.NewRecorder() - req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase0", nil) - proxy, err := NewOAuthProxy(opts, func(email string) bool { - return email == emailAddress - }) - if err != nil { - t.Fatal(err) - } - proxy.addHeadersForProxying(rw, req, session) - assert.Equal(t, userName, req.Header["X-Forwarded-User"][0]) - } - - opts.PreferEmailToUser = true - { - rw := httptest.NewRecorder() - req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase1", nil) - - proxy, err := NewOAuthProxy(opts, func(email string) bool { - return email == emailAddress - }) - if err != nil { - t.Fatal(err) - } - proxy.addHeadersForProxying(rw, req, session) - assert.Equal(t, emailAddress, req.Header["X-Forwarded-User"][0]) - } -} - func TestPassGroupsHeadersWithGroups(t *testing.T) { opts := baseTestOptions() + opts.InjectRequestHeaders = []options.Header{ + { + Name: "X-Forwarded-Groups", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "groups", + }, + }, + }, + }, + } + err := validation.Validate(opts) assert.NoError(t, err) @@ -681,161 +590,27 @@ func TestPassGroupsHeadersWithGroups(t *testing.T) { AccessToken: "oauth_token", CreatedAt: &created, } - { - rw := httptest.NewRecorder() - req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase0", nil) - proxy, err := NewOAuthProxy(opts, func(email string) bool { - return email == emailAddress - }) - if err != nil { - t.Fatal(err) - } - proxy.addHeadersForProxying(rw, req, session) - assert.Equal(t, groups, req.Header["X-Forwarded-Groups"]) - } -} -func TestStripAuthHeaders(t *testing.T) { - testCases := map[string]struct { - SkipAuthStripHeaders bool - PassBasicAuth bool - PassUserHeaders bool - PassAccessToken bool - PassAuthorization bool - StrippedHeaders map[string]bool - }{ - "Default options": { - SkipAuthStripHeaders: true, - PassBasicAuth: true, - PassUserHeaders: true, - PassAccessToken: false, - PassAuthorization: false, - StrippedHeaders: map[string]bool{ - "X-Forwarded-User": true, - "X-Forwared-Groups": true, - "X-Forwarded-Email": true, - "X-Forwarded-Preferred-Username": true, - "X-Forwarded-Access-Token": false, - "Authorization": true, - }, - }, - "Pass access token": { - SkipAuthStripHeaders: true, - PassBasicAuth: true, - PassUserHeaders: true, - PassAccessToken: true, - PassAuthorization: false, - StrippedHeaders: map[string]bool{ - "X-Forwarded-User": true, - "X-Forwared-Groups": true, - "X-Forwarded-Email": true, - "X-Forwarded-Preferred-Username": true, - "X-Forwarded-Access-Token": true, - "Authorization": true, - }, - }, - "Nothing setting Authorization": { - SkipAuthStripHeaders: true, - PassBasicAuth: false, - PassUserHeaders: true, - PassAccessToken: true, - PassAuthorization: false, - StrippedHeaders: map[string]bool{ - "X-Forwarded-User": true, - "X-Forwared-Groups": true, - "X-Forwarded-Email": true, - "X-Forwarded-Preferred-Username": true, - "X-Forwarded-Access-Token": true, - "Authorization": false, - }, - }, - "Only Authorization header modified": { - SkipAuthStripHeaders: true, - PassBasicAuth: false, - PassUserHeaders: false, - PassAccessToken: false, - PassAuthorization: true, - StrippedHeaders: map[string]bool{ - "X-Forwarded-User": false, - "X-Forwared-Groups": false, - "X-Forwarded-Email": false, - "X-Forwarded-Preferred-Username": false, - "X-Forwarded-Access-Token": false, - "Authorization": true, - }, - }, - "Don't strip any headers (default options)": { - SkipAuthStripHeaders: false, - PassBasicAuth: true, - PassUserHeaders: true, - PassAccessToken: false, - PassAuthorization: false, - StrippedHeaders: map[string]bool{ - "X-Forwarded-User": false, - "X-Forwared-Groups": false, - "X-Forwarded-Email": false, - "X-Forwarded-Preferred-Username": false, - "X-Forwarded-Access-Token": false, - "Authorization": false, - }, - }, - "Don't strip any headers (custom options)": { - SkipAuthStripHeaders: false, - PassBasicAuth: true, - PassUserHeaders: true, - PassAccessToken: true, - PassAuthorization: false, - StrippedHeaders: map[string]bool{ - "X-Forwarded-User": false, - "X-Forwared-Groups": false, - "X-Forwarded-Email": false, - "X-Forwarded-Preferred-Username": false, - "X-Forwarded-Access-Token": false, - "Authorization": false, - }, - }, - } + proxy, err := NewOAuthProxy(opts, func(email string) bool { + return email == emailAddress + }) + assert.NoError(t, err) - initialHeaders := map[string]string{ - "X-Forwarded-User": "9fcab5c9b889a557", - "X-Forwarded-Email": "john.doe@example.com", - "X-Forwarded-Groups": "a,b,c", - "X-Forwarded-Preferred-Username": "john.doe", - "X-Forwarded-Access-Token": "AccessToken", - "Authorization": "bearer IDToken", - } + // Save the required session + rw := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/", nil) + err = proxy.sessionStore.Save(rw, req, session) + assert.NoError(t, err) - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - opts := baseTestOptions() - opts.SkipAuthStripHeaders = tc.SkipAuthStripHeaders - opts.PassBasicAuth = tc.PassBasicAuth - opts.PassUserHeaders = tc.PassUserHeaders - opts.PassAccessToken = tc.PassAccessToken - opts.PassAuthorization = tc.PassAuthorization - err := validation.Validate(opts) - assert.NoError(t, err) + // Extract the cookie value to inject into the test request + cookie := rw.Header().Values("Set-Cookie")[0] - req, _ := http.NewRequest("GET", fmt.Sprintf("%s/testCase", opts.ProxyPrefix), nil) - for header, val := range initialHeaders { - req.Header.Set(header, val) - } + req, _ = http.NewRequest("GET", "/", nil) + req.Header.Set("Cookie", cookie) + rw = httptest.NewRecorder() + proxy.ServeHTTP(rw, req) - proxy, err := NewOAuthProxy(opts, func(_ string) bool { return true }) - assert.NoError(t, err) - if proxy.skipAuthStripHeaders { - proxy.stripAuthHeaders(req) - } - - for header, stripped := range tc.StrippedHeaders { - if stripped { - assert.Equal(t, req.Header.Get(header), "") - } else { - assert.Equal(t, req.Header.Get(header), initialHeaders[header]) - } - } - }) - } + assert.Equal(t, groups, req.Header["X-Forwarded-Groups"]) } type PassAccessTokenTest struct { @@ -884,7 +659,21 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) (*PassAccessTokenTe } patt.opts.Cookie.Secure = false - patt.opts.PassAccessToken = opts.PassAccessToken + if opts.PassAccessToken { + patt.opts.InjectRequestHeaders = []options.Header{ + { + Name: "X-Forwarded-Access-Token", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "access_token", + }, + }, + }, + }, + } + } + err := validation.Validate(patt.opts) if err != nil { return nil, err @@ -1442,7 +1231,48 @@ func TestAuthOnlyEndpointSetXAuthRequestHeaders(t *testing.T) { var pcTest ProcessCookieTest pcTest.opts = baseTestOptions() - pcTest.opts.SetXAuthRequest = true + pcTest.opts.InjectResponseHeaders = []options.Header{ + { + Name: "X-Auth-Request-User", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "user", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Email", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "email", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Groups", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "groups", + }, + }, + }, + }, + { + Name: "X-Forwarded-Preferred-Username", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "preferred_username", + }, + }, + }, + }, + } pcTest.opts.AllowedGroups = []string{"oauth_groups"} err := validation.Validate(pcTest.opts) assert.NoError(t, err) @@ -1480,8 +1310,62 @@ func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) { var pcTest ProcessCookieTest pcTest.opts = baseTestOptions() - pcTest.opts.SetXAuthRequest = true - pcTest.opts.SetBasicAuth = true + pcTest.opts.InjectResponseHeaders = []options.Header{ + { + Name: "X-Auth-Request-User", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "user", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Email", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "email", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Groups", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "groups", + }, + }, + }, + }, + { + Name: "X-Forwarded-Preferred-Username", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "preferred_username", + }, + }, + }, + }, + { + Name: "Authorization", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "user", + BasicAuthPassword: &options.SecretSource{ + Value: []byte(base64.StdEncoding.EncodeToString([]byte("This is a secure password"))), + }, + }, + }, + }, + }, + } + err := validation.Validate(pcTest.opts) assert.NoError(t, err) @@ -1511,7 +1395,7 @@ func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) { assert.Equal(t, http.StatusAccepted, pcTest.rw.Code) assert.Equal(t, "oauth_user", pcTest.rw.Header().Values("X-Auth-Request-User")[0]) assert.Equal(t, "oauth_user@example.com", pcTest.rw.Header().Values("X-Auth-Request-Email")[0]) - expectedHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte("oauth_user:"+pcTest.opts.BasicAuthPassword)) + expectedHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte("oauth_user:This is a secure password")) assert.Equal(t, expectedHeader, pcTest.rw.Header().Values("Authorization")[0]) } @@ -1519,8 +1403,48 @@ func TestAuthOnlyEndpointSetBasicAuthFalseRequestHeaders(t *testing.T) { var pcTest ProcessCookieTest pcTest.opts = baseTestOptions() - pcTest.opts.SetXAuthRequest = true - pcTest.opts.SetBasicAuth = false + pcTest.opts.InjectResponseHeaders = []options.Header{ + { + Name: "X-Auth-Request-User", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "user", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Email", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "email", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Groups", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "groups", + }, + }, + }, + }, + { + Name: "X-Forwarded-Preferred-Username", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "preferred_username", + }, + }, + }, + }, + } err := validation.Validate(pcTest.opts) assert.NoError(t, err) @@ -1985,9 +1909,74 @@ func TestGetJwtSession(t *testing.T) { &oidc.Config{ClientID: "https://test.myapp.com", SkipExpiryCheck: true}) test, err := NewAuthOnlyEndpointTest(func(opts *options.Options) { - opts.PassAuthorization = true - opts.SetAuthorization = true - opts.SetXAuthRequest = true + opts.InjectRequestHeaders = []options.Header{ + { + Name: "Authorization", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "id_token", + Prefix: "Bearer ", + }, + }, + }, + }, + { + Name: "X-Forwarded-User", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "user", + }, + }, + }, + }, + { + Name: "X-Forwarded-Email", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "email", + }, + }, + }, + }, + } + + opts.InjectResponseHeaders = []options.Header{ + { + Name: "Authorization", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "id_token", + Prefix: "Bearer ", + }, + }, + }, + }, + { + Name: "X-Auth-Request-User", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "user", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Email", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "email", + }, + }, + }, + }, + } + opts.SkipJwtBearerTokens = true opts.SetJWTBearerVerifiers(append(opts.GetJWTBearerVerifiers(), verifier)) }) @@ -2004,15 +1993,6 @@ func TestGetJwtSession(t *testing.T) { "Authorization": {authHeader}, } - // Bearer - expires := time.Unix(1912151821, 0) - session, err := test.proxy.getAuthenticatedSession(test.rw, test.req) - assert.NoError(t, err) - assert.Equal(t, session.User, "1234567890") - assert.Equal(t, session.Email, "john@example.com") - assert.Equal(t, session.ExpiresOn, &expires) - assert.Equal(t, session.IDToken, goodJwt) - test.proxy.ServeHTTP(test.rw, test.req) if test.rw.Code >= 400 { t.Fatalf("expected 3xx got %d", test.rw.Code) @@ -2140,6 +2120,43 @@ func baseTestOptions() *options.Options { opts.ClientID = clientID opts.ClientSecret = clientSecret opts.EmailDomains = []string{"*"} + + // Default injected headers for legacy configuration + opts.InjectRequestHeaders = []options.Header{ + { + Name: "Authorization", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "user", + BasicAuthPassword: &options.SecretSource{ + Value: []byte(base64.StdEncoding.EncodeToString([]byte("This is a secure password"))), + }, + }, + }, + }, + }, + { + Name: "X-Forwarded-User", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "user", + }, + }, + }, + }, + { + Name: "X-Forwarded-Email", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "email", + }, + }, + }, + }, + } return opts } diff --git a/pkg/validation/options.go b/pkg/validation/options.go index 29d8f71e..16a19612 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -73,10 +73,6 @@ func Validate(o *options.Options) error { "\n use email-domain=* to authorize all email addresses") } - if o.SetBasicAuth && o.SetAuthorization { - msgs = append(msgs, "mutually exclusive: set-basic-auth and set-authorization-header can not both be true") - } - if o.OIDCIssuerURL != "" { ctx := context.Background() @@ -161,10 +157,6 @@ func Validate(o *options.Options) error { } } - if o.PreferEmailToUser && !o.PassBasicAuth && !o.PassUserHeaders { - msgs = append(msgs, "PreferEmailToUser should only be used with PassBasicAuth or PassUserHeaders") - } - if o.SkipJwtBearerTokens { // Configure extra issuers if len(o.ExtraJwtIssuers) > 0 { diff --git a/pkg/validation/options_test.go b/pkg/validation/options_test.go index f88ef7af..e19c6991 100644 --- a/pkg/validation/options_test.go +++ b/pkg/validation/options_test.go @@ -162,29 +162,6 @@ func TestDefaultProviderApiSettings(t *testing.T) { assert.Equal(t, "profile email", p.Scope) } -func TestPassAccessTokenRequiresSpecificCookieSecretLengths(t *testing.T) { - o := testOptions() - assert.Equal(t, nil, Validate(o)) - - assert.Equal(t, false, o.PassAccessToken) - o.PassAccessToken = true - o.Cookie.Secret = "cookie of invalid length-" - assert.NotEqual(t, nil, Validate(o)) - - o.PassAccessToken = false - o.Cookie.Refresh = time.Duration(24) * time.Hour - assert.NotEqual(t, nil, Validate(o)) - - o.Cookie.Secret = "16 bytes AES-128" - assert.Equal(t, nil, Validate(o)) - - o.Cookie.Secret = "24 byte secret AES-192--" - assert.Equal(t, nil, Validate(o)) - - o.Cookie.Secret = "32 byte secret for AES-256------" - assert.Equal(t, nil, Validate(o)) -} - func TestCookieRefreshMustBeLessThanCookieExpire(t *testing.T) { o := testOptions() assert.Equal(t, nil, Validate(o)) diff --git a/pkg/validation/sessions.go b/pkg/validation/sessions.go index 8cacd48b..48d4042a 100644 --- a/pkg/validation/sessions.go +++ b/pkg/validation/sessions.go @@ -16,18 +16,21 @@ func validateSessionCookieMinimal(o *options.Options) []string { } msgs := []string{} - if o.PassAuthorization { - msgs = append(msgs, - "pass_authorization_header requires oauth tokens in sessions. session_cookie_minimal cannot be set") - } - if o.SetAuthorization { - msgs = append(msgs, - "set_authorization_header requires oauth tokens in sessions. session_cookie_minimal cannot be set") - } - if o.PassAccessToken { - msgs = append(msgs, - "pass_access_token requires oauth tokens in sessions. session_cookie_minimal cannot be set") + for _, header := range append(o.InjectRequestHeaders, o.InjectResponseHeaders...) { + for _, value := range header.Values { + if value.ClaimSource != nil { + if value.ClaimSource.Claim == "access_token" { + msgs = append(msgs, + fmt.Sprintf("access_token claim for header %q requires oauth tokens in sessions. session_cookie_minimal cannot be set", header.Name)) + } + if value.ClaimSource.Claim == "id_token" { + msgs = append(msgs, + fmt.Sprintf("id_token claim for header %q requires oauth tokens in sessions. session_cookie_minimal cannot be set", header.Name)) + } + } + } } + if o.Cookie.Refresh != time.Duration(0) { msgs = append(msgs, "cookie_refresh > 0 requires oauth tokens in sessions. session_cookie_minimal cannot be set") diff --git a/pkg/validation/sessions_test.go b/pkg/validation/sessions_test.go index f6463431..f1943288 100644 --- a/pkg/validation/sessions_test.go +++ b/pkg/validation/sessions_test.go @@ -13,10 +13,9 @@ import ( var _ = Describe("Sessions", func() { const ( - passAuthorizationMsg = "pass_authorization_header requires oauth tokens in sessions. session_cookie_minimal cannot be set" - setAuthorizationMsg = "set_authorization_header requires oauth tokens in sessions. session_cookie_minimal cannot be set" - passAccessTokenMsg = "pass_access_token requires oauth tokens in sessions. session_cookie_minimal cannot be set" - cookieRefreshMsg = "cookie_refresh > 0 requires oauth tokens in sessions. session_cookie_minimal cannot be set" + idTokenConflictMsg = "id_token claim for header \"X-ID-Token\" requires oauth tokens in sessions. session_cookie_minimal cannot be set" + accessTokenConflictMsg = "access_token claim for header \"X-Access-Token\" requires oauth tokens in sessions. session_cookie_minimal cannot be set" + cookieRefreshMsg = "cookie_refresh > 0 requires oauth tokens in sessions. session_cookie_minimal cannot be set" ) type cookieMinimalTableInput struct { @@ -38,14 +37,25 @@ var _ = Describe("Sessions", func() { }, errStrings: []string{}, }), - Entry("No minimal cookie session & passAuthorization", &cookieMinimalTableInput{ + Entry("No minimal cookie session & request header has access_token claim", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ Minimal: false, }, }, - PassAuthorization: true, + InjectRequestHeaders: []options.Header{ + { + Name: "X-Access-Token", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "access_token", + }, + }, + }, + }, + }, }, errStrings: []string{}, }), @@ -59,38 +69,71 @@ var _ = Describe("Sessions", func() { }, errStrings: []string{}, }), - Entry("PassAuthorization conflict", &cookieMinimalTableInput{ + Entry("Request Header id_token conflict", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ Minimal: true, }, }, - PassAuthorization: true, + InjectRequestHeaders: []options.Header{ + { + Name: "X-ID-Token", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "id_token", + }, + }, + }, + }, + }, }, - errStrings: []string{passAuthorizationMsg}, + errStrings: []string{idTokenConflictMsg}, }), - Entry("SetAuthorization conflict", &cookieMinimalTableInput{ + Entry("Response Header id_token conflict", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ Minimal: true, }, }, - SetAuthorization: true, + InjectResponseHeaders: []options.Header{ + { + Name: "X-ID-Token", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "id_token", + }, + }, + }, + }, + }, }, - errStrings: []string{setAuthorizationMsg}, + errStrings: []string{idTokenConflictMsg}, }), - Entry("PassAccessToken conflict", &cookieMinimalTableInput{ + Entry("Request Header access_token conflict", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ Minimal: true, }, }, - PassAccessToken: true, + InjectRequestHeaders: []options.Header{ + { + Name: "X-Access-Token", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "access_token", + }, + }, + }, + }, + }, }, - errStrings: []string{passAccessTokenMsg}, + errStrings: []string{accessTokenConflictMsg}, }), Entry("CookieRefresh conflict", &cookieMinimalTableInput{ opts: &options.Options{ @@ -112,10 +155,32 @@ var _ = Describe("Sessions", func() { Minimal: true, }, }, - PassAuthorization: true, - PassAccessToken: true, + InjectResponseHeaders: []options.Header{ + { + Name: "X-ID-Token", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "id_token", + }, + }, + }, + }, + }, + InjectRequestHeaders: []options.Header{ + { + Name: "X-Access-Token", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "access_token", + }, + }, + }, + }, + }, }, - errStrings: []string{passAuthorizationMsg, passAccessTokenMsg}, + errStrings: []string{idTokenConflictMsg, accessTokenConflictMsg}, }), ) From 1dac1419b3269e20cef06b820ddf5d5d49e67f16 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sat, 24 Oct 2020 07:17:01 +0100 Subject: [PATCH 4/7] Add tests for SecretSource validation --- pkg/validation/common.go | 4 +- pkg/validation/common_test.go | 138 ++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 pkg/validation/common_test.go diff --git a/pkg/validation/common.go b/pkg/validation/common.go index ccde822d..bc9dba28 100644 --- a/pkg/validation/common.go +++ b/pkg/validation/common.go @@ -8,6 +8,8 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" ) +const multipleValuesForSecretSource = "multiple values specified for secret source: specify either value, fromEnv of fromFile" + func validateSecretSource(source options.SecretSource) string { switch { case len(source.Value) > 0 && source.FromEnv == "" && source.FromFile == "": @@ -17,7 +19,7 @@ func validateSecretSource(source options.SecretSource) string { case len(source.Value) == 0 && source.FromEnv == "" && source.FromFile != "": return validateSecretSourceFile(source.FromFile) default: - return "multiple values specified for secret source: specify either value, fromEnv of fromFile" + return multipleValuesForSecretSource } } diff --git a/pkg/validation/common_test.go b/pkg/validation/common_test.go new file mode 100644 index 00000000..bdce5415 --- /dev/null +++ b/pkg/validation/common_test.go @@ -0,0 +1,138 @@ +package validation + +import ( + "encoding/base64" + "io/ioutil" + "os" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" +) + +var _ = Describe("Common", func() { + var validSecretSourceValue []byte + const validSecretSourceEnv = "OAUTH2_PROXY_TEST_SECRET_SOURCE_ENV" + var validSecretSourceFile string + + BeforeEach(func() { + validSecretSourceValue = []byte(base64.StdEncoding.EncodeToString([]byte("This is a secret source value"))) + Expect(os.Setenv(validSecretSourceEnv, "This is a secret source env")).To(Succeed()) + tmp, err := ioutil.TempFile("", "oauth2-proxy-secret-source-test") + Expect(err).ToNot(HaveOccurred()) + defer tmp.Close() + + _, err = tmp.Write([]byte("This is a secret source file")) + Expect(err).ToNot(HaveOccurred()) + + validSecretSourceFile = tmp.Name() + }) + + AfterEach(func() { + Expect(os.Unsetenv(validSecretSourceEnv)).To(Succeed()) + Expect(os.Remove(validSecretSourceFile)).To(Succeed()) + }) + + type validateSecretSourceTableInput struct { + source func() options.SecretSource + expectedMsg string + } + + DescribeTable("validateSecretSource should", + func(in validateSecretSourceTableInput) { + Expect(validateSecretSource(in.source())).To(Equal(in.expectedMsg)) + }, + Entry("with no entries", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{} + }, + expectedMsg: multipleValuesForSecretSource, + }), + Entry("with a Value and FromEnv", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{ + Value: validSecretSourceValue, + FromEnv: validSecretSourceEnv, + } + }, + expectedMsg: multipleValuesForSecretSource, + }), + Entry("with a Value and FromFile", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{ + Value: validSecretSourceValue, + FromFile: validSecretSourceFile, + } + }, + expectedMsg: multipleValuesForSecretSource, + }), + Entry("with FromEnv and FromFile", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{ + FromEnv: validSecretSourceEnv, + FromFile: validSecretSourceFile, + } + }, + expectedMsg: multipleValuesForSecretSource, + }), + Entry("with a Value, FromEnv and FromFile", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{ + Value: validSecretSourceValue, + FromEnv: validSecretSourceEnv, + FromFile: validSecretSourceFile, + } + }, + expectedMsg: multipleValuesForSecretSource, + }), + Entry("with a valid Value", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{ + Value: validSecretSourceValue, + } + }, + expectedMsg: "", + }), + Entry("with a valid FromEnv", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{ + FromEnv: validSecretSourceEnv, + } + }, + expectedMsg: "", + }), + Entry("with a valid FromFile", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{ + FromFile: validSecretSourceFile, + } + }, + expectedMsg: "", + }), + Entry("with an invalid Value", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{ + Value: []byte("Invalid Base64 Value"), + } + }, + expectedMsg: "error decoding secret value: illegal base64 data at input byte 7", + }), + Entry("with an invalid FromEnv", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{ + FromEnv: "INVALID_ENV", + } + }, + expectedMsg: "error loading secret from environent: no value for for key \"INVALID_ENV\"", + }), + Entry("with an invalid FromFile", validateSecretSourceTableInput{ + source: func() options.SecretSource { + return options.SecretSource{ + FromFile: "invalidFile", + } + }, + expectedMsg: "error loadig secret from file: stat invalidFile: no such file or directory", + }), + ) +}) From 8d1bbf33b1f232e2228586a29a42438ca83807a8 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 28 Oct 2020 20:08:09 +0000 Subject: [PATCH 5/7] Add tests for headers validation --- pkg/validation/header_test.go | 164 ++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 pkg/validation/header_test.go diff --git a/pkg/validation/header_test.go b/pkg/validation/header_test.go new file mode 100644 index 00000000..fee4525d --- /dev/null +++ b/pkg/validation/header_test.go @@ -0,0 +1,164 @@ +package validation + +import ( + "encoding/base64" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" +) + +var _ = Describe("Headers", func() { + type validateHeaderTableInput struct { + headers []options.Header + expectedMsgs []string + } + + validHeader1 := options.Header{ + Name: "X-Email", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "email", + }, + }, + }, + } + + validHeader2 := options.Header{ + Name: "X-Forwarded-Auth", + Values: []options.HeaderValue{ + { + SecretSource: &options.SecretSource{ + Value: []byte(base64.StdEncoding.EncodeToString([]byte("secret"))), + }, + }, + }, + } + + validHeader3 := options.Header{ + Name: "Authorization", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "email", + BasicAuthPassword: &options.SecretSource{ + Value: []byte(base64.StdEncoding.EncodeToString([]byte("secret"))), + }, + }, + }, + }, + } + + DescribeTable("validateHeaders", + func(in validateHeaderTableInput) { + Expect(validateHeaders(in.headers)).To(ConsistOf(in.expectedMsgs)) + }, + Entry("with no headers", validateHeaderTableInput{ + headers: []options.Header{}, + expectedMsgs: []string{}, + }), + Entry("with valid headers", validateHeaderTableInput{ + headers: []options.Header{ + validHeader1, + validHeader2, + validHeader3, + }, + expectedMsgs: []string{}, + }), + Entry("with multiple headers with the same name", validateHeaderTableInput{ + headers: []options.Header{ + validHeader1, + validHeader1, + validHeader2, + validHeader2, + }, + expectedMsgs: []string{ + "multiple headers found with name \"X-Email\": header names must be unique", + "multiple headers found with name \"X-Forwarded-Auth\": header names must be unique", + }, + }), + Entry("with an unamed header", validateHeaderTableInput{ + headers: []options.Header{ + {}, + validHeader2, + }, + expectedMsgs: []string{ + "header has empty name: names are required for all headers", + }, + }), + Entry("with a header which has a claim and secret source", validateHeaderTableInput{ + headers: []options.Header{ + { + Name: "With-Claim-And-Secret", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{}, + SecretSource: &options.SecretSource{}, + }, + }, + }, + validHeader1, + }, + expectedMsgs: []string{ + "invalid header \"With-Claim-And-Secret\": invalid values: header value has multiple entries: only one entry per value is allowed", + }, + }), + Entry("with a header which has a claim without a claim", validateHeaderTableInput{ + headers: []options.Header{ + { + Name: "Without-Claim", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Prefix: "prefix", + }, + }, + }, + }, + validHeader3, + }, + expectedMsgs: []string{ + "invalid header \"Without-Claim\": invalid values: claim should not be empty", + }, + }), + Entry("with a header with invalid secret source", validateHeaderTableInput{ + headers: []options.Header{ + { + Name: "With-Invalid-Secret", + Values: []options.HeaderValue{ + { + SecretSource: &options.SecretSource{}, + }, + }, + }, + validHeader1, + }, + expectedMsgs: []string{ + "invalid header \"With-Invalid-Secret\": invalid values: multiple values specified for secret source: specify either value, fromEnv of fromFile", + }, + }), + Entry("with a header with invalid basicAuthPassword source", validateHeaderTableInput{ + headers: []options.Header{ + { + Name: "With-Invalid-Basic-Auth", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "user", + BasicAuthPassword: &options.SecretSource{ + Value: []byte("secret"), + }, + }, + }, + }, + }, + validHeader1, + }, + expectedMsgs: []string{ + "invalid header \"With-Invalid-Basic-Auth\": invalid values: invalid basicAuthPassword: error decoding secret value: illegal base64 data at input byte 4", + }, + }), + ) +}) From 92d09343d26cdccf801c8e236ab78423e161a780 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 3 Nov 2020 17:40:12 +0000 Subject: [PATCH 6/7] Add tests for legacy header conversion --- pkg/apis/options/legacy_options_test.go | 422 ++++++++++++++++++++++++ 1 file changed, 422 insertions(+) diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index fb250590..a00061a3 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -1,6 +1,7 @@ package options import ( + "encoding/base64" "time" . "github.com/onsi/ginkgo" @@ -259,4 +260,425 @@ var _ = Describe("Legacy Options", func() { }), ) }) + + Context("Legacy Headers", func() { + const basicAuthSecret = "super-secret-password" + + type legacyHeadersTableInput struct { + legacyHeaders *LegacyHeaders + expectedRequestHeaders []Header + expectedResponseHeaders []Header + } + + withPreserveRequestValue := func(h Header, preserve bool) Header { + h.PreserveRequestValue = preserve + return h + } + + xForwardedUser := Header{ + Name: "X-Forwarded-User", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "user", + }, + }, + }, + } + + xForwardedEmail := Header{ + Name: "X-Forwarded-Email", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + }, + }, + }, + } + + xForwardedGroups := Header{ + Name: "X-Forwarded-Groups", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "groups", + }, + }, + }, + } + + xForwardedPreferredUsername := Header{ + Name: "X-Forwarded-Preferred-Username", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "preferred_username", + }, + }, + }, + } + + basicAuthHeader := Header{ + Name: "Authorization", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "user", + BasicAuthPassword: &SecretSource{ + Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthSecret))), + }, + }, + }, + }, + } + + xForwardedUserWithEmail := Header{ + Name: "X-Forwarded-User", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + }, + }, + }, + } + + basicAuthHeaderWithEmail := Header{ + Name: "Authorization", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + BasicAuthPassword: &SecretSource{ + Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthSecret))), + }, + }, + }, + }, + } + + xAuthRequestUser := Header{ + Name: "X-Auth-Request-User", + PreserveRequestValue: false, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "user", + }, + }, + }, + } + + xAuthRequestEmail := Header{ + Name: "X-Auth-Request-Email", + PreserveRequestValue: false, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + }, + }, + }, + } + + xAuthRequestGroups := Header{ + Name: "X-Auth-Request-Groups", + PreserveRequestValue: false, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "groups", + }, + }, + }, + } + + xForwardedAccessToken := Header{ + Name: "X-Forwarded-Access-Token", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "access_token", + }, + }, + }, + } + + xAuthRequestAccessToken := Header{ + Name: "X-Auth-Request-Access-Token", + PreserveRequestValue: false, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "access_token", + }, + }, + }, + } + + authorizationHeader := Header{ + Name: "Authorization", + PreserveRequestValue: true, + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "id_token", + Prefix: "Bearer ", + }, + }, + }, + } + + DescribeTable("should convert to injectRequestHeaders", + func(in legacyHeadersTableInput) { + requestHeaders, responseHeaders := in.legacyHeaders.convert() + Expect(requestHeaders).To(ConsistOf(in.expectedRequestHeaders)) + Expect(responseHeaders).To(ConsistOf(in.expectedResponseHeaders)) + }, + Entry("with all header options off", legacyHeadersTableInput{ + legacyHeaders: &LegacyHeaders{ + PassBasicAuth: false, + PassAccessToken: false, + PassUserHeaders: false, + PassAuthorization: false, + + SetBasicAuth: false, + SetXAuthRequest: false, + SetAuthorization: false, + + PreferEmailToUser: false, + BasicAuthPassword: "", + SkipAuthStripHeaders: false, + }, + expectedRequestHeaders: []Header{}, + expectedResponseHeaders: []Header{}, + }), + Entry("with basic auth enabled", legacyHeadersTableInput{ + legacyHeaders: &LegacyHeaders{ + PassBasicAuth: true, + PassAccessToken: false, + PassUserHeaders: false, + PassAuthorization: false, + + SetBasicAuth: true, + SetXAuthRequest: false, + SetAuthorization: false, + + PreferEmailToUser: false, + BasicAuthPassword: basicAuthSecret, + SkipAuthStripHeaders: false, + }, + expectedRequestHeaders: []Header{ + xForwardedUser, + xForwardedEmail, + xForwardedGroups, + xForwardedPreferredUsername, + basicAuthHeader, + }, + expectedResponseHeaders: []Header{ + withPreserveRequestValue(basicAuthHeader, false), + }, + }), + Entry("with basic auth enabled and skipAuthStripHeaders", legacyHeadersTableInput{ + legacyHeaders: &LegacyHeaders{ + PassBasicAuth: true, + PassAccessToken: false, + PassUserHeaders: false, + PassAuthorization: false, + + SetBasicAuth: true, + SetXAuthRequest: false, + SetAuthorization: false, + + PreferEmailToUser: false, + BasicAuthPassword: basicAuthSecret, + SkipAuthStripHeaders: true, + }, + expectedRequestHeaders: []Header{ + withPreserveRequestValue(xForwardedUser, false), + withPreserveRequestValue(xForwardedEmail, false), + withPreserveRequestValue(xForwardedGroups, false), + withPreserveRequestValue(xForwardedPreferredUsername, false), + withPreserveRequestValue(basicAuthHeader, false), + }, + expectedResponseHeaders: []Header{ + withPreserveRequestValue(basicAuthHeader, false), + }, + }), + Entry("with basic auth enabled and preferEmailToUser", legacyHeadersTableInput{ + legacyHeaders: &LegacyHeaders{ + PassBasicAuth: true, + PassAccessToken: false, + PassUserHeaders: false, + PassAuthorization: false, + + SetBasicAuth: true, + SetXAuthRequest: false, + SetAuthorization: false, + + PreferEmailToUser: true, + BasicAuthPassword: basicAuthSecret, + SkipAuthStripHeaders: false, + }, + expectedRequestHeaders: []Header{ + xForwardedUserWithEmail, + xForwardedGroups, + xForwardedPreferredUsername, + basicAuthHeaderWithEmail, + }, + expectedResponseHeaders: []Header{ + withPreserveRequestValue(basicAuthHeaderWithEmail, false), + }, + }), + Entry("with basic auth enabled and passUserHeaders", legacyHeadersTableInput{ + legacyHeaders: &LegacyHeaders{ + PassBasicAuth: true, + PassAccessToken: false, + PassUserHeaders: true, + PassAuthorization: false, + + SetBasicAuth: true, + SetXAuthRequest: false, + SetAuthorization: false, + + PreferEmailToUser: false, + BasicAuthPassword: basicAuthSecret, + SkipAuthStripHeaders: false, + }, + expectedRequestHeaders: []Header{ + xForwardedUser, + xForwardedEmail, + xForwardedGroups, + xForwardedPreferredUsername, + basicAuthHeader, + }, + expectedResponseHeaders: []Header{ + withPreserveRequestValue(basicAuthHeader, false), + }, + }), + Entry("with passUserHeaders", legacyHeadersTableInput{ + legacyHeaders: &LegacyHeaders{ + PassBasicAuth: false, + PassAccessToken: false, + PassUserHeaders: true, + PassAuthorization: false, + + SetBasicAuth: false, + SetXAuthRequest: false, + SetAuthorization: false, + + PreferEmailToUser: false, + BasicAuthPassword: "", + SkipAuthStripHeaders: false, + }, + expectedRequestHeaders: []Header{ + xForwardedUser, + xForwardedEmail, + xForwardedGroups, + xForwardedPreferredUsername, + }, + expectedResponseHeaders: []Header{}, + }), + Entry("with setXAuthRequest", legacyHeadersTableInput{ + legacyHeaders: &LegacyHeaders{ + PassBasicAuth: false, + PassAccessToken: false, + PassUserHeaders: false, + PassAuthorization: false, + + SetBasicAuth: false, + SetXAuthRequest: true, + SetAuthorization: false, + + PreferEmailToUser: false, + BasicAuthPassword: "", + SkipAuthStripHeaders: false, + }, + expectedRequestHeaders: []Header{}, + expectedResponseHeaders: []Header{ + xAuthRequestUser, + xAuthRequestEmail, + xAuthRequestGroups, + withPreserveRequestValue(xForwardedPreferredUsername, false), + }, + }), + Entry("with passAccessToken", legacyHeadersTableInput{ + legacyHeaders: &LegacyHeaders{ + PassBasicAuth: false, + PassAccessToken: true, + PassUserHeaders: false, + PassAuthorization: false, + + SetBasicAuth: false, + SetXAuthRequest: false, + SetAuthorization: false, + + PreferEmailToUser: false, + BasicAuthPassword: "", + SkipAuthStripHeaders: false, + }, + expectedRequestHeaders: []Header{ + xForwardedAccessToken, + }, + expectedResponseHeaders: []Header{}, + }), + Entry("with passAcessToken and setXAuthRequest", legacyHeadersTableInput{ + legacyHeaders: &LegacyHeaders{ + PassBasicAuth: false, + PassAccessToken: true, + PassUserHeaders: false, + PassAuthorization: false, + + SetBasicAuth: false, + SetXAuthRequest: true, + SetAuthorization: false, + + PreferEmailToUser: false, + BasicAuthPassword: "", + SkipAuthStripHeaders: false, + }, + expectedRequestHeaders: []Header{ + xForwardedAccessToken, + }, + expectedResponseHeaders: []Header{ + xAuthRequestUser, + xAuthRequestEmail, + xAuthRequestGroups, + withPreserveRequestValue(xForwardedPreferredUsername, false), + xAuthRequestAccessToken, + }, + }), + 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: false, + }, + expectedRequestHeaders: []Header{ + authorizationHeader, + }, + expectedResponseHeaders: []Header{ + withPreserveRequestValue(authorizationHeader, false), + }, + }), + ) + }) }) From 1270104806a5c2dd0741a02016ff25e576a2d1e9 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 3 Nov 2020 17:50:23 +0000 Subject: [PATCH 7/7] Update changelog to include integration of new header injection --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b660dc1..87666b87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ ## Changes since v6.1.1 +- [#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) - [#825](https://github.com/oauth2-proxy/oauth2-proxy/pull/825) Fix code coverage reporting on GitHub actions(@JoelSpeed)