feat(config): convert cookie property (Not)HTTPOnly boolean to enum

Signed-off-by: Jan Larwig <jan@larwig.com>
This commit is contained in:
Jan Larwig
2026-03-26 22:24:21 +01:00
parent 64bf0fc6f3
commit bb30b83c6f
13 changed files with 312 additions and 202 deletions
+9 -3
View File
@@ -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
}
+32 -7
View File
@@ -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"`
@@ -60,6 +67,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 {
@@ -74,6 +83,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()
@@ -98,8 +123,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)
+7 -4
View File
@@ -44,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 != "" {
@@ -65,7 +68,7 @@ func (l *LegacyCookie) convert() Cookie {
Path: l.Path,
Expire: l.Expire,
Insecure: &insecure,
NotHttpOnly: &notHTTPOnly,
ScriptAccess: scriptAccess,
SameSite: SameSiteMode(l.SameSite),
CSRFPerRequest: &l.CSRFPerRequest,
CSRFPerRequestLimit: l.CSRFPerRequestLimit,
+1 -1
View File
@@ -1101,7 +1101,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,
+6 -1
View File
@@ -26,12 +26,17 @@ func MakeCookieFromOptions(req *http.Request, value string, opts *options.Cookie
domain = opts.Domains[len(opts.Domains)-1]
}
httpOnly := true
if opts.ScriptAccess == options.ScriptAccessAllowed {
httpOnly = false
}
c := &http.Cookie{
Name: opts.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),
}
+24 -24
View File
@@ -111,14 +111,14 @@ var _ = Describe("Cookie Tests", func() {
host: "www.cookies.test",
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: "",
},
now: now,
expectedOutput: int((15 * time.Minute).Seconds()),
@@ -127,14 +127,14 @@ var _ = Describe("Cookie Tests", func() {
host: "www.cookies.test",
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: "",
},
now: now,
expectedOutput: -1,
@@ -143,14 +143,14 @@ var _ = Describe("Cookie Tests", func() {
host: "www.cookies.test",
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: "",
},
now: now,
expectedOutput: expectedMaxAge,
+1 -1
View File
@@ -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,
}
+1 -1
View File
@@ -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,
}
+5 -5
View File
@@ -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: "",
}
})
+30 -23
View File
@@ -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))
}
})
+122 -122
View File
@@ -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"},