diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f6301e9ae150dbd38f2648f7f2bff270b7bb258..e2a026b0dbce4d3482625b165c4d77be8bb531bb 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 058b90c6ce4538ec3697c4c7c3faf87d0f7ebf10..51a888f4ef6cd23e04faf4523c10f83c9d3d8ca2 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 eb72a83ef442080dbf9089f3621117f448282884..c8ee75c274aa49e497c7bd90577b2a977d73127c 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 1e9bb7cafe873fa438a79e80b78f7289cfdfcb7c..516340c51eba15ae9477c62229aa1cd18e735441 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 47b277437f9381b02434b58c8ba145461e8e65ab..50df087259efc8af57aebf207a3bb98fe9c17adc 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 50dee28b85495b8fddf0aacfdce04d750f223e8d..8b3e461b2aea042152d1ec02a008a194aafebe2d 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 a6bbc9ce1d8e0cbb93c301a6d5787d1d1ddc7cf8..42a96895874a87902b019c4b292538957ab0d491 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 4d26773145138d853881a720a07a740c62b893cd..2cb77eb61afffd80fb1269660201f6f6d7ff4f6f 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 7e31464eeb910bc045a113debf68000a134d0ccc..5235bb66ced784918a34415a4b61c1ee6fbed870 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 1ef0134e4c950aff3fbcc7a2e6fefea6de3599fc..a01d8050541c3f5b4cd38da3dd907e69ce8eca9b 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_<number> - 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 4d6ce9cf6b8dccf3f642d3ca93549225484af1f5..28eff8c48d62b025736e61735e0bae98830a701c 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 14707a2c960080f68a1e9bfa2d9d96e8db65383e..69ccf7db7b17083d5117449aac459c3fc7a6d679 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{