Skip to content

OCPBUGS-71237: Persist console sessions across pod restarts #16911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jhadvig wants to merge 9 commits into
base: main
Choose a base branch
from
Open

OCPBUGS-71237: Persist console sessions across pod restarts #16911

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8c83365
OCPBUGS-71237: Persist console sessions across pod restarts
jhadvig Aug 5, 2026
9d72995
OCPBUGS-71237: Address security review findings
jhadvig Aug 5, 2026
e200791
OCPBUGS-71237: Add e2e test for session persistence
jhadvig Aug 5, 2026
8afcc78
OCPBUGS-71237: Add access token recovery cookie for OpenShift auth
jhadvig Aug 7, 2026
ae77e5e
OCPBUGS-71237: Fix CI lint failures
jhadvig Aug 10, 2026
5d9e366
OCPBUGS-71237: Handle missing session key files gracefully
jhadvig Aug 11, 2026
07b0832
OCPBUGS-71237: Address CodeRabbit review findings
jhadvig Aug 12, 2026
60c662a
OCPBUGS-71237: Support graceful cookie encryption key rotation
jhadvig Aug 12, 2026
aac1845
Fix formatting issues causing build failure
rhamilto Aug 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions cmd/bridge/config/session/sessionoptions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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'"))
}
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/auth/oauth2/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/auth/oauth2/auth_oidc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ type oidcConfig struct {
consoleBaseAddress string
cookiePath string
secureCookies bool
cookieAuthenticationKey []byte
cookieEncryptionKey []byte
constructOAuth2Config oauth2ConfigConstructor
}

Expand Down
27 changes: 9 additions & 18 deletions pkg/auth/oauth2/auth_oidc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
36 changes: 24 additions & 12 deletions pkg/auth/oauth2/auth_openshift.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -206,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)
}
}

Comment on lines +235 to +240

@coderabbitai coderabbitai Bot Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- relevant symbols ---'
rg -n -C 12 'func .*DeleteSession|DeleteSession\(|failed to revoke refresh token|OAuthAuthorizeTokens\(\)\.Delete|RefreshToken\(\)' pkg/auth/oauth2 pkg/auth/sessions
printf '%s\n' '--- bounded source slice ---'
sed -n '180,245p' pkg/auth/oauth2/auth_openshift.go

Repository: openshift/console

Length of output: 35701

Broken Authentication (CWE-613): Insufficient Session Expiration

Reachability: External · Exploitability: Moderate

Return an error when refresh-token revocation fails.

OAuthAuthorizeTokens().Delete errors are logged, but the handler still deletes the local session and returns 204. A copied refresh-token cookie can remain valid. Return a failure response and add a test for this path.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/auth/oauth2/auth_openshift.go` around lines 216 - 221, Update the logout
handler around OAuthAuthorizeTokens().Delete so a refresh-token revocation error
is returned as a failure response instead of only being logged; stop the flow
before deleting the local session or returning 204. Preserve the existing
successful revocation behavior, and add a test covering the failed deletion
path.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

sed -n '180,235p' pkg/auth/oauth2/auth_openshift.go
printf '\n-- related symbols --\n'
rg -n -C 3 'DeleteSession|refreshSession|OAuthAccessTokens\(\)\.Delete|OAuthAuthorizeTokens\(\)\.Delete|RefreshToken\(\)' pkg/auth/oauth2 pkg/auth/sessions

Repository: openshift/console

Length of output: 20240

🏁 Script executed:

python3 - <<'PY'
from pathlib import Path

p = Path("pkg/auth/oauth2/auth_openshift.go")
lines = p.read_text().splitlines()
window = "\n".join(lines[177:224])
required = [
    "err = oauthClient.OAuthAccessTokens().Delete",
    'http.Error(w, "removing the session failed", http.StatusInternalServerError)',
    "return",
    'if refreshToken := ls.RefreshToken(); refreshToken != "" {',
    "o.sessions.DeleteSession(w, r)",
]
positions = {text: window.find(text) for text in required}
print("logout control-flow positions:", positions)
print("access-delete failure returns before refresh cleanup:",
      positions[required[1]] < positions[required[3]] and positions[required[2]] < positions[required[3]])
print("access-delete failure returns before local session deletion:",
      positions[required[1]] < positions[required[4]] and positions[required[2]] < positions[required[4]])
PY

printf '\n-- session cookie deletion and refresh lookup --\n'
sed -n '88,108p' pkg/auth/sessions/combined_sessions.go
sed -n '219,250p' pkg/auth/sessions/combined_sessions.go
sed -n '252,274p' pkg/auth/oauth2/auth_openshift.go

Repository: openshift/console

Length of output: 3274

Broken Authentication (CWE-613): Insufficient Session Expiration

Reachability: External

Clear the session when access-token revocation fails.

When OAuthAccessTokens().Delete returns an error, logout returns before refresh-token cleanup and o.sessions.DeleteSession. The refresh-token cookie and server-side session can then reach refreshSession. Run cleanup on this failure path and preserve the revocation error separately.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/auth/oauth2/auth_openshift.go` around lines 216 - 221, Update logout’s
access-token revocation failure path to clear the session before returning:
preserve the revocation error separately, perform refresh-token cleanup and call
sessions.DeleteSession, then return the original error. Ensure cleanup also runs
when OAuthAccessTokens().Delete fails rather than exiting before the existing
logout cleanup flow.

@TheRealJon TheRealJon Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seem like valid concerns, have you looked into them yet?

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- auth OpenShift structure and logout-related tests ---'
ast-grep outline pkg/auth/oauth2/auth_openshift.go --items all --type function --match 'logout|DeleteSession|getLoginState|refreshSession'
ast-grep outline pkg/auth/oauth2/auth_openshift_test.go --items all --type function --match 'logout|Logout|DeleteSession|refresh'

printf '%s\n' '--- session store structure ---'
ast-grep outline pkg/auth/sessions/combined_sessions.go --items all --type function --match 'DeleteSession|GetSession|GetCookieRefreshToken|ClearRecoveryCookie'

printf '%s\n' '--- logout and deletion references with context ---'
rg -n -C 8 -P '\b(?:logout|DeleteSession|OAuthAccessTokens\(\)\.Delete|OAuthAuthorizeTokens\(\)\.Delete|GetCookieRefreshToken)\s*\(' \
  pkg/auth/oauth2 pkg/auth/sessions \
  -g '*.go'

printf '%s\n' '--- bounded implementation slices ---'
sed -n '180,245p' pkg/auth/oauth2/auth_openshift.go
sed -n '200,300p' pkg/auth/sessions/combined_sessions.go

Length of output: 31672

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- candidate OpenShift authentication test files ---'
fd -t f -e go . pkg/auth/oauth2 | sort

printf '%s\n' '--- OpenShift auth construction and logout test references ---'
rg -n -C 10 -P '\b(?:newOpenShiftAuth|openShiftAuth|LogoutFunc|func\s+Test.*(?:OpenShift|Logout)|OAuthAuthorizeTokens|OAuthAccessTokens)\b' \
  pkg/auth/oauth2 -g '*_test.go'

printf '%s\n' '--- test helpers that provide HTTP/API-client behavior ---'
rg -n -C 8 -P '\b(?:httptest\.NewServer|RoundTripper|Transport:|http\.Client|NewForConfig|issuerURL|testValidRefreshToken)\b' \
  pkg/auth/oauth2 -g '*_test.go'

printf '%s\n' '--- complete local deletion and refresh-token lookup behavior ---'
sed -n '120,175p' pkg/auth/sessions/combined_sessions.go
sed -n '220,270p' pkg/auth/sessions/combined_sessions.go

Length of output: 1403

🏁 Script executed:

#!/bin/bash
set -u

printf '%s\n' '--- auth test file structure ---'
ast-grep outline pkg/auth/oauth2/auth_test.go --items all --type function
printf '%s\n' '--- bounded auth test source ---'
sed -n '1,260p' pkg/auth/oauth2/auth_test.go

printf '%s\n' '--- repository-wide OpenShift logout and API client test references ---'
rg -n -C 8 -P '\b(?:newOpenShiftAuth|openShiftAuth|OAuthAuthorizeTokens|OAuthAccessTokens|LogoutFunc|logout)\b' \
  -g '*_test.go' . || true

printf '%s\n' '--- HTTP transport fakes in authentication tests ---'
rg -n -C 8 -P '\b(?:RoundTripper|Transport|httptest\.NewServer|http\.Client|NewForConfig)\b' \
  pkg/auth -g '*_test.go' || true

printf '%s\n' '--- relevant auth construction ---'
sed -n '240,340p' pkg/auth/oauth2/auth.go

Length of output: 37782

@TheRealJon, yes. I reviewed the implementation, the session-store behavior, and the existing tests.

Both findings identify valid defects:

  1. If OAuthAccessTokens().Delete fails, logout returns before it calls DeleteSession and ClearRecoveryCookie. The encrypted refresh-token cookie remains available. getLoginState can use it to refresh or restore the session.

  2. If OAuthAuthorizeTokens().Delete fails, the remote refresh token can remain valid. The current handler logs the error and returns 204.

The first finding needs one correction. On a refresh-token revocation failure, the handler must not return before local cleanup. It must expire the local session, refresh-token cookie, and recovery cookie first. It should then return an error response to report that remote revocation failed.

The robust flow is:

  • Attempt both remote revocations.
  • Record any revocation error.
  • Always clear local session state and cookies.
  • Return 204 only if both remote revocations succeed.
  • Return 500 after local cleanup if either remote revocation fails.

There is no existing OpenShift logout test. pkg/auth/oauth2/auth_test.go only covers authenticator construction. The change should add failure-path tests that assert both the error response and local cookie/session cleanup.

You are interacting with an AI system.

o.sessions.DeleteSession(w, r)
w.WriteHeader(http.StatusNoContent)
}
Expand Down Expand Up @@ -234,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)
Expand Down
83 changes: 38 additions & 45 deletions pkg/auth/sessions/combined_sessions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"
"sync"

"github.com/gorilla/securecookie"
gorilla "github.com/gorilla/sessions"
"golang.org/x/oauth2"
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -79,8 +86,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)
}
Expand Down Expand Up @@ -125,12 +131,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
}
}
Expand All @@ -140,10 +147,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
}
Expand All @@ -152,49 +161,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
Expand All @@ -203,19 +203,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
}
Expand All @@ -241,18 +235,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)
Expand Down
Loading