From 458710149ca8a843b5be13d6d439a6fd29cc80f2 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sun, 12 Apr 2020 14:00:59 +0100 Subject: [PATCH] Rename Cookie Options to remove extra 'Cookie' --- CHANGELOG.md | 8 +++- env_options.go | 2 +- env_options_test.go | 18 +++++++++ oauthproxy.go | 26 ++++++------ oauthproxy_test.go | 48 +++++++++++----------- options.go | 49 +++++++++++------------ options_test.go | 38 +++++++++--------- pkg/apis/options/cookie.go | 18 ++++----- pkg/cookies/cookies.go | 14 +++---- pkg/sessions/cookie/session_store.go | 14 +++---- pkg/sessions/redis/redis_store.go | 34 ++++++++-------- pkg/sessions/session_store_test.go | 60 ++++++++++++++-------------- 12 files changed, 175 insertions(+), 154 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f6301e9..e2a026b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,14 +11,18 @@ - Migration from Pusher to independent org may have introduced breaking changes for your environment. - See the changes listed below for PR [#464](https://github.com/oauth2-proxy/oauth2-proxy/pull/464) for full details - Binaries renamed from `oauth2_proxy` to `oauth2-proxy` - - [#440](https://github.com/oauth2-proxy/oauth2-proxy/pull/440) Switch Azure AD Graph API to Microsoft Graph API (@johejo) - - The Azure AD Graph API has been [deprecated](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-graph-api) and is being replaced by the Microsoft Graph API. + - The Azure AD Graph API has been [deprecated](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-graph-api) and is being replaced by the Microsoft Graph API. If your application relies on the access token being passed to it to access the Azure AD Graph API, you should migrate your application to use the Microsoft Graph API. Existing behaviour can be retained by setting `-resource=https://graph.windows.net`. +- [#484](https://github.com/oauth2-proxy/oauth2-proxy/pull/484) Configuration loading has been replaced with Viper and PFlag + - Flags now require a `--` prefix before the option + - Previously flags allowed either `-` or `--` to prefix the option name + - Eg `-provider` must now be `--provider` ## Changes since v5.1.0 +- [#484](https://github.com/oauth2-proxy/oauth2-proxy/pull/484) Replace configuration loading with Viper (@JoelSpeed) - [#499](https://github.com/oauth2-proxy/oauth2-proxy/pull/469) Add `-user-id-claim` to support generic claims in addition to email - [#486](https://github.com/oauth2-proxy/oauth2-proxy/pull/486) Add new linters (@johejo) - [#440](https://github.com/oauth2-proxy/oauth2-proxy/pull/440) Switch Azure AD Graph API to Microsoft Graph API (@johejo) diff --git a/env_options.go b/env_options.go index 058b90c6..51a888f4 100644 --- a/env_options.go +++ b/env_options.go @@ -31,7 +31,7 @@ func (cfg EnvOptions) LoadEnvForStruct(options interface{}) { field := typ.Field(i) fieldV := reflect.Indirect(val).Field(i) - if field.Type.Kind() == reflect.Struct && field.Anonymous { + if field.Type.Kind() == reflect.Struct { cfg.LoadEnvForStruct(fieldV.Interface()) continue } diff --git a/env_options_test.go b/env_options_test.go index eb72a83e..c8ee75c2 100644 --- a/env_options_test.go +++ b/env_options_test.go @@ -11,12 +11,17 @@ import ( type EnvTest struct { TestField string `cfg:"target_field" env:"TEST_ENV_FIELD"` EnvTestEmbed + Named EnvTestNamed } type EnvTestEmbed struct { TestFieldEmbed string `cfg:"target_field_embed" env:"TEST_ENV_FIELD_EMBED"` } +type EnvTestNamed struct { + TestFieldNamed string `cfg:"target_field_named" env:"TEST_ENV_FIELD_NAMED"` +} + func TestLoadEnvForStruct(t *testing.T) { cfg := make(proxy.EnvOptions) @@ -44,3 +49,16 @@ func TestLoadEnvForStructWithEmbeddedFields(t *testing.T) { v := cfg["target_field_embed"] assert.Equal(t, v, "1234abcd") } + +func TestLoadEnvForStructWithNamedStructFields(t *testing.T) { + cfg := make(proxy.EnvOptions) + cfg.LoadEnvForStruct(&EnvTest{}) + + _, ok := cfg["target_field_named"] + assert.Equal(t, ok, false) + + os.Setenv("TEST_ENV_FIELD_NAMED", "1234abcd") + cfg.LoadEnvForStruct(&EnvTest{}) + v := cfg["target_field_named"] + assert.Equal(t, v, "1234abcd") +} diff --git a/oauthproxy.go b/oauthproxy.go index 1e9bb7ca..516340c5 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -264,23 +264,23 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { logger.Printf("OAuthProxy configured for %s Client ID: %s", opts.provider.Data().ProviderName, opts.ClientID) refresh := "disabled" - if opts.CookieRefresh != time.Duration(0) { - refresh = fmt.Sprintf("after %s", opts.CookieRefresh) + if opts.Cookie.Refresh != time.Duration(0) { + refresh = fmt.Sprintf("after %s", opts.Cookie.Refresh) } - logger.Printf("Cookie settings: name:%s secure(https):%v httponly:%v expiry:%s domains:%s path:%s samesite:%s refresh:%s", opts.CookieName, opts.CookieSecure, opts.CookieHTTPOnly, opts.CookieExpire, strings.Join(opts.CookieDomains, ","), opts.CookiePath, opts.CookieSameSite, 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) return &OAuthProxy{ - CookieName: opts.CookieName, - CSRFCookieName: fmt.Sprintf("%v_%v", opts.CookieName, "csrf"), - CookieSeed: opts.CookieSecret, - CookieDomains: opts.CookieDomains, - CookiePath: opts.CookiePath, - CookieSecure: opts.CookieSecure, - CookieHTTPOnly: opts.CookieHTTPOnly, - CookieExpire: opts.CookieExpire, - CookieRefresh: opts.CookieRefresh, - CookieSameSite: opts.CookieSameSite, + CookieName: opts.Cookie.Name, + CSRFCookieName: fmt.Sprintf("%v_%v", opts.Cookie.Name, "csrf"), + CookieSeed: opts.Cookie.Secret, + CookieDomains: opts.Cookie.Domains, + CookiePath: opts.Cookie.Path, + CookieSecure: opts.Cookie.Secure, + CookieHTTPOnly: opts.Cookie.HTTPOnly, + CookieExpire: opts.Cookie.Expire, + CookieRefresh: opts.Cookie.Refresh, + CookieSameSite: opts.Cookie.SameSite, Validator: validator, RobotsPath: "/robots.txt", diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 47b27743..50df0872 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -164,7 +164,7 @@ func TestRobotsTxt(t *testing.T) { opts := NewOptions() opts.ClientID = "asdlkjx" opts.ClientSecret = "alkgks" - opts.CookieSecret = "asdkugkj" + opts.Cookie.Secret = "asdkugkj" opts.Validate() proxy := NewOAuthProxy(opts, func(string) bool { return true }) @@ -179,7 +179,7 @@ func TestIsValidRedirect(t *testing.T) { opts := NewOptions() opts.ClientID = "skdlfj" opts.ClientSecret = "fgkdsgj" - opts.CookieSecret = "ljgiogbj" + opts.Cookie.Secret = "ljgiogbj" // Should match domains that are exactly foo.bar and any subdomain of bar.foo opts.WhitelistDomains = []string{ "foo.bar", @@ -398,10 +398,10 @@ func TestBasicAuthPassword(t *testing.T) { opts.Upstreams = append(opts.Upstreams, providerServer.URL) // The CookieSecret must be 32 bytes in order to create the AES // cipher. - opts.CookieSecret = "xyzzyplughxyzzyplughxyzzyplughxp" + opts.Cookie.Secret = "xyzzyplughxyzzyplughxyzzyplughxp" opts.ClientID = "dlgkj" opts.ClientSecret = "alkgret" - opts.CookieSecure = false + opts.Cookie.Secure = false opts.PassBasicAuth = true opts.SetBasicAuth = true opts.PassUserHeaders = true @@ -582,10 +582,10 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) *PassAccessTokenTes } // The CookieSecret must be 32 bytes in order to create the AES // cipher. - t.opts.CookieSecret = "xyzzyplughxyzzyplughxyzzyplughxp" + t.opts.Cookie.Secret = "xyzzyplughxyzzyplughxyzzyplughxp" t.opts.ClientID = "slgkj" t.opts.ClientSecret = "gfjgojl" - t.opts.CookieSecure = false + t.opts.Cookie.Secure = false t.opts.PassAccessToken = opts.PassAccessToken t.opts.Validate() @@ -735,7 +735,7 @@ func NewSignInPageTest(skipProvider bool) *SignInPageTest { var sipTest SignInPageTest sipTest.opts = NewOptions() - sipTest.opts.CookieSecret = "adklsj2" + sipTest.opts.Cookie.Secret = "adklsj2" sipTest.opts.ClientID = "lkdgj" sipTest.opts.ClientSecret = "sgiufgoi" sipTest.opts.SkipProviderButton = skipProvider @@ -841,10 +841,10 @@ func NewProcessCookieTest(opts ProcessCookieTestOpts, modifiers ...OptionsModifi } pcTest.opts.ClientID = "asdfljk" pcTest.opts.ClientSecret = "lkjfdsig" - pcTest.opts.CookieSecret = "0123456789abcdefabcd" + pcTest.opts.Cookie.Secret = "0123456789abcdefabcd" // First, set the CookieRefresh option so proxy.AesCipher is created, // needed to encrypt the access_token. - pcTest.opts.CookieRefresh = time.Hour + pcTest.opts.Cookie.Refresh = time.Hour pcTest.opts.Validate() pcTest.proxy = NewOAuthProxy(pcTest.opts, func(email string) bool { @@ -915,7 +915,7 @@ func TestProcessCookieNoCookieError(t *testing.T) { func TestProcessCookieRefreshNotSet(t *testing.T) { pcTest := NewProcessCookieTestWithOptionsModifiers(func(opts *Options) { - opts.CookieExpire = time.Duration(23) * time.Hour + opts.Cookie.Expire = time.Duration(23) * time.Hour }) reference := time.Now().Add(time.Duration(-2) * time.Hour) @@ -932,7 +932,7 @@ func TestProcessCookieRefreshNotSet(t *testing.T) { func TestProcessCookieFailIfCookieExpired(t *testing.T) { pcTest := NewProcessCookieTestWithOptionsModifiers(func(opts *Options) { - opts.CookieExpire = time.Duration(24) * time.Hour + opts.Cookie.Expire = time.Duration(24) * time.Hour }) reference := time.Now().Add(time.Duration(25) * time.Hour * -1) startSession := &sessions.SessionState{Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: reference} @@ -947,7 +947,7 @@ func TestProcessCookieFailIfCookieExpired(t *testing.T) { func TestProcessCookieFailIfRefreshSetAndCookieExpired(t *testing.T) { pcTest := NewProcessCookieTestWithOptionsModifiers(func(opts *Options) { - opts.CookieExpire = time.Duration(24) * time.Hour + opts.Cookie.Expire = time.Duration(24) * time.Hour }) reference := time.Now().Add(time.Duration(25) * time.Hour * -1) startSession := &sessions.SessionState{Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: reference} @@ -1017,7 +1017,7 @@ func TestAuthOnlyEndpointUnauthorizedOnNoCookieSetError(t *testing.T) { func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) { test := NewAuthOnlyEndpointTest(func(opts *Options) { - opts.CookieExpire = time.Duration(24) * time.Hour + opts.Cookie.Expire = time.Duration(24) * time.Hour }) reference := time.Now().Add(time.Duration(25) * time.Hour * -1) startSession := &sessions.SessionState{ @@ -1149,7 +1149,7 @@ func TestAuthSkippedForPreflightRequests(t *testing.T) { opts.Upstreams = append(opts.Upstreams, upstream.URL) opts.ClientID = "aljsal" opts.ClientSecret = "jglkfsdgj" - opts.CookieSecret = "dkfjgdls" + opts.Cookie.Secret = "dkfjgdls" opts.SkipAuthPreflight = true opts.Validate() @@ -1196,7 +1196,7 @@ type SignatureTest struct { func NewSignatureTest() *SignatureTest { opts := NewOptions() - opts.CookieSecret = "cookie secret" + opts.Cookie.Secret = "cookie secret" opts.ClientID = "client ID" opts.ClientSecret = "client secret" opts.EmailDomains = []string{"acm.org"} @@ -1343,7 +1343,7 @@ type ajaxRequestTest struct { func newAjaxRequestTest() *ajaxRequestTest { test := &ajaxRequestTest{} test.opts = NewOptions() - test.opts.CookieSecret = "sdflsw" + test.opts.Cookie.Secret = "sdflsw" test.opts.ClientID = "gkljfdl" test.opts.ClientSecret = "sdflkjs" test.opts.Validate() @@ -1401,11 +1401,11 @@ func TestAjaxForbiddendRequest(t *testing.T) { func TestClearSplitCookie(t *testing.T) { opts := NewOptions() - opts.CookieName = "oauth2" - opts.CookieDomains = []string{"abc"} - store, err := cookie.NewCookieSessionStore(&opts.SessionOptions, &opts.CookieOptions) + opts.Cookie.Name = "oauth2" + opts.Cookie.Domains = []string{"abc"} + store, err := cookie.NewCookieSessionStore(&opts.SessionOptions, &opts.Cookie) assert.Equal(t, err, nil) - p := OAuthProxy{CookieName: opts.CookieName, CookieDomains: opts.CookieDomains, sessionStore: store} + p := OAuthProxy{CookieName: opts.Cookie.Name, CookieDomains: opts.Cookie.Domains, sessionStore: store} var rw = httptest.NewRecorder() req := httptest.NewRequest("get", "/", nil) @@ -1430,11 +1430,11 @@ func TestClearSplitCookie(t *testing.T) { func TestClearSingleCookie(t *testing.T) { opts := NewOptions() - opts.CookieName = "oauth2" - opts.CookieDomains = []string{"abc"} - store, err := cookie.NewCookieSessionStore(&opts.SessionOptions, &opts.CookieOptions) + opts.Cookie.Name = "oauth2" + opts.Cookie.Domains = []string{"abc"} + store, err := cookie.NewCookieSessionStore(&opts.SessionOptions, &opts.Cookie) assert.Equal(t, err, nil) - p := OAuthProxy{CookieName: opts.CookieName, CookieDomains: opts.CookieDomains, sessionStore: store} + p := OAuthProxy{CookieName: opts.Cookie.Name, CookieDomains: opts.Cookie.Domains, sessionStore: store} var rw = httptest.NewRecorder() req := httptest.NewRequest("get", "/", nil) diff --git a/options.go b/options.go index 50dee28b..8b3e461b 100644 --- a/options.go +++ b/options.go @@ -64,8 +64,7 @@ type Options struct { Banner string `flag:"banner" cfg:"banner" env:"OAUTH2_PROXY_BANNER"` Footer string `flag:"footer" cfg:"footer" env:"OAUTH2_PROXY_FOOTER"` - // Embed CookieOptions - options.CookieOptions + Cookie options.CookieOptions // Embed SessionOptions options.SessionOptions @@ -158,12 +157,12 @@ func NewOptions() *Options { HTTPSAddress: ":443", ForceHTTPS: false, DisplayHtpasswdForm: true, - CookieOptions: options.CookieOptions{ - CookieName: "_oauth2_proxy", - CookieSecure: true, - CookieHTTPOnly: true, - CookieExpire: time.Duration(168) * time.Hour, - CookieRefresh: time.Duration(0), + Cookie: options.CookieOptions{ + Name: "_oauth2_proxy", + Secure: true, + HTTPOnly: true, + Expire: time.Duration(168) * time.Hour, + Refresh: time.Duration(0), }, SessionOptions: options.SessionOptions{ Type: "cookie", @@ -227,7 +226,7 @@ func (o *Options) Validate() error { } msgs := make([]string, 0) - if o.CookieSecret == "" { + if o.Cookie.Secret == "" { msgs = append(msgs, "missing setting: cookie-secret") } if o.ClientID == "" { @@ -382,31 +381,31 @@ func (o *Options) Validate() error { msgs = parseProviderInfo(o, msgs) var cipher *encryption.Cipher - if o.PassAccessToken || o.SetAuthorization || o.PassAuthorization || (o.CookieRefresh != time.Duration(0)) { + if o.PassAccessToken || o.SetAuthorization || o.PassAuthorization || (o.Cookie.Refresh != time.Duration(0)) { validCookieSecretSize := false for _, i := range []int{16, 24, 32} { - if len(secretBytes(o.CookieSecret)) == i { + if len(secretBytes(o.Cookie.Secret)) == i { validCookieSecretSize = true } } var decoded bool - if string(secretBytes(o.CookieSecret)) != o.CookieSecret { + if string(secretBytes(o.Cookie.Secret)) != o.Cookie.Secret { decoded = true } if !validCookieSecretSize { var suffix string if decoded { - suffix = fmt.Sprintf(" note: cookie secret was base64 decoded from %q", o.CookieSecret) + suffix = fmt.Sprintf(" note: cookie secret was base64 decoded from %q", o.Cookie.Secret) } msgs = append(msgs, fmt.Sprintf( "cookie_secret must be 16, 24, or 32 bytes "+ "to create an AES cipher when "+ "pass_access_token == true or "+ "cookie_refresh != 0, but is %d bytes.%s", - len(secretBytes(o.CookieSecret)), suffix)) + len(secretBytes(o.Cookie.Secret)), suffix)) } else { var err error - cipher, err = encryption.NewCipher(secretBytes(o.CookieSecret)) + cipher, err = encryption.NewCipher(secretBytes(o.Cookie.Secret)) if err != nil { msgs = append(msgs, fmt.Sprintf("cookie-secret error: %v", err)) } @@ -414,19 +413,19 @@ func (o *Options) Validate() error { } o.SessionOptions.Cipher = cipher - sessionStore, err := sessions.NewSessionStore(&o.SessionOptions, &o.CookieOptions) + sessionStore, err := sessions.NewSessionStore(&o.SessionOptions, &o.Cookie) if err != nil { msgs = append(msgs, fmt.Sprintf("error initialising session storage: %v", err)) } else { o.sessionStore = sessionStore } - if o.CookieRefresh >= o.CookieExpire { + if o.Cookie.Refresh >= o.Cookie.Expire { msgs = append(msgs, fmt.Sprintf( "cookie_refresh (%s) must be less than "+ "cookie_expire (%s)", - o.CookieRefresh.String(), - o.CookieExpire.String())) + o.Cookie.Refresh.String(), + o.Cookie.Expire.String())) } if len(o.GoogleGroups) > 0 || o.GoogleAdminEmail != "" || o.GoogleServiceAccountJSON != "" { @@ -441,16 +440,16 @@ func (o *Options) Validate() error { } } - switch o.CookieSameSite { + switch o.Cookie.SameSite { case "", "none", "lax", "strict": default: - msgs = append(msgs, fmt.Sprintf("cookie_samesite (%s) must be one of ['', 'lax', 'strict', 'none']", o.CookieSameSite)) + msgs = append(msgs, fmt.Sprintf("cookie_samesite (%s) must be one of ['', 'lax', 'strict', 'none']", o.Cookie.SameSite)) } // Sort cookie domains by length, so that we try longer (and more specific) // domains first - sort.Slice(o.CookieDomains, func(i, j int) bool { - return len(o.CookieDomains[i]) > len(o.CookieDomains[j]) + sort.Slice(o.Cookie.Domains, func(i, j int) bool { + return len(o.Cookie.Domains[i]) > len(o.Cookie.Domains[j]) }) msgs = parseSignatureKey(o, msgs) @@ -627,9 +626,9 @@ func newVerifierFromJwtIssuer(jwtIssuer jwtIssuer) (*oidc.IDTokenVerifier, error } func validateCookieName(o *Options, msgs []string) []string { - cookie := &http.Cookie{Name: o.CookieName} + cookie := &http.Cookie{Name: o.Cookie.Name} if cookie.String() == "" { - return append(msgs, fmt.Sprintf("invalid cookie name: %q", o.CookieName)) + return append(msgs, fmt.Sprintf("invalid cookie name: %q", o.Cookie.Name)) } return msgs } diff --git a/options_test.go b/options_test.go index a6bbc9ce..42a96895 100644 --- a/options_test.go +++ b/options_test.go @@ -22,7 +22,7 @@ const ( func testOptions() *Options { o := NewOptions() o.Upstreams = append(o.Upstreams, "http://127.0.0.1:8080/") - o.CookieSecret = cookieSecret + o.Cookie.Secret = cookieSecret o.ClientID = clientID o.ClientSecret = clientSecret o.EmailDomains = []string{"*"} @@ -51,7 +51,7 @@ func TestNewOptions(t *testing.T) { func TestClientSecretFileOptionFails(t *testing.T) { o := NewOptions() - o.CookieSecret = cookieSecret + o.Cookie.Secret = cookieSecret o.ClientID = clientID o.ClientSecretFile = clientSecret o.EmailDomains = []string{"*"} @@ -81,7 +81,7 @@ func TestClientSecretFileOption(t *testing.T) { defer os.Remove(clientSecretFileName) o := NewOptions() - o.CookieSecret = cookieSecret + o.Cookie.Secret = cookieSecret o.ClientID = clientID o.ClientSecretFile = clientSecretFileName o.EmailDomains = []string{"*"} @@ -212,20 +212,20 @@ func TestPassAccessTokenRequiresSpecificCookieSecretLengths(t *testing.T) { assert.Equal(t, false, o.PassAccessToken) o.PassAccessToken = true - o.CookieSecret = "cookie of invalid length-" + o.Cookie.Secret = "cookie of invalid length-" assert.NotEqual(t, nil, o.Validate()) o.PassAccessToken = false - o.CookieRefresh = time.Duration(24) * time.Hour + o.Cookie.Refresh = time.Duration(24) * time.Hour assert.NotEqual(t, nil, o.Validate()) - o.CookieSecret = "16 bytes AES-128" + o.Cookie.Secret = "16 bytes AES-128" assert.Equal(t, nil, o.Validate()) - o.CookieSecret = "24 byte secret AES-192--" + o.Cookie.Secret = "24 byte secret AES-192--" assert.Equal(t, nil, o.Validate()) - o.CookieSecret = "32 byte secret for AES-256------" + o.Cookie.Secret = "32 byte secret for AES-256------" assert.Equal(t, nil, o.Validate()) } @@ -233,11 +233,11 @@ func TestCookieRefreshMustBeLessThanCookieExpire(t *testing.T) { o := testOptions() assert.Equal(t, nil, o.Validate()) - o.CookieSecret = "0123456789abcdefabcd" - o.CookieRefresh = o.CookieExpire + o.Cookie.Secret = "0123456789abcdefabcd" + o.Cookie.Refresh = o.Cookie.Expire assert.NotEqual(t, nil, o.Validate()) - o.CookieRefresh -= time.Duration(1) + o.Cookie.Refresh -= time.Duration(1) assert.Equal(t, nil, o.Validate()) } @@ -246,23 +246,23 @@ func TestBase64CookieSecret(t *testing.T) { assert.Equal(t, nil, o.Validate()) // 32 byte, base64 (urlsafe) encoded key - o.CookieSecret = "yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ=" + o.Cookie.Secret = "yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ=" assert.Equal(t, nil, o.Validate()) // 32 byte, base64 (urlsafe) encoded key, w/o padding - o.CookieSecret = "yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ" + o.Cookie.Secret = "yHBw2lh2Cvo6aI_jn_qMTr-pRAjtq0nzVgDJNb36jgQ" assert.Equal(t, nil, o.Validate()) // 24 byte, base64 (urlsafe) encoded key - o.CookieSecret = "Kp33Gj-GQmYtz4zZUyUDdqQKx5_Hgkv3" + o.Cookie.Secret = "Kp33Gj-GQmYtz4zZUyUDdqQKx5_Hgkv3" assert.Equal(t, nil, o.Validate()) // 16 byte, base64 (urlsafe) encoded key - o.CookieSecret = "LFEqZYvYUwKwzn0tEuTpLA==" + o.Cookie.Secret = "LFEqZYvYUwKwzn0tEuTpLA==" assert.Equal(t, nil, o.Validate()) // 16 byte, base64 (urlsafe) encoded key, w/o padding - o.CookieSecret = "LFEqZYvYUwKwzn0tEuTpLA" + o.Cookie.Secret = "LFEqZYvYUwKwzn0tEuTpLA" assert.Equal(t, nil, o.Validate()) } @@ -292,16 +292,16 @@ func TestValidateSignatureKeyUnsupportedAlgorithm(t *testing.T) { func TestValidateCookie(t *testing.T) { o := testOptions() - o.CookieName = "_valid_cookie_name" + o.Cookie.Name = "_valid_cookie_name" assert.Equal(t, nil, o.Validate()) } func TestValidateCookieBadName(t *testing.T) { o := testOptions() - o.CookieName = "_bad_cookie_name{}" + o.Cookie.Name = "_bad_cookie_name{}" err := o.Validate() assert.Equal(t, err.Error(), "invalid configuration:\n"+ - fmt.Sprintf(" invalid cookie name: %q", o.CookieName)) + fmt.Sprintf(" invalid cookie name: %q", o.Cookie.Name)) } func TestSkipOIDCDiscovery(t *testing.T) { diff --git a/pkg/apis/options/cookie.go b/pkg/apis/options/cookie.go index 4d267731..2cb77eb6 100644 --- a/pkg/apis/options/cookie.go +++ b/pkg/apis/options/cookie.go @@ -4,13 +4,13 @@ import "time" // CookieOptions contains configuration options relating to Cookie configuration type CookieOptions struct { - CookieName string `flag:"cookie-name" cfg:"cookie_name" env:"OAUTH2_PROXY_COOKIE_NAME"` - CookieSecret string `flag:"cookie-secret" cfg:"cookie_secret" env:"OAUTH2_PROXY_COOKIE_SECRET"` - CookieDomains []string `flag:"cookie-domain" cfg:"cookie_domain" env:"OAUTH2_PROXY_COOKIE_DOMAIN"` - CookiePath string `flag:"cookie-path" cfg:"cookie_path" env:"OAUTH2_PROXY_COOKIE_PATH"` - CookieExpire time.Duration `flag:"cookie-expire" cfg:"cookie_expire" env:"OAUTH2_PROXY_COOKIE_EXPIRE"` - CookieRefresh time.Duration `flag:"cookie-refresh" cfg:"cookie_refresh" env:"OAUTH2_PROXY_COOKIE_REFRESH"` - CookieSecure bool `flag:"cookie-secure" cfg:"cookie_secure" env:"OAUTH2_PROXY_COOKIE_SECURE"` - CookieHTTPOnly bool `flag:"cookie-httponly" cfg:"cookie_httponly" env:"OAUTH2_PROXY_COOKIE_HTTPONLY"` - CookieSameSite string `flag:"cookie-samesite" cfg:"cookie_samesite" env:"OAUTH2_PROXY_COOKIE_SAMESITE"` + Name string `flag:"cookie-name" cfg:"cookie_name" env:"OAUTH2_PROXY_COOKIE_NAME"` + Secret string `flag:"cookie-secret" cfg:"cookie_secret" env:"OAUTH2_PROXY_COOKIE_SECRET"` + Domains []string `flag:"cookie-domain" cfg:"cookie_domain" env:"OAUTH2_PROXY_COOKIE_DOMAIN"` + Path string `flag:"cookie-path" cfg:"cookie_path" env:"OAUTH2_PROXY_COOKIE_PATH"` + Expire time.Duration `flag:"cookie-expire" cfg:"cookie_expire" env:"OAUTH2_PROXY_COOKIE_EXPIRE"` + Refresh time.Duration `flag:"cookie-refresh" cfg:"cookie_refresh" env:"OAUTH2_PROXY_COOKIE_REFRESH"` + Secure bool `flag:"cookie-secure" cfg:"cookie_secure" env:"OAUTH2_PROXY_COOKIE_SECURE"` + HTTPOnly bool `flag:"cookie-httponly" cfg:"cookie_httponly" env:"OAUTH2_PROXY_COOKIE_HTTPONLY"` + SameSite string `flag:"cookie-samesite" cfg:"cookie_samesite" env:"OAUTH2_PROXY_COOKIE_SAMESITE"` } diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index 7e31464e..5235bb66 100644 --- a/pkg/cookies/cookies.go +++ b/pkg/cookies/cookies.go @@ -38,19 +38,19 @@ func MakeCookie(req *http.Request, name string, value string, path string, domai // MakeCookieFromOptions constructs a cookie based on the given *options.CookieOptions, // value and creation time -func MakeCookieFromOptions(req *http.Request, name string, value string, opts *options.CookieOptions, expiration time.Duration, now time.Time) *http.Cookie { - domain := GetCookieDomain(req, opts.CookieDomains) +func MakeCookieFromOptions(req *http.Request, name string, value string, cookieOpts *options.CookieOptions, expiration time.Duration, now time.Time) *http.Cookie { + domain := GetCookieDomain(req, cookieOpts.Domains) if domain != "" { - return MakeCookie(req, name, value, opts.CookiePath, domain, opts.CookieHTTPOnly, opts.CookieSecure, expiration, now, ParseSameSite(opts.CookieSameSite)) + return MakeCookie(req, name, value, cookieOpts.Path, domain, cookieOpts.HTTPOnly, cookieOpts.Secure, expiration, now, ParseSameSite(cookieOpts.SameSite)) } // If nothing matches, create the cookie with the shortest domain - logger.Printf("Warning: request host %q did not match any of the specific cookie domains of %q", GetRequestHost(req), strings.Join(opts.CookieDomains, ",")) + logger.Printf("Warning: request host %q did not match any of the specific cookie domains of %q", GetRequestHost(req), strings.Join(cookieOpts.Domains, ",")) defaultDomain := "" - if len(opts.CookieDomains) > 0 { - defaultDomain = opts.CookieDomains[len(opts.CookieDomains)-1] + if len(cookieOpts.Domains) > 0 { + defaultDomain = cookieOpts.Domains[len(cookieOpts.Domains)-1] } - return MakeCookie(req, name, value, opts.CookiePath, defaultDomain, opts.CookieHTTPOnly, opts.CookieSecure, expiration, now, ParseSameSite(opts.CookieSameSite)) + return MakeCookie(req, name, value, cookieOpts.Path, defaultDomain, cookieOpts.HTTPOnly, cookieOpts.Secure, expiration, now, ParseSameSite(cookieOpts.SameSite)) } // GetCookieDomain returns the correct cookie domain given a list of domains diff --git a/pkg/sessions/cookie/session_store.go b/pkg/sessions/cookie/session_store.go index 1ef0134e..a01d8050 100644 --- a/pkg/sessions/cookie/session_store.go +++ b/pkg/sessions/cookie/session_store.go @@ -49,12 +49,12 @@ func (s *SessionStore) Save(rw http.ResponseWriter, req *http.Request, ss *sessi // Load reads sessions.SessionState information from Cookies within the // HTTP request object func (s *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) { - c, err := loadCookie(req, s.CookieOptions.CookieName) + c, err := loadCookie(req, s.CookieOptions.Name) if err != nil { // always http.ErrNoCookie - return nil, fmt.Errorf("cookie %q not present", s.CookieOptions.CookieName) + return nil, fmt.Errorf("cookie %q not present", s.CookieOptions.Name) } - val, _, ok := encryption.Validate(c, s.CookieOptions.CookieSecret, s.CookieOptions.CookieExpire) + val, _, ok := encryption.Validate(c, s.CookieOptions.Secret, s.CookieOptions.Expire) if !ok { return nil, errors.New("cookie signature not valid") } @@ -70,7 +70,7 @@ func (s *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) { // clear the session func (s *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error { // matches CookieName, CookieName_ - var cookieNameRegex = regexp.MustCompile(fmt.Sprintf("^%s(_\\d+)?$", s.CookieOptions.CookieName)) + var cookieNameRegex = regexp.MustCompile(fmt.Sprintf("^%s(_\\d+)?$", s.CookieOptions.Name)) for _, c := range req.Cookies() { if cookieNameRegex.MatchString(c.Name) { @@ -94,10 +94,10 @@ func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Reques // authentication details func (s *SessionStore) makeSessionCookie(req *http.Request, value string, now time.Time) []*http.Cookie { if value != "" { - value = encryption.SignedValue(s.CookieOptions.CookieSecret, s.CookieOptions.CookieName, value, now) + value = encryption.SignedValue(s.CookieOptions.Secret, s.CookieOptions.Name, value, now) } - c := s.makeCookie(req, s.CookieOptions.CookieName, value, s.CookieOptions.CookieExpire, now) - if len(c.Value) > 4096-len(s.CookieOptions.CookieName) { + c := s.makeCookie(req, s.CookieOptions.Name, value, s.CookieOptions.Expire, now) + if len(c.Value) > 4096-len(s.CookieOptions.Name) { return splitCookie(c) } return []*http.Cookie{c} diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go index 4d6ce9cf..28eff8c4 100644 --- a/pkg/sessions/redis/redis_store.go +++ b/pkg/sessions/redis/redis_store.go @@ -117,13 +117,13 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se // Old sessions that we are refreshing would have a request cookie // New sessions don't, so we ignore the error. storeValue will check requestCookie - requestCookie, _ := req.Cookie(store.CookieOptions.CookieName) + requestCookie, _ := req.Cookie(store.CookieOptions.Name) value, err := s.EncodeSessionState(store.CookieCipher) if err != nil { return err } ctx := req.Context() - ticketString, err := store.storeValue(ctx, value, store.CookieOptions.CookieExpire, requestCookie) + ticketString, err := store.storeValue(ctx, value, store.CookieOptions.Expire, requestCookie) if err != nil { return err } @@ -131,7 +131,7 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se ticketCookie := store.makeCookie( req, ticketString, - store.CookieOptions.CookieExpire, + store.CookieOptions.Expire, s.CreatedAt, ) @@ -142,12 +142,12 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se // Load reads sessions.SessionState information from a ticket // cookie within the HTTP request object func (store *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) { - requestCookie, err := req.Cookie(store.CookieOptions.CookieName) + requestCookie, err := req.Cookie(store.CookieOptions.Name) if err != nil { return nil, fmt.Errorf("error loading session: %s", err) } - val, _, ok := encryption.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire) + val, _, ok := encryption.Validate(requestCookie, store.CookieOptions.Secret, store.CookieOptions.Expire) if !ok { return nil, fmt.Errorf("cookie signature not valid") } @@ -161,12 +161,12 @@ func (store *SessionStore) Load(req *http.Request) (*sessions.SessionState, erro // loadSessionFromString loads the session based on the ticket value func (store *SessionStore) loadSessionFromString(ctx context.Context, value string) (*sessions.SessionState, error) { - ticket, err := decodeTicket(store.CookieOptions.CookieName, value) + ticket, err := decodeTicket(store.CookieOptions.Name, value) if err != nil { return nil, err } - resultBytes, err := store.Client.Get(ctx, ticket.asHandle(store.CookieOptions.CookieName)) + resultBytes, err := store.Client.Get(ctx, ticket.asHandle(store.CookieOptions.Name)) if err != nil { return nil, err } @@ -199,7 +199,7 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro http.SetCookie(rw, clearCookie) // If there was an existing cookie we should clear the session in redis - requestCookie, err := req.Cookie(store.CookieOptions.CookieName) + requestCookie, err := req.Cookie(store.CookieOptions.Name) if err != nil && err == http.ErrNoCookie { // No existing cookie so can't clear redis return nil @@ -207,17 +207,17 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro return fmt.Errorf("error retrieving cookie: %v", err) } - val, _, ok := encryption.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire) + val, _, ok := encryption.Validate(requestCookie, store.CookieOptions.Secret, store.CookieOptions.Expire) if !ok { return fmt.Errorf("cookie signature not valid") } // We only return an error if we had an issue with redis // If there's an issue decoding the ticket, ignore it - ticket, _ := decodeTicket(store.CookieOptions.CookieName, val) + ticket, _ := decodeTicket(store.CookieOptions.Name, val) if ticket != nil { ctx := req.Context() - err := store.Client.Del(ctx, ticket.asHandle(store.CookieOptions.CookieName)) + err := store.Client.Del(ctx, ticket.asHandle(store.CookieOptions.Name)) if err != nil { return fmt.Errorf("error clearing cookie from redis: %s", err) } @@ -228,11 +228,11 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro // makeCookie makes a cookie, signing the value if present func (store *SessionStore) makeCookie(req *http.Request, value string, expires time.Duration, now time.Time) *http.Cookie { if value != "" { - value = encryption.SignedValue(store.CookieOptions.CookieSecret, store.CookieOptions.CookieName, value, now) + value = encryption.SignedValue(store.CookieOptions.Secret, store.CookieOptions.Name, value, now) } return cookies.MakeCookieFromOptions( req, - store.CookieOptions.CookieName, + store.CookieOptions.Name, value, store.CookieOptions, expires, @@ -256,12 +256,12 @@ func (store *SessionStore) storeValue(ctx context.Context, value string, expirat stream := cipher.NewCFBEncrypter(block, ticket.Secret) stream.XORKeyStream(ciphertext, []byte(value)) - handle := ticket.asHandle(store.CookieOptions.CookieName) + handle := ticket.asHandle(store.CookieOptions.Name) err = store.Client.Set(ctx, handle, ciphertext, expiration) if err != nil { return "", err } - return ticket.encodeTicket(store.CookieOptions.CookieName), nil + return ticket.encodeTicket(store.CookieOptions.Name), nil } // getTicket retrieves an existing ticket from the cookie if present, @@ -272,14 +272,14 @@ func (store *SessionStore) getTicket(requestCookie *http.Cookie) (*TicketData, e } // An existing cookie exists, try to retrieve the ticket - val, _, ok := encryption.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire) + val, _, ok := encryption.Validate(requestCookie, store.CookieOptions.Secret, store.CookieOptions.Expire) if !ok { // Cookie is invalid, create a new ticket return newTicket() } // Valid cookie, decode the ticket - ticket, err := decodeTicket(store.CookieOptions.CookieName, val) + ticket, err := decodeTicket(store.CookieOptions.Name, val) if err != nil { // If we can't decode the ticket we have to create a new one return newTicket() diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go index 14707a2c..69ccf7db 100644 --- a/pkg/sessions/session_store_test.go +++ b/pkg/sessions/session_store_test.go @@ -47,25 +47,25 @@ var _ = Describe("NewSessionStore", func() { It("have the correct name set", func() { if len(cookies) == 1 { - Expect(cookies[0].Name).To(Equal(cookieOpts.CookieName)) + Expect(cookies[0].Name).To(Equal(cookieOpts.Name)) } else { for _, cookie := range cookies { - Expect(cookie.Name).To(ContainSubstring(cookieOpts.CookieName)) + Expect(cookie.Name).To(ContainSubstring(cookieOpts.Name)) } } }) It("have the correct path set", func() { for _, cookie := range cookies { - Expect(cookie.Path).To(Equal(cookieOpts.CookiePath)) + Expect(cookie.Path).To(Equal(cookieOpts.Path)) } }) It("have the correct domain set", func() { for _, cookie := range cookies { specifiedDomain := "" - if len(cookieOpts.CookieDomains) > 0 { - specifiedDomain = cookieOpts.CookieDomains[0] + if len(cookieOpts.Domains) > 0 { + specifiedDomain = cookieOpts.Domains[0] } Expect(cookie.Domain).To(Equal(specifiedDomain)) } @@ -73,19 +73,19 @@ var _ = Describe("NewSessionStore", func() { It("have the correct HTTPOnly set", func() { for _, cookie := range cookies { - Expect(cookie.HttpOnly).To(Equal(cookieOpts.CookieHTTPOnly)) + Expect(cookie.HttpOnly).To(Equal(cookieOpts.HTTPOnly)) } }) It("have the correct secure set", func() { for _, cookie := range cookies { - Expect(cookie.Secure).To(Equal(cookieOpts.CookieSecure)) + Expect(cookie.Secure).To(Equal(cookieOpts.Secure)) } }) It("have the correct SameSite set", func() { for _, cookie := range cookies { - Expect(cookie.SameSite).To(Equal(cookiesapi.ParseSameSite(cookieOpts.CookieSameSite))) + Expect(cookie.SameSite).To(Equal(cookiesapi.ParseSameSite(cookieOpts.SameSite))) } }) @@ -168,8 +168,8 @@ var _ = Describe("NewSessionStore", func() { BeforeEach(func() { By("Using a valid cookie with a different providers session encoding") broken := "BrokenSessionFromADifferentSessionImplementation" - value := encryption.SignedValue(cookieOpts.CookieSecret, cookieOpts.CookieName, broken, time.Now()) - cookie := cookiesapi.MakeCookieFromOptions(request, cookieOpts.CookieName, value, cookieOpts, cookieOpts.CookieExpire, time.Now()) + value := encryption.SignedValue(cookieOpts.Secret, cookieOpts.Name, broken, time.Now()) + cookie := cookiesapi.MakeCookieFromOptions(request, cookieOpts.Name, value, cookieOpts, cookieOpts.Expire, time.Now()) request.AddCookie(cookie) err := ss.Save(response, request, session) @@ -245,7 +245,7 @@ var _ = Describe("NewSessionStore", func() { }) It("loads a session equal to the original session", func() { - if cookieOpts.CookieSecret == "" { + if cookieOpts.Secret == "" { // Only Email and User stored in session when encrypted Expect(loadedSession.Email).To(Equal(session.Email)) Expect(loadedSession.User).To(Equal(session.User)) @@ -290,7 +290,7 @@ var _ = Describe("NewSessionStore", func() { BeforeEach(func() { switch ss.(type) { case *redis.SessionStore: - mr.FastForward(cookieOpts.CookieRefresh + time.Minute) + mr.FastForward(cookieOpts.Refresh + time.Minute) } }) @@ -304,7 +304,7 @@ var _ = Describe("NewSessionStore", func() { BeforeEach(func() { switch ss.(type) { case *redis.SessionStore: - mr.FastForward(cookieOpts.CookieExpire + time.Minute) + mr.FastForward(cookieOpts.Expire + time.Minute) } loadedSession, err = ss.Load(request) @@ -341,14 +341,14 @@ var _ = Describe("NewSessionStore", func() { Context("with non-default options", func() { BeforeEach(func() { cookieOpts = &options.CookieOptions{ - CookieName: "_cookie_name", - CookiePath: "/path", - CookieExpire: time.Duration(72) * time.Hour, - CookieRefresh: time.Duration(2) * time.Hour, - CookieSecure: false, - CookieHTTPOnly: false, - CookieDomains: []string{"example.com"}, - CookieSameSite: "strict", + Name: "_cookie_name", + Path: "/path", + Expire: time.Duration(72) * time.Hour, + Refresh: time.Duration(2) * time.Hour, + Secure: false, + HTTPOnly: false, + Domains: []string{"example.com"}, + SameSite: "strict", } var err error @@ -364,8 +364,8 @@ var _ = Describe("NewSessionStore", func() { secret := make([]byte, 32) _, err := rand.Read(secret) Expect(err).ToNot(HaveOccurred()) - cookieOpts.CookieSecret = base64.URLEncoding.EncodeToString(secret) - cipher, err := encryption.NewCipher(utils.SecretBytes(cookieOpts.CookieSecret)) + cookieOpts.Secret = base64.URLEncoding.EncodeToString(secret) + cipher, err := encryption.NewCipher(utils.SecretBytes(cookieOpts.Secret)) Expect(err).ToNot(HaveOccurred()) Expect(cipher).ToNot(BeNil()) opts.Cipher = cipher @@ -384,13 +384,13 @@ var _ = Describe("NewSessionStore", func() { // Set default options in CookieOptions cookieOpts = &options.CookieOptions{ - CookieName: "_oauth2_proxy", - CookiePath: "/", - CookieExpire: time.Duration(168) * time.Hour, - CookieRefresh: time.Duration(1) * time.Hour, - CookieSecure: true, - CookieHTTPOnly: true, - CookieSameSite: "", + Name: "_oauth2_proxy", + Path: "/", + Expire: time.Duration(168) * time.Hour, + Refresh: time.Duration(1) * time.Hour, + Secure: true, + HTTPOnly: true, + SameSite: "", } session = &sessionsapi.SessionState{