Skip to content
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

SNOW-993600 External OAuth2.0 Support #718

Merged
merged 8 commits into from
Mar 25, 2024
Merged

Conversation

sfc-gh-alhuang
Copy link
Contributor

@sfc-gh-alhuang sfc-gh-alhuang commented Mar 19, 2024

External OAuth2.0 support for client sdk. Auth servers should adheres to the OAuth 2.0 grant flow, issuing refresh tokens. Additional parameters are required for OAuth support. Please refer to the accompanying image for details.
image
To integrate OAuth with Snowflake, please check this public doc.

JIRA

@sfc-gh-alhuang sfc-gh-alhuang changed the title External OAuth Support [SNOW-993600] External OAuth Support Mar 19, 2024
@sfc-gh-alhuang sfc-gh-alhuang changed the title [SNOW-993600] External OAuth Support SNOW-993600 External OAuth Support Mar 19, 2024
@sfc-gh-alhuang sfc-gh-alhuang marked this pull request as ready for review March 19, 2024 21:33
@sfc-gh-alhuang sfc-gh-alhuang self-assigned this Mar 19, 2024
Copy link
Contributor

@sfc-gh-tzhang sfc-gh-tzhang left a comment

Choose a reason for hiding this comment

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

Left some minor comments, otherwise LGTM, thanks!

@@ -187,11 +190,29 @@ public class SnowflakeStreamingIngestClientInternal<T> implements SnowflakeStrea
throw new SFException(e, ErrorCode.KEYPAIR_CREATION_FAILURE);
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do a check here that the AUTHORIZATION_TYPE is OAuth, and throw an exception if user gives us something we don't understand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The properties check is here before passing props to the internal client. Should we also add a check here?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, this looks good, thanks!

@sfc-gh-tzhang
Copy link
Contributor

External OAuth support for client sdk, please check this doc for more details.

JIRA

Do you want to add a bit more descriptions (or maybe a screenshot is sufficient)? This is a public repo and external forks don't have access to internal docs :)

@sfc-gh-tzhang
Copy link
Contributor

@sfc-gh-xhuang do you want to work with Alec to document our support for OAuth so that customer can start to try it out? I don't see many usage so far

@sfc-gh-alhuang sfc-gh-alhuang changed the title SNOW-993600 External OAuth Support SNOW-993600 External OAuth2.0 Support Mar 20, 2024
@sfc-gh-xhuang
Copy link
Contributor

sfc-gh-xhuang commented Mar 20, 2024

@sfc-gh-tzhang I believe we already document our support for Snowflake Oauth
https://docs.snowflake.com/en/user-guide/data-load-snowpipe-streaming-configuration#using-snowflake-oauth

Oauth may not be a heavy usage feature. Similar to replication, big companies like to leverage it but not everyone

Copy link
Contributor

@sfc-gh-tjones sfc-gh-tjones left a comment

Choose a reason for hiding this comment

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

Nice!

@sfc-gh-alhuang sfc-gh-alhuang merged commit dbd0ce0 into master Mar 25, 2024
14 checks passed
@sfc-gh-alhuang sfc-gh-alhuang deleted the alhuang-ext-oauth branch March 25, 2024 22:44
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.

None yet

4 participants