OAuth2 support via planet_auth python libraries

[carl-adams-planet]

Change planet.Auth over to using the planet-auth-python library to move towards OAuth2 as the preferred authentication mechanism..

[ischneider]

So far, so good! Tested out and found things to be pretty straight-forward and it's definitely nice to have a means to grab/refresh a token

My initial review:

Good:

• many comprehensive docs (auth.Auth)
• encapsulation of internals with minimal leakage/churn
• worked smoothly (except what's noted below)

Less Good

• unused code? _BuiltinConfigurationProvider
• mutating globals _OIDC_AUTH_CLIENT_CONFIG__USER_SKEL
• the Auth ABC with static methods that return a concrete implementation? seems confusing for the (rare) case when someone would make their own Auth

Questions

• raising DeprecationWarning vs using deprecated annotation (seems contrary to the goal of deprecation - will be removed soon vs 'doesn't work anymore'
• auth profile of "none"?

Nits

• redundant/stuttering names with typing info in name e.g. _BuiltinConfigurationProvider.builtin_client_authclient_config_dicts and test_cli_session.py::test_CliSession_auth_valid
• EmptyBuiltinProfileConstants YAGNI/TMA (too-much-abstraction?)

General

• prefer E2E style testing over units as much as possible

Bugs
on install (with existing apikey), CLI auth print-access-token and refresh both printed
Error: [Errno 2] No such file or directory: '$HOME/.planet/_pl_api_key/token.json' (real HOME redacted)

To be improved
planet_auth profile show (on initial "upgrade", e.g. with existing apikey):

  Current: _pl_api_key
  User Default: _PL_API_KEY
  Global Built-in Default: planet-user

[carl-adams-planet]

I'll take the comments more-or-less one at a time (may not get to them all today)

_BuiltinConfigurationProvider is used, but I agree it's a little non-obvious.

_BuiltinConfigurationProvider defines some built-in client configs, which are used by the plauth CLI. For a good UX, it's nice for the CLI to have some built-in configs (see planet auth profile list or plauth profile list). The plauth CLI itself however is provided by the planet_auth_utils package, which is more general than the SDK and its planet CLI. So, the trick is to inject this built-in config.

plauth tries to be flexible. I want it to work with other built in config providers. On the janky side, it knows to see if the planet or planet-auth-config packages are present. The latter is Planet internal, and includes configs for staging environments. But, this jankyness is for smooth dev ex: You install the generic planet_auth_utils and the internal planet-auth-config in a venv, and it will do the right thing with no public SDK present. The auth lib serves more than the SDK.

Past the janky magic knowledge in planet_auth_utils about the SDK or the internal config package, it will fall back to a built-in config provider class injected by env var. (That's the PL_AUTH_BUILTIN_CONFIG_PROVIDER env var that we also set)

[carl-adams-planet]

Addressed the mutating of the SKEL. I now copy before I modify.

Change pushed to MR

[carl-adams-planet]

The Auth ABC preserves the v2 structure. I didn't really like the old structure, either, but I was aiming for minimizing impact between v2 and v3 auth for developers.

v2 had Auth(ABC) with static from_file() -> AuthType and other methods that returned a concrete APIAuthKey(httpx.AuthBasic, Auth) . These leaned on a AuthClient class that knew how to speak the legacy protocol.

AuthClient is gone, as it's entirely replaced by the planet_auth and planet_auth_utils that speak legacy, a bunch of flavors of OAuth, or plain old static key.

APIKeyAuth(httpx.AuthBasic, Auth) is replaced by _PLAuthLibAuth(Auth), which connects the concepts implemented in planet_auth to the structures already present in the SDK.

One thing that was and still is different from the SDK's planet.Auth class and planet_auth's planet_auth.Auth class is the SDK's Auth takes the approach that the Auth object is a httpx.Auth authenticator object (which is the thing that can mutate a request() to be auth'ed). planet_auth.Auth on the other hand is an auth context that has a request authenticator, since I treat the business of obtaining, refreshing, revoking, auth tokens as distinct from using the auth tokens to make requests. The planet_auth.Auth also has planet_auth's equlivilent of the AuthClient, carrying it round in a way the old SDK did not. This is necessary for OAuth in a way it was not when we just used the legacy auth protocol to unwrap a JWT and get the API key, because unlike API keys the OAuth JWTs expire, and we need to keep the AuthClient equivalent around to make transparent token refresh work.

One change I did make to the SDK's planet.Auth interface was to push the extension of httpx.Auth up from the old child class of APIKeyAuth(planet.Auth) into the abstract base class. This let me take out the old AuthType = httpx.Auth declaration which I felt let my statics have a clear factory signatures like def some_static_factory_methd() -> Auth. The old signature with def some_static_factory() -> AuthType I felt tried to pretent it was hiding httpx, but wasn't while also preventing me from having more than just httpx.Auth methods on the AuthType (which I wanted so the rest of the code could do the right thing for managing multi-step device code flow login or saving state to application defined storage for smooth use of refresh tokens across sessions.

Layers, man. This problem can have twisty layers. Open to better ideas.

[carl-adams-planet]

Deprecation - Maybe I missed a spot. I tried to do this:

• Deprecated Now. It doesn't work. I raise DeprecationWarning and using that path will fail. The value I see in in the raise vs just removing the code outright is devx hand holding. We can just as well release note it, and I actually love it more when the merge has more delete lines.
• Pending Deprecation Warning. I use warnings.warn("some message", PendingDeprecationWarning) . Taking this path should still work, but it's not advised and we would like to discourage this path and maybe someday remove it.
• Annotation - I guess I am not using that. I could I suppose. I think I set down this path because in an earlier version of my changes the warnings were raised in particular branches of if statements. That might not be true of any of the (pending) deprecated paths

[carl-adams-planet]

Auth profile of none (and EmptyBuiltinProfileConstants):

The none profile lets you try a request unauthenticated easily. Maybe not super useful, but possible and somewhat falls out of plauth being more general than planet. I'm sure I copy-pasta that from elsewhere. Say the word and I can remove it from planet. I almost did myself, but might have left it for unit tests. If you want to, it's possible to do something like planet --auth-profile none data search-list. If a client had unprotected endpoints, this does provide a path to using those when your session is foobar without the client needing to think about paths for authenticated and unauthenticated sessions. (somewhat contrived, I admit)

Similar story with EmptyBuiltinProfileConstants. The built-in provider is a later/newer concept in this history of these libs. I found it far less disruptive to be able to fall back to an empty shell than NPE and sprinkle a bunch of "If not None" all over the code.

[carl-adams-planet]

As a side comment, using plauth outside planet APIs and the SDK isn't academic. I do this today in my dev envs. I use it to talk to vendor APIs. I use it to talk to other internal services not under Planet's OAuth servers that may not have mature client libs. I have my Bruno API client wired up to understand my ~/.planet/ directory to share access tokens with any number of auth servers and identities.

[carl-adams-planet]

Bug:
Can you share with me what you have in various PL_ env vars, and the structure of your ~/.planet.json file (if it exists)?

[carl-adams-planet]

Removed "none" built-in profile per feedback.

Change pushed to MR.