SQL-76 Add pgwire password authentication to OIDC authenticator #34801
+281
−86
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SangJunBak
force-pushed
the
jun/add-password-auth-to-oidc
branch
7 times, most recently
from
a8d789b to
aed6472
Compare
SangJunBak
changed the title
SangJunBak
force-pushed
the
jun/add-password-auth-to-oidc
branch
from
aed6472 to
1ccd67a
Compare
SangJunBak
force-pushed
the
jun/add-password-auth-to-oidc
branch
from
1ccd67a to
0d5b665
Compare
SangJunBak
force-pushed
the
jun/add-password-auth-to-oidc
branch
from
0d5b665 to
c04b2aa
Compare
SangJunBak
force-pushed
the
jun/add-password-auth-to-oidc
branch
2 times, most recently
from
047394c to
5de9377
Compare
- Add external_metadata_rx() method to OidcAuthSessionHandle trait with default None impl
This allows us to create a helper functions for anything implementing OidcAuthenticator.
- Update Authenticator::Oidc to use named fields: {oidc, password}
SangJunBak
force-pushed
the
jun/add-password-auth-to-oidc
branch
from
5de9377 to
31359cd
Compare
- Add authenticate_with_oidc_token for token-based auth (Frontegg/OIDC JWT) - Add authenticate_with_password for password-based auth
- Enables tests to pass connection-level options like --oidc_auth_enabled - Verifies that when oidc_auth_enabled is not set in the connection options, the OIDC authenticator falls back to password authentication.
Call stacks above the critical recursion can grow as we add code elsewhere in the system
SangJunBak
force-pushed
the
jun/add-password-auth-to-oidc
branch
from
31359cd to
b04753d
Compare
teskje
reviewed
Feb 9, 2026
Contributor
teskje
left a comment
teskje
left a comment
There was a problem hiding this comment.
Looks fine to me.
I'm a bit unhappy with adding password auth as a special case for OIDC auth, ideally we would leave the concerns separated and leave the password auth to the password authenticator. Would it be difficult to move the dispatching on the auth method one level up and just use the password authenticator when OIDC is disabled via the option?
| Oidc(GenericOidcAuthenticator), | ||
| Oidc { | ||
| oidc: GenericOidcAuthenticator, | ||
| password: AdapterClient, |
Contributor
There was a problem hiding this comment.
Maybe call this adapter_client? I think you name it password because it's used for password auth, but I was confused initially thinking this field stores a password instead.
| # the query planner and optimizer. | ||
|
|
||
| # 475 UNIONs | ||
| # 418 UNIONs |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
see commit messages for details, starting from commit "Add password fallback for OIDC pgwire". Commits before are. being reviewed in #34690.
Will do http pgwire password authentication in a followup PR.
Motivation
Tips for reviewer
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.