Skip to content

Oidc password authentication on uninitialized Oidc fix#36671

Merged
SangJunBak merged 2 commits into
MaterializeInc:mainfrom
SangJunBak:jun/-fix-console
May 21, 2026
Merged

Oidc password authentication on uninitialized Oidc fix#36671
SangJunBak merged 2 commits into
MaterializeInc:mainfrom
SangJunBak:jun/-fix-console

Conversation

@SangJunBak
Copy link
Copy Markdown
Contributor

@SangJunBak SangJunBak commented May 21, 2026

Before we were getting throws on password login when Oidc was never initialized. This allows us to not have to check if the oidcManager is available everytime we want Oidc auth details.

Motivation

Bug found during product review when logging in as mz_system before Oidc was configured

Verification

Run locally against Oidc instance and login as mz_system. Before you would crash because we were trying to access properties in useAuth that didn't exist. Now it doesn't crash

@SangJunBak SangJunBak requested a review from a team as a code owner May 21, 2026 13:21
@SangJunBak SangJunBak requested review from a team, Alphadelta14 and leedqin and removed request for a team and Alphadelta14 May 21, 2026 13:21
@SangJunBak SangJunBak changed the title Oidc UI fixes Oidc password authentication on uninitialized Oidc fix May 21, 2026
@SangJunBak SangJunBak marked this pull request as draft May 21, 2026 13:30
@SangJunBak SangJunBak marked this pull request as ready for review May 21, 2026 13:35
@SangJunBak
Copy link
Copy Markdown
Contributor Author

I feel like a follow up to this is an e2e test framework that:

  1. Runs the Console locally
  2. Stands up a Materialize instance with the right config using mzcompose.ts
  3. Uses playwright to just login as mz_system using password auth

Then perhaps using frontegg-mock, we could do something similar for oidc.

@SangJunBak SangJunBak force-pushed the jun/-fix-console branch 2 times, most recently from 155e5b9 to d2890ab Compare May 21, 2026 15:15
Copy link
Copy Markdown
Contributor

@leedqin leedqin left a comment

Choose a reason for hiding this comment

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

LGTM

Before we were getting throws on password login when Oidc was never initialized. This allows us to not have to check if the oidcManager is available everytime we want Oidc auth details.
enabled: isOidc,
staleTime: Infinity,
retry: false,
retryOnMount: false, // Do not retry on mount, otherwise we will retry indefinitely
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.

this is cool! Didn't know retryOnMount was a thing

@SangJunBak SangJunBak force-pushed the jun/-fix-console branch 2 times, most recently from c8e1d69 to da708c0 Compare May 21, 2026 17:24
Comment thread console/src/platform/auth/Login.tsx Outdated
}
const oidcAuthErrorMessage = auth?.error?.message ?? null;

useEffect(() => {
Copy link
Copy Markdown
Contributor

@leedqin leedqin May 21, 2026

Choose a reason for hiding this comment

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

Suggested change
useEffect(() => {
const oidcError = auth.error?.message? ?? null;
const displayError =
error ||
(oidcAuthErrorMessage &&
`${oidcAuthErrorMessage}. It looks like there may be an issue with the sign-in configuration. Please review your OIDC settings or check the console logs for more information.`) ||
null;
return (
<VStack spacing="2" alignItems="center">
{displayError&& <Alert variant="error" minWidth="100%" message={error} />}

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.

I imagine this flow should work

  1. . App mounts → auth.error = nulloidcError = undefineddisplayError = null → no Alert.
  2. react-oidc-context populates auth.error → AuthProvider re-renders consumers.
  3. SsoLoginLink re-renders → oidcError is recomputed from auth.error.messagedisplayError is the friendly message → Alert appears.
  4. User clicks SSO → handleSsoLogin runs → if signinRedirect() rejects, setError(...) runs → next render, error (local) wins via || short-circuit → Alert swaps to that message.
  5. Issuer is fixed and the user reloads → auth.error clears → displayError = null → Alert disappears.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense to me!

@SangJunBak SangJunBak enabled auto-merge (squash) May 21, 2026 18:34
@SangJunBak SangJunBak merged commit 6119a24 into MaterializeInc:main May 21, 2026
109 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants