Skip to content

SNOW-1825621 refresh token implementation and OAuth token cache #2162

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.

Sign up for GitHub

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

Jump to bottom
Closed
sfc-gh-mkeller wants to merge 110 commits into from
Closed

SNOW-1825621 refresh token implementation and OAuth token cache #2162

sfc-gh-mkeller wants to merge 110 commits into from

Conversation

@sfc-gh-mkeller
Copy link
Collaborator

@sfc-gh-mkeller sfc-gh-mkeller commented Jan 30, 2025 •
edited
Loading

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-1825621

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am modifying authorization mechanisms
    • I am adding new credentials
    • I am modifying OCSP code
    • I am adding a new dependency

  3. Please describe how your code solves the related issue.

    The last piece of the new OAuth code flow implementation.
    The PR builds on top of SNOW-1825621 OAuth code flow PKCE support #2137 and SNOW-1825495 OAuth flows implementation #2135, so those should be merged before this one is reviewed.
    I've added support for refresh token consumption and tested it. Unfortunately this is kind of useless until we introduce a OAUTH token cache, so that needs adding still.

  4. (Optional) PR for stored-proc connector:

@sfc-gh-mkeller sfc-gh-mkeller force-pushed the mkeller/SNOW-1825621/refresh-token-token-cache branch from 7f15277 to 7a861c1 Compare January 30, 2025 18:43
@sfc-gh-mkeller sfc-gh-mkeller changed the base branch from main to mkeller/SNOW-1825621/pkce-support January 31, 2025 17:40
sfc-gh-eworoshow
sfc-gh-eworoshow reviewed Jan 31, 2025
View reviewed changes
Copy link
Contributor

@sfc-gh-eworoshow sfc-gh-eworoshow left a comment

Choose a reason for hiding this comment

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

Nice!

src/snowflake/connector/connection.py
),
"oauth_security_features": (
("pkce",),
("pkce", "refresh_token"),
Copy link
Contributor

@sfc-gh-eworoshow sfc-gh-eworoshow Jan 31, 2025

Choose a reason for hiding this comment

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

we should omit refresh_token from the default, since the default oauth integration will not have refresh tokens enabled unless the admin turns them on explicitly

src/snowflake/connector/auth/oauth_code.py
logger.info(
"oauth refresh token is available, going to try consuming it to recover"
)
self._consume_refresh_token(conn=conn)
Copy link
Contributor

@sfc-gh-eworoshow sfc-gh-eworoshow Jan 31, 2025

Choose a reason for hiding this comment

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

we only need to use the refresh token after the access token has expired, in order to create a new session without reprompting for credentials

@sfc-gh-mmishchenko sfc-gh-mmishchenko self-assigned this Feb 12, 2025
sfc-gh-mkeller and others added 23 commits March 4, 2025 15:17
@sfc-gh-mkeller @sfc-gh-mmishchenko
adding new AuthHttpServer class
02e0a28
@sfc-gh-mkeller @sfc-gh-mmishchenko
unrelated simplifications
6e1beb0
@sfc-gh-mkeller @sfc-gh-mmishchenko
adding new oauth code flow implementation
895c99f
@sfc-gh-mkeller @sfc-gh-mmishchenko
adding new TODO
60bc86e
@sfc-gh-mkeller @sfc-gh-mmishchenko
adding changelog entry
e3b9fc0
@sfc-gh-mkeller @sfc-gh-mmishchenko
making oauth state random
f156f6d
@sfc-gh-mkeller @sfc-gh-mmishchenko
make client_secret optional
6ca5474
@sfc-gh-mkeller @sfc-gh-mmishchenko
make state check failure fail auth
8ae7397
@sfc-gh-mkeller @sfc-gh-mmishchenko
switch to cryptographically-secure source
6c6eee6
@sfc-gh-mkeller @sfc-gh-mmishchenko
redo connection parameters based on feedback
200c5f7
@sfc-gh-mkeller @sfc-gh-mmishchenko
some string updates
20c14fa
@sfc-gh-mkeller @sfc-gh-mmishchenko
add Authorization header to token request
f91ae95
@sfc-gh-mkeller @sfc-gh-mmishchenko
fix logging bug
92c35e2
@sfc-gh-mkeller @sfc-gh-mmishchenko
review feedback
0b678c4
@sfc-gh-mkeller @sfc-gh-mmishchenko
rewording parameters based on discussion
87f58ea
@sfc-gh-mkeller @sfc-gh-mmishchenko
reworking the defaut oauth scope
85945d4
@sfc-gh-mkeller @sfc-gh-mmishchenko
adding port support into oauth code flow auth
ab229b8
@sfc-gh-mkeller @sfc-gh-mmishchenko
do not log state
4c5fa51
@sfc-gh-mkeller @sfc-gh-mhofman @sfc-gh-mmishchenko
Update src/snowflake/connector/connection.py
a921ec5 
Co-authored-by: Michał Hofman <[email protected]>
@sfc-gh-pbulawa @sfc-gh-mkeller
@sfc-gh-mhofman @sfc-gh-mmishchenko
SNOW-1917265: Add OAUTH_TYPE for authorization flow (#2174)
b85a824 
Co-authored-by: Mark Keller <[email protected]>
Co-authored-by: Michał Hofman <[email protected]>
@sfc-gh-mmishchenko
Merge branch 'mkeller/SNOW-1825621/oauth-code-flow-support' into mkel…
a83d09c 
…ler/SNOW-1825621/pkce-support
@sfc-gh-mmishchenko
Merge branch 'mkeller/SNOW-1825621/pkce-support' into mkeller/SNOW-18…
7bc3b6c 
…25621/refresh-token-token-cache
@sfc-gh-mkeller
SNOW-1825621 OAuth code flow PKCE support (#2137)
493efad
Base automatically changed from mkeller/SNOW-1825621/pkce-support to mkeller/SNOW-1825621/oauth-code-flow-support March 4, 2025 15:04
@sfc-gh-mmishchenko sfc-gh-mmishchenko force-pushed the mkeller/SNOW-1825621/oauth-code-flow-support branch 2 times, most recently from f4d206c to dafeb5b Compare March 10, 2025 00:17
@sfc-gh-mmishchenko sfc-gh-mmishchenko force-pushed the mkeller/SNOW-1825621/oauth-code-flow-support branch from e920d82 to 984246e Compare March 13, 2025 10:33
@github-actions github-actions bot locked and limited conversation to collaborators Mar 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sfc-gh-eworoshow sfc-gh-eworoshow sfc-gh-eworoshow left review comments

Assignees

@sfc-gh-mmishchenko sfc-gh-mmishchenko

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sfc-gh-mkeller @sfc-gh-eworoshow @sfc-gh-mmishchenko @sfc-gh-pbulawa