From 1d73f140bf7b14634c196ada9e0926636f60f994 Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Fri, 25 Jul 2025 13:29:22 +0200 Subject: [PATCH] revert: secrets as []byte instead of string Signed-off-by: Jan Larwig --- docs/docs/configuration/alpha_config.md | 4 ++-- main_test.go | 2 +- oauthproxy_test.go | 6 +++--- pkg/apis/options/legacy_options.go | 2 +- pkg/apis/options/legacy_options_test.go | 4 ++-- pkg/apis/options/load_test.go | 2 +- pkg/apis/options/secret_source.go | 4 ++-- pkg/apis/options/util/util.go | 2 +- pkg/apis/options/util/util_test.go | 2 +- pkg/header/injector_test.go | 16 ++++++++-------- pkg/http/http_suite_test.go | 12 ++++++------ pkg/http/server_test.go | 8 ++++---- pkg/middleware/headers_test.go | 8 ++++---- pkg/validation/common_test.go | 4 ++-- pkg/validation/header.go | 2 ++ pkg/validation/header_test.go | 4 ++-- 16 files changed, 42 insertions(+), 40 deletions(-) diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 425dde8b..6e578bbb 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -265,7 +265,7 @@ make up the header value | Field | Type | Description | | ----- | ---- | ----------- | -| `value` | _string_ | Value expects a base64 encoded string value. | +| `value` | _[]byte_ | Value expects a base64 encoded []byte | | `fromEnv` | _string_ | FromEnv expects the name of an environment variable. | | `fromFile` | _string_ | FromFile expects a path to a file containing the secret value. | | `claim` | _string_ | 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`. | @@ -477,7 +477,7 @@ Only one source within the struct should be defined at any time. | Field | Type | Description | | ----- | ---- | ----------- | -| `value` | _string_ | Value expects a base64 encoded string value. | +| `value` | _[]byte_ | Value expects a base64 encoded []byte | | `fromEnv` | _string_ | FromEnv expects the name of an environment variable. | | `fromFile` | _string_ | FromFile expects a path to a file containing the secret value. | diff --git a/main_test.go b/main_test.go index 7de1ac72..60b7eafd 100644 --- a/main_test.go +++ b/main_test.go @@ -139,7 +139,7 @@ redirect_url="http://localhost:4180/oauth2/callback" Claim: "user", Prefix: "Basic ", BasicAuthPassword: &options.SecretSource{ - Value: "super-secret-password", + Value: []byte("super-secret-password"), }, }, }, diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 7b495bff..488b8cea 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -215,7 +215,7 @@ func TestBasicAuthPassword(t *testing.T) { ClaimSource: &options.ClaimSource{ Claim: "email", BasicAuthPassword: &options.SecretSource{ - Value: basicAuthPassword, + Value: []byte(basicAuthPassword), }, }, }, @@ -1282,7 +1282,7 @@ func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: "This is a secure password", + Value: []byte("This is a secure password"), }, }, }, @@ -2044,7 +2044,7 @@ func baseTestOptions() *options.Options { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: base64.StdEncoding.EncodeToString([]byte("This is a secure password")), + Value: []byte(base64.StdEncoding.EncodeToString([]byte("This is a secure password"))), }, }, }, diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index db73f910..99746553 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -294,7 +294,7 @@ func getBasicAuthHeader(preferEmailToUser bool, basicAuthPassword string) Header Claim: claim, Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: basicAuthPassword, + Value: []byte(basicAuthPassword), }, }, }, diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index c5f0e4da..98a89601 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -369,7 +369,7 @@ var _ = Describe("Legacy Options", func() { Claim: "user", Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: basicAuthSecret, + Value: []byte(basicAuthSecret), }, }, }, @@ -409,7 +409,7 @@ var _ = Describe("Legacy Options", func() { Claim: "email", Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: basicAuthSecret, + Value: []byte(basicAuthSecret), }, }, }, diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index a0079267..29d6511e 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -581,7 +581,7 @@ injectResponseHeaders: Values: []HeaderValue{ { SecretSource: &SecretSource{ - Value: "secret", + Value: []byte("secret"), }, }, }, diff --git a/pkg/apis/options/secret_source.go b/pkg/apis/options/secret_source.go index 9d82c605..e73d019f 100644 --- a/pkg/apis/options/secret_source.go +++ b/pkg/apis/options/secret_source.go @@ -3,8 +3,8 @@ package options // SecretSource references an individual secret value. // Only one source within the struct should be defined at any time. type SecretSource struct { - // Value expects a base64 encoded string value. - Value string `yaml:"value,omitempty"` + // Value expects a base64 encoded []byte + Value []byte `yaml:"value,omitempty"` // FromEnv expects the name of an environment variable. FromEnv string `yaml:"fromEnv,omitempty"` diff --git a/pkg/apis/options/util/util.go b/pkg/apis/options/util/util.go index 794a6e91..03f0a134 100644 --- a/pkg/apis/options/util/util.go +++ b/pkg/apis/options/util/util.go @@ -11,7 +11,7 @@ import ( func GetSecretValue(source *options.SecretSource) ([]byte, error) { switch { case len(source.Value) > 0 && source.FromEnv == "" && source.FromFile == "": - return []byte(source.Value), nil + return source.Value, nil case len(source.Value) == 0 && source.FromEnv != "" && source.FromFile == "": return []byte(os.Getenv(source.FromEnv)), nil case len(source.Value) == 0 && source.FromEnv == "" && source.FromFile != "": diff --git a/pkg/apis/options/util/util_test.go b/pkg/apis/options/util/util_test.go index 5c4bfa6d..e84db1ec 100644 --- a/pkg/apis/options/util/util_test.go +++ b/pkg/apis/options/util/util_test.go @@ -31,7 +31,7 @@ var _ = Describe("GetSecretValue", func() { It("returns the correct value from the string value", func() { value, err := GetSecretValue(&options.SecretSource{ - Value: "secret-value-1", + Value: []byte("secret-value-1"), }) Expect(err).ToNot(HaveOccurred()) Expect(string(value)).To(Equal("secret-value-1")) diff --git a/pkg/header/injector_test.go b/pkg/header/injector_test.go index bb37261d..25c276dc 100644 --- a/pkg/header/injector_test.go +++ b/pkg/header/injector_test.go @@ -55,7 +55,7 @@ var _ = Describe("Injector Suite", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: "super-secret", + Value: []byte("super-secret"), }, }, }, @@ -199,7 +199,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: "basic-password", + Value: []byte("basic-password"), }, }, }, @@ -227,7 +227,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: base64.StdEncoding.EncodeToString([]byte("basic-password")), + Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), }, }, }, @@ -322,7 +322,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: base64.StdEncoding.EncodeToString([]byte("basic-password")), + Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), FromEnv: "SECRET_ENV", }, }, @@ -348,7 +348,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: "basic-password", + Value: []byte("basic-password"), }, }, }, @@ -379,17 +379,17 @@ var _ = Describe("Injector Suite", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: "major=1", + Value: []byte("major=1"), }, }, { SecretSource: &options.SecretSource{ - Value: "minor=2", + Value: []byte("minor=2"), }, }, { SecretSource: &options.SecretSource{ - Value: "patch=3", + Value: []byte("patch=3"), }, }, }, diff --git a/pkg/http/http_suite_test.go b/pkg/http/http_suite_test.go index 219f26ea..19d4d3ff 100644 --- a/pkg/http/http_suite_test.go +++ b/pkg/http/http_suite_test.go @@ -48,10 +48,10 @@ var _ = BeforeSuite(func() { certOut := new(bytes.Buffer) Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed()) - ipv4CertDataSource.Value = certOut.String() + ipv4CertDataSource.Value = certOut.Bytes() keyOut := new(bytes.Buffer) Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed()) - ipv4KeyDataSource.Value = keyOut.String() + ipv4KeyDataSource.Value = keyOut.Bytes() }) By("Generating a ipv6 self-signed cert for TLS tests", func() { @@ -61,16 +61,16 @@ var _ = BeforeSuite(func() { certOut := new(bytes.Buffer) Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed()) - ipv6CertDataSource.Value = certOut.String() + ipv6CertDataSource.Value = certOut.Bytes() keyOut := new(bytes.Buffer) Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed()) - ipv6KeyDataSource.Value = keyOut.String() + ipv6KeyDataSource.Value = keyOut.Bytes() }) By("Setting up a http client", func() { - ipv4cert, err := tls.X509KeyPair([]byte(ipv4CertDataSource.Value), []byte(ipv4KeyDataSource.Value)) + ipv4cert, err := tls.X509KeyPair(ipv4CertDataSource.Value, ipv4KeyDataSource.Value) Expect(err).ToNot(HaveOccurred()) - ipv6cert, err := tls.X509KeyPair([]byte(ipv6CertDataSource.Value), []byte(ipv6KeyDataSource.Value)) + ipv6cert, err := tls.X509KeyPair(ipv6CertDataSource.Value, ipv6KeyDataSource.Value) Expect(err).ToNot(HaveOccurred()) ipv4certificate, err := x509.ParseCertificate(ipv4cert.Certificate[0]) diff --git a/pkg/http/server_test.go b/pkg/http/server_test.go index 6584b757..8dfa13af 100644 --- a/pkg/http/server_test.go +++ b/pkg/http/server_test.go @@ -234,7 +234,7 @@ var _ = Describe("Server", func() { SecureBindAddress: "127.0.0.1:0", TLS: &options.TLS{ Key: &options.SecretSource{ - Value: "invalid", + Value: []byte("invalid"), }, Cert: &ipv4CertDataSource, }, @@ -250,7 +250,7 @@ var _ = Describe("Server", func() { TLS: &options.TLS{ Key: &ipv4KeyDataSource, Cert: &options.SecretSource{ - Value: "invalid", + Value: []byte("invalid"), }, }, }, @@ -506,7 +506,7 @@ var _ = Describe("Server", func() { SecureBindAddress: "[::1]:0", TLS: &options.TLS{ Key: &options.SecretSource{ - Value: "invalid", + Value: []byte("invalid"), }, Cert: &ipv6CertDataSource, }, @@ -523,7 +523,7 @@ var _ = Describe("Server", func() { TLS: &options.TLS{ Key: &ipv6KeyDataSource, Cert: &options.SecretSource{ - Value: "invalid", + Value: []byte("invalid"), }, }, }, diff --git a/pkg/middleware/headers_test.go b/pkg/middleware/headers_test.go index b1848ae0..06440eea 100644 --- a/pkg/middleware/headers_test.go +++ b/pkg/middleware/headers_test.go @@ -188,7 +188,7 @@ var _ = Describe("Headers Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: base64.StdEncoding.EncodeToString([]byte("basic-password")), + Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), FromEnv: "SECRET_ENV", }, }, @@ -260,7 +260,7 @@ var _ = Describe("Headers Suite", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: "_oauth2_proxy=ey123123123", + Value: []byte("_oauth2_proxy=ey123123123"), }, }, }, @@ -270,7 +270,7 @@ var _ = Describe("Headers Suite", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: "oauth_user", + Value: []byte("oauth_user"), }, }, }, @@ -416,7 +416,7 @@ var _ = Describe("Headers Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: base64.StdEncoding.EncodeToString([]byte("basic-password")), + Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), FromEnv: "SECRET_ENV", }, }, diff --git a/pkg/validation/common_test.go b/pkg/validation/common_test.go index bb7c2dd6..9e873c35 100644 --- a/pkg/validation/common_test.go +++ b/pkg/validation/common_test.go @@ -9,12 +9,12 @@ import ( ) var _ = Describe("Common", func() { - var validSecretSourceValue string + var validSecretSourceValue []byte const validSecretSourceEnv = "OAUTH2_PROXY_TEST_SECRET_SOURCE_ENV" var validSecretSourceFile string BeforeEach(func() { - validSecretSourceValue = "This is a secret source value" + validSecretSourceValue = []byte("This is a secret source value") Expect(os.Setenv(validSecretSourceEnv, "This is a secret source env")).To(Succeed()) tmp, err := os.CreateTemp("", "oauth2-proxy-secret-source-test") Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/validation/header.go b/pkg/validation/header.go index 713113f4..b1258144 100644 --- a/pkg/validation/header.go +++ b/pkg/validation/header.go @@ -51,9 +51,11 @@ func validateHeaderValue(_ string, value options.HeaderValue) []string { func validateHeaderValueClaimSource(claim options.ClaimSource) []string { msgs := []string{} + if claim.Claim == "" { msgs = append(msgs, "claim should not be empty") } + if claim.BasicAuthPassword != nil { msgs = append(msgs, prefixValues("invalid basicAuthPassword: ", validateSecretSource(*claim.BasicAuthPassword))...) } diff --git a/pkg/validation/header_test.go b/pkg/validation/header_test.go index 88849ea1..2d9ef6dd 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: base64.StdEncoding.EncodeToString([]byte("secret")), + Value: []byte(base64.StdEncoding.EncodeToString([]byte("secret"))), }, }, }, @@ -43,7 +43,7 @@ var _ = Describe("Headers", func() { ClaimSource: &options.ClaimSource{ Claim: "email", BasicAuthPassword: &options.SecretSource{ - Value: base64.StdEncoding.EncodeToString([]byte("secret")), + Value: []byte(base64.StdEncoding.EncodeToString([]byte("secret"))), }, }, },