From 5f6f40d7762fb0f5db27217c043d3ca63755cc22 Mon Sep 17 00:00:00 2001 From: Laurin <42897588+suxatcode@users.noreply.github.com> Date: Thu, 5 Jun 2025 17:38:01 +0200 Subject: [PATCH 1/3] test: replicate refresh failure session clearing bug --- pkg/middleware/stored_session_test.go | 53 ++++++++++++++++----------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/pkg/middleware/stored_session_test.go b/pkg/middleware/stored_session_test.go index 904c2028fa..e3811cdb66 100644 --- a/pkg/middleware/stored_session_test.go +++ b/pkg/middleware/stored_session_test.go @@ -295,27 +295,38 @@ var _ = Describe("Stored Session Suite", func() { refreshSession: defaultRefreshFunc, validateSession: defaultValidateFunc, }), - Entry("when the provider refresh fails but validation succeeds", storedSessionLoaderTableInput{ - requestHeaders: http.Header{ - "Cookie": []string{"_oauth2_proxy=RefreshError"}, - }, - existingSession: nil, - expectedSession: &sessionsapi.SessionState{ - RefreshToken: "RefreshError", - CreatedAt: &createdPast, - ExpiresOn: &createdFuture, - Lock: &sessionsapi.NoOpLock{}, - }, - store: defaultSessionStore, - refreshPeriod: 1 * time.Minute, - refreshSession: defaultRefreshFunc, - validateSession: defaultValidateFunc, - }), - Entry("when the provider refresh fails and validation fails", storedSessionLoaderTableInput{ - requestHeaders: http.Header{ - "Cookie": []string{"_oauth2_proxy=RefreshError"}, - }, - existingSession: nil, + Entry("when the provider refresh fails but validation succeeds", storedSessionLoaderTableInput{ + requestHeaders: http.Header{ + "Cookie": []string{"_oauth2_proxy=RefreshError"}, + }, + existingSession: nil, + expectedSession: &sessionsapi.SessionState{ + RefreshToken: "RefreshError", + CreatedAt: &createdPast, + ExpiresOn: &createdFuture, + Lock: &sessionsapi.NoOpLock{}, + }, + store: defaultSessionStore, + refreshPeriod: 1 * time.Minute, + refreshSession: defaultRefreshFunc, + validateSession: defaultValidateFunc, + }), + Entry("when the provider refresh fails and the session should be cleared (issue 3057)", storedSessionLoaderTableInput{ + requestHeaders: http.Header{ + "Cookie": []string{"_oauth2_proxy=RefreshError"}, + }, + existingSession: nil, + expectedSession: nil, + store: defaultSessionStore, + refreshPeriod: 1 * time.Minute, + refreshSession: defaultRefreshFunc, + validateSession: defaultValidateFunc, + }), + Entry("when the provider refresh fails and validation fails", storedSessionLoaderTableInput{ + requestHeaders: http.Header{ + "Cookie": []string{"_oauth2_proxy=RefreshError"}, + }, + existingSession: nil, expectedSession: nil, store: defaultSessionStore, refreshPeriod: 1 * time.Minute, From df42289b73a04d4ac54d017539a5593720bf3a4e Mon Sep 17 00:00:00 2001 From: Laurin <42897588+suxatcode@users.noreply.github.com> Date: Thu, 5 Jun 2025 18:24:56 +0200 Subject: [PATCH 2/3] Add integration test for refresh failure --- pkg/middleware/stored_session_test.go | 85 +++++++++++++++++---------- 1 file changed, 53 insertions(+), 32 deletions(-) diff --git a/pkg/middleware/stored_session_test.go b/pkg/middleware/stored_session_test.go index e3811cdb66..559dc3e4d7 100644 --- a/pkg/middleware/stored_session_test.go +++ b/pkg/middleware/stored_session_test.go @@ -295,38 +295,27 @@ var _ = Describe("Stored Session Suite", func() { refreshSession: defaultRefreshFunc, validateSession: defaultValidateFunc, }), - Entry("when the provider refresh fails but validation succeeds", storedSessionLoaderTableInput{ - requestHeaders: http.Header{ - "Cookie": []string{"_oauth2_proxy=RefreshError"}, - }, - existingSession: nil, - expectedSession: &sessionsapi.SessionState{ - RefreshToken: "RefreshError", - CreatedAt: &createdPast, - ExpiresOn: &createdFuture, - Lock: &sessionsapi.NoOpLock{}, - }, - store: defaultSessionStore, - refreshPeriod: 1 * time.Minute, - refreshSession: defaultRefreshFunc, - validateSession: defaultValidateFunc, - }), - Entry("when the provider refresh fails and the session should be cleared (issue 3057)", storedSessionLoaderTableInput{ - requestHeaders: http.Header{ - "Cookie": []string{"_oauth2_proxy=RefreshError"}, - }, - existingSession: nil, - expectedSession: nil, - store: defaultSessionStore, - refreshPeriod: 1 * time.Minute, - refreshSession: defaultRefreshFunc, - validateSession: defaultValidateFunc, - }), - Entry("when the provider refresh fails and validation fails", storedSessionLoaderTableInput{ - requestHeaders: http.Header{ - "Cookie": []string{"_oauth2_proxy=RefreshError"}, - }, - existingSession: nil, + Entry("when the provider refresh fails but validation succeeds", storedSessionLoaderTableInput{ + requestHeaders: http.Header{ + "Cookie": []string{"_oauth2_proxy=RefreshError"}, + }, + existingSession: nil, + expectedSession: &sessionsapi.SessionState{ + RefreshToken: "RefreshError", + CreatedAt: &createdPast, + ExpiresOn: &createdFuture, + Lock: &sessionsapi.NoOpLock{}, + }, + store: defaultSessionStore, + refreshPeriod: 1 * time.Minute, + refreshSession: defaultRefreshFunc, + validateSession: defaultValidateFunc, + }), + Entry("when the provider refresh fails and validation fails", storedSessionLoaderTableInput{ + requestHeaders: http.Header{ + "Cookie": []string{"_oauth2_proxy=RefreshError"}, + }, + existingSession: nil, expectedSession: nil, store: defaultSessionStore, refreshPeriod: 1 * time.Minute, @@ -346,6 +335,38 @@ var _ = Describe("Stored Session Suite", func() { }), ) + It("clears the session from the store when refresh fails (issue 3057)", func() { + stored := &sessionsapi.SessionState{ + RefreshToken: "RefreshError", + CreatedAt: &createdPast, + ExpiresOn: &createdFuture, + } + store := &fakeSessionStore{ + LoadFunc: func(*http.Request) (*sessionsapi.SessionState, error) { return stored, nil }, + ClearFunc: func(http.ResponseWriter, *http.Request) error { stored = nil; return nil }, + } + + scope := &middlewareapi.RequestScope{} + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Cookie", "_oauth2_proxy=RefreshError") + req = middlewareapi.AddRequestScope(req, scope) + rw := httptest.NewRecorder() + + opts := &StoredSessionLoaderOptions{ + SessionStore: store, + RefreshPeriod: 1 * time.Minute, + RefreshSession: func(context.Context, *sessionsapi.SessionState) (bool, error) { + return false, errors.New("refresh failed") + }, + ValidateSession: func(context.Context, *sessionsapi.SessionState) bool { return true }, + } + + handler := NewStoredSessionLoader(opts)(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + handler.ServeHTTP(rw, req) + + Expect(stored).To(BeNil()) + }) + type storedSessionLoaderConcurrentTableInput struct { existingSession *sessionsapi.SessionState refreshPeriod time.Duration From 544fe12a47982af4f6a5fa96b1b29ebd5b680146 Mon Sep 17 00:00:00 2001 From: Laurin <42897588+suxatcode@users.noreply.github.com> Date: Thu, 5 Jun 2025 18:33:27 +0200 Subject: [PATCH 3/3] docs: add script to reproduce session clearing bug --- contrib/local-environment/README.md | 14 ++++ .../docker-compose-keycloak-redis.yaml | 68 +++++++++++++++++++ .../oauth2-proxy-keycloak-redis.cfg | 23 +++++++ contrib/local-environment/reproduce-3057.sh | 25 +++++++ 4 files changed, 130 insertions(+) create mode 100644 contrib/local-environment/docker-compose-keycloak-redis.yaml create mode 100644 contrib/local-environment/oauth2-proxy-keycloak-redis.cfg create mode 100755 contrib/local-environment/reproduce-3057.sh diff --git a/contrib/local-environment/README.md b/contrib/local-environment/README.md index edfeceacc0..328605bcbf 100644 --- a/contrib/local-environment/README.md +++ b/contrib/local-environment/README.md @@ -1,3 +1,17 @@ # oauth2-proxy: local-environment Run `make up` to deploy local dex, etcd and oauth2-proxy instances in Docker containers. Review the [`Makefile`](Makefile) for additional deployment options. + +## Reproducing issue #3057 + +The script `reproduce-3057.sh` runs a local Keycloak environment with Redis session storage. Use this to observe how sessions are not cleared when a refresh fails. + +```bash +# Start the environment +./reproduce-3057.sh up + +# When finished +./reproduce-3057.sh down +``` + +After starting, login at `http://oauth2-proxy.localtest.me:4180`. Then delete the session for that user in the Keycloak admin console (`http://keycloak.localtest.me:9080`). Wait about 30 seconds (the configured `cookie_refresh`), refresh the protected page and note that the session remains active. diff --git a/contrib/local-environment/docker-compose-keycloak-redis.yaml b/contrib/local-environment/docker-compose-keycloak-redis.yaml new file mode 100644 index 0000000000..73a70010e9 --- /dev/null +++ b/contrib/local-environment/docker-compose-keycloak-redis.yaml @@ -0,0 +1,68 @@ +# This docker-compose file can be used to reproduce session clearing issue 3057 +# It extends the Keycloak example with a Redis backend for session storage. +# Access http://oauth2-proxy.localtest.me:4180 to start a login. +version: '3.0' +services: + oauth2-proxy: + container_name: oauth2-proxy + image: quay.io/oauth2-proxy/oauth2-proxy:v7.9.0 + command: --config /oauth2-proxy.cfg + hostname: oauth2-proxy + volumes: + - "./oauth2-proxy-keycloak-redis.cfg:/oauth2-proxy.cfg" + restart: unless-stopped + ports: + - 4180:4180/tcp + networks: + keycloak: {} + httpbin: {} + oauth2-proxy: {} + depends_on: + - httpbin + - keycloak + - redis + + httpbin: + container_name: httpbin + image: kennethreitz/httpbin:latest + hostname: httpbin + ports: + - 8080:80/tcp + networks: + httpbin: + aliases: + - httpbin.localtest.me + + keycloak: + container_name: keycloak + image: keycloak/keycloak:25.0 + hostname: keycloak + command: + - 'start-dev' + - '--http-port=9080' + - '--import-realm' + volumes: + - ./keycloak:/opt/keycloak/data/import + environment: + KC_HTTP_PORT: 9080 + KEYCLOAK_ADMIN: admin@example.com + KEYCLOAK_ADMIN_PASSWORD: password + ports: + - 9080:9080/tcp + networks: + keycloak: + aliases: + - keycloak.localtest.me + + redis: + container_name: redis + image: redis:7 + ports: + - 6379:6379/tcp + networks: + oauth2-proxy: {} + +networks: + httpbin: {} + keycloak: {} + oauth2-proxy: {} diff --git a/contrib/local-environment/oauth2-proxy-keycloak-redis.cfg b/contrib/local-environment/oauth2-proxy-keycloak-redis.cfg new file mode 100644 index 0000000000..1089fd02e3 --- /dev/null +++ b/contrib/local-environment/oauth2-proxy-keycloak-redis.cfg @@ -0,0 +1,23 @@ +http_address="0.0.0.0:4180" +cookie_secret="OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" +email_domains="example.com" +cookie_secure="false" +upstreams="http://httpbin" +cookie_domains=["oauth2-proxy.localtest.me:4080", "httpbin.localtest.me:8080", "keycloak.localtest.me:9080"] +whitelist_domains=[".localtest.me"] + +# keycloak provider +client_secret="72341b6d-7065-4518-a0e4-50ee15025608" +client_id="oauth2-proxy" +redirect_url="http://oauth2-proxy.localtest.me:4180/oauth2/callback" + +# in this case oauth2-proxy is going to visit +# http://keycloak.localtest.me:9080/realms/oauth2-proxy/.well-known/openid-configuration for configuration +oidc_issuer_url="http://keycloak.localtest.me:9080/realms/oauth2-proxy" +provider="oidc" +provider_display_name="Keycloak" + +cookie_refresh="30s" +cookie_expire="2m" +session_store_type="redis" +redis_connection_url="redis://redis:6379/0" diff --git a/contrib/local-environment/reproduce-3057.sh b/contrib/local-environment/reproduce-3057.sh new file mode 100755 index 0000000000..e5f7838f2a --- /dev/null +++ b/contrib/local-environment/reproduce-3057.sh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +# Helper script to start the Keycloak environment with Redis to reproduce issue #3057 +set -euo pipefail + +cd "$(dirname "$0")" + +if ! command -v docker > /dev/null; then + echo "docker is required" >&2 + exit 1 +fi + +COMPOSE_FILE="docker-compose-keycloak-redis.yaml" + +# bring up or down the environment based on first argument +cmd=${1:-up} + +docker compose -f "$COMPOSE_FILE" $cmd + +if [ "$cmd" = "up" ]; then + echo "\nEnvironment started." + echo "1. Visit http://oauth2-proxy.localtest.me:4180 and log in with admin@example.com / password." + echo "2. Open Keycloak admin at http://keycloak.localtest.me:9080/ and remove the active session." + echo "3. Wait about 30 seconds (cookie_refresh). Send another request to http://oauth2-proxy.localtest.me:4180/." + echo "If the session remains active despite the refresh failure, the bug is reproduced." +fi