OCPBUGS-71237: Persist console sessions across pod restarts

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'"))
 		}
 	}
 

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,
 	}
 

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
 }
 

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)
 	}

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,
 	)

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,49 +154,40 @@ 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)
 }
 
 func (cs *CombinedSessionStore) UpdateTokens(w http.ResponseWriter, r *http.Request, tokenVerifier IDTokenVerifier, tokenResponse *oauth2.Token) (*LoginState, error) {
 	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)