fix: cookie secret source and header value conversion workflow

Signed-off-by: Jan Larwig <jan@larwig.com>
This commit is contained in:
Jan Larwig 2025-12-28 20:33:03 +01:00
parent d0c125d173
commit df0cf59f97
9 changed files with 196 additions and 32 deletions

View File

@ -285,6 +285,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
@ -380,6 +394,7 @@ Cookie contains configuration options relating session and CSRF cookies
| `insecure` | _bool_ | Insecure indicates whether the cookie allows to be sent over HTTP<br/>Default is false, which requires HTTPS |
| `scriptAccess` | _[ScriptAccess](#scriptaccess)_ | ScriptAccess is a wrapper enum for HTTPOnly; indicates whether the<br/>cookie is accessible to JavaScript. Default is deny which translates<br/>to true for HTTPOnly, which helps mitigate certain XSS attacks |
| `sameSite` | _[SameSiteMode](#samesitemode)_ | SameSite sets the SameSite attribute on the cookie |
| `sameSite` | _[SameSiteMode](#samesitemode)_ | CSRFSameSite sets the SameSite attribute on the csrf cookies |
| `csrfPerRequest` | _bool_ | CSRFPerRequest indicates whether a unique CSRF token is generated for each request<br/>Enables parallel requests from clients (e.g., multiple tabs)<br/>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<br/>Used to prevent unbounded memory growth from storing too many tokens |
| `csrfExpire` | _duration_ | CSRFExpire sets the duration before a CSRF token expires |
@ -689,14 +704,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

View File

@ -285,6 +285,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

View File

@ -189,14 +189,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)
}

View File

@ -24,7 +24,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"
@ -56,7 +56,7 @@ injectRequestHeaders:
claim: user
prefix: "Basic "
basicAuthPassword:
value: YzNWd1pYSXRjMlZqY21WMExYQmhjM04zYjNKaw==
value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk
- name: X-Forwarded-Groups
preserveRequestValue: false
values:
@ -84,7 +84,7 @@ injectResponseHeaders:
claim: user
prefix: "Basic "
basicAuthPassword:
value: "YzNWd1pYSXRjMlZqY21WMExYQmhjM04zYjNKaw=="
value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk
server:
bindAddress: "127.0.0.1:4180"
cookie:
@ -156,7 +156,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")},
},
},
},

View File

@ -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
}

View File

@ -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 (

View File

@ -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 {

View File

@ -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

View File

@ -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)
})
}