SQL-85 Convert OIDC config into system params by SangJunBak · Pull Request #34845 · MaterializeInc/materialize

SangJunBak · 2026-01-28T07:10:29Z

See commit messages

Start reviewing from Update OIDC tests to use system parameter defaults as config

Motivation

Tips for reviewer

Checklist

This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

…nfig - Remove OIDC CLI args - Initialize oidc authenticator with adapter client to get access to system variables - Refactor tests to use system parameter default

- Tests the runtime nature of OIDC configuration

teskje

Looks good, I just have one question/concern about the issuer default and how we behave when no issuer was set.

teskje · 2026-02-18T16:58:31Z

 use mz_frontegg_auth::Authenticator as FronteggAuthenticator;

-pub use oidc::{GenericOidcAuthenticator, OidcClaims, OidcConfig, OidcError};
+pub use mz_adapter::Client as AdapterClient;


Does this need to be pub? I don't see it used anywhere.

teskje · 2026-02-18T17:25:58Z

+/// issued by a dummy application, but from the same identity provider.
+pub const OIDC_AUDIENCE: Config<&'static str> = Config::new(
+    "oidc_audience",
+    "",


Is it possible to make this an Option, so we remember to handle the case where this wasn't set? Also for OIDC_ISSUER above.

I think I chose to make it a &'static str because an Option didn't exist for Config, but I can create one!

teskje · 2026-02-18T17:27:18Z

    ) -> Result<OidcClaims, OidcError> {
+        // Fetch current OIDC configuration from system variables
+        let system_vars = self.adapter_client.get_system_vars().await;
+        let issuer = OIDC_ISSUER.get(system_vars.dyncfgs());


Here we might get the empty string if this wasn't set before. Probably want to handle that specially to return a proper error?

teskje · 2026-02-18T17:35:36Z

Can you also add a test for the behavior when trying to authenticate with OIDC without an issuer set?

Added a test! Right now it just asserts on "Invalid Password" but I have a future PR that surfaces a more informative error

- Remove pub from AdapterClient - Add Option<String> as a possible dyncfg type

teskje

LGTM

teskje · 2026-02-19T13:11:38Z

 use mz_frontegg_auth::Authenticator as FronteggAuthenticator;

-pub use mz_adapter::Client as AdapterClient;
+use mz_adapter::Client as AdapterClient;


Nit: Mind putting this up to the non-pub block again?

teskje · 2026-02-19T13:15:54Z

+        let issuer = if let Some(issuer) = OIDC_ISSUER.get(system_vars.dyncfgs()) {
+            issuer
+        } else {
+            return Err(OidcError::MissingIssuer);
+        };


Suggested change

let issuer = if let Some(issuer) = OIDC_ISSUER.get(system_vars.dyncfgs()) {

issuer

} else {

return Err(OidcError::MissingIssuer);

};

let Some(issuer) = OIDC_ISSUER.get(system_vars.dyncfgs()) else {

return Err(OidcError::MissingIssuer);

};

- Set constants as Option<String> and initializes as None - Adds test to validate that issuer exists

SangJunBak force-pushed the jun/oidc-config-as-system-params branch from 4f46b9b to 6013469 Compare January 28, 2026 14:53

SangJunBak marked this pull request as ready for review January 28, 2026 14:54

SangJunBak requested a review from a team as a code owner January 28, 2026 14:54

SangJunBak requested review from ggevay and teskje and removed request for ggevay January 28, 2026 14:54

SangJunBak force-pushed the jun/oidc-config-as-system-params branch from 6013469 to 4effde6 Compare January 28, 2026 16:56

SangJunBak requested a review from a team as a code owner January 28, 2026 16:56

SangJunBak mentioned this pull request Feb 5, 2026

SQL-64: Create OIDC authenticator #34690

Merged

5 tasks

SangJunBak force-pushed the jun/oidc-config-as-system-params branch 3 times, most recently from 72e4912 to c06caf6 Compare February 9, 2026 20:16

SangJunBak requested a review from ggevay as a code owner February 9, 2026 20:16

SangJunBak force-pushed the jun/oidc-config-as-system-params branch from c06caf6 to a477778 Compare February 9, 2026 23:17

SangJunBak removed the request for review from ggevay February 9, 2026 23:18

SangJunBak force-pushed the jun/oidc-config-as-system-params branch 3 times, most recently from eef5a9e to c589475 Compare February 18, 2026 14:55

SangJunBak marked this pull request as draft February 18, 2026 15:02

SangJunBak added 2 commits February 18, 2026 10:03

adapter: Update OIDC authenticator to use system parameters to get co…

bd54763

…nfig - Remove OIDC CLI args - Initialize oidc authenticator with adapter client to get access to system variables - Refactor tests to use system parameter default

Add an OIDC test where OIDC config can change

ed01884

- Tests the runtime nature of OIDC configuration

SangJunBak force-pushed the jun/oidc-config-as-system-params branch from c589475 to ed01884 Compare February 18, 2026 15:04

SangJunBak marked this pull request as ready for review February 18, 2026 15:51

teskje reviewed Feb 18, 2026

View reviewed changes

Add Option<String> as a dyncfg type

0fe96b9

- Remove pub from AdapterClient - Add Option<String> as a possible dyncfg type

SangJunBak force-pushed the jun/oidc-config-as-system-params branch 2 times, most recently from 3c2a49c to fac6923 Compare February 19, 2026 04:05

SangJunBak requested a review from teskje February 19, 2026 04:25

teskje approved these changes Feb 19, 2026

View reviewed changes

adapter: Change OIDC dyncfgs to be optional

54c8ab5

- Set constants as Option<String> and initializes as None - Adds test to validate that issuer exists

SangJunBak force-pushed the jun/oidc-config-as-system-params branch from fac6923 to 54c8ab5 Compare February 19, 2026 22:01

SangJunBak merged commit f8348f3 into MaterializeInc:main Feb 20, 2026
134 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SQL-85 Convert OIDC config into system params#34845

SQL-85 Convert OIDC config into system params#34845
SangJunBak merged 4 commits intoMaterializeInc:mainfrom
SangJunBak:jun/oidc-config-as-system-params

SangJunBak commented Jan 28, 2026

Uh oh!

teskje left a comment

Uh oh!

teskje Feb 18, 2026

Uh oh!

teskje Feb 18, 2026

Uh oh!

SangJunBak Feb 18, 2026

Uh oh!

teskje Feb 18, 2026

Uh oh!

teskje Feb 18, 2026

Uh oh!

SangJunBak Feb 19, 2026 •

edited

Loading

Uh oh!

teskje left a comment

Uh oh!

teskje Feb 19, 2026

Uh oh!

teskje Feb 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

SangJunBak commented Jan 28, 2026

Motivation

Tips for reviewer

Checklist

Uh oh!

teskje left a comment

Choose a reason for hiding this comment

Uh oh!

teskje Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

teskje Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

SangJunBak Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

teskje Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

teskje Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

SangJunBak Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teskje left a comment

Choose a reason for hiding this comment

Uh oh!

teskje Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

teskje Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SangJunBak Feb 19, 2026 •

edited

Loading