From 51e80f24ef25252d65476cf33e31242e43919af9 Mon Sep 17 00:00:00 2001 From: stagswtf <142280349+stagswtf@users.noreply.github.com> Date: Tue, 28 Oct 2025 00:37:25 -0700 Subject: [PATCH 1/6] fix: use GetSecret() in ticket.go makeCookie to respect cookie-secret-file (#3228) * fix: use GetSecret() in ticket.go makeCookie The makeCookie method in ticket.go was using t.options.Secret directly, which meant cookie-secret-file was not being respected. Updated to use GetSecret() which handles both cookie-secret and cookie-secret-file properly. Also added test coverage for cookie-secret-file functionality. Fixes #3224 Signed-off-by: stagswtf <142280349+stagswtf@users.noreply.github.com> * docs: update CHANGELOG.md for cookie-secret-file fix Signed-off-by: stagswtf <142280349+stagswtf@users.noreply.github.com> * correct PR link and undo file formatting Signed-off-by: stagswtf <142280349+stagswtf@users.noreply.github.com> * fix: error wrapping Signed-off-by: Jan Larwig --------- Signed-off-by: stagswtf <142280349+stagswtf@users.noreply.github.com> Signed-off-by: Jan Larwig Co-authored-by: Jan Larwig --- CHANGELOG.md | 4 ++- pkg/sessions/persistence/ticket.go | 11 +++++-- pkg/sessions/tests/session_store_tests.go | 37 +++++++++++++++++++++++ 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2c9b441..5aefaafa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.12.0 +- [#3228](https://github.com/oauth2-proxy/oauth2-proxy/pull/3228) fix: use GetSecret() in ticket.go makeCookie to respect cookie-secret-file (@stagswtf) + # V7.12.0 ## Release Highlights @@ -119,7 +121,7 @@ For detailed information, migration guidance, and security implications, see the - 🕵️‍♀️ Vulnerabilities have been addressed - [CVE-2025-22871](https://github.com/advisories/GHSA-g9pc-8g42-g6vq) - 🐛 Squashed some bugs - + ## Important Notes ## Breaking Changes diff --git a/pkg/sessions/persistence/ticket.go b/pkg/sessions/persistence/ticket.go index 7855db45..56d6bd9b 100644 --- a/pkg/sessions/persistence/ticket.go +++ b/pkg/sessions/persistence/ticket.go @@ -233,12 +233,17 @@ func (t *ticket) clearCookie(rw http.ResponseWriter, req *http.Request) { // makeCookie makes a cookie, signing the value if present func (t *ticket) makeCookie(req *http.Request, value string, expires time.Duration, now time.Time) (*http.Cookie, error) { if value != "" { - var err error - value, err = encryption.SignedValue(t.options.Secret, t.options.Name, []byte(value), now) + secret, err := t.options.GetSecret() if err != nil { - return nil, err + return nil, fmt.Errorf("retrieving secret failed: %w", err) + } + + value, err = encryption.SignedValue(secret, t.options.Name, []byte(value), now) + if err != nil { + return nil, fmt.Errorf("signing cookie value failed: %w", err) } } + return cookies.MakeCookieFromOptions( req, t.options.Name, diff --git a/pkg/sessions/tests/session_store_tests.go b/pkg/sessions/tests/session_store_tests.go index a4818ef2..05b67d8d 100644 --- a/pkg/sessions/tests/session_store_tests.go +++ b/pkg/sessions/tests/session_store_tests.go @@ -4,6 +4,7 @@ import ( "crypto/rand" "net/http" "net/http/httptest" + "os" "strconv" "strings" "time" @@ -133,6 +134,42 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi PersistentSessionStoreInterfaceTests(&input) } }) + + Context("with cookie secret file", func() { + var tmpfile *os.File + var err error + BeforeEach(func() { + tmpfile, err = os.CreateTemp("", "cookie-secret-test") + secretBytes := make([]byte, 32) + tmpfile.Write(secretBytes) + tmpfile.Close() + + input.cookieOpts = &options.Cookie{ + Name: "_oauth2_proxy_file", + Path: "/", + Expire: time.Duration(168) * time.Hour, + Refresh: time.Duration(1) * time.Hour, + Secure: true, + HTTPOnly: true, + SameSite: "", + Secret: "", + SecretFile: tmpfile.Name(), + } + ss, err = newSS(opts, input.cookieOpts) + Expect(err).ToNot(HaveOccurred()) + }) + + AfterEach(func() { + if tmpfile != nil { + os.Remove(tmpfile.Name()) + } + }) + + SessionStoreInterfaceTests(&input) + if persistentFastForward != nil { + PersistentSessionStoreInterfaceTests(&input) + } + }) }) } From ea1dc3f606fa5a5817bc168ecb64c67f792ca25e Mon Sep 17 00:00:00 2001 From: Vincent Privat <146961743+vprivat-ads@users.noreply.github.com> Date: Tue, 28 Oct 2025 08:40:51 +0100 Subject: [PATCH 2/6] Fix typo: diffrerent -> different (#3222) Signed-off-by: Vincent Privat --- docs/docs/configuration/providers/ms_entra_id.md | 2 +- .../version-7.10.x/configuration/providers/ms_entra_id.md | 2 +- .../version-7.11.x/configuration/providers/ms_entra_id.md | 2 +- .../version-7.12.x/configuration/providers/ms_entra_id.md | 2 +- .../version-7.8.x/configuration/providers/ms_entra_id.md | 2 +- .../version-7.9.x/configuration/providers/ms_entra_id.md | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/docs/configuration/providers/ms_entra_id.md b/docs/docs/configuration/providers/ms_entra_id.md index c5d9594e..95fb99bc 100644 --- a/docs/docs/configuration/providers/ms_entra_id.md +++ b/docs/docs/configuration/providers/ms_entra_id.md @@ -112,7 +112,7 @@ insecure_oidc_skip_issuer_verification=true ``` `insecure_oidc_skip_issuer_verification` setting is required to disable following checks: * Startup check for matching issuer URL returned from [discovery document](https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration) with `oidc_issuer_url` setting. Required, as document's `issuer` field doesn't equal to `https://login.microsoftonline.com/common/v2.0`. See [OIDC Discovery 4.3](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationValidation). -* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by diffrerent tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). +* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by different tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). To provide additional security, Entra ID provider performs check on the ID token's `issuer` claim to match the `https://login.microsoftonline.com/{tenant-id}/v2.0` template. diff --git a/docs/versioned_docs/version-7.10.x/configuration/providers/ms_entra_id.md b/docs/versioned_docs/version-7.10.x/configuration/providers/ms_entra_id.md index c5d9594e..95fb99bc 100644 --- a/docs/versioned_docs/version-7.10.x/configuration/providers/ms_entra_id.md +++ b/docs/versioned_docs/version-7.10.x/configuration/providers/ms_entra_id.md @@ -112,7 +112,7 @@ insecure_oidc_skip_issuer_verification=true ``` `insecure_oidc_skip_issuer_verification` setting is required to disable following checks: * Startup check for matching issuer URL returned from [discovery document](https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration) with `oidc_issuer_url` setting. Required, as document's `issuer` field doesn't equal to `https://login.microsoftonline.com/common/v2.0`. See [OIDC Discovery 4.3](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationValidation). -* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by diffrerent tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). +* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by different tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). To provide additional security, Entra ID provider performs check on the ID token's `issuer` claim to match the `https://login.microsoftonline.com/{tenant-id}/v2.0` template. diff --git a/docs/versioned_docs/version-7.11.x/configuration/providers/ms_entra_id.md b/docs/versioned_docs/version-7.11.x/configuration/providers/ms_entra_id.md index c5d9594e..95fb99bc 100644 --- a/docs/versioned_docs/version-7.11.x/configuration/providers/ms_entra_id.md +++ b/docs/versioned_docs/version-7.11.x/configuration/providers/ms_entra_id.md @@ -112,7 +112,7 @@ insecure_oidc_skip_issuer_verification=true ``` `insecure_oidc_skip_issuer_verification` setting is required to disable following checks: * Startup check for matching issuer URL returned from [discovery document](https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration) with `oidc_issuer_url` setting. Required, as document's `issuer` field doesn't equal to `https://login.microsoftonline.com/common/v2.0`. See [OIDC Discovery 4.3](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationValidation). -* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by diffrerent tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). +* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by different tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). To provide additional security, Entra ID provider performs check on the ID token's `issuer` claim to match the `https://login.microsoftonline.com/{tenant-id}/v2.0` template. diff --git a/docs/versioned_docs/version-7.12.x/configuration/providers/ms_entra_id.md b/docs/versioned_docs/version-7.12.x/configuration/providers/ms_entra_id.md index c5d9594e..95fb99bc 100644 --- a/docs/versioned_docs/version-7.12.x/configuration/providers/ms_entra_id.md +++ b/docs/versioned_docs/version-7.12.x/configuration/providers/ms_entra_id.md @@ -112,7 +112,7 @@ insecure_oidc_skip_issuer_verification=true ``` `insecure_oidc_skip_issuer_verification` setting is required to disable following checks: * Startup check for matching issuer URL returned from [discovery document](https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration) with `oidc_issuer_url` setting. Required, as document's `issuer` field doesn't equal to `https://login.microsoftonline.com/common/v2.0`. See [OIDC Discovery 4.3](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationValidation). -* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by diffrerent tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). +* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by different tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). To provide additional security, Entra ID provider performs check on the ID token's `issuer` claim to match the `https://login.microsoftonline.com/{tenant-id}/v2.0` template. diff --git a/docs/versioned_docs/version-7.8.x/configuration/providers/ms_entra_id.md b/docs/versioned_docs/version-7.8.x/configuration/providers/ms_entra_id.md index c5d9594e..95fb99bc 100644 --- a/docs/versioned_docs/version-7.8.x/configuration/providers/ms_entra_id.md +++ b/docs/versioned_docs/version-7.8.x/configuration/providers/ms_entra_id.md @@ -112,7 +112,7 @@ insecure_oidc_skip_issuer_verification=true ``` `insecure_oidc_skip_issuer_verification` setting is required to disable following checks: * Startup check for matching issuer URL returned from [discovery document](https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration) with `oidc_issuer_url` setting. Required, as document's `issuer` field doesn't equal to `https://login.microsoftonline.com/common/v2.0`. See [OIDC Discovery 4.3](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationValidation). -* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by diffrerent tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). +* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by different tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). To provide additional security, Entra ID provider performs check on the ID token's `issuer` claim to match the `https://login.microsoftonline.com/{tenant-id}/v2.0` template. diff --git a/docs/versioned_docs/version-7.9.x/configuration/providers/ms_entra_id.md b/docs/versioned_docs/version-7.9.x/configuration/providers/ms_entra_id.md index c5d9594e..95fb99bc 100644 --- a/docs/versioned_docs/version-7.9.x/configuration/providers/ms_entra_id.md +++ b/docs/versioned_docs/version-7.9.x/configuration/providers/ms_entra_id.md @@ -112,7 +112,7 @@ insecure_oidc_skip_issuer_verification=true ``` `insecure_oidc_skip_issuer_verification` setting is required to disable following checks: * Startup check for matching issuer URL returned from [discovery document](https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration) with `oidc_issuer_url` setting. Required, as document's `issuer` field doesn't equal to `https://login.microsoftonline.com/common/v2.0`. See [OIDC Discovery 4.3](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationValidation). -* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by diffrerent tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). +* Matching ID token's `issuer` claim with `oidc_issuer_url` setting during ID token validation. Required to support tokens issued by different tenants. See [OIDC Core 3.1.3.7](https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation). To provide additional security, Entra ID provider performs check on the ID token's `issuer` claim to match the `https://login.microsoftonline.com/{tenant-id}/v2.0` template. From 31b275f5805ee1d6c62906a81274c21587e70a40 Mon Sep 17 00:00:00 2001 From: Schmitt Paul <100230730+paulsc54@users.noreply.github.com> Date: Tue, 28 Oct 2025 08:48:23 +0100 Subject: [PATCH 3/6] docs: clarify ingress-nginx integration and remove Lua block example (#3202) * docs: clarify ingress-nginx integration and remove Lua block example for oauth2-proxy This PR revises the integration guide for oauth2-proxy with ingress-nginx in Kubernetes: Recommends the minimal configuration: just auth-url and auth-signin annotations. Removes the Lua block example, as it did not work in practice despite following nginx documentation and extensive testing. Clearly states that the official ingress-nginx external auth example is the recommended approach for most users. Notes that advanced Lua/cookie handling is only needed for rare, advanced scenarios. Signed-off-by: Jan Larwig * doc: update 3 latest docs versions Signed-off-by: Jan Larwig --------- Signed-off-by: Jan Larwig Co-authored-by: Jan Larwig --- docs/docs/configuration/integration.md | 20 ++++++------------- .../configuration/integration.md | 20 ++++++------------- .../configuration/integration.md | 20 ++++++------------- .../configuration/integration.md | 20 ++++++------------- 4 files changed, 24 insertions(+), 56 deletions(-) diff --git a/docs/docs/configuration/integration.md b/docs/docs/configuration/integration.md index 05d39281..c57cfa6b 100644 --- a/docs/docs/configuration/integration.md +++ b/docs/docs/configuration/integration.md @@ -77,23 +77,15 @@ server { } ``` -When you use ingress-nginx in Kubernetes, you MUST use `kubernetes/ingress-nginx` (which includes the Lua module) and the following configuration snippet for your `Ingress`. -Variables set with `auth_request_set` are not `set`-able in plain nginx config when the location is processed via `proxy_pass` and then may only be processed by Lua. -Note that `nginxinc/kubernetes-ingress` does not include the Lua module. +When you use ingress-nginx in Kubernetes, you can configure the same behavior with the following annotations on your Ingress resource: ```yaml -nginx.ingress.kubernetes.io/auth-response-headers: Authorization -nginx.ingress.kubernetes.io/auth-signin: https://$host/oauth2/start?rd=$escaped_request_uri -nginx.ingress.kubernetes.io/auth-url: https://$host/oauth2/auth -nginx.ingress.kubernetes.io/configuration-snippet: | - auth_request_set $name_upstream_1 $upstream_cookie_name_1; - - access_by_lua_block { - if ngx.var.name_upstream_1 ~= "" then - ngx.header["Set-Cookie"] = "name_1=" .. ngx.var.name_upstream_1 .. ngx.var.auth_cookie:match("(; .*)") - end - } +nginx.ingress.kubernetes.io/auth-url: "https:///oauth2/auth" +nginx.ingress.kubernetes.io/auth-signin: "https:///oauth2/start?rd=$escaped_request_uri" ``` + +This minimal configuration works for standard authentication flows. Lua/cookie handling is only needed for advanced scenarios (e.g., multi-part cookies, custom session logic). See the official ingress-nginx example: https://kubernetes.github.io/ingress-nginx/examples/auth/oauth-external-auth/. + It is recommended to use `--session-store-type=redis` when expecting large sessions/OIDC tokens (_e.g._ with MS Azure). You have to substitute *name* with the actual cookie name you configured via --cookie-name parameter. If you don't set a custom cookie name the variable should be "$upstream_cookie__oauth2_proxy_1" instead of "$upstream_cookie_name_1" and the new cookie-name should be "_oauth2_proxy_1=" instead of "name_1=". diff --git a/docs/versioned_docs/version-7.10.x/configuration/integration.md b/docs/versioned_docs/version-7.10.x/configuration/integration.md index 05d39281..c57cfa6b 100644 --- a/docs/versioned_docs/version-7.10.x/configuration/integration.md +++ b/docs/versioned_docs/version-7.10.x/configuration/integration.md @@ -77,23 +77,15 @@ server { } ``` -When you use ingress-nginx in Kubernetes, you MUST use `kubernetes/ingress-nginx` (which includes the Lua module) and the following configuration snippet for your `Ingress`. -Variables set with `auth_request_set` are not `set`-able in plain nginx config when the location is processed via `proxy_pass` and then may only be processed by Lua. -Note that `nginxinc/kubernetes-ingress` does not include the Lua module. +When you use ingress-nginx in Kubernetes, you can configure the same behavior with the following annotations on your Ingress resource: ```yaml -nginx.ingress.kubernetes.io/auth-response-headers: Authorization -nginx.ingress.kubernetes.io/auth-signin: https://$host/oauth2/start?rd=$escaped_request_uri -nginx.ingress.kubernetes.io/auth-url: https://$host/oauth2/auth -nginx.ingress.kubernetes.io/configuration-snippet: | - auth_request_set $name_upstream_1 $upstream_cookie_name_1; - - access_by_lua_block { - if ngx.var.name_upstream_1 ~= "" then - ngx.header["Set-Cookie"] = "name_1=" .. ngx.var.name_upstream_1 .. ngx.var.auth_cookie:match("(; .*)") - end - } +nginx.ingress.kubernetes.io/auth-url: "https:///oauth2/auth" +nginx.ingress.kubernetes.io/auth-signin: "https:///oauth2/start?rd=$escaped_request_uri" ``` + +This minimal configuration works for standard authentication flows. Lua/cookie handling is only needed for advanced scenarios (e.g., multi-part cookies, custom session logic). See the official ingress-nginx example: https://kubernetes.github.io/ingress-nginx/examples/auth/oauth-external-auth/. + It is recommended to use `--session-store-type=redis` when expecting large sessions/OIDC tokens (_e.g._ with MS Azure). You have to substitute *name* with the actual cookie name you configured via --cookie-name parameter. If you don't set a custom cookie name the variable should be "$upstream_cookie__oauth2_proxy_1" instead of "$upstream_cookie_name_1" and the new cookie-name should be "_oauth2_proxy_1=" instead of "name_1=". diff --git a/docs/versioned_docs/version-7.11.x/configuration/integration.md b/docs/versioned_docs/version-7.11.x/configuration/integration.md index 05d39281..c57cfa6b 100644 --- a/docs/versioned_docs/version-7.11.x/configuration/integration.md +++ b/docs/versioned_docs/version-7.11.x/configuration/integration.md @@ -77,23 +77,15 @@ server { } ``` -When you use ingress-nginx in Kubernetes, you MUST use `kubernetes/ingress-nginx` (which includes the Lua module) and the following configuration snippet for your `Ingress`. -Variables set with `auth_request_set` are not `set`-able in plain nginx config when the location is processed via `proxy_pass` and then may only be processed by Lua. -Note that `nginxinc/kubernetes-ingress` does not include the Lua module. +When you use ingress-nginx in Kubernetes, you can configure the same behavior with the following annotations on your Ingress resource: ```yaml -nginx.ingress.kubernetes.io/auth-response-headers: Authorization -nginx.ingress.kubernetes.io/auth-signin: https://$host/oauth2/start?rd=$escaped_request_uri -nginx.ingress.kubernetes.io/auth-url: https://$host/oauth2/auth -nginx.ingress.kubernetes.io/configuration-snippet: | - auth_request_set $name_upstream_1 $upstream_cookie_name_1; - - access_by_lua_block { - if ngx.var.name_upstream_1 ~= "" then - ngx.header["Set-Cookie"] = "name_1=" .. ngx.var.name_upstream_1 .. ngx.var.auth_cookie:match("(; .*)") - end - } +nginx.ingress.kubernetes.io/auth-url: "https:///oauth2/auth" +nginx.ingress.kubernetes.io/auth-signin: "https:///oauth2/start?rd=$escaped_request_uri" ``` + +This minimal configuration works for standard authentication flows. Lua/cookie handling is only needed for advanced scenarios (e.g., multi-part cookies, custom session logic). See the official ingress-nginx example: https://kubernetes.github.io/ingress-nginx/examples/auth/oauth-external-auth/. + It is recommended to use `--session-store-type=redis` when expecting large sessions/OIDC tokens (_e.g._ with MS Azure). You have to substitute *name* with the actual cookie name you configured via --cookie-name parameter. If you don't set a custom cookie name the variable should be "$upstream_cookie__oauth2_proxy_1" instead of "$upstream_cookie_name_1" and the new cookie-name should be "_oauth2_proxy_1=" instead of "name_1=". diff --git a/docs/versioned_docs/version-7.12.x/configuration/integration.md b/docs/versioned_docs/version-7.12.x/configuration/integration.md index 05d39281..c57cfa6b 100644 --- a/docs/versioned_docs/version-7.12.x/configuration/integration.md +++ b/docs/versioned_docs/version-7.12.x/configuration/integration.md @@ -77,23 +77,15 @@ server { } ``` -When you use ingress-nginx in Kubernetes, you MUST use `kubernetes/ingress-nginx` (which includes the Lua module) and the following configuration snippet for your `Ingress`. -Variables set with `auth_request_set` are not `set`-able in plain nginx config when the location is processed via `proxy_pass` and then may only be processed by Lua. -Note that `nginxinc/kubernetes-ingress` does not include the Lua module. +When you use ingress-nginx in Kubernetes, you can configure the same behavior with the following annotations on your Ingress resource: ```yaml -nginx.ingress.kubernetes.io/auth-response-headers: Authorization -nginx.ingress.kubernetes.io/auth-signin: https://$host/oauth2/start?rd=$escaped_request_uri -nginx.ingress.kubernetes.io/auth-url: https://$host/oauth2/auth -nginx.ingress.kubernetes.io/configuration-snippet: | - auth_request_set $name_upstream_1 $upstream_cookie_name_1; - - access_by_lua_block { - if ngx.var.name_upstream_1 ~= "" then - ngx.header["Set-Cookie"] = "name_1=" .. ngx.var.name_upstream_1 .. ngx.var.auth_cookie:match("(; .*)") - end - } +nginx.ingress.kubernetes.io/auth-url: "https:///oauth2/auth" +nginx.ingress.kubernetes.io/auth-signin: "https:///oauth2/start?rd=$escaped_request_uri" ``` + +This minimal configuration works for standard authentication flows. Lua/cookie handling is only needed for advanced scenarios (e.g., multi-part cookies, custom session logic). See the official ingress-nginx example: https://kubernetes.github.io/ingress-nginx/examples/auth/oauth-external-auth/. + It is recommended to use `--session-store-type=redis` when expecting large sessions/OIDC tokens (_e.g._ with MS Azure). You have to substitute *name* with the actual cookie name you configured via --cookie-name parameter. If you don't set a custom cookie name the variable should be "$upstream_cookie__oauth2_proxy_1" instead of "$upstream_cookie_name_1" and the new cookie-name should be "_oauth2_proxy_1=" instead of "name_1=". From f950dc99426014f3973b2c9193ee5eecf515e39b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Tue, 28 Oct 2025 09:13:35 +0100 Subject: [PATCH 4/6] feat(makefile): simplify validate-go-version (#3147) Since Go 1.21 the go toolchain validates strictly the "go" version directive in go.mod, and downloads and uses the requested toolchain if necessary. See https://go.dev/doc/toolchain So we can just run "go list" to tell the Go toolchain to validate our build environment according to go.mod. To extract the "go" directive version from go.mod (used to select the Docker build image) we also use "go list". --- Makefile | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/Makefile b/Makefile index 20ed88c9..06471100 100644 --- a/Makefile +++ b/Makefile @@ -36,13 +36,12 @@ REPOSITORY ?= oauth2-proxy DATE := $(shell date +"%Y%m%d") .NOTPARALLEL: -GO_MAJOR_VERSION = $(shell $(GO) version | cut -c 14- | cut -d' ' -f1 | cut -d'.' -f1) -GO_MINOR_VERSION = $(shell $(GO) version | cut -c 14- | cut -d' ' -f1 | cut -d'.' -f2) +# The go version in go.mod used for the Docker build toolchain, without the patch +GO_MOD_VERSION_MINOR := $(shell $(GO) list -f '{{printf "%.4s" .Module.GoVersion}}' ) -GO_MOD_VERSION = $(shell sed -En 's/^go ([[:digit:]]\.[[:digit:]]+)\.[[:digit:]]+/\1/p' go.mod) -MINIMUM_SUPPORTED_GO_MAJOR_VERSION = $(shell echo ${GO_MOD_VERSION} | cut -d' ' -f1 | cut -d'.' -f1) -MINIMUM_SUPPORTED_GO_MINOR_VERSION = $(shell echo ${GO_MOD_VERSION} | cut -d' ' -f1 | cut -d'.' -f2) -GO_VERSION_VALIDATION_ERR_MSG = Golang version is not supported, please update to at least $(MINIMUM_SUPPORTED_GO_MAJOR_VERSION).$(MINIMUM_SUPPORTED_GO_MINOR_VERSION) +# From go1.21 go will transparently download the toolchain declared in go.mod. https://go.dev/doc/toolchain +# We don't need to keep this message updated: the important info is in go.mod. +GO_VERSION_VALIDATION_ERR_MSG = Golang version is not supported, please update to at least go1.21 ifeq ($(COVER),true) TESTCOVER ?= -coverprofile c.out @@ -56,7 +55,7 @@ build: validate-go-version clean $(BINARY) ## Build and create oauth2-proxy bina $(BINARY): CGO_ENABLED=0 $(GO) build -a -installsuffix cgo -ldflags="-X github.com/oauth2-proxy/oauth2-proxy/v7/pkg/version.VERSION=${VERSION}" -o $@ github.com/oauth2-proxy/oauth2-proxy/v7 -DOCKER_BUILDX_COMMON_ARGS ?= --build-arg BUILD_IMAGE=docker.io/library/golang:${GO_MOD_VERSION}-bookworm --build-arg VERSION=${VERSION} +DOCKER_BUILDX_COMMON_ARGS ?= --build-arg BUILD_IMAGE=docker.io/library/golang:$(GO_MOD_VERSION_MINOR)-bookworm --build-arg VERSION=$(VERSION) DOCKER_BUILD_PLATFORM ?= linux/amd64,linux/arm64,linux/ppc64le,linux/arm/v7,linux/s390x DOCKER_BUILD_RUNTIME_IMAGE ?= gcr.io/distroless/static:nonroot @@ -158,15 +157,7 @@ lint: validate-go-version ## Lint all files using golangci-lint .PHONY: validate-go-version validate-go-version: ## Validate Go environment requirements - @if [ $(GO_MAJOR_VERSION) -gt $(MINIMUM_SUPPORTED_GO_MAJOR_VERSION) ]; then \ - exit 0 ;\ - elif [ $(GO_MAJOR_VERSION) -lt $(MINIMUM_SUPPORTED_GO_MAJOR_VERSION) ]; then \ - echo '$(GO_VERSION_VALIDATION_ERR_MSG)';\ - exit 1; \ - elif [ $(GO_MINOR_VERSION) -lt $(MINIMUM_SUPPORTED_GO_MINOR_VERSION) ] ; then \ - echo '$(GO_VERSION_VALIDATION_ERR_MSG)';\ - exit 1; \ - fi + @$(GO) list . >/dev/null || { echo '$(GO_VERSION_VALIDATION_ERR_MSG)'; exit 1; } # local-env can be used to interact with the local development environment # eg: From 8f687e4d0cb416232397b1076f387a23c9622183 Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Tue, 28 Oct 2025 09:54:10 +0100 Subject: [PATCH 5/6] chore(deps): upgrade to latest go1.25.3 (#3244) Signed-off-by: Jan Larwig --- CHANGELOG.md | 1 + go.mod | 2 +- go.sum | 34 ++-------------------------------- 3 files changed, 4 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5aefaafa..c52eedef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ## Changes since v7.12.0 - [#3228](https://github.com/oauth2-proxy/oauth2-proxy/pull/3228) fix: use GetSecret() in ticket.go makeCookie to respect cookie-secret-file (@stagswtf) +- [#3244](https://github.com/oauth2-proxy/oauth2-proxy/pull/3244) chore(deps): upgrade to latest go1.25.3 (@tuunit) # V7.12.0 diff --git a/go.mod b/go.mod index 24f316e4..3aeeda0a 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/oauth2-proxy/oauth2-proxy/v7 -go 1.24.6 +go 1.25.3 require ( cloud.google.com/go/compute/metadata v0.7.0 diff --git a/go.sum b/go.sum index caa8e2a0..2e8db1ef 100644 --- a/go.sum +++ b/go.sum @@ -61,12 +61,8 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= -github.com/go-viper/mapstructure/v2 v2.3.0 h1:27XbWsHIqhbdR5TIC911OfYvgSaW93HM+dX7970Q7jk= -github.com/go-viper/mapstructure/v2 v2.3.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= -github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8= -github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang-jwt/jwt/v5 v5.2.3 h1:kkGXqQOBSDDWRhWNXTFpqGSCMyh/PLnqUvMGJPDJDs0= github.com/golang-jwt/jwt/v5 v5.2.3/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= @@ -85,8 +81,6 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/enterprise-certificate-proxy v0.3.6 h1:GW/XbdyBFQ8Qe+YAmFU9uHLo7OnF5tL52HFAgMmyrf4= github.com/googleapis/enterprise-certificate-proxy v0.3.6/go.mod h1:MkHOF77EYAE7qfSuSS9PU6g4Nt4e11cnsDUowfwewLA= -github.com/googleapis/gax-go/v2 v2.14.2 h1:eBLnkZ9635krYIPD+ag1USrOAI0Nr0QYF3+/3GqO0k0= -github.com/googleapis/gax-go/v2 v2.14.2/go.mod h1:ON64QhlJkhVtSqp4v1uaK92VyZ2gmvDQsweuyLV+8+w= github.com/googleapis/gax-go/v2 v2.15.0 h1:SyjDc1mGgZU5LncH8gimWo9lW1DtIfPibOG81vgd/bo= github.com/googleapis/gax-go/v2 v2.15.0/go.mod h1:zVVkkxAQHa1RQpg9z2AUCMnKhi0Qld9rcmyfL1OZhoc= github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= @@ -142,8 +136,6 @@ github.com/spf13/afero v1.14.0 h1:9tH6MapGnn/j0eb0yIXiLjERO8RB6xIVZRDCX7PtqWA= github.com/spf13/afero v1.14.0/go.mod h1:acJQ8t0ohCGuMN3O+Pv0V0hgMxNYDlvdk+VTfyZmbYo= github.com/spf13/cast v1.9.2 h1:SsGfm7M8QOFtEzumm7UZrZdLLquNdzFYfIbEXntcFbE= github.com/spf13/cast v1.9.2/go.mod h1:jNfB8QC9IA6ZuY2ZjDp0KtFO2LZZlg4S/7bzP6qqeHo= -github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= -github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/pflag v1.0.7 h1:vN6T9TfwStFPFM5XzjsvmzZkLuaLX+HS+0SeFLRgU6M= github.com/spf13/pflag v1.0.7/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.20.1 h1:ZMi+z/lvLyPSCoNtFCpqjy0S4kPbirhpTMwl8BkW9X4= @@ -186,8 +178,6 @@ go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN8 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= -golang.org/x/crypto v0.39.0 h1:SHs+kF4LP+f+p14esP5jAoDpHU8Gu/v9lFRK6IT5imM= -golang.org/x/crypto v0.39.0/go.mod h1:L+Xg3Wf6HoL4Bn4238Z6ft6KfEpN0tJGo53AAPC632U= golang.org/x/crypto v0.40.0 h1:r4x+VvoG5Fm+eJcxMaY8CQM7Lb0l1lsmjGBQ6s8BfKM= golang.org/x/crypto v0.40.0/go.mod h1:Qr1vMER5WyS2dfPHAlsOj01wgLbsyWtFn/aY+5+ZdxY= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= @@ -197,8 +187,6 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= -golang.org/x/net v0.41.0 h1:vBTly1HeNPEn3wtREYfy4GZ/NECgw2Cnl+nK6Nz3uvw= -golang.org/x/net v0.41.0/go.mod h1:B/K4NNqkfmg07DQYrbwvSluqCJOOXwUjeb/5lOisjbA= golang.org/x/net v0.42.0 h1:jzkYrhi3YQWD6MLBJcsklgQsoAcw89EcZbJw8Z614hs= golang.org/x/net v0.42.0/go.mod h1:FF1RA5d3u7nAYA4z2TkclSCKh68eSXtiFwcWQpPXdt8= golang.org/x/oauth2 v0.30.0 h1:dnDm7JmhM45NNpd8FDDeLhK6FwqbOf4MLCM9zb1BOHI= @@ -206,8 +194,6 @@ golang.org/x/oauth2 v0.30.0/go.mod h1:B++QgG3ZKulg6sRPGD/mqlHQs5rB3Ml9erfeDY7xKl golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.15.0 h1:KWH3jNZsfyT6xfAfKiz6MRNmd46ByHDYaZ7KSkCtdW8= -golang.org/x/sync v0.15.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw= golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20190204203706-41f3e6584952/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -219,8 +205,6 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= -golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/sys v0.34.0 h1:H5Y5sJ2L2JRdyv7ROF1he/lPdvFsd0mJHFw2ThKHxLA= golang.org/x/sys v0.34.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= @@ -234,8 +218,6 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/text v0.26.0 h1:P42AVeLghgTYr4+xUnTRKDMqpar+PtX7KWuNQL21L8M= -golang.org/x/text v0.26.0/go.mod h1:QK15LZJUUQVJxhz7wXgxSy/CJaTFjd0G+YLonydOVQA= golang.org/x/text v0.27.0 h1:4fGWRpyh641NLlecmyl4LOe6yDdfaYNrGb2zdfo4JV4= golang.org/x/text v0.27.0/go.mod h1:1D28KMCvyooCX9hBiosv5Tz/+YLxj0j7XhWjpSUF7CU= golang.org/x/time v0.12.0 h1:ScB/8o8olJvc+CQPWrK3fPZNfh7qgwCrY0zJmoEQLSE= @@ -244,25 +226,15 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/tools v0.34.0 h1:qIpSLOxeCYGg9TrcJokLBG4KFA6d795g0xkBkiESGlo= -golang.org/x/tools v0.34.0/go.mod h1:pAP9OwEaY1CAW3HOmg3hLZC5Z0CCmzjAF2UQMSqNARg= golang.org/x/tools v0.35.0 h1:mBffYraMEf7aa0sB+NuKnuCy8qI/9Bughn8dC2Gu5r0= golang.org/x/tools v0.35.0/go.mod h1:NKdj5HkL/73byiZSJjqJgKn3ep7KjFkBOkR/Hps3VPw= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -google.golang.org/api v0.240.0 h1:PxG3AA2UIqT1ofIzWV2COM3j3JagKTKSwy7L6RHNXNU= -google.golang.org/api v0.240.0/go.mod h1:cOVEm2TpdAGHL2z+UwyS+kmlGr3bVWQQ6sYEqkKje50= -google.golang.org/api v0.241.0 h1:QKwqWQlkc6O895LchPEDUSYr22Xp3NCxpQRiWTB6avE= -google.golang.org/api v0.241.0/go.mod h1:cOVEm2TpdAGHL2z+UwyS+kmlGr3bVWQQ6sYEqkKje50= google.golang.org/api v0.242.0 h1:7Lnb1nfnpvbkCiZek6IXKdJ0MFuAZNAJKQfA1ws62xg= google.golang.org/api v0.242.0/go.mod h1:cOVEm2TpdAGHL2z+UwyS+kmlGr3bVWQQ6sYEqkKje50= -google.golang.org/genproto v0.0.0-20250505200425-f936aa4a68b2 h1:1tXaIXCracvtsRxSBsYDiSBN0cuJvM7QYW+MrpIRY78= -google.golang.org/genproto v0.0.0-20250505200425-f936aa4a68b2/go.mod h1:49MsLSx0oWMOZqcpB3uL8ZOkAh1+TndpJ8ONoCBWiZk= google.golang.org/genproto v0.0.0-20250603155806-513f23925822 h1:rHWScKit0gvAPuOnu87KpaYtjK5zBMLcULh7gxkCXu4= -google.golang.org/genproto/googleapis/api v0.0.0-20250505200425-f936aa4a68b2 h1:vPV0tzlsK6EzEDHNNH5sa7Hs9bd7iXR7B1tSiPepkV0= -google.golang.org/genproto/googleapis/api v0.0.0-20250505200425-f936aa4a68b2/go.mod h1:pKLAc5OolXC3ViWGI62vvC0n10CpwAtRcTNCFwTKBEw= +google.golang.org/genproto v0.0.0-20250603155806-513f23925822/go.mod h1:HubltRL7rMh0LfnQPkMH4NPDFEWp0jw3vixw7jEM53s= google.golang.org/genproto/googleapis/api v0.0.0-20250603155806-513f23925822 h1:oWVWY3NzT7KJppx2UKhKmzPq4SRe0LdCijVRwvGeikY= -google.golang.org/genproto/googleapis/rpc v0.0.0-20250603155806-513f23925822 h1:fc6jSaCT0vBduLYZHYrBBNY4dsWuvgyff9noRNDdBeE= -google.golang.org/genproto/googleapis/rpc v0.0.0-20250603155806-513f23925822/go.mod h1:qQ0YXyHHx3XkvlzUtpXDkS29lDSafHMZBAZDc03LQ3A= +google.golang.org/genproto/googleapis/api v0.0.0-20250603155806-513f23925822/go.mod h1:h3c4v36UTKzUiuaOKQ6gr3S+0hovBtUrXzTG/i3+XEc= google.golang.org/genproto/googleapis/rpc v0.0.0-20250707201910-8d1bb00bc6a7 h1:pFyd6EwwL2TqFf8emdthzeX+gZE1ElRq3iM8pui4KBY= google.golang.org/genproto/googleapis/rpc v0.0.0-20250707201910-8d1bb00bc6a7/go.mod h1:qQ0YXyHHx3XkvlzUtpXDkS29lDSafHMZBAZDc03LQ3A= google.golang.org/grpc v1.73.0 h1:VIWSmpI2MegBtTuFt5/JWy2oXxtjJ/e89Z70ImfD2ok= @@ -280,7 +252,5 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -k8s.io/apimachinery v0.33.2 h1:IHFVhqg59mb8PJWTLi8m1mAoepkUNYmptHsV+Z1m5jY= -k8s.io/apimachinery v0.33.2/go.mod h1:BHW0YOu7n22fFv/JkYOEfkUYNRN0fj0BlvMFWA7b+SM= k8s.io/apimachinery v0.33.3 h1:4ZSrmNa0c/ZpZJhAgRdcsFcZOw1PQU1bALVQ0B3I5LA= k8s.io/apimachinery v0.33.3/go.mod h1:BHW0YOu7n22fFv/JkYOEfkUYNRN0fj0BlvMFWA7b+SM= From 110d51d1d742b5cde9cb629d6912f47e5d9ab878 Mon Sep 17 00:00:00 2001 From: David Symonds Date: Tue, 28 Oct 2025 20:05:02 +1100 Subject: [PATCH 6/6] test: replace mock pkg/clock with narrowly targeted stub clocks. (#3238) The package under pkg/clock is github.com/benbjohnson/clock, which is archived. It's also way more complex than is what is actually needed here, so we can entirely remove the dependency and remove the helper package. Fixes #2840. Signed-off-by: David Symonds --- CHANGELOG.md | 1 + go.mod | 1 - go.sum | 2 - pkg/apis/sessions/session_state.go | 18 +- pkg/apis/sessions/session_state_test.go | 6 +- pkg/clock/clock.go | 157 ---------- pkg/clock/clock_suite_test.go | 17 -- pkg/clock/clock_test.go | 380 ------------------------ pkg/cookies/csrf.go | 16 +- pkg/cookies/csrf_per_request_test.go | 10 +- pkg/cookies/csrf_test.go | 6 +- pkg/middleware/stored_session_test.go | 25 +- providers/provider_default_test.go | 5 +- 13 files changed, 50 insertions(+), 594 deletions(-) delete mode 100644 pkg/clock/clock.go delete mode 100644 pkg/clock/clock_suite_test.go delete mode 100644 pkg/clock/clock_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index c52eedef..e014aee3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - [#3228](https://github.com/oauth2-proxy/oauth2-proxy/pull/3228) fix: use GetSecret() in ticket.go makeCookie to respect cookie-secret-file (@stagswtf) - [#3244](https://github.com/oauth2-proxy/oauth2-proxy/pull/3244) chore(deps): upgrade to latest go1.25.3 (@tuunit) +- [#3238](https://github.com/oauth2-proxy/oauth2-proxy/pull/3238) chore: Replace pkg/clock with narrowly targeted stub clocks (@dsymonds) # V7.12.0 diff --git a/go.mod b/go.mod index 3aeeda0a..0e14464c 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,6 @@ require ( github.com/Bose/minisentinel v0.0.0-20200130220412-917c5a9223bb github.com/a8m/envsubst v1.4.3 github.com/alicebob/miniredis/v2 v2.35.0 - github.com/benbjohnson/clock v1.3.5 github.com/bitly/go-simplejson v0.5.1 github.com/bsm/redislock v0.9.4 github.com/coreos/go-oidc/v3 v3.14.1 diff --git a/go.sum b/go.sum index 2e8db1ef..8bc31660 100644 --- a/go.sum +++ b/go.sum @@ -14,8 +14,6 @@ github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6/go.mod h1:SGn github.com/alicebob/miniredis/v2 v2.11.1/go.mod h1:UA48pmi7aSazcGAvcdKcBB49z521IC9VjTTRz2nIaJE= github.com/alicebob/miniredis/v2 v2.35.0 h1:QwLphYqCEAo1eu1TqPRN2jgVMPBweeQcR21jeqDCONI= github.com/alicebob/miniredis/v2 v2.35.0/go.mod h1:TcL7YfarKPGDAthEtl5NBeHZfeUQj6OXMm/+iu5cLMM= -github.com/benbjohnson/clock v1.3.5 h1:VvXlSJBzZpA/zum6Sj74hxwYI2DIxRWuNIoXAzHZz5o= -github.com/benbjohnson/clock v1.3.5/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bitly/go-simplejson v0.5.1 h1:xgwPbetQScXt1gh9BmoJ6j9JMr3TElvuIyjR8pgdoow= diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go index b5e4fc83..5b063c3f 100644 --- a/pkg/apis/sessions/session_state.go +++ b/pkg/apis/sessions/session_state.go @@ -7,7 +7,6 @@ import ( "io" "time" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/clock" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" "github.com/pierrec/lz4/v4" "github.com/vmihailenco/msgpack/v5" @@ -30,8 +29,15 @@ type SessionState struct { PreferredUsername string `msgpack:"pu,omitempty"` // Internal helpers, not serialized - Clock clock.Clock `msgpack:"-"` - Lock Lock `msgpack:"-"` + Clock func() time.Time `msgpack:"-"` // override for time.Now, for testing + Lock Lock `msgpack:"-"` +} + +func (s *SessionState) now() time.Time { + if s.Clock != nil { + return s.Clock() + } + return time.Now() } func (s *SessionState) ObtainLock(ctx context.Context, expiration time.Duration) error { @@ -64,7 +70,7 @@ func (s *SessionState) PeekLock(ctx context.Context) (bool, error) { // CreatedAtNow sets a SessionState's CreatedAt to now func (s *SessionState) CreatedAtNow() { - now := s.Clock.Now() + now := s.now() s.CreatedAt = &now } @@ -85,7 +91,7 @@ func (s *SessionState) ExpiresIn(d time.Duration) { // IsExpired checks whether the session has expired func (s *SessionState) IsExpired() bool { - if s.ExpiresOn != nil && !s.ExpiresOn.IsZero() && s.ExpiresOn.Before(s.Clock.Now()) { + if s.ExpiresOn != nil && !s.ExpiresOn.IsZero() && s.ExpiresOn.Before(s.now()) { return true } return false @@ -94,7 +100,7 @@ func (s *SessionState) IsExpired() bool { // Age returns the age of a session func (s *SessionState) Age() time.Duration { if s.CreatedAt != nil && !s.CreatedAt.IsZero() { - return s.Clock.Now().Truncate(time.Second).Sub(*s.CreatedAt) + return s.now().Truncate(time.Second).Sub(*s.CreatedAt) } return 0 } diff --git a/pkg/apis/sessions/session_state_test.go b/pkg/apis/sessions/session_state_test.go index e12c2776..442fcea8 100644 --- a/pkg/apis/sessions/session_state_test.go +++ b/pkg/apis/sessions/session_state_test.go @@ -22,7 +22,7 @@ func TestCreatedAtNow(t *testing.T) { ss := &SessionState{} now := time.Unix(1234567890, 0) - ss.Clock.Set(now) + ss.Clock = func() time.Time { return now } ss.CreatedAtNow() g.Expect(*ss.CreatedAt).To(Equal(now)) @@ -33,9 +33,9 @@ func TestExpiresIn(t *testing.T) { ss := &SessionState{} now := time.Unix(1234567890, 0) - ss.Clock.Set(now) + ss.Clock = func() time.Time { return now } - ttl := time.Duration(743) * time.Second + ttl := 743 * time.Second ss.ExpiresIn(ttl) g.Expect(*ss.ExpiresOn).To(Equal(ss.CreatedAt.Add(ttl))) diff --git a/pkg/clock/clock.go b/pkg/clock/clock.go deleted file mode 100644 index 887bf0aa..00000000 --- a/pkg/clock/clock.go +++ /dev/null @@ -1,157 +0,0 @@ -package clock - -import ( - "errors" - "sync" - "time" - - clockapi "github.com/benbjohnson/clock" -) - -var ( - globalClock = clockapi.New() - mu sync.Mutex -) - -// Set the global clock to a clockapi.Mock with the given time.Time -func Set(t time.Time) { - mu.Lock() - defer mu.Unlock() - mock, ok := globalClock.(*clockapi.Mock) - if !ok { - mock = clockapi.NewMock() - } - mock.Set(t) - globalClock = mock -} - -// Add moves the mocked global clock forward the given duration. It will error -// if the global clock is not mocked. -func Add(d time.Duration) error { - mu.Lock() - defer mu.Unlock() - mock, ok := globalClock.(*clockapi.Mock) - if !ok { - return errors.New("time not mocked") - } - mock.Add(d) - return nil -} - -// Reset sets the global clock to a pure time implementation. Returns any -// existing Mock if set in case lingering time operations are attached to it. -func Reset() *clockapi.Mock { - mu.Lock() - defer mu.Unlock() - existing := globalClock - globalClock = clockapi.New() - - mock, ok := existing.(*clockapi.Mock) - if !ok { - return nil - } - return mock -} - -// Clock is a non-package level wrapper around time that supports stubbing. -// It will use its localized stubs (allowing for parallelized unit tests -// where package level stubbing would cause issues). It falls back to any -// package level time stubs for non-parallel, cross-package integration -// testing scenarios. -// -// If nothing is stubbed, it defaults to default time behavior in the time -// package. -type Clock struct { - mock *clockapi.Mock -} - -// Set sets the Clock to a clock.Mock at the given time.Time -func (c *Clock) Set(t time.Time) { - if c.mock == nil { - c.mock = clockapi.NewMock() - } - c.mock.Set(t) -} - -// Add moves clock forward time.Duration if it is mocked. It will error -// if the clock is not mocked. -func (c *Clock) Add(d time.Duration) error { - if c.mock == nil { - return errors.New("clock not mocked") - } - c.mock.Add(d) - return nil -} - -// Reset removes local clock.Mock. Returns any existing Mock if set in case -// lingering time operations are attached to it. -func (c *Clock) Reset() *clockapi.Mock { - existing := c.mock - c.mock = nil - return existing -} - -func (c *Clock) After(d time.Duration) <-chan time.Time { - m := c.mock - if m == nil { - return globalClock.After(d) - } - return m.After(d) -} - -func (c *Clock) AfterFunc(d time.Duration, f func()) *clockapi.Timer { - m := c.mock - if m == nil { - return globalClock.AfterFunc(d, f) - } - return m.AfterFunc(d, f) -} - -func (c *Clock) Now() time.Time { - m := c.mock - if m == nil { - return globalClock.Now() - } - return m.Now() -} - -func (c *Clock) Since(t time.Time) time.Duration { - m := c.mock - if m == nil { - return globalClock.Since(t) - } - return m.Since(t) -} - -func (c *Clock) Sleep(d time.Duration) { - m := c.mock - if m == nil { - globalClock.Sleep(d) - return - } - m.Sleep(d) -} - -func (c *Clock) Tick(d time.Duration) <-chan time.Time { - m := c.mock - if m == nil { - return globalClock.Tick(d) - } - return m.Tick(d) -} - -func (c *Clock) Ticker(d time.Duration) *clockapi.Ticker { - m := c.mock - if m == nil { - return globalClock.Ticker(d) - } - return m.Ticker(d) -} - -func (c *Clock) Timer(d time.Duration) *clockapi.Timer { - m := c.mock - if m == nil { - return globalClock.Timer(d) - } - return m.Timer(d) -} diff --git a/pkg/clock/clock_suite_test.go b/pkg/clock/clock_suite_test.go deleted file mode 100644 index 99d39f24..00000000 --- a/pkg/clock/clock_suite_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package clock_test - -import ( - "testing" - - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -func TestClockSuite(t *testing.T) { - logger.SetOutput(GinkgoWriter) - logger.SetErrOutput(GinkgoWriter) - - RegisterFailHandler(Fail) - RunSpecs(t, "Clock") -} diff --git a/pkg/clock/clock_test.go b/pkg/clock/clock_test.go deleted file mode 100644 index e1b6d440..00000000 --- a/pkg/clock/clock_test.go +++ /dev/null @@ -1,380 +0,0 @@ -package clock_test - -import ( - "sync" - "sync/atomic" - "time" - - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/clock" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -const ( - testGlobalEpoch = 1000000000 - testLocalEpoch = 1234567890 -) - -var _ = Describe("Clock suite", func() { - var testClock = clock.Clock{} - - AfterEach(func() { - clock.Reset() - testClock.Reset() - }) - - Context("Global time not overridden", func() { - It("errors when trying to Add", func() { - err := clock.Add(123 * time.Hour) - Expect(err).To(HaveOccurred()) - }) - - Context("Clock not mocked via Set", func() { - const ( - outsideTolerance = int32(0) - withinTolerance = int32(1) - ) - - It("uses time.After for After", func() { - var tolerance int32 - go func() { - time.Sleep(10 * time.Millisecond) - atomic.StoreInt32(&tolerance, withinTolerance) - }() - go func() { - time.Sleep(30 * time.Millisecond) - atomic.StoreInt32(&tolerance, outsideTolerance) - }() - - Expect(atomic.LoadInt32(&tolerance)).To(Equal(outsideTolerance)) - - <-testClock.After(20 * time.Millisecond) - Expect(atomic.LoadInt32(&tolerance)).To(Equal(withinTolerance)) - - <-testClock.After(20 * time.Millisecond) - Expect(atomic.LoadInt32(&tolerance)).To(Equal(outsideTolerance)) - }) - - It("uses time.AfterFunc for AfterFunc", func() { - var tolerance int32 - go func() { - time.Sleep(10 * time.Millisecond) - atomic.StoreInt32(&tolerance, withinTolerance) - }() - go func() { - time.Sleep(30 * time.Millisecond) - atomic.StoreInt32(&tolerance, outsideTolerance) - }() - - Expect(atomic.LoadInt32(&tolerance)).To(Equal(outsideTolerance)) - - var wg sync.WaitGroup - wg.Add(1) - testClock.AfterFunc(20*time.Millisecond, func() { - wg.Done() - }) - wg.Wait() - Expect(atomic.LoadInt32(&tolerance)).To(Equal(withinTolerance)) - - wg.Add(1) - testClock.AfterFunc(20*time.Millisecond, func() { - wg.Done() - }) - wg.Wait() - Expect(atomic.LoadInt32(&tolerance)).To(Equal(outsideTolerance)) - }) - - It("uses time.Now for Now", func() { - a := time.Now() - b := testClock.Now() - Expect(b.Sub(a).Round(10 * time.Millisecond)).To(Equal(0 * time.Millisecond)) - }) - - It("uses time.Since for Since", func() { - past := time.Now().Add(-60 * time.Second) - Expect(time.Since(past).Round(10 * time.Millisecond)). - To(Equal(60 * time.Second)) - }) - - It("uses time.Sleep for Sleep", func() { - var tolerance int32 - go func() { - time.Sleep(10 * time.Millisecond) - atomic.StoreInt32(&tolerance, withinTolerance) - }() - go func() { - time.Sleep(30 * time.Millisecond) - atomic.StoreInt32(&tolerance, outsideTolerance) - }() - - Expect(atomic.LoadInt32(&tolerance)).To(Equal(outsideTolerance)) - - testClock.Sleep(20 * time.Millisecond) - Expect(atomic.LoadInt32(&tolerance)).To(Equal(withinTolerance)) - - testClock.Sleep(20 * time.Millisecond) - Expect(atomic.LoadInt32(&tolerance)).To(Equal(outsideTolerance)) - }) - - It("uses time.Tick for Tick", func() { - var tolerance int32 - go func() { - time.Sleep(10 * time.Millisecond) - atomic.StoreInt32(&tolerance, withinTolerance) - }() - go func() { - time.Sleep(50 * time.Millisecond) - atomic.StoreInt32(&tolerance, outsideTolerance) - }() - - ch := testClock.Tick(20 * time.Millisecond) - Expect(atomic.LoadInt32(&tolerance)).To(Equal(outsideTolerance)) - <-ch - Expect(atomic.LoadInt32(&tolerance)).To(Equal(withinTolerance)) - <-ch - Expect(atomic.LoadInt32(&tolerance)).To(Equal(withinTolerance)) - <-ch - Expect(atomic.LoadInt32(&tolerance)).To(Equal(outsideTolerance)) - }) - - It("uses time.Ticker for Ticker", func() { - var tolerance int32 - go func() { - time.Sleep(10 * time.Millisecond) - atomic.StoreInt32(&tolerance, withinTolerance) - }() - go func() { - time.Sleep(50 * time.Millisecond) - atomic.StoreInt32(&tolerance, outsideTolerance) - }() - - ticker := testClock.Ticker(20 * time.Millisecond) - Expect(atomic.LoadInt32(&tolerance)).To(Equal(outsideTolerance)) - <-ticker.C - Expect(atomic.LoadInt32(&tolerance)).To(Equal(withinTolerance)) - <-ticker.C - Expect(atomic.LoadInt32(&tolerance)).To(Equal(withinTolerance)) - <-ticker.C - Expect(atomic.LoadInt32(&tolerance)).To(Equal(outsideTolerance)) - }) - - It("errors if Add is used", func() { - err := testClock.Add(100 * time.Second) - Expect(err).To(HaveOccurred()) - }) - }) - - Context("Clock mocked via Set", func() { - var now = time.Unix(testLocalEpoch, 0) - - BeforeEach(func() { - testClock.Set(now) - }) - - It("mocks After", func() { - var after int32 - ready := make(chan struct{}) - ch := testClock.After(10 * time.Second) - go func(ch <-chan time.Time) { - close(ready) - <-ch - atomic.StoreInt32(&after, 1) - }(ch) - <-ready - - err := testClock.Add(9 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&after)).To(Equal(int32(0))) - - err = testClock.Add(1 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&after)).To(Equal(int32(1))) - }) - - It("mocks AfterFunc", func() { - var after int32 - testClock.AfterFunc(10*time.Second, func() { - atomic.StoreInt32(&after, 1) - }) - - err := testClock.Add(9 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&after)).To(Equal(int32(0))) - - err = testClock.Add(1 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&after)).To(Equal(int32(1))) - }) - - It("mocks AfterFunc with a stopped timer", func() { - var after int32 - timer := testClock.AfterFunc(10*time.Second, func() { - atomic.StoreInt32(&after, 1) - }) - timer.Stop() - - err := testClock.Add(11 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&after)).To(Equal(int32(0))) - }) - - It("mocks Now", func() { - Expect(testClock.Now()).To(Equal(now)) - err := testClock.Add(123 * time.Hour) - Expect(err).ToNot(HaveOccurred()) - Expect(testClock.Now()).To(Equal(now.Add(123 * time.Hour))) - }) - - It("mocks Since", func() { - Expect(testClock.Since(time.Unix(testLocalEpoch-100, 0))). - To(Equal(100 * time.Second)) - }) - - It("mocks Sleep", func() { - var after int32 - ready := make(chan struct{}) - go func() { - close(ready) - testClock.Sleep(10 * time.Second) - atomic.StoreInt32(&after, 1) - }() - <-ready - - err := testClock.Add(9 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&after)).To(Equal(int32(0))) - - err = testClock.Add(1 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&after)).To(Equal(int32(1))) - }) - - It("mocks Tick", func() { - var ticks int32 - ready := make(chan struct{}) - go func() { - close(ready) - tick := testClock.Tick(10 * time.Second) - for ticks < 5 { - <-tick - atomic.AddInt32(&ticks, 1) - } - }() - <-ready - - Expect(atomic.LoadInt32(&ticks)).To(Equal(int32(0))) - - err := testClock.Add(9 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&ticks)).To(Equal(int32(0))) - - err = testClock.Add(1 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&ticks)).To(Equal(int32(1))) - - err = testClock.Add(30 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&ticks)).To(Equal(int32(4))) - - err = testClock.Add(10 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&ticks)).To(Equal(int32(5))) - }) - - It("mocks Ticker", func() { - var ticks int32 - ready := make(chan struct{}) - go func() { - ticker := testClock.Ticker(10 * time.Second) - close(ready) - for ticks < 5 { - <-ticker.C - atomic.AddInt32(&ticks, 1) - } - }() - <-ready - - Expect(atomic.LoadInt32(&ticks)).To(Equal(int32(0))) - - err := testClock.Add(9 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&ticks)).To(Equal(int32(0))) - - err = testClock.Add(1 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&ticks)).To(Equal(int32(1))) - - err = testClock.Add(30 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&ticks)).To(Equal(int32(4))) - - err = testClock.Add(10 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&ticks)).To(Equal(int32(5))) - }) - - It("mocks Timer", func() { - var after int32 - ready := make(chan struct{}) - go func() { - timer := testClock.Timer(10 * time.Second) - close(ready) - <-timer.C - atomic.AddInt32(&after, 1) - }() - <-ready - - err := testClock.Add(9 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&after)).To(Equal(int32(0))) - - err = testClock.Add(1 * time.Second) - Expect(err).ToNot(HaveOccurred()) - Expect(atomic.LoadInt32(&after)).To(Equal(int32(1))) - }) - }) - }) - - Context("Global time overridden", func() { - var ( - globalNow = time.Unix(testGlobalEpoch, 0) - localNow = time.Unix(testLocalEpoch, 0) - ) - - BeforeEach(func() { - clock.Set(globalNow) - }) - - Context("Clock not mocked via Set", func() { - It("uses globally mocked Now", func() { - Expect(testClock.Now()).To(Equal(globalNow)) - err := clock.Add(123 * time.Hour) - Expect(err).ToNot(HaveOccurred()) - Expect(testClock.Now()).To(Equal(globalNow.Add(123 * time.Hour))) - }) - - It("errors when Add is called on the local Clock", func() { - err := testClock.Add(100 * time.Hour) - Expect(err).To(HaveOccurred()) - }) - }) - - Context("Clock is mocked via Set", func() { - BeforeEach(func() { - testClock.Set(localNow) - }) - - It("uses the local mock and ignores the global", func() { - Expect(testClock.Now()).To(Equal(localNow)) - - err := clock.Add(456 * time.Hour) - Expect(err).ToNot(HaveOccurred()) - - err = testClock.Add(123 * time.Hour) - Expect(err).ToNot(HaveOccurred()) - - Expect(testClock.Now()).To(Equal(localNow.Add(123 * time.Hour))) - }) - }) - }) -}) diff --git a/pkg/cookies/csrf.go b/pkg/cookies/csrf.go index 3b8efaf3..939578a2 100644 --- a/pkg/cookies/csrf.go +++ b/pkg/cookies/csrf.go @@ -10,7 +10,6 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/clock" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" "github.com/vmihailenco/msgpack/v5" ) @@ -47,7 +46,7 @@ type csrf struct { CodeVerifier string `msgpack:"cv,omitempty"` cookieOpts *options.Cookie - time clock.Clock + clock func() time.Time } // csrtStateTrim will indicate the length of the state trimmed for the name of the csrf cookie @@ -70,6 +69,7 @@ func NewCSRF(opts *options.Cookie, codeVerifier string) (CSRF, error) { CodeVerifier: codeVerifier, cookieOpts: opts, + clock: time.Now, }, nil } @@ -187,7 +187,7 @@ func ClearExtraCsrfCookies(opts *options.Cookie, rw http.ResponseWriter, req *ht // delete the X oldest cookies slices.SortStableFunc(decodedCookies, func(a, b *csrf) int { - return a.time.Now().Compare(b.time.Now()) + return a.clock().Compare(b.clock()) }) for i := 0; i < len(decodedCookies)-opts.CSRFPerRequestLimit; i++ { @@ -223,7 +223,7 @@ func (c *csrf) encodeCookie() (string, error) { if err != nil { return "", fmt.Errorf("error getting cookie secret: %v", err) } - return encryption.SignedValue(secret, c.cookieName(), encrypted, c.time.Now()) + return encryption.SignedValue(secret, c.cookieName(), encrypted, c.clock()) } // decodeCSRFCookie validates the signature then decrypts and decodes a CSRF @@ -249,10 +249,10 @@ func decodeCSRFCookie(cookie *http.Cookie, opts *options.Cookie) (*csrf, error) // unmarshalCSRF unmarshals decrypted data into a CSRF struct func unmarshalCSRF(decrypted []byte, opts *options.Cookie, csrfTime time.Time) (*csrf, error) { - clock := clock.Clock{} - clock.Set(csrfTime) - - csrf := &csrf{cookieOpts: opts, time: clock} + csrf := &csrf{ + cookieOpts: opts, + clock: func() time.Time { return csrfTime }, + } if err := msgpack.Unmarshal(decrypted, csrf); err != nil { return nil, fmt.Errorf("error unmarshalling data to CSRF: %v", err) } diff --git a/pkg/cookies/csrf_per_request_test.go b/pkg/cookies/csrf_per_request_test.go index 9b7d4e59..6a17013b 100644 --- a/pkg/cookies/csrf_per_request_test.go +++ b/pkg/cookies/csrf_per_request_test.go @@ -128,7 +128,7 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { testNow := time.Unix(nowEpoch, 0) BeforeEach(func() { - privateCSRF.time.Set(testNow) + privateCSRF.clock = func() time.Time { return testNow } req = &http.Request{ Method: http.MethodGet, @@ -144,7 +144,7 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { }) AfterEach(func() { - privateCSRF.time.Reset() + privateCSRF.clock = time.Now }) Context("SetCookie", func() { @@ -200,17 +200,17 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { publicCSRF1, err := NewCSRF(cookieOpts, "verifier") Expect(err).ToNot(HaveOccurred()) privateCSRF1 := publicCSRF1.(*csrf) - privateCSRF1.time.Set(testNow) + privateCSRF1.clock = func() time.Time { return testNow } publicCSRF2, err := NewCSRF(cookieOpts, "verifier") Expect(err).ToNot(HaveOccurred()) privateCSRF2 := publicCSRF2.(*csrf) - privateCSRF2.time.Set(testNow.Add(time.Minute)) + privateCSRF2.clock = func() time.Time { return testNow.Add(time.Minute) } publicCSRF3, err := NewCSRF(cookieOpts, "verifier") Expect(err).ToNot(HaveOccurred()) privateCSRF3 := publicCSRF3.(*csrf) - privateCSRF3.time.Set(testNow.Add(time.Minute * 2)) + privateCSRF3.clock = func() time.Time { return testNow.Add(time.Minute * 2) } cookies := []string{} for _, csrf := range []*csrf{privateCSRF1, privateCSRF2, privateCSRF3} { diff --git a/pkg/cookies/csrf_test.go b/pkg/cookies/csrf_test.go index 37527bd0..085b91df 100644 --- a/pkg/cookies/csrf_test.go +++ b/pkg/cookies/csrf_test.go @@ -130,7 +130,7 @@ var _ = Describe("CSRF Cookie Tests", func() { testNow := time.Unix(nowEpoch, 0) BeforeEach(func() { - privateCSRF.time.Set(testNow) + privateCSRF.clock = func() time.Time { return testNow } req = &http.Request{ Method: http.MethodGet, @@ -146,7 +146,7 @@ var _ = Describe("CSRF Cookie Tests", func() { }) AfterEach(func() { - privateCSRF.time.Reset() + privateCSRF.clock = time.Now }) Context("SetCookie", func() { @@ -173,7 +173,7 @@ var _ = Describe("CSRF Cookie Tests", func() { Context("LoadCSRFCookie", func() { BeforeEach(func() { // we need to reset the time to ensure the cookie is valid - privateCSRF.time.Reset() + privateCSRF.clock = time.Now }) It("should return error when no cookie is set", func() { diff --git a/pkg/middleware/stored_session_test.go b/pkg/middleware/stored_session_test.go index 904c2028..d8e78f2f 100644 --- a/pkg/middleware/stored_session_test.go +++ b/pkg/middleware/stored_session_test.go @@ -11,7 +11,6 @@ import ( middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/clock" "github.com/oauth2-proxy/oauth2-proxy/v7/providers" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -95,6 +94,7 @@ var _ = Describe("Stored Session Suite", func() { now := time.Now() createdPast := now.Add(-5 * time.Minute) createdFuture := now.Add(5 * time.Minute) + clock := func() time.Time { return now } var defaultRefreshFunc = func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) { switch ss.RefreshToken { @@ -120,6 +120,7 @@ var _ = Describe("Stored Session Suite", func() { RefreshToken: noRefresh, CreatedAt: &createdPast, ExpiresOn: &createdFuture, + Clock: clock, }, nil case "_oauth2_proxy=InvalidNoRefreshSession": return &sessionsapi.SessionState{ @@ -127,24 +128,28 @@ var _ = Describe("Stored Session Suite", func() { RefreshToken: noRefresh, CreatedAt: &createdPast, ExpiresOn: &createdFuture, + Clock: clock, }, nil case "_oauth2_proxy=ExpiredNoRefreshSession": return &sessionsapi.SessionState{ RefreshToken: noRefresh, CreatedAt: &createdPast, ExpiresOn: &createdPast, + Clock: clock, }, nil case "_oauth2_proxy=RefreshSession": return &sessionsapi.SessionState{ RefreshToken: refresh, CreatedAt: &createdPast, ExpiresOn: &createdFuture, + Clock: clock, }, nil case "_oauth2_proxy=RefreshError": return &sessionsapi.SessionState{ RefreshToken: "RefreshError", CreatedAt: &createdPast, ExpiresOn: &createdFuture, + Clock: clock, }, nil case "_oauth2_proxy=NonExistent": return nil, fmt.Errorf("invalid cookie") @@ -154,14 +159,6 @@ var _ = Describe("Stored Session Suite", func() { }, } - BeforeEach(func() { - clock.Set(now) - }) - - AfterEach(func() { - clock.Reset() - }) - type storedSessionLoaderTableInput struct { requestHeaders http.Header existingSession *sessionsapi.SessionState @@ -200,7 +197,15 @@ var _ = Describe("Stored Session Suite", func() { })) handler.ServeHTTP(rw, req) - Expect(gotSession).To(Equal(in.expectedSession)) + // Compare, ignoring testing Clock. + if in.expectedSession == nil { + Expect(gotSession).To(BeNil()) + return + } + Expect(gotSession).ToNot(BeNil()) + got := *gotSession + got.Clock = nil + Expect(&got).To(Equal(in.expectedSession)) }, Entry("with no cookie", storedSessionLoaderTableInput{ requestHeaders: http.Header{}, diff --git a/providers/provider_default_test.go b/providers/provider_default_test.go index f678d13d..0fbe7abd 100644 --- a/providers/provider_default_test.go +++ b/providers/provider_default_test.go @@ -17,8 +17,9 @@ func TestRefresh(t *testing.T) { now := time.Unix(1234567890, 10) expires := time.Unix(1234567890, 0) - ss := &sessions.SessionState{} - ss.Clock.Set(now) + ss := &sessions.SessionState{ + Clock: func() time.Time { return now }, + } ss.SetExpiresOn(expires) refreshed, err := p.RefreshSession(context.Background(), ss)