feat: Add token caching for ClientCredentials + DeviceFlow + Auth0 Functionality

[PudgyPigeon]

TL;DR

Adds token caching for ClientCredentials/DeviceFlow and also adds support for Auth0.

Type

• [x ] Bug Fix
• [x ] Feature
• [ ] Plugin

Are all requirements met?

• [x ] Code completed
• [ ] Smoke tested
• [ ] Unit tests added
• [ ] Code documentation added
• [ ] Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

One change to note is that this PR adds the ability to manually specify the scopes requested during the FlyteClient authentication process. These scopes (if provided manually), are sourced from the config.yaml file and will override the default behaviour for finding scopes. This is in support of Auth0 which manually requires requesting OIDC scopes in the request body - but also the idiosyncrasy wherein Auth0 does not allow custom user APIs to be assigned the OIDC scopes explicitly (This leads to an error during Flytepropeller). Our workaround prevents this behaviour and also keeps the default behaviour intact for other IDPs.

The way the scopes parameter was parsed in the token_client module was incompatible with Auth0. It is also our opinion that it was parsed incorrectly for other IDPs as well. We have changed it to match the PKCE flow's scopes parameter and things are working. Now, the refresh_token is able to be retrieved properly. (No refresh token in response in Device/CC flow before.

Also refactored the ClientCredentials + DeviceFlow to use a Credentials dataclass object and to store and retrieve this object by calling Keyring. This enables caching for Auth in both flows.

Tested over remote SSH and localhost and can confirm all three flows are working with the suggested changes.

Though we have had to make sure that Keyring was present in our environments. Absence of Keyring may lead to errors using ClientCredentials + DeviceFlow.

Feedback is welcome.

Tracking Issue

https://github.com/flyteorg/flyte/issues/

Follow-up issue

NA OR https://github.com/flyteorg/flyte/issues/

[PudgyPigeon]

@wild-endeavor @kumare3 @eapolinario

If you have the time to look over this and give feedback, that would be more than welcome. We are quite keen to give back to the community.

Hopefully this PR adds functionality and utility for other end-users of Flyte.

Thanks for the great work as always.

[codecov[bot]]

Codecov Report

Patch coverage: 17.16% and project coverage change: -52.32% :warning:

Comparison is base (3370a96) 71.03% compared to head (800aa7a) 18.71%. Report is 78 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #1805       +/-   ##
===========================================
- Coverage   71.03%   18.71%   -52.32%     
===========================================
  Files         336      332        -4     
  Lines       30798    31399      +601     
  Branches     5589     3089     -2500     
===========================================
- Hits        21876     5877    -15999     
- Misses       8375    25440    +17065     
+ Partials      547       82      -465     

Files Changed	Coverage Δ
flytekit/clients/auth/auth_client.py	34.70% <0.00%> (+12.81%)	:arrow_up:
flytekit/clients/auth/token_client.py	35.29% <0.00%> (-15.93%)	:arrow_down:
flytekit/clients/auth_helper.py	27.39% <0.00%> (-17.81%)	:arrow_down:
flytekit/clis/sdk_in_container/build.py	0.00% <0.00%> (-94.65%)	:arrow_down:
flytekit/clis/sdk_in_container/constants.py	0.00% <0.00%> (-100.00%)	:arrow_down:
flytekit/clis/sdk_in_container/run.py	0.00% <0.00%> (-85.57%)	:arrow_down:
flytekit/clis/sdk_in_container/serialize.py	0.00% <0.00%> (-57.15%)	:arrow_down:
flytekit/clis/sdk_in_container/serve.py	0.00% <0.00%> (-100.00%)	:arrow_down:
flytekit/configuration/__init__.py	73.04% <0.00%> (+35.65%)	:arrow_up:
flytekit/core/context_manager.py	56.88% <0.00%> (+16.04%)	:arrow_up:
... and 63 more

... and 275 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[PudgyPigeon]

do you think we can move the saving of the credentials in keyring to another PR? I'm honestly not sure that that should happen for the non-pkce flows. esp for something like clientcredentials - usually this is for automated situations like CI, and I think saving the credentials is actually not preferable. what was your use-case?

Also when you say "The way the scopes parameter was parsed in the token_client module was incompatible with Auth0" - could you given an example of this please? Where are the scopes coming from?

Also pushed a formatting change to your PR.

Sorry for the late reply. I will come back to all your comments soon and make the appropriate changes.

[PudgyPigeon]

?

Also pushed a formatting change to your PR.

Also in terms of caching - we have a business use case where we wish to cache access/refresh tokens with Auth0. If we don't modify the scopes being passed into the API requests, we can't get the appropriate tokens as a response - therefore no caching. So this was a necessary change to make Keyring work with Auth0.

In regards to ClientCredentials, I agree there's no point in caching a refresh token - but we do have a business directive to limit the amount of requests made to Auth0 - and so we cache the access_token even in the ClientCredentials flow. But we do point our end-users to use DeviceFlow for the most part.

But we do have a business need to cache the DeviceFlow creds. End-users who are not programmers initially noted that having to undergo the auth flow in the terminal with DeviceCodes was cumbersome.