From 8c833652a91913abe2e6b7b388f43dafc26c9704 Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Wed, 5 Aug 2026 15:20:31 +0200 Subject: [PATCH 1/9] OCPBUGS-71237: Persist console sessions across pod restarts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Store the actual encrypted OAuth refresh token in the browser cookie instead of a reference ID that maps to an in-memory store. This allows any console pod to recover a user's session after a restart by decrypting the cookie and exchanging the refresh token with the OAuth server. To enable cross-pod cookie decryption, accept shared encryption keys from files (managed by the console-operator via a session-secret Secret) instead of generating random keys per process. The existing recovery mechanism in getLoginState() already handles the token refresh — the only gap was making the refresh token available from the cookie. Backward compatible: old-format cookies with reference IDs are still accepted via fallback lookup during rolling upgrades. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/bridge/config/session/sessionoptions.go | 12 ++- pkg/auth/oauth2/auth.go | 2 + pkg/auth/oauth2/auth_oidc.go | 2 + pkg/auth/oauth2/auth_oidc_test.go | 27 +++---- pkg/auth/oauth2/auth_openshift.go | 27 ++++--- pkg/auth/sessions/combined_sessions.go | 76 ++++++++----------- pkg/auth/sessions/combined_sessions_test.go | 81 +++++++++++++++------ pkg/auth/sessions/loginstate.go | 7 +- pkg/auth/sessions/server_session.go | 6 -- 9 files changed, 130 insertions(+), 110 deletions(-) diff --git a/cmd/bridge/config/session/sessionoptions.go b/cmd/bridge/config/session/sessionoptions.go index 0df81e19c37..34620fe88c9 100644 --- a/cmd/bridge/config/session/sessionoptions.go +++ b/cmd/bridge/config/session/sessionoptions.go @@ -33,8 +33,8 @@ func NewSessionOptions() *SessionOptions { } func (opts *SessionOptions) AddFlags(fs *flag.FlagSet) { - fs.StringVar(&opts.CookieEncryptionKeyPath, "cookie-encryption-key-file", "", "Encryption key used to encrypt cookies. Must be set when --user-auth is 'oidc'.") - fs.StringVar(&opts.CookieAuthenticationKeyPath, "cookie-authentication-key-file", "", "Authentication key used to sign cookies. Must be set when --user-auth is 'oidc'.") + fs.StringVar(&opts.CookieEncryptionKeyPath, "cookie-encryption-key-file", "", "Encryption key used to encrypt cookies. Required when --user-auth is 'oidc', optional when 'openshift'.") + fs.StringVar(&opts.CookieAuthenticationKeyPath, "cookie-authentication-key-file", "", "Authentication key used to sign cookies. Required when --user-auth is 'oidc', optional when 'openshift'.") } func (opts *SessionOptions) ApplyConfig(config *serverconfig.Session) { @@ -50,9 +50,15 @@ func (opts *SessionOptions) Validate(userAuthType flagvalues.AuthType) []error { if opts.CookieEncryptionKeyPath == "" || opts.CookieAuthenticationKeyPath == "" { errs = append(errs, fmt.Errorf("cookie-encryption-key-file and cookie-authentication-key-file must be set when --user-auth is 'oidc'")) } + case flagvalues.AuthTypeOpenShift: + bothSet := opts.CookieEncryptionKeyPath != "" && opts.CookieAuthenticationKeyPath != "" + neitherSet := opts.CookieEncryptionKeyPath == "" && opts.CookieAuthenticationKeyPath == "" + if !bothSet && !neitherSet { + errs = append(errs, fmt.Errorf("cookie-encryption-key-file and cookie-authentication-key-file must both be set or both be unset when --user-auth is 'openshift'")) + } default: if opts.CookieEncryptionKeyPath != "" || opts.CookieAuthenticationKeyPath != "" { - errs = append(errs, fmt.Errorf("cookie-encryption-key-file and cookie-authentication-key-file must not be set when --user-auth is not 'oidc'")) + errs = append(errs, fmt.Errorf("cookie-encryption-key-file and cookie-authentication-key-file must not be set when --user-auth is not 'oidc' or 'openshift'")) } } diff --git a/pkg/auth/oauth2/auth.go b/pkg/auth/oauth2/auth.go index dbd9c8c987d..2b1194c49bc 100644 --- a/pkg/auth/oauth2/auth.go +++ b/pkg/auth/oauth2/auth.go @@ -213,6 +213,8 @@ func NewOAuth2Authenticator(ctx context.Context, config *Config) (*OAuth2Authent consoleBaseAddress: c.ConsoleBaseAddress, cookiePath: c.CookiePath, secureCookies: c.SecureCookies, + cookieAuthenticationKey: c.CookieAuthenticationKey, + cookieEncryptionKey: c.CookieEncryptionKey, constructOAuth2Config: a.oauth2ConfigConstructor, } diff --git a/pkg/auth/oauth2/auth_oidc.go b/pkg/auth/oauth2/auth_oidc.go index 987c34d35a4..bce65c533af 100644 --- a/pkg/auth/oauth2/auth_oidc.go +++ b/pkg/auth/oauth2/auth_oidc.go @@ -41,6 +41,8 @@ type oidcConfig struct { consoleBaseAddress string cookiePath string secureCookies bool + cookieAuthenticationKey []byte + cookieEncryptionKey []byte constructOAuth2Config oauth2ConfigConstructor } diff --git a/pkg/auth/oauth2/auth_oidc_test.go b/pkg/auth/oauth2/auth_oidc_test.go index 23d84f55d8a..ea222c70957 100644 --- a/pkg/auth/oauth2/auth_oidc_test.go +++ b/pkg/auth/oauth2/auth_oidc_test.go @@ -965,14 +965,13 @@ func testOAuth2ConfigConstructor(endpointConfig oauth2.Endpoint) *oauth2.Config } type testCookieFactory struct { - cookieCodecs []securecookie.Codec - sessionToken *string - refreshToken *string - refreshTokenID *string - customCookies map[string]map[interface{}]interface{} - serverStore *sessions.SessionStore // needed to set up refresh token ID mapping - tokenVerifier sessions.IDTokenVerifier - signPayload func(string) string // function to sign an ID token payload + cookieCodecs []securecookie.Codec + sessionToken *string + refreshToken *string + customCookies map[string]map[interface{}]interface{} + serverStore *sessions.SessionStore + tokenVerifier sessions.IDTokenVerifier + signPayload func(string) string } func (f *testCookieFactory) WithSessionToken(sessionToken string) *testCookieFactory { @@ -1002,26 +1001,18 @@ func (f *testCookieFactory) Complete(t testing.TB, req *http.Request) *http.Requ f.cookieCodecs) } if f.refreshToken != nil { - var id string if f.serverStore != nil && f.tokenVerifier != nil && f.signPayload != nil { - // Use AddSession to properly set up the state token := addIDToken( &oauth2.Token{RefreshToken: *f.refreshToken}, f.signPayload(`{"sub":"testuser","exp":`+strconv.FormatInt(time.Now().Add(5*time.Minute).Unix(), 10)+`}`), ) - loginState, err := f.serverStore.AddSession(f.tokenVerifier, token) + _, err := f.serverStore.AddSession(f.tokenVerifier, token) require.NoError(t, err) - id = loginState.RefreshTokenID() - } else { - // Fallback to generating a random ID - id = randomString(32) } - f.refreshTokenID = &id - // Store only the ID in the cookie attachCookieOrDie(t, req, "openshift-refresh-token", map[interface{}]interface{}{ - "refresh-token-id": id, + "refresh-token": *f.refreshToken, }, f.cookieCodecs) } diff --git a/pkg/auth/oauth2/auth_openshift.go b/pkg/auth/oauth2/auth_openshift.go index f081c563407..35f54949e77 100644 --- a/pkg/auth/oauth2/auth_openshift.go +++ b/pkg/auth/oauth2/auth_openshift.go @@ -72,19 +72,26 @@ func newOpenShiftAuth(ctx context.Context, k8sClient *http.Client, c *oidcConfig } o.oauthEndpointCache.Run(ctx) - authnKey, err := utils.RandomString(64) - if err != nil { - return nil, err - } - - encryptionKey, err := utils.RandomString(32) - if err != nil { - return nil, err + var authnKey, encryptionKey []byte + if len(c.cookieAuthenticationKey) > 0 && len(c.cookieEncryptionKey) > 0 { + authnKey = c.cookieAuthenticationKey + encryptionKey = c.cookieEncryptionKey + } else { + authnKeyStr, err := utils.RandomString(64) + if err != nil { + return nil, err + } + encryptionKeyStr, err := utils.RandomString(32) + if err != nil { + return nil, err + } + authnKey = []byte(authnKeyStr) + encryptionKey = []byte(encryptionKeyStr) } o.sessions = sessions.NewSessionStore( - []byte(authnKey), - []byte(encryptionKey), + authnKey, + encryptionKey, c.secureCookies, c.cookiePath, ) diff --git a/pkg/auth/sessions/combined_sessions.go b/pkg/auth/sessions/combined_sessions.go index 18c9f6ac3b1..6dbdb95c14e 100644 --- a/pkg/auth/sessions/combined_sessions.go +++ b/pkg/auth/sessions/combined_sessions.go @@ -79,8 +79,7 @@ func (cs *CombinedSessionStore) AddSession(w http.ResponseWriter, r *http.Reques clientSession := cs.getCookieSession(r) clientSession.sessionToken.Values["session-token"] = ls.sessionToken - // Store only the small reference ID in the cookie, not the full refresh token - clientSession.refreshToken.Values["refresh-token-id"] = ls.refreshTokenID + clientSession.refreshToken.Values["refresh-token"] = ls.refreshToken return ls, clientSession.save(r, w) } @@ -125,12 +124,13 @@ func (cs *CombinedSessionStore) GetSession(w http.ResponseWriter, r *http.Reques refreshToken string ) - if sessionTokenIface, ok := clientSession.sessionToken.Values["session-token"]; ok { - sessionToken = sessionTokenIface.(string) + if sessionTokenStr, ok := clientSession.sessionToken.Values["session-token"].(string); ok { + sessionToken = sessionTokenStr } - if refreshTokenID, ok := clientSession.refreshToken.Values["refresh-token-id"]; ok { - // Look up the actual refresh token from the ID - if actualToken, exists := cs.serverStore.byRefreshTokenID[refreshTokenID.(string)]; exists { + if rt, ok := clientSession.refreshToken.Values["refresh-token"].(string); ok { + refreshToken = rt + } else if refreshTokenID, ok := clientSession.refreshToken.Values["refresh-token-id"].(string); ok { + if actualToken, exists := cs.serverStore.byRefreshTokenID[refreshTokenID]; exists { refreshToken = actualToken } } @@ -140,10 +140,12 @@ func (cs *CombinedSessionStore) GetSession(w http.ResponseWriter, r *http.Reques } func (cs *CombinedSessionStore) GetCookieRefreshToken(r *http.Request) string { - // Get always returns a session, even if empty. clientSession, _ := cs.clientStore.Get(r, openshiftRefreshTokenCookieName) + if refreshToken, ok := clientSession.Values["refresh-token"].(string); ok { + return refreshToken + } + // Backward compatibility: fall back to reference ID lookup if refreshTokenID, ok := clientSession.Values["refresh-token-id"].(string); ok { - // Look up the actual refresh token using the ID if actualToken, exists := cs.serverStore.byRefreshTokenID[refreshTokenID]; exists { return actualToken } @@ -152,13 +154,9 @@ func (cs *CombinedSessionStore) GetCookieRefreshToken(r *http.Request) string { } func (cs *CombinedSessionStore) UpdateCookieRefreshToken(w http.ResponseWriter, r *http.Request, refreshToken string) error { - // Generate a new ID for the refresh token - newID := RandomString(32) - cs.serverStore.byRefreshTokenID[newID] = refreshToken - - // Store the ID in the cookie, not the full token clientSession, _ := cs.clientStore.Get(r, openshiftRefreshTokenCookieName) - clientSession.Values["refresh-token-id"] = newID + clientSession.Values["refresh-token"] = refreshToken + delete(clientSession.Values, "refresh-token-id") return clientSession.Save(r, w) } @@ -166,35 +164,30 @@ func (cs *CombinedSessionStore) UpdateTokens(w http.ResponseWriter, r *http.Requ cs.sessionLock.Lock() defer cs.sessionLock.Unlock() - // Clean up old session cookies from previous pods when refreshing tokens - // This handles the case where a user is load-balanced to a different pod cs.expireOldPodCookies(w, r) clientSession := cs.getCookieSession(r) - var oldRefreshTokenID string + + // Resolve old refresh token from cookie (new format or legacy ID lookup) var oldRefreshToken string - if oldID, ok := clientSession.refreshToken.Values["refresh-token-id"]; ok { - oldRefreshTokenID = oldID.(string) - // Look up the actual refresh token from the ID - if actualToken, exists := cs.serverStore.byRefreshTokenID[oldRefreshTokenID]; exists { + if rt, ok := clientSession.refreshToken.Values["refresh-token"].(string); ok { + oldRefreshToken = rt + } else if oldID, ok := clientSession.refreshToken.Values["refresh-token-id"].(string); ok { + if actualToken, exists := cs.serverStore.byRefreshTokenID[oldID]; exists { oldRefreshToken = actualToken } + delete(cs.serverStore.byRefreshTokenID, oldID) } - // Generate a new ID for the new refresh token - newRefreshTokenID := RandomString(32) newRefreshToken := tokenResponse.RefreshToken - if newRefreshToken != "" { - cs.serverStore.byRefreshTokenID[newRefreshTokenID] = newRefreshToken - } - // Store the new ID in the cookie - clientSession.refreshToken.Values["refresh-token-id"] = newRefreshTokenID + // Store actual refresh token in cookie + clientSession.refreshToken.Values["refresh-token"] = newRefreshToken + delete(clientSession.refreshToken.Values, "refresh-token-id") var loginState *LoginState - sessionToken, ok := clientSession.sessionToken.Values["session-token"] - if ok { - loginState = cs.serverStore.GetSession(sessionToken.(string), "") + if sessionToken, ok := clientSession.sessionToken.Values["session-token"].(string); ok { + loginState = cs.serverStore.GetSession(sessionToken, "") } if loginState == nil { var err error @@ -203,19 +196,13 @@ func (cs *CombinedSessionStore) UpdateTokens(w http.ResponseWriter, r *http.Requ return nil, fmt.Errorf("failed to add session to server store: %w", err) } clientSession.sessionToken.Values["session-token"] = loginState.sessionToken - // AddSession already generated an ID, so update the cookie with it - clientSession.refreshToken.Values["refresh-token-id"] = loginState.refreshTokenID } else { - // loginState is a pointer to the cache so this effectively mutates it for everyone if err := loginState.UpdateTokens(tokenVerifier, tokenResponse); err != nil { return nil, err } - // Update the ID in the LoginState - loginState.refreshTokenID = newRefreshTokenID } - // index by the old refresh token so that any follow-up requests that arrived - // before their cookie was updated with an actual session can still find the login state + // Index by old refresh token for in-flight requests with stale cookies if oldRefreshToken != "" { cs.serverStore.byRefreshToken[oldRefreshToken] = loginState } @@ -241,18 +228,17 @@ func (cs *CombinedSessionStore) DeleteSession(w http.ResponseWriter, r *http.Req } cookieSession := cs.getCookieSession(r) - if refreshTokenID, ok := cookieSession.refreshToken.Values["refresh-token-id"]; ok { - refreshTokenIDStr := refreshTokenID.(string) - // Look up the actual refresh token from the ID and delete + if refreshToken, ok := cookieSession.refreshToken.Values["refresh-token"].(string); ok && refreshToken != "" { + cs.serverStore.DeleteByRefreshToken(refreshToken) + } else if refreshTokenIDStr, ok := cookieSession.refreshToken.Values["refresh-token-id"].(string); ok { if actualToken, exists := cs.serverStore.byRefreshTokenID[refreshTokenIDStr]; exists { cs.serverStore.DeleteByRefreshToken(actualToken) - // Clean up the ID mapping delete(cs.serverStore.byRefreshTokenID, refreshTokenIDStr) } } - if sessionToken, ok := cookieSession.sessionToken.Values["session-token"]; ok { - cs.serverStore.DeleteBySessionToken(sessionToken.(string)) + if sessionToken, ok := cookieSession.sessionToken.Values["session-token"].(string); ok { + cs.serverStore.DeleteBySessionToken(sessionToken) } refreshTokenCookie, _ := cs.clientStore.Get(r, openshiftRefreshTokenCookieName) diff --git a/pkg/auth/sessions/combined_sessions_test.go b/pkg/auth/sessions/combined_sessions_test.go index 268d210e03e..3483f0bcbf0 100644 --- a/pkg/auth/sessions/combined_sessions_test.go +++ b/pkg/auth/sessions/combined_sessions_test.go @@ -166,11 +166,9 @@ func TestCombinedSessionStore_AddSession(t *testing.T) { refreshFound = true gotRefresh := make(map[interface{}]interface{}) require.NoError(t, securecookie.DecodeMulti(openshiftRefreshTokenCookieName, c.Value, &gotRefresh, cookieCodecs...)) - // The cookie now contains an ID, not the actual refresh token - refreshTokenID := gotRefresh["refresh-token-id"].(string) - actualRefreshToken := cs.serverStore.byRefreshTokenID[refreshTokenID] + actualRefreshToken := gotRefresh["refresh-token"].(string) if actualRefreshToken != tt.wantRefreshToken { - t.Errorf("wanted refresh token to be %q, got %q (via ID %q)", tt.wantRefreshToken, actualRefreshToken, refreshTokenID) + t.Errorf("wanted refresh token to be %q, got %q", tt.wantRefreshToken, actualRefreshToken) } } } @@ -268,6 +266,36 @@ func TestCombinedSessionStore_GetSession(t *testing.T) { } } +func TestCombinedSessionStore_GetSession_LegacyCookie(t *testing.T) { + encryptionKey := []byte(randomString(32)) + authnKey := []byte(randomString(64)) + cookieCodecs := securecookie.CodecsFromPairs(authnKey, encryptionKey) + + testServerSessions := NewServerSessionStore(10) + testServerSessions.byToken["1"] = &LoginState{sessionToken: "1"} + + cs := NewSessionStore(authnKey, encryptionKey, true, "/") + cs.serverStore = testServerSessions + + testCookies := &testCookieFactory{ + cookieCodecs: cookieCodecs, + serverStore: cs.serverStore, + } + + req, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) + + testCookies.WithRefreshToken("refresh-old").WithLegacyFormat() + req = testCookies.Complete(t, req) + + testWriter := httptest.NewRecorder() + got, err := cs.GetSession(testWriter, req) + require.NoError(t, err) + + // Legacy format should resolve through byRefreshTokenID map + require.Nil(t, got, "should not find session by refresh token alone without byRefreshToken mapping") +} + func addIDToken(t *oauth2.Token, idtoken string) *oauth2.Token { extra := map[string]interface{}{ "id_token": idtoken, @@ -623,12 +651,12 @@ func TestCombinedSessionStore_DeleteSession(t *testing.T) { } type testCookieFactory struct { - cookieCodecs []securecookie.Codec - sessionToken *string - refreshToken *string - refreshTokenID *string - customCookies map[string]map[interface{}]interface{} - serverStore *SessionStore // needed to set up refresh token ID mapping + cookieCodecs []securecookie.Codec + sessionToken *string + refreshToken *string + useLegacyFormat bool // use old refresh-token-id format for backward compat testing + customCookies map[string]map[interface{}]interface{} + serverStore *SessionStore } func (f *testCookieFactory) WithSessionToken(sessionToken string) *testCookieFactory { @@ -649,6 +677,11 @@ func (f *testCookieFactory) WithCustomCookie(cookieName string, cookieValue map[ return f } +func (f *testCookieFactory) WithLegacyFormat() *testCookieFactory { + f.useLegacyFormat = true + return f +} + func (f *testCookieFactory) Complete(t *testing.T, req *http.Request) *http.Request { if f.sessionToken != nil { attachCookieOrDie(t, req, SessionCookieName(), @@ -658,19 +691,23 @@ func (f *testCookieFactory) Complete(t *testing.T, req *http.Request) *http.Requ f.cookieCodecs) } if f.refreshToken != nil { - // Generate an ID for the refresh token and store the mapping - id := randomString(32) - if f.serverStore != nil { - f.serverStore.byRefreshTokenID[id] = *f.refreshToken + if f.useLegacyFormat { + id := randomString(32) + if f.serverStore != nil { + f.serverStore.byRefreshTokenID[id] = *f.refreshToken + } + attachCookieOrDie(t, req, openshiftRefreshTokenCookieName, + map[interface{}]interface{}{ + "refresh-token-id": id, + }, + f.cookieCodecs) + } else { + attachCookieOrDie(t, req, openshiftRefreshTokenCookieName, + map[interface{}]interface{}{ + "refresh-token": *f.refreshToken, + }, + f.cookieCodecs) } - f.refreshTokenID = &id - - // Store only the ID in the cookie - attachCookieOrDie(t, req, openshiftRefreshTokenCookieName, - map[interface{}]interface{}{ - "refresh-token-id": id, - }, - f.cookieCodecs) } for cookieName, cookieValue := range f.customCookies { diff --git a/pkg/auth/sessions/loginstate.go b/pkg/auth/sessions/loginstate.go index b30df54f0fa..326fa75b6ee 100644 --- a/pkg/auth/sessions/loginstate.go +++ b/pkg/auth/sessions/loginstate.go @@ -29,8 +29,7 @@ type LoginState struct { now nowFunc sessionToken string rawToken string - refreshToken string - refreshTokenID string // Small reference ID for the refresh token (stored in cookie) + refreshToken string } type LoginJSON struct { @@ -165,10 +164,6 @@ func (ls *LoginState) RefreshToken() string { return ls.refreshToken } -func (ls *LoginState) RefreshTokenID() string { - return ls.refreshTokenID -} - func (ls *LoginState) IsExpired() bool { return ls.now().After(ls.exp) } diff --git a/pkg/auth/sessions/server_session.go b/pkg/auth/sessions/server_session.go index 321b8e9dcab..671ca1a874d 100644 --- a/pkg/auth/sessions/server_session.go +++ b/pkg/auth/sessions/server_session.go @@ -59,14 +59,8 @@ func (ss *SessionStore) AddSession(tokenVerifier IDTokenVerifier, token *oauth2. } ls.sessionToken = sessionToken - // Generate a small reference ID for the refresh token (stored in cookie instead of full token) - ls.refreshTokenID = RandomString(32) - ss.mux.Lock() ss.byToken[sessionToken] = ls - if ls.refreshToken != "" { - ss.byRefreshTokenID[ls.refreshTokenID] = ls.refreshToken - } // Assume token expiration is always the same time in the future. Should be close enough for government work. ss.byAge = append(ss.byAge, ls) From 9d72995c431fcf22d64dc11bf5538276d9996214 Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Wed, 5 Aug 2026 19:49:56 +0200 Subject: [PATCH 2/9] OCPBUGS-71237: Address security review findings - Remove refresh token value from error log message to prevent credential leakage in pod logs - Revoke refresh token (OAuthAuthorizeToken) at the OAuth server on logout to prevent cookie replay attacks - Raise securecookie MaxLength to 8192 to accommodate large OIDC JWT refresh tokens that exceed the default 4096 limit Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/auth/oauth2/auth_openshift.go | 9 +++++++-- pkg/auth/sessions/combined_sessions.go | 7 +++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/auth/oauth2/auth_openshift.go b/pkg/auth/oauth2/auth_openshift.go index 35f54949e77..9950a5c063b 100644 --- a/pkg/auth/oauth2/auth_openshift.go +++ b/pkg/auth/oauth2/auth_openshift.go @@ -213,7 +213,12 @@ func (o *openShiftAuth) logout(w http.ResponseWriter, r *http.Request) { return } - // Delete the session + if refreshToken := ls.RefreshToken(); refreshToken != "" { + if delErr := oauthClient.OAuthAuthorizeTokens().Delete(ctx, tokenToObjectName(refreshToken), metav1.DeleteOptions{}); delErr != nil { + klog.V(4).Infof("failed to revoke refresh token on logout: %v", delErr) + } + } + o.sessions.DeleteSession(w, r) w.WriteHeader(http.StatusNoContent) } @@ -241,7 +246,7 @@ func (o *openShiftAuth) refreshSession(ctx context.Context, w http.ResponseWrite ).Token() if err != nil { - return nil, fmt.Errorf("failed to refresh a token %s: %w", cookieRefreshToken, err) + return nil, fmt.Errorf("failed to refresh a token: %w", err) } ls, err := o.sessions.UpdateTokens(w, r, nil, newTokens) diff --git a/pkg/auth/sessions/combined_sessions.go b/pkg/auth/sessions/combined_sessions.go index 6dbdb95c14e..7a641d979db 100644 --- a/pkg/auth/sessions/combined_sessions.go +++ b/pkg/auth/sessions/combined_sessions.go @@ -7,6 +7,7 @@ import ( "strings" "sync" + "github.com/gorilla/securecookie" gorilla "github.com/gorilla/sessions" "golang.org/x/oauth2" ) @@ -35,6 +36,12 @@ func NewSessionStore(authnKey, encryptKey []byte, secureCookies bool, cookiePath clientStore.Options.SameSite = http.SameSiteStrictMode clientStore.Options.Path = cookiePath + for _, codec := range clientStore.Codecs { + if sc, ok := codec.(*securecookie.SecureCookie); ok { + sc.MaxLength(8192) + } + } + return &CombinedSessionStore{ serverStore: NewServerSessionStore(32768), clientStore: clientStore, From e2007911dd27145318a40933fca72744e30e0d47 Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Wed, 5 Aug 2026 20:52:34 +0200 Subject: [PATCH 3/9] OCPBUGS-71237: Add e2e test for session persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two test cases: - Session survives console pod deletion and replacement - Session survives plugin toggle (operator-triggered rollout) Both tests log in (kubeadmin by default, htpasswd when env vars are set), trigger a console pod restart, and assert the user is still authenticated without a login redirect. Tagged @slow — pod rollout takes 30-60s per test. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../tests/console/session-persistence.spec.ts | 159 ++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 frontend/e2e/tests/console/session-persistence.spec.ts diff --git a/frontend/e2e/tests/console/session-persistence.spec.ts b/frontend/e2e/tests/console/session-persistence.spec.ts new file mode 100644 index 00000000000..ea00b02281c --- /dev/null +++ b/frontend/e2e/tests/console/session-persistence.spec.ts @@ -0,0 +1,159 @@ +import { test, expect } from '../../fixtures'; +import { performLogin } from '../../setup/login-helper'; + +const CONSOLE_NAMESPACE = 'openshift-console'; +const CONSOLE_DEPLOYMENT = 'console'; + +test.describe( + 'Session persistence across pod restarts', + { tag: ['@admin', '@slow'] }, + () => { + test.setTimeout(300_000); + + test('session survives console pod deletion', async ({ page, k8sClient }) => { + const baseURL = process.env.WEB_CONSOLE_URL || page.url() || 'http://localhost:9000'; + + await test.step('Log in to the console', async () => { + const htpasswdUser = process.env.BRIDGE_HTPASSWD_USERNAME; + const htpasswdPass = process.env.BRIDGE_HTPASSWD_PASSWORD; + const htpasswdIdp = process.env.BRIDGE_HTPASSWD_IDP; + + if (htpasswdUser && htpasswdPass) { + await performLogin(page, baseURL, htpasswdUser, htpasswdPass, htpasswdIdp); + } else { + const kubeadminPassword = process.env.BRIDGE_KUBEADMIN_PASSWORD; + test.skip(!kubeadminPassword, 'No credentials configured'); + await performLogin(page, baseURL, 'kubeadmin', kubeadminPassword!, 'kube:admin'); + } + + await expect(page.getByTestId('user-dropdown-toggle')).toBeVisible({ timeout: 60_000 }); + }); + + await test.step('Verify dashboard loads', async () => { + await page.goto(`${baseURL}/dashboards`, { waitUntil: 'domcontentloaded' }); + await expect(page).toHaveTitle(/Overview/); + }); + + await test.step('Delete all console pods', async () => { + const pods = await k8sClient.getPods(CONSOLE_NAMESPACE); + const consolePods = pods.filter( + (p) => p.metadata?.labels?.['component'] === 'ui', + ); + + expect(consolePods.length).toBeGreaterThan(0); + + for (const pod of consolePods) { + await k8sClient.deletePod(pod.metadata!.name!, CONSOLE_NAMESPACE); + } + }); + + await test.step('Wait for new console pods to be ready', async () => { + await k8sClient.waitForDeploymentReady(CONSOLE_DEPLOYMENT, CONSOLE_NAMESPACE, 180_000); + }); + + await test.step('Verify session persisted — no login redirect', async () => { + await page.goto(`${baseURL}/k8s/cluster/nodes`, { + waitUntil: 'domcontentloaded', + timeout: 60_000, + }); + + await expect(page.getByTestId('user-dropdown-toggle')).toBeVisible({ timeout: 30_000 }); + await expect(page).toHaveTitle(/Nodes/); + expect(page.url()).not.toContain('oauth-openshift'); + expect(page.url()).not.toContain('/login'); + }); + }); + + test('session survives console plugin toggle', async ({ page, k8sClient }) => { + const baseURL = process.env.WEB_CONSOLE_URL || page.url() || 'http://localhost:9000'; + + await test.step('Log in to the console', async () => { + const htpasswdUser = process.env.BRIDGE_HTPASSWD_USERNAME; + const htpasswdPass = process.env.BRIDGE_HTPASSWD_PASSWORD; + const htpasswdIdp = process.env.BRIDGE_HTPASSWD_IDP; + + if (htpasswdUser && htpasswdPass) { + await performLogin(page, baseURL, htpasswdUser, htpasswdPass, htpasswdIdp); + } else { + const kubeadminPassword = process.env.BRIDGE_KUBEADMIN_PASSWORD; + test.skip(!kubeadminPassword, 'No credentials configured'); + await performLogin(page, baseURL, 'kubeadmin', kubeadminPassword!, 'kube:admin'); + } + + await expect(page.getByTestId('user-dropdown-toggle')).toBeVisible({ timeout: 60_000 }); + }); + + let pluginName: string | undefined; + + await test.step('Find an enabled ConsolePlugin to toggle', async () => { + const consoleOperator = await k8sClient.customObjectsApi.getClusterCustomObject({ + group: 'operator.openshift.io', + version: 'v1', + plural: 'consoles', + name: 'cluster', + }); + + const plugins: string[] = + (consoleOperator.body as any)?.spec?.plugins ?? []; + pluginName = plugins[0]; + test.skip(!pluginName, 'No enabled ConsolePlugins found on this cluster'); + }); + + await test.step('Disable the plugin via operator config', async () => { + const consoleOperator = await k8sClient.customObjectsApi.getClusterCustomObject({ + group: 'operator.openshift.io', + version: 'v1', + plural: 'consoles', + name: 'cluster', + }); + + const currentPlugins: string[] = + (consoleOperator.body as any)?.spec?.plugins ?? []; + const updatedPlugins = currentPlugins.filter((p: string) => p !== pluginName); + + await k8sClient.mergePatchResource( + '/apis/operator.openshift.io/v1/consoles/cluster', + { spec: { plugins: updatedPlugins } }, + ); + }); + + await test.step('Wait for console rollout', async () => { + // The operator triggers a new rollout when plugin config changes + // Wait briefly for the rollout to start, then wait for it to complete + await page.waitForTimeout(10_000); + await k8sClient.waitForDeploymentReady(CONSOLE_DEPLOYMENT, CONSOLE_NAMESPACE, 180_000); + }); + + await test.step('Verify session persisted after plugin toggle', async () => { + await page.goto(`${baseURL}/dashboards`, { + waitUntil: 'domcontentloaded', + timeout: 60_000, + }); + + await expect(page.getByTestId('user-dropdown-toggle')).toBeVisible({ timeout: 30_000 }); + await expect(page).toHaveTitle(/Overview/); + expect(page.url()).not.toContain('oauth-openshift'); + expect(page.url()).not.toContain('/login'); + }); + + await test.step('Re-enable the plugin', async () => { + const consoleOperator = await k8sClient.customObjectsApi.getClusterCustomObject({ + group: 'operator.openshift.io', + version: 'v1', + plural: 'consoles', + name: 'cluster', + }); + + const currentPlugins: string[] = + (consoleOperator.body as any)?.spec?.plugins ?? []; + if (!currentPlugins.includes(pluginName!)) { + currentPlugins.push(pluginName!); + await k8sClient.mergePatchResource( + '/apis/operator.openshift.io/v1/consoles/cluster', + { spec: { plugins: currentPlugins } }, + ); + } + }); + }); + }, +); From 8afcc78205b0ded7368d0a018371d1d9683c01a8 Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Fri, 7 Aug 2026 12:35:32 +0200 Subject: [PATCH 4/9] OCPBUGS-71237: Add access token recovery cookie for OpenShift auth OpenShift's internal OAuth server does not support refresh tokens, so the refresh-token-in-cookie approach only works for OIDC auth. For OpenShift auth, store the encrypted access token + expiry in a separate recovery cookie (openshift-recovery-token). After pod restart, the backend reads the token from the cookie, validates expiry locally, and creates a new server-side session transparently. No OAuth server interaction or page redirect needed. - SetRecoveryCookie/GetRecoveryCookie/ClearRecoveryCookie methods - recoverSession() called in getLoginState() when session is nil - Recovery cookie cleared on logout and DeleteSession - MaxAge set to match token expiry - Unit tests for recovery cookie lifecycle Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/auth/oauth2/auth_openshift.go | 37 ++++++++++ pkg/auth/sessions/combined_sessions.go | 42 ++++++++++- pkg/auth/sessions/combined_sessions_test.go | 79 ++++++++++++++++++++- pkg/auth/sessions/server_session.go | 1 + 4 files changed, 155 insertions(+), 4 deletions(-) diff --git a/pkg/auth/oauth2/auth_openshift.go b/pkg/auth/oauth2/auth_openshift.go index 9950a5c063b..590bef0884a 100644 --- a/pkg/auth/oauth2/auth_openshift.go +++ b/pkg/auth/oauth2/auth_openshift.go @@ -168,11 +168,16 @@ func (o *openShiftAuth) login(w http.ResponseWriter, r *http.Request, token *oau return nil, fmt.Errorf("failed to create session: %w", err) } + if err := o.sessions.SetRecoveryCookie(w, r, token.AccessToken, token.Expiry); err != nil { + klog.V(4).Infof("failed to set recovery cookie: %v", err) + } + return ls, nil } func (o *openShiftAuth) DeleteSession(w http.ResponseWriter, r *http.Request) { o.sessions.DeleteSession(w, r) + o.sessions.ClearRecoveryCookie(w, r) } func (o *openShiftAuth) logout(w http.ResponseWriter, r *http.Request) { @@ -220,6 +225,7 @@ func (o *openShiftAuth) logout(w http.ResponseWriter, r *http.Request) { } o.sessions.DeleteSession(w, r) + o.sessions.ClearRecoveryCookie(w, r) w.WriteHeader(http.StatusNoContent) } @@ -268,11 +274,42 @@ func (o *openShiftAuth) getLoginState(w http.ResponseWriter, r *http.Request) (* return o.refreshSession(r.Context(), w, r, o.oauth2Config(), refreshToken) } + if ls == nil { + if recovered, recoverErr := o.recoverSession(w, r); recoverErr == nil { + return recovered, nil + } + } + return nil, fmt.Errorf("a session was not found on server or is expired") } return ls, nil } +func (o *openShiftAuth) recoverSession(w http.ResponseWriter, r *http.Request) (*sessions.LoginState, error) { + accessToken, expiry, ok := o.sessions.GetRecoveryCookie(r) + if !ok { + return nil, fmt.Errorf("no recovery cookie") + } + + if time.Now().After(expiry) { + o.sessions.ClearRecoveryCookie(w, r) + return nil, fmt.Errorf("recovery token expired") + } + + token := &oauth2.Token{ + AccessToken: accessToken, + Expiry: expiry, + } + + ls, err := o.sessions.AddSession(w, r, nil, token) + if err != nil { + return nil, fmt.Errorf("failed to recover session: %w", err) + } + + klog.V(4).Info("session recovered from cookie after pod restart") + return ls, nil +} + func (o *openShiftAuth) LogoutRedirectURL() string { return o.logoutRedirectOverride } diff --git a/pkg/auth/sessions/combined_sessions.go b/pkg/auth/sessions/combined_sessions.go index 7a641d979db..4b7666b9d3b 100644 --- a/pkg/auth/sessions/combined_sessions.go +++ b/pkg/auth/sessions/combined_sessions.go @@ -6,6 +6,7 @@ import ( "os" "strings" "sync" + "time" "github.com/gorilla/securecookie" gorilla "github.com/gorilla/sessions" @@ -250,14 +251,51 @@ func (cs *CombinedSessionStore) DeleteSession(w http.ResponseWriter, r *http.Req refreshTokenCookie, _ := cs.clientStore.Get(r, openshiftRefreshTokenCookieName) if !refreshTokenCookie.IsNew { - // Get always returns a session, only timeout current sessions refreshTokenCookie.Options.MaxAge = -1 - return cs.clientStore.Save(r, w, refreshTokenCookie) + if err := cs.clientStore.Save(r, w, refreshTokenCookie); err != nil { + return err + } } return nil } +func (cs *CombinedSessionStore) SetRecoveryCookie(w http.ResponseWriter, r *http.Request, accessToken string, expiry time.Time) error { + s, _ := cs.clientStore.Get(r, openshiftRecoveryTokenCookieName) + s.Values["access-token"] = accessToken + s.Values["expiry"] = expiry.Unix() + maxAge := int(time.Until(expiry).Seconds()) + if maxAge > 0 { + s.Options.MaxAge = maxAge + } + return s.Save(r, w) +} + +func (cs *CombinedSessionStore) GetRecoveryCookie(r *http.Request) (string, time.Time, bool) { + s, _ := cs.clientStore.Get(r, openshiftRecoveryTokenCookieName) + accessToken, ok := s.Values["access-token"].(string) + if !ok || accessToken == "" { + return "", time.Time{}, false + } + expiryUnix, ok := s.Values["expiry"].(int64) + if !ok { + return "", time.Time{}, false + } + return accessToken, time.Unix(expiryUnix, 0), true +} + +func (cs *CombinedSessionStore) ClearRecoveryCookie(w http.ResponseWriter, r *http.Request) { + http.SetCookie(w, &http.Cookie{ + Name: openshiftRecoveryTokenCookieName, + Value: "", + Path: cs.clientStore.Options.Path, + MaxAge: -1, + Secure: cs.clientStore.Options.Secure, + HttpOnly: cs.clientStore.Options.HttpOnly, + SameSite: cs.clientStore.Options.SameSite, + }) +} + // ServerStore returns the underlying server session store. // This is primarily used for testing purposes. func (cs *CombinedSessionStore) ServerStore() *SessionStore { diff --git a/pkg/auth/sessions/combined_sessions_test.go b/pkg/auth/sessions/combined_sessions_test.go index 3483f0bcbf0..2090274a387 100644 --- a/pkg/auth/sessions/combined_sessions_test.go +++ b/pkg/auth/sessions/combined_sessions_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "reflect" "strconv" + "strings" "testing" "time" @@ -296,6 +297,72 @@ func TestCombinedSessionStore_GetSession_LegacyCookie(t *testing.T) { require.Nil(t, got, "should not find session by refresh token alone without byRefreshToken mapping") } +func TestCombinedSessionStore_RecoveryCookie(t *testing.T) { + encryptionKey := []byte(randomString(32)) + authnKey := []byte(randomString(64)) + + cs := NewSessionStore(authnKey, encryptionKey, false, "/") + + accessToken := "sha256~test-access-token-12345" + expiry := time.Now().Add(24 * time.Hour) + + t.Run("set and get recovery cookie", func(t *testing.T) { + req, _ := http.NewRequest(http.MethodGet, "/", nil) + w := httptest.NewRecorder() + + err := cs.SetRecoveryCookie(w, req, accessToken, expiry) + require.NoError(t, err) + + // Build a new request with the cookie from the response + req2, _ := http.NewRequest(http.MethodGet, "/", nil) + for _, c := range w.Result().Cookies() { + req2.AddCookie(c) + } + + gotToken, gotExpiry, ok := cs.GetRecoveryCookie(req2) + require.True(t, ok) + require.Equal(t, accessToken, gotToken) + require.Equal(t, expiry.Unix(), gotExpiry.Unix()) + }) + + t.Run("get recovery cookie from empty request", func(t *testing.T) { + req, _ := http.NewRequest(http.MethodGet, "/", nil) + _, _, ok := cs.GetRecoveryCookie(req) + require.False(t, ok) + }) + + t.Run("clear recovery cookie", func(t *testing.T) { + req, _ := http.NewRequest(http.MethodGet, "/", nil) + w := httptest.NewRecorder() + + cs.ClearRecoveryCookie(w, req) + + cookies := w.Result().Cookies() + require.Len(t, cookies, 1) + require.Equal(t, openshiftRecoveryTokenCookieName, cookies[0].Name) + require.Equal(t, -1, cookies[0].MaxAge) + }) + + t.Run("recovery cookie with expired token", func(t *testing.T) { + req, _ := http.NewRequest(http.MethodGet, "/", nil) + w := httptest.NewRecorder() + + pastExpiry := time.Now().Add(-1 * time.Hour) + err := cs.SetRecoveryCookie(w, req, accessToken, pastExpiry) + require.NoError(t, err) + + req2, _ := http.NewRequest(http.MethodGet, "/", nil) + for _, c := range w.Result().Cookies() { + req2.AddCookie(c) + } + + gotToken, gotExpiry, ok := cs.GetRecoveryCookie(req2) + require.True(t, ok, "cookie should be readable even if token is expired") + require.Equal(t, accessToken, gotToken) + require.True(t, time.Now().After(gotExpiry), "expiry should be in the past") + }) +} + func addIDToken(t *oauth2.Token, idtoken string) *oauth2.Token { extra := map[string]interface{}{ "id_token": idtoken, @@ -598,8 +665,14 @@ func TestCombinedSessionStore_DeleteSession(t *testing.T) { } setCookies := testWriter.Result().Header.Values("Set-Cookie") - if len(tt.wantCookieTimeouts) == 0 && len(setCookies) > 0 { - t.Errorf("CombinedSessionStore.DeleteSession() unexpected cookies set: %v", setCookies) + nonRecoveryCookies := make([]string, 0, len(setCookies)) + for _, c := range setCookies { + if !strings.HasPrefix(c, openshiftRecoveryTokenCookieName+"=") { + nonRecoveryCookies = append(nonRecoveryCookies, c) + } + } + if len(tt.wantCookieTimeouts) == 0 && len(nonRecoveryCookies) > 0 { + t.Errorf("CombinedSessionStore.DeleteSession() unexpected cookies set: %v", nonRecoveryCookies) } gotCookies := map[string]*http.Cookie{} @@ -608,6 +681,8 @@ func TestCombinedSessionStore_DeleteSession(t *testing.T) { gotCookies[c.Name] = c } } + // Recovery cookie is always cleared on delete — not part of per-test assertions + delete(gotCookies, openshiftRecoveryTokenCookieName) for _, cookieName := range tt.wantCookieTimeouts { cookie, ok := gotCookies[cookieName] diff --git a/pkg/auth/sessions/server_session.go b/pkg/auth/sessions/server_session.go index 671ca1a874d..0fbbbca5e23 100644 --- a/pkg/auth/sessions/server_session.go +++ b/pkg/auth/sessions/server_session.go @@ -16,6 +16,7 @@ import ( const ( OpenshiftAccessTokenCookieName = "openshift-session-token" openshiftRefreshTokenCookieName = "openshift-refresh-token" + openshiftRecoveryTokenCookieName = "openshift-recovery-token" ) var sessionPruningPeriod = 5 * time.Minute From ae77e5ef251f4e125c6bc7a3557ec2a0eb200216 Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Mon, 10 Aug 2026 15:06:38 +0200 Subject: [PATCH 5/9] OCPBUGS-71237: Fix CI lint failures - gofmt formatting on 4 Go files - Replace page.waitForTimeout with pod deletion in e2e test to satisfy playwright/no-wait-for-timeout ESLint rule Co-Authored-By: Claude Opus 4.6 (1M context) --- .../tests/console/session-persistence.spec.ts | 12 +++++++++--- pkg/auth/oauth2/auth.go | 18 +++++++++--------- pkg/auth/oauth2/auth_oidc.go | 18 +++++++++--------- pkg/auth/sessions/loginstate.go | 16 ++++++++-------- pkg/auth/sessions/server_session.go | 4 ++-- 5 files changed, 37 insertions(+), 31 deletions(-) diff --git a/frontend/e2e/tests/console/session-persistence.spec.ts b/frontend/e2e/tests/console/session-persistence.spec.ts index ea00b02281c..d590b84bcf5 100644 --- a/frontend/e2e/tests/console/session-persistence.spec.ts +++ b/frontend/e2e/tests/console/session-persistence.spec.ts @@ -118,9 +118,15 @@ test.describe( }); await test.step('Wait for console rollout', async () => { - // The operator triggers a new rollout when plugin config changes - // Wait briefly for the rollout to start, then wait for it to complete - await page.waitForTimeout(10_000); + // The operator triggers a new rollout when plugin config changes. + // Delete the console pods to force immediate restart, then wait for readiness. + const pods = await k8sClient.getPods(CONSOLE_NAMESPACE); + const consolePods = pods.filter( + (p) => p.metadata?.labels?.['component'] === 'ui', + ); + for (const pod of consolePods) { + await k8sClient.deletePod(pod.metadata!.name!, CONSOLE_NAMESPACE); + } await k8sClient.waitForDeploymentReady(CONSOLE_DEPLOYMENT, CONSOLE_NAMESPACE, 180_000); }); diff --git a/pkg/auth/oauth2/auth.go b/pkg/auth/oauth2/auth.go index 2b1194c49bc..0664e329bb7 100644 --- a/pkg/auth/oauth2/auth.go +++ b/pkg/auth/oauth2/auth.go @@ -206,16 +206,16 @@ func NewOAuth2Authenticator(ctx context.Context, config *Config) (*OAuth2Authent a := newUnstartedAuthenticator(c) authConfig := &oidcConfig{ - getClient: a.clientFunc, - issuerURL: c.IssuerURL, - logoutRedirectOverride: c.LogoutRedirectOverride, - clientID: c.ClientID, - consoleBaseAddress: c.ConsoleBaseAddress, - cookiePath: c.CookiePath, - secureCookies: c.SecureCookies, + getClient: a.clientFunc, + issuerURL: c.IssuerURL, + logoutRedirectOverride: c.LogoutRedirectOverride, + clientID: c.ClientID, + consoleBaseAddress: c.ConsoleBaseAddress, + cookiePath: c.CookiePath, + secureCookies: c.SecureCookies, cookieAuthenticationKey: c.CookieAuthenticationKey, - cookieEncryptionKey: c.CookieEncryptionKey, - constructOAuth2Config: a.oauth2ConfigConstructor, + cookieEncryptionKey: c.CookieEncryptionKey, + constructOAuth2Config: a.oauth2ConfigConstructor, } var tokenHandler loginMethod diff --git a/pkg/auth/oauth2/auth_oidc.go b/pkg/auth/oauth2/auth_oidc.go index bce65c533af..5c8d388e0f4 100644 --- a/pkg/auth/oauth2/auth_oidc.go +++ b/pkg/auth/oauth2/auth_oidc.go @@ -34,16 +34,16 @@ type oidcAuth struct { } type oidcConfig struct { - getClient func() *http.Client - issuerURL string - logoutRedirectOverride string - clientID string - consoleBaseAddress string - cookiePath string - secureCookies bool + getClient func() *http.Client + issuerURL string + logoutRedirectOverride string + clientID string + consoleBaseAddress string + cookiePath string + secureCookies bool cookieAuthenticationKey []byte - cookieEncryptionKey []byte - constructOAuth2Config oauth2ConfigConstructor + cookieEncryptionKey []byte + constructOAuth2Config oauth2ConfigConstructor } func newOIDCAuth(ctx context.Context, sessionStore *sessions.CombinedSessionStore, c *oidcConfig, metrics *auth.Metrics) (*oidcAuth, error) { diff --git a/pkg/auth/sessions/loginstate.go b/pkg/auth/sessions/loginstate.go index 326fa75b6ee..b17ed47d106 100644 --- a/pkg/auth/sessions/loginstate.go +++ b/pkg/auth/sessions/loginstate.go @@ -21,14 +21,14 @@ type IDTokenVerifier func(context.Context, string) (*oidc.IDToken, error) // and should be safe to send as a non-http-only cookie. type LoginState struct { // IMPORTANT: if adding any ref type, change the DeepCopy() implementation - userID string - name string - email string - exp time.Time - rotateAt time.Time // 80% of token's lifetime - now nowFunc - sessionToken string - rawToken string + userID string + name string + email string + exp time.Time + rotateAt time.Time // 80% of token's lifetime + now nowFunc + sessionToken string + rawToken string refreshToken string } diff --git a/pkg/auth/sessions/server_session.go b/pkg/auth/sessions/server_session.go index 0fbbbca5e23..2a55d238848 100644 --- a/pkg/auth/sessions/server_session.go +++ b/pkg/auth/sessions/server_session.go @@ -14,8 +14,8 @@ import ( ) const ( - OpenshiftAccessTokenCookieName = "openshift-session-token" - openshiftRefreshTokenCookieName = "openshift-refresh-token" + OpenshiftAccessTokenCookieName = "openshift-session-token" + openshiftRefreshTokenCookieName = "openshift-refresh-token" openshiftRecoveryTokenCookieName = "openshift-recovery-token" ) From 5d9e366518087e0788474758261946406552a849 Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Tue, 11 Aug 2026 12:17:19 +0200 Subject: [PATCH 6/9] OCPBUGS-71237: Handle missing session key files gracefully During initial cluster install, the console config may reference session key file paths before the session-secret Secret is created and mounted. For OpenShift auth, treat missing key files as non-fatal (log warning, fall back to random keys) instead of crashing the console process. OIDC auth retains the fatal error since key files are required for that mode. This fixes the bootstrap timing issue where the console deployment fails with ProgressDeadlineExceeded because pods crash before the operator creates the session-secret. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/bridge/config/session/sessionoptions.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/cmd/bridge/config/session/sessionoptions.go b/cmd/bridge/config/session/sessionoptions.go index 34620fe88c9..9a65c86595b 100644 --- a/cmd/bridge/config/session/sessionoptions.go +++ b/cmd/bridge/config/session/sessionoptions.go @@ -6,6 +6,7 @@ import ( "os" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/klog/v2" "github.com/openshift/console/cmd/bridge/config/flagvalues" "github.com/openshift/console/pkg/serverconfig" @@ -75,17 +76,27 @@ func (opts *SessionOptions) Complete(userAuthType flagvalues.AuthType) (*Complet if len(opts.CookieEncryptionKeyPath) > 0 { encKey, err := os.ReadFile(opts.CookieEncryptionKeyPath) if err != nil { - return nil, fmt.Errorf("failed to open cookie encryption key file %q: %w", opts.CookieEncryptionKeyPath, err) + if userAuthType == flagvalues.AuthTypeOpenShift { + klog.Warningf("could not read cookie encryption key file %q, falling back to random keys: %v", opts.CookieEncryptionKeyPath, err) + } else { + return nil, fmt.Errorf("failed to open cookie encryption key file %q: %w", opts.CookieEncryptionKeyPath, err) + } + } else { + completed.CookieEncryptionKey = encKey } - completed.CookieEncryptionKey = encKey } if len(opts.CookieAuthenticationKeyPath) > 0 { authnKey, err := os.ReadFile(opts.CookieAuthenticationKeyPath) if err != nil { - return nil, fmt.Errorf("failed to open cookie authentication key file %q: %w", opts.CookieAuthenticationKeyPath, err) + if userAuthType == flagvalues.AuthTypeOpenShift { + klog.Warningf("could not read cookie authentication key file %q, falling back to random keys: %v", opts.CookieAuthenticationKeyPath, err) + } else { + return nil, fmt.Errorf("failed to open cookie authentication key file %q: %w", opts.CookieAuthenticationKeyPath, err) + } + } else { + completed.CookieAuthenticationKey = authnKey } - completed.CookieAuthenticationKey = authnKey } return &CompletedOptions{ From 07b08326afacf6a55bc24b157d48417676edd9b0 Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Wed, 12 Aug 2026 15:06:48 +0200 Subject: [PATCH 7/9] OCPBUGS-71237: Address CodeRabbit review findings - Move local session cleanup to defer in logout() so cookies are always cleared even when remote token revocation fails (CWE-613) - Remote revocations are now best-effort; logout always returns 204 - Add positive test case for legacy cookie resolution via byRefreshToken - Remove page.url() fallback from e2e test baseURL (returns about:blank) - Add require.NoError to http.NewRequest calls in recovery cookie tests Co-Authored-By: Claude Opus 4.6 (1M context) --- .../tests/console/session-persistence.spec.ts | 4 +- pkg/auth/oauth2/auth_openshift.go | 26 +++++++----- pkg/auth/sessions/combined_sessions_test.go | 41 +++++++++++++++---- 3 files changed, 51 insertions(+), 20 deletions(-) diff --git a/frontend/e2e/tests/console/session-persistence.spec.ts b/frontend/e2e/tests/console/session-persistence.spec.ts index d590b84bcf5..e3bcae95216 100644 --- a/frontend/e2e/tests/console/session-persistence.spec.ts +++ b/frontend/e2e/tests/console/session-persistence.spec.ts @@ -11,7 +11,7 @@ test.describe( test.setTimeout(300_000); test('session survives console pod deletion', async ({ page, k8sClient }) => { - const baseURL = process.env.WEB_CONSOLE_URL || page.url() || 'http://localhost:9000'; + const baseURL = process.env.WEB_CONSOLE_URL || 'http://localhost:9000'; await test.step('Log in to the console', async () => { const htpasswdUser = process.env.BRIDGE_HTPASSWD_USERNAME; @@ -65,7 +65,7 @@ test.describe( }); test('session survives console plugin toggle', async ({ page, k8sClient }) => { - const baseURL = process.env.WEB_CONSOLE_URL || page.url() || 'http://localhost:9000'; + const baseURL = process.env.WEB_CONSOLE_URL || 'http://localhost:9000'; await test.step('Log in to the console', async () => { const htpasswdUser = process.env.BRIDGE_HTPASSWD_USERNAME; diff --git a/pkg/auth/oauth2/auth_openshift.go b/pkg/auth/oauth2/auth_openshift.go index 590bef0884a..27ce7cf2f7a 100644 --- a/pkg/auth/oauth2/auth_openshift.go +++ b/pkg/auth/oauth2/auth_openshift.go @@ -183,17 +183,26 @@ func (o *openShiftAuth) DeleteSession(w http.ResponseWriter, r *http.Request) { func (o *openShiftAuth) logout(w http.ResponseWriter, r *http.Request) { ctx := r.Context() + // Always clean up local state (session + recovery cookie) before writing the + // HTTP response. This ensures the user is logged out of the console even when + // remote token revocation fails. Set-Cookie headers must be added before + // WriteHeader / http.Error, otherwise Go's ResponseWriter silently drops them. + defer func() { + o.sessions.DeleteSession(w, r) + o.sessions.ClearRecoveryCookie(w, r) + }() + k8sURL, err := url.Parse(o.issuerURL) if err != nil { klog.Errorf("failed to parse the URL to kube-apiserver: %v", err) - http.Error(w, "removing the session failed", http.StatusInternalServerError) + w.WriteHeader(http.StatusNoContent) return } ls, err := o.getLoginState(w, r) if err != nil { klog.Errorf("error logging out: %v", err) - w.WriteHeader(http.StatusInternalServerError) + w.WriteHeader(http.StatusNoContent) return } @@ -208,14 +217,13 @@ func (o *openShiftAuth) logout(w http.ResponseWriter, r *http.Request) { oauthClient, err := oauthv1client.NewForConfig(configWithBearerToken) if err != nil { - klog.Infof("failed setting up the oauthaccesstokens client: %v", err) - http.Error(w, "removing the session failed", http.StatusInternalServerError) + klog.Errorf("failed setting up the oauthaccesstokens client: %v", err) + w.WriteHeader(http.StatusNoContent) return } - err = oauthClient.OAuthAccessTokens().Delete(ctx, tokenToObjectName(token), metav1.DeleteOptions{}) - if err != nil { - http.Error(w, "removing the session failed", http.StatusInternalServerError) - return + + if err := oauthClient.OAuthAccessTokens().Delete(ctx, tokenToObjectName(token), metav1.DeleteOptions{}); err != nil { + klog.Errorf("failed to revoke access token on logout: %v", err) } if refreshToken := ls.RefreshToken(); refreshToken != "" { @@ -224,8 +232,6 @@ func (o *openShiftAuth) logout(w http.ResponseWriter, r *http.Request) { } } - o.sessions.DeleteSession(w, r) - o.sessions.ClearRecoveryCookie(w, r) w.WriteHeader(http.StatusNoContent) } diff --git a/pkg/auth/sessions/combined_sessions_test.go b/pkg/auth/sessions/combined_sessions_test.go index 2090274a387..3ee6091becf 100644 --- a/pkg/auth/sessions/combined_sessions_test.go +++ b/pkg/auth/sessions/combined_sessions_test.go @@ -295,6 +295,25 @@ func TestCombinedSessionStore_GetSession_LegacyCookie(t *testing.T) { // Legacy format should resolve through byRefreshTokenID map require.Nil(t, got, "should not find session by refresh token alone without byRefreshToken mapping") + + // Positive case: legacy cookie resolves when byRefreshToken is populated + expectedSession := &LoginState{sessionToken: "legacy-session", refreshToken: "refresh-old"} + testServerSessions.byRefreshToken["refresh-old"] = expectedSession + + req2, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) + + testCookies2 := &testCookieFactory{ + cookieCodecs: cookieCodecs, + serverStore: cs.serverStore, + } + testCookies2.WithRefreshToken("refresh-old").WithLegacyFormat() + req2 = testCookies2.Complete(t, req2) + + testWriter2 := httptest.NewRecorder() + got2, err := cs.GetSession(testWriter2, req2) + require.NoError(t, err) + require.Equal(t, expectedSession, got2, "legacy cookie should resolve to session via byRefreshToken") } func TestCombinedSessionStore_RecoveryCookie(t *testing.T) { @@ -307,14 +326,16 @@ func TestCombinedSessionStore_RecoveryCookie(t *testing.T) { expiry := time.Now().Add(24 * time.Hour) t.Run("set and get recovery cookie", func(t *testing.T) { - req, _ := http.NewRequest(http.MethodGet, "/", nil) + req, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) w := httptest.NewRecorder() - err := cs.SetRecoveryCookie(w, req, accessToken, expiry) + err = cs.SetRecoveryCookie(w, req, accessToken, expiry) require.NoError(t, err) // Build a new request with the cookie from the response - req2, _ := http.NewRequest(http.MethodGet, "/", nil) + req2, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) for _, c := range w.Result().Cookies() { req2.AddCookie(c) } @@ -326,13 +347,15 @@ func TestCombinedSessionStore_RecoveryCookie(t *testing.T) { }) t.Run("get recovery cookie from empty request", func(t *testing.T) { - req, _ := http.NewRequest(http.MethodGet, "/", nil) + req, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) _, _, ok := cs.GetRecoveryCookie(req) require.False(t, ok) }) t.Run("clear recovery cookie", func(t *testing.T) { - req, _ := http.NewRequest(http.MethodGet, "/", nil) + req, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) w := httptest.NewRecorder() cs.ClearRecoveryCookie(w, req) @@ -344,14 +367,16 @@ func TestCombinedSessionStore_RecoveryCookie(t *testing.T) { }) t.Run("recovery cookie with expired token", func(t *testing.T) { - req, _ := http.NewRequest(http.MethodGet, "/", nil) + req, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) w := httptest.NewRecorder() pastExpiry := time.Now().Add(-1 * time.Hour) - err := cs.SetRecoveryCookie(w, req, accessToken, pastExpiry) + err = cs.SetRecoveryCookie(w, req, accessToken, pastExpiry) require.NoError(t, err) - req2, _ := http.NewRequest(http.MethodGet, "/", nil) + req2, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) for _, c := range w.Result().Cookies() { req2.AddCookie(c) } From 60c662a99561a554beb4cdff3f88e9372921ef74 Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Thu, 13 Aug 2026 00:11:23 +0200 Subject: [PATCH 8/9] OCPBUGS-71237: Support graceful cookie encryption key rotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accept optional previous key pair files for cookie decryption. gorilla/securecookie tries each key pair in order — new cookies use the current keys, old cookies are still readable with the previous keys until they expire. - NewSessionStore accepts variadic previousKeyPairs - SessionOptions reads previous key file paths from config - Previous keys are always optional and non-fatal if missing Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/bridge/config/auth/authoptions.go | 6 ++-- cmd/bridge/config/session/sessionoptions.go | 31 +++++++++++++++------ pkg/auth/oauth2/auth.go | 19 +++++++++---- pkg/auth/oauth2/auth_oidc.go | 8 ++++-- pkg/auth/oauth2/auth_openshift.go | 6 ++++ pkg/auth/sessions/combined_sessions.go | 6 ++-- pkg/serverconfig/types.go | 6 ++-- 7 files changed, 60 insertions(+), 22 deletions(-) diff --git a/cmd/bridge/config/auth/authoptions.go b/cmd/bridge/config/auth/authoptions.go index edb678531c0..e113ac2fd1c 100644 --- a/cmd/bridge/config/auth/authoptions.go +++ b/cmd/bridge/config/auth/authoptions.go @@ -324,8 +324,10 @@ func (c *completedOptions) getAuthenticator( CookiePath: cookiePath, SecureCookies: useSecureCookies, - CookieEncryptionKey: sessionConfig.CookieEncryptionKey, - CookieAuthenticationKey: sessionConfig.CookieAuthenticationKey, + CookieEncryptionKey: sessionConfig.CookieEncryptionKey, + CookieAuthenticationKey: sessionConfig.CookieAuthenticationKey, + PreviousCookieEncryptionKey: sessionConfig.PreviousCookieEncryptionKey, + PreviousCookieAuthenticationKey: sessionConfig.PreviousCookieAuthenticationKey, K8sConfig: k8sClientConfig, Metrics: authMetrics, diff --git a/cmd/bridge/config/session/sessionoptions.go b/cmd/bridge/config/session/sessionoptions.go index 9a65c86595b..f31817f6f84 100644 --- a/cmd/bridge/config/session/sessionoptions.go +++ b/cmd/bridge/config/session/sessionoptions.go @@ -13,8 +13,10 @@ import ( ) type SessionOptions struct { - CookieEncryptionKeyPath string - CookieAuthenticationKeyPath string + CookieEncryptionKeyPath string + CookieAuthenticationKeyPath string + PreviousCookieEncryptionKeyPath string + PreviousCookieAuthenticationKeyPath string } type CompletedOptions struct { @@ -22,15 +24,14 @@ type CompletedOptions struct { } type completedOptions struct { - CookieEncryptionKey []byte - CookieAuthenticationKey []byte + CookieEncryptionKey []byte + CookieAuthenticationKey []byte + PreviousCookieEncryptionKey []byte + PreviousCookieAuthenticationKey []byte } func NewSessionOptions() *SessionOptions { - return &SessionOptions{ - CookieEncryptionKeyPath: "", - CookieAuthenticationKeyPath: "", - } + return &SessionOptions{} } func (opts *SessionOptions) AddFlags(fs *flag.FlagSet) { @@ -41,6 +42,8 @@ func (opts *SessionOptions) AddFlags(fs *flag.FlagSet) { func (opts *SessionOptions) ApplyConfig(config *serverconfig.Session) { serverconfig.SetIfUnset(&opts.CookieEncryptionKeyPath, config.CookieEncryptionKeyFile) serverconfig.SetIfUnset(&opts.CookieAuthenticationKeyPath, config.CookieAuthenticationKeyFile) + serverconfig.SetIfUnset(&opts.PreviousCookieEncryptionKeyPath, config.PreviousCookieEncryptionKeyFile) + serverconfig.SetIfUnset(&opts.PreviousCookieAuthenticationKeyPath, config.PreviousCookieAuthenticationKeyFile) } func (opts *SessionOptions) Validate(userAuthType flagvalues.AuthType) []error { @@ -99,6 +102,18 @@ func (opts *SessionOptions) Complete(userAuthType flagvalues.AuthType) (*Complet } } + // Previous keys are always optional — used for graceful key rotation + if len(opts.PreviousCookieEncryptionKeyPath) > 0 { + if prevEncKey, err := os.ReadFile(opts.PreviousCookieEncryptionKeyPath); err == nil { + completed.PreviousCookieEncryptionKey = prevEncKey + } + } + if len(opts.PreviousCookieAuthenticationKeyPath) > 0 { + if prevAuthnKey, err := os.ReadFile(opts.PreviousCookieAuthenticationKeyPath); err == nil { + completed.PreviousCookieAuthenticationKey = prevAuthnKey + } + } + return &CompletedOptions{ completedOptions: completed, }, nil diff --git a/pkg/auth/oauth2/auth.go b/pkg/auth/oauth2/auth.go index 0664e329bb7..60dc6888f62 100644 --- a/pkg/auth/oauth2/auth.go +++ b/pkg/auth/oauth2/auth.go @@ -126,8 +126,10 @@ type Config struct { // cookiePath is an abstraction leak. (unfortunately, a necessary one.) CookiePath string SecureCookies bool - CookieEncryptionKey []byte - CookieAuthenticationKey []byte + CookieEncryptionKey []byte + CookieAuthenticationKey []byte + PreviousCookieEncryptionKey []byte + PreviousCookieAuthenticationKey []byte K8sConfig *rest.Config Metrics *auth.Metrics @@ -213,9 +215,11 @@ func NewOAuth2Authenticator(ctx context.Context, config *Config) (*OAuth2Authent consoleBaseAddress: c.ConsoleBaseAddress, cookiePath: c.CookiePath, secureCookies: c.SecureCookies, - cookieAuthenticationKey: c.CookieAuthenticationKey, - cookieEncryptionKey: c.CookieEncryptionKey, - constructOAuth2Config: a.oauth2ConfigConstructor, + cookieAuthenticationKey: c.CookieAuthenticationKey, + cookieEncryptionKey: c.CookieEncryptionKey, + previousCookieAuthenticationKey: c.PreviousCookieAuthenticationKey, + previousCookieEncryptionKey: c.PreviousCookieEncryptionKey, + constructOAuth2Config: a.oauth2ConfigConstructor, } var tokenHandler loginMethod @@ -236,11 +240,16 @@ func NewOAuth2Authenticator(ctx context.Context, config *Config) (*OAuth2Authent return nil, err } case AuthSourceOIDC: + var prevKeys [][]byte + if len(c.PreviousCookieAuthenticationKey) > 0 && len(c.PreviousCookieEncryptionKey) > 0 { + prevKeys = [][]byte{c.PreviousCookieAuthenticationKey, c.PreviousCookieEncryptionKey} + } sessionStore := sessions.NewSessionStore( c.CookieAuthenticationKey, c.CookieEncryptionKey, c.SecureCookies, c.CookiePath, + prevKeys..., ) tokenHandler, err = newOIDCAuth(ctx, sessionStore, authConfig, a.metrics) if err != nil { diff --git a/pkg/auth/oauth2/auth_oidc.go b/pkg/auth/oauth2/auth_oidc.go index 5c8d388e0f4..da8780bf591 100644 --- a/pkg/auth/oauth2/auth_oidc.go +++ b/pkg/auth/oauth2/auth_oidc.go @@ -41,9 +41,11 @@ type oidcConfig struct { consoleBaseAddress string cookiePath string secureCookies bool - cookieAuthenticationKey []byte - cookieEncryptionKey []byte - constructOAuth2Config oauth2ConfigConstructor + cookieAuthenticationKey []byte + cookieEncryptionKey []byte + previousCookieAuthenticationKey []byte + previousCookieEncryptionKey []byte + constructOAuth2Config oauth2ConfigConstructor } func newOIDCAuth(ctx context.Context, sessionStore *sessions.CombinedSessionStore, c *oidcConfig, metrics *auth.Metrics) (*oidcAuth, error) { diff --git a/pkg/auth/oauth2/auth_openshift.go b/pkg/auth/oauth2/auth_openshift.go index 27ce7cf2f7a..3d7d45315bc 100644 --- a/pkg/auth/oauth2/auth_openshift.go +++ b/pkg/auth/oauth2/auth_openshift.go @@ -89,11 +89,17 @@ func newOpenShiftAuth(ctx context.Context, k8sClient *http.Client, c *oidcConfig encryptionKey = []byte(encryptionKeyStr) } + var previousKeyPairs [][]byte + if len(c.previousCookieAuthenticationKey) > 0 && len(c.previousCookieEncryptionKey) > 0 { + previousKeyPairs = [][]byte{c.previousCookieAuthenticationKey, c.previousCookieEncryptionKey} + } + o.sessions = sessions.NewSessionStore( authnKey, encryptionKey, c.secureCookies, c.cookiePath, + previousKeyPairs..., ) return o, nil diff --git a/pkg/auth/sessions/combined_sessions.go b/pkg/auth/sessions/combined_sessions.go index 4b7666b9d3b..e3226d4c522 100644 --- a/pkg/auth/sessions/combined_sessions.go +++ b/pkg/auth/sessions/combined_sessions.go @@ -30,8 +30,10 @@ func SessionCookieName() string { return OpenshiftAccessTokenCookieName + "-" + podName } -func NewSessionStore(authnKey, encryptKey []byte, secureCookies bool, cookiePath string) *CombinedSessionStore { - clientStore := gorilla.NewCookieStore(authnKey, encryptKey) +func NewSessionStore(authnKey, encryptKey []byte, secureCookies bool, cookiePath string, previousKeyPairs ...[]byte) *CombinedSessionStore { + keyPairs := [][]byte{authnKey, encryptKey} + keyPairs = append(keyPairs, previousKeyPairs...) + clientStore := gorilla.NewCookieStore(keyPairs...) clientStore.Options.Secure = secureCookies clientStore.Options.HttpOnly = true clientStore.Options.SameSite = http.SameSiteStrictMode diff --git a/pkg/serverconfig/types.go b/pkg/serverconfig/types.go index 2fa2ca9ae6d..4edbe5b2c41 100644 --- a/pkg/serverconfig/types.go +++ b/pkg/serverconfig/types.go @@ -100,8 +100,10 @@ type Auth struct { // Session holds configuration for web-session related configuration type Session struct { - CookieEncryptionKeyFile string `yaml:"cookieEncryptionKeyFile,omitempty"` - CookieAuthenticationKeyFile string `yaml:"cookieAuthenticationKeyFile,omitempty"` + CookieEncryptionKeyFile string `yaml:"cookieEncryptionKeyFile,omitempty"` + CookieAuthenticationKeyFile string `yaml:"cookieAuthenticationKeyFile,omitempty"` + PreviousCookieEncryptionKeyFile string `yaml:"previousCookieEncryptionKeyFile,omitempty"` + PreviousCookieAuthenticationKeyFile string `yaml:"previousCookieAuthenticationKeyFile,omitempty"` // TODO: move InactivityTimeoutSeconds here } From aac1845515f4b19fd82d6eec4bad2ba80db4fb8a Mon Sep 17 00:00:00 2001 From: Robb Hamilton Date: Fri, 21 Aug 2026 15:20:12 -0400 Subject: [PATCH 9/9] Fix formatting issues causing build failure --- cmd/bridge/config/auth/authoptions.go | 4 ++-- pkg/auth/oauth2/auth.go | 20 ++++++++++---------- pkg/auth/oauth2/auth_oidc.go | 16 ++++++++-------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/cmd/bridge/config/auth/authoptions.go b/cmd/bridge/config/auth/authoptions.go index e113ac2fd1c..d207ca090d0 100644 --- a/cmd/bridge/config/auth/authoptions.go +++ b/cmd/bridge/config/auth/authoptions.go @@ -322,8 +322,8 @@ func (c *completedOptions) getAuthenticator( ErrorURL: authLoginErrorEndpoint, SuccessURL: authLoginSuccessEndpoint, - CookiePath: cookiePath, - SecureCookies: useSecureCookies, + CookiePath: cookiePath, + SecureCookies: useSecureCookies, CookieEncryptionKey: sessionConfig.CookieEncryptionKey, CookieAuthenticationKey: sessionConfig.CookieAuthenticationKey, PreviousCookieEncryptionKey: sessionConfig.PreviousCookieEncryptionKey, diff --git a/pkg/auth/oauth2/auth.go b/pkg/auth/oauth2/auth.go index 60dc6888f62..f4888349f25 100644 --- a/pkg/auth/oauth2/auth.go +++ b/pkg/auth/oauth2/auth.go @@ -124,8 +124,8 @@ type Config struct { SuccessURL string ErrorURL string // cookiePath is an abstraction leak. (unfortunately, a necessary one.) - CookiePath string - SecureCookies bool + CookiePath string + SecureCookies bool CookieEncryptionKey []byte CookieAuthenticationKey []byte PreviousCookieEncryptionKey []byte @@ -208,14 +208,14 @@ func NewOAuth2Authenticator(ctx context.Context, config *Config) (*OAuth2Authent a := newUnstartedAuthenticator(c) authConfig := &oidcConfig{ - getClient: a.clientFunc, - issuerURL: c.IssuerURL, - logoutRedirectOverride: c.LogoutRedirectOverride, - clientID: c.ClientID, - consoleBaseAddress: c.ConsoleBaseAddress, - cookiePath: c.CookiePath, - secureCookies: c.SecureCookies, - cookieAuthenticationKey: c.CookieAuthenticationKey, + getClient: a.clientFunc, + issuerURL: c.IssuerURL, + logoutRedirectOverride: c.LogoutRedirectOverride, + clientID: c.ClientID, + consoleBaseAddress: c.ConsoleBaseAddress, + cookiePath: c.CookiePath, + secureCookies: c.SecureCookies, + cookieAuthenticationKey: c.CookieAuthenticationKey, cookieEncryptionKey: c.CookieEncryptionKey, previousCookieAuthenticationKey: c.PreviousCookieAuthenticationKey, previousCookieEncryptionKey: c.PreviousCookieEncryptionKey, diff --git a/pkg/auth/oauth2/auth_oidc.go b/pkg/auth/oauth2/auth_oidc.go index da8780bf591..68bdd8b3956 100644 --- a/pkg/auth/oauth2/auth_oidc.go +++ b/pkg/auth/oauth2/auth_oidc.go @@ -34,14 +34,14 @@ type oidcAuth struct { } type oidcConfig struct { - getClient func() *http.Client - issuerURL string - logoutRedirectOverride string - clientID string - consoleBaseAddress string - cookiePath string - secureCookies bool - cookieAuthenticationKey []byte + getClient func() *http.Client + issuerURL string + logoutRedirectOverride string + clientID string + consoleBaseAddress string + cookiePath string + secureCookies bool + cookieAuthenticationKey []byte cookieEncryptionKey []byte previousCookieAuthenticationKey []byte previousCookieEncryptionKey []byte