-
-
Notifications
You must be signed in to change notification settings - Fork 75
feat(server): #298 postgres 18 oauth support #349
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.
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
Conversation
sunng87
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @Lilit0x ! This is a major feature of pgwire and I hope you enjoyed it when implementing within the pgwire framework. I left some minor comments.
| use std::sync::Arc; | ||
|
|
||
| use async_trait::async_trait; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can reorganize the imports, first section for std, second section for other deps
|
By the way, do you know if it's possible to include some reusable validator implementation in pgwire? |
| match (state.is_oauth(), &res, &new_state) { | ||
| (true, Authentication::Ok, SASLState::Finished) => { | ||
| // we skip sending Authentication::Ok for OAuth because finish_authentication will send it | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we need different workflow for oauth and scram here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per the docs:
OAUTHBEARER does not support channel binding, and there is no "OAUTHBEARER-PLUS" mechanism. This mechanism does not make use of server data during a successful authentication, so the AuthenticationSASLFinal message is not used in the exchange.
Here, Oauth differs from SCRAM in the sense that we don't need to send SASLFinal after successful oauth validation, but we still need to return Authentication::Ok. The issue is that finish_authentication also returns Authentication::Ok, so, two success messages will be sent for Oauth. That's why I decided to skip sending the Authentication::Ok that process_oauth_message returns, just relying on the one that finish_authentication will send.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. By design, I was not going to return Authentication::Ok from these process_xxx_message methods. But in OAuth, there is no SASLFinal, so we will still need to return an Authentication variant here.
I think we can change process_oauth_message to return (Option<Authentication>, SASLState), so it will be clear to code readers.
Co-authored-by: Ning Sun <[email protected]>
Well, I can look into adding a simple JWT validator implementation. |
| use pgwire::api::auth::{DefaultServerParameterProvider, StartupHandler}; | ||
| use pgwire::api::PgWireServerHandlers; | ||
| use pgwire::error::PgWireResult; | ||
| use pgwire::tokio::process_socket; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remember to reorder these imports
Thanks! This can be done in next patch. |
sunng87
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Let me finish the last few changes.
Closes: #298
This PR adds OAuth 2.0 authentication support to pgwire using the SASL OAUTHBEARER mechanism. The implementation includes a pluggable
OauthValidatortrait that allows custom token validation logic, along with an almost production-grade Keycloak integration example that performs full JWT validation with RS256 signatures, OIDC discovery, and JWKS key caching.