Add support for refresh_token to device flow auth

[Sovietaced]

Tracking issue

Closes flyteorg/flyte#6044

Why are the changes needed?

Device flow auth did not support refresh tokens which makes the pyflyte user experience less enjoyable since you may need to login often.

What changes were proposed in this pull request?

Passes along the refresh token from the token response to the code which writes it to the keyring.

How was this patch tested?

Unit tests as well as in our production environment.

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

[Sovietaced]

I think this still needs a little work.

[eapolinario]

what's missing, @Sovietaced ?

[Sovietaced]

what's missing, @Sovietaced ?

Need to use the auth client to get a new access token using the refresh token. I'll ping you when I'm done. Working on it now.

[Sovietaced]

There was a warning about this possibly not being defined in my editor due to the if statement below.

[thomasjhuang]

One thing to note is that you can actually receive the refresh token in the body. In our internal implementation we add a check like this:

    if refresh_token:
        body["refresh_token"] = refresh_token

and then later:

    j = response.json()
    # Check if refresh_token already existed and was sent with the body, or if we are getting it for the first time
    if refresh_token or "refresh_token" in j and j["refresh_token"] is not None:
        return j["access_token"], j["refresh_token"], j["expires_in"]
    else:
        logging.debug("Refresh token was not received in the response, only storing access token")
        return j["access_token"], None, j["expires_in"]

[Sovietaced]

Not sure what you mean, wouldn't that be adding the refresh token into the request payload?

[thomasjhuang]

Yes, that's correct. I believe you do need both the refresh_token + the associated grant type "refresh_token" in the request payload parameters per OAuth2.0 spec - https://www.oauth.com/oauth2-servers/access-tokens/refreshing-access-tokens

[thomasjhuang]

So what this would mean is that you fetch the pre-existing refresh token if there is one, alongside the request payload.

[Sovietaced]

I see. I think I'm confused because the code that I modified that uses this currently just passes grant type DEVICE_CODE. The code path that I'm using to refresh the token (which I copied from the PKCE auth) uses an auth client as opposed to the token client and that has worked for us.

[Sovietaced]

I think I could reuse the token client if I made that change you suggested, let me try that.

[Sovietaced]

That seems to work. Added in fe8277b

[codecov]

Codecov Report

Attention: Patch coverage is 84.61538% with 4 lines in your changes missing coverage. Please review.

Project coverage is 76.49%. Comparing base (3b7cb3c) to head (fe8277b).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
flytekit/clients/auth/token_client.py	77.77%	1 Missing and 1 partial ⚠️
flytekit/clients/auth/auth_client.py	0.00%	1 Missing ⚠️
flytekit/clients/auth/authenticator.py	93.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2947   +/-   ##
=======================================
  Coverage   76.49%   76.49%           
=======================================
  Files         200      200           
  Lines       20901    20919   +18     
  Branches     2689     2692    +3     
=======================================
+ Hits        15989    16003   +14     
- Misses       4195     4197    +2     
- Partials      717      719    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[thomasjhuang]

lgtm!