From 919e7da3fa9e21c7ed8755feda88720d3dd01a0d Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Wed, 19 Feb 2025 16:14:15 -0800 Subject: [PATCH] work to write auth client configs, update token data, and provide initial token data --- src/planet_auth/auth.py | 34 ++++++-- src/planet_auth/oidc/request_authenticator.py | 11 +++ .../planet_legacy/request_authenticator.py | 13 +++ src/planet_auth/request_authenticator.py | 24 +++++- src/planet_auth_utils/plauth_factory.py | 81 +++++++++++++++++-- .../oidc/test_oidc_request_authenticator.py | 24 +++++- ...est_planet_legacy_request_authenticator.py | 62 ++++++++++++-- ...st_static_api_key_request_authenticator.py | 44 +++++++++- 8 files changed, 268 insertions(+), 25 deletions(-) diff --git a/src/planet_auth/auth.py b/src/planet_auth/auth.py index d212418..25e5148 100644 --- a/src/planet_auth/auth.py +++ b/src/planet_auth/auth.py @@ -28,8 +28,6 @@ def __init__(self, **kwargs): super().__init__(**kwargs) -# TODO: should this be renamed? AuthClientContext? ClientAuthContext? ClientAuth? -# This isn't really geared towards resource server use cases. class Auth: """ A container class for initializing and managing a working set of @@ -139,6 +137,7 @@ def device_login_complete(self, initiated_login_data: dict) -> Credential: @staticmethod def initialize_from_client( auth_client: AuthClient, + initial_token_data: Optional[dict] = None, token_file: Optional[Union[str, pathlib.PurePath]] = None, profile_name: Optional[str] = None, ) -> Auth: @@ -148,6 +147,8 @@ def initialize_from_client( Parameters: auth_client: An already constructed [AuthClient][planet_auth.AuthClient] object. + initial_token_data: Token data to use for initial state, if not to be + read from a file for initialization. token_file: A path to a file location that should be used for credential storage. Credentials will be read from this file, and this location may be used to save refreshed credentials. @@ -164,7 +165,8 @@ def initialize_from_client( token_file_path = None request_authenticator = auth_client.default_request_authenticator(credential=token_file_path) # type: ignore - + if initial_token_data: + request_authenticator.update_credential_data(initial_token_data) return Auth( auth_client=auth_client, request_authenticator=request_authenticator, @@ -175,6 +177,7 @@ def initialize_from_client( @staticmethod def initialize_from_config( client_config: AuthClientConfig, + initial_token_data: Optional[dict] = None, token_file: Optional[Union[str, pathlib.PurePath]] = None, profile_name: Optional[str] = None, ) -> Auth: @@ -183,6 +186,8 @@ def initialize_from_config( returning them in a new container object. Parameters: client_config: A constructed AuthClientConfig object. + initial_token_data: Token data to use for initial state, if not to be + read from a file for initialization. token_file: A path to a file location that should be used for credential storage. Credentials will be read from this file, and this location may be used to save refreshed credentials. @@ -194,11 +199,17 @@ def initialize_from_config( configuration files or where to save tokens. """ auth_client = AuthClient.from_config(config=client_config) - return Auth.initialize_from_client(auth_client=auth_client, token_file=token_file, profile_name=profile_name) + return Auth.initialize_from_client( + auth_client=auth_client, + initial_token_data=initial_token_data, + token_file=token_file, + profile_name=profile_name, + ) @staticmethod def initialize_from_config_dict( client_config: dict, + initial_token_data: Optional[dict] = None, token_file: Optional[Union[str, pathlib.PurePath]] = None, profile_name: Optional[str] = None, ) -> Auth: @@ -207,6 +218,8 @@ def initialize_from_config_dict( returning them in a new container object. Parameters: client_config: A dictionary containing a valid auth client configuration. + initial_token_data: Token data to use for initial state, if not to be + read from a file for initialization. token_file: A path to a file location that should be used for credential storage. Credentials will be read from this file, and this location may be used to save refreshed credentials. @@ -219,12 +232,16 @@ def initialize_from_config_dict( """ auth_client_config = AuthClientConfig.from_dict(client_config) return Auth.initialize_from_config( - client_config=auth_client_config, token_file=token_file, profile_name=profile_name + client_config=auth_client_config, + initial_token_data=initial_token_data, + token_file=token_file, + profile_name=profile_name, ) @staticmethod def initialize_from_config_file( client_config_file: Union[str, pathlib.PurePath], + initial_token_data: Optional[dict] = None, token_file: Union[str, pathlib.PurePath] = None, profile_name: Optional[str] = None, ) -> Auth: @@ -234,6 +251,8 @@ def initialize_from_config_file( Parameters: client_config_file: A file containing a client config json definition. The file should be a .json or .sops.json file + initial_token_data: Token data to use for initial state, if not to be + read from a file for initialization. token_file: A path to a file location that should be used for credential storage. Credentials will be read from this file, and this location may be used to save refreshed credentials. @@ -246,5 +265,8 @@ def initialize_from_config_file( """ auth_client_config = AuthClientConfig.from_file(client_config_file) return Auth.initialize_from_config( - client_config=auth_client_config, token_file=token_file, profile_name=profile_name + client_config=auth_client_config, + initial_token_data=initial_token_data, + token_file=token_file, + profile_name=profile_name, ) diff --git a/src/planet_auth/oidc/request_authenticator.py b/src/planet_auth/oidc/request_authenticator.py index ea018ad..149953e 100644 --- a/src/planet_auth/oidc/request_authenticator.py +++ b/src/planet_auth/oidc/request_authenticator.py @@ -119,6 +119,17 @@ def update_credential(self, new_credential: Credential): self._refresh_at = 0 # self._load() # Mimic __init__. Don't load, let that happen JIT. + def update_credential_data(self, new_credential_data: dict): + # This is more different than update_credential() than it may + # appear. Inherent in being passed a Credential in update_credential() + # is that it may not yet be loaded from disk, and so deferring + # the _load() as a JIT operation is appropriate. In this case, + # being passed the data struct is as if a network refresh call + # has already taken place or a _load() is in progress, and we + # should behave as if we are finishing a _refresh() call. + super().update_credential_data(new_credential_data=new_credential_data) + self._load() + class RefreshOrReloginOidcTokenRequestAuthenticator(RefreshingOidcTokenRequestAuthenticator): """ diff --git a/src/planet_auth/planet_legacy/request_authenticator.py b/src/planet_auth/planet_legacy/request_authenticator.py index 2f77643..79c0dc2 100644 --- a/src/planet_auth/planet_legacy/request_authenticator.py +++ b/src/planet_auth/planet_legacy/request_authenticator.py @@ -31,3 +31,16 @@ def __init__(self, planet_legacy_credential: FileBackedPlanetLegacyApiKey, **kwa def pre_request_hook(self): self._token_body = self._api_key_file.legacy_api_key() + + def update_credential(self, new_credential): + if not isinstance(new_credential, FileBackedPlanetLegacyApiKey): + raise TypeError( + f"{type(self).__name__} does not support {type(new_credential)} credentials. Use FileBackedPlanetLegacyApiKey." + ) + super().update_credential(new_credential) + self._api_key_file = new_credential + + # def update_credential_data(self, new_credential_data: dict): + # super().update_credential_data(new_credential_data=new_credential_data) + # # The super class is not changing the instance, so we don't need to update our reference + # # self._api_key_file = self._credential diff --git a/src/planet_auth/request_authenticator.py b/src/planet_auth/request_authenticator.py index b72e967..f1ce71b 100644 --- a/src/planet_auth/request_authenticator.py +++ b/src/planet_auth/request_authenticator.py @@ -107,6 +107,28 @@ def update_credential(self, new_credential: Credential): # requests. self._token_body = None + def update_credential_data(self, new_credential_data: dict): + """ + Provide raw data that should be used to update the Credential + object used to authenticate requests. This information will + be treated as newer than any file back data associated + with the credential held by the authenticator, and the file + will be overwritten. + """ + if not self._credential: + # The use cases for this being unset are narrow - mostly in the noop_auth.py module, + # and the service side client_validator.py. Neither of these use cases should + # ever need to call update_credential_data(). + # Should we fail loud, or quiet? Loud for now. + raise RuntimeError("Programming error. Cannot update a credential that has has never been set.") + + self._credential.set_data(new_credential_data) + self._credential.save() # Clobber old data that may be saved to disk. + # Clear-out auth material when a new credential is set. + # child classes are expected to populate it JIT for auth + # requests. + self._token_body = None + def credential(self): """ Return the current credential. @@ -121,7 +143,7 @@ def credential(self): class SimpleInMemoryRequestAuthenticator(CredentialRequestAuthenticator): # This SimpleInMemoryRequestAuthenticator subclasses from # CredentialRequestAuthenticator so it can fit in places that - # is needed. It does not provide actually know about any Credential + # is needed. It does not actually know about any Credential # types, since how credential types map to the HTTP request # authentication fields is credential specific. This class # is more useful for testing and stubbing out interfaces. diff --git a/src/planet_auth_utils/plauth_factory.py b/src/planet_auth_utils/plauth_factory.py index b9428eb..7bff2fe 100644 --- a/src/planet_auth_utils/plauth_factory.py +++ b/src/planet_auth_utils/plauth_factory.py @@ -23,7 +23,12 @@ PlanetLegacyRequestAuthenticator, AuthException, ) -from planet_auth.constants import TOKEN_FILE_SOPS, TOKEN_FILE_PLAIN +from planet_auth.constants import ( + TOKEN_FILE_SOPS, + TOKEN_FILE_PLAIN, + AUTH_CONFIG_FILE_SOPS, + AUTH_CONFIG_FILE_PLAIN, +) from planet_auth_utils.profile import Profile from planet_auth_utils.builtins import Builtins @@ -33,6 +38,10 @@ logger = logging.getLogger(__name__) +# TODO: we should probably add support for https://pypi.org/project/keyring/ +# or the like. We would have to work out how we want independent installations +# to interact, if not through ~/.planet/. How would QGIS interact with the CLI +# to manage a session, for example. class PlanetAuthFactory: @staticmethod def _token_file_path(profile_name: str, overide_path: Optional[str], save_token_file: bool): @@ -47,6 +56,32 @@ def _token_file_path(profile_name: str, overide_path: Optional[str], save_token_ else: return None + @staticmethod + def _auth_client_config_file_path(profile_name: str): + return Profile.get_profile_file_path_with_priority( + filenames=[AUTH_CONFIG_FILE_SOPS, AUTH_CONFIG_FILE_PLAIN], profile=profile_name + ) + + @staticmethod + def _update_saved_profile_config(plauth_context: Auth): + # Fixme: make this a method on Auth? + # FIXME: integrate with app provided storage? + # FIXME: What about sops? + + # FIXME: this is probably redundant with the code to save profiles wired to the CLI. + # Consolidate these paths + # FIXME: Per above, should we clobber or check? Options may change. + # FIXME: Should not save built-in profiles + + _profile_name = plauth_context.profile_name() + if not _profile_name: + raise MissingArgumentException("A profile name must be provided if persisting the profile configuration") + + plauth_context._auth_client.config().set_path( + PlanetAuthFactory._auth_client_config_file_path(profile_name=_profile_name) + ) + plauth_context._auth_client.config().save() + @staticmethod def _init_context_from_profile( profile_name: str, @@ -78,6 +113,8 @@ def _init_context_from_oauth_svc_account( client_secret: str, token_file_opt: Optional[str] = None, save_token_file: bool = True, + # profile_name: Optional[str] = None, # XXXX Ad-Hoc? + save_profile_config: bool = False, ) -> Auth: # TODO: support oauth service accounts that use pubkey, and not just client secrets. # TODO: Can we handle different trust realms when initializing a M2M client with @@ -96,27 +133,38 @@ def _init_context_from_oauth_svc_account( profile_name=adhoc_profile_name, overide_path=token_file_opt, save_token_file=save_token_file ) - return Auth.initialize_from_config_dict( + plauth_context = Auth.initialize_from_config_dict( client_config=constructed_client_config_dict, token_file=token_file_path, profile_name=adhoc_profile_name, ) + if save_profile_config: + PlanetAuthFactory._update_saved_profile_config(plauth_context) + + return plauth_context @staticmethod def _init_from_client_config( client_config: dict, profile_name: str, + initial_token_data: Optional[dict] = None, save_token_file: bool = True, + save_profile_config: bool = True, ) -> Auth: token_file_path = PlanetAuthFactory._token_file_path( profile_name=profile_name, overide_path=None, save_token_file=save_token_file ) - return Auth.initialize_from_config_dict( + plauth_context = Auth.initialize_from_config_dict( client_config=client_config, + initial_token_data=initial_token_data, token_file=token_file_path, profile_name=profile_name, ) + if save_profile_config: + PlanetAuthFactory._update_saved_profile_config(plauth_context) + + return plauth_context # @staticmethod # def _init_context_from_legacy_username_password_key( @@ -151,12 +199,17 @@ def _init_context_from_api_key(api_key: str) -> Auth: "bearer_token_prefix": PlanetLegacyRequestAuthenticator.TOKEN_PREFIX, } adhoc_profile_name = "_PL_API_KEY" - return Auth.initialize_from_config_dict( + plauth_context = Auth.initialize_from_config_dict( client_config=constructed_client_config_dict, token_file=None, - profile_name=adhoc_profile_name, + profile_name=adhoc_profile_name, # XXX ??? ) + # if save_profile_config: + # PlanetAuthFactory._update_saved_profile_config(plauth_context) + + return plauth_context + @staticmethod def initialize_auth_client_context( auth_profile_opt: Optional[str] = None, @@ -164,7 +217,9 @@ def initialize_auth_client_context( auth_client_secret_opt: Optional[str] = None, auth_api_key_opt: Optional[str] = None, # Deprecated token_file_opt: Optional[str] = None, # TODO: Remove, but we still depend on it for Planet Legacy use cases. + # TODO?: initial_token_data: dict = None, save_token_file: bool = True, + save_profile_config: bool = False, # Implicitly create/update a profile on disk ) -> Auth: """ Helper function to initialize the Auth context in applications. @@ -231,6 +286,8 @@ def my_cli_main(ctx, auth_profile, auth_client_id, auth_client_secret): client_secret=auth_client_secret_opt, token_file_opt=token_file_opt, save_token_file=save_token_file, + # profile_name="FIXME1", + save_profile_config=save_profile_config, ) if auth_api_key_opt: @@ -280,6 +337,8 @@ def my_cli_main(ctx, auth_profile, auth_client_id, auth_client_secret): client_secret=effective_user_selected_client_secret, token_file_opt=token_file_opt, save_token_file=save_token_file, + # profile_name="FIXME2", + save_profile_config=save_profile_config, ) effective_user_selected_api_key = user_config_file.effective_conf_value( @@ -311,12 +370,20 @@ def my_cli_main(ctx, auth_profile, auth_client_id, auth_client_secret): ) @staticmethod - def initialize_auth_client_context_with_config( + def initialize_auth_client_context_from_config( client_config: dict, profile_name: str, + initial_token_data: Optional[dict] = None, save_token_file: bool = True, + save_profile_config: bool = True, # Implicitly create/update a profile on disk ) -> Auth: - return PlanetAuthFactory._init_from_client_config(client_config, profile_name, save_token_file) + return PlanetAuthFactory._init_from_client_config( + client_config=client_config, + profile_name=profile_name, + initial_token_data=initial_token_data, + save_token_file=save_token_file, + save_profile_config=save_profile_config, + ) @staticmethod def initialize_resource_server_validator( diff --git a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_oidc_request_authenticator.py b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_oidc_request_authenticator.py index 8cc99c1..9b52af1 100644 --- a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_oidc_request_authenticator.py +++ b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_oidc_request_authenticator.py @@ -14,9 +14,9 @@ from unittest.mock import MagicMock +import freezegun import pathlib import tempfile -import freezegun import unittest from planet_auth.oidc.oidc_credential import FileBackedOidcCredential @@ -408,6 +408,28 @@ def test_side_band_update_credential(self, frozen_time): # def test_side_band_update_credential_in_memory(self): # under_test = self.under_test_happy_path_in_memory() + def test_side_band_update_credential_data(self): + # Similar to above, the credential is updated by a + # side-band call to update the data. + under_test = self.under_test_happy_path() + self.mock_api_call(under_test) # Should ensure that our old data is all primed and used + initial_credential_data = under_test.credential().data() + + # abuse login to dummy up some other valid credential structure. + sideband_credential = self.stub_auth_client.login( + get_access_token=True, get_refresh_token=True, get_id_token=False + ) + under_test.update_credential_data(sideband_credential.data()) + + # update_credential_data() should leave is us in a freshly _load()'ed state. + # It should not be necessary to simulate an API call for everything to be set + current_credential_data = under_test._credential.data() + self.assertNotEqual(current_credential_data, initial_credential_data) + self.assertEqual(current_credential_data, sideband_credential.data()) + self.assertEqual(under_test._token_body, sideband_credential.access_token()) + self.assertNotEqual(0, under_test._refresh_at) + self.assertNotEqual(0, under_test._credential._load_time) + class RefreshOrReloginOidcRequestAuthenticatorTest(unittest.TestCase): def setUp(self): diff --git a/tests/test_planet_auth/unit/auth/auth_clients/planet_legacy/test_planet_legacy_request_authenticator.py b/tests/test_planet_auth/unit/auth/auth_clients/planet_legacy/test_planet_legacy_request_authenticator.py index e60d5f3..3b4bbed 100644 --- a/tests/test_planet_auth/unit/auth/auth_clients/planet_legacy/test_planet_legacy_request_authenticator.py +++ b/tests/test_planet_auth/unit/auth/auth_clients/planet_legacy/test_planet_legacy_request_authenticator.py @@ -13,6 +13,9 @@ # limitations under the License. import unittest +import shutil +import tempfile +import pathlib from planet_auth.planet_legacy.legacy_api_key import FileBackedPlanetLegacyApiKey from planet_auth.planet_legacy.request_authenticator import PlanetLegacyRequestAuthenticator @@ -21,20 +24,67 @@ class PlanetLegacyRequestAuthenticatorTest(unittest.TestCase): + def setUp(self): + # Copy because some actions may modify the test data files. + self.tmp_dir = tempfile.TemporaryDirectory() + self.tmp_dir_path = pathlib.Path(self.tmp_dir.name) + + self.invalid_cred_file = self.tmp_dir_path.joinpath("invalid_test_credential.json") + shutil.copy( + tdata_resource_file_path("keys/invalid_test_credential.json"), + self.invalid_cred_file, + ) + + self.valid_cred_file = self.tmp_dir_path.joinpath("planet_legacy_test_credential.json") + shutil.copy( + tdata_resource_file_path("keys/planet_legacy_test_credential.json"), + self.valid_cred_file, + ) + def test_pre_request_hook_loads_from_file_happy_path(self): under_test = PlanetLegacyRequestAuthenticator( - planet_legacy_credential=FileBackedPlanetLegacyApiKey( - api_key_file=tdata_resource_file_path("keys/planet_legacy_test_credential.json") - ) + planet_legacy_credential=FileBackedPlanetLegacyApiKey(api_key_file=self.valid_cred_file) ) under_test.pre_request_hook() self.assertEqual("test_legacy_api_key", under_test._api_key_file.legacy_api_key()) def test_pre_request_hook_loads_from_file_invalid_throws(self): under_test = PlanetLegacyRequestAuthenticator( - planet_legacy_credential=FileBackedPlanetLegacyApiKey( - api_key_file=tdata_resource_file_path("keys/invalid_test_credential.json") - ) + planet_legacy_credential=FileBackedPlanetLegacyApiKey(api_key_file=self.invalid_cred_file) ) with self.assertRaises(FileBackedJsonObjectException): under_test.pre_request_hook() + + def test_update_credential_data(self): + under_test = PlanetLegacyRequestAuthenticator( + planet_legacy_credential=FileBackedPlanetLegacyApiKey(api_key_file=self.valid_cred_file) + ) + under_test.pre_request_hook() # Triggers a JIT load from the file + + orig_data = under_test._credential.data() + new_data = orig_data.copy() + new_data["key"] = "utest_new_api_key" + + under_test.update_credential_data(new_credential_data=new_data) + # Side effect : clobbers the file contents. + + under_test.pre_request_hook() # Simulate another login event + self.assertEqual("utest_new_api_key", under_test._token_body) # Base class view + self.assertEqual("utest_new_api_key", under_test._api_key_file.legacy_api_key()) # child class view + + # self.assertEqual("TODO?", under_test._token_prefix) + + def test_update_credential(self): + under_test = PlanetLegacyRequestAuthenticator( + planet_legacy_credential=FileBackedPlanetLegacyApiKey(api_key_file=self.valid_cred_file) + ) + under_test.pre_request_hook() # Triggers a JIT load from the file of the initial credential. + + new_credential = FileBackedPlanetLegacyApiKey(api_key="utest_new_api_key_2") + under_test.update_credential(new_credential=new_credential) + + under_test.pre_request_hook() # Simulate another login event + self.assertEqual("utest_new_api_key_2", under_test._token_body) # Base class view + self.assertEqual("utest_new_api_key_2", under_test._api_key_file.legacy_api_key()) # child class view + + # self.assertEqual("TODO?", under_test._token_prefix) diff --git a/tests/test_planet_auth/unit/auth/auth_clients/static_api_key/test_static_api_key_request_authenticator.py b/tests/test_planet_auth/unit/auth/auth_clients/static_api_key/test_static_api_key_request_authenticator.py index 697918e..213a919 100644 --- a/tests/test_planet_auth/unit/auth/auth_clients/static_api_key/test_static_api_key_request_authenticator.py +++ b/tests/test_planet_auth/unit/auth/auth_clients/static_api_key/test_static_api_key_request_authenticator.py @@ -13,6 +13,9 @@ # limitations under the License. import unittest +import shutil +import tempfile +import pathlib from planet_auth.static_api_key.request_authenticator import FileBackedApiKeyRequestAuthenticator from planet_auth.static_api_key.static_api_key import FileBackedApiKey @@ -21,10 +24,27 @@ class StaticApiKeyRequestAuthenticatorTest(unittest.TestCase): + def setUp(self): + # Copy because some actions may modify the test data files. + self.tmp_dir = tempfile.TemporaryDirectory() + self.tmp_dir_path = pathlib.Path(self.tmp_dir.name) + + self.invalid_cred_file = self.tmp_dir_path.joinpath("invalid_test_credential.json") + shutil.copy( + tdata_resource_file_path("keys/invalid_test_credential.json"), + self.invalid_cred_file, + ) + + self.valid_cred_file = self.tmp_dir_path.joinpath("static_api_key_test_credential.json") + shutil.copy( + tdata_resource_file_path("keys/static_api_key_test_credential.json"), + self.valid_cred_file, + ) + def test_pre_request_hook_loads_from_file_happy_path(self): under_test = FileBackedApiKeyRequestAuthenticator( api_key_credential=FileBackedApiKey( - api_key_file=tdata_resource_file_path("keys/static_api_key_test_credential.json") + api_key_file=self.valid_cred_file, ) ) under_test.pre_request_hook() @@ -33,9 +53,25 @@ def test_pre_request_hook_loads_from_file_happy_path(self): def test_pre_request_hook_loads_from_file_invalid_throws(self): under_test = FileBackedApiKeyRequestAuthenticator( - api_key_credential=FileBackedApiKey( - api_key_file=tdata_resource_file_path("keys/invalid_test_credential.json") - ) + api_key_credential=FileBackedApiKey(api_key_file=self.invalid_cred_file) ) with self.assertRaises(FileBackedJsonObjectException): under_test.pre_request_hook() + + def test_update_credential_data(self): + under_test = FileBackedApiKeyRequestAuthenticator( + api_key_credential=FileBackedApiKey(api_key_file=self.valid_cred_file) + ) + under_test.pre_request_hook() # Triggers a JIT load from the file + + orig_data = under_test._credential.data() + new_data = orig_data.copy() + new_data["api_key"] = "utest_new_api_key" + new_data["bearer_token_prefix"] = "utest_new_prefix" + + under_test.update_credential_data(new_credential_data=new_data) + # Side effect : clobbers the file contents. + + under_test.pre_request_hook() # Simulate another login event + self.assertEqual("utest_new_api_key", under_test._token_body) + self.assertEqual("utest_new_prefix", under_test._token_prefix)