-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
feat(ingest/snowflake): Okta OAuth support; update docs #8157
feat(ingest/snowflake): Okta OAuth support; update docs #8157
Conversation
+ Ensure client authentication method is `Client secret` | ||
- Create an Okta user to sign into, noting the `Username` and `Password` | ||
- Create a Snowflake user to correspond to your newly created Okta client credentials | ||
+ *Ensure the user's `Login Name` matches your Okta user's `Username` (likely a password)* |
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.
What does this mean -
likely a password?
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.
Oops, supposed to be email, thx
raise ValueError( | ||
f"'oauth_config.scopes' was none " | ||
f"but should be set when using {v} authentication" | ||
"Certificate authentication is not supported for Okta." |
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.
If this is always true for okta, it would help to move this to a validator in OAuthConfiguration .
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.
Pretty sure you can set up okta auth with public / private keys, I just couldn't get it to work easily with snowflake
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.
can't say i fully understand what's going on here, but overall seems good
logger.info(f"using authenticator type '{v}'") | ||
return v | ||
elif oauth_config.client_secret is None: | ||
raise ValueError( |
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.
can some of this validation logic live on the OAuthConfiguration object?
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.
Probably, but I'm not sure what will be snowflake specific and what won't just yet.
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.
thanks for updating these docs
OauthConfiguration
, to simplify logicOPERATE
permissionChecklist