From 37e8e2aa9dd9b569595cd9fbb83cedfc262414f3 Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Sat, 29 Nov 2025 15:31:41 +0100 Subject: [PATCH 1/7] feat: support for cookie in alpha config and legacy file structure refactoring Signed-off-by: Jan Larwig --- .vscode/launch.json | 24 +- .../oauth2-proxy-alpha-config.cfg | 2 - .../oauth2-proxy-alpha-config.yaml | 17 +- docs/docs/configuration/alpha_config.md | 45 +- docs/docs/configuration/alpha_config.md.tmpl | 22 +- main.go | 24 +- main_test.go | 20 +- oauthproxy.go | 3 +- oauthproxy_test.go | 8 +- pkg/apis/options/alpha_options.go | 6 + pkg/apis/options/cookie.go | 124 +-- pkg/apis/options/cookie_test.go | 2 +- pkg/apis/options/legacy_cookie.go | 61 ++ pkg/apis/options/legacy_header.go | 283 +++++++ pkg/apis/options/legacy_options.go | 756 +----------------- pkg/apis/options/legacy_options_test.go | 50 ++ pkg/apis/options/legacy_provider.go | 317 ++++++++ pkg/apis/options/legacy_server.go | 79 ++ pkg/apis/options/legacy_upstream.go | 105 +++ pkg/apis/options/load_test.go | 15 +- pkg/apis/options/options.go | 7 +- pkg/apis/options/providers.go | 23 - pkg/cookies/cookies.go | 5 +- pkg/cookies/cookies_test.go | 13 +- pkg/cookies/csrf.go | 7 +- pkg/cookies/csrf_per_request_test.go | 7 +- pkg/cookies/csrf_test.go | 7 +- pkg/sessions/session_store_test.go | 5 +- pkg/sessions/tests/session_store_tests.go | 17 +- pkg/validation/cookie_test.go | 65 +- pkg/validation/options.go | 2 +- pkg/validation/options_test.go | 3 + 32 files changed, 1212 insertions(+), 912 deletions(-) create mode 100644 pkg/apis/options/legacy_cookie.go create mode 100644 pkg/apis/options/legacy_header.go create mode 100644 pkg/apis/options/legacy_provider.go create mode 100644 pkg/apis/options/legacy_server.go create mode 100644 pkg/apis/options/legacy_upstream.go diff --git a/.vscode/launch.json b/.vscode/launch.json index 36c7fb09..f63b3b9e 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -1,6 +1,18 @@ { "version": "0.2.0", "configurations": [ + { + "name": "Config conversion", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "${workspaceFolder}", + "args": [ + "--convert-config-to-alpha", + "--config", + "contrib/local-environment/oauth2-proxy.cfg" + ] + }, { "name": "OAuth2 Proxy for Dex", "type": "go", @@ -8,7 +20,8 @@ "mode": "auto", "program": "${workspaceFolder}", "args": [ - "--config", "contrib/local-environment/oauth2-proxy.cfg" + "--config", + "contrib/local-environment/oauth2-proxy.cfg" ] }, { @@ -18,7 +31,8 @@ "mode": "auto", "program": "${workspaceFolder}", "args": [ - "--config", "contrib/local-environment/oauth2-proxy-keycloak.cfg" + "--config", + "contrib/local-environment/oauth2-proxy-keycloak.cfg" ] }, { @@ -28,8 +42,10 @@ "mode": "auto", "program": "${workspaceFolder}", "args": [ - "--config", "contrib/local-environment/oauth2-proxy-alpha-config.cfg", - "--alpha-config", "contrib/local-environment/oauth2-proxy-alpha-config.yaml" + "--config", + "contrib/local-environment/oauth2-proxy-alpha-config.cfg", + "--alpha-config", + "contrib/local-environment/oauth2-proxy-alpha-config.yaml" ] } ] diff --git a/contrib/local-environment/oauth2-proxy-alpha-config.cfg b/contrib/local-environment/oauth2-proxy-alpha-config.cfg index c913ec4f..1a82f681 100644 --- a/contrib/local-environment/oauth2-proxy-alpha-config.cfg +++ b/contrib/local-environment/oauth2-proxy-alpha-config.cfg @@ -1,4 +1,2 @@ -cookie_secret="OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" email_domains="example.com" -cookie_secure="false" redirect_url="http://oauth2-proxy.localtest.me:4180/oauth2/callback" diff --git a/contrib/local-environment/oauth2-proxy-alpha-config.yaml b/contrib/local-environment/oauth2-proxy-alpha-config.yaml index e423db98..5a99e6a8 100644 --- a/contrib/local-environment/oauth2-proxy-alpha-config.yaml +++ b/contrib/local-environment/oauth2-proxy-alpha-config.yaml @@ -1,5 +1,15 @@ server: bindAddress: "0.0.0.0:4180" +cookie: + secret: OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w= + secure: false +providers: + - id: oidc + provider: oidc + clientSecret: b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK + clientID: oauth2-proxy + oidcConfig: + issuerURL: http://dex.localtest.me:5556/dex upstreamConfig: upstreams: - id: httpbin @@ -14,10 +24,3 @@ injectRequestHeaders: values: - claimSource: claim: email -providers: - - id: oidc - provider: oidc - clientSecret: b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK - clientID: oauth2-proxy - oidcConfig: - issuerURL: http://dex.localtest.me:5556/dex diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 495bc206..6f23f887 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -86,14 +86,14 @@ More information and available patterns can be found [here](https://github.com/a The following flags/options and their respective environment variables are no longer available when using alpha configuration: - +### Legacy Upstream options - `flush-interval`/`flush_interval` - `pass-host-header`/`pass_host_header` - `proxy-websockets`/`proxy_websockets` - `ssl-upstream-insecure-skip-verify`/`ssl_upstream_insecure_skip_verify` - `upstream`/`upstreams` - +### Legacy Headers options - `pass-basic-auth`/`pass_basic_auth` - `pass-access-token`/`pass_access_token` - `pass-user-headers`/`pass_user_headers` @@ -105,7 +105,7 @@ longer available when using alpha configuration: - `basic-auth-password`/`basic_auth_password` - `skip-auth-strip-headers`/`skip_auth_strip_headers` - +### Legacy Provider options - `client-id`/`client_id` - `client-secret`/`client_secret`, and `client-secret-file`/`client_secret_file` - `provider` @@ -127,6 +127,22 @@ longer available when using alpha configuration: - `jwt-key-file`/`jwt_key_file` - `pubjwk-url`/`pubjwk_url` +### Legacy Cookie options +- `cookie-name`/`cookie_name` +- `cookie-name`/`cookie_name` +- `cookie-secret`/`cookie_secret` +- `cookie-secret-file`/`cookie_secret_file` +- `cookie-domain`/`cookie_domains` +- `cookie-path`/`cookie_path` +- `cookie-expire`/`cookie_expire` +- `cookie-refresh`/`cookie_refresh` +- `cookie-secure`/`cookie_secure` +- `cookie-httponly`/`cookie_httponly` +- `cookie-samesite`/`cookie_samesite` +- `cookie-csrf-per-request`/`cookie_csrf_per_request` +- `cookie-csrf-per-request-limit`/`cookie_csrf_per_request_limit` +- `cookie-csrf-expire`/`cookie_csrf_expire` + and all provider-specific options, i.e. any option whose name includes `oidc`, `azure`, `bitbucket`, `github`, `gitlab`, `google` or `keycloak`. Attempting to use any of these options via flags or via config when `--alpha-config` is @@ -169,6 +185,7 @@ They may change between releases without notice. | `server` | _[Server](#server)_ | Server is used to configure the HTTP(S) server for the proxy application.
You may choose to run both HTTP and HTTPS servers simultaneously.
This can be done by setting the BindAddress and the SecureBindAddress simultaneously.
To use the secure server you must configure a TLS certificate and key. | | `metricsServer` | _[Server](#server)_ | MetricsServer is used to configure the HTTP(S) server for metrics.
You may choose to run both HTTP and HTTPS servers simultaneously.
This can be done by setting the BindAddress and the SecureBindAddress simultaneously.
To use the secure server you must configure a TLS certificate and key. | | `providers` | _[Providers](#providers)_ | Providers is used to configure your provider. **Multiple-providers is not
yet working.** [This feature is tracked in
#925](https://github.com/oauth2-proxy/oauth2-proxy/issues/926) | +| `cookie` | _[Cookie](#cookie)_ | Cookie is used to configure the cookies used by OAuth2 Proxy.
This includes session and CSRF cookies. | ### AzureOptions @@ -204,6 +221,28 @@ ClaimSource allows loading a header value from a claim within the session | `prefix` | _string_ | Prefix is an optional prefix that will be prepended to the value of the
claim if it is non-empty. | | `basicAuthPassword` | _[SecretSource](#secretsource)_ | BasicAuthPassword converts this claim into a basic auth header.
Note the value of claim will become the basic auth username and the
basicAuthPassword will be used as the password value. | +### Cookie + +(**Appears on:** [AlphaOptions](#alphaoptions)) + +Cookie contains configuration options relating session and CSRF cookies + +| Field | Type | Description | +| ----- | ---- | ----------- | +| `name` | _string_ | Name is the name of the cookie | +| `secret` | _string_ | Secret is the secret used to encrypt/sign the cookie value | +| `secretFile` | _string_ | SecretFile is a file containing the secret used to encrypt/sign the cookie value
instead of specifying it directly in the config. Secret takes precedence over SecretFile | +| `domains` | _[]string_ | Domains is a list of domains for which the cookie is valid | +| `path` | _string_ | Path is the path for which the cookie is valid | +| `expire` | _duration_ | Expire is the duration before the cookie expires | +| `refresh` | _duration_ | Refresh is the duration after which the cookie is refreshable | +| `secure` | _bool_ | Secure indicates whether the cookie is only sent over HTTPS | +| `httpOnly` | _bool_ | HTTPOnly indicates whether the cookie is inaccessible to JavaScript | +| `sameSite` | _string_ | SameSite sets the SameSite attribute on the cookie | +| `csrfPerRequest` | _bool_ | CSRFPerRequest indicates whether a unique CSRF token is generated for each request
Enables parallel requests from clients (e.g., multiple tabs) | +| `csrfPerRequestLimit` | _int_ | CSRFPerRequestLimit sets a limit on the number of valid CSRF tokens when CSRFPerRequest is enabled
Used to prevent unbounded memory growth from storing too many tokens | +| `csrfExpire` | _duration_ | CSRFExpire sets the duration before a CSRF token expires | + ### GitHubOptions (**Appears on:** [Provider](#provider)) diff --git a/docs/docs/configuration/alpha_config.md.tmpl b/docs/docs/configuration/alpha_config.md.tmpl index 8258201f..95c07534 100644 --- a/docs/docs/configuration/alpha_config.md.tmpl +++ b/docs/docs/configuration/alpha_config.md.tmpl @@ -86,14 +86,14 @@ More information and available patterns can be found [here](https://github.com/a The following flags/options and their respective environment variables are no longer available when using alpha configuration: - +### Legacy Upstream options - `flush-interval`/`flush_interval` - `pass-host-header`/`pass_host_header` - `proxy-websockets`/`proxy_websockets` - `ssl-upstream-insecure-skip-verify`/`ssl_upstream_insecure_skip_verify` - `upstream`/`upstreams` - +### Legacy Headers options - `pass-basic-auth`/`pass_basic_auth` - `pass-access-token`/`pass_access_token` - `pass-user-headers`/`pass_user_headers` @@ -105,7 +105,7 @@ longer available when using alpha configuration: - `basic-auth-password`/`basic_auth_password` - `skip-auth-strip-headers`/`skip_auth_strip_headers` - +### Legacy Provider options - `client-id`/`client_id` - `client-secret`/`client_secret`, and `client-secret-file`/`client_secret_file` - `provider` @@ -127,6 +127,22 @@ longer available when using alpha configuration: - `jwt-key-file`/`jwt_key_file` - `pubjwk-url`/`pubjwk_url` +### Legacy Cookie options +- `cookie-name`/`cookie_name` +- `cookie-name`/`cookie_name` +- `cookie-secret`/`cookie_secret` +- `cookie-secret-file`/`cookie_secret_file` +- `cookie-domain`/`cookie_domains` +- `cookie-path`/`cookie_path` +- `cookie-expire`/`cookie_expire` +- `cookie-refresh`/`cookie_refresh` +- `cookie-secure`/`cookie_secure` +- `cookie-httponly`/`cookie_httponly` +- `cookie-samesite`/`cookie_samesite` +- `cookie-csrf-per-request`/`cookie_csrf_per_request` +- `cookie-csrf-per-request-limit`/`cookie_csrf_per_request_limit` +- `cookie-csrf-expire`/`cookie_csrf_expire` + and all provider-specific options, i.e. any option whose name includes `oidc`, `azure`, `bitbucket`, `github`, `gitlab`, `google` or `keycloak`. Attempting to use any of these options via flags or via config when `--alpha-config` is diff --git a/main.go b/main.go index 42e8bab0..2a2d99a3 100644 --- a/main.go +++ b/main.go @@ -97,7 +97,7 @@ func loadLegacyOptions(config string, extraFlags *pflag.FlagSet, args []string) legacyOpts := options.NewLegacyOptions() if err := options.Load(config, optionsFlagSet, legacyOpts); err != nil { - return nil, fmt.Errorf("failed to load config: %v", err) + return nil, fmt.Errorf("failed to load legacy config: %v", err) } opts, err := legacyOpts.ToOptions() @@ -150,6 +150,28 @@ func loadOptions(config string, extraFlags *pflag.FlagSet, args []string) (*opti func printConvertedConfig(opts *options.Options) error { alphaConfig := options.NewAlphaOptions(opts) + if len(alphaConfig.Providers) == 1 { + providerType := alphaConfig.Providers[0].Type + + if providerType != options.ADFSProvider { + alphaConfig.Providers[0].ADFSConfig = options.ADFSOptions{} + } + + if providerType != options.AzureProvider { + alphaConfig.Providers[0].AzureConfig = options.AzureOptions{} + } + + if providerType != options.GoogleProvider { + alphaConfig.Providers[0].GoogleConfig = options.GoogleOptions{} + } + + if providerType != options.MicrosoftEntraIDProvider { + alphaConfig.Providers[0].MicrosoftEntraIDConfig = options.MicrosoftEntraIDOptions{} + } + } else { + return fmt.Errorf("only single provider conversion is supported") + } + // Generic interface for loading arbitrary yaml structure var buffer map[string]interface{} diff --git a/main_test.go b/main_test.go index cbe79683..f896c398 100644 --- a/main_test.go +++ b/main_test.go @@ -15,18 +15,24 @@ import ( var _ = Describe("Configuration Loading Suite", func() { // For comparing the full configuration differences of our structs we need to increase the gomega limits - format.MaxLength = 50000 + format.MaxLength = 0 format.MaxDepth = 10 const testLegacyConfig = ` http_address="127.0.0.1:4180" upstreams="http://httpbin" + set_basic_auth="true" basic_auth_password="c3VwZXItc2VjcmV0LXBhc3N3b3Jk" + client_id="oauth2-proxy" client_secret="b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK" + google_admin_email="admin@example.com" google_target_principal="principal" + +cookie_secret="OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" +cookie_secure="false" ` const testAlphaConfig = ` @@ -80,6 +86,9 @@ injectResponseHeaders: value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk server: bindAddress: "127.0.0.1:4180" +cookie: + secure: false + secret: "OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" providers: - id: google=oauth2-proxy provider: google @@ -106,10 +115,7 @@ providers: ` const testCoreConfig = ` -cookie_secret="OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" email_domains="example.com" -cookie_secure="false" - redirect_url="http://localhost:4180/oauth2/callback" ` @@ -119,7 +125,7 @@ redirect_url="http://localhost:4180/oauth2/callback" opts.Cookie.Secret = "OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" opts.EmailDomains = []string{"example.com"} - opts.Cookie.Secure = false + opts.Cookie.Secure = ptr.To(false) opts.RawRedirectURL = "http://localhost:4180/oauth2/callback" opts.UpstreamServers = options.UpstreamConfig{ @@ -276,7 +282,7 @@ redirect_url="http://localhost:4180/oauth2/callback" Entry("with bad legacy configuration", loadConfigurationTableInput{ configContent: testCoreConfig + "unknown_field=\"something\"", expectedOptions: func() *options.Options { return nil }, - expectedErr: errors.New("failed to load legacy options: failed to load config: error unmarshalling config: decoding failed due to the following error(s):\n\n'' has invalid keys: unknown_field"), + expectedErr: errors.New("failed to load legacy options: failed to load legacy config: error unmarshalling config: decoding failed due to the following error(s):\n\n'' has invalid keys: unknown_field"), }), Entry("with bad alpha configuration", loadConfigurationTableInput{ configContent: testCoreConfig, @@ -288,7 +294,7 @@ redirect_url="http://localhost:4180/oauth2/callback" configContent: testCoreConfig + "unknown_field=\"something\"", alphaConfigContent: testAlphaConfig, expectedOptions: func() *options.Options { return nil }, - expectedErr: errors.New("failed to load legacy options: failed to load config: error unmarshalling config: decoding failed due to the following error(s):\n\n'' has invalid keys: unknown_field"), + expectedErr: errors.New("failed to load legacy options: failed to load legacy config: error unmarshalling config: decoding failed due to the following error(s):\n\n'' has invalid keys: unknown_field"), }), ) }) diff --git a/oauthproxy.go b/oauthproxy.go index d933d930..a42b7325 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -30,6 +30,7 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/proxyhttp" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/version" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip" @@ -1098,7 +1099,7 @@ func (p *OAuthProxy) getOAuthRedirectURI(req *http.Request) string { // If CookieSecure is true, return `https` no matter what // Not all reverse proxies set X-Forwarded-Proto - if p.CookieOptions.Secure { + if ptr.Deref(p.CookieOptions.Secure, options.DefaultCookieSecure) { rd.Scheme = schemeHTTPS } return rd.String() diff --git a/oauthproxy_test.go b/oauthproxy_test.go index ccabdbbd..20360af4 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -207,7 +207,7 @@ func TestBasicAuthPassword(t *testing.T) { }, } - opts.Cookie.Secure = false + opts.Cookie.Secure = ptr.To(false) opts.InjectRequestHeaders = []options.Header{ { Name: "Authorization", @@ -362,7 +362,7 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) (*PassAccessTokenTe patt.opts.UpstreamServers.Upstreams = append(patt.opts.UpstreamServers.Upstreams, opts.ProxyUpstream) } - patt.opts.Cookie.Secure = false + patt.opts.Cookie.Secure = ptr.To(false) if opts.PassAccessToken { patt.opts.InjectRequestHeaders = []options.Header{ { @@ -2073,6 +2073,8 @@ func baseTestOptions() *options.Options { }, } + opts.EnsureDefaults() + return opts } @@ -3468,7 +3470,7 @@ func TestGetOAuthRedirectURI(t *testing.T) { { name: "redirect with http schema", setupOpts: func(baseOpts *options.Options) *options.Options { - baseOpts.Cookie.Secure = false + baseOpts.Cookie.Secure = ptr.To(false) return baseOpts }, req: &http.Request{ diff --git a/pkg/apis/options/alpha_options.go b/pkg/apis/options/alpha_options.go index 33daf17f..977bdb18 100644 --- a/pkg/apis/options/alpha_options.go +++ b/pkg/apis/options/alpha_options.go @@ -45,6 +45,10 @@ type AlphaOptions struct { // yet working.** [This feature is tracked in // #925](https://github.com/oauth2-proxy/oauth2-proxy/issues/926) Providers Providers `yaml:"providers,omitempty"` + + // Cookie is used to configure the cookies used by OAuth2 Proxy. + // This includes session and CSRF cookies. + Cookie Cookie `yaml:"cookie,omitempty"` } // Initialize alpha options with default values and settings of the core options @@ -63,6 +67,7 @@ func (a *AlphaOptions) ExtractFrom(opts *Options) { a.Server = opts.Server a.MetricsServer = opts.MetricsServer a.Providers = opts.Providers + a.Cookie = opts.Cookie } // MergeOptionsWithDefaults replaces alpha options in the Options struct @@ -74,4 +79,5 @@ func (a *AlphaOptions) MergeOptionsWithDefaults(opts *Options) { opts.Server = a.Server opts.MetricsServer = a.MetricsServer opts.Providers = a.Providers + opts.Cookie = a.Cookie } diff --git a/pkg/apis/options/cookie.go b/pkg/apis/options/cookie.go index 3653b7d0..2d594b75 100644 --- a/pkg/apis/options/cookie.go +++ b/pkg/apis/options/cookie.go @@ -1,67 +1,53 @@ package options import ( - "errors" + "fmt" "os" "time" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" - "github.com/spf13/pflag" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" ) -// Cookie contains configuration options relating to Cookie configuration +const ( + // DefaultCookieSecure is the default value for Cookie.Secure + DefaultCookieSecure bool = true + // DefaultCookieHTTPOnly is the default value for Cookie.HTTPOnly + DefaultCookieHTTPOnly bool = true + // DefaultCSRFPerRequest is the default value for Cookie.CSRFPerRequest + DefaultCSRFPerRequest bool = false +) + +// Cookie contains configuration options relating session and CSRF cookies type Cookie struct { - Name string `flag:"cookie-name" cfg:"cookie_name"` - Secret string `flag:"cookie-secret" cfg:"cookie_secret"` - SecretFile string `flag:"cookie-secret-file" cfg:"cookie_secret_file"` - Domains []string `flag:"cookie-domain" cfg:"cookie_domains"` - Path string `flag:"cookie-path" cfg:"cookie_path"` - Expire time.Duration `flag:"cookie-expire" cfg:"cookie_expire"` - Refresh time.Duration `flag:"cookie-refresh" cfg:"cookie_refresh"` - Secure bool `flag:"cookie-secure" cfg:"cookie_secure"` - HTTPOnly bool `flag:"cookie-httponly" cfg:"cookie_httponly"` - SameSite string `flag:"cookie-samesite" cfg:"cookie_samesite"` - CSRFPerRequest bool `flag:"cookie-csrf-per-request" cfg:"cookie_csrf_per_request"` - CSRFPerRequestLimit int `flag:"cookie-csrf-per-request-limit" cfg:"cookie_csrf_per_request_limit"` - CSRFExpire time.Duration `flag:"cookie-csrf-expire" cfg:"cookie_csrf_expire"` -} - -func cookieFlagSet() *pflag.FlagSet { - flagSet := pflag.NewFlagSet("cookie", pflag.ExitOnError) - - flagSet.String("cookie-name", "_oauth2_proxy", "the name of the cookie that the oauth_proxy creates") - flagSet.String("cookie-secret", "", "the seed string for secure cookies (optionally base64 encoded)") - flagSet.String("cookie-secret-file", "", "For defining a separate cookie secret file to read the encryption key from") - flagSet.StringSlice("cookie-domain", []string{}, "Optional cookie domains to force cookies to (ie: `.yourcompany.com`). The longest domain matching the request's host will be used (or the shortest cookie domain if there is no match).") - flagSet.String("cookie-path", "/", "an optional cookie path to force cookies to (ie: /poc/)*") - flagSet.Duration("cookie-expire", time.Duration(168)*time.Hour, "expire timeframe for cookie") - flagSet.Duration("cookie-refresh", time.Duration(0), "refresh the cookie after this duration; 0 to disable") - flagSet.Bool("cookie-secure", true, "set secure (HTTPS) cookie flag") - flagSet.Bool("cookie-httponly", true, "set HttpOnly cookie flag") - flagSet.String("cookie-samesite", "", "set SameSite cookie attribute (ie: \"lax\", \"strict\", \"none\", or \"\"). ") - flagSet.Bool("cookie-csrf-per-request", false, "When this property is set to true, then the CSRF cookie name is built based on the state and varies per request. If property is set to false, then CSRF cookie has the same name for all requests.") - flagSet.Int("cookie-csrf-per-request-limit", 0, "Sets a limit on the number of CSRF requests cookies that oauth2-proxy will create. The oldest cookies will be removed. Useful if users end up with 431 Request headers too large status codes.") - flagSet.Duration("cookie-csrf-expire", time.Duration(15)*time.Minute, "expire timeframe for CSRF cookie") - return flagSet -} - -// cookieDefaults creates a Cookie populating each field with its default value -func cookieDefaults() Cookie { - return Cookie{ - Name: "_oauth2_proxy", - Secret: "", - SecretFile: "", - Domains: nil, - Path: "/", - Expire: time.Duration(168) * time.Hour, - Refresh: time.Duration(0), - Secure: true, - HTTPOnly: true, - SameSite: "", - CSRFPerRequest: false, - CSRFPerRequestLimit: 0, - CSRFExpire: time.Duration(15) * time.Minute, - } + // Name is the name of the cookie + Name string `yaml:"name,omitempty"` + // Secret is the secret used to encrypt/sign the cookie value + Secret string `yaml:"secret,omitempty"` + // SecretFile is a file containing the secret used to encrypt/sign the cookie value + // instead of specifying it directly in the config. Secret takes precedence over SecretFile + SecretFile string `yaml:"secretFile,omitempty"` + // Domains is a list of domains for which the cookie is valid + Domains []string `yaml:"domains,omitempty"` + // Path is the path for which the cookie is valid + Path string `yaml:"path,omitempty"` + // Expire is the duration before the cookie expires + Expire time.Duration `yaml:"expire,omitempty"` + // Refresh is the duration after which the cookie is refreshable + Refresh time.Duration `yaml:"refresh,omitempty"` + // Secure indicates whether the cookie is only sent over HTTPS + Secure *bool `yaml:"secure,omitempty"` + // HTTPOnly indicates whether the cookie is inaccessible to JavaScript + HTTPOnly *bool `yaml:"httpOnly,omitempty"` + // SameSite sets the SameSite attribute on the cookie + SameSite string `yaml:"sameSite,omitempty"` + // CSRFPerRequest indicates whether a unique CSRF token is generated for each request + // Enables parallel requests from clients (e.g., multiple tabs) + CSRFPerRequest *bool `yaml:"csrfPerRequest,omitempty"` + // CSRFPerRequestLimit sets a limit on the number of valid CSRF tokens when CSRFPerRequest is enabled + // Used to prevent unbounded memory growth from storing too many tokens + CSRFPerRequestLimit int `yaml:"csrfPerRequestLimit,omitempty"` + // CSRFExpire sets the duration before a CSRF token expires + CSRFExpire time.Duration `yaml:"csrfExpire,omitempty"` } // GetSecret returns the cookie secret, reading from file if SecretFile is set @@ -72,9 +58,33 @@ func (c *Cookie) GetSecret() (secret string, err error) { fileSecret, err := os.ReadFile(c.SecretFile) if err != nil { - logger.Errorf("error reading cookie secret file %s: %s", c.SecretFile, err) - return "", errors.New("could not read cookie secret file") + return "", fmt.Errorf("error reading cookie secret file %s: %w", c.SecretFile, err) } return string(fileSecret), nil } + +// EnsureDefaults sets any default values for the Cookie configuration +func (c *Cookie) EnsureDefaults() { + if c.Name == "" { + c.Name = "_oauth2_proxy" + } + if c.Path == "" { + c.Path = "/" + } + if c.Expire == 0 { + c.Expire = time.Duration(168) * time.Hour + } + if c.Secure == nil { + c.Secure = ptr.To(DefaultCookieSecure) + } + if c.HTTPOnly == nil { + c.HTTPOnly = ptr.To(DefaultCookieHTTPOnly) + } + if c.CSRFPerRequest == nil { + c.CSRFPerRequest = ptr.To(DefaultCSRFPerRequest) + } + if c.CSRFExpire == 0 { + c.CSRFExpire = time.Duration(15) * time.Minute + } +} diff --git a/pkg/apis/options/cookie_test.go b/pkg/apis/options/cookie_test.go index a1486fed..4fb0e62e 100644 --- a/pkg/apis/options/cookie_test.go +++ b/pkg/apis/options/cookie_test.go @@ -55,7 +55,7 @@ func TestCookieGetSecret(t *testing.T) { secret, err := c.GetSecret() assert.Error(t, err) assert.Equal(t, "", secret) - assert.Contains(t, err.Error(), "could not read cookie secret file") + assert.Contains(t, err.Error(), "error reading cookie secret file /nonexistent/file:") }) t.Run("returns empty when both Secret and SecretFile are empty", func(t *testing.T) { diff --git a/pkg/apis/options/legacy_cookie.go b/pkg/apis/options/legacy_cookie.go new file mode 100644 index 00000000..287db87f --- /dev/null +++ b/pkg/apis/options/legacy_cookie.go @@ -0,0 +1,61 @@ +package options + +import ( + "time" + + "github.com/spf13/pflag" +) + +// LegacyCookie contains configuration options relating to Cookie configuration +type LegacyCookie struct { + Name string `flag:"cookie-name" cfg:"cookie_name"` + Secret string `flag:"cookie-secret" cfg:"cookie_secret"` + SecretFile string `flag:"cookie-secret-file" cfg:"cookie_secret_file"` + Domains []string `flag:"cookie-domain" cfg:"cookie_domains"` + Path string `flag:"cookie-path" cfg:"cookie_path"` + Expire time.Duration `flag:"cookie-expire" cfg:"cookie_expire"` + Refresh time.Duration `flag:"cookie-refresh" cfg:"cookie_refresh"` + Secure bool `flag:"cookie-secure" cfg:"cookie_secure"` + HTTPOnly bool `flag:"cookie-httponly" cfg:"cookie_httponly"` + SameSite string `flag:"cookie-samesite" cfg:"cookie_samesite"` + CSRFPerRequest bool `flag:"cookie-csrf-per-request" cfg:"cookie_csrf_per_request"` + CSRFPerRequestLimit int `flag:"cookie-csrf-per-request-limit" cfg:"cookie_csrf_per_request_limit"` + CSRFExpire time.Duration `flag:"cookie-csrf-expire" cfg:"cookie_csrf_expire"` +} + +func legacyCookieFlagSet() *pflag.FlagSet { + flagSet := pflag.NewFlagSet("cookie", pflag.ExitOnError) + + flagSet.String("cookie-name", "_oauth2_proxy", "the name of the cookie that the oauth_proxy creates") + flagSet.String("cookie-secret", "", "the seed string for secure cookies (optionally base64 encoded)") + flagSet.String("cookie-secret-file", "", "For defining a separate cookie secret file to read the encryption key from") + flagSet.StringSlice("cookie-domain", []string{}, "Optional cookie domains to force cookies to (ie: `.yourcompany.com`). The longest domain matching the request's host will be used (or the shortest cookie domain if there is no match).") + flagSet.String("cookie-path", "/", "an optional cookie path to force cookies to (ie: /poc/)*") + flagSet.Duration("cookie-expire", time.Duration(168)*time.Hour, "expire timeframe for cookie") + flagSet.Duration("cookie-refresh", time.Duration(0), "refresh the cookie after this duration; 0 to disable") + flagSet.Bool("cookie-secure", true, "set secure (HTTPS) cookie flag") + flagSet.Bool("cookie-httponly", true, "set HttpOnly cookie flag") + flagSet.String("cookie-samesite", "", "set SameSite cookie attribute (ie: \"lax\", \"strict\", \"none\", or \"\"). ") + flagSet.Bool("cookie-csrf-per-request", false, "When this property is set to true, then the CSRF cookie name is built based on the state and varies per request. If property is set to false, then CSRF cookie has the same name for all requests.") + flagSet.Int("cookie-csrf-per-request-limit", 0, "Sets a limit on the number of CSRF requests cookies that oauth2-proxy will create. The oldest cookies will be removed. Useful if users end up with 431 Request headers too large status codes.") + flagSet.Duration("cookie-csrf-expire", time.Duration(15)*time.Minute, "expire timeframe for CSRF cookie") + return flagSet +} + +func (l *LegacyCookie) convert() Cookie { + return Cookie{ + Name: l.Name, + Secret: l.Secret, + SecretFile: l.SecretFile, + Domains: l.Domains, + Path: l.Path, + Expire: l.Expire, + Refresh: l.Refresh, + Secure: &l.Secure, + HTTPOnly: &l.HTTPOnly, + SameSite: l.SameSite, + CSRFPerRequest: &l.CSRFPerRequest, + CSRFPerRequestLimit: l.CSRFPerRequestLimit, + CSRFExpire: l.CSRFExpire, + } +} diff --git a/pkg/apis/options/legacy_header.go b/pkg/apis/options/legacy_header.go new file mode 100644 index 00000000..58c337a8 --- /dev/null +++ b/pkg/apis/options/legacy_header.go @@ -0,0 +1,283 @@ +package options + +import ( + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" + "github.com/spf13/pflag" +) + +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", true, "strips X-Forwarded-* style authentication headers & Authorization header if they would be set by oauth2-proxy") + + 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 = ptr.To(!l.SkipAuthStripHeaders) + } + + return requestHeaders +} + +func (l *LegacyHeaders) getResponseHeaders() []Header { + responseHeaders := []Header{} + + if l.SetXAuthRequest { + responseHeaders = append(responseHeaders, getXAuthRequestHeaders()...) + if l.PassAccessToken { + responseHeaders = append(responseHeaders, getXAuthRequestAccessTokenHeader()) + } + } + + 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", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: claim, + Prefix: "Basic ", + BasicAuthPassword: &SecretSource{ + Value: []byte(basicAuthPassword), + }, + }, + }, + }, + } +} + +func getPassUserHeaders(preferEmailToUser bool) []Header { + headers := []Header{ + { + Name: "X-Forwarded-Groups", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "groups", + }, + }, + }, + }, + } + + if preferEmailToUser { + return append(headers, + Header{ + Name: "X-Forwarded-User", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + }, + }, + }, + }, + ) + } + + return append(headers, + Header{ + Name: "X-Forwarded-User", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "user", + }, + }, + }, + }, + Header{ + Name: "X-Forwarded-Email", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + }, + }, + }, + }, + ) +} + +func getPassAccessTokenHeader() Header { + return Header{ + Name: "X-Forwarded-Access-Token", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "access_token", + }, + }, + }, + } +} + +func getAuthorizationHeader() Header { + return Header{ + Name: "Authorization", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "id_token", + Prefix: "Bearer ", + }, + }, + }, + } +} + +func getPreferredUsernameHeader() Header { + return Header{ + Name: "X-Forwarded-Preferred-Username", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "preferred_username", + }, + }, + }, + } +} + +func getXAuthRequestHeaders() []Header { + headers := []Header{ + { + Name: "X-Auth-Request-User", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "user", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Email", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "email", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Preferred-Username", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "preferred_username", + }, + }, + }, + }, + { + Name: "X-Auth-Request-Groups", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "groups", + }, + }, + }, + }, + } + + return headers +} + +func getXAuthRequestAccessTokenHeader() Header { + return Header{ + Name: "X-Auth-Request-Access-Token", + PreserveRequestValue: ptr.To(false), + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "access_token", + }, + }, + }, + } +} diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 6ce730fd..d8033a0d 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -2,14 +2,8 @@ package options import ( "fmt" - "net/url" - "reflect" - "strconv" - "strings" "time" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" "github.com/spf13/pflag" ) @@ -26,6 +20,9 @@ type LegacyOptions struct { // Legacy options for single provider LegacyProvider LegacyProvider `cfg:",squash"` + // Legacy options for cookie configuration + LegacyCookie LegacyCookie `cfg:",squash"` + Options Options `cfg:",squash"` } @@ -62,6 +59,21 @@ func NewLegacyOptions() *LegacyOptions { InsecureOIDCSkipNonce: true, }, + LegacyCookie: LegacyCookie{ + Name: "_oauth2_proxy", + Secret: "", + Domains: nil, + Path: "/", + Expire: time.Duration(168) * time.Hour, + Refresh: time.Duration(0), + Secure: true, + HTTPOnly: true, + SameSite: "", + CSRFPerRequest: false, + CSRFPerRequestLimit: 0, + CSRFExpire: time.Duration(15) * time.Minute, + }, + Options: *NewOptions(), } } @@ -74,6 +86,7 @@ func NewLegacyFlagSet() *pflag.FlagSet { flagSet.AddFlagSet(legacyServerFlagset()) flagSet.AddFlagSet(legacyProviderFlagSet()) flagSet.AddFlagSet(legacyGoogleFlagSet()) + flagSet.AddFlagSet(legacyCookieFlagSet()) return flagSet } @@ -88,7 +101,6 @@ func (l *LegacyOptions) ToOptions() (*Options, error) { l.Options.InjectRequestHeaders, l.Options.InjectResponseHeaders = l.LegacyHeaders.convert() l.Options.Server, l.Options.MetricsServer = l.LegacyServer.convert() - l.Options.LegacyPreferEmailToUser = l.LegacyHeaders.PreferEmailToUser providers, err := l.LegacyProvider.convert() @@ -96,735 +108,9 @@ func (l *LegacyOptions) ToOptions() (*Options, error) { return nil, fmt.Errorf("error converting provider: %v", err) } l.Options.Providers = providers + l.Options.Cookie = l.LegacyCookie.convert() + l.Options.EnsureDefaults() return &l.Options, nil } - -type LegacyUpstreams struct { - FlushInterval time.Duration `flag:"flush-interval" cfg:"flush_interval"` - PassHostHeader bool `flag:"pass-host-header" cfg:"pass_host_header"` - ProxyWebSockets bool `flag:"proxy-websockets" cfg:"proxy_websockets"` - SSLUpstreamInsecureSkipVerify bool `flag:"ssl-upstream-insecure-skip-verify" cfg:"ssl_upstream_insecure_skip_verify"` - Upstreams []string `flag:"upstream" cfg:"upstreams"` - Timeout time.Duration `flag:"upstream-timeout" cfg:"upstream_timeout"` - DisableKeepAlives bool `flag:"disable-keep-alives" cfg:"disable_keep_alives"` -} - -func legacyUpstreamsFlagSet() *pflag.FlagSet { - flagSet := pflag.NewFlagSet("upstreams", pflag.ExitOnError) - - flagSet.Duration("flush-interval", DefaultUpstreamFlushInterval, "period between response flushing when streaming responses") - flagSet.Bool("pass-host-header", true, "pass the request Host Header to upstream") - flagSet.Bool("proxy-websockets", true, "enables WebSocket proxying") - flagSet.Bool("ssl-upstream-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS upstreams") - flagSet.StringSlice("upstream", []string{}, "the http url(s) of the upstream endpoint, file:// paths for static files or static:// for static response. Routing is based on the path") - flagSet.Duration("upstream-timeout", DefaultUpstreamTimeout, "maximum amount of time the server will wait for a response from the upstream") - flagSet.Bool("disable-keep-alives", false, "disable HTTP keep-alive connections to the upstream server") - - return flagSet -} - -func (l *LegacyUpstreams) convert() (UpstreamConfig, error) { - upstreams := UpstreamConfig{} - - for _, upstreamString := range l.Upstreams { - u, err := url.Parse(upstreamString) - if err != nil { - return UpstreamConfig{}, fmt.Errorf("could not parse upstream %q: %v", upstreamString, err) - } - - if u.Path == "" { - u.Path = "/" - } - - flushInterval := l.FlushInterval - timeout := l.Timeout - upstream := Upstream{ - ID: u.Path, - Path: u.Path, - URI: upstreamString, - InsecureSkipTLSVerify: &l.SSLUpstreamInsecureSkipVerify, - PassHostHeader: &l.PassHostHeader, - ProxyWebSockets: &l.ProxyWebSockets, - FlushInterval: &flushInterval, - Timeout: &timeout, - DisableKeepAlives: &l.DisableKeepAlives, - } - - switch u.Scheme { - case "file": - if u.Fragment != "" { - upstream.ID = u.Fragment - upstream.Path = u.Fragment - // Trim the fragment from the end of the URI - upstream.URI = strings.SplitN(upstreamString, "#", 2)[0] - } - case "static": - responseCode, err := strconv.Atoi(u.Host) - if err != nil { - logger.Errorf("unable to convert %q to int, use default \"200\"", u.Host) - responseCode = 200 - } - upstream.Static = ptr.To(true) - upstream.StaticCode = &responseCode - - // This is not allowed to be empty and must be unique - upstream.ID = upstreamString - - // We only support the root path in the legacy config - upstream.Path = "/" - - // Force defaults compatible with static responses - upstream.URI = "" - upstream.InsecureSkipTLSVerify = ptr.To(false) - upstream.DisableKeepAlives = ptr.To(false) - upstream.PassHostHeader = nil - upstream.ProxyWebSockets = nil - upstream.FlushInterval = nil - upstream.Timeout = nil - case "unix": - upstream.Path = "/" - } - - upstreams.Upstreams = append(upstreams.Upstreams, upstream) - } - - 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", true, "strips X-Forwarded-* style authentication headers & Authorization header if they would be set by oauth2-proxy") - - 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 = ptr.To(!l.SkipAuthStripHeaders) - } - - return requestHeaders -} - -func (l *LegacyHeaders) getResponseHeaders() []Header { - responseHeaders := []Header{} - - if l.SetXAuthRequest { - responseHeaders = append(responseHeaders, getXAuthRequestHeaders()...) - if l.PassAccessToken { - responseHeaders = append(responseHeaders, getXAuthRequestAccessTokenHeader()) - } - } - - 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", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: claim, - Prefix: "Basic ", - BasicAuthPassword: &SecretSource{ - Value: []byte(basicAuthPassword), - }, - }, - }, - }, - } -} - -func getPassUserHeaders(preferEmailToUser bool) []Header { - headers := []Header{ - { - Name: "X-Forwarded-Groups", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "groups", - }, - }, - }, - }, - } - - if preferEmailToUser { - return append(headers, - Header{ - Name: "X-Forwarded-User", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "email", - }, - }, - }, - }, - ) - } - - return append(headers, - Header{ - Name: "X-Forwarded-User", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "user", - }, - }, - }, - }, - Header{ - Name: "X-Forwarded-Email", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "email", - }, - }, - }, - }, - ) -} - -func getPassAccessTokenHeader() Header { - return Header{ - Name: "X-Forwarded-Access-Token", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "access_token", - }, - }, - }, - } -} - -func getAuthorizationHeader() Header { - return Header{ - Name: "Authorization", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "id_token", - Prefix: "Bearer ", - }, - }, - }, - } -} - -func getPreferredUsernameHeader() Header { - return Header{ - Name: "X-Forwarded-Preferred-Username", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "preferred_username", - }, - }, - }, - } -} - -func getXAuthRequestHeaders() []Header { - headers := []Header{ - { - Name: "X-Auth-Request-User", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "user", - }, - }, - }, - }, - { - Name: "X-Auth-Request-Email", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "email", - }, - }, - }, - }, - { - Name: "X-Auth-Request-Preferred-Username", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "preferred_username", - }, - }, - }, - }, - { - Name: "X-Auth-Request-Groups", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "groups", - }, - }, - }, - }, - } - - return headers -} - -func getXAuthRequestAccessTokenHeader() Header { - return Header{ - Name: "X-Auth-Request-Access-Token", - PreserveRequestValue: ptr.To(false), - Values: []HeaderValue{ - { - ClaimSource: &ClaimSource{ - Claim: "access_token", - }, - }, - }, - } -} - -type LegacyServer struct { - MetricsAddress string `flag:"metrics-address" cfg:"metrics_address"` - MetricsSecureAddress string `flag:"metrics-secure-address" cfg:"metrics_secure_address"` - MetricsTLSCertFile string `flag:"metrics-tls-cert-file" cfg:"metrics_tls_cert_file"` - MetricsTLSKeyFile string `flag:"metrics-tls-key-file" cfg:"metrics_tls_key_file"` - HTTPAddress string `flag:"http-address" cfg:"http_address"` - HTTPSAddress string `flag:"https-address" cfg:"https_address"` - TLSCertFile string `flag:"tls-cert-file" cfg:"tls_cert_file"` - TLSKeyFile string `flag:"tls-key-file" cfg:"tls_key_file"` - TLSMinVersion string `flag:"tls-min-version" cfg:"tls_min_version"` - TLSCipherSuites []string `flag:"tls-cipher-suite" cfg:"tls_cipher_suites"` -} - -func legacyServerFlagset() *pflag.FlagSet { - flagSet := pflag.NewFlagSet("server", pflag.ExitOnError) - - flagSet.String("metrics-address", "", "the address /metrics will be served on (e.g. \":9100\")") - flagSet.String("metrics-secure-address", "", "the address /metrics will be served on for HTTPS clients (e.g. \":9100\")") - flagSet.String("metrics-tls-cert-file", "", "path to certificate file for secure metrics server") - flagSet.String("metrics-tls-key-file", "", "path to private key file for secure metrics server") - flagSet.String("http-address", "127.0.0.1:4180", "[http://]: or unix:// or fd: (case insensitive) to listen on for HTTP clients") - flagSet.String("https-address", ":443", ": to listen on for HTTPS clients") - flagSet.String("tls-cert-file", "", "path to certificate file") - flagSet.String("tls-key-file", "", "path to private key file") - flagSet.String("tls-min-version", "", "minimal TLS version for HTTPS clients (either \"TLS1.2\" or \"TLS1.3\")") - flagSet.StringSlice("tls-cipher-suite", []string{}, "restricts TLS cipher suites to those listed (e.g. TLS_RSA_WITH_RC4_128_SHA) (may be given multiple times)") - - return flagSet -} - -type LegacyProvider struct { - ClientID string `flag:"client-id" cfg:"client_id"` - ClientSecret string `flag:"client-secret" cfg:"client_secret"` - ClientSecretFile string `flag:"client-secret-file" cfg:"client_secret_file"` - - KeycloakGroups []string `flag:"keycloak-group" cfg:"keycloak_groups"` - AzureTenant string `flag:"azure-tenant" cfg:"azure_tenant"` - AzureGraphGroupField string `flag:"azure-graph-group-field" cfg:"azure_graph_group_field"` - EntraIDAllowedTenants []string `flag:"entra-id-allowed-tenant" cfg:"entra_id_allowed_tenants"` - EntraIDFederatedTokenAuth bool `flag:"entra-id-federated-token-auth" cfg:"entra_id_federated_token_auth"` - BitbucketTeam string `flag:"bitbucket-team" cfg:"bitbucket_team"` - BitbucketRepository string `flag:"bitbucket-repository" cfg:"bitbucket_repository"` - GitHubOrg string `flag:"github-org" cfg:"github_org"` - GitHubTeam string `flag:"github-team" cfg:"github_team"` - GitHubRepo string `flag:"github-repo" cfg:"github_repo"` - GitHubToken string `flag:"github-token" cfg:"github_token"` - GitHubUsers []string `flag:"github-user" cfg:"github_users"` - GitLabGroup []string `flag:"gitlab-group" cfg:"gitlab_groups"` - GitLabProjects []string `flag:"gitlab-project" cfg:"gitlab_projects"` - GoogleGroupsLegacy []string `flag:"google-group" cfg:"google_group"` - GoogleGroups []string `flag:"google-group" cfg:"google_groups"` - GoogleAdminEmail string `flag:"google-admin-email" cfg:"google_admin_email"` - GoogleServiceAccountJSON string `flag:"google-service-account-json" cfg:"google_service_account_json"` - GoogleUseApplicationDefaultCredentials bool `flag:"google-use-application-default-credentials" cfg:"google_use_application_default_credentials"` - GoogleTargetPrincipal string `flag:"google-target-principal" cfg:"google_target_principal"` - GoogleUseOrganizationID bool `flag:"google-use-organization-id" cfg:"google_use_organization_id"` - GoogleAdminAPIUserScope string `flag:"google-admin-api-user-scope" cfg:"google_admin_api_user_scope"` - - // These options allow for other providers besides Google, with - // potential overrides. - ProviderType string `flag:"provider" cfg:"provider"` - ProviderName string `flag:"provider-display-name" cfg:"provider_display_name"` - ProviderCAFiles []string `flag:"provider-ca-file" cfg:"provider_ca_files"` - UseSystemTrustStore bool `flag:"use-system-trust-store" cfg:"use_system_trust_store"` - OIDCIssuerURL string `flag:"oidc-issuer-url" cfg:"oidc_issuer_url"` - InsecureOIDCAllowUnverifiedEmail bool `flag:"insecure-oidc-allow-unverified-email" cfg:"insecure_oidc_allow_unverified_email"` - InsecureOIDCSkipIssuerVerification bool `flag:"insecure-oidc-skip-issuer-verification" cfg:"insecure_oidc_skip_issuer_verification"` - InsecureOIDCSkipNonce bool `flag:"insecure-oidc-skip-nonce" cfg:"insecure_oidc_skip_nonce"` - SkipOIDCDiscovery bool `flag:"skip-oidc-discovery" cfg:"skip_oidc_discovery"` - OIDCJwksURL string `flag:"oidc-jwks-url" cfg:"oidc_jwks_url"` - OIDCEmailClaim string `flag:"oidc-email-claim" cfg:"oidc_email_claim"` - OIDCGroupsClaim string `flag:"oidc-groups-claim" cfg:"oidc_groups_claim"` - OIDCAudienceClaims []string `flag:"oidc-audience-claim" cfg:"oidc_audience_claims"` - OIDCExtraAudiences []string `flag:"oidc-extra-audience" cfg:"oidc_extra_audiences"` - OIDCPublicKeyFiles []string `flag:"oidc-public-key-file" cfg:"oidc_public_key_files"` - LoginURL string `flag:"login-url" cfg:"login_url"` - AuthRequestResponseMode string `flag:"auth-request-response-mode" cfg:"auth_request_response_mode"` - RedeemURL string `flag:"redeem-url" cfg:"redeem_url"` - ProfileURL string `flag:"profile-url" cfg:"profile_url"` - SkipClaimsFromProfileURL bool `flag:"skip-claims-from-profile-url" cfg:"skip_claims_from_profile_url"` - ProtectedResource string `flag:"resource" cfg:"resource"` - ValidateURL string `flag:"validate-url" cfg:"validate_url"` - Scope string `flag:"scope" cfg:"scope"` - Prompt string `flag:"prompt" cfg:"prompt"` - ApprovalPrompt string `flag:"approval-prompt" cfg:"approval_prompt"` // Deprecated by OIDC 1.0 - UserIDClaim string `flag:"user-id-claim" cfg:"user_id_claim"` - AllowedGroups []string `flag:"allowed-group" cfg:"allowed_groups"` - AllowedRoles []string `flag:"allowed-role" cfg:"allowed_roles"` - BackendLogoutURL string `flag:"backend-logout-url" cfg:"backend_logout_url"` - - AcrValues string `flag:"acr-values" cfg:"acr_values"` - JWTKey string `flag:"jwt-key" cfg:"jwt_key"` - JWTKeyFile string `flag:"jwt-key-file" cfg:"jwt_key_file"` - PubJWKURL string `flag:"pubjwk-url" cfg:"pubjwk_url"` - // PKCE Code Challenge method to use (either S256 or plain) - CodeChallengeMethod string `flag:"code-challenge-method" cfg:"code_challenge_method"` - // Provided for legacy reasons, to be dropped in newer version see #1667 - ForceCodeChallengeMethod string `flag:"force-code-challenge-method" cfg:"force_code_challenge_method"` -} - -func legacyProviderFlagSet() *pflag.FlagSet { - flagSet := pflag.NewFlagSet("provider", pflag.ExitOnError) - - flagSet.StringSlice("keycloak-group", []string{}, "restrict logins to members of these groups (may be given multiple times)") - flagSet.String("azure-tenant", "common", "go to a tenant-specific or common (tenant-independent) endpoint.") - flagSet.String("azure-graph-group-field", "", "configures the group field to be used when building the groups list(`id` or `displayName`. Default is `id`) from Microsoft Graph(available only for v2.0 oidc url). Based on this value, the `allowed-group` config values should be adjusted accordingly. If using `id` as group field, `allowed-group` should contains groups IDs, if using `displayName` as group field, `allowed-group` should contains groups name") - flagSet.StringSlice("entra-id-allowed-tenant", []string{}, "list of tenants allowed for MS Entra ID multi-tenant application") - flagSet.Bool("entra-id-federated-token-auth", false, "enable oAuth client authentication with federated token projected by Azure Workload Identity plugin, instead of client secret.") - flagSet.String("bitbucket-team", "", "restrict logins to members of this team") - flagSet.String("bitbucket-repository", "", "restrict logins to user with access to this repository") - flagSet.String("github-org", "", "restrict logins to members of this organisation") - flagSet.String("github-team", "", "restrict logins to members of this team") - flagSet.String("github-repo", "", "restrict logins to collaborators of this repository") - flagSet.String("github-token", "", "the token to use when verifying repository collaborators (must have push access to the repository)") - flagSet.StringSlice("github-user", []string{}, "allow users with these usernames to login even if they do not belong to the specified org and team or collaborators (may be given multiple times)") - flagSet.StringSlice("gitlab-group", []string{}, "restrict logins to members of this group (may be given multiple times)") - flagSet.StringSlice("gitlab-project", []string{}, "restrict logins to members of this project (may be given multiple times) (eg `group/project=accesslevel`). Access level should be a value matching Gitlab access levels (see https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent") - flagSet.String("client-id", "", "the OAuth Client ID: ie: \"123456.apps.googleusercontent.com\"") - flagSet.String("client-secret", "", "the OAuth Client Secret") - flagSet.String("client-secret-file", "", "the file with OAuth Client Secret") - - flagSet.String("provider", "google", "OAuth provider") - flagSet.String("provider-display-name", "", "Provider display name") - flagSet.StringSlice("provider-ca-file", []string{}, "One or more paths to CA certificates that should be used when connecting to the provider. If not specified, the default Go trust sources are used instead.") - flagSet.Bool("use-system-trust-store", false, "Determines if 'provider-ca-file' files and the system trust store are used. If set to true, your custom CA files and the system trust store are used otherwise only your custom CA files.") - flagSet.String("oidc-issuer-url", "", "OpenID Connect issuer URL (ie: https://accounts.google.com)") - flagSet.Bool("insecure-oidc-allow-unverified-email", false, "Don't fail if an email address in an id_token is not verified") - flagSet.Bool("insecure-oidc-skip-issuer-verification", false, "Do not verify if issuer matches OIDC discovery URL") - flagSet.Bool("insecure-oidc-skip-nonce", true, "skip verifying the OIDC ID Token's nonce claim") - flagSet.Bool("skip-oidc-discovery", false, "Skip OIDC discovery and use manually supplied Endpoints") - flagSet.String("oidc-jwks-url", "", "OpenID Connect JWKS URL (ie: https://www.googleapis.com/oauth2/v3/certs)") - flagSet.String("oidc-groups-claim", OIDCGroupsClaim, "which OIDC claim contains the user groups") - flagSet.String("oidc-email-claim", OIDCEmailClaim, "which OIDC claim contains the user's email") - flagSet.StringSlice("oidc-audience-claim", OIDCAudienceClaims, "which OIDC claims are used as audience to verify against client id") - flagSet.StringSlice("oidc-extra-audience", []string{}, "additional audiences allowed to pass audience verification") - flagSet.StringSlice("oidc-public-key-file", []string{}, "path to public key file in PEM format to use for verifying JWT tokens (may be given multiple times)") - flagSet.String("login-url", "", "Authentication endpoint") - flagSet.String("redeem-url", "", "Token redemption endpoint") - flagSet.String("profile-url", "", "Profile access endpoint") - flagSet.String("auth-request-response-mode", "", "Authorization request response mode") - flagSet.Bool("skip-claims-from-profile-url", false, "Skip loading missing claims from profile URL") - flagSet.String("resource", "", "The resource that is protected (Azure AD only)") - flagSet.String("validate-url", "", "Access token validation endpoint") - flagSet.String("scope", "", "OAuth scope specification") - flagSet.String("prompt", "", "OIDC prompt") - flagSet.String("approval-prompt", "force", "OAuth approval_prompt") - flagSet.String("code-challenge-method", "", "use PKCE code challenges with the specified method. Either 'plain' or 'S256'") - flagSet.String("force-code-challenge-method", "", "Deprecated - use --code-challenge-method") - - flagSet.String("acr-values", "", "acr values string: optional") - flagSet.String("jwt-key", "", "private key in PEM format used to sign JWT, so that you can say something like -jwt-key=\"${OAUTH2_PROXY_JWT_KEY}\": required by login.gov") - flagSet.String("jwt-key-file", "", "path to the private key file in PEM format used to sign the JWT so that you can say something like -jwt-key-file=/etc/ssl/private/jwt_signing_key.pem: required by login.gov") - flagSet.String("pubjwk-url", "", "JWK pubkey access endpoint: required by login.gov") - - flagSet.String("user-id-claim", OIDCEmailClaim, "(DEPRECATED for `oidc-email-claim`) which claim contains the user ID") - flagSet.StringSlice("allowed-group", []string{}, "restrict logins to members of this group (may be given multiple times)") - flagSet.StringSlice("allowed-role", []string{}, "(keycloak-oidc) restrict logins to members of these roles (may be given multiple times)") - flagSet.String("backend-logout-url", "", "url to perform a backend logout, {id_token} can be used as placeholder for the id_token") - - return flagSet -} - -func legacyGoogleFlagSet() *pflag.FlagSet { - flagSet := pflag.NewFlagSet("google", pflag.ExitOnError) - - flagSet.StringSlice("google-group", []string{}, "restrict logins to members of this google group (may be given multiple times).") - flagSet.String("google-admin-email", "", "the google admin to impersonate for api calls") - flagSet.String("google-service-account-json", "", "the path to the service account json credentials") - flagSet.String("google-use-application-default-credentials", "", "use application default credentials instead of service account json (i.e. GKE Workload Identity)") - flagSet.String("google-target-principal", "", "the target principal to impersonate when using ADC") - flagSet.String("google-use-organization-id", "", "use organization id as preferred username") - flagSet.String("google-admin-api-user-scope", "", "authorization scope required to call users.get, can be one of ") - - return flagSet -} - -func (l LegacyServer) convert() (Server, Server) { - appServer := Server{ - BindAddress: l.HTTPAddress, - SecureBindAddress: l.HTTPSAddress, - } - if l.TLSKeyFile != "" || l.TLSCertFile != "" { - appServer.TLS = &TLS{ - Key: &SecretSource{ - FromFile: l.TLSKeyFile, - }, - Cert: &SecretSource{ - FromFile: l.TLSCertFile, - }, - MinVersion: l.TLSMinVersion, - } - if len(l.TLSCipherSuites) != 0 { - appServer.TLS.CipherSuites = l.TLSCipherSuites - } - // Preserve backwards compatibility, only run one server - appServer.BindAddress = "" - } else { - // Disable the HTTPS server if there's no certificates. - // This preserves backwards compatibility. - appServer.SecureBindAddress = "" - } - - metricsServer := Server{ - BindAddress: l.MetricsAddress, - SecureBindAddress: l.MetricsSecureAddress, - } - if l.MetricsTLSKeyFile != "" || l.MetricsTLSCertFile != "" { - metricsServer.TLS = &TLS{ - Key: &SecretSource{ - FromFile: l.MetricsTLSKeyFile, - }, - Cert: &SecretSource{ - FromFile: l.MetricsTLSCertFile, - }, - } - } - - return appServer, metricsServer -} - -func (l *LegacyProvider) convert() (Providers, error) { - providers := Providers{} - - provider := Provider{ - ClientID: l.ClientID, - ClientSecret: l.ClientSecret, - ClientSecretFile: l.ClientSecretFile, - Type: ProviderType(l.ProviderType), - CAFiles: l.ProviderCAFiles, - UseSystemTrustStore: &l.UseSystemTrustStore, - LoginURL: l.LoginURL, - RedeemURL: l.RedeemURL, - ProfileURL: l.ProfileURL, - SkipClaimsFromProfileURL: &l.SkipClaimsFromProfileURL, - ProtectedResource: l.ProtectedResource, - ValidateURL: l.ValidateURL, - Scope: l.Scope, - AllowedGroups: l.AllowedGroups, - CodeChallengeMethod: l.CodeChallengeMethod, - BackendLogoutURL: l.BackendLogoutURL, - AuthRequestResponseMode: l.AuthRequestResponseMode, - } - - // This part is out of the switch section for all providers that support OIDC - provider.OIDCConfig = OIDCOptions{ - IssuerURL: l.OIDCIssuerURL, - InsecureAllowUnverifiedEmail: &l.InsecureOIDCAllowUnverifiedEmail, - InsecureSkipIssuerVerification: &l.InsecureOIDCSkipIssuerVerification, - InsecureSkipNonce: &l.InsecureOIDCSkipNonce, - SkipDiscovery: &l.SkipOIDCDiscovery, - JwksURL: l.OIDCJwksURL, - UserIDClaim: l.UserIDClaim, - EmailClaim: l.OIDCEmailClaim, - GroupsClaim: l.OIDCGroupsClaim, - AudienceClaims: l.OIDCAudienceClaims, - ExtraAudiences: l.OIDCExtraAudiences, - PublicKeyFiles: l.OIDCPublicKeyFiles, - } - - // Support for legacy configuration option - if l.ForceCodeChallengeMethod != "" && l.CodeChallengeMethod == "" { - provider.CodeChallengeMethod = l.ForceCodeChallengeMethod - } - - // This part is out of the switch section because azure has a default tenant - // that needs to be added from legacy options - provider.AzureConfig = AzureOptions{ - Tenant: l.AzureTenant, - GraphGroupField: l.AzureGraphGroupField, - } - - switch provider.Type { - case "github": - provider.GitHubConfig = GitHubOptions{ - Org: l.GitHubOrg, - Team: l.GitHubTeam, - Repo: l.GitHubRepo, - Token: l.GitHubToken, - Users: l.GitHubUsers, - } - case "keycloak-oidc": - provider.KeycloakConfig = KeycloakOptions{ - Groups: l.KeycloakGroups, - Roles: l.AllowedRoles, - } - case "keycloak": - provider.KeycloakConfig = KeycloakOptions{ - Groups: l.KeycloakGroups, - } - case "gitlab": - provider.GitLabConfig = GitLabOptions{ - Group: l.GitLabGroup, - Projects: l.GitLabProjects, - } - case "login.gov": - provider.LoginGovConfig = LoginGovOptions{ - JWTKey: l.JWTKey, - JWTKeyFile: l.JWTKeyFile, - PubJWKURL: l.PubJWKURL, - } - case "bitbucket": - provider.BitbucketConfig = BitbucketOptions{ - Team: l.BitbucketTeam, - Repository: l.BitbucketRepository, - } - case "google": - if len(l.GoogleGroupsLegacy) != 0 && !reflect.DeepEqual(l.GoogleGroupsLegacy, l.GoogleGroups) { - // Log the deprecation notice - logger.Error( - "WARNING: The 'OAUTH2_PROXY_GOOGLE_GROUP' environment variable is deprecated and will likely be removed in the next major release. Use 'OAUTH2_PROXY_GOOGLE_GROUPS' instead.", - ) - l.GoogleGroups = l.GoogleGroupsLegacy - } - provider.GoogleConfig = GoogleOptions{ - Groups: l.GoogleGroups, - AdminEmail: l.GoogleAdminEmail, - ServiceAccountJSON: l.GoogleServiceAccountJSON, - UseApplicationDefaultCredentials: &l.GoogleUseApplicationDefaultCredentials, - TargetPrincipal: l.GoogleTargetPrincipal, - UseOrganizationID: &l.GoogleUseOrganizationID, - AdminAPIUserScope: l.GoogleAdminAPIUserScope, - } - case "entra-id": - provider.MicrosoftEntraIDConfig = MicrosoftEntraIDOptions{ - AllowedTenants: l.EntraIDAllowedTenants, - FederatedTokenAuth: &l.EntraIDFederatedTokenAuth, - } - } - - if l.ProviderName != "" { - provider.ID = l.ProviderName - provider.Name = l.ProviderName - } else { - provider.ID = l.ProviderType + "=" + l.ClientID - } - - // handle AcrValues, Prompt and ApprovalPrompt - var urlParams []LoginURLParameter - if l.AcrValues != "" { - urlParams = append(urlParams, LoginURLParameter{Name: "acr_values", Default: []string{l.AcrValues}}) - } - switch { - case l.Prompt != "": - urlParams = append(urlParams, LoginURLParameter{Name: "prompt", Default: []string{l.Prompt}}) - case l.ApprovalPrompt != "": - urlParams = append(urlParams, LoginURLParameter{Name: "approval_prompt", Default: []string{l.ApprovalPrompt}}) - default: - // match legacy behaviour by default - if neither prompt nor approval_prompt - // specified, use approval_prompt=force - urlParams = append(urlParams, LoginURLParameter{Name: "approval_prompt", Default: []string{"force"}}) - } - - provider.LoginURLParameters = urlParams - - providers = append(providers, provider) - - return providers, nil -} diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index ceba53af..f69e6fea 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -1084,4 +1084,54 @@ var _ = Describe("Legacy Options", func() { }), ) }) + + Context("Legacy Cookie", func() { + type convertCookieTableInput struct { + legacyCookie LegacyCookie + expectedCookie Cookie + } + + // Test cases and expected outcomes + fullCookie := Cookie{ + Name: "_oauth2_proxy", + Secret: "", + Domains: nil, + Path: "/", + Expire: time.Duration(168) * time.Hour, + Refresh: time.Duration(0), + Secure: ptr.To(true), + HTTPOnly: ptr.To(true), + SameSite: "", + CSRFPerRequest: ptr.To(false), + CSRFPerRequestLimit: 0, + CSRFExpire: time.Duration(15) * time.Minute, + } + + fullLegacyCookie := LegacyCookie{ + Name: "_oauth2_proxy", + Secret: "", + Domains: nil, + Path: "/", + Expire: time.Duration(168) * time.Hour, + Refresh: time.Duration(0), + Secure: true, + HTTPOnly: true, + SameSite: "", + CSRFPerRequest: false, + CSRFPerRequestLimit: 0, + CSRFExpire: time.Duration(15) * time.Minute, + } + + DescribeTable("convertLegacyCookie", + func(in *convertCookieTableInput) { + cookie := in.legacyCookie.convert() + + Expect(cookie).To(BeEquivalentTo(in.expectedCookie)) + }, + Entry("with all attributes", &convertCookieTableInput{ + legacyCookie: fullLegacyCookie, + expectedCookie: fullCookie, + }), + ) + }) }) diff --git a/pkg/apis/options/legacy_provider.go b/pkg/apis/options/legacy_provider.go new file mode 100644 index 00000000..87302511 --- /dev/null +++ b/pkg/apis/options/legacy_provider.go @@ -0,0 +1,317 @@ +package options + +import ( + "reflect" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" + "github.com/spf13/pflag" +) + +type LegacyProvider struct { + ClientID string `flag:"client-id" cfg:"client_id"` + ClientSecret string `flag:"client-secret" cfg:"client_secret"` + ClientSecretFile string `flag:"client-secret-file" cfg:"client_secret_file"` + + KeycloakGroups []string `flag:"keycloak-group" cfg:"keycloak_groups"` + AzureTenant string `flag:"azure-tenant" cfg:"azure_tenant"` + AzureGraphGroupField string `flag:"azure-graph-group-field" cfg:"azure_graph_group_field"` + EntraIDAllowedTenants []string `flag:"entra-id-allowed-tenant" cfg:"entra_id_allowed_tenants"` + EntraIDFederatedTokenAuth bool `flag:"entra-id-federated-token-auth" cfg:"entra_id_federated_token_auth"` + BitbucketTeam string `flag:"bitbucket-team" cfg:"bitbucket_team"` + BitbucketRepository string `flag:"bitbucket-repository" cfg:"bitbucket_repository"` + GitHubOrg string `flag:"github-org" cfg:"github_org"` + GitHubTeam string `flag:"github-team" cfg:"github_team"` + GitHubRepo string `flag:"github-repo" cfg:"github_repo"` + GitHubToken string `flag:"github-token" cfg:"github_token"` + GitHubUsers []string `flag:"github-user" cfg:"github_users"` + GitLabGroup []string `flag:"gitlab-group" cfg:"gitlab_groups"` + GitLabProjects []string `flag:"gitlab-project" cfg:"gitlab_projects"` + GoogleGroupsLegacy []string `flag:"google-group" cfg:"google_group"` + GoogleGroups []string `flag:"google-group" cfg:"google_groups"` + GoogleAdminEmail string `flag:"google-admin-email" cfg:"google_admin_email"` + GoogleServiceAccountJSON string `flag:"google-service-account-json" cfg:"google_service_account_json"` + GoogleUseApplicationDefaultCredentials bool `flag:"google-use-application-default-credentials" cfg:"google_use_application_default_credentials"` + GoogleTargetPrincipal string `flag:"google-target-principal" cfg:"google_target_principal"` + GoogleUseOrganizationID bool `flag:"google-use-organization-id" cfg:"google_use_organization_id"` + GoogleAdminAPIUserScope string `flag:"google-admin-api-user-scope" cfg:"google_admin_api_user_scope"` + + // These options allow for other providers besides Google, with + // potential overrides. + ProviderType string `flag:"provider" cfg:"provider"` + ProviderName string `flag:"provider-display-name" cfg:"provider_display_name"` + ProviderCAFiles []string `flag:"provider-ca-file" cfg:"provider_ca_files"` + UseSystemTrustStore bool `flag:"use-system-trust-store" cfg:"use_system_trust_store"` + OIDCIssuerURL string `flag:"oidc-issuer-url" cfg:"oidc_issuer_url"` + InsecureOIDCAllowUnverifiedEmail bool `flag:"insecure-oidc-allow-unverified-email" cfg:"insecure_oidc_allow_unverified_email"` + InsecureOIDCSkipIssuerVerification bool `flag:"insecure-oidc-skip-issuer-verification" cfg:"insecure_oidc_skip_issuer_verification"` + InsecureOIDCSkipNonce bool `flag:"insecure-oidc-skip-nonce" cfg:"insecure_oidc_skip_nonce"` + SkipOIDCDiscovery bool `flag:"skip-oidc-discovery" cfg:"skip_oidc_discovery"` + OIDCJwksURL string `flag:"oidc-jwks-url" cfg:"oidc_jwks_url"` + OIDCEmailClaim string `flag:"oidc-email-claim" cfg:"oidc_email_claim"` + OIDCGroupsClaim string `flag:"oidc-groups-claim" cfg:"oidc_groups_claim"` + OIDCAudienceClaims []string `flag:"oidc-audience-claim" cfg:"oidc_audience_claims"` + OIDCExtraAudiences []string `flag:"oidc-extra-audience" cfg:"oidc_extra_audiences"` + OIDCPublicKeyFiles []string `flag:"oidc-public-key-file" cfg:"oidc_public_key_files"` + LoginURL string `flag:"login-url" cfg:"login_url"` + AuthRequestResponseMode string `flag:"auth-request-response-mode" cfg:"auth_request_response_mode"` + RedeemURL string `flag:"redeem-url" cfg:"redeem_url"` + ProfileURL string `flag:"profile-url" cfg:"profile_url"` + SkipClaimsFromProfileURL bool `flag:"skip-claims-from-profile-url" cfg:"skip_claims_from_profile_url"` + ProtectedResource string `flag:"resource" cfg:"resource"` + ValidateURL string `flag:"validate-url" cfg:"validate_url"` + Scope string `flag:"scope" cfg:"scope"` + Prompt string `flag:"prompt" cfg:"prompt"` + ApprovalPrompt string `flag:"approval-prompt" cfg:"approval_prompt"` // Deprecated by OIDC 1.0 + UserIDClaim string `flag:"user-id-claim" cfg:"user_id_claim"` + AllowedGroups []string `flag:"allowed-group" cfg:"allowed_groups"` + AllowedRoles []string `flag:"allowed-role" cfg:"allowed_roles"` + BackendLogoutURL string `flag:"backend-logout-url" cfg:"backend_logout_url"` + + AcrValues string `flag:"acr-values" cfg:"acr_values"` + JWTKey string `flag:"jwt-key" cfg:"jwt_key"` + JWTKeyFile string `flag:"jwt-key-file" cfg:"jwt_key_file"` + PubJWKURL string `flag:"pubjwk-url" cfg:"pubjwk_url"` + // PKCE Code Challenge method to use (either S256 or plain) + CodeChallengeMethod string `flag:"code-challenge-method" cfg:"code_challenge_method"` + // Provided for legacy reasons, to be dropped in newer version see #1667 + ForceCodeChallengeMethod string `flag:"force-code-challenge-method" cfg:"force_code_challenge_method"` +} + +func legacyProviderFlagSet() *pflag.FlagSet { + flagSet := pflag.NewFlagSet("provider", pflag.ExitOnError) + + flagSet.StringSlice("keycloak-group", []string{}, "restrict logins to members of these groups (may be given multiple times)") + flagSet.String("azure-tenant", "common", "go to a tenant-specific or common (tenant-independent) endpoint.") + flagSet.String("azure-graph-group-field", "", "configures the group field to be used when building the groups list(`id` or `displayName`. Default is `id`) from Microsoft Graph(available only for v2.0 oidc url). Based on this value, the `allowed-group` config values should be adjusted accordingly. If using `id` as group field, `allowed-group` should contains groups IDs, if using `displayName` as group field, `allowed-group` should contains groups name") + flagSet.StringSlice("entra-id-allowed-tenant", []string{}, "list of tenants allowed for MS Entra ID multi-tenant application") + flagSet.Bool("entra-id-federated-token-auth", false, "enable oAuth client authentication with federated token projected by Azure Workload Identity plugin, instead of client secret.") + flagSet.String("bitbucket-team", "", "restrict logins to members of this team") + flagSet.String("bitbucket-repository", "", "restrict logins to user with access to this repository") + flagSet.String("github-org", "", "restrict logins to members of this organisation") + flagSet.String("github-team", "", "restrict logins to members of this team") + flagSet.String("github-repo", "", "restrict logins to collaborators of this repository") + flagSet.String("github-token", "", "the token to use when verifying repository collaborators (must have push access to the repository)") + flagSet.StringSlice("github-user", []string{}, "allow users with these usernames to login even if they do not belong to the specified org and team or collaborators (may be given multiple times)") + flagSet.StringSlice("gitlab-group", []string{}, "restrict logins to members of this group (may be given multiple times)") + flagSet.StringSlice("gitlab-project", []string{}, "restrict logins to members of this project (may be given multiple times) (eg `group/project=accesslevel`). Access level should be a value matching Gitlab access levels (see https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent") + flagSet.String("client-id", "", "the OAuth Client ID: ie: \"123456.apps.googleusercontent.com\"") + flagSet.String("client-secret", "", "the OAuth Client Secret") + flagSet.String("client-secret-file", "", "the file with OAuth Client Secret") + + flagSet.String("provider", "google", "OAuth provider") + flagSet.String("provider-display-name", "", "Provider display name") + flagSet.StringSlice("provider-ca-file", []string{}, "One or more paths to CA certificates that should be used when connecting to the provider. If not specified, the default Go trust sources are used instead.") + flagSet.Bool("use-system-trust-store", false, "Determines if 'provider-ca-file' files and the system trust store are used. If set to true, your custom CA files and the system trust store are used otherwise only your custom CA files.") + flagSet.String("oidc-issuer-url", "", "OpenID Connect issuer URL (ie: https://accounts.google.com)") + flagSet.Bool("insecure-oidc-allow-unverified-email", false, "Don't fail if an email address in an id_token is not verified") + flagSet.Bool("insecure-oidc-skip-issuer-verification", false, "Do not verify if issuer matches OIDC discovery URL") + flagSet.Bool("insecure-oidc-skip-nonce", true, "skip verifying the OIDC ID Token's nonce claim") + flagSet.Bool("skip-oidc-discovery", false, "Skip OIDC discovery and use manually supplied Endpoints") + flagSet.String("oidc-jwks-url", "", "OpenID Connect JWKS URL (ie: https://www.googleapis.com/oauth2/v3/certs)") + flagSet.String("oidc-groups-claim", OIDCGroupsClaim, "which OIDC claim contains the user groups") + flagSet.String("oidc-email-claim", OIDCEmailClaim, "which OIDC claim contains the user's email") + flagSet.StringSlice("oidc-audience-claim", OIDCAudienceClaims, "which OIDC claims are used as audience to verify against client id") + flagSet.StringSlice("oidc-extra-audience", []string{}, "additional audiences allowed to pass audience verification") + flagSet.StringSlice("oidc-public-key-file", []string{}, "path to public key file in PEM format to use for verifying JWT tokens (may be given multiple times)") + flagSet.String("login-url", "", "Authentication endpoint") + flagSet.String("redeem-url", "", "Token redemption endpoint") + flagSet.String("profile-url", "", "Profile access endpoint") + flagSet.String("auth-request-response-mode", "", "Authorization request response mode") + flagSet.Bool("skip-claims-from-profile-url", false, "Skip loading missing claims from profile URL") + flagSet.String("resource", "", "The resource that is protected (Azure AD only)") + flagSet.String("validate-url", "", "Access token validation endpoint") + flagSet.String("scope", "", "OAuth scope specification") + flagSet.String("prompt", "", "OIDC prompt") + flagSet.String("approval-prompt", "force", "OAuth approval_prompt") + flagSet.String("code-challenge-method", "", "use PKCE code challenges with the specified method. Either 'plain' or 'S256'") + flagSet.String("force-code-challenge-method", "", "Deprecated - use --code-challenge-method") + + flagSet.String("acr-values", "", "acr values string: optional") + flagSet.String("jwt-key", "", "private key in PEM format used to sign JWT, so that you can say something like -jwt-key=\"${OAUTH2_PROXY_JWT_KEY}\": required by login.gov") + flagSet.String("jwt-key-file", "", "path to the private key file in PEM format used to sign the JWT so that you can say something like -jwt-key-file=/etc/ssl/private/jwt_signing_key.pem: required by login.gov") + flagSet.String("pubjwk-url", "", "JWK pubkey access endpoint: required by login.gov") + + flagSet.String("user-id-claim", OIDCEmailClaim, "(DEPRECATED for `oidc-email-claim`) which claim contains the user ID") + flagSet.StringSlice("allowed-group", []string{}, "restrict logins to members of this group (may be given multiple times)") + flagSet.StringSlice("allowed-role", []string{}, "(keycloak-oidc) restrict logins to members of these roles (may be given multiple times)") + flagSet.String("backend-logout-url", "", "url to perform a backend logout, {id_token} can be used as placeholder for the id_token") + + return flagSet +} + +func legacyGoogleFlagSet() *pflag.FlagSet { + flagSet := pflag.NewFlagSet("google", pflag.ExitOnError) + + flagSet.StringSlice("google-group", []string{}, "restrict logins to members of this google group (may be given multiple times).") + flagSet.String("google-admin-email", "", "the google admin to impersonate for api calls") + flagSet.String("google-service-account-json", "", "the path to the service account json credentials") + flagSet.String("google-use-application-default-credentials", "", "use application default credentials instead of service account json (i.e. GKE Workload Identity)") + flagSet.String("google-target-principal", "", "the target principal to impersonate when using ADC") + flagSet.String("google-use-organization-id", "", "use organization id as preferred username") + flagSet.String("google-admin-api-user-scope", "", "authorization scope required to call users.get, can be one of ") + + return flagSet +} + +func (l *LegacyProvider) convert() (Providers, error) { + providers := Providers{} + + provider := Provider{ + ClientID: l.ClientID, + ClientSecret: l.ClientSecret, + ClientSecretFile: l.ClientSecretFile, + Type: ProviderType(l.ProviderType), + CAFiles: l.ProviderCAFiles, + UseSystemTrustStore: &l.UseSystemTrustStore, + LoginURL: l.LoginURL, + RedeemURL: l.RedeemURL, + ProfileURL: l.ProfileURL, + SkipClaimsFromProfileURL: &l.SkipClaimsFromProfileURL, + ProtectedResource: l.ProtectedResource, + ValidateURL: l.ValidateURL, + Scope: l.Scope, + AllowedGroups: l.AllowedGroups, + CodeChallengeMethod: l.CodeChallengeMethod, + BackendLogoutURL: l.BackendLogoutURL, + AuthRequestResponseMode: l.AuthRequestResponseMode, + } + + // This part is out of the switch section for all providers that support OIDC + provider.OIDCConfig = OIDCOptions{ + IssuerURL: l.OIDCIssuerURL, + InsecureAllowUnverifiedEmail: &l.InsecureOIDCAllowUnverifiedEmail, + InsecureSkipIssuerVerification: &l.InsecureOIDCSkipIssuerVerification, + InsecureSkipNonce: &l.InsecureOIDCSkipNonce, + SkipDiscovery: &l.SkipOIDCDiscovery, + JwksURL: l.OIDCJwksURL, + UserIDClaim: l.UserIDClaim, + EmailClaim: l.OIDCEmailClaim, + GroupsClaim: l.OIDCGroupsClaim, + AudienceClaims: l.OIDCAudienceClaims, + ExtraAudiences: l.OIDCExtraAudiences, + PublicKeyFiles: l.OIDCPublicKeyFiles, + } + + // Support for legacy configuration option + if l.ForceCodeChallengeMethod != "" && l.CodeChallengeMethod == "" { + provider.CodeChallengeMethod = l.ForceCodeChallengeMethod + } + + // This part is out of the switch section because azure has a default tenant + // that needs to be added from legacy options + provider.AzureConfig = AzureOptions{ + Tenant: l.AzureTenant, + GraphGroupField: l.AzureGraphGroupField, + } + + switch provider.Type { + case "github": + provider.GitHubConfig = GitHubOptions{ + Org: l.GitHubOrg, + Team: l.GitHubTeam, + Repo: l.GitHubRepo, + Token: l.GitHubToken, + Users: l.GitHubUsers, + } + case "keycloak-oidc": + provider.KeycloakConfig = KeycloakOptions{ + Groups: l.KeycloakGroups, + Roles: l.AllowedRoles, + } + case "keycloak": + provider.KeycloakConfig = KeycloakOptions{ + Groups: l.KeycloakGroups, + } + case "gitlab": + provider.GitLabConfig = GitLabOptions{ + Group: l.GitLabGroup, + Projects: l.GitLabProjects, + } + case "login.gov": + provider.LoginGovConfig = LoginGovOptions{ + JWTKey: l.JWTKey, + JWTKeyFile: l.JWTKeyFile, + PubJWKURL: l.PubJWKURL, + } + case "bitbucket": + provider.BitbucketConfig = BitbucketOptions{ + Team: l.BitbucketTeam, + Repository: l.BitbucketRepository, + } + case "google": + if len(l.GoogleGroupsLegacy) != 0 && !reflect.DeepEqual(l.GoogleGroupsLegacy, l.GoogleGroups) { + // Log the deprecation notice + logger.Error( + "WARNING: The 'OAUTH2_PROXY_GOOGLE_GROUP' environment variable is deprecated and will likely be removed in the next major release. Use 'OAUTH2_PROXY_GOOGLE_GROUPS' instead.", + ) + l.GoogleGroups = l.GoogleGroupsLegacy + } + provider.GoogleConfig = GoogleOptions{ + Groups: l.GoogleGroups, + AdminEmail: l.GoogleAdminEmail, + ServiceAccountJSON: l.GoogleServiceAccountJSON, + UseApplicationDefaultCredentials: &l.GoogleUseApplicationDefaultCredentials, + TargetPrincipal: l.GoogleTargetPrincipal, + UseOrganizationID: &l.GoogleUseOrganizationID, + AdminAPIUserScope: l.GoogleAdminAPIUserScope, + } + case "entra-id": + provider.MicrosoftEntraIDConfig = MicrosoftEntraIDOptions{ + AllowedTenants: l.EntraIDAllowedTenants, + FederatedTokenAuth: &l.EntraIDFederatedTokenAuth, + } + } + + if l.ProviderName != "" { + provider.ID = l.ProviderName + provider.Name = l.ProviderName + } else { + provider.ID = l.ProviderType + "=" + l.ClientID + } + + // handle AcrValues, Prompt and ApprovalPrompt + var urlParams []LoginURLParameter + if l.AcrValues != "" { + urlParams = append(urlParams, LoginURLParameter{Name: "acr_values", Default: []string{l.AcrValues}}) + } + switch { + case l.Prompt != "": + urlParams = append(urlParams, LoginURLParameter{Name: "prompt", Default: []string{l.Prompt}}) + case l.ApprovalPrompt != "": + urlParams = append(urlParams, LoginURLParameter{Name: "approval_prompt", Default: []string{l.ApprovalPrompt}}) + default: + // match legacy behaviour by default - if neither prompt nor approval_prompt + // specified, use approval_prompt=force + urlParams = append(urlParams, LoginURLParameter{Name: "approval_prompt", Default: []string{"force"}}) + } + + provider.LoginURLParameters = urlParams + + providers = append(providers, provider) + + return providers, nil +} + +// Legacy default providers configuration +func providerDefaults() Providers { + providers := Providers{ + { + Type: "google", + AzureConfig: AzureOptions{ + Tenant: "common", + }, + OIDCConfig: OIDCOptions{ + InsecureAllowUnverifiedEmail: ptr.To(DefaultInsecureAllowUnverifiedEmail), + InsecureSkipNonce: ptr.To(DefaultInsecureSkipNonce), + SkipDiscovery: ptr.To(DefaultSkipDiscovery), + UserIDClaim: OIDCEmailClaim, // Deprecated: Use OIDCEmailClaim + EmailClaim: OIDCEmailClaim, + GroupsClaim: OIDCGroupsClaim, + AudienceClaims: OIDCAudienceClaims, + ExtraAudiences: []string{}, + }, + }, + } + return providers +} diff --git a/pkg/apis/options/legacy_server.go b/pkg/apis/options/legacy_server.go new file mode 100644 index 00000000..84019793 --- /dev/null +++ b/pkg/apis/options/legacy_server.go @@ -0,0 +1,79 @@ +package options + +import ( + "github.com/spf13/pflag" +) + +type LegacyServer struct { + MetricsAddress string `flag:"metrics-address" cfg:"metrics_address"` + MetricsSecureAddress string `flag:"metrics-secure-address" cfg:"metrics_secure_address"` + MetricsTLSCertFile string `flag:"metrics-tls-cert-file" cfg:"metrics_tls_cert_file"` + MetricsTLSKeyFile string `flag:"metrics-tls-key-file" cfg:"metrics_tls_key_file"` + HTTPAddress string `flag:"http-address" cfg:"http_address"` + HTTPSAddress string `flag:"https-address" cfg:"https_address"` + TLSCertFile string `flag:"tls-cert-file" cfg:"tls_cert_file"` + TLSKeyFile string `flag:"tls-key-file" cfg:"tls_key_file"` + TLSMinVersion string `flag:"tls-min-version" cfg:"tls_min_version"` + TLSCipherSuites []string `flag:"tls-cipher-suite" cfg:"tls_cipher_suites"` +} + +func legacyServerFlagset() *pflag.FlagSet { + flagSet := pflag.NewFlagSet("server", pflag.ExitOnError) + + flagSet.String("metrics-address", "", "the address /metrics will be served on (e.g. \":9100\")") + flagSet.String("metrics-secure-address", "", "the address /metrics will be served on for HTTPS clients (e.g. \":9100\")") + flagSet.String("metrics-tls-cert-file", "", "path to certificate file for secure metrics server") + flagSet.String("metrics-tls-key-file", "", "path to private key file for secure metrics server") + flagSet.String("http-address", "127.0.0.1:4180", "[http://]: or unix:// or fd: (case insensitive) to listen on for HTTP clients") + flagSet.String("https-address", ":443", ": to listen on for HTTPS clients") + flagSet.String("tls-cert-file", "", "path to certificate file") + flagSet.String("tls-key-file", "", "path to private key file") + flagSet.String("tls-min-version", "", "minimal TLS version for HTTPS clients (either \"TLS1.2\" or \"TLS1.3\")") + flagSet.StringSlice("tls-cipher-suite", []string{}, "restricts TLS cipher suites to those listed (e.g. TLS_RSA_WITH_RC4_128_SHA) (may be given multiple times)") + + return flagSet +} + +func (l LegacyServer) convert() (Server, Server) { + appServer := Server{ + BindAddress: l.HTTPAddress, + SecureBindAddress: l.HTTPSAddress, + } + if l.TLSKeyFile != "" || l.TLSCertFile != "" { + appServer.TLS = &TLS{ + Key: &SecretSource{ + FromFile: l.TLSKeyFile, + }, + Cert: &SecretSource{ + FromFile: l.TLSCertFile, + }, + MinVersion: l.TLSMinVersion, + } + if len(l.TLSCipherSuites) != 0 { + appServer.TLS.CipherSuites = l.TLSCipherSuites + } + // Preserve backwards compatibility, only run one server + appServer.BindAddress = "" + } else { + // Disable the HTTPS server if there's no certificates. + // This preserves backwards compatibility. + appServer.SecureBindAddress = "" + } + + metricsServer := Server{ + BindAddress: l.MetricsAddress, + SecureBindAddress: l.MetricsSecureAddress, + } + if l.MetricsTLSKeyFile != "" || l.MetricsTLSCertFile != "" { + metricsServer.TLS = &TLS{ + Key: &SecretSource{ + FromFile: l.MetricsTLSKeyFile, + }, + Cert: &SecretSource{ + FromFile: l.MetricsTLSCertFile, + }, + } + } + + return appServer, metricsServer +} diff --git a/pkg/apis/options/legacy_upstream.go b/pkg/apis/options/legacy_upstream.go new file mode 100644 index 00000000..8843e7da --- /dev/null +++ b/pkg/apis/options/legacy_upstream.go @@ -0,0 +1,105 @@ +package options + +import ( + "fmt" + "net/url" + "strconv" + "strings" + "time" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" + "github.com/spf13/pflag" +) + +type LegacyUpstreams struct { + FlushInterval time.Duration `flag:"flush-interval" cfg:"flush_interval"` + PassHostHeader bool `flag:"pass-host-header" cfg:"pass_host_header"` + ProxyWebSockets bool `flag:"proxy-websockets" cfg:"proxy_websockets"` + SSLUpstreamInsecureSkipVerify bool `flag:"ssl-upstream-insecure-skip-verify" cfg:"ssl_upstream_insecure_skip_verify"` + Upstreams []string `flag:"upstream" cfg:"upstreams"` + Timeout time.Duration `flag:"upstream-timeout" cfg:"upstream_timeout"` + DisableKeepAlives bool `flag:"disable-keep-alives" cfg:"disable_keep_alives"` +} + +func legacyUpstreamsFlagSet() *pflag.FlagSet { + flagSet := pflag.NewFlagSet("upstreams", pflag.ExitOnError) + + flagSet.Duration("flush-interval", DefaultUpstreamFlushInterval, "period between response flushing when streaming responses") + flagSet.Bool("pass-host-header", true, "pass the request Host Header to upstream") + flagSet.Bool("proxy-websockets", true, "enables WebSocket proxying") + flagSet.Bool("ssl-upstream-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS upstreams") + flagSet.StringSlice("upstream", []string{}, "the http url(s) of the upstream endpoint, file:// paths for static files or static:// for static response. Routing is based on the path") + flagSet.Duration("upstream-timeout", DefaultUpstreamTimeout, "maximum amount of time the server will wait for a response from the upstream") + flagSet.Bool("disable-keep-alives", false, "disable HTTP keep-alive connections to the upstream server") + + return flagSet +} + +func (l *LegacyUpstreams) convert() (UpstreamConfig, error) { + upstreams := UpstreamConfig{} + + for _, upstreamString := range l.Upstreams { + u, err := url.Parse(upstreamString) + if err != nil { + return UpstreamConfig{}, fmt.Errorf("could not parse upstream %q: %v", upstreamString, err) + } + + if u.Path == "" { + u.Path = "/" + } + + flushInterval := l.FlushInterval + timeout := l.Timeout + upstream := Upstream{ + ID: u.Path, + Path: u.Path, + URI: upstreamString, + InsecureSkipTLSVerify: &l.SSLUpstreamInsecureSkipVerify, + PassHostHeader: &l.PassHostHeader, + ProxyWebSockets: &l.ProxyWebSockets, + FlushInterval: &flushInterval, + Timeout: &timeout, + DisableKeepAlives: &l.DisableKeepAlives, + } + + switch u.Scheme { + case "file": + if u.Fragment != "" { + upstream.ID = u.Fragment + upstream.Path = u.Fragment + // Trim the fragment from the end of the URI + upstream.URI = strings.SplitN(upstreamString, "#", 2)[0] + } + case "static": + responseCode, err := strconv.Atoi(u.Host) + if err != nil { + logger.Errorf("unable to convert %q to int, use default \"200\"", u.Host) + responseCode = 200 + } + upstream.Static = ptr.To(true) + upstream.StaticCode = &responseCode + + // This is not allowed to be empty and must be unique + upstream.ID = upstreamString + + // We only support the root path in the legacy config + upstream.Path = "/" + + // Force defaults compatible with static responses + upstream.URI = "" + upstream.InsecureSkipTLSVerify = ptr.To(false) + upstream.DisableKeepAlives = ptr.To(false) + upstream.PassHostHeader = nil + upstream.ProxyWebSockets = nil + upstream.FlushInterval = nil + upstream.Timeout = nil + case "unix": + upstream.Path = "/" + } + + upstreams.Upstreams = append(upstreams.Upstreams, upstream) + } + + return upstreams, nil +} diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index 42083f76..e6d7a890 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -46,6 +46,20 @@ var _ = Describe("Load", func() { InsecureOIDCSkipNonce: true, }, + LegacyCookie: LegacyCookie{ + Name: "_oauth2_proxy", + Secret: "", + Domains: nil, + Path: "/", + Expire: time.Duration(168) * time.Hour, + Refresh: time.Duration(0), + Secure: true, + HTTPOnly: true, + SameSite: "", + CSRFPerRequest: false, + CSRFExpire: time.Duration(15) * time.Minute, + }, + Options: Options{ BearerTokenLoginFallback: true, ProxyPrefix: "/oauth2", @@ -53,7 +67,6 @@ var _ = Describe("Load", func() { ReadyPath: "/ready", RealClientIPHeader: "X-Real-IP", ForceHTTPS: false, - Cookie: cookieDefaults(), Session: sessionOptionsDefaults(), Templates: templatesDefaults(), SkipAuthPreflight: false, diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index b57d5aed..02421eee 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -35,7 +35,7 @@ type Options struct { HtpasswdFile string `flag:"htpasswd-file" cfg:"htpasswd_file"` HtpasswdUserGroups []string `flag:"htpasswd-user-group" cfg:"htpasswd_user_groups"` - Cookie Cookie `cfg:",squash"` + Cookie Cookie `cfg:",internal"` Session SessionOptions `cfg:",squash"` Logging Logging `cfg:",squash"` Templates Templates `cfg:",squash"` @@ -105,7 +105,6 @@ func NewOptions() *Options { ReadyPath: "/ready", RealClientIPHeader: "X-Real-IP", ForceHTTPS: false, - Cookie: cookieDefaults(), Session: sessionOptionsDefaults(), Templates: templatesDefaults(), SkipAuthPreflight: false, @@ -162,7 +161,6 @@ func NewFlagSet() *pflag.FlagSet { flagSet.String("signature-key", "", "GAP-Signature request signature key (algorithm:secretkey)") flagSet.Bool("gcp-healthchecks", false, "Enable GCP/GKE healthcheck endpoints") - flagSet.AddFlagSet(cookieFlagSet()) flagSet.AddFlagSet(loggingFlagSet()) flagSet.AddFlagSet(templatesFlagSet()) @@ -182,8 +180,9 @@ func (o *Options) EnsureDefaults() { o.InjectResponseHeaders[i].EnsureDefaults() } + o.Cookie.EnsureDefaults() + // TBD: Uncomment as we add EnsureDefaults methods - // o.Cookie.EnsureDefaults() // o.Session.EnsureDefaults() // o.Templates.EnsureDefaults() // o.Logging.EnsureDefaults() diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go index 4dd206e2..35476497 100644 --- a/pkg/apis/options/providers.go +++ b/pkg/apis/options/providers.go @@ -329,29 +329,6 @@ type LoginGovOptions struct { PubJWKURL string `yaml:"pubjwkURL,omitempty"` } -// Legacy default providers configuration -func providerDefaults() Providers { - providers := Providers{ - { - Type: "google", - AzureConfig: AzureOptions{ - Tenant: "common", - }, - OIDCConfig: OIDCOptions{ - InsecureAllowUnverifiedEmail: ptr.To(DefaultInsecureAllowUnverifiedEmail), - InsecureSkipNonce: ptr.To(DefaultInsecureSkipNonce), - SkipDiscovery: ptr.To(DefaultSkipDiscovery), - UserIDClaim: OIDCEmailClaim, // Deprecated: Use OIDCEmailClaim - EmailClaim: OIDCEmailClaim, - GroupsClaim: OIDCGroupsClaim, - AudienceClaims: OIDCAudienceClaims, - ExtraAudiences: []string{}, - }, - }, - } - return providers -} - // EnsureDefaults sets any default values for Providers fields. func (p Providers) EnsureDefaults() { for i := range p { diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index 24ae2841..236890f5 100644 --- a/pkg/cookies/cookies.go +++ b/pkg/cookies/cookies.go @@ -10,6 +10,7 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" requestutil "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests/util" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" ) // MakeCookieFromOptions constructs a cookie based on the given *options.CookieOptions, @@ -30,8 +31,8 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, opts *o Value: value, Path: opts.Path, Domain: domain, - HttpOnly: opts.HTTPOnly, - Secure: opts.Secure, + HttpOnly: ptr.Deref(opts.HTTPOnly, options.DefaultCookieHTTPOnly), + Secure: ptr.Deref(opts.Secure, options.DefaultCookieSecure), SameSite: ParseSameSite(opts.SameSite), } diff --git a/pkg/cookies/cookies_test.go b/pkg/cookies/cookies_test.go index ef0fbd4f..485e79a7 100644 --- a/pkg/cookies/cookies_test.go +++ b/pkg/cookies/cookies_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" . "github.com/onsi/ginkgo/v2" @@ -119,8 +120,8 @@ var _ = Describe("Cookie Tests", func() { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, expiration: 15 * time.Minute, @@ -138,8 +139,8 @@ var _ = Describe("Cookie Tests", func() { Path: "", Expire: time.Hour * -1, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, expiration: time.Hour * -1, @@ -157,8 +158,8 @@ var _ = Describe("Cookie Tests", func() { Path: "", Expire: 0, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, expiration: 0, diff --git a/pkg/cookies/csrf.go b/pkg/cookies/csrf.go index 939578a2..59893625 100644 --- a/pkg/cookies/csrf.go +++ b/pkg/cookies/csrf.go @@ -11,6 +11,7 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" "github.com/vmihailenco/msgpack/v5" ) @@ -96,7 +97,7 @@ func LoadCSRFCookie(req *http.Request, cookieName string, opts *options.Cookie) // build name based on the state func GenerateCookieName(opts *options.Cookie, state string) string { stateSubstring := "" - if opts.CSRFPerRequest { + if ptr.Deref(opts.CSRFPerRequest, options.DefaultCSRFPerRequest) { // csrfCookieName will include a substring of the state to enable multiple csrf cookies // in case of parallel requests stateSubstring = ExtractStateSubstring(state) @@ -156,7 +157,7 @@ func (c *csrf) SetCookie(rw http.ResponseWriter, req *http.Request) (*http.Cooki // ClearExtraCsrfCookies limits the amount of existing CSRF cookies by deleting // an excess of cookies controlled through the option CSRFPerRequestLimit func ClearExtraCsrfCookies(opts *options.Cookie, rw http.ResponseWriter, req *http.Request) { - if !opts.CSRFPerRequest || opts.CSRFPerRequestLimit <= 0 { + if !ptr.Deref(opts.CSRFPerRequest, options.DefaultCSRFPerRequest) || opts.CSRFPerRequestLimit <= 0 { return } @@ -262,7 +263,7 @@ func unmarshalCSRF(decrypted []byte, opts *options.Cookie, csrfTime time.Time) ( // cookieName returns the CSRF cookie's name func (c *csrf) cookieName() string { stateSubstring := "" - if c.cookieOpts.CSRFPerRequest { + if ptr.Deref(c.cookieOpts.CSRFPerRequest, options.DefaultCSRFPerRequest) { stateSubstring = encryption.HashNonce(c.OAuthState)[0 : csrfStateLength-1] } return csrfCookieName(c.cookieOpts, stateSubstring) diff --git a/pkg/cookies/csrf_per_request_test.go b/pkg/cookies/csrf_per_request_test.go index 6a17013b..048c5752 100644 --- a/pkg/cookies/csrf_per_request_test.go +++ b/pkg/cookies/csrf_per_request_test.go @@ -10,6 +10,7 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -28,9 +29,9 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { Domains: []string{cookieDomain}, Path: cookiePath, Expire: time.Hour, - Secure: true, - HTTPOnly: true, - CSRFPerRequest: true, + Secure: ptr.To(true), + HTTPOnly: ptr.To(true), + CSRFPerRequest: ptr.To(true), CSRFExpire: time.Duration(5) * time.Minute, } diff --git a/pkg/cookies/csrf_test.go b/pkg/cookies/csrf_test.go index 085b91df..7e50d0b1 100644 --- a/pkg/cookies/csrf_test.go +++ b/pkg/cookies/csrf_test.go @@ -10,6 +10,7 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -29,9 +30,9 @@ var _ = Describe("CSRF Cookie Tests", func() { Domains: []string{cookieDomain}, Path: cookiePath, Expire: time.Hour, - Secure: true, - HTTPOnly: true, - CSRFPerRequest: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(true), + CSRFPerRequest: ptr.To(false), CSRFExpire: time.Hour, } diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go index 0db45f78..3b811903 100644 --- a/pkg/sessions/session_store_test.go +++ b/pkg/sessions/session_store_test.go @@ -12,6 +12,7 @@ import ( sessionscookie "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/cookie" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/persistence" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/redis" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -44,8 +45,8 @@ var _ = Describe("NewSessionStore", func() { Path: "/", Expire: time.Duration(168) * time.Hour, Refresh: time.Duration(1) * time.Hour, - Secure: true, - HTTPOnly: true, + Secure: ptr.To(true), + HTTPOnly: ptr.To(true), SameSite: "", } }) diff --git a/pkg/sessions/tests/session_store_tests.go b/pkg/sessions/tests/session_store_tests.go index 05b67d8d..4ab7315e 100644 --- a/pkg/sessions/tests/session_store_tests.go +++ b/pkg/sessions/tests/session_store_tests.go @@ -13,6 +13,7 @@ import ( sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" cookiesapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/cookies" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -68,8 +69,8 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi Path: "/", Expire: time.Duration(168) * time.Hour, Refresh: time.Duration(1) * time.Hour, - Secure: true, - HTTPOnly: true, + Secure: ptr.To(true), + HTTPOnly: ptr.To(true), SameSite: "", Secret: string(cookieSecret), } @@ -117,8 +118,8 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi Path: "/path", Expire: time.Duration(72) * time.Hour, Refresh: time.Duration(2) * time.Hour, - Secure: false, - HTTPOnly: false, + Secure: ptr.To(false), + HTTPOnly: ptr.To(false), Domains: []string{"example.com"}, SameSite: "strict", Secret: string(cookieSecret), @@ -149,8 +150,8 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi Path: "/", Expire: time.Duration(168) * time.Hour, Refresh: time.Duration(1) * time.Hour, - Secure: true, - HTTPOnly: true, + Secure: ptr.To(true), + HTTPOnly: ptr.To(true), SameSite: "", Secret: "", SecretFile: tmpfile.Name(), @@ -208,13 +209,13 @@ func CheckCookieOptions(in *testInput) { It("have the correct HTTPOnly set", func() { for _, cookie := range cookies { - Expect(cookie.HttpOnly).To(Equal(in.cookieOpts.HTTPOnly)) + Expect(cookie.HttpOnly).To(Equal(*in.cookieOpts.HTTPOnly)) } }) It("have the correct secure set", func() { for _, cookie := range cookies { - Expect(cookie.Secure).To(Equal(in.cookieOpts.Secure)) + Expect(cookie.Secure).To(Equal(*in.cookieOpts.Secure)) } }) diff --git a/pkg/validation/cookie_test.go b/pkg/validation/cookie_test.go index d11134da..95e3203e 100644 --- a/pkg/validation/cookie_test.go +++ b/pkg/validation/cookie_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" . "github.com/onsi/gomega" ) @@ -66,8 +67,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, errStrings: []string{}, @@ -81,8 +82,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, errStrings: []string{ @@ -98,8 +99,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, errStrings: []string{ @@ -115,8 +116,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, errStrings: []string{}, @@ -130,8 +131,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, errStrings: []string{ @@ -147,8 +148,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, errStrings: []string{ @@ -164,8 +165,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, errStrings: []string{ @@ -181,8 +182,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: 15 * time.Minute, Refresh: time.Hour, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, errStrings: []string{ @@ -198,8 +199,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "none", }, errStrings: []string{}, @@ -213,8 +214,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "none", }, errStrings: []string{}, @@ -228,8 +229,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "none", }, errStrings: []string{}, @@ -243,8 +244,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: time.Hour, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "invalid", }, errStrings: []string{ @@ -260,8 +261,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: 15 * time.Minute, Refresh: time.Hour, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "invalid", }, errStrings: []string{ @@ -280,8 +281,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: 0, Refresh: 15 * time.Minute, - Secure: true, - HTTPOnly: false, + Secure: ptr.To(true), + HTTPOnly: ptr.To(false), SameSite: "", }, errStrings: []string{}, @@ -296,8 +297,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: 24 * time.Hour, Refresh: 0, - Secure: true, - HTTPOnly: true, + Secure: ptr.To(true), + HTTPOnly: ptr.To(true), SameSite: "", }, errStrings: []string{}, @@ -312,8 +313,8 @@ func TestValidateCookie(t *testing.T) { Path: "", Expire: 24 * time.Hour, Refresh: 0, - Secure: true, - HTTPOnly: true, + Secure: ptr.To(true), + HTTPOnly: ptr.To(true), SameSite: "", }, errStrings: []string{"could not read cookie secret file: /nonexistent/file.txt"}, diff --git a/pkg/validation/options.go b/pkg/validation/options.go index 13ce2e0b..ec0e05f0 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -74,7 +74,7 @@ func Validate(o *options.Options) error { var redirectURL *url.URL redirectURL, msgs = parseURL(o.RawRedirectURL, "redirect", msgs) o.SetRedirectURL(redirectURL) - if o.RawRedirectURL == "" && !o.Cookie.Secure && !o.ReverseProxy { + if o.RawRedirectURL == "" && !ptr.Deref(o.Cookie.Secure, options.DefaultCookieSecure) && !o.ReverseProxy { logger.Print("WARNING: no explicit redirect URL: redirects will default to insecure HTTP") } diff --git a/pkg/validation/options_test.go b/pkg/validation/options_test.go index 5ea748c1..cb13ee82 100644 --- a/pkg/validation/options_test.go +++ b/pkg/validation/options_test.go @@ -32,6 +32,7 @@ func testOptions() *options.Options { o.Providers[0].ClientID = clientID o.Providers[0].ClientSecret = clientSecret o.EmailDomains = []string{"*"} + o.EnsureDefaults() return o } @@ -45,6 +46,8 @@ func errorMsg(msgs []string) string { func TestNewOptions(t *testing.T) { o := options.NewOptions() o.EmailDomains = []string{"*"} + o.EnsureDefaults() + err := Validate(o) assert.NotEqual(t, nil, err) From ab6ab29258f7b872e39e6d77a354fb1ad4492255 Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Wed, 10 Dec 2025 12:32:01 +0100 Subject: [PATCH 2/7] feat: support for session options in alpha config and refactoring of cookie options Signed-off-by: Jan Larwig --- oauthproxy.go | 12 +- oauthproxy_test.go | 10 +- pkg/apis/options/cookie.go | 73 +++-- pkg/apis/options/cookie_test.go | 30 +- pkg/apis/options/legacy_cookie.go | 27 +- pkg/apis/options/legacy_header.go | 8 +- pkg/apis/options/legacy_options.go | 12 + pkg/apis/options/legacy_options_test.go | 7 +- pkg/apis/options/legacy_sessions.go | 79 +++++ pkg/apis/options/load_test.go | 8 +- pkg/apis/options/options.go | 18 +- pkg/apis/options/secret_source.go | 43 +++ pkg/apis/options/sessions.go | 115 ++++++-- pkg/cookies/cookies.go | 6 +- pkg/cookies/cookies_suite_test.go | 5 +- pkg/cookies/cookies_test.go | 53 ++-- pkg/cookies/csrf_per_request_test.go | 11 +- pkg/cookies/csrf_test.go | 10 +- pkg/middleware/headers_test.go | 2 +- pkg/sessions/cookie/session_store.go | 3 +- pkg/sessions/redis/redis_store.go | 10 +- pkg/sessions/redis/redis_store_test.go | 11 +- pkg/sessions/redis/redis_store_tls_test.go | 11 +- pkg/sessions/session_store_test.go | 25 +- pkg/sessions/tests/session_store_tests.go | 76 ++--- pkg/validation/cookie.go | 36 +-- pkg/validation/cookie_test.go | 322 +++++++++++---------- pkg/validation/header_test.go | 4 +- pkg/validation/options.go | 4 +- pkg/validation/options_test.go | 21 +- pkg/validation/sessions.go | 5 +- pkg/validation/sessions_test.go | 36 +-- 32 files changed, 671 insertions(+), 422 deletions(-) create mode 100644 pkg/apis/options/legacy_sessions.go diff --git a/oauthproxy.go b/oauthproxy.go index a42b7325..86ebfebc 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -176,11 +176,11 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr logger.Printf("OAuthProxy configured for %s Client ID: %s", provider.Data().ProviderName, opts.Providers[0].ClientID) refresh := "disabled" - if opts.Cookie.Refresh != time.Duration(0) { - refresh = fmt.Sprintf("after %s", opts.Cookie.Refresh) + if opts.Session.Refresh != time.Duration(0) { + refresh = fmt.Sprintf("after %s", opts.Session.Refresh) } - logger.Printf("Cookie settings: name:%s secure(https):%v httponly:%v expiry:%s domains:%s path:%s samesite:%s refresh:%s", opts.Cookie.Name, opts.Cookie.Secure, opts.Cookie.HTTPOnly, opts.Cookie.Expire, strings.Join(opts.Cookie.Domains, ","), opts.Cookie.Path, opts.Cookie.SameSite, refresh) + logger.Printf("Cookie settings: name:%s insecure(http):%v nothttponly:%v expiry:%s domains:%s path:%s samesite:%s refresh:%s", opts.Cookie.Name, opts.Cookie.Insecure, opts.Cookie.NotHttpOnly, opts.Cookie.Expire, strings.Join(opts.Cookie.Domains, ","), opts.Cookie.Path, opts.Cookie.SameSite, refresh) trustedIPs := ip.NewNetSet() for _, ipStr := range opts.TrustedIPs { @@ -416,7 +416,7 @@ func buildSessionChain(opts *options.Options, provider providers.Provider, sessi chain = chain.Append(middleware.NewStoredSessionLoader(&middleware.StoredSessionLoaderOptions{ SessionStore: sessionStore, - RefreshPeriod: opts.Cookie.Refresh, + RefreshPeriod: opts.Session.Refresh, RefreshSession: provider.RefreshSession, ValidateSession: provider.ValidateSession, })) @@ -1097,9 +1097,9 @@ func (p *OAuthProxy) getOAuthRedirectURI(req *http.Request) string { rd.Scheme = schemeHTTP } - // If CookieSecure is true, return `https` no matter what + // If CookieInsecure is false, return `https` no matter what // Not all reverse proxies set X-Forwarded-Proto - if ptr.Deref(p.CookieOptions.Secure, options.DefaultCookieSecure) { + if !ptr.Deref(p.CookieOptions.Insecure, options.DefaultCookieInsecure) { rd.Scheme = schemeHTTPS } return rd.String() diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 20360af4..722d84ee 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -819,9 +819,9 @@ func NewProcessCookieTest(opts ProcessCookieTestOpts, modifiers ...OptionsModifi for _, modifier := range modifiers { modifier(pcTest.opts) } - // First, set the CookieRefresh option so proxy.AesCipher is created, + // First, set the Session Refresh option so proxy.AesCipher is created, // needed to encrypt the access_token. - pcTest.opts.Cookie.Refresh = time.Hour + pcTest.opts.Session.Refresh = time.Hour err := validation.Validate(pcTest.opts) if err != nil { return nil, err @@ -845,9 +845,9 @@ func NewProcessCookieTest(opts ProcessCookieTestOpts, modifiers ...OptionsModifi } pcTest.proxy.provider = testProvider - // Now, zero-out proxy.CookieRefresh for the cases that don't involve + // Now, zero-out Session Refresh for the cases that don't involve // access_token validation. - pcTest.proxy.CookieOptions.Refresh = time.Duration(0) + pcTest.opts.Session.Refresh = time.Duration(0) pcTest.rw = httptest.NewRecorder() pcTest.req, _ = http.NewRequest("GET", "/", strings.NewReader("")) pcTest.validateUser = true @@ -969,7 +969,7 @@ func TestProcessCookieFailIfRefreshSetAndCookieExpired(t *testing.T) { err = pcTest.SaveSession(startSession) assert.NoError(t, err) - pcTest.proxy.CookieOptions.Refresh = time.Hour + pcTest.opts.Session.Refresh = time.Hour session, err := pcTest.LoadCookiedSession() assert.NotEqual(t, nil, err) if session != nil { diff --git a/pkg/apis/options/cookie.go b/pkg/apis/options/cookie.go index 2d594b75..6269bfbf 100644 --- a/pkg/apis/options/cookie.go +++ b/pkg/apis/options/cookie.go @@ -2,46 +2,53 @@ package options import ( "fmt" - "os" "time" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" + "go.yaml.in/yaml/v3" ) const ( - // DefaultCookieSecure is the default value for Cookie.Secure - DefaultCookieSecure bool = true - // DefaultCookieHTTPOnly is the default value for Cookie.HTTPOnly - DefaultCookieHTTPOnly bool = true + // DefaultCookieInsecure is the default value for Cookie.Insecure + DefaultCookieInsecure bool = false + // DefaultCookieNotHttpOnly is the default value for Cookie.NotHttpOnly + DefaultCookieNotHttpOnly bool = false // DefaultCSRFPerRequest is the default value for Cookie.CSRFPerRequest DefaultCSRFPerRequest bool = false ) +type SameSiteMode string + +const ( + SameSiteLax SameSiteMode = "lax" + SameSiteStrict SameSiteMode = "strict" + SameSiteNone SameSiteMode = "none" + SameSiteDefault SameSiteMode = "" +) + // Cookie contains configuration options relating session and CSRF cookies type Cookie struct { // Name is the name of the cookie Name string `yaml:"name,omitempty"` - // Secret is the secret used to encrypt/sign the cookie value - Secret string `yaml:"secret,omitempty"` - // SecretFile is a file containing the secret used to encrypt/sign the cookie value - // instead of specifying it directly in the config. Secret takes precedence over SecretFile - SecretFile string `yaml:"secretFile,omitempty"` + // Secret is the secret source used to encrypt/sign the cookie value + Secret SecretSource `yaml:"secret,omitempty"` // Domains is a list of domains for which the cookie is valid Domains []string `yaml:"domains,omitempty"` // Path is the path for which the cookie is valid Path string `yaml:"path,omitempty"` // Expire is the duration before the cookie expires Expire time.Duration `yaml:"expire,omitempty"` - // Refresh is the duration after which the cookie is refreshable - Refresh time.Duration `yaml:"refresh,omitempty"` - // Secure indicates whether the cookie is only sent over HTTPS - Secure *bool `yaml:"secure,omitempty"` - // HTTPOnly indicates whether the cookie is inaccessible to JavaScript - HTTPOnly *bool `yaml:"httpOnly,omitempty"` + // Insecure indicates whether the cookie allows to be sent over HTTP + // Default is false, which requires HTTPS + Insecure *bool `yaml:"insecure,omitempty"` + // NotHttpOnly is the inverse of HTTPOnly; indicates whether the cookie is accessible to JavaScript + // Default is false, which helps mitigate certain XSS attacks + NotHttpOnly *bool `yaml:"notHttpOnly,omitempty"` // SameSite sets the SameSite attribute on the cookie - SameSite string `yaml:"sameSite,omitempty"` + SameSite SameSiteMode `yaml:"sameSite,omitempty"` // CSRFPerRequest indicates whether a unique CSRF token is generated for each request // Enables parallel requests from clients (e.g., multiple tabs) + // Default is false, which uses a single CSRF token per session CSRFPerRequest *bool `yaml:"csrfPerRequest,omitempty"` // CSRFPerRequestLimit sets a limit on the number of valid CSRF tokens when CSRFPerRequest is enabled // Used to prevent unbounded memory growth from storing too many tokens @@ -50,18 +57,28 @@ type Cookie struct { CSRFExpire time.Duration `yaml:"csrfExpire,omitempty"` } -// GetSecret returns the cookie secret, reading from file if SecretFile is set -func (c *Cookie) GetSecret() (secret string, err error) { - if c.Secret != "" || c.SecretFile == "" { - return c.Secret, nil +func (m *SameSiteMode) UnmarshalYAML(value *yaml.Node) error { + var s string + if err := value.Decode(&s); err != nil { + return err } + switch SameSiteMode(s) { + case SameSiteLax, SameSiteStrict, SameSiteNone, SameSiteDefault: + *m = SameSiteMode(s) + return nil + default: + return fmt.Errorf("invalid same site mode: %s", s) + } +} - fileSecret, err := os.ReadFile(c.SecretFile) +// GetSecret returns the cookie secret as a string from the SecretSource +func (c *Cookie) GetSecret() (string, error) { + secret, err := c.Secret.GetSecretValue() if err != nil { - return "", fmt.Errorf("error reading cookie secret file %s: %w", c.SecretFile, err) + return "", fmt.Errorf("error getting cookie secret: %w", err) } - return string(fileSecret), nil + return string(secret), nil } // EnsureDefaults sets any default values for the Cookie configuration @@ -75,11 +92,11 @@ func (c *Cookie) EnsureDefaults() { if c.Expire == 0 { c.Expire = time.Duration(168) * time.Hour } - if c.Secure == nil { - c.Secure = ptr.To(DefaultCookieSecure) + if c.Insecure == nil { + c.Insecure = ptr.To(DefaultCookieInsecure) } - if c.HTTPOnly == nil { - c.HTTPOnly = ptr.To(DefaultCookieHTTPOnly) + if c.NotHttpOnly == nil { + c.NotHttpOnly = ptr.To(DefaultCookieNotHttpOnly) } if c.CSRFPerRequest == nil { c.CSRFPerRequest = ptr.To(DefaultCSRFPerRequest) diff --git a/pkg/apis/options/cookie_test.go b/pkg/apis/options/cookie_test.go index 4fb0e62e..053d0e6b 100644 --- a/pkg/apis/options/cookie_test.go +++ b/pkg/apis/options/cookie_test.go @@ -10,8 +10,10 @@ import ( func TestCookieGetSecret(t *testing.T) { t.Run("returns secret when Secret is set", func(t *testing.T) { c := &Cookie{ - Secret: "my-secret", - SecretFile: "", + Secret: SecretSource{ + Value: []byte("my-secret"), + FromFile: "", + }, } secret, err := c.GetSecret() assert.NoError(t, err) @@ -20,8 +22,10 @@ func TestCookieGetSecret(t *testing.T) { t.Run("returns secret when both Secret and SecretFile are set", func(t *testing.T) { c := &Cookie{ - Secret: "my-secret", - SecretFile: "/some/file", + Secret: SecretSource{ + Value: []byte("my-secret"), + FromFile: "/some/file", + }, } secret, err := c.GetSecret() assert.NoError(t, err) @@ -39,8 +43,10 @@ func TestCookieGetSecret(t *testing.T) { tmpfile.Close() c := &Cookie{ - Secret: "", - SecretFile: tmpfile.Name(), + Secret: SecretSource{ + Value: []byte(""), + FromFile: tmpfile.Name(), + }, } secret, err := c.GetSecret() assert.NoError(t, err) @@ -49,8 +55,10 @@ func TestCookieGetSecret(t *testing.T) { t.Run("returns error when file does not exist", func(t *testing.T) { c := &Cookie{ - Secret: "", - SecretFile: "/nonexistent/file", + Secret: SecretSource{ + Value: []byte(""), + FromFile: "/nonexistent/file", + }, } secret, err := c.GetSecret() assert.Error(t, err) @@ -60,8 +68,10 @@ func TestCookieGetSecret(t *testing.T) { t.Run("returns empty when both Secret and SecretFile are empty", func(t *testing.T) { c := &Cookie{ - Secret: "", - SecretFile: "", + Secret: SecretSource{ + Value: []byte(""), + FromFile: "", + }, } secret, err := c.GetSecret() assert.NoError(t, err) diff --git a/pkg/apis/options/legacy_cookie.go b/pkg/apis/options/legacy_cookie.go index 287db87f..be1235ca 100644 --- a/pkg/apis/options/legacy_cookie.go +++ b/pkg/apis/options/legacy_cookie.go @@ -1,8 +1,10 @@ package options import ( + "encoding/base64" "time" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/spf13/pflag" ) @@ -39,21 +41,34 @@ func legacyCookieFlagSet() *pflag.FlagSet { flagSet.Bool("cookie-csrf-per-request", false, "When this property is set to true, then the CSRF cookie name is built based on the state and varies per request. If property is set to false, then CSRF cookie has the same name for all requests.") flagSet.Int("cookie-csrf-per-request-limit", 0, "Sets a limit on the number of CSRF requests cookies that oauth2-proxy will create. The oldest cookies will be removed. Useful if users end up with 431 Request headers too large status codes.") flagSet.Duration("cookie-csrf-expire", time.Duration(15)*time.Minute, "expire timeframe for CSRF cookie") + return flagSet } func (l *LegacyCookie) convert() Cookie { + // Invert Secure and HTTPOnly to match the new Cookie struct + // which uses Insecure and NotHttpOnly + insecure := !l.Secure + notHTTPOnly := !l.HTTPOnly + + var secret *SecretSource + if l.Secret != "" { + secret = NewSecretSourceFromString(l.Secret) + } else if l.SecretFile != "" { + secret = &SecretSource{ + FromFile: l.SecretFile, + } + } + return Cookie{ Name: l.Name, - Secret: l.Secret, - SecretFile: l.SecretFile, + Secret: *secret, Domains: l.Domains, Path: l.Path, Expire: l.Expire, - Refresh: l.Refresh, - Secure: &l.Secure, - HTTPOnly: &l.HTTPOnly, - SameSite: l.SameSite, + Insecure: &insecure, + NotHttpOnly: ¬HTTPOnly, + SameSite: SameSiteMode(l.SameSite), CSRFPerRequest: &l.CSRFPerRequest, CSRFPerRequestLimit: l.CSRFPerRequestLimit, CSRFExpire: l.CSRFExpire, diff --git a/pkg/apis/options/legacy_header.go b/pkg/apis/options/legacy_header.go index 58c337a8..aeec99e4 100644 --- a/pkg/apis/options/legacy_header.go +++ b/pkg/apis/options/legacy_header.go @@ -106,11 +106,9 @@ func getBasicAuthHeader(preferEmailToUser bool, basicAuthPassword string) Header Values: []HeaderValue{ { ClaimSource: &ClaimSource{ - Claim: claim, - Prefix: "Basic ", - BasicAuthPassword: &SecretSource{ - Value: []byte(basicAuthPassword), - }, + Claim: claim, + Prefix: "Basic ", + BasicAuthPassword: NewSecretSourceFromString(basicAuthPassword), }, }, }, diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index d8033a0d..404debd1 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -23,6 +23,9 @@ type LegacyOptions struct { // Legacy options for cookie configuration LegacyCookie LegacyCookie `cfg:",squash"` + // Legacy options for session store configuration + LegacySessionOptions LegacySessionOptions `cfg:",squash"` + Options Options `cfg:",squash"` } @@ -74,6 +77,13 @@ func NewLegacyOptions() *LegacyOptions { CSRFExpire: time.Duration(15) * time.Minute, }, + LegacySessionOptions: LegacySessionOptions{ + Type: "cookie", + Cookie: LegacyCookieStoreOptions{ + Minimal: false, + }, + }, + Options: *NewOptions(), } } @@ -87,6 +97,7 @@ func NewLegacyFlagSet() *pflag.FlagSet { flagSet.AddFlagSet(legacyProviderFlagSet()) flagSet.AddFlagSet(legacyGoogleFlagSet()) flagSet.AddFlagSet(legacyCookieFlagSet()) + flagSet.AddFlagSet(legacySessionFlagSet()) return flagSet } @@ -109,6 +120,7 @@ func (l *LegacyOptions) ToOptions() (*Options, error) { } l.Options.Providers = providers l.Options.Cookie = l.LegacyCookie.convert() + l.Options.Session = l.LegacySessionOptions.convert(l.LegacyCookie.Refresh) l.Options.EnsureDefaults() diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index f69e6fea..b6be7a73 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -1094,13 +1094,12 @@ var _ = Describe("Legacy Options", func() { // Test cases and expected outcomes fullCookie := Cookie{ Name: "_oauth2_proxy", - Secret: "", + Secret: SecretSource{}, Domains: nil, Path: "/", Expire: time.Duration(168) * time.Hour, - Refresh: time.Duration(0), - Secure: ptr.To(true), - HTTPOnly: ptr.To(true), + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(false), SameSite: "", CSRFPerRequest: ptr.To(false), CSRFPerRequestLimit: 0, diff --git a/pkg/apis/options/legacy_sessions.go b/pkg/apis/options/legacy_sessions.go new file mode 100644 index 00000000..08a2b263 --- /dev/null +++ b/pkg/apis/options/legacy_sessions.go @@ -0,0 +1,79 @@ +package options + +import ( + "time" + + "github.com/spf13/pflag" +) + +// LegacySessionOptions contains configuration options for the SessionStore providers. +type LegacySessionOptions struct { + Type string `flag:"session-store-type" cfg:"session_store_type"` + Cookie LegacyCookieStoreOptions `cfg:",squash"` + Redis LegacyRedisStoreOptions `cfg:",squash"` +} + +// LegacyCookieStoreOptions contains configuration options for the CookieSessionStore. +type LegacyCookieStoreOptions struct { + Minimal bool `flag:"session-cookie-minimal" cfg:"session_cookie_minimal"` +} + +// RedisStoreOptions contains configuration options for the RedisSessionStore. +type LegacyRedisStoreOptions struct { + ConnectionURL string `flag:"redis-connection-url" cfg:"redis_connection_url"` + Username string `flag:"redis-username" cfg:"redis_username"` + Password string `flag:"redis-password" cfg:"redis_password"` + UseSentinel bool `flag:"redis-use-sentinel" cfg:"redis_use_sentinel"` + SentinelPassword string `flag:"redis-sentinel-password" cfg:"redis_sentinel_password"` + SentinelMasterName string `flag:"redis-sentinel-master-name" cfg:"redis_sentinel_master_name"` + SentinelConnectionURLs []string `flag:"redis-sentinel-connection-urls" cfg:"redis_sentinel_connection_urls"` + UseCluster bool `flag:"redis-use-cluster" cfg:"redis_use_cluster"` + ClusterConnectionURLs []string `flag:"redis-cluster-connection-urls" cfg:"redis_cluster_connection_urls"` + CAPath string `flag:"redis-ca-path" cfg:"redis_ca_path"` + InsecureSkipTLSVerify bool `flag:"redis-insecure-skip-tls-verify" cfg:"redis_insecure_skip_tls_verify"` + IdleTimeout int `flag:"redis-connection-idle-timeout" cfg:"redis_connection_idle_timeout"` +} + +func legacySessionFlagSet() *pflag.FlagSet { + flagSet := pflag.NewFlagSet("session", pflag.ExitOnError) + + flagSet.String("session-store-type", "cookie", "the session storage provider to use") + flagSet.Bool("session-cookie-minimal", false, "strip OAuth tokens from cookie session stores if they aren't needed (cookie session store only)") + flagSet.String("redis-connection-url", "", "URL of redis server for redis session storage (eg: redis://[USER[:PASSWORD]@]HOST[:PORT])") + flagSet.String("redis-username", "", "Redis username. Applicable for Redis configurations where ACL has been configured. Will override any username set in `--redis-connection-url`") + flagSet.String("redis-password", "", "Redis password. Applicable for all Redis configurations. Will override any password set in `--redis-connection-url`") + flagSet.Bool("redis-use-sentinel", false, "Connect to redis via sentinels. Must set --redis-sentinel-master-name and --redis-sentinel-connection-urls to use this feature") + flagSet.String("redis-sentinel-password", "", "Redis sentinel password. Used only for sentinel connection; any redis node passwords need to use `--redis-password`") + flagSet.String("redis-sentinel-master-name", "", "Redis sentinel master name. Used in conjunction with --redis-use-sentinel") + flagSet.String("redis-ca-path", "", "Redis custom CA path") + flagSet.Bool("redis-insecure-skip-tls-verify", false, "Use insecure TLS connection to redis") + flagSet.StringSlice("redis-sentinel-connection-urls", []string{}, "List of Redis sentinel connection URLs (eg redis://[USER[:PASSWORD]@]HOST[:PORT]). Used in conjunction with --redis-use-sentinel") + flagSet.Bool("redis-use-cluster", false, "Connect to redis cluster. Must set --redis-cluster-connection-urls to use this feature") + flagSet.StringSlice("redis-cluster-connection-urls", []string{}, "List of Redis cluster connection URLs (eg redis://[USER[:PASSWORD]@]HOST[:PORT]). Used in conjunction with --redis-use-cluster") + flagSet.Int("redis-connection-idle-timeout", 0, "Redis connection idle timeout seconds, if Redis timeout option is non-zero, the --redis-connection-idle-timeout must be less then Redis timeout option") + + return flagSet +} + +func (l *LegacySessionOptions) convert(legacyCookieRefresh time.Duration) SessionOptions { + return SessionOptions{ + Type: SessionStoreType(l.Type), + Refresh: legacyCookieRefresh, + Cookie: CookieStoreOptions{ + Minimal: &l.Cookie.Minimal, + }, + Redis: RedisStoreOptions{ + ConnectionURL: l.Redis.ConnectionURL, + Password: l.Redis.Password, + UseSentinel: &l.Redis.UseSentinel, + SentinelPassword: l.Redis.SentinelPassword, + SentinelMasterName: l.Redis.SentinelMasterName, + SentinelConnectionURLs: l.Redis.SentinelConnectionURLs, + UseCluster: &l.Redis.UseCluster, + ClusterConnectionURLs: l.Redis.ClusterConnectionURLs, + CAPath: l.Redis.CAPath, + InsecureSkipTLSVerify: &l.Redis.InsecureSkipTLSVerify, + IdleTimeout: l.Redis.IdleTimeout, + }, + } +} diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index e6d7a890..fca8147e 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -60,6 +60,13 @@ var _ = Describe("Load", func() { CSRFExpire: time.Duration(15) * time.Minute, }, + LegacySessionOptions: LegacySessionOptions{ + Type: "cookie", + Cookie: LegacyCookieStoreOptions{ + Minimal: false, + }, + }, + Options: Options{ BearerTokenLoginFallback: true, ProxyPrefix: "/oauth2", @@ -67,7 +74,6 @@ var _ = Describe("Load", func() { ReadyPath: "/ready", RealClientIPHeader: "X-Real-IP", ForceHTTPS: false, - Session: sessionOptionsDefaults(), Templates: templatesDefaults(), SkipAuthPreflight: false, Logging: loggingDefaults(), diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 02421eee..3555f55f 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -105,7 +105,6 @@ func NewOptions() *Options { ReadyPath: "/ready", RealClientIPHeader: "X-Real-IP", ForceHTTPS: false, - Session: sessionOptionsDefaults(), Templates: templatesDefaults(), SkipAuthPreflight: false, Logging: loggingDefaults(), @@ -144,20 +143,6 @@ func NewFlagSet() *pflag.FlagSet { flagSet.String("ping-path", "/ping", "the ping endpoint that can be used for basic health checks") flagSet.String("ping-user-agent", "", "special User-Agent that will be used for basic health checks") flagSet.String("ready-path", "/ready", "the ready endpoint that can be used for deep health checks") - flagSet.String("session-store-type", "cookie", "the session storage provider to use") - flagSet.Bool("session-cookie-minimal", false, "strip OAuth tokens from cookie session stores if they aren't needed (cookie session store only)") - flagSet.String("redis-connection-url", "", "URL of redis server for redis session storage (eg: redis://[USER[:PASSWORD]@]HOST[:PORT])") - flagSet.String("redis-username", "", "Redis username. Applicable for Redis configurations where ACL has been configured. Will override any username set in `--redis-connection-url`") - flagSet.String("redis-password", "", "Redis password. Applicable for all Redis configurations. Will override any password set in `--redis-connection-url`") - flagSet.Bool("redis-use-sentinel", false, "Connect to redis via sentinels. Must set --redis-sentinel-master-name and --redis-sentinel-connection-urls to use this feature") - flagSet.String("redis-sentinel-password", "", "Redis sentinel password. Used only for sentinel connection; any redis node passwords need to use `--redis-password`") - flagSet.String("redis-sentinel-master-name", "", "Redis sentinel master name. Used in conjunction with --redis-use-sentinel") - flagSet.String("redis-ca-path", "", "Redis custom CA path") - flagSet.Bool("redis-insecure-skip-tls-verify", false, "Use insecure TLS connection to redis") - flagSet.StringSlice("redis-sentinel-connection-urls", []string{}, "List of Redis sentinel connection URLs (eg redis://[USER[:PASSWORD]@]HOST[:PORT]). Used in conjunction with --redis-use-sentinel") - flagSet.Bool("redis-use-cluster", false, "Connect to redis cluster. Must set --redis-cluster-connection-urls to use this feature") - flagSet.StringSlice("redis-cluster-connection-urls", []string{}, "List of Redis cluster connection URLs (eg redis://[USER[:PASSWORD]@]HOST[:PORT]). Used in conjunction with --redis-use-cluster") - flagSet.Int("redis-connection-idle-timeout", 0, "Redis connection idle timeout seconds, if Redis timeout option is non-zero, the --redis-connection-idle-timeout must be less then Redis timeout option") flagSet.String("signature-key", "", "GAP-Signature request signature key (algorithm:secretkey)") flagSet.Bool("gcp-healthchecks", false, "Enable GCP/GKE healthcheck endpoints") @@ -181,9 +166,8 @@ func (o *Options) EnsureDefaults() { } o.Cookie.EnsureDefaults() - + o.Session.EnsureDefaults() // TBD: Uncomment as we add EnsureDefaults methods - // o.Session.EnsureDefaults() // o.Templates.EnsureDefaults() // o.Logging.EnsureDefaults() } diff --git a/pkg/apis/options/secret_source.go b/pkg/apis/options/secret_source.go index 8b783342..d73ac41d 100644 --- a/pkg/apis/options/secret_source.go +++ b/pkg/apis/options/secret_source.go @@ -1,5 +1,11 @@ package options +import ( + "encoding/base64" + "fmt" + "os" +) + // SecretSource references an individual secret value. // Only one source within the struct should be defined at any time. type SecretSource struct { @@ -13,6 +19,43 @@ type SecretSource struct { FromFile string `yaml:"fromFile,omitempty"` } +func NewSecretSourceFromValue(value []byte) *SecretSource { + encoded := make([]byte, base64.RawStdEncoding.EncodedLen(len(value))) + base64.RawStdEncoding.Encode(encoded, value) + return &SecretSource{ + Value: encoded, + } +} + +func NewSecretSourceFromString(s string) *SecretSource { + return NewSecretSourceFromValue([]byte(s)) +} + +func (ss *SecretSource) GetSecretValue() ([]byte, error) { + if len(ss.Value) > 0 { + var decoded []byte + if _, err := base64.RawStdEncoding.Decode(decoded, ss.Value); err != nil { + return nil, fmt.Errorf("error decoding secret value: %w", err) + } + return decoded, nil + } + + if ss.FromEnv != "" { + envValue := os.Getenv(ss.FromEnv) + return []byte(envValue), nil + } + + if ss.FromFile != "" { + fileData, err := os.ReadFile(ss.FromFile) + if err != nil { + return nil, fmt.Errorf("error reading secret from file %q: %w", ss.FromFile, err) + } + return fileData, nil + } + + return nil, nil +} + // EnsureDefaults sets any default values for SecretSource fields. func (ss *SecretSource) EnsureDefaults() { // No defaults to set currently diff --git a/pkg/apis/options/sessions.go b/pkg/apis/options/sessions.go index c90c0ac2..8b51b6ef 100644 --- a/pkg/apis/options/sessions.go +++ b/pkg/apis/options/sessions.go @@ -1,46 +1,101 @@ package options +import ( + "time" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" +) + +type SessionStoreType string + +const ( + // CookieSessionStoreType is used to indicate the CookieSessionStore should be + // used for storing sessions. + CookieSessionStoreType SessionStoreType = "cookie" + + // RedisSessionStoreType is used to indicate the RedisSessionStore should be + // used for storing sessions. + RedisSessionStoreType SessionStoreType = "redis" + + // DefaultCookieStoreMinimal is the default value for CookieStoreOptions.Minimal + DefaultCookieStoreMinimal bool = false + + // DefaultRedisStoreUseSentinel is the default value for RedisStoreOptions.UseSentinel + DefaultRedisStoreUseSentinel bool = false + + // DefaultRedisStoreUseCluster is the default value for RedisStoreOptions.UseCluster + DefaultRedisStoreUseCluster bool = false + + // DefaultRedisStoreInsecureSkipTLSVerify is the default value for RedisStoreOptions.InsecureSkipTLSVerify + DefaultRedisStoreInsecureSkipTLSVerify bool = false +) + // SessionOptions contains configuration options for the SessionStore providers. type SessionOptions struct { - Type string `flag:"session-store-type" cfg:"session_store_type"` - Cookie CookieStoreOptions `cfg:",squash"` - Redis RedisStoreOptions `cfg:",squash"` + // Type is the type of session store to use + // Options are "cookie" or "redis" + // Default is "cookie" + Type SessionStoreType `yaml:"type,omitempty"` + // Refresh is the duration after which the session is refreshable + Refresh time.Duration `yaml:"refresh,omitempty"` + // Cookie is the configuration options for the CookieSessionStore + Cookie CookieStoreOptions `yaml:"cookie,omitempty"` + // Redis is the configuration options for the RedisSessionStore + Redis RedisStoreOptions `yaml:"redis,omitempty"` } -// CookieSessionStoreType is used to indicate the CookieSessionStore should be -// used for storing sessions. -var CookieSessionStoreType = "cookie" - -// RedisSessionStoreType is used to indicate the RedisSessionStore should be -// used for storing sessions. -var RedisSessionStoreType = "redis" - // CookieStoreOptions contains configuration options for the CookieSessionStore. type CookieStoreOptions struct { - Minimal bool `flag:"session-cookie-minimal" cfg:"session_cookie_minimal"` + // Minimal indicates whether to use minimal cookies for session storage + // Default is false + Minimal *bool `yaml:"minimal,omitempty"` } // RedisStoreOptions contains configuration options for the RedisSessionStore. type RedisStoreOptions struct { - ConnectionURL string `flag:"redis-connection-url" cfg:"redis_connection_url"` - Username string `flag:"redis-username" cfg:"redis_username"` - Password string `flag:"redis-password" cfg:"redis_password"` - UseSentinel bool `flag:"redis-use-sentinel" cfg:"redis_use_sentinel"` - SentinelPassword string `flag:"redis-sentinel-password" cfg:"redis_sentinel_password"` - SentinelMasterName string `flag:"redis-sentinel-master-name" cfg:"redis_sentinel_master_name"` - SentinelConnectionURLs []string `flag:"redis-sentinel-connection-urls" cfg:"redis_sentinel_connection_urls"` - UseCluster bool `flag:"redis-use-cluster" cfg:"redis_use_cluster"` - ClusterConnectionURLs []string `flag:"redis-cluster-connection-urls" cfg:"redis_cluster_connection_urls"` - CAPath string `flag:"redis-ca-path" cfg:"redis_ca_path"` - InsecureSkipTLSVerify bool `flag:"redis-insecure-skip-tls-verify" cfg:"redis_insecure_skip_tls_verify"` - IdleTimeout int `flag:"redis-connection-idle-timeout" cfg:"redis_connection_idle_timeout"` + // ConnectionURL is the Redis connection URL + ConnectionURL string `yaml:"connectionURL,omitempty"` + // Username is the Redis username + Username string `yaml:"username,omitempty"` + // Password is the Redis password + Password string `yaml:"password,omitempty"` + // UseSentinel indicates whether to use Redis Sentinel + // Default is false + UseSentinel *bool `yaml:"useSentinel,omitempty"` + // SentinelPassword is the Redis Sentinel password + SentinelPassword string `yaml:"sentinelPassword,omitempty"` + // SentinelMasterName is the Redis Sentinel master name + SentinelMasterName string `yaml:"sentinelMasterName,omitempty"` + // SentinelConnectionURLs is a list of Redis Sentinel connection URLs + SentinelConnectionURLs []string `yaml:"sentinelConnectionURLs,omitempty"` + // UseCluster indicates whether to use Redis Cluster + // Default is false + UseCluster *bool `yaml:"useCluster,omitempty"` + // ClusterConnectionURLs is a list of Redis Cluster connection URLs + ClusterConnectionURLs []string `yaml:"clusterConnectionURLs,omitempty"` + // CAPath is the path to the CA certificate for Redis TLS connections + CAPath string `yaml:"caPath,omitempty"` + // InsecureSkipTLSVerify indicates whether to skip TLS verification for Redis connections + InsecureSkipTLSVerify *bool `yaml:"insecureSkipTLSVerify,omitempty"` + // IdleTimeout is the Redis connection idle timeout in seconds + IdleTimeout int `yaml:"idleTimeout,omitempty"` } -func sessionOptionsDefaults() SessionOptions { - return SessionOptions{ - Type: CookieSessionStoreType, - Cookie: CookieStoreOptions{ - Minimal: false, - }, +// EnsureDefaults sets default values for SessionOptions +func (s *SessionOptions) EnsureDefaults() { + if s.Type == "" { + s.Type = CookieSessionStoreType + } + if s.Cookie.Minimal == nil { + s.Cookie.Minimal = ptr.To(DefaultCookieStoreMinimal) + } + if s.Redis.UseSentinel == nil { + s.Redis.UseSentinel = ptr.To(DefaultRedisStoreUseSentinel) + } + if s.Redis.UseCluster == nil { + s.Redis.UseCluster = ptr.To(DefaultRedisStoreUseCluster) + } + if s.Redis.InsecureSkipTLSVerify == nil { + s.Redis.InsecureSkipTLSVerify = ptr.To(DefaultRedisStoreInsecureSkipTLSVerify) } } diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index 236890f5..252daeb6 100644 --- a/pkg/cookies/cookies.go +++ b/pkg/cookies/cookies.go @@ -31,8 +31,8 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, opts *o Value: value, Path: opts.Path, Domain: domain, - HttpOnly: ptr.Deref(opts.HTTPOnly, options.DefaultCookieHTTPOnly), - Secure: ptr.Deref(opts.Secure, options.DefaultCookieSecure), + HttpOnly: !ptr.Deref(opts.NotHttpOnly, options.DefaultCookieNotHttpOnly), + Secure: !ptr.Deref(opts.Insecure, options.DefaultCookieInsecure), SameSite: ParseSameSite(opts.SameSite), } @@ -60,7 +60,7 @@ func GetCookieDomain(req *http.Request, cookieDomains []string) string { } // Parse a valid http.SameSite value from a user supplied string for use of making cookies. -func ParseSameSite(v string) http.SameSite { +func ParseSameSite(v options.SameSiteMode) http.SameSite { switch v { case "lax": return http.SameSiteLaxMode diff --git a/pkg/cookies/cookies_suite_test.go b/pkg/cookies/cookies_suite_test.go index f4893cbd..0d152b4d 100644 --- a/pkg/cookies/cookies_suite_test.go +++ b/pkg/cookies/cookies_suite_test.go @@ -13,13 +13,16 @@ const ( csrfNonce = "0987lkjh0987lkjh0987lkjh" cookieName = "cookie_test_12345" - cookieSecret = "3q48hmFH30FJ2HfJF0239UFJCVcl3kj3" cookieDomain = "o2p.cookies.test" cookiePath = "/cookie-tests" nowEpoch = 1609366421 ) +var ( + cookieSecret = []byte("3q48hmFH30FJ2HfJF0239UFJCVcl3kj3") +) + func TestProviderSuite(t *testing.T) { logger.SetOutput(GinkgoWriter) diff --git a/pkg/cookies/cookies_test.go b/pkg/cookies/cookies_test.go index 485e79a7..13131c4b 100644 --- a/pkg/cookies/cookies_test.go +++ b/pkg/cookies/cookies_test.go @@ -92,7 +92,7 @@ var _ = Describe("Cookie Tests", func() { } validName := "_oauth2_proxy" - validSecret := "secretthirtytwobytes+abcdefghijk" + validSecret := []byte("secretthirtytwobytes+abcdefghijk") domains := []string{"www.cookies.test"} now := time.Now() @@ -114,15 +114,14 @@ var _ = Describe("Cookie Tests", func() { name: validName, value: "1", opts: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: domains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: validName, + Secret: options.SecretSource{Value: validSecret}, + Domains: domains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, expiration: 15 * time.Minute, now: now, @@ -133,15 +132,14 @@ var _ = Describe("Cookie Tests", func() { name: validName, value: "1", opts: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: domains, - Path: "", - Expire: time.Hour * -1, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: validName, + Secret: options.SecretSource{Value: validSecret}, + Domains: domains, + Path: "", + Expire: time.Hour * -1, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, expiration: time.Hour * -1, now: now, @@ -152,15 +150,14 @@ var _ = Describe("Cookie Tests", func() { name: validName, value: "1", opts: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: domains, - Path: "", - Expire: 0, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: validName, + Secret: options.SecretSource{Value: validSecret}, + Domains: domains, + Path: "", + Expire: 0, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, expiration: 0, now: now, diff --git a/pkg/cookies/csrf_per_request_test.go b/pkg/cookies/csrf_per_request_test.go index 048c5752..bb0fcaee 100644 --- a/pkg/cookies/csrf_per_request_test.go +++ b/pkg/cookies/csrf_per_request_test.go @@ -25,12 +25,12 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { BeforeEach(func() { cookieOpts = &options.Cookie{ Name: cookieName, - Secret: cookieSecret, + Secret: options.SecretSource{Value: cookieSecret}, Domains: []string{cookieDomain}, Path: cookiePath, Expire: time.Hour, - Secure: ptr.To(true), - HTTPOnly: ptr.To(true), + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(false), CSRFPerRequest: ptr.To(true), CSRFExpire: time.Duration(5) * time.Minute, } @@ -118,7 +118,10 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { Value: encoded, } - _, _, valid := encryption.Validate(cookie, cookieOpts.Secret, cookieOpts.Expire) + cookieSecret, err := cookieOpts.GetSecret() + Expect(err).ToNot(HaveOccurred()) + + _, _, valid := encryption.Validate(cookie, cookieSecret, cookieOpts.Expire) Expect(valid).To(BeTrue()) }) }) diff --git a/pkg/cookies/csrf_test.go b/pkg/cookies/csrf_test.go index 7e50d0b1..dfb0bae7 100644 --- a/pkg/cookies/csrf_test.go +++ b/pkg/cookies/csrf_test.go @@ -26,12 +26,12 @@ var _ = Describe("CSRF Cookie Tests", func() { BeforeEach(func() { cookieOpts = &options.Cookie{ Name: cookieName, - Secret: cookieSecret, + Secret: options.SecretSource{Value: cookieSecret}, Domains: []string{cookieDomain}, Path: cookiePath, Expire: time.Hour, - Secure: ptr.To(true), - HTTPOnly: ptr.To(true), + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(false), CSRFPerRequest: ptr.To(false), CSRFExpire: time.Hour, } @@ -119,8 +119,10 @@ var _ = Describe("CSRF Cookie Tests", func() { Name: privateCSRF.cookieName(), Value: encoded, } + cookieSecret, err := cookieOpts.GetSecret() + Expect(err).ToNot(HaveOccurred()) - _, _, valid := encryption.Validate(cookie, cookieOpts.Secret, cookieOpts.Expire) + _, _, valid := encryption.Validate(cookie, cookieSecret, cookieOpts.Expire) Expect(valid).To(BeTrue()) }) }) diff --git a/pkg/middleware/headers_test.go b/pkg/middleware/headers_test.go index c3c402f0..e8bbe665 100644 --- a/pkg/middleware/headers_test.go +++ b/pkg/middleware/headers_test.go @@ -189,7 +189,7 @@ var _ = Describe("Headers Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), + Value: []byte(base64.RawStdEncoding.EncodeToString([]byte("basic-password"))), FromEnv: "SECRET_ENV", }, }, diff --git a/pkg/sessions/cookie/session_store.go b/pkg/sessions/cookie/session_store.go index 095bc0e7..ee23077d 100644 --- a/pkg/sessions/cookie/session_store.go +++ b/pkg/sessions/cookie/session_store.go @@ -13,6 +13,7 @@ import ( pkgcookies "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/cookies" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" ) const ( @@ -164,7 +165,7 @@ func NewCookieSessionStore(opts *options.SessionOptions, cookieOpts *options.Coo return &SessionStore{ CookieCipher: cipher, Cookie: cookieOpts, - Minimal: opts.Cookie.Minimal, + Minimal: ptr.Deref(opts.Cookie.Minimal, options.DefaultCookieStoreMinimal), }, nil } diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go index 4e846e9b..bfacd03b 100644 --- a/pkg/sessions/redis/redis_store.go +++ b/pkg/sessions/redis/redis_store.go @@ -12,6 +12,7 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/persistence" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" "github.com/redis/go-redis/v9" ) @@ -82,13 +83,14 @@ func (store *SessionStore) VerifyConnection(ctx context.Context) error { // NewRedisClient makes a redis.Client (either standalone, sentinel aware, or // redis cluster) func NewRedisClient(opts options.RedisStoreOptions) (Client, error) { - if opts.UseSentinel && opts.UseCluster { + if ptr.Deref(opts.UseSentinel, options.DefaultRedisStoreUseSentinel) && + ptr.Deref(opts.UseCluster, options.DefaultRedisStoreUseCluster) { return nil, fmt.Errorf("options redis-use-sentinel and redis-use-cluster are mutually exclusive") } - if opts.UseSentinel { + if ptr.Deref(opts.UseSentinel, options.DefaultRedisStoreUseSentinel) { return buildSentinelClient(opts) } - if opts.UseCluster { + if ptr.Deref(opts.UseCluster, options.DefaultRedisStoreUseCluster) { return buildClusterClient(opts) } @@ -181,7 +183,7 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) { // setupTLSConfig sets the TLSConfig if the TLS option is given in redis.Options func setupTLSConfig(opts options.RedisStoreOptions, opt *redis.Options) error { - if opts.InsecureSkipTLSVerify { + if ptr.Deref(opts.InsecureSkipTLSVerify, options.DefaultRedisStoreInsecureSkipTLSVerify) { if opt.TLSConfig == nil { /* #nosec */ opt.TLSConfig = &tls.Config{} diff --git a/pkg/sessions/redis/redis_store_test.go b/pkg/sessions/redis/redis_store_test.go index 1bff6855..325237b0 100644 --- a/pkg/sessions/redis/redis_store_test.go +++ b/pkg/sessions/redis/redis_store_test.go @@ -9,6 +9,7 @@ import ( sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/persistence" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/tests" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -80,7 +81,7 @@ var _ = Describe("Redis SessionStore Tests", func() { sentinelAddr := redisProtocol + ms.Addr() opts.Type = options.RedisSessionStoreType opts.Redis.SentinelConnectionURLs = []string{sentinelAddr} - opts.Redis.UseSentinel = true + opts.Redis.UseSentinel = ptr.To(true) opts.Redis.SentinelMasterName = ms.MasterInfo().Name // Capture the session store so that we can close the client @@ -101,7 +102,7 @@ var _ = Describe("Redis SessionStore Tests", func() { clusterAddr := redisProtocol + mr.Addr() opts.Type = options.RedisSessionStoreType opts.Redis.ClusterConnectionURLs = []string{clusterAddr} - opts.Redis.UseCluster = true + opts.Redis.UseCluster = ptr.To(true) // Capture the session store so that we can close the client var err error @@ -156,7 +157,7 @@ var _ = Describe("Redis SessionStore Tests", func() { sentinelAddr := redisProtocol + ms.Addr() opts.Type = options.RedisSessionStoreType opts.Redis.SentinelConnectionURLs = []string{sentinelAddr} - opts.Redis.UseSentinel = true + opts.Redis.UseSentinel = ptr.To(true) opts.Redis.SentinelMasterName = ms.MasterInfo().Name opts.Redis.Password = redisPassword @@ -178,7 +179,7 @@ var _ = Describe("Redis SessionStore Tests", func() { clusterAddr := redisProtocol + mr.Addr() opts.Type = options.RedisSessionStoreType opts.Redis.ClusterConnectionURLs = []string{clusterAddr} - opts.Redis.UseCluster = true + opts.Redis.UseCluster = ptr.To(true) opts.Redis.Password = redisPassword // Capture the session store so that we can close the client @@ -227,7 +228,7 @@ var _ = Describe("Redis SessionStore Tests", func() { clusterAddr := "redis://" + redisUsername + "@" + mr.Addr() opts.Type = options.RedisSessionStoreType opts.Redis.ClusterConnectionURLs = []string{clusterAddr} - opts.Redis.UseCluster = true + opts.Redis.UseCluster = ptr.To(true) opts.Redis.Username = redisUsername opts.Redis.Password = redisPassword diff --git a/pkg/sessions/redis/redis_store_tls_test.go b/pkg/sessions/redis/redis_store_tls_test.go index 54d94b1f..5bb62909 100644 --- a/pkg/sessions/redis/redis_store_tls_test.go +++ b/pkg/sessions/redis/redis_store_tls_test.go @@ -9,6 +9,7 @@ import ( sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/persistence" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/tests" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -69,7 +70,7 @@ var _ = Describe("Redis SessionStore Tests", func() { clusterAddr := redissProtocol + mr.Addr() opts.Type = options.RedisSessionStoreType opts.Redis.ClusterConnectionURLs = []string{clusterAddr} - opts.Redis.UseCluster = true + opts.Redis.UseCluster = ptr.To(true) opts.Redis.CAPath = caPath // Capture the session store so that we can close the client @@ -92,7 +93,7 @@ var _ = Describe("Redis SessionStore Tests", func() { // Set the connection URL opts.Type = options.RedisSessionStoreType opts.Redis.ConnectionURL = redissProtocol + mr.Addr() - opts.Redis.InsecureSkipTLSVerify = true + opts.Redis.InsecureSkipTLSVerify = ptr.To(true) // Capture the session store so that we can close the client ss, err := NewRedisSessionStore(opts, cookieOpts) @@ -111,8 +112,8 @@ var _ = Describe("Redis SessionStore Tests", func() { clusterAddr := redissProtocol + mr.Addr() opts.Type = options.RedisSessionStoreType opts.Redis.ClusterConnectionURLs = []string{clusterAddr} - opts.Redis.UseCluster = true - opts.Redis.InsecureSkipTLSVerify = true + opts.Redis.UseCluster = ptr.To(true) + opts.Redis.InsecureSkipTLSVerify = ptr.To(true) // Capture the session store so that we can close the client var err error @@ -153,7 +154,7 @@ var _ = Describe("Redis SessionStore Tests", func() { // Set the connection URL opts.Type = options.RedisSessionStoreType opts.Redis.ConnectionURL = "redis://127.0.0.1:" + mr.Port() // func (*Miniredis) StartTLS listens on 127.0.0.1 - opts.Redis.InsecureSkipTLSVerify = true + opts.Redis.InsecureSkipTLSVerify = ptr.To(true) // Capture the session store so that we can close the client var err error diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go index 3b811903..2e982352 100644 --- a/pkg/sessions/session_store_test.go +++ b/pkg/sessions/session_store_test.go @@ -30,24 +30,31 @@ var _ = Describe("NewSessionStore", func() { var cookieOpts *options.Cookie BeforeEach(func() { - opts = &options.SessionOptions{} + opts = &options.SessionOptions{ + Refresh: time.Duration(1) * time.Hour, + } // A secret is required to create a Cipher, validation ensures it is the correct // length before a session store is initialised. secret := make([]byte, 32) _, err := rand.Read(secret) Expect(err).ToNot(HaveOccurred()) + var secretValue []byte + + base64.URLEncoding.Encode(secretValue, secret) + Expect(secretValue).ToNot(BeEmpty()) // Set default options in CookieOptions cookieOpts = &options.Cookie{ - Name: "_oauth2_proxy", - Secret: base64.URLEncoding.EncodeToString(secret), - Path: "/", - Expire: time.Duration(168) * time.Hour, - Refresh: time.Duration(1) * time.Hour, - Secure: ptr.To(true), - HTTPOnly: ptr.To(true), - SameSite: "", + Name: "_oauth2_proxy", + Secret: options.SecretSource{ + Value: secretValue, + }, + Path: "/", + Expire: time.Duration(168) * time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(false), + SameSite: "", } }) diff --git a/pkg/sessions/tests/session_store_tests.go b/pkg/sessions/tests/session_store_tests.go index 4ab7315e..80f59bb1 100644 --- a/pkg/sessions/tests/session_store_tests.go +++ b/pkg/sessions/tests/session_store_tests.go @@ -24,6 +24,7 @@ import ( // Interfaces have to be wrapped in closures otherwise nil pointers are thrown. type testInput struct { cookieOpts *options.Cookie + sessionOpts *options.SessionOptions ss sessionStoreFunc session *sessionsapi.SessionState request *http.Request @@ -44,7 +45,6 @@ type NewSessionStoreFunc func(sessionOpts *options.SessionOptions, cookieOpts *o func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward PersistentStoreFastForwardFunc) { Describe("Session Store Suite", func() { - var opts *options.SessionOptions var ss sessionsapi.SessionStore var input testInput var cookieSecret []byte @@ -55,7 +55,9 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi BeforeEach(func() { ss = nil - opts = &options.SessionOptions{} + sessionOpts := &options.SessionOptions{ + Refresh: time.Duration(1) * time.Hour, + } // A secret is required to create a Cipher, validation ensures it is the correct // length before a session store is initialised. @@ -65,14 +67,13 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi // Set default options in CookieOptions cookieOpts := &options.Cookie{ - Name: "_oauth2_proxy", - Path: "/", - Expire: time.Duration(168) * time.Hour, - Refresh: time.Duration(1) * time.Hour, - Secure: ptr.To(true), - HTTPOnly: ptr.To(true), - SameSite: "", - Secret: string(cookieSecret), + Name: "_oauth2_proxy", + Path: "/", + Expire: time.Duration(168) * time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(false), + SameSite: options.SameSiteDefault, + Secret: options.SecretSource{Value: cookieSecret}, } expires := time.Now().Add(1 * time.Hour) @@ -90,6 +91,7 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi input = testInput{ cookieOpts: cookieOpts, + sessionOpts: sessionOpts, ss: getSessionStore, session: session, request: request, @@ -101,7 +103,7 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi Context("with default options", func() { BeforeEach(func() { var err error - ss, err = newSS(opts, input.cookieOpts) + ss, err = newSS(input.sessionOpts, input.cookieOpts) Expect(err).ToNot(HaveOccurred()) }) @@ -113,20 +115,20 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi Context("with non-default options", func() { BeforeEach(func() { + input.sessionOpts.Refresh = time.Duration(2) * time.Hour input.cookieOpts = &options.Cookie{ - Name: "_cookie_name", - Path: "/path", - Expire: time.Duration(72) * time.Hour, - Refresh: time.Duration(2) * time.Hour, - Secure: ptr.To(false), - HTTPOnly: ptr.To(false), - Domains: []string{"example.com"}, - SameSite: "strict", - Secret: string(cookieSecret), + Name: "_cookie_name", + Path: "/path", + Expire: time.Duration(72) * time.Hour, + Insecure: ptr.To(true), + NotHttpOnly: ptr.To(true), + Domains: []string{"example.com"}, + SameSite: options.SameSiteStrict, + Secret: options.SecretSource{Value: cookieSecret}, } var err error - ss, err = newSS(opts, input.cookieOpts) + ss, err = newSS(input.sessionOpts, input.cookieOpts) Expect(err).ToNot(HaveOccurred()) }) @@ -145,18 +147,17 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi tmpfile.Write(secretBytes) tmpfile.Close() + input.sessionOpts.Refresh = time.Duration(1) * time.Hour input.cookieOpts = &options.Cookie{ - Name: "_oauth2_proxy_file", - Path: "/", - Expire: time.Duration(168) * time.Hour, - Refresh: time.Duration(1) * time.Hour, - Secure: ptr.To(true), - HTTPOnly: ptr.To(true), - SameSite: "", - Secret: "", - SecretFile: tmpfile.Name(), + Name: "_oauth2_proxy_file", + Path: "/", + Expire: time.Duration(168) * time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(false), + SameSite: options.SameSiteDefault, + Secret: options.SecretSource{FromFile: tmpfile.Name()}, } - ss, err = newSS(opts, input.cookieOpts) + ss, err = newSS(input.sessionOpts, input.cookieOpts) Expect(err).ToNot(HaveOccurred()) }) @@ -209,13 +210,13 @@ func CheckCookieOptions(in *testInput) { It("have the correct HTTPOnly set", func() { for _, cookie := range cookies { - Expect(cookie.HttpOnly).To(Equal(*in.cookieOpts.HTTPOnly)) + Expect(cookie.HttpOnly).To(Equal(!(*in.cookieOpts.NotHttpOnly))) } }) It("have the correct secure set", func() { for _, cookie := range cookies { - Expect(cookie.Secure).To(Equal(*in.cookieOpts.Secure)) + Expect(cookie.Secure).To(Equal(!(*in.cookieOpts.Insecure))) } }) @@ -298,7 +299,7 @@ func PersistentSessionStoreInterfaceTests(in *testInput) { Context("after the refresh period, but before the cookie expire period", func() { BeforeEach(func() { - Expect(in.persistentFastForward(in.cookieOpts.Refresh + time.Minute)).To(Succeed()) + Expect(in.persistentFastForward(in.sessionOpts.Refresh + time.Minute)).To(Succeed()) }) LoadSessionTests(in) @@ -421,8 +422,13 @@ func SessionStoreInterfaceTests(in *testInput) { BeforeEach(func() { By("Using a valid cookie with a different providers session encoding") broken := "BrokenSessionFromADifferentSessionImplementation" - value, err := encryption.SignedValue(in.cookieOpts.Secret, in.cookieOpts.Name, []byte(broken), time.Now()) + + cookieSecret, err := in.cookieOpts.GetSecret() Expect(err).ToNot(HaveOccurred()) + + value, err := encryption.SignedValue(cookieSecret, in.cookieOpts.Name, []byte(broken), time.Now()) + Expect(err).ToNot(HaveOccurred()) + cookie := cookiesapi.MakeCookieFromOptions(in.request, in.cookieOpts.Name, value, in.cookieOpts, in.cookieOpts.Expire) in.request.AddCookie(cookie) diff --git a/pkg/validation/cookie.go b/pkg/validation/cookie.go index 5f2dd8ac..76d8dd12 100644 --- a/pkg/validation/cookie.go +++ b/pkg/validation/cookie.go @@ -3,7 +3,6 @@ package validation import ( "fmt" "net/http" - "os" "sort" "time" @@ -11,13 +10,13 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" ) -func validateCookie(o options.Cookie) []string { - msgs := validateCookieSecret(o.Secret, o.SecretFile) +func validateCookie(o options.Cookie, refresh time.Duration) []string { + msgs := validateCookieSecret(o.Secret) - if o.Expire != time.Duration(0) && o.Refresh >= o.Expire { + if o.Expire != time.Duration(0) && refresh >= o.Expire { msgs = append(msgs, fmt.Sprintf( "cookie_refresh (%q) must be less than cookie_expire (%q)", - o.Refresh.String(), + refresh.String(), o.Expire.String())) } @@ -50,30 +49,17 @@ func validateCookieName(name string) []string { return msgs } -func validateCookieSecret(secret string, secretFile string) []string { - if secret == "" && secretFile == "" { +func validateCookieSecret(secret options.SecretSource) []string { + if len(secret.Value) == 0 && secret.FromFile == "" { return []string{"missing setting: cookie-secret or cookie-secret-file"} } - if secret == "" && secretFile != "" { - fileData, err := os.ReadFile(secretFile) - if err != nil { - return []string{"could not read cookie secret file: " + secretFile} - } - // Validate the file content as a secret - secretBytes := encryption.SecretBytes(string(fileData)) - switch len(secretBytes) { - case 16, 24, 32: - // Valid secret size found - return []string{} - } - // Invalid secret size found, return a message - return []string{fmt.Sprintf( - "cookie_secret from file must be 16, 24, or 32 bytes to create an AES cipher, but is %d bytes", - len(secretBytes)), - } + + value, err := secret.GetSecretValue() + if err != nil { + return []string{fmt.Sprintf("error retrieving cookie secret: %v", err)} } - secretBytes := encryption.SecretBytes(secret) + secretBytes := encryption.SecretBytes(string(value)) // Check if the secret is a valid length switch len(secretBytes) { case 16, 24, 32: diff --git a/pkg/validation/cookie_test.go b/pkg/validation/cookie_test.go index 95e3203e..b919ff2b 100644 --- a/pkg/validation/cookie_test.go +++ b/pkg/validation/cookie_test.go @@ -18,10 +18,22 @@ func TestValidateCookie(t *testing.T) { invalidName := "_oauth2;proxy" // Separater character not allowed // 10 times the alphabet should be longer than 256 characters longName := strings.Repeat(alphabet, 10) - validSecret := "secretthirtytwobytes+abcdefghijk" - invalidSecret := "abcdef" // 6 bytes is not a valid size - validBase64Secret := "c2VjcmV0dGhpcnR5dHdvYnl0ZXMrYWJjZGVmZ2hpams" // Base64 encoding of "secretthirtytwobytes+abcdefghijk" - invalidBase64Secret := "YWJjZGVmCg" // Base64 encoding of "abcdef" + validSecret := options.SecretSource{ + Value: []byte("secretthirtytwobytes+abcdefghijk"), + } + // 6 bytes is not a valid size + invalidSecret := options.SecretSource{ + Value: []byte("abcdef"), + } + + // Base64 encoding of "secretthirtytwobytes+abcdefghijk" + validBase64Secret := options.SecretSource{ + Value: []byte("c2VjcmV0dGhpcnR5dHdvYnl0ZXMrYWJjZGVmZ2hpams"), + } + // Base64 encoding of "abcdef" + invalidBase64Secret := options.SecretSource{ + Value: []byte("YWJjZGVmCg"), + } emptyDomains := []string{} domains := []string{ "a.localhost", @@ -39,7 +51,7 @@ func TestValidateCookie(t *testing.T) { defer os.Remove(tmpfile.Name()) // Write a valid 32-byte secret to the file - _, err = tmpfile.Write([]byte(validSecret)) + _, err = tmpfile.Write(validSecret.Value) if err != nil { t.Fatalf("Failed to write to temporary file: %v", err) } @@ -56,36 +68,40 @@ func TestValidateCookie(t *testing.T) { testCases := []struct { name string cookie options.Cookie + refresh time.Duration errStrings []string }{ { name: "with valid configuration", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: domains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: validName, + Secret: validSecret, + Domains: domains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, + refresh: 15 * time.Minute, errStrings: []string{}, }, { name: "with no cookie secret", cookie: options.Cookie{ - Name: validName, - Secret: "", - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: validName, + Secret: options.SecretSource{ + Value: nil, + FromFile: "", + }, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, + refresh: 15 * time.Minute, errStrings: []string{ missingSecretMsg, }, @@ -93,16 +109,16 @@ func TestValidateCookie(t *testing.T) { { name: "with an invalid cookie secret", cookie: options.Cookie{ - Name: validName, - Secret: invalidSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: validName, + Secret: invalidSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, + refresh: 15 * time.Minute, errStrings: []string{ invalidSecretMsg, }, @@ -110,31 +126,31 @@ func TestValidateCookie(t *testing.T) { { name: "with a valid Base64 secret", cookie: options.Cookie{ - Name: validName, - Secret: validBase64Secret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: validName, + Secret: validBase64Secret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, + refresh: 15 * time.Minute, errStrings: []string{}, }, { name: "with an invalid Base64 secret", cookie: options.Cookie{ - Name: validName, - Secret: invalidBase64Secret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: validName, + Secret: invalidBase64Secret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, + refresh: 15 * time.Minute, errStrings: []string{ invalidBase64SecretMsg, }, @@ -142,16 +158,16 @@ func TestValidateCookie(t *testing.T) { { name: "with an invalid name", cookie: options.Cookie{ - Name: invalidName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: invalidName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, + refresh: 15 * time.Minute, errStrings: []string{ invalidNameMsg, }, @@ -159,16 +175,16 @@ func TestValidateCookie(t *testing.T) { { name: "with a name that is too long", cookie: options.Cookie{ - Name: longName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: longName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, + refresh: 15 * time.Minute, errStrings: []string{ longNameMsg, }, @@ -176,16 +192,16 @@ func TestValidateCookie(t *testing.T) { { name: "with refresh longer than expire", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: 15 * time.Minute, - Refresh: time.Hour, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: validName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: 15 * time.Minute, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, + refresh: time.Hour, errStrings: []string{ refreshLongerThanExpireMsg, }, @@ -193,61 +209,61 @@ func TestValidateCookie(t *testing.T) { { name: "with samesite \"none\"", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "none", + Name: validName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "none", }, + refresh: 15 * time.Minute, errStrings: []string{}, }, { name: "with samesite \"lax\"", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "none", + Name: validName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "none", }, + refresh: 15 * time.Minute, errStrings: []string{}, }, { name: "with samesite \"strict\"", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "none", + Name: validName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "none", }, + refresh: 15 * time.Minute, errStrings: []string{}, }, { name: "with samesite \"invalid\"", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "invalid", + Name: validName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "invalid", }, + refresh: 15 * time.Minute, errStrings: []string{ invalidSameSiteMsg, }, @@ -255,16 +271,16 @@ func TestValidateCookie(t *testing.T) { { name: "with a combination of configuration errors", cookie: options.Cookie{ - Name: invalidName, - Secret: invalidSecret, - Domains: domains, - Path: "", - Expire: 15 * time.Minute, - Refresh: time.Hour, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "invalid", + Name: invalidName, + Secret: invalidSecret, + Domains: domains, + Path: "", + Expire: 15 * time.Minute, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "invalid", }, + refresh: time.Hour, errStrings: []string{ invalidNameMsg, invalidSecretMsg, @@ -275,55 +291,57 @@ func TestValidateCookie(t *testing.T) { { name: "with session cookie configuration", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: domains, - Path: "", - Expire: 0, - Refresh: 15 * time.Minute, - Secure: ptr.To(true), - HTTPOnly: ptr.To(false), - SameSite: "", + Name: validName, + Secret: validSecret, + Domains: domains, + Path: "", + Expire: 0, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(true), + SameSite: "", }, + refresh: 15 * time.Minute, errStrings: []string{}, }, { name: "with valid secret file", cookie: options.Cookie{ - Name: validName, - Secret: "", - SecretFile: tmpfile.Name(), - Domains: domains, - Path: "", - Expire: 24 * time.Hour, - Refresh: 0, - Secure: ptr.To(true), - HTTPOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: options.SecretSource{ + FromFile: tmpfile.Name(), + }, + Domains: domains, + Path: "", + Expire: 24 * time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(false), + SameSite: "", }, + refresh: 0, errStrings: []string{}, }, { name: "with nonexistent secret file", cookie: options.Cookie{ - Name: validName, - Secret: "", - SecretFile: "/nonexistent/file.txt", - Domains: domains, - Path: "", - Expire: 24 * time.Hour, - Refresh: 0, - Secure: ptr.To(true), - HTTPOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: options.SecretSource{ + FromFile: "/nonexistent/file.txt", + }, + Domains: domains, + Path: "", + Expire: 24 * time.Hour, + Insecure: ptr.To(false), + NotHttpOnly: ptr.To(false), + SameSite: "", }, + refresh: 0, errStrings: []string{"could not read cookie secret file: /nonexistent/file.txt"}, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - errStrings := validateCookie(tc.cookie) + errStrings := validateCookie(tc.cookie, tc.refresh) g := NewWithT(t) g.Expect(errStrings).To(ConsistOf(tc.errStrings)) diff --git a/pkg/validation/header_test.go b/pkg/validation/header_test.go index 2d9ef6dd..e9566752 100644 --- a/pkg/validation/header_test.go +++ b/pkg/validation/header_test.go @@ -30,7 +30,7 @@ var _ = Describe("Headers", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("secret"))), + Value: []byte(base64.RawStdEncoding.EncodeToString([]byte("secret"))), }, }, }, @@ -43,7 +43,7 @@ var _ = Describe("Headers", func() { ClaimSource: &options.ClaimSource{ Claim: "email", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("secret"))), + Value: []byte(base64.RawStdEncoding.EncodeToString([]byte("secret"))), }, }, }, diff --git a/pkg/validation/options.go b/pkg/validation/options.go index ec0e05f0..fac2d532 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -21,7 +21,7 @@ import ( // Validate checks that required options are set and validates those that they // are of the correct format func Validate(o *options.Options) error { - msgs := validateCookie(o.Cookie) + msgs := validateCookie(o.Cookie, o.Session.Refresh) msgs = append(msgs, validateSessionCookieMinimal(o)...) msgs = append(msgs, validateRedisSessionStore(o)...) msgs = append(msgs, prefixValues("injectRequestHeaders: ", validateHeaders(o.InjectRequestHeaders)...)...) @@ -74,7 +74,7 @@ func Validate(o *options.Options) error { var redirectURL *url.URL redirectURL, msgs = parseURL(o.RawRedirectURL, "redirect", msgs) o.SetRedirectURL(redirectURL) - if o.RawRedirectURL == "" && !ptr.Deref(o.Cookie.Secure, options.DefaultCookieSecure) && !o.ReverseProxy { + if o.RawRedirectURL == "" && ptr.Deref(o.Cookie.Insecure, options.DefaultCookieInsecure) && !o.ReverseProxy { logger.Print("WARNING: no explicit redirect URL: redirects will default to insecure HTTP") } diff --git a/pkg/validation/options_test.go b/pkg/validation/options_test.go index cb13ee82..e0f7bbb0 100644 --- a/pkg/validation/options_test.go +++ b/pkg/validation/options_test.go @@ -14,12 +14,15 @@ import ( ) const ( - cookieSecret = "secretthirtytwobytes+abcdefghijk" clientID = "bazquux" clientSecret = "xyzzyplugh" providerID = "providerID" ) +var ( + cookieSecret = options.NewSecretSourceFromString("secretthirtytwobytes+abcdefghijk") +) + func testOptions() *options.Options { o := options.NewOptions() o.UpstreamServers.Upstreams = append(o.UpstreamServers.Upstreams, options.Upstream{ @@ -125,11 +128,11 @@ func TestCookieRefreshMustBeLessThanCookieExpire(t *testing.T) { o := testOptions() assert.Equal(t, nil, Validate(o)) - o.Cookie.Secret = "0123456789abcdef" - o.Cookie.Refresh = o.Cookie.Expire + o.Cookie.Secret = options.NewSecretSourceFromString("0123456789abcdef") + o.Session.Refresh = o.Cookie.Expire assert.NotEqual(t, nil, Validate(o)) - o.Cookie.Refresh -= time.Duration(1) + o.Session.Refresh -= time.Duration(1) assert.Equal(t, nil, Validate(o)) } @@ -138,23 +141,23 @@ func TestBase64CookieSecret(t *testing.T) { assert.Equal(t, nil, Validate(o)) // 32 byte, base64 (urlsafe) encoded key - o.Cookie.Secret = "yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ=" + o.Cookie.Secret = options.NewSecretSourceFromString("yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ=") assert.Equal(t, nil, Validate(o)) // 32 byte, base64 (urlsafe) encoded key, w/o padding - o.Cookie.Secret = "yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ" + o.Cookie.Secret = options.NewSecretSourceFromString("yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ") assert.Equal(t, nil, Validate(o)) // 24 byte, base64 (urlsafe) encoded key - o.Cookie.Secret = "Kp33Gj-GQmYtz4zZUyUDdqQKx5_Hgkv3" + o.Cookie.Secret = options.NewSecretSourceFromString("Kp33Gj-GQmYtz4zZUyUDdqQKx5_Hgkv3") assert.Equal(t, nil, Validate(o)) // 16 byte, base64 (urlsafe) encoded key - o.Cookie.Secret = "LFEqZYvYUwKwzn0tEuTpLA==" + o.Cookie.Secret = options.NewSecretSourceFromString("LFEqZYvYUwKwzn0tEuTpLA==") assert.Equal(t, nil, Validate(o)) // 16 byte, base64 (urlsafe) encoded key, w/o padding - o.Cookie.Secret = "LFEqZYvYUwKwzn0tEuTpLA" + o.Cookie.Secret = options.NewSecretSourceFromString("LFEqZYvYUwKwzn0tEuTpLA") assert.Equal(t, nil, Validate(o)) } diff --git a/pkg/validation/sessions.go b/pkg/validation/sessions.go index 96ea6d4f..4a1c825d 100644 --- a/pkg/validation/sessions.go +++ b/pkg/validation/sessions.go @@ -9,10 +9,11 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/redis" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" ) func validateSessionCookieMinimal(o *options.Options) []string { - if !o.Session.Cookie.Minimal { + if !ptr.Deref(o.Session.Cookie.Minimal, options.DefaultCookieStoreMinimal) { return []string{} } @@ -32,7 +33,7 @@ func validateSessionCookieMinimal(o *options.Options) []string { } } - if o.Cookie.Refresh != time.Duration(0) { + if o.Session.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 cb54c571..829df089 100644 --- a/pkg/validation/sessions_test.go +++ b/pkg/validation/sessions_test.go @@ -6,6 +6,7 @@ import ( "github.com/Bose/minisentinel" "github.com/alicebob/miniredis/v2" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -30,7 +31,7 @@ var _ = Describe("Sessions", func() { opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ - Minimal: false, + Minimal: ptr.To(false), }, }, }, @@ -40,7 +41,7 @@ var _ = Describe("Sessions", func() { opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ - Minimal: false, + Minimal: ptr.To(false), }, }, InjectRequestHeaders: []options.Header{ @@ -62,7 +63,7 @@ var _ = Describe("Sessions", func() { opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ - Minimal: true, + Minimal: ptr.To(true), }, }, }, @@ -72,7 +73,7 @@ var _ = Describe("Sessions", func() { opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ - Minimal: true, + Minimal: ptr.To(true), }, }, InjectRequestHeaders: []options.Header{ @@ -94,7 +95,7 @@ var _ = Describe("Sessions", func() { opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ - Minimal: true, + Minimal: ptr.To(true), }, }, InjectResponseHeaders: []options.Header{ @@ -116,7 +117,7 @@ var _ = Describe("Sessions", func() { opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ - Minimal: true, + Minimal: ptr.To(true), }, }, InjectRequestHeaders: []options.Header{ @@ -136,12 +137,11 @@ var _ = Describe("Sessions", func() { }), Entry("CookieRefresh conflict", &cookieMinimalTableInput{ opts: &options.Options{ - Cookie: options.Cookie{ - Refresh: time.Hour, - }, + Cookie: options.Cookie{}, Session: options.SessionOptions{ + Refresh: time.Hour, Cookie: options.CookieStoreOptions{ - Minimal: true, + Minimal: ptr.To(true), }, }, }, @@ -151,7 +151,7 @@ var _ = Describe("Sessions", func() { opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ - Minimal: true, + Minimal: ptr.To(true), }, }, InjectResponseHeaders: []options.Header{ @@ -323,7 +323,7 @@ var _ = Describe("Sessions", func() { Session: options.SessionOptions{ Type: options.RedisSessionStoreType, Redis: options.RedisStoreOptions{ - UseSentinel: true, + UseSentinel: ptr.To(true), }, }, }, @@ -340,7 +340,7 @@ var _ = Describe("Sessions", func() { Type: options.RedisSessionStoreType, Redis: options.RedisStoreOptions{ Password: "abcdef123", - UseSentinel: true, + UseSentinel: ptr.To(true), }, }, }, @@ -357,7 +357,7 @@ var _ = Describe("Sessions", func() { Type: options.RedisSessionStoreType, Redis: options.RedisStoreOptions{ Password: "zyxwtuv987", - UseSentinel: true, + UseSentinel: ptr.To(true), }, }, }, @@ -371,7 +371,7 @@ var _ = Describe("Sessions", func() { Session: options.SessionOptions{ Type: options.RedisSessionStoreType, Redis: options.RedisStoreOptions{ - UseSentinel: true, + UseSentinel: ptr.To(true), SentinelMasterName: "WRONG", }, }, @@ -386,7 +386,7 @@ var _ = Describe("Sessions", func() { Session: options.SessionOptions{ Type: options.RedisSessionStoreType, Redis: options.RedisStoreOptions{ - UseSentinel: true, + UseSentinel: ptr.To(true), SentinelConnectionURLs: []string{"redis://127.0.0.1:65535"}, }, }, @@ -398,8 +398,8 @@ var _ = Describe("Sessions", func() { Session: options.SessionOptions{ Type: options.RedisSessionStoreType, Redis: options.RedisStoreOptions{ - UseSentinel: true, - UseCluster: true, + UseSentinel: ptr.To(true), + UseCluster: ptr.To(true), }, }, }, From 82a74a541a5ccb07823b284abebada7e693a5b4b Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Sun, 28 Dec 2025 10:07:48 +0100 Subject: [PATCH 3/7] feat(config): convert cookie property (Not)HTTPOnly boolean to enum Signed-off-by: Jan Larwig --- docs/docs/configuration/alpha_config.md | 82 +++++++- oauthproxy.go | 2 +- pkg/apis/options/alpha_options.go | 12 +- pkg/apis/options/cookie.go | 39 +++- pkg/apis/options/legacy_cookie.go | 13 +- pkg/apis/options/legacy_options_test.go | 2 +- pkg/cookies/cookies.go | 7 +- pkg/cookies/cookies_test.go | 48 ++--- pkg/cookies/csrf_per_request_test.go | 2 +- pkg/cookies/csrf_test.go | 2 +- pkg/sessions/session_store_test.go | 10 +- pkg/sessions/tests/session_store_tests.go | 53 +++-- pkg/validation/cookie_test.go | 244 +++++++++++----------- 13 files changed, 312 insertions(+), 204 deletions(-) diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 6f23f887..5d3e57c7 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -184,8 +184,9 @@ They may change between releases without notice. | `injectResponseHeaders` | _[[]Header](#header)_ | InjectResponseHeaders is used to configure headers that should be added
to responses from the proxy.
This is typically used when using the proxy as an external authentication
provider in conjunction with another proxy such as NGINX and its
auth_request module.
Headers may source values from either the authenticated user's session
or from a static secret value. | | `server` | _[Server](#server)_ | Server is used to configure the HTTP(S) server for the proxy application.
You may choose to run both HTTP and HTTPS servers simultaneously.
This can be done by setting the BindAddress and the SecureBindAddress simultaneously.
To use the secure server you must configure a TLS certificate and key. | | `metricsServer` | _[Server](#server)_ | MetricsServer is used to configure the HTTP(S) server for metrics.
You may choose to run both HTTP and HTTPS servers simultaneously.
This can be done by setting the BindAddress and the SecureBindAddress simultaneously.
To use the secure server you must configure a TLS certificate and key. | -| `providers` | _[Providers](#providers)_ | Providers is used to configure your provider. **Multiple-providers is not
yet working.** [This feature is tracked in
#925](https://github.com/oauth2-proxy/oauth2-proxy/issues/926) | +| `providers` | _[Providers](#providers)_ | Providers is used to configure your provider.
**Multiple-providers is not yet working.**
[This feature is tracked in #925](https://github.com/oauth2-proxy/oauth2-proxy/issues/926) | | `cookie` | _[Cookie](#cookie)_ | Cookie is used to configure the cookies used by OAuth2 Proxy.
This includes session and CSRF cookies. | +| `session` | _[SessionOptions](#sessionoptions)_ | Session is used to configure session options used by OAuth2 Proxy.
This includes session storage options. | ### AzureOptions @@ -230,19 +231,27 @@ Cookie contains configuration options relating session and CSRF cookies | Field | Type | Description | | ----- | ---- | ----------- | | `name` | _string_ | Name is the name of the cookie | -| `secret` | _string_ | Secret is the secret used to encrypt/sign the cookie value | -| `secretFile` | _string_ | SecretFile is a file containing the secret used to encrypt/sign the cookie value
instead of specifying it directly in the config. Secret takes precedence over SecretFile | +| `secret` | _[SecretSource](#secretsource)_ | Secret is the secret source used to encrypt/sign the cookie value | | `domains` | _[]string_ | Domains is a list of domains for which the cookie is valid | | `path` | _string_ | Path is the path for which the cookie is valid | | `expire` | _duration_ | Expire is the duration before the cookie expires | -| `refresh` | _duration_ | Refresh is the duration after which the cookie is refreshable | -| `secure` | _bool_ | Secure indicates whether the cookie is only sent over HTTPS | -| `httpOnly` | _bool_ | HTTPOnly indicates whether the cookie is inaccessible to JavaScript | -| `sameSite` | _string_ | SameSite sets the SameSite attribute on the cookie | -| `csrfPerRequest` | _bool_ | CSRFPerRequest indicates whether a unique CSRF token is generated for each request
Enables parallel requests from clients (e.g., multiple tabs) | +| `insecure` | _bool_ | Insecure indicates whether the cookie allows to be sent over HTTP
Default is false, which requires HTTPS | +| `scriptAccess` | _[ScriptAccess](#scriptaccess)_ | ScriptAccess is a wrapper enum for HTTPOnly; indicates whether the
cookie is accessible to JavaScript. Default is deny which translates
to true for HTTPOnly, which helps mitigate certain XSS attacks | +| `sameSite` | _[SameSiteMode](#samesitemode)_ | SameSite sets the SameSite attribute on the cookie | +| `csrfPerRequest` | _bool_ | CSRFPerRequest indicates whether a unique CSRF token is generated for each request
Enables parallel requests from clients (e.g., multiple tabs)
Default is false, which uses a single CSRF token per session | | `csrfPerRequestLimit` | _int_ | CSRFPerRequestLimit sets a limit on the number of valid CSRF tokens when CSRFPerRequest is enabled
Used to prevent unbounded memory growth from storing too many tokens | | `csrfExpire` | _duration_ | CSRFExpire sets the duration before a CSRF token expires | +### CookieStoreOptions + +(**Appears on:** [SessionOptions](#sessionoptions)) + +CookieStoreOptions contains configuration options for the CookieSessionStore. + +| Field | Type | Description | +| ----- | ---- | ----------- | +| `minimal` | _bool_ | Minimal indicates whether to use minimal cookies for session storage
Default is false | + ### GitHubOptions (**Appears on:** [Provider](#provider)) @@ -510,9 +519,44 @@ AlphaConfig](https://oauth2-proxy.github.io/oauth2-proxy/configuration/alpha-con However, [**the feature to implement multiple providers is not complete**](https://github.com/oauth2-proxy/oauth2-proxy/issues/926). +### RedisStoreOptions + +(**Appears on:** [SessionOptions](#sessionoptions)) + +RedisStoreOptions contains configuration options for the RedisSessionStore. + +| Field | Type | Description | +| ----- | ---- | ----------- | +| `connectionURL` | _string_ | ConnectionURL is the Redis connection URL | +| `username` | _string_ | Username is the Redis username | +| `password` | _string_ | Password is the Redis password | +| `useSentinel` | _bool_ | UseSentinel indicates whether to use Redis Sentinel
Default is false | +| `sentinelPassword` | _string_ | SentinelPassword is the Redis Sentinel password | +| `sentinelMasterName` | _string_ | SentinelMasterName is the Redis Sentinel master name | +| `sentinelConnectionURLs` | _[]string_ | SentinelConnectionURLs is a list of Redis Sentinel connection URLs | +| `useCluster` | _bool_ | UseCluster indicates whether to use Redis Cluster
Default is false | +| `clusterConnectionURLs` | _[]string_ | ClusterConnectionURLs is a list of Redis Cluster connection URLs | +| `caPath` | _string_ | CAPath is the path to the CA certificate for Redis TLS connections | +| `insecureSkipTLSVerify` | _bool_ | InsecureSkipTLSVerify indicates whether to skip TLS verification for Redis connections | +| `idleTimeout` | _int_ | IdleTimeout is the Redis connection idle timeout in seconds | + +### SameSiteMode +#### (`string` alias) + +(**Appears on:** [Cookie](#cookie)) + + + +### ScriptAccess +#### (`string` alias) + +(**Appears on:** [Cookie](#cookie)) + + + ### SecretSource -(**Appears on:** [ClaimSource](#claimsource), [HeaderValue](#headervalue), [TLS](#tls)) +(**Appears on:** [ClaimSource](#claimsource), [Cookie](#cookie), [HeaderValue](#headervalue), [TLS](#tls)) SecretSource references an individual secret value. Only one source within the struct should be defined at any time. @@ -535,6 +579,26 @@ Server represents the configuration for an HTTP(S) server | `secureBindAddress` | _string_ | SecureBindAddress is the address on which to serve secure traffic.
Leave blank or set to "-" to disable. | | `tls` | _[TLS](#tls)_ | TLS contains the information for loading the certificate and key for the
secure traffic and further configuration for the TLS server. | +### SessionOptions + +(**Appears on:** [AlphaOptions](#alphaoptions)) + +SessionOptions contains configuration options for the SessionStore providers. + +| Field | Type | Description | +| ----- | ---- | ----------- | +| `type` | _[SessionStoreType](#sessionstoretype)_ | Type is the type of session store to use
Options are "cookie" or "redis"
Default is "cookie" | +| `refresh` | _duration_ | Refresh is the duration after which the session is refreshable | +| `cookie` | _[CookieStoreOptions](#cookiestoreoptions)_ | Cookie is the configuration options for the CookieSessionStore | +| `redis` | _[RedisStoreOptions](#redisstoreoptions)_ | Redis is the configuration options for the RedisSessionStore | + +### SessionStoreType +#### (`string` alias) + +(**Appears on:** [SessionOptions](#sessionoptions)) + + + ### TLS (**Appears on:** [Server](#server)) diff --git a/oauthproxy.go b/oauthproxy.go index 86ebfebc..c22ac5b2 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -180,7 +180,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr refresh = fmt.Sprintf("after %s", opts.Session.Refresh) } - logger.Printf("Cookie settings: name:%s insecure(http):%v nothttponly:%v expiry:%s domains:%s path:%s samesite:%s refresh:%s", opts.Cookie.Name, opts.Cookie.Insecure, opts.Cookie.NotHttpOnly, opts.Cookie.Expire, strings.Join(opts.Cookie.Domains, ","), opts.Cookie.Path, opts.Cookie.SameSite, refresh) + logger.Printf("Cookie settings: name:%s insecure(http):%v scriptaccess:%v expiry:%s domains:%s path:%s samesite:%s refresh:%s", opts.Cookie.Name, opts.Cookie.Insecure, opts.Cookie.ScriptAccess, opts.Cookie.Expire, strings.Join(opts.Cookie.Domains, ","), opts.Cookie.Path, opts.Cookie.SameSite, refresh) trustedIPs := ip.NewNetSet() for _, ipStr := range opts.TrustedIPs { diff --git a/pkg/apis/options/alpha_options.go b/pkg/apis/options/alpha_options.go index 977bdb18..c75347a9 100644 --- a/pkg/apis/options/alpha_options.go +++ b/pkg/apis/options/alpha_options.go @@ -41,14 +41,18 @@ type AlphaOptions struct { // To use the secure server you must configure a TLS certificate and key. MetricsServer Server `yaml:"metricsServer,omitempty"` - // Providers is used to configure your provider. **Multiple-providers is not - // yet working.** [This feature is tracked in - // #925](https://github.com/oauth2-proxy/oauth2-proxy/issues/926) + // Providers is used to configure your provider. + // **Multiple-providers is not yet working.** + // [This feature is tracked in #925](https://github.com/oauth2-proxy/oauth2-proxy/issues/926) Providers Providers `yaml:"providers,omitempty"` // Cookie is used to configure the cookies used by OAuth2 Proxy. // This includes session and CSRF cookies. Cookie Cookie `yaml:"cookie,omitempty"` + + // Session is used to configure session options used by OAuth2 Proxy. + // This includes session storage options. + Session SessionOptions `yaml:"session,omitempty"` } // Initialize alpha options with default values and settings of the core options @@ -68,6 +72,7 @@ func (a *AlphaOptions) ExtractFrom(opts *Options) { a.MetricsServer = opts.MetricsServer a.Providers = opts.Providers a.Cookie = opts.Cookie + a.Session = opts.Session } // MergeOptionsWithDefaults replaces alpha options in the Options struct @@ -80,4 +85,5 @@ func (a *AlphaOptions) MergeOptionsWithDefaults(opts *Options) { opts.MetricsServer = a.MetricsServer opts.Providers = a.Providers opts.Cookie = a.Cookie + opts.Session = a.Session } diff --git a/pkg/apis/options/cookie.go b/pkg/apis/options/cookie.go index 6269bfbf..4b71544a 100644 --- a/pkg/apis/options/cookie.go +++ b/pkg/apis/options/cookie.go @@ -11,8 +11,6 @@ import ( const ( // DefaultCookieInsecure is the default value for Cookie.Insecure DefaultCookieInsecure bool = false - // DefaultCookieNotHttpOnly is the default value for Cookie.NotHttpOnly - DefaultCookieNotHttpOnly bool = false // DefaultCSRFPerRequest is the default value for Cookie.CSRFPerRequest DefaultCSRFPerRequest bool = false ) @@ -26,6 +24,14 @@ const ( SameSiteDefault SameSiteMode = "" ) +type ScriptAccess string + +const ( + ScriptAccessDenied ScriptAccess = "deny" + ScriptAccessAllowed ScriptAccess = "allow" + ScriptAccessNone ScriptAccess = "" +) + // Cookie contains configuration options relating session and CSRF cookies type Cookie struct { // Name is the name of the cookie @@ -41,9 +47,10 @@ type Cookie struct { // Insecure indicates whether the cookie allows to be sent over HTTP // Default is false, which requires HTTPS Insecure *bool `yaml:"insecure,omitempty"` - // NotHttpOnly is the inverse of HTTPOnly; indicates whether the cookie is accessible to JavaScript - // Default is false, which helps mitigate certain XSS attacks - NotHttpOnly *bool `yaml:"notHttpOnly,omitempty"` + // ScriptAccess is a wrapper enum for HTTPOnly; indicates whether the + // cookie is accessible to JavaScript. Default is deny which translates + // to true for HTTPOnly, which helps mitigate certain XSS attacks + ScriptAccess ScriptAccess `yaml:"scriptAccess,omitempty"` // SameSite sets the SameSite attribute on the cookie SameSite SameSiteMode `yaml:"sameSite,omitempty"` // CSRFPerRequest indicates whether a unique CSRF token is generated for each request @@ -57,6 +64,8 @@ type Cookie struct { CSRFExpire time.Duration `yaml:"csrfExpire,omitempty"` } +// UnmarshalYAML unmarshalles the strings provided for the +// SameSite property to the enum type SameSiteMode func (m *SameSiteMode) UnmarshalYAML(value *yaml.Node) error { var s string if err := value.Decode(&s); err != nil { @@ -71,6 +80,22 @@ func (m *SameSiteMode) UnmarshalYAML(value *yaml.Node) error { } } +// UnmarshalYAML unmarshalles the strings provided for the +// ScriptAccess property to the enum type ScriptAccess +func (sa *ScriptAccess) UnmarshalYAML(value *yaml.Node) error { + var s string + if err := value.Decode(&s); err != nil { + return err + } + switch ScriptAccess(s) { + case ScriptAccessAllowed, ScriptAccessDenied, ScriptAccessNone: + *sa = ScriptAccess(s) + return nil + default: + return fmt.Errorf("invalid script access: %s", s) + } +} + // GetSecret returns the cookie secret as a string from the SecretSource func (c *Cookie) GetSecret() (string, error) { secret, err := c.Secret.GetSecretValue() @@ -95,8 +120,8 @@ func (c *Cookie) EnsureDefaults() { if c.Insecure == nil { c.Insecure = ptr.To(DefaultCookieInsecure) } - if c.NotHttpOnly == nil { - c.NotHttpOnly = ptr.To(DefaultCookieNotHttpOnly) + if c.ScriptAccess == ScriptAccessNone { + c.ScriptAccess = ScriptAccessDenied } if c.CSRFPerRequest == nil { c.CSRFPerRequest = ptr.To(DefaultCSRFPerRequest) diff --git a/pkg/apis/options/legacy_cookie.go b/pkg/apis/options/legacy_cookie.go index be1235ca..8240e7c8 100644 --- a/pkg/apis/options/legacy_cookie.go +++ b/pkg/apis/options/legacy_cookie.go @@ -1,10 +1,8 @@ package options import ( - "encoding/base64" "time" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/spf13/pflag" ) @@ -46,10 +44,13 @@ func legacyCookieFlagSet() *pflag.FlagSet { } func (l *LegacyCookie) convert() Cookie { - // Invert Secure and HTTPOnly to match the new Cookie struct - // which uses Insecure and NotHttpOnly + // Invert Secure and use ScriptAccess property instead of + // HTTPOnly to match new Cookie struct insecure := !l.Secure - notHTTPOnly := !l.HTTPOnly + scriptAccess := ScriptAccessDenied + if !l.HTTPOnly { + scriptAccess = ScriptAccessAllowed + } var secret *SecretSource if l.Secret != "" { @@ -67,7 +68,7 @@ func (l *LegacyCookie) convert() Cookie { Path: l.Path, Expire: l.Expire, Insecure: &insecure, - NotHttpOnly: ¬HTTPOnly, + ScriptAccess: scriptAccess, SameSite: SameSiteMode(l.SameSite), CSRFPerRequest: &l.CSRFPerRequest, CSRFPerRequestLimit: l.CSRFPerRequestLimit, diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index b6be7a73..d055be25 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -1099,7 +1099,7 @@ var _ = Describe("Legacy Options", func() { Path: "/", Expire: time.Duration(168) * time.Hour, Insecure: ptr.To(false), - NotHttpOnly: ptr.To(false), + ScriptAccess: ScriptAccessDenied, SameSite: "", CSRFPerRequest: ptr.To(false), CSRFPerRequestLimit: 0, diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index 252daeb6..913d9b9b 100644 --- a/pkg/cookies/cookies.go +++ b/pkg/cookies/cookies.go @@ -26,12 +26,17 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, opts *o domain = opts.Domains[len(opts.Domains)-1] } + httpOnly := true + if opts.ScriptAccess == options.ScriptAccessAllowed { + httpOnly = false + } + c := &http.Cookie{ Name: name, Value: value, Path: opts.Path, Domain: domain, - HttpOnly: !ptr.Deref(opts.NotHttpOnly, options.DefaultCookieNotHttpOnly), + HttpOnly: httpOnly, Secure: !ptr.Deref(opts.Insecure, options.DefaultCookieInsecure), SameSite: ParseSameSite(opts.SameSite), } diff --git a/pkg/cookies/cookies_test.go b/pkg/cookies/cookies_test.go index 13131c4b..dece3e95 100644 --- a/pkg/cookies/cookies_test.go +++ b/pkg/cookies/cookies_test.go @@ -114,14 +114,14 @@ var _ = Describe("Cookie Tests", func() { name: validName, value: "1", opts: options.Cookie{ - Name: validName, - Secret: options.SecretSource{Value: validSecret}, - Domains: domains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: options.SecretSource{Value: validSecret}, + Domains: domains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, expiration: 15 * time.Minute, now: now, @@ -132,14 +132,14 @@ var _ = Describe("Cookie Tests", func() { name: validName, value: "1", opts: options.Cookie{ - Name: validName, - Secret: options.SecretSource{Value: validSecret}, - Domains: domains, - Path: "", - Expire: time.Hour * -1, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: options.SecretSource{Value: validSecret}, + Domains: domains, + Path: "", + Expire: time.Hour * -1, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, expiration: time.Hour * -1, now: now, @@ -150,14 +150,14 @@ var _ = Describe("Cookie Tests", func() { name: validName, value: "1", opts: options.Cookie{ - Name: validName, - Secret: options.SecretSource{Value: validSecret}, - Domains: domains, - Path: "", - Expire: 0, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: options.SecretSource{Value: validSecret}, + Domains: domains, + Path: "", + Expire: 0, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, expiration: 0, now: now, diff --git a/pkg/cookies/csrf_per_request_test.go b/pkg/cookies/csrf_per_request_test.go index bb0fcaee..0f9e2963 100644 --- a/pkg/cookies/csrf_per_request_test.go +++ b/pkg/cookies/csrf_per_request_test.go @@ -30,7 +30,7 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { Path: cookiePath, Expire: time.Hour, Insecure: ptr.To(false), - NotHttpOnly: ptr.To(false), + ScriptAccess: options.ScriptAccessDenied, CSRFPerRequest: ptr.To(true), CSRFExpire: time.Duration(5) * time.Minute, } diff --git a/pkg/cookies/csrf_test.go b/pkg/cookies/csrf_test.go index dfb0bae7..c89f7d12 100644 --- a/pkg/cookies/csrf_test.go +++ b/pkg/cookies/csrf_test.go @@ -31,7 +31,7 @@ var _ = Describe("CSRF Cookie Tests", func() { Path: cookiePath, Expire: time.Hour, Insecure: ptr.To(false), - NotHttpOnly: ptr.To(false), + ScriptAccess: options.ScriptAccessDenied, CSRFPerRequest: ptr.To(false), CSRFExpire: time.Hour, } diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go index 2e982352..d1fd5881 100644 --- a/pkg/sessions/session_store_test.go +++ b/pkg/sessions/session_store_test.go @@ -50,11 +50,11 @@ var _ = Describe("NewSessionStore", func() { Secret: options.SecretSource{ Value: secretValue, }, - Path: "/", - Expire: time.Duration(168) * time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(false), - SameSite: "", + Path: "/", + Expire: time.Duration(168) * time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessDenied, + SameSite: "", } }) diff --git a/pkg/sessions/tests/session_store_tests.go b/pkg/sessions/tests/session_store_tests.go index 80f59bb1..d78fa679 100644 --- a/pkg/sessions/tests/session_store_tests.go +++ b/pkg/sessions/tests/session_store_tests.go @@ -67,13 +67,13 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi // Set default options in CookieOptions cookieOpts := &options.Cookie{ - Name: "_oauth2_proxy", - Path: "/", - Expire: time.Duration(168) * time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(false), - SameSite: options.SameSiteDefault, - Secret: options.SecretSource{Value: cookieSecret}, + Name: "_oauth2_proxy", + Path: "/", + Expire: time.Duration(168) * time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessDenied, + SameSite: options.SameSiteDefault, + Secret: options.SecretSource{Value: cookieSecret}, } expires := time.Now().Add(1 * time.Hour) @@ -117,14 +117,14 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi BeforeEach(func() { input.sessionOpts.Refresh = time.Duration(2) * time.Hour input.cookieOpts = &options.Cookie{ - Name: "_cookie_name", - Path: "/path", - Expire: time.Duration(72) * time.Hour, - Insecure: ptr.To(true), - NotHttpOnly: ptr.To(true), - Domains: []string{"example.com"}, - SameSite: options.SameSiteStrict, - Secret: options.SecretSource{Value: cookieSecret}, + Name: "_cookie_name", + Path: "/path", + Expire: time.Duration(72) * time.Hour, + Insecure: ptr.To(true), + ScriptAccess: options.ScriptAccessAllowed, + Domains: []string{"example.com"}, + SameSite: options.SameSiteStrict, + Secret: options.SecretSource{Value: cookieSecret}, } var err error @@ -149,13 +149,13 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi input.sessionOpts.Refresh = time.Duration(1) * time.Hour input.cookieOpts = &options.Cookie{ - Name: "_oauth2_proxy_file", - Path: "/", - Expire: time.Duration(168) * time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(false), - SameSite: options.SameSiteDefault, - Secret: options.SecretSource{FromFile: tmpfile.Name()}, + Name: "_oauth2_proxy_file", + Path: "/", + Expire: time.Duration(168) * time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessDenied, + SameSite: options.SameSiteDefault, + Secret: options.SecretSource{FromFile: tmpfile.Name()}, } ss, err = newSS(input.sessionOpts, input.cookieOpts) Expect(err).ToNot(HaveOccurred()) @@ -210,7 +210,14 @@ func CheckCookieOptions(in *testInput) { It("have the correct HTTPOnly set", func() { for _, cookie := range cookies { - Expect(cookie.HttpOnly).To(Equal(!(*in.cookieOpts.NotHttpOnly))) + var httpOnly bool + if in.cookieOpts.ScriptAccess == options.ScriptAccessAllowed { + httpOnly = false + } + if in.cookieOpts.ScriptAccess == options.ScriptAccessDenied { + httpOnly = true + } + Expect(cookie.HttpOnly).To(Equal(httpOnly)) } }) diff --git a/pkg/validation/cookie_test.go b/pkg/validation/cookie_test.go index b919ff2b..cd9a6bdf 100644 --- a/pkg/validation/cookie_test.go +++ b/pkg/validation/cookie_test.go @@ -74,14 +74,14 @@ func TestValidateCookie(t *testing.T) { { name: "with valid configuration", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: domains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: validSecret, + Domains: domains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, refresh: 15 * time.Minute, errStrings: []string{}, @@ -94,12 +94,12 @@ func TestValidateCookie(t *testing.T) { Value: nil, FromFile: "", }, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, refresh: 15 * time.Minute, errStrings: []string{ @@ -109,14 +109,14 @@ func TestValidateCookie(t *testing.T) { { name: "with an invalid cookie secret", cookie: options.Cookie{ - Name: validName, - Secret: invalidSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: invalidSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, refresh: 15 * time.Minute, errStrings: []string{ @@ -126,14 +126,14 @@ func TestValidateCookie(t *testing.T) { { name: "with a valid Base64 secret", cookie: options.Cookie{ - Name: validName, - Secret: validBase64Secret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: validBase64Secret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, refresh: 15 * time.Minute, errStrings: []string{}, @@ -141,14 +141,14 @@ func TestValidateCookie(t *testing.T) { { name: "with an invalid Base64 secret", cookie: options.Cookie{ - Name: validName, - Secret: invalidBase64Secret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: invalidBase64Secret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, refresh: 15 * time.Minute, errStrings: []string{ @@ -158,14 +158,14 @@ func TestValidateCookie(t *testing.T) { { name: "with an invalid name", cookie: options.Cookie{ - Name: invalidName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: invalidName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, refresh: 15 * time.Minute, errStrings: []string{ @@ -175,14 +175,14 @@ func TestValidateCookie(t *testing.T) { { name: "with a name that is too long", cookie: options.Cookie{ - Name: longName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: longName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, refresh: 15 * time.Minute, errStrings: []string{ @@ -192,14 +192,14 @@ func TestValidateCookie(t *testing.T) { { name: "with refresh longer than expire", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: 15 * time.Minute, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: 15 * time.Minute, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, refresh: time.Hour, errStrings: []string{ @@ -209,14 +209,14 @@ func TestValidateCookie(t *testing.T) { { name: "with samesite \"none\"", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "none", + Name: validName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "none", }, refresh: 15 * time.Minute, errStrings: []string{}, @@ -224,14 +224,14 @@ func TestValidateCookie(t *testing.T) { { name: "with samesite \"lax\"", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "none", + Name: validName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "none", }, refresh: 15 * time.Minute, errStrings: []string{}, @@ -239,14 +239,14 @@ func TestValidateCookie(t *testing.T) { { name: "with samesite \"strict\"", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "none", + Name: validName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "none", }, refresh: 15 * time.Minute, errStrings: []string{}, @@ -254,14 +254,14 @@ func TestValidateCookie(t *testing.T) { { name: "with samesite \"invalid\"", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: emptyDomains, - Path: "", - Expire: time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "invalid", + Name: validName, + Secret: validSecret, + Domains: emptyDomains, + Path: "", + Expire: time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "invalid", }, refresh: 15 * time.Minute, errStrings: []string{ @@ -271,14 +271,14 @@ func TestValidateCookie(t *testing.T) { { name: "with a combination of configuration errors", cookie: options.Cookie{ - Name: invalidName, - Secret: invalidSecret, - Domains: domains, - Path: "", - Expire: 15 * time.Minute, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "invalid", + Name: invalidName, + Secret: invalidSecret, + Domains: domains, + Path: "", + Expire: 15 * time.Minute, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "invalid", }, refresh: time.Hour, errStrings: []string{ @@ -291,14 +291,14 @@ func TestValidateCookie(t *testing.T) { { name: "with session cookie configuration", cookie: options.Cookie{ - Name: validName, - Secret: validSecret, - Domains: domains, - Path: "", - Expire: 0, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(true), - SameSite: "", + Name: validName, + Secret: validSecret, + Domains: domains, + Path: "", + Expire: 0, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessAllowed, + SameSite: "", }, refresh: 15 * time.Minute, errStrings: []string{}, @@ -310,12 +310,12 @@ func TestValidateCookie(t *testing.T) { Secret: options.SecretSource{ FromFile: tmpfile.Name(), }, - Domains: domains, - Path: "", - Expire: 24 * time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(false), - SameSite: "", + Domains: domains, + Path: "", + Expire: 24 * time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessDenied, + SameSite: "", }, refresh: 0, errStrings: []string{}, @@ -327,12 +327,12 @@ func TestValidateCookie(t *testing.T) { Secret: options.SecretSource{ FromFile: "/nonexistent/file.txt", }, - Domains: domains, - Path: "", - Expire: 24 * time.Hour, - Insecure: ptr.To(false), - NotHttpOnly: ptr.To(false), - SameSite: "", + Domains: domains, + Path: "", + Expire: 24 * time.Hour, + Insecure: ptr.To(false), + ScriptAccess: options.ScriptAccessDenied, + SameSite: "", }, refresh: 0, errStrings: []string{"could not read cookie secret file: /nonexistent/file.txt"}, From dda89305d88b93c4c7ad9405712e28baa757cd16 Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Sun, 28 Dec 2025 13:29:03 +0100 Subject: [PATCH 4/7] fix: cookie secret related test cases Signed-off-by: Jan Larwig --- main.go | 11 +++++---- main_test.go | 23 +++++++++--------- oauthproxy_test.go | 17 +++++++------ pkg/apis/options/cookie.go | 7 ++++-- pkg/apis/options/cookie_test.go | 12 +++++----- pkg/apis/options/legacy_cookie.go | 8 ++++--- pkg/apis/options/legacy_options_test.go | 18 ++++++-------- pkg/apis/options/options.go | 2 +- pkg/apis/options/secret_source.go | 29 ++++++++++++++++------- pkg/cookies/cookies.go | 4 +++- pkg/cookies/cookies_test.go | 8 +++---- pkg/cookies/csrf_per_request_test.go | 2 +- pkg/cookies/csrf_test.go | 2 +- pkg/encryption/utils.go | 7 +++--- pkg/middleware/headers_test.go | 4 ++-- pkg/sessions/redis/client_test.go | 3 ++- pkg/sessions/session_store_test.go | 4 ++-- pkg/sessions/tests/session_store_tests.go | 6 ++--- pkg/validation/cookie.go | 6 ++--- pkg/validation/cookie_test.go | 16 ++++++------- pkg/validation/header_test.go | 12 +++------- pkg/validation/options_test.go | 14 +++++------ 22 files changed, 115 insertions(+), 100 deletions(-) diff --git a/main.go b/main.go index 2a2d99a3..e16b782f 100644 --- a/main.go +++ b/main.go @@ -68,10 +68,8 @@ func main() { // It will either load the alpha configuration (if alphaConfig is given) // or the legacy configuration. func loadConfiguration(config, yamlConfig string, extraFlags *pflag.FlagSet, args []string) (*options.Options, error) { - opts, err := loadLegacyOptions(config, extraFlags, args) - if err != nil { - return nil, fmt.Errorf("failed to load legacy options: %w", err) - } + var err error + var opts *options.Options if yamlConfig != "" { logger.Printf("WARNING: You are using alpha configuration. The structure in this configuration file may change without notice. You MUST remove conflicting options from your existing configuration.") @@ -79,6 +77,11 @@ func loadConfiguration(config, yamlConfig string, extraFlags *pflag.FlagSet, arg if err != nil { return nil, fmt.Errorf("failed to load yaml options: %w", err) } + } else { + opts, err = loadLegacyOptions(config, extraFlags, args) + if err != nil { + return nil, fmt.Errorf("failed to load legacy options: %w", err) + } } // Ensure defaults after loading configuration diff --git a/main_test.go b/main_test.go index f896c398..2a93bb06 100644 --- a/main_test.go +++ b/main_test.go @@ -55,7 +55,7 @@ injectRequestHeaders: claim: user prefix: "Basic " basicAuthPassword: - value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk + value: YzNWd1pYSXRjMlZqY21WMExYQmhjM04zYjNKaw== - name: X-Forwarded-Groups preserveRequestValue: false values: @@ -83,12 +83,13 @@ injectResponseHeaders: claim: user prefix: "Basic " basicAuthPassword: - value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk + value: "YzNWd1pYSXRjMlZqY21WMExYQmhjM04zYjNKaw==" server: bindAddress: "127.0.0.1:4180" cookie: - secure: false - secret: "OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" + insecure: true + secret: + value: "OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" providers: - id: google=oauth2-proxy provider: google @@ -123,9 +124,9 @@ redirect_url="http://localhost:4180/oauth2/callback" opts, err := options.NewLegacyOptions().ToOptions() Expect(err).ToNot(HaveOccurred()) - opts.Cookie.Secret = "OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" + opts.Cookie.Secret = &options.SecretSource{Value: []byte("OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=")} opts.EmailDomains = []string{"example.com"} - opts.Cookie.Secure = ptr.To(false) + opts.Cookie.Insecure = ptr.To(true) opts.RawRedirectURL = "http://localhost:4180/oauth2/callback" opts.UpstreamServers = options.UpstreamConfig{ @@ -152,11 +153,9 @@ redirect_url="http://localhost:4180/oauth2/callback" Values: []options.HeaderValue{ { ClaimSource: &options.ClaimSource{ - Claim: "user", - Prefix: "Basic ", - BasicAuthPassword: &options.SecretSource{ - Value: []byte("c3VwZXItc2VjcmV0LXBhc3N3b3Jk"), - }, + Claim: "user", + Prefix: "Basic ", + BasicAuthPassword: options.NewSecretSourceFromString("c3VwZXItc2VjcmV0LXBhc3N3b3Jk"), }, }, }, @@ -294,7 +293,7 @@ redirect_url="http://localhost:4180/oauth2/callback" configContent: testCoreConfig + "unknown_field=\"something\"", alphaConfigContent: testAlphaConfig, expectedOptions: func() *options.Options { return nil }, - expectedErr: errors.New("failed to load legacy options: failed to load legacy config: error unmarshalling config: decoding failed due to the following error(s):\n\n'' has invalid keys: unknown_field"), + expectedErr: errors.New("failed to load yaml options: failed to load core options: failed to load config: error unmarshalling config: decoding failed due to the following error(s):\n\n'' has invalid keys: unknown_field"), }), ) }) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 722d84ee..6f6b2611 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -33,10 +33,13 @@ import ( const ( // The rawCookieSecret is 32 bytes and the base64CookieSecret is the base64 // encoded version of this. - rawCookieSecret = "secretthirtytwobytes+abcdefghijk" - base64CookieSecret = "c2VjcmV0dGhpcnR5dHdvYnl0ZXMrYWJjZGVmZ2hpams" - clientID = "3984n253984d7348dm8234yf982t" - clientSecret = "gv3498mfc9t23y23974dm2394dm9" + clientID = "3984n253984d7348dm8234yf982t" + clientSecret = "gv3498mfc9t23y23974dm2394dm9" +) + +var ( + rawCookieSecret = &options.SecretSource{Value: []byte("secretthirtytwobytes+abcdefghijk")} + base64CookieSecret = &options.SecretSource{Value: []byte("c2VjcmV0dGhpcnR5dHdvYnl0ZXMrYWJjZGVmZ2hpams")} ) func init() { @@ -207,7 +210,7 @@ func TestBasicAuthPassword(t *testing.T) { }, } - opts.Cookie.Secure = ptr.To(false) + opts.Cookie.Insecure = ptr.To(true) opts.InjectRequestHeaders = []options.Header{ { Name: "Authorization", @@ -362,7 +365,7 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) (*PassAccessTokenTe patt.opts.UpstreamServers.Upstreams = append(patt.opts.UpstreamServers.Upstreams, opts.ProxyUpstream) } - patt.opts.Cookie.Secure = ptr.To(false) + patt.opts.Cookie.Insecure = ptr.To(true) if opts.PassAccessToken { patt.opts.InjectRequestHeaders = []options.Header{ { @@ -3470,7 +3473,7 @@ func TestGetOAuthRedirectURI(t *testing.T) { { name: "redirect with http schema", setupOpts: func(baseOpts *options.Options) *options.Options { - baseOpts.Cookie.Secure = ptr.To(false) + baseOpts.Cookie.Insecure = ptr.To(true) return baseOpts }, req: &http.Request{ diff --git a/pkg/apis/options/cookie.go b/pkg/apis/options/cookie.go index 4b71544a..93baacec 100644 --- a/pkg/apis/options/cookie.go +++ b/pkg/apis/options/cookie.go @@ -37,7 +37,7 @@ type Cookie struct { // Name is the name of the cookie Name string `yaml:"name,omitempty"` // Secret is the secret source used to encrypt/sign the cookie value - Secret SecretSource `yaml:"secret,omitempty"` + Secret *SecretSource `yaml:"secret,omitempty"` // Domains is a list of domains for which the cookie is valid Domains []string `yaml:"domains,omitempty"` // Path is the path for which the cookie is valid @@ -98,7 +98,7 @@ func (sa *ScriptAccess) UnmarshalYAML(value *yaml.Node) error { // GetSecret returns the cookie secret as a string from the SecretSource func (c *Cookie) GetSecret() (string, error) { - secret, err := c.Secret.GetSecretValue() + secret, err := c.Secret.GetRawSecretValue() if err != nil { return "", fmt.Errorf("error getting cookie secret: %w", err) } @@ -117,6 +117,9 @@ func (c *Cookie) EnsureDefaults() { if c.Expire == 0 { c.Expire = time.Duration(168) * time.Hour } + if c.Secret == nil { + c.Secret = &SecretSource{} + } if c.Insecure == nil { c.Insecure = ptr.To(DefaultCookieInsecure) } diff --git a/pkg/apis/options/cookie_test.go b/pkg/apis/options/cookie_test.go index 053d0e6b..df1f63f2 100644 --- a/pkg/apis/options/cookie_test.go +++ b/pkg/apis/options/cookie_test.go @@ -10,7 +10,7 @@ import ( func TestCookieGetSecret(t *testing.T) { t.Run("returns secret when Secret is set", func(t *testing.T) { c := &Cookie{ - Secret: SecretSource{ + Secret: &SecretSource{ Value: []byte("my-secret"), FromFile: "", }, @@ -22,7 +22,7 @@ func TestCookieGetSecret(t *testing.T) { t.Run("returns secret when both Secret and SecretFile are set", func(t *testing.T) { c := &Cookie{ - Secret: SecretSource{ + Secret: &SecretSource{ Value: []byte("my-secret"), FromFile: "/some/file", }, @@ -43,7 +43,7 @@ func TestCookieGetSecret(t *testing.T) { tmpfile.Close() c := &Cookie{ - Secret: SecretSource{ + Secret: &SecretSource{ Value: []byte(""), FromFile: tmpfile.Name(), }, @@ -55,7 +55,7 @@ func TestCookieGetSecret(t *testing.T) { t.Run("returns error when file does not exist", func(t *testing.T) { c := &Cookie{ - Secret: SecretSource{ + Secret: &SecretSource{ Value: []byte(""), FromFile: "/nonexistent/file", }, @@ -63,12 +63,12 @@ func TestCookieGetSecret(t *testing.T) { secret, err := c.GetSecret() assert.Error(t, err) assert.Equal(t, "", secret) - assert.Contains(t, err.Error(), "error reading cookie secret file /nonexistent/file:") + assert.Contains(t, err.Error(), "error getting cookie secret: error reading secret from file \"/nonexistent/file\": open /nonexistent/file: no such file or directory") }) t.Run("returns empty when both Secret and SecretFile are empty", func(t *testing.T) { c := &Cookie{ - Secret: SecretSource{ + Secret: &SecretSource{ Value: []byte(""), FromFile: "", }, diff --git a/pkg/apis/options/legacy_cookie.go b/pkg/apis/options/legacy_cookie.go index 8240e7c8..b214ff13 100644 --- a/pkg/apis/options/legacy_cookie.go +++ b/pkg/apis/options/legacy_cookie.go @@ -52,9 +52,11 @@ func (l *LegacyCookie) convert() Cookie { scriptAccess = ScriptAccessAllowed } - var secret *SecretSource + secret := &SecretSource{} if l.Secret != "" { - secret = NewSecretSourceFromString(l.Secret) + secret = &SecretSource{ + Value: []byte(l.Secret), + } } else if l.SecretFile != "" { secret = &SecretSource{ FromFile: l.SecretFile, @@ -63,7 +65,7 @@ func (l *LegacyCookie) convert() Cookie { return Cookie{ Name: l.Name, - Secret: *secret, + Secret: secret, Domains: l.Domains, Path: l.Path, Expire: l.Expire, diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index d055be25..81659e0e 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -370,11 +370,9 @@ var _ = Describe("Legacy Options", func() { Values: []HeaderValue{ { ClaimSource: &ClaimSource{ - Claim: "user", - Prefix: "Basic ", - BasicAuthPassword: &SecretSource{ - Value: []byte(basicAuthSecret), - }, + Claim: "user", + Prefix: "Basic ", + BasicAuthPassword: NewSecretSourceFromString(basicAuthSecret), }, }, }, @@ -410,11 +408,9 @@ var _ = Describe("Legacy Options", func() { Values: []HeaderValue{ { ClaimSource: &ClaimSource{ - Claim: "email", - Prefix: "Basic ", - BasicAuthPassword: &SecretSource{ - Value: []byte(basicAuthSecret), - }, + Claim: "email", + Prefix: "Basic ", + BasicAuthPassword: NewSecretSourceFromString(basicAuthSecret), }, }, }, @@ -1094,7 +1090,7 @@ var _ = Describe("Legacy Options", func() { // Test cases and expected outcomes fullCookie := Cookie{ Name: "_oauth2_proxy", - Secret: SecretSource{}, + Secret: &SecretSource{}, Domains: nil, Path: "/", Expire: time.Duration(168) * time.Hour, diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 3555f55f..5440268a 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -36,7 +36,7 @@ type Options struct { HtpasswdUserGroups []string `flag:"htpasswd-user-group" cfg:"htpasswd_user_groups"` Cookie Cookie `cfg:",internal"` - Session SessionOptions `cfg:",squash"` + Session SessionOptions `cfg:",internal"` Logging Logging `cfg:",squash"` Templates Templates `cfg:",squash"` diff --git a/pkg/apis/options/secret_source.go b/pkg/apis/options/secret_source.go index d73ac41d..f3b35f2b 100644 --- a/pkg/apis/options/secret_source.go +++ b/pkg/apis/options/secret_source.go @@ -20,8 +20,8 @@ type SecretSource struct { } func NewSecretSourceFromValue(value []byte) *SecretSource { - encoded := make([]byte, base64.RawStdEncoding.EncodedLen(len(value))) - base64.RawStdEncoding.Encode(encoded, value) + encoded := make([]byte, base64.URLEncoding.EncodedLen(len(value))) + base64.URLEncoding.Encode(encoded, value) return &SecretSource{ Value: encoded, } @@ -31,13 +31,9 @@ func NewSecretSourceFromString(s string) *SecretSource { return NewSecretSourceFromValue([]byte(s)) } -func (ss *SecretSource) GetSecretValue() ([]byte, error) { +func (ss *SecretSource) GetRawSecretValue() ([]byte, error) { if len(ss.Value) > 0 { - var decoded []byte - if _, err := base64.RawStdEncoding.Decode(decoded, ss.Value); err != nil { - return nil, fmt.Errorf("error decoding secret value: %w", err) - } - return decoded, nil + return ss.Value, nil } if ss.FromEnv != "" { @@ -56,6 +52,23 @@ func (ss *SecretSource) GetSecretValue() ([]byte, error) { return nil, nil } +func (ss *SecretSource) GetSecretValue() ([]byte, error) { + value, err := ss.GetRawSecretValue() + if err != nil { + return nil, fmt.Errorf("failed getting raw secret value: %w", err) + } + + if value == nil { + return nil, fmt.Errorf("failed retrieving secret value: no source defined") + } + + decoded := make([]byte, base64.URLEncoding.DecodedLen(len(value))) + if _, err := base64.URLEncoding.Decode(decoded, value); err != nil { + return nil, fmt.Errorf("error decoding secret value: %w", err) + } + return decoded, nil +} + // EnsureDefaults sets any default values for SecretSource fields. func (ss *SecretSource) EnsureDefaults() { // No defaults to set currently diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index 913d9b9b..a221ac91 100644 --- a/pkg/cookies/cookies.go +++ b/pkg/cookies/cookies.go @@ -26,9 +26,11 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, opts *o domain = opts.Domains[len(opts.Domains)-1] } - httpOnly := true + var httpOnly bool if opts.ScriptAccess == options.ScriptAccessAllowed { httpOnly = false + } else { + httpOnly = true } c := &http.Cookie{ diff --git a/pkg/cookies/cookies_test.go b/pkg/cookies/cookies_test.go index dece3e95..8e4c213a 100644 --- a/pkg/cookies/cookies_test.go +++ b/pkg/cookies/cookies_test.go @@ -92,7 +92,7 @@ var _ = Describe("Cookie Tests", func() { } validName := "_oauth2_proxy" - validSecret := []byte("secretthirtytwobytes+abcdefghijk") + validSecret := &options.SecretSource{Value: []byte("secretthirtytwobytes+abcdefghijk")} domains := []string{"www.cookies.test"} now := time.Now() @@ -115,7 +115,7 @@ var _ = Describe("Cookie Tests", func() { value: "1", opts: options.Cookie{ Name: validName, - Secret: options.SecretSource{Value: validSecret}, + Secret: validSecret, Domains: domains, Path: "", Expire: time.Hour, @@ -133,7 +133,7 @@ var _ = Describe("Cookie Tests", func() { value: "1", opts: options.Cookie{ Name: validName, - Secret: options.SecretSource{Value: validSecret}, + Secret: validSecret, Domains: domains, Path: "", Expire: time.Hour * -1, @@ -151,7 +151,7 @@ var _ = Describe("Cookie Tests", func() { value: "1", opts: options.Cookie{ Name: validName, - Secret: options.SecretSource{Value: validSecret}, + Secret: validSecret, Domains: domains, Path: "", Expire: 0, diff --git a/pkg/cookies/csrf_per_request_test.go b/pkg/cookies/csrf_per_request_test.go index 0f9e2963..8247c22e 100644 --- a/pkg/cookies/csrf_per_request_test.go +++ b/pkg/cookies/csrf_per_request_test.go @@ -25,7 +25,7 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { BeforeEach(func() { cookieOpts = &options.Cookie{ Name: cookieName, - Secret: options.SecretSource{Value: cookieSecret}, + Secret: &options.SecretSource{Value: cookieSecret}, Domains: []string{cookieDomain}, Path: cookiePath, Expire: time.Hour, diff --git a/pkg/cookies/csrf_test.go b/pkg/cookies/csrf_test.go index c89f7d12..02b476b8 100644 --- a/pkg/cookies/csrf_test.go +++ b/pkg/cookies/csrf_test.go @@ -26,7 +26,7 @@ var _ = Describe("CSRF Cookie Tests", func() { BeforeEach(func() { cookieOpts = &options.Cookie{ Name: cookieName, - Secret: options.SecretSource{Value: cookieSecret}, + Secret: &options.SecretSource{Value: cookieSecret}, Domains: []string{cookieDomain}, Path: cookiePath, Expire: time.Hour, diff --git a/pkg/encryption/utils.go b/pkg/encryption/utils.go index df0158b8..642f80a8 100644 --- a/pkg/encryption/utils.go +++ b/pkg/encryption/utils.go @@ -9,6 +9,7 @@ import ( "hash" "io" "net/http" + "slices" "strconv" "strings" "time" @@ -27,10 +28,8 @@ func SecretBytes(secret string) []byte { // Only return decoded form if a valid AES length // Don't want unintentional decoding resulting in invalid lengths confusing a user // that thought they used a 16, 24, 32 length string - for _, i := range []int{16, 24, 32} { - if len(b) == i { - return b - } + if slices.Contains([]int{16, 24, 32}, len(b)) { + return b } } // If decoding didn't work or resulted in non-AES compliant length, diff --git a/pkg/middleware/headers_test.go b/pkg/middleware/headers_test.go index e8bbe665..c3ce016b 100644 --- a/pkg/middleware/headers_test.go +++ b/pkg/middleware/headers_test.go @@ -189,7 +189,7 @@ var _ = Describe("Headers Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.RawStdEncoding.EncodeToString([]byte("basic-password"))), + Value: []byte(base64.URLEncoding.EncodeToString([]byte("basic-password"))), FromEnv: "SECRET_ENV", }, }, @@ -461,7 +461,7 @@ var _ = Describe("Headers Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), + Value: []byte(base64.RawStdEncoding.EncodeToString([]byte("basic-password"))), FromEnv: "SECRET_ENV", }, }, diff --git a/pkg/sessions/redis/client_test.go b/pkg/sessions/redis/client_test.go index 2a07aba8..1f25ea10 100644 --- a/pkg/sessions/redis/client_test.go +++ b/pkg/sessions/redis/client_test.go @@ -9,6 +9,7 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/redis" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -26,7 +27,7 @@ var _ = Describe("Redis Client Tests", func() { RunClientTests(func(mr *miniredis.Miniredis) options.RedisStoreOptions { return options.RedisStoreOptions{ ClusterConnectionURLs: []string{"redis://" + mr.Addr()}, - UseCluster: true, + UseCluster: ptr.To(true), } }) }) diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go index d1fd5881..470de3e6 100644 --- a/pkg/sessions/session_store_test.go +++ b/pkg/sessions/session_store_test.go @@ -39,7 +39,7 @@ var _ = Describe("NewSessionStore", func() { secret := make([]byte, 32) _, err := rand.Read(secret) Expect(err).ToNot(HaveOccurred()) - var secretValue []byte + secretValue := make([]byte, base64.URLEncoding.EncodedLen(len(secret))) base64.URLEncoding.Encode(secretValue, secret) Expect(secretValue).ToNot(BeEmpty()) @@ -47,7 +47,7 @@ var _ = Describe("NewSessionStore", func() { // Set default options in CookieOptions cookieOpts = &options.Cookie{ Name: "_oauth2_proxy", - Secret: options.SecretSource{ + Secret: &options.SecretSource{ Value: secretValue, }, Path: "/", diff --git a/pkg/sessions/tests/session_store_tests.go b/pkg/sessions/tests/session_store_tests.go index d78fa679..5e47c007 100644 --- a/pkg/sessions/tests/session_store_tests.go +++ b/pkg/sessions/tests/session_store_tests.go @@ -73,7 +73,7 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi Insecure: ptr.To(false), ScriptAccess: options.ScriptAccessDenied, SameSite: options.SameSiteDefault, - Secret: options.SecretSource{Value: cookieSecret}, + Secret: &options.SecretSource{Value: cookieSecret}, } expires := time.Now().Add(1 * time.Hour) @@ -124,7 +124,7 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi ScriptAccess: options.ScriptAccessAllowed, Domains: []string{"example.com"}, SameSite: options.SameSiteStrict, - Secret: options.SecretSource{Value: cookieSecret}, + Secret: &options.SecretSource{Value: cookieSecret}, } var err error @@ -155,7 +155,7 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi Insecure: ptr.To(false), ScriptAccess: options.ScriptAccessDenied, SameSite: options.SameSiteDefault, - Secret: options.SecretSource{FromFile: tmpfile.Name()}, + Secret: &options.SecretSource{FromFile: tmpfile.Name()}, } ss, err = newSS(input.sessionOpts, input.cookieOpts) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/validation/cookie.go b/pkg/validation/cookie.go index 76d8dd12..31e7b238 100644 --- a/pkg/validation/cookie.go +++ b/pkg/validation/cookie.go @@ -49,12 +49,12 @@ func validateCookieName(name string) []string { return msgs } -func validateCookieSecret(secret options.SecretSource) []string { - if len(secret.Value) == 0 && secret.FromFile == "" { +func validateCookieSecret(secret *options.SecretSource) []string { + if secret == nil || len(secret.Value) == 0 && secret.FromFile == "" { return []string{"missing setting: cookie-secret or cookie-secret-file"} } - value, err := secret.GetSecretValue() + value, err := secret.GetRawSecretValue() if err != nil { return []string{fmt.Sprintf("error retrieving cookie secret: %v", err)} } diff --git a/pkg/validation/cookie_test.go b/pkg/validation/cookie_test.go index cd9a6bdf..30002126 100644 --- a/pkg/validation/cookie_test.go +++ b/pkg/validation/cookie_test.go @@ -18,11 +18,11 @@ func TestValidateCookie(t *testing.T) { invalidName := "_oauth2;proxy" // Separater character not allowed // 10 times the alphabet should be longer than 256 characters longName := strings.Repeat(alphabet, 10) - validSecret := options.SecretSource{ + validSecret := &options.SecretSource{ Value: []byte("secretthirtytwobytes+abcdefghijk"), } // 6 bytes is not a valid size - invalidSecret := options.SecretSource{ + invalidSecret := &options.SecretSource{ Value: []byte("abcdef"), } @@ -90,7 +90,7 @@ func TestValidateCookie(t *testing.T) { name: "with no cookie secret", cookie: options.Cookie{ Name: validName, - Secret: options.SecretSource{ + Secret: &options.SecretSource{ Value: nil, FromFile: "", }, @@ -127,7 +127,7 @@ func TestValidateCookie(t *testing.T) { name: "with a valid Base64 secret", cookie: options.Cookie{ Name: validName, - Secret: validBase64Secret, + Secret: &validBase64Secret, Domains: emptyDomains, Path: "", Expire: time.Hour, @@ -142,7 +142,7 @@ func TestValidateCookie(t *testing.T) { name: "with an invalid Base64 secret", cookie: options.Cookie{ Name: validName, - Secret: invalidBase64Secret, + Secret: &invalidBase64Secret, Domains: emptyDomains, Path: "", Expire: time.Hour, @@ -307,7 +307,7 @@ func TestValidateCookie(t *testing.T) { name: "with valid secret file", cookie: options.Cookie{ Name: validName, - Secret: options.SecretSource{ + Secret: &options.SecretSource{ FromFile: tmpfile.Name(), }, Domains: domains, @@ -324,7 +324,7 @@ func TestValidateCookie(t *testing.T) { name: "with nonexistent secret file", cookie: options.Cookie{ Name: validName, - Secret: options.SecretSource{ + Secret: &options.SecretSource{ FromFile: "/nonexistent/file.txt", }, Domains: domains, @@ -335,7 +335,7 @@ func TestValidateCookie(t *testing.T) { SameSite: "", }, refresh: 0, - errStrings: []string{"could not read cookie secret file: /nonexistent/file.txt"}, + errStrings: []string{"error retrieving cookie secret: error reading secret from file \"/nonexistent/file.txt\": open /nonexistent/file.txt: no such file or directory"}, }, } diff --git a/pkg/validation/header_test.go b/pkg/validation/header_test.go index e9566752..eb46f5af 100644 --- a/pkg/validation/header_test.go +++ b/pkg/validation/header_test.go @@ -1,8 +1,6 @@ package validation import ( - "encoding/base64" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -29,9 +27,7 @@ var _ = Describe("Headers", func() { Name: "X-Forwarded-Auth", Values: []options.HeaderValue{ { - SecretSource: &options.SecretSource{ - Value: []byte(base64.RawStdEncoding.EncodeToString([]byte("secret"))), - }, + SecretSource: options.NewSecretSourceFromString("secret"), }, }, } @@ -41,10 +37,8 @@ var _ = Describe("Headers", func() { Values: []options.HeaderValue{ { ClaimSource: &options.ClaimSource{ - Claim: "email", - BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.RawStdEncoding.EncodeToString([]byte("secret"))), - }, + Claim: "email", + BasicAuthPassword: options.NewSecretSourceFromString("secret"), }, }, }, diff --git a/pkg/validation/options_test.go b/pkg/validation/options_test.go index e0f7bbb0..0d193af8 100644 --- a/pkg/validation/options_test.go +++ b/pkg/validation/options_test.go @@ -20,7 +20,7 @@ const ( ) var ( - cookieSecret = options.NewSecretSourceFromString("secretthirtytwobytes+abcdefghijk") + cookieSecret = &options.SecretSource{Value: []byte("secretthirtytwobytes+abcdefghijk")} ) func testOptions() *options.Options { @@ -128,7 +128,7 @@ func TestCookieRefreshMustBeLessThanCookieExpire(t *testing.T) { o := testOptions() assert.Equal(t, nil, Validate(o)) - o.Cookie.Secret = options.NewSecretSourceFromString("0123456789abcdef") + o.Cookie.Secret = &options.SecretSource{Value: []byte("0123456789abcdef")} o.Session.Refresh = o.Cookie.Expire assert.NotEqual(t, nil, Validate(o)) @@ -141,23 +141,23 @@ func TestBase64CookieSecret(t *testing.T) { assert.Equal(t, nil, Validate(o)) // 32 byte, base64 (urlsafe) encoded key - o.Cookie.Secret = options.NewSecretSourceFromString("yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ=") + o.Cookie.Secret = &options.SecretSource{Value: []byte("yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ=")} assert.Equal(t, nil, Validate(o)) // 32 byte, base64 (urlsafe) encoded key, w/o padding - o.Cookie.Secret = options.NewSecretSourceFromString("yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ") + o.Cookie.Secret = &options.SecretSource{Value: []byte("yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ")} assert.Equal(t, nil, Validate(o)) // 24 byte, base64 (urlsafe) encoded key - o.Cookie.Secret = options.NewSecretSourceFromString("Kp33Gj-GQmYtz4zZUyUDdqQKx5_Hgkv3") + o.Cookie.Secret = &options.SecretSource{Value: []byte("Kp33Gj-GQmYtz4zZUyUDdqQKx5_Hgkv3")} assert.Equal(t, nil, Validate(o)) // 16 byte, base64 (urlsafe) encoded key - o.Cookie.Secret = options.NewSecretSourceFromString("LFEqZYvYUwKwzn0tEuTpLA==") + o.Cookie.Secret = &options.SecretSource{Value: []byte("LFEqZYvYUwKwzn0tEuTpLA==")} assert.Equal(t, nil, Validate(o)) // 16 byte, base64 (urlsafe) encoded key, w/o padding - o.Cookie.Secret = options.NewSecretSourceFromString("LFEqZYvYUwKwzn0tEuTpLA") + o.Cookie.Secret = &options.SecretSource{Value: []byte("LFEqZYvYUwKwzn0tEuTpLA")} assert.Equal(t, nil, Validate(o)) } From 6ac03bcad6ac57caf5db1a841c7b483b5b3a50cf Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Sun, 28 Dec 2025 20:33:03 +0100 Subject: [PATCH 5/7] fix: cookie secret source and header value conversion workflow Signed-off-by: Jan Larwig --- docs/docs/configuration/alpha_config.md | 78 +++++++++++++++++-- docs/docs/configuration/alpha_config.md.tmpl | 72 ++++++++++++++++-- main.go | 9 +-- main_test.go | 8 +- pkg/apis/options/claim_source.go | 45 +++++++++++ pkg/apis/options/cookie.go | 4 + pkg/apis/options/header.go | 21 +----- pkg/apis/options/secret_source.go | 27 +++++++ pkg/apis/options/secret_source_test.go | 79 ++++++++++++++++++++ 9 files changed, 299 insertions(+), 44 deletions(-) create mode 100644 pkg/apis/options/claim_source.go create mode 100644 pkg/apis/options/secret_source_test.go diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 5d3e57c7..6fb53fe9 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -24,12 +24,18 @@ When using alpha configuration, your config file will look something like below: upstreams: - id: ... ...: ... +providers: + - id: ... + ...: ... +cookie: + secret: ... + ...: ... injectRequestHeaders: - - name: ... - ...: ... + - secretSource: + ...: ... injectResponseHeaders: - - name: ... - ...: ... + - claimSource: + ...: ... ``` Please browse the [reference](#configuration-reference) below for the structure @@ -67,9 +73,9 @@ the new config. oauth2-proxy --alpha-config ./path/to/new/config.yaml --config ./path/to/existing/config.cfg ``` -## Using ENV variables in the alpha configuration +### How to use environment variables -The alpha package supports the use of environment variables in place of yaml keys, allowing sensitive values to be pulled from somewhere other than the yaml file. +The alpha package supports the use of environment variables in place of yaml values, allowing sensitive data to be pulled from somewhere other than the yaml file. When using environment variables, your yaml will look like this: ```yaml @@ -81,6 +87,46 @@ When using environment variables, your yaml will look like this: Where CLIENT_SECRET is an environment variable. More information and available patterns can be found [here](https://github.com/a8m/envsubst#docs) +### How to inject custom headers + +Configure `injectRequestHeaders` and `injectResponseHeaders` in alpha config YAML. + +```yaml +injectRequestHeaders: + - name: "X-User-Email" + values: + - claimSource: + claim: "email" # extract the email claim contents from the id token + - name: "X-Static-Secret" + values: + # secrets need to be encoded with base64 when directly in the yaml config but will be send decoded + - secretSource: + value: "c3VwZXItc2VjcmV0" + - name: "X-Static-File-Secret" + - secretSource: + fromFile: "/path/to/my/secret" + - name: "X-Static-Env-Secret" + - secretSource: + value: "${MY_SECRET_ENV}" # content still needs to be base64 encoded +injectResponseHeaders: + # Following will result in a header "Authorization: Basic (encoded)" + - name: "Authorization" + values: + - claimSource: + claim: user + prefix: "Basic " + basicAuthPassword: + value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk # base64 encoded password +``` + +**Value sources:** +* `claimSource` - `claim` (session claims either from id token or from profile URL) +* `secretSource` - `value` (base64), `fromFile` (file path) + +**Request option:** `preserveRequestValue: true` retains existing header values + +**Incompatibility:** Remove legacy flags `pass-user-headers`, `set-xauthrequest` + ## Removed options The following flags/options and their respective environment variables are no @@ -143,6 +189,20 @@ longer available when using alpha configuration: - `cookie-csrf-per-request-limit`/`cookie_csrf_per_request_limit` - `cookie-csrf-expire`/`cookie_csrf_expire` +### Legacy Session options +- `session-cookie-minimal`/`session_cookie_minimal` +- `session-store-type`/`session_store_type` +- `redis-cluster-connection-urls`/`redis_cluster_connection_urls` +- `redis-connection-url`/`redis_connection_url` +- `redis-insecure-skip-tls-verify`/`redis_insecure_skip_tls_verify` +- `redis-password`/`redis_password` +- `redis-sentinel-password`/`redis_sentinel_password` +- `redis-sentinel-master-name`/`redis_sentinel_master_name` +- `redis-sentinel-connection-urls`/`redis_sentinel_connection_urls` +- `redis-use-cluster`/`redis_use_cluster` +- `redis-use-sentinel`/`redis_use_sentinel` +- `redis-connection-idle-timeout`/`redis_connection_idle_timeout` + and all provider-specific options, i.e. any option whose name includes `oidc`, `azure`, `bitbucket`, `github`, `gitlab`, `google` or `keycloak`. Attempting to use any of these options via flags or via config when `--alpha-config` is @@ -545,14 +605,16 @@ RedisStoreOptions contains configuration options for the RedisSessionStore. (**Appears on:** [Cookie](#cookie)) - +SameSiteMode is an enum representing the different SameSite modes for cookies +Available modes are "lax", "strict", "none", and "" (default browser behavior) ### ScriptAccess #### (`string` alias) (**Appears on:** [Cookie](#cookie)) - +ScriptAccess is an enum representing whether a cookie is accessible to JavaScript +Available modes are "allow", "deny" (default behavior) ### SecretSource diff --git a/docs/docs/configuration/alpha_config.md.tmpl b/docs/docs/configuration/alpha_config.md.tmpl index 95c07534..1666dd2e 100644 --- a/docs/docs/configuration/alpha_config.md.tmpl +++ b/docs/docs/configuration/alpha_config.md.tmpl @@ -24,12 +24,18 @@ When using alpha configuration, your config file will look something like below: upstreams: - id: ... ...: ... +providers: + - id: ... + ...: ... +cookie: + secret: ... + ...: ... injectRequestHeaders: - - name: ... - ...: ... + - secretSource: + ...: ... injectResponseHeaders: - - name: ... - ...: ... + - claimSource: + ...: ... ``` Please browse the [reference](#configuration-reference) below for the structure @@ -67,9 +73,9 @@ the new config. oauth2-proxy --alpha-config ./path/to/new/config.yaml --config ./path/to/existing/config.cfg ``` -## Using ENV variables in the alpha configuration +### How to use environment variables -The alpha package supports the use of environment variables in place of yaml keys, allowing sensitive values to be pulled from somewhere other than the yaml file. +The alpha package supports the use of environment variables in place of yaml values, allowing sensitive data to be pulled from somewhere other than the yaml file. When using environment variables, your yaml will look like this: ```yaml @@ -81,6 +87,46 @@ When using environment variables, your yaml will look like this: Where CLIENT_SECRET is an environment variable. More information and available patterns can be found [here](https://github.com/a8m/envsubst#docs) +### How to inject custom headers + +Configure `injectRequestHeaders` and `injectResponseHeaders` in alpha config YAML. + +```yaml +injectRequestHeaders: + - name: "X-User-Email" + values: + - claimSource: + claim: "email" # extract the email claim contents from the id token + - name: "X-Static-Secret" + values: + # secrets need to be encoded with base64 when directly in the yaml config but will be send decoded + - secretSource: + value: "c3VwZXItc2VjcmV0" + - name: "X-Static-File-Secret" + - secretSource: + fromFile: "/path/to/my/secret" + - name: "X-Static-Env-Secret" + - secretSource: + value: "${MY_SECRET_ENV}" # content still needs to be base64 encoded +injectResponseHeaders: + # Following will result in a header "Authorization: Basic (encoded)" + - name: "Authorization" + values: + - claimSource: + claim: user + prefix: "Basic " + basicAuthPassword: + value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk # base64 encoded password +``` + +**Value sources:** +* `claimSource` - `claim` (session claims either from id token or from profile URL) +* `secretSource` - `value` (base64), `fromFile` (file path) + +**Request option:** `preserveRequestValue: true` retains existing header values + +**Incompatibility:** Remove legacy flags `pass-user-headers`, `set-xauthrequest` + ## Removed options The following flags/options and their respective environment variables are no @@ -143,6 +189,20 @@ longer available when using alpha configuration: - `cookie-csrf-per-request-limit`/`cookie_csrf_per_request_limit` - `cookie-csrf-expire`/`cookie_csrf_expire` +### Legacy Session options +- `session-cookie-minimal`/`session_cookie_minimal` +- `session-store-type`/`session_store_type` +- `redis-cluster-connection-urls`/`redis_cluster_connection_urls` +- `redis-connection-url`/`redis_connection_url` +- `redis-insecure-skip-tls-verify`/`redis_insecure_skip_tls_verify` +- `redis-password`/`redis_password` +- `redis-sentinel-password`/`redis_sentinel_password` +- `redis-sentinel-master-name`/`redis_sentinel_master_name` +- `redis-sentinel-connection-urls`/`redis_sentinel_connection_urls` +- `redis-use-cluster`/`redis_use_cluster` +- `redis-use-sentinel`/`redis_use_sentinel` +- `redis-connection-idle-timeout`/`redis_connection_idle_timeout` + and all provider-specific options, i.e. any option whose name includes `oidc`, `azure`, `bitbucket`, `github`, `gitlab`, `google` or `keycloak`. Attempting to use any of these options via flags or via config when `--alpha-config` is diff --git a/main.go b/main.go index e16b782f..8a6869da 100644 --- a/main.go +++ b/main.go @@ -175,14 +175,7 @@ func printConvertedConfig(opts *options.Options) error { return fmt.Errorf("only single provider conversion is supported") } - // Generic interface for loading arbitrary yaml structure - var buffer map[string]interface{} - - if err := options.Decode(alphaConfig, &buffer); err != nil { - return fmt.Errorf("unable to decode alpha config into interface: %w", err) - } - - data, err := yaml.Marshal(buffer) + data, err := yaml.Marshal(alphaConfig) if err != nil { return fmt.Errorf("unable to marshal config: %v", err) } diff --git a/main_test.go b/main_test.go index 2a93bb06..b15a798f 100644 --- a/main_test.go +++ b/main_test.go @@ -23,7 +23,7 @@ http_address="127.0.0.1:4180" upstreams="http://httpbin" set_basic_auth="true" -basic_auth_password="c3VwZXItc2VjcmV0LXBhc3N3b3Jk" +basic_auth_password="super-secret-password" client_id="oauth2-proxy" client_secret="b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK" @@ -55,7 +55,7 @@ injectRequestHeaders: claim: user prefix: "Basic " basicAuthPassword: - value: YzNWd1pYSXRjMlZqY21WMExYQmhjM04zYjNKaw== + value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk - name: X-Forwarded-Groups preserveRequestValue: false values: @@ -83,7 +83,7 @@ injectResponseHeaders: claim: user prefix: "Basic " basicAuthPassword: - value: "YzNWd1pYSXRjMlZqY21WMExYQmhjM04zYjNKaw==" + value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk server: bindAddress: "127.0.0.1:4180" cookie: @@ -155,7 +155,7 @@ redirect_url="http://localhost:4180/oauth2/callback" ClaimSource: &options.ClaimSource{ Claim: "user", Prefix: "Basic ", - BasicAuthPassword: options.NewSecretSourceFromString("c3VwZXItc2VjcmV0LXBhc3N3b3Jk"), + BasicAuthPassword: &options.SecretSource{Value: []byte("c3VwZXItc2VjcmV0LXBhc3N3b3Jk")}, }, }, }, diff --git a/pkg/apis/options/claim_source.go b/pkg/apis/options/claim_source.go new file mode 100644 index 00000000..561e2810 --- /dev/null +++ b/pkg/apis/options/claim_source.go @@ -0,0 +1,45 @@ +package options + +import "fmt" + +// ClaimSource allows loading a header value from a claim within the session +type ClaimSource struct { + // Claim is the name of the claim in the session that the value should be + // loaded from. Available claims: `access_token` `id_token` `created_at` + // `expires_on` `refresh_token` `email` `user` `groups` `preferred_username`. + Claim string `yaml:"claim,omitempty"` + + // Prefix is an optional prefix that will be prepended to the value of the + // claim if it is non-empty. + Prefix string `yaml:"prefix,omitempty"` + + // BasicAuthPassword converts this claim into a basic auth header. + // Note the value of claim will become the basic auth username and the + // basicAuthPassword will be used as the password value. + BasicAuthPassword *SecretSource `yaml:"basicAuthPassword,omitempty"` +} + +// MarshalYAML implements the yaml.Marshaler interface for ClaimSource. +// This is only necessary for the conversion workflow from toml to yaml +func (c *ClaimSource) MarshalYAML() (interface{}, error) { + if c == nil { + return nil, nil + } + + mapped := make(map[string]interface{}) + if c.Claim != "" { + mapped["claim"] = c.Claim + } + if c.Prefix != "" { + mapped["prefix"] = c.Prefix + } + if c.BasicAuthPassword != nil { + basicAuthPasswordYAML, err := c.BasicAuthPassword.MarshalYAML() + if err != nil { + return nil, fmt.Errorf("error marshaling basicAuthPassword for ClaimSource: %w", err) + } + mapped["basicAuthPassword"] = basicAuthPasswordYAML + } + + return mapped, nil +} diff --git a/pkg/apis/options/cookie.go b/pkg/apis/options/cookie.go index 93baacec..60b2706e 100644 --- a/pkg/apis/options/cookie.go +++ b/pkg/apis/options/cookie.go @@ -15,6 +15,8 @@ const ( DefaultCSRFPerRequest bool = false ) +// SameSiteMode is an enum representing the different SameSite modes for cookies +// Available modes are "lax", "strict", "none", and "" (default browser behavior) type SameSiteMode string const ( @@ -24,6 +26,8 @@ const ( SameSiteDefault SameSiteMode = "" ) +// ScriptAccess is an enum representing whether a cookie is accessible to JavaScript +// Available modes are "allow", "deny" (default behavior) type ScriptAccess string const ( diff --git a/pkg/apis/options/header.go b/pkg/apis/options/header.go index d9509de0..f0d29c1f 100644 --- a/pkg/apis/options/header.go +++ b/pkg/apis/options/header.go @@ -1,6 +1,8 @@ package options -import "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" +import ( + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" +) const ( // DefaultHeaderPreserveRequestValue is the default value for Header.PreserveRequestValue @@ -46,23 +48,6 @@ type HeaderValue struct { *ClaimSource `yaml:"claimSource,omitempty"` } -// ClaimSource allows loading a header value from a claim within the session -type ClaimSource struct { - // Claim is the name of the claim in the session that the value should be - // loaded from. Available claims: `access_token` `id_token` `created_at` - // `expires_on` `refresh_token` `email` `user` `groups` `preferred_username`. - Claim string `yaml:"claim,omitempty"` - - // Prefix is an optional prefix that will be prepended to the value of the - // claim if it is non-empty. - Prefix string `yaml:"prefix,omitempty"` - - // BasicAuthPassword converts this claim into a basic auth header. - // Note the value of claim will become the basic auth username and the - // basicAuthPassword will be used as the password value. - BasicAuthPassword *SecretSource `yaml:"basicAuthPassword,omitempty"` -} - // EnsureDefaults sets any default values for Header fields. func (h *Header) EnsureDefaults() { if h.PreserveRequestValue == nil { diff --git a/pkg/apis/options/secret_source.go b/pkg/apis/options/secret_source.go index f3b35f2b..34fd0ba6 100644 --- a/pkg/apis/options/secret_source.go +++ b/pkg/apis/options/secret_source.go @@ -62,6 +62,10 @@ func (ss *SecretSource) GetSecretValue() ([]byte, error) { return nil, fmt.Errorf("failed retrieving secret value: no source defined") } + if len(ss.Value) == 0 && ss.FromFile != "" { + return value, nil + } + decoded := make([]byte, base64.URLEncoding.DecodedLen(len(value))) if _, err := base64.URLEncoding.Decode(decoded, value); err != nil { return nil, fmt.Errorf("error decoding secret value: %w", err) @@ -69,6 +73,29 @@ func (ss *SecretSource) GetSecretValue() ([]byte, error) { return decoded, nil } +// MarshalYAML implements the yaml.Marshaler interface for SecretSource. +// This is only necessary for the conversion workflow from toml to yaml +func (ss *SecretSource) MarshalYAML() (interface{}, error) { + if ss == nil { + return nil, nil + } + + if ss.FromFile != "" { + return map[string]string{ + "fromFile": ss.FromFile, + }, nil + } + + encodedValue, err := ss.GetRawSecretValue() + if err != nil { + return nil, fmt.Errorf("error getting raw secret value for marshaling: %w", err) + } + + return map[string]string{ + "value": string(encodedValue), + }, nil +} + // EnsureDefaults sets any default values for SecretSource fields. func (ss *SecretSource) EnsureDefaults() { // No defaults to set currently diff --git a/pkg/apis/options/secret_source_test.go b/pkg/apis/options/secret_source_test.go new file mode 100644 index 00000000..40250798 --- /dev/null +++ b/pkg/apis/options/secret_source_test.go @@ -0,0 +1,79 @@ +package options + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +const ( + testSecretValue = "bXktc2VjcmV0" // base64 for "my-secret" + testFileSecretContent = "file-secret" +) + +func TestSecretSourceGetSecretValue(t *testing.T) { + t.Run("returns secret when Value is set", func(t *testing.T) { + ss := &SecretSource{ + Value: []byte(testSecretValue), + FromFile: "", + } + secret, err := ss.GetSecretValue() + assert.NoError(t, err) + assert.Equal(t, "my-secret", string(secret)) + }) + + t.Run("returns secret when both Value and FromFile are set", func(t *testing.T) { + ss := &SecretSource{ + Value: []byte(testSecretValue), + FromFile: "/some/file", + } + secret, err := ss.GetSecretValue() + assert.NoError(t, err) + assert.Equal(t, "my-secret", string(secret)) + }) + + t.Run("reads from file when only FromFile is set", func(t *testing.T) { + // Create a temporary file + tmpfile, err := os.CreateTemp("", "secret-source-test") + assert.NoError(t, err) + defer os.Remove(tmpfile.Name()) + + _, err = tmpfile.Write([]byte(testFileSecretContent)) + assert.NoError(t, err) + tmpfile.Close() + + ss := &SecretSource{ + FromFile: tmpfile.Name(), + } + secret, err := ss.GetSecretValue() + assert.NoError(t, err) + assert.Equal(t, "file-secret", string(secret)) + }) + + t.Run("returns error when file does not exist", func(t *testing.T) { + ss := &SecretSource{ + FromFile: "/nonexistent/file", + } + secret, err := ss.GetSecretValue() + assert.Error(t, err) + assert.Nil(t, secret) + }) + + t.Run("returns error when no source is defined", func(t *testing.T) { + ss := &SecretSource{} + secret, err := ss.GetSecretValue() + assert.Error(t, err) + assert.Nil(t, secret) + }) + + t.Run("returns error when Value is not valid base64", func(t *testing.T) { + ss := &SecretSource{ + Value: []byte("invalid-base64"), + FromFile: "", + } + secret, err := ss.GetSecretValue() + assert.Error(t, err) + assert.Nil(t, secret) + }) +} From d29b846052096c8b22bd30ef01ff6418ef7d829e Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Mon, 29 Dec 2025 21:24:33 +0100 Subject: [PATCH 6/7] fix: boolean print and contrib example file Signed-off-by: Jan Larwig --- contrib/local-environment/oauth2-proxy-alpha-config.yaml | 5 +++-- oauthproxy.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/contrib/local-environment/oauth2-proxy-alpha-config.yaml b/contrib/local-environment/oauth2-proxy-alpha-config.yaml index 5a99e6a8..2f18e459 100644 --- a/contrib/local-environment/oauth2-proxy-alpha-config.yaml +++ b/contrib/local-environment/oauth2-proxy-alpha-config.yaml @@ -1,8 +1,9 @@ server: bindAddress: "0.0.0.0:4180" cookie: - secret: OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w= - secure: false + secret: + value: OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w= + insecure: true providers: - id: oidc provider: oidc diff --git a/oauthproxy.go b/oauthproxy.go index c22ac5b2..4559d1f3 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -180,7 +180,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr refresh = fmt.Sprintf("after %s", opts.Session.Refresh) } - logger.Printf("Cookie settings: name:%s insecure(http):%v scriptaccess:%v expiry:%s domains:%s path:%s samesite:%s refresh:%s", opts.Cookie.Name, opts.Cookie.Insecure, opts.Cookie.ScriptAccess, opts.Cookie.Expire, strings.Join(opts.Cookie.Domains, ","), opts.Cookie.Path, opts.Cookie.SameSite, refresh) + logger.Printf("Cookie settings: name:%s insecure(http):%v scriptaccess:%v expiry:%s domains:%s path:%s samesite:%s refresh:%s", opts.Cookie.Name, *opts.Cookie.Insecure, opts.Cookie.ScriptAccess, opts.Cookie.Expire, strings.Join(opts.Cookie.Domains, ","), opts.Cookie.Path, opts.Cookie.SameSite, refresh) trustedIPs := ip.NewNetSet() for _, ipStr := range opts.TrustedIPs { From f289a516e2749c4c91be96f3c78bd796a3a6fd6d Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Tue, 6 Jan 2026 20:22:07 +0100 Subject: [PATCH 7/7] feat(cookie): change SameSiteMode and ScriptAccess enum values to PascalCasing Signed-off-by: Jan Larwig --- docs/docs/configuration/alpha_config.md | 4 ++-- pkg/apis/options/cookie.go | 14 +++++++------- pkg/apis/options/legacy_cookie.go | 14 +++++++++++++- pkg/cookies/cookies.go | 8 ++++---- pkg/validation/cookie.go | 4 ++-- pkg/validation/cookie_test.go | 8 ++++---- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 6fb53fe9..c9b6ae94 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -606,7 +606,7 @@ RedisStoreOptions contains configuration options for the RedisSessionStore. (**Appears on:** [Cookie](#cookie)) SameSiteMode is an enum representing the different SameSite modes for cookies -Available modes are "lax", "strict", "none", and "" (default browser behavior) +Available modes are "Lax", "Strict", "None", and "" (default browser behavior) ### ScriptAccess #### (`string` alias) @@ -614,7 +614,7 @@ Available modes are "lax", "strict", "none", and "" (default browser behavior) (**Appears on:** [Cookie](#cookie)) ScriptAccess is an enum representing whether a cookie is accessible to JavaScript -Available modes are "allow", "deny" (default behavior) +Available modes are "Allow", "Deny" (default behavior) ### SecretSource diff --git a/pkg/apis/options/cookie.go b/pkg/apis/options/cookie.go index 60b2706e..d4a4fb99 100644 --- a/pkg/apis/options/cookie.go +++ b/pkg/apis/options/cookie.go @@ -16,23 +16,23 @@ const ( ) // SameSiteMode is an enum representing the different SameSite modes for cookies -// Available modes are "lax", "strict", "none", and "" (default browser behavior) +// Available modes are "Lax", "Strict", "None", and "" (default browser behavior) type SameSiteMode string const ( - SameSiteLax SameSiteMode = "lax" - SameSiteStrict SameSiteMode = "strict" - SameSiteNone SameSiteMode = "none" + SameSiteLax SameSiteMode = "Lax" + SameSiteStrict SameSiteMode = "Strict" + SameSiteNone SameSiteMode = "None" SameSiteDefault SameSiteMode = "" ) // ScriptAccess is an enum representing whether a cookie is accessible to JavaScript -// Available modes are "allow", "deny" (default behavior) +// Available modes are "Allow", "Deny" (default behavior) type ScriptAccess string const ( - ScriptAccessDenied ScriptAccess = "deny" - ScriptAccessAllowed ScriptAccess = "allow" + ScriptAccessDenied ScriptAccess = "Deny" + ScriptAccessAllowed ScriptAccess = "Allow" ScriptAccessNone ScriptAccess = "" ) diff --git a/pkg/apis/options/legacy_cookie.go b/pkg/apis/options/legacy_cookie.go index b214ff13..9262ff3a 100644 --- a/pkg/apis/options/legacy_cookie.go +++ b/pkg/apis/options/legacy_cookie.go @@ -63,6 +63,18 @@ func (l *LegacyCookie) convert() Cookie { } } + var sameSite SameSiteMode + switch l.SameSite { + case "lax": + sameSite = SameSiteLax + case "strict": + sameSite = SameSiteStrict + case "none": + sameSite = SameSiteNone + default: + sameSite = SameSiteDefault + } + return Cookie{ Name: l.Name, Secret: secret, @@ -71,7 +83,7 @@ func (l *LegacyCookie) convert() Cookie { Expire: l.Expire, Insecure: &insecure, ScriptAccess: scriptAccess, - SameSite: SameSiteMode(l.SameSite), + SameSite: sameSite, CSRFPerRequest: &l.CSRFPerRequest, CSRFPerRequestLimit: l.CSRFPerRequestLimit, CSRFExpire: l.CSRFExpire, diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index a221ac91..fef560de 100644 --- a/pkg/cookies/cookies.go +++ b/pkg/cookies/cookies.go @@ -69,13 +69,13 @@ func GetCookieDomain(req *http.Request, cookieDomains []string) string { // Parse a valid http.SameSite value from a user supplied string for use of making cookies. func ParseSameSite(v options.SameSiteMode) http.SameSite { switch v { - case "lax": + case options.SameSiteLax: return http.SameSiteLaxMode - case "strict": + case options.SameSiteStrict: return http.SameSiteStrictMode - case "none": + case options.SameSiteNone: return http.SameSiteNoneMode - case "": + case options.SameSiteDefault: return 0 default: panic(fmt.Sprintf("Invalid value for SameSite: %s", v)) diff --git a/pkg/validation/cookie.go b/pkg/validation/cookie.go index 31e7b238..c4ab9b9d 100644 --- a/pkg/validation/cookie.go +++ b/pkg/validation/cookie.go @@ -21,9 +21,9 @@ func validateCookie(o options.Cookie, refresh time.Duration) []string { } switch o.SameSite { - case "", "none", "lax", "strict": + case options.SameSiteLax, options.SameSiteStrict, options.SameSiteNone, options.SameSiteDefault: default: - msgs = append(msgs, fmt.Sprintf("cookie_samesite (%q) must be one of ['', 'lax', 'strict', 'none']", o.SameSite)) + msgs = append(msgs, fmt.Sprintf("cookie_samesite (%q) must be one of ['', 'Lax', 'Strict', 'None']", o.SameSite)) } // Sort cookie domains by length, so that we try longer (and more specific) domains first diff --git a/pkg/validation/cookie_test.go b/pkg/validation/cookie_test.go index 30002126..b2e3219a 100644 --- a/pkg/validation/cookie_test.go +++ b/pkg/validation/cookie_test.go @@ -63,7 +63,7 @@ func TestValidateCookie(t *testing.T) { invalidSecretMsg := "cookie_secret must be 16, 24, or 32 bytes to create an AES cipher, but is 6 bytes" invalidBase64SecretMsg := "cookie_secret must be 16, 24, or 32 bytes to create an AES cipher, but is 10 bytes" refreshLongerThanExpireMsg := "cookie_refresh (\"1h0m0s\") must be less than cookie_expire (\"15m0s\")" - invalidSameSiteMsg := "cookie_samesite (\"invalid\") must be one of ['', 'lax', 'strict', 'none']" + invalidSameSiteMsg := "cookie_samesite (\"invalid\") must be one of ['', 'Lax', 'Strict', 'None']" testCases := []struct { name string @@ -216,7 +216,7 @@ func TestValidateCookie(t *testing.T) { Expire: time.Hour, Insecure: ptr.To(false), ScriptAccess: options.ScriptAccessAllowed, - SameSite: "none", + SameSite: options.SameSiteNone, }, refresh: 15 * time.Minute, errStrings: []string{}, @@ -231,7 +231,7 @@ func TestValidateCookie(t *testing.T) { Expire: time.Hour, Insecure: ptr.To(false), ScriptAccess: options.ScriptAccessAllowed, - SameSite: "none", + SameSite: options.SameSiteLax, }, refresh: 15 * time.Minute, errStrings: []string{}, @@ -246,7 +246,7 @@ func TestValidateCookie(t *testing.T) { Expire: time.Hour, Insecure: ptr.To(false), ScriptAccess: options.ScriptAccessAllowed, - SameSite: "none", + SameSite: options.SameSiteStrict, }, refresh: 15 * time.Minute, errStrings: []string{},