Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions cmd/bridge/config/auth/authoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
62 changes: 47 additions & 15 deletions cmd/bridge/config/session/sessionoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,40 +6,44 @@ 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"
)

type SessionOptions struct {
CookieEncryptionKeyPath string
CookieAuthenticationKeyPath string
CookieEncryptionKeyPath string
CookieAuthenticationKeyPath string
PreviousCookieEncryptionKeyPath string
PreviousCookieAuthenticationKeyPath string
}

type CompletedOptions struct {
*completedOptions
}

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) {
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) {
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 {
Expand All @@ -50,9 +54,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 All @@ -69,17 +79,39 @@ 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
}
}

// 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
}
completed.CookieAuthenticationKey = authnKey
}

return &CompletedOptions{
Expand Down
165 changes: 165 additions & 0 deletions frontend/e2e/tests/console/session-persistence.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
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 || '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);
});
Comment on lines +50 to +52

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the readiness helper and determine whether it checks a target generation
# or newly created pods rather than only current Deployment readiness.
rg -n -C 8 'waitForDeploymentReady|waitFor.*Pod|metadata\.generation|updatedReplicas' frontend/e2e

Repository: openshift/console

Length of output: 15116


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- session-persistence.spec.ts ---'
sed -n '1,145p' frontend/e2e/tests/console/session-persistence.spec.ts

printf '%s\n' '--- waitForDeploymentReady and related client methods ---'
sed -n '580,750p' frontend/e2e/clients/kubernetes-client.ts

printf '%s\n' '--- console pod discovery and deployment patch call sites ---'
rg -n -C 12 'CONSOLE_DEPLOYMENT|CONSOLE_NAMESPACE|list.*Pod|readNamespacedPod|patchNamespacedCustomObject|plugins' frontend/e2e/tests/console/session-persistence.spec.ts frontend/e2e/clients/kubernetes-client.ts

Repository: openshift/console

Length of output: 30341


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from dataclasses import dataclass

`@dataclass`
class DeploymentObservation:
    desired: int
    available: int
    updated: int
    available_condition: bool
    generation: int
    pod_uids: tuple[str, ...]

def wait_for_deployment_ready(d: DeploymentObservation) -> bool:
    return (
        d.available == d.desired
        and d.updated == d.desired
        and d.available_condition
    )

old = DeploymentObservation(2, 2, 2, True, 7, ("old-a", "old-b"))

# A stale observation can pass immediately after deletion is accepted.
stale_after_delete = DeploymentObservation(2, 2, 2, True, 7, ("old-a", "old-b"))
assert wait_for_deployment_ready(stale_after_delete)

# A plugin change can leave the Deployment ready without a new generation.
no_rollout_after_plugin_patch = DeploymentObservation(2, 2, 2, True, 7, ("old-a", "old-b"))
assert wait_for_deployment_ready(no_rollout_after_plugin_patch)
assert no_rollout_after_plugin_patch.generation == old.generation

# The intended checks distinguish both cases.
replacement = DeploymentObservation(2, 2, 2, True, 7, ("new-a", "new-b"))
assert not set(replacement.pod_uids) & set(old.pod_uids)

rollout = DeploymentObservation(2, 2, 2, True, 8, ("new-a", "new-b"))
assert rollout.generation > old.generation

print("waitForDeploymentReady ignores generation and pod UIDs")
print("stale readiness and no-rollout readiness both satisfy its predicate")
PY

Repository: openshift/console

Length of output: 278


Gate readiness on the requested disruption.

  • Before deleting console pods, record their UIDs and wait for Ready replacement pods with different UIDs.
  • Before the plugin patch, record metadata.generation and wait for it to increase before calling waitForDeploymentReady. Otherwise, stale readiness can pass without a rollout.
📍 Affects 1 file
  • frontend/e2e/tests/console/session-persistence.spec.ts#L50-L52 (this comment)
  • frontend/e2e/tests/console/session-persistence.spec.ts#L120-L124
🤖 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 `@frontend/e2e/tests/console/session-persistence.spec.ts` around lines 50 - 52,
Update the console session-persistence test in
frontend/e2e/tests/console/session-persistence.spec.ts at lines 50-52 and
120-124: before deleting console pods, record their UIDs and require Ready
replacement pods with different UIDs; before applying the plugin patch, record
metadata.generation and wait for it to increase before calling
waitForDeploymentReady. Ensure readiness checks cannot succeed against stale
pods or an unchanged deployment.


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 || '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.
// 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);
});

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 } },
);
}
});
});
},
);
31 changes: 21 additions & 10 deletions pkg/auth/oauth2/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -206,14 +208,18 @@ 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,
constructOAuth2Config: a.oauth2ConfigConstructor,
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,
constructOAuth2Config: a.oauth2ConfigConstructor,
}

var tokenHandler loginMethod
Expand All @@ -234,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 {
Expand Down
20 changes: 12 additions & 8 deletions pkg/auth/oauth2/auth_oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,18 @@ type oidcAuth struct {
}

type oidcConfig struct {
getClient func() *http.Client
issuerURL string
logoutRedirectOverride string
clientID string
consoleBaseAddress string
cookiePath string
secureCookies bool
constructOAuth2Config oauth2ConfigConstructor
getClient func() *http.Client
issuerURL string
logoutRedirectOverride string
clientID string
consoleBaseAddress string
cookiePath string
secureCookies bool
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) {
Expand Down
Loading