diff --git a/docs/changelog.md b/docs/changelog.md index 23123ef..826645b 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,17 @@ # Changelog +## 2.3.0 - TBD +- Improve the user experience around old stale sessions that appear to be + initialized, but are actually expired. This is done by providing the new + utility method `Auth.ensure_request_authenticator_is_ready()`. +- Save computed expiration time and issued time in token files. This allows + for the persistence of this information when dealing with opaque tokens. + - **Note**: Previously saved OAuth access tokens that are not JWTs with + an `exp` claim that can be inspected will be considered to expire in + `expires_in` seconds from the time they are loaded, since the time + they were issued was not saved in the past. +- Support non-expiring tokens. + ## 2.2.0 - 2025-10-02 - Update supported python versions. Support for 3.9 dropped. Support through 3.14 added. diff --git a/docs/examples/auth-client/oauth/make-oauth-authenticated-httpx-request.py b/docs/examples/auth-client/oauth/make-oauth-authenticated-httpx-request.py index 422a0c4..6a7795b 100644 --- a/docs/examples/auth-client/oauth/make-oauth-authenticated-httpx-request.py +++ b/docs/examples/auth-client/oauth/make-oauth-authenticated-httpx-request.py @@ -6,6 +6,7 @@ def main(): logging.basicConfig(level=logging.DEBUG) auth_ctx = planet_auth_utils.PlanetAuthFactory.initialize_auth_client_context(auth_profile_opt="my-custom-profile") + auth_ctx.ensure_request_authenticator_is_ready(allow_open_browser=True, allow_tty_prompt=True) result = httpx.get( url="https://api.planet.com/basemaps/v1/mosaics", auth=auth_ctx.request_authenticator(), diff --git a/docs/examples/auth-client/oauth/make-oauth-authenticated-requests-request.py b/docs/examples/auth-client/oauth/make-oauth-authenticated-requests-request.py index 84c0bf8..8e4725c 100644 --- a/docs/examples/auth-client/oauth/make-oauth-authenticated-requests-request.py +++ b/docs/examples/auth-client/oauth/make-oauth-authenticated-requests-request.py @@ -6,6 +6,7 @@ def main(): logging.basicConfig(level=logging.DEBUG) auth_ctx = planet_auth_utils.PlanetAuthFactory.initialize_auth_client_context(auth_profile_opt="my-custom-profile") + auth_ctx.ensure_request_authenticator_is_ready(allow_open_browser=True, allow_tty_prompt=True) result = requests.get( url="https://api.planet.com/basemaps/v1/mosaics", auth=auth_ctx.request_authenticator(), diff --git a/docs/examples/auth-client/oauth/perform-oauth-initial-login-2.py b/docs/examples/auth-client/oauth/perform-oauth-initial-login-2.py new file mode 100644 index 0000000..b75d095 --- /dev/null +++ b/docs/examples/auth-client/oauth/perform-oauth-initial-login-2.py @@ -0,0 +1,13 @@ +import logging +import planet_auth_utils + + +def main(): + logging.basicConfig(level=logging.DEBUG) + auth_ctx = planet_auth_utils.PlanetAuthFactory.initialize_auth_client_context(auth_profile_opt="my-custom-profile") + + auth_ctx.ensure_request_authenticator_is_ready(allow_open_browser=True, allow_tty_prompt=True) + + +if __name__ == "__main__": + main() diff --git a/src/planet_auth/__init__.py b/src/planet_auth/__init__.py index 7704315..667fcc7 100644 --- a/src/planet_auth/__init__.py +++ b/src/planet_auth/__init__.py @@ -56,7 +56,7 @@ class exists. """ -from .auth import Auth +from .auth import Auth, AuthClientContextException from .auth_exception import AuthException from .auth_client import AuthClientConfig, AuthClient from .credential import Credential @@ -145,6 +145,7 @@ class exists. "Auth", "AuthClient", "AuthClientConfig", + "AuthClientContextException", "AuthCodeAuthClient", "AuthCodeClientConfig", "AuthCodeWithClientSecretAuthClient", diff --git a/src/planet_auth/auth.py b/src/planet_auth/auth.py index 85d3ecf..216c417 100644 --- a/src/planet_auth/auth.py +++ b/src/planet_auth/auth.py @@ -18,6 +18,7 @@ from typing import Optional, Union from planet_auth.auth_client import AuthClient, AuthClientConfig +from planet_auth.auth_exception import AuthException from planet_auth.credential import Credential from planet_auth.request_authenticator import CredentialRequestAuthenticator from planet_auth.storage_utils import ObjectStorageProvider @@ -25,9 +26,10 @@ auth_logger = getAuthLogger() -# class AuthClientContextException(AuthException): -# def __init__(self, **kwargs): -# super().__init__(**kwargs) + +class AuthClientContextException(AuthException): + def __init__(self, **kwargs): + super().__init__(**kwargs) class Auth: @@ -113,10 +115,108 @@ def request_authenticator_is_ready(self) -> bool: For example, simple API key clients only need an API key in their configuration. OAuth2 user clients need to have performed a user login and obtained access or refresh tokens. + + Note: This will not detect when a credential is expired or + otherwise invalid. """ return self._request_authenticator.is_initialized() or self._auth_client.can_login_unattended() - def login(self, **kwargs) -> Credential: + def ensure_request_authenticator_is_ready( + self, allow_open_browser: Optional[bool] = False, allow_tty_prompt: Optional[bool] = False + ) -> None: + """ + Do everything necessary to ensure the request authenticator is ready for use, + while still biasing towards just-in-time operations and not making + unnecessary network requests or prompts for user interaction. + + This can be more complex than it sounds given the variations in the + capabilities of authentication clients and possible session states. + Clients may be initialized with active sessions, initialized with stale + but still valid sessions, initialized with invalid or expired + sessions, or completely uninitialized. The process taken to ensure + client readiness with as little user disruption as possible + is as follows: + + 1. If the client has been logged in and has a non-expired + short-term access token, the client will be considered + ready without prompting the user or probing the network. + This will not require user interaction. + 2. If the client has not been logged in and is a type that + can do so without prompting the user, the client will be + considered ready without prompting the user or probing + the network. This will not require user interaction. + Login will be delayed until it is required. + 3. If the client has been logged in and has an expired + short-term access token, the network will be probed to attempt + a refresh of the session. This should not require user interaction. + If refresh fails, the user will be prompted to perform a fresh + login, requiring user interaction. + 4. If the client has never been logged in and is a type that + requires a user interactive login, a user interactive + login will be initiated. + + There still may be conditions where we believe we are + ready, but requests still ultimately fail. For example, if + the auth context holds a static API key or username/password, it is + assumed to be ready but the credentials could be bad. Even when ready + with valid credentia, requests could fail if the service + rejects the request due to its own policy configuration. + + Parameters: + allow_open_browser: specify whether login is permitted to open + a browser window. + allow_tty_prompt: specify whether login is permitted to request + input from the terminal. + """ + + def _has_credential() -> bool: + # Does not do any JIT checks + return self._request_authenticator.is_initialized() + + def _can_obtain_credentials_unattended() -> bool: + # Does not do any JIT checks + return self._auth_client.can_login_unattended() + + def _is_expired() -> bool: + # Does not do any JIT check + new_cred = self._request_authenticator.credential(refresh_if_needed=False) + if new_cred: + return new_cred.is_expired() + return True + + # Case #1 above. + if _has_credential() and not _is_expired(): + return + + # Case #2 above. + if _can_obtain_credentials_unattended(): + # Should we fetch one? We do not by default because the bias is towards + # JIT operations and silent operations. This so programs can initialize and + # not fail for auth reasons unless the credential is actually needed. + return + + # Case #3 above. + if _has_credential() and _is_expired(): + try: + # This takes care of making sure the authenticator's credential is + # current with the update. No further action needed on our part. + new_cred = self._request_authenticator.credential(refresh_if_needed=True) + if not new_cred: + raise RuntimeError("Unable to refresh credentials - Unknown error") + if new_cred.is_expired(): + raise RuntimeError("Unable to refresh credentials - Refreshed credentials are still expired.") + return + except Exception as e: + auth_logger.warning( + msg=f"Failed to refresh expired credentials (Error: {str(e)}). Attempting interactive login." + ) + + # Case #4 above. + self.login(allow_open_browser=allow_open_browser, allow_tty_prompt=allow_tty_prompt) + + def login( + self, allow_open_browser: Optional[bool] = False, allow_tty_prompt: Optional[bool] = False, **kwargs + ) -> Credential: """ Perform a login with the configured auth client. This higher level function will ensure that the token is saved to @@ -124,8 +224,20 @@ def login(self, **kwargs) -> Credential: storage path. Otherwise, the token will be held only in memory. In all cases, the request authenticator will also be updated with the new credentials. + + Parameters: + allow_open_browser: specify whether login is permitted to open + a browser window. + allow_tty_prompt: specify whether login is permitted to request + input from the terminal. """ - new_credential = self._auth_client.login(**kwargs) + new_credential = self._auth_client.login( + allow_open_browser=allow_open_browser, allow_tty_prompt=allow_tty_prompt, **kwargs + ) + if not new_credential: + # AuthClient.login() is supposed to raise on failure. + raise AuthClientContextException(message="Unknown login failure. No credentials and no error returned.") + new_credential.set_path(self._token_file_path) new_credential.set_storage_provider(self._storage_provider) new_credential.save() diff --git a/src/planet_auth/credential.py b/src/planet_auth/credential.py index 4b81e2e..d1aa57c 100644 --- a/src/planet_auth/credential.py +++ b/src/planet_auth/credential.py @@ -12,6 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import time +from typing import Optional + from planet_auth.storage_utils import FileBackedJsonObject @@ -22,7 +25,69 @@ class Credential(FileBackedJsonObject): storage provider implementation, clear-text .json files or .sops.json files with field level encryption are supported. Custom storage providers may offer different functionality. + + Subclass Implementor Notes: + The `Credential` class reserves the data fields `_iat` and `_exp` for + internal use. These are used to record the time the credential was + issued and the time that it expires respectively, expressed as seconds + since the epoch. A None or NULL value for `_exp` indicates that + the credential never expires. + + This base class does not set these values. It is the responsibility + of subclasses to set these values as appropriate. If left unset, + the credential will be treated as a non-expiring credential with an + indeterminate issued time. + + Subclasses that wish to provide values should do so in their + constructor and in their `set_data()` methods. """ def __init__(self, data=None, file_path=None, storage_provider=None): super().__init__(data=data, file_path=file_path, storage_provider=storage_provider) + + def expiry_time(self) -> Optional[int]: + """ + The time that the credential expires, expressed as seconds since the epoch. + """ + return self.lazy_get("_exp") + + def issued_time(self) -> Optional[int]: + """ + The time that the credential was issued, expressed as seconds since the epoch. + """ + return self.lazy_get("_iat") + + def is_expiring(self) -> bool: + """ + Return true if the credential has an expiry time. + """ + return self.expiry_time() is not None + + def is_non_expiring(self) -> bool: + """ + Return true if the credential never expires. + """ + return not self.is_expiring() + + def is_expired(self, at_time: Optional[int] = None) -> bool: + """ + Return true if the credential is expired at the specified time. + If no time is specified, the current time is used. + Non-expiring credentials will always return false. + """ + if self.is_non_expiring(): + return False + + if at_time is None: + at_time = int(time.time()) + + exp = self.expiry_time() + return bool(at_time >= exp) # type: ignore[operator] + + def is_not_expired(self, at_time: Optional[int] = None) -> bool: + """ + Return true if the credential is not expired at the specified time. + If no time is specified, the current time is used. + Non-expiring credentials will always return true. + """ + return not self.is_expired(at_time) diff --git a/src/planet_auth/oidc/oidc_credential.py b/src/planet_auth/oidc/oidc_credential.py index ce5e43c..06cdab2 100644 --- a/src/planet_auth/oidc/oidc_credential.py +++ b/src/planet_auth/oidc/oidc_credential.py @@ -12,19 +12,25 @@ # See the License for the specific language governing permissions and # limitations under the License. +import time from typing import Optional from planet_auth.credential import Credential from planet_auth.storage_utils import InvalidDataException, ObjectStorageProvider +from planet_auth.oidc.token_validator import TokenValidator, InvalidArgumentException class FileBackedOidcCredential(Credential): """ Credential object for storing OAuth/OIDC tokens. + Credential should conform to the "token" response + defined in RFC 6749 for OAuth access tokens with OIDC extensions + for ID tokens. """ def __init__(self, data=None, credential_file=None, storage_provider: Optional[ObjectStorageProvider] = None): super().__init__(data=data, file_path=credential_file, storage_provider=storage_provider) + self._augment_rfc6749_data() def check_data(self, data): """ @@ -36,6 +42,65 @@ def check_data(self, data): message="'access_token', 'id_token', or 'refresh_token' not found in file {}".format(self._file_path) ) + def _augment_rfc6749_data(self): + # RFC 6749 includes an optional "expires_in" expressing the lifespan of + # the token. But without knowing when a token was issued it tells us + # nothing about whether a token is actually valid. + # + # This function lest us augment our representation of this data to + # make this credential useful when reconstructed from saved data + # at a time that is distant from when the token was obtained from the + # authorization server. + # + # Edge case - It's possible that a JWT ID token has an expiration time + # that is different from the access token. It's also possible that + # we have a refresh token and not any other tokens (this state could + # be used for bootstrapping). We are really only tracking + # access token expiration at this time. + if not self._data: + return + + try: + access_token_str = self.access_token() + if access_token_str: + (_, jwt_hazmat_body, _) = TokenValidator.hazmat_unverified_decode(access_token_str) + else: + jwt_hazmat_body = None + except InvalidArgumentException: + # Proceed as if it's not a JWT. + jwt_hazmat_body = None + + # It's possible for the combination of a transparent bearer token, + # saved iat and exp values, and a expires_in value to be + # over-constrained. We apply the following priority, from highest + # to lowest: + # - Bearer token claims + # - Saved values in the credential file + # - Newly calculated values + # If a reasonable expiration time cannot be derived, + # tokens are assumed to never expire. + rfc6749_lifespan = self._data.get("expires_in", 0) + if jwt_hazmat_body: + _iat = jwt_hazmat_body.get("iat", self._data.get("_iat", int(time.time()))) + _exp = jwt_hazmat_body.get("exp", self._data.get("_exp", None)) + else: + _iat = self._data.get("_iat", int(time.time())) + _exp = self._data.get("_exp", None) + + if _exp is None and rfc6749_lifespan > 0: + _exp = _iat + rfc6749_lifespan + + self._data["_iat"] = _iat + self._data["_exp"] = _exp + + def set_data(self, data, copy_data: bool = True): + """ + Set credential data for an OAuth/OIDC credential. The data structure is expected + to be an RFC 6749 /token response structure. + """ + super().set_data(data, copy_data) + self._augment_rfc6749_data() + def access_token(self): """ Get the current access token. diff --git a/src/planet_auth/oidc/request_authenticator.py b/src/planet_auth/oidc/request_authenticator.py index 3ab06a2..12b2977 100644 --- a/src/planet_auth/oidc/request_authenticator.py +++ b/src/planet_auth/oidc/request_authenticator.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import jwt import time from typing import Dict, Optional @@ -53,29 +52,19 @@ def __init__(self, credential: FileBackedOidcCredential, auth_client: OidcAuthCl self._auth_client = auth_client self._refresh_at = 0 - def _load(self): + def _load_and_prime(self): if self._credential.path(): # allow in memory operation. self._credential.load() - access_token_str = self._credential.access_token() - # Absolutely not appropriate to not verify the signature in a token - # validation context (e.g. server side auth of a client). Here we - # know that's not what we are doing. This is a client helper class - # for clients who will be presenting tokens to such a server. We - # are inspecting ourselves, not verifying for trust purposes. - # We are not expected to be the audience. - # TODO: we should use `expires_in` from the response from the - # OAuth server, which would work for non-JWT opaque OAuth - # tokens. Since that is a relative time, we would also need - # to augment our FileBackedOidcCredential with an issued - # at time. - unverified_decoded_atoken = jwt.decode(access_token_str, options={"verify_signature": False}) # nosemgrep - iat = unverified_decoded_atoken.get("iat") or 0 - exp = unverified_decoded_atoken.get("exp") or 0 - # refresh at the 3/4 life - self._refresh_at = int(iat + (3 * (exp - iat) / 4)) - self._token_body = access_token_str + self._token_body = self._credential.access_token() + iat = self._credential.issued_time() or 0 + exp = self._credential.expiry_time() + if exp is None: + # Never expires. + self._refresh_at = None + else: + self._refresh_at = int(iat + (3 * (exp - iat) / 4)) def _refresh(self): if self._auth_client: @@ -91,15 +80,31 @@ def _refresh(self): # self.update_credential(new_credential=new_credentials) self._credential = new_credentials - self._load() + self._load_and_prime() + + def _refresh_needed(self, check_time: Optional[int] = None) -> bool: + if self._token_body is None: + # Always consider not having the token loaded and primed to be + # in need of a refresh. In _refresh_if_needed(), this will + # trigger a reload before doing a network refresh. + return True + + if self._refresh_at is None: + # Non-expiring token is currently loaded. + return False + + if check_time is None: + check_time = int(time.time()) + + return check_time > self._refresh_at def _refresh_if_needed(self): - # Reload the file before refreshing. Another process might - # have done it for us, and save us the network call. + # Reload the file before doing a refresh with the auth server. + # Another process might have done it for us, and save us the + # network call. # - # Also, if refresh tokens are configured to be one time use, - # we want a fresh refresh token. Stale refresh tokens may be - # invalid depending on server configuration. + # Also, we should always try to use a fresh refresh token. + # Refresh might be configured to be one time use. # # Also, it's possible that we have a valid refresh token, # but not an access token. When that's true, we should @@ -107,15 +112,17 @@ def _refresh_if_needed(self): # # If everything fails, continue with what we have. Let the API # we are calling decide if it's good enough. + now = int(time.time()) - if now > self._refresh_at: + if self._refresh_needed(now): try: - self._load() + self._load_and_prime() except Exception as e: # pylint: disable=broad-exception-caught auth_logger.warning( msg=f"Error loading auth token. Continuing with old configuration and token data. Load error: {str(e)}" ) - if now > self._refresh_at: + + if self._refresh_needed(now): try: self._refresh() except Exception as e: # pylint: disable=broad-exception-caught @@ -146,8 +153,11 @@ def update_credential(self, new_credential: Credential) -> None: f"{type(self).__name__} does not support {type(new_credential)} credentials. Use FileBackedOidcCredential." ) super().update_credential(new_credential=new_credential) + # Mimic __init__. + # Set refresh_at to 0 to force a JIT check. + # Do not load the credential at this time. Let that happen JIT. self._refresh_at = 0 - # self._load() # Mimic __init__. Don't load, let that happen JIT. + # self._load_and_prime() def update_credential_data(self, new_credential_data: Dict) -> None: # This is more different than update_credential() than it may @@ -158,7 +168,7 @@ def update_credential_data(self, new_credential_data: Dict) -> None: # 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() + self._load_and_prime() def credential(self, refresh_if_needed: bool = False) -> Optional[Credential]: if refresh_if_needed: @@ -201,4 +211,4 @@ def _refresh(self): # self.update_credential(new_credential=new_credentials) self._credential = new_credentials - self._load() + self._load_and_prime() diff --git a/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_no_lifespan_augmented.json b/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_no_lifespan_augmented.json new file mode 100644 index 0000000..e4d1290 --- /dev/null +++ b/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_no_lifespan_augmented.json @@ -0,0 +1,10 @@ +{ + "_comment": "_exp, _iat, and expires_in do not match the token. The token claims should take priority.", + "_exp": 2345, + "_iat": 1234, + "access_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Rfa2V5cGFpcjEiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJ0ZXN0X3ByaW1hcnlfaXNzdWVyIiwic3ViIjoidGVzdF91c2VyIiwiYXVkIjoidGVzdF9hdWRpZW5jZSIsImlhdCI6MTc1OTIwNjg5MSwiZXhwIjoxNzU5MjA2OTUxLCJqdGkiOiI5ODM5MDQ4MC0wNjczLTRmMjMtODRlNi0wNTZlYzYyZDg0ODciLCJ0ZXN0X2NsYXNzIjoiVGVzdE9pZGNDcmVkZW50aWFsIiwic2NvcGUiOiJ0ZXN0X3NjcDAgdGVzdF9zY3AxIn0.rJmP7KggXzYPvyKOZvE5m4XkR4H8tKdCxEUrUpJ66xfvx6nnbieFDO2sA5GtyNwkDgLcZhHkIILSq2WvfU5Wx1koyoFvaYTVJ3AaE4B3raJmrsSEroZR0uY89hlZ2f0bH5pTExizp0qpFs7f1HfvCyTkHIeks6xXXDHRFE8nTMR23BhIpl64sfYBBuXoWvFV6ptFrVUPZItX5iSkCZx-rAAA1MsoM0yoVXpaeY6yBN3z_jhq2AjeWsDD2LEjt4KoWrEvneqUKLbotp5SLqbin0qZ9xqHtxrXO1nUlpTvMhx8byE4wlw26dr93cbggvgEbqIprLs7-Jk44gwo4ilmNA", + "id_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Rfa2V5cGFpcjEiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJ0ZXN0X3ByaW1hcnlfaXNzdWVyIiwic3ViIjoiX19zdHViX2NsaWVudF9pZF9fIiwiYXVkIjoiX19zdHViX2NsaWVudF9pZF9fIiwiaWF0IjoxNzU5MjA2ODkxLCJleHAiOjE3NTkyMDY5NTEsImp0aSI6IjU3OWFlNGYwLWI1ZTUtNDRkNS1iNGM2LWY2MTVjMzY4Mjk4ZSIsInRlc3RfY2xhc3MiOiJUZXN0T2lkY0NyZWRlbnRpYWwiLCJzY29wZSI6InRlc3Rfc2NwMCB0ZXN0X3NjcDEifQ.RF8Hb8xf13yLPVedPJe3jqgjpD2kpjHz-RpP-vusW7ofuN1MLwZMmG7wbCriJbAk7OXUavfHytgivKGkCkkepQ0fwKACKNiveVCakQiYnEXIeLS9uu7wnOrLRbzCI6j0tm5996n8Wt-YuDFjbnpj5_isfIumjvB6oKYfLdTMZShlYccHDF8YG4Ug-8gHgZQktAVKGofMwbvabzOJdpBRn8-y5HIiC362mTVglG3zhZdiBEzBQpzgyDoDKl7CBjBOC4u7IjVsRsJ1pyuGALEbTOPnz-n7B_lr_C6-yp3ERFjd83e23gYPddn-GCe5_CkvGuaGf_dMcYMG25_5Drk3Pg", + "refresh_token": "44abe4de-d12d-4d28-a5a1-d18672e7163f", + "scope": "test_scp0 test_scp1", + "token_type": "Bearer" +} diff --git a/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_no_lifespan_non_augmented.json b/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_no_lifespan_non_augmented.json new file mode 100644 index 0000000..b3a9fb7 --- /dev/null +++ b/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_no_lifespan_non_augmented.json @@ -0,0 +1,8 @@ +{ + "_comment": "_exp, _iat, and expires_in do not match the token. The token claims should take priority.", + "access_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Rfa2V5cGFpcjEiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJ0ZXN0X3ByaW1hcnlfaXNzdWVyIiwic3ViIjoidGVzdF91c2VyIiwiYXVkIjoidGVzdF9hdWRpZW5jZSIsImlhdCI6MTc1OTIwNjg5MSwiZXhwIjoxNzU5MjA2OTUxLCJqdGkiOiI5ODM5MDQ4MC0wNjczLTRmMjMtODRlNi0wNTZlYzYyZDg0ODciLCJ0ZXN0X2NsYXNzIjoiVGVzdE9pZGNDcmVkZW50aWFsIiwic2NvcGUiOiJ0ZXN0X3NjcDAgdGVzdF9zY3AxIn0.rJmP7KggXzYPvyKOZvE5m4XkR4H8tKdCxEUrUpJ66xfvx6nnbieFDO2sA5GtyNwkDgLcZhHkIILSq2WvfU5Wx1koyoFvaYTVJ3AaE4B3raJmrsSEroZR0uY89hlZ2f0bH5pTExizp0qpFs7f1HfvCyTkHIeks6xXXDHRFE8nTMR23BhIpl64sfYBBuXoWvFV6ptFrVUPZItX5iSkCZx-rAAA1MsoM0yoVXpaeY6yBN3z_jhq2AjeWsDD2LEjt4KoWrEvneqUKLbotp5SLqbin0qZ9xqHtxrXO1nUlpTvMhx8byE4wlw26dr93cbggvgEbqIprLs7-Jk44gwo4ilmNA", + "id_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Rfa2V5cGFpcjEiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJ0ZXN0X3ByaW1hcnlfaXNzdWVyIiwic3ViIjoiX19zdHViX2NsaWVudF9pZF9fIiwiYXVkIjoiX19zdHViX2NsaWVudF9pZF9fIiwiaWF0IjoxNzU5MjA2ODkxLCJleHAiOjE3NTkyMDY5NTEsImp0aSI6IjU3OWFlNGYwLWI1ZTUtNDRkNS1iNGM2LWY2MTVjMzY4Mjk4ZSIsInRlc3RfY2xhc3MiOiJUZXN0T2lkY0NyZWRlbnRpYWwiLCJzY29wZSI6InRlc3Rfc2NwMCB0ZXN0X3NjcDEifQ.RF8Hb8xf13yLPVedPJe3jqgjpD2kpjHz-RpP-vusW7ofuN1MLwZMmG7wbCriJbAk7OXUavfHytgivKGkCkkepQ0fwKACKNiveVCakQiYnEXIeLS9uu7wnOrLRbzCI6j0tm5996n8Wt-YuDFjbnpj5_isfIumjvB6oKYfLdTMZShlYccHDF8YG4Ug-8gHgZQktAVKGofMwbvabzOJdpBRn8-y5HIiC362mTVglG3zhZdiBEzBQpzgyDoDKl7CBjBOC4u7IjVsRsJ1pyuGALEbTOPnz-n7B_lr_C6-yp3ERFjd83e23gYPddn-GCe5_CkvGuaGf_dMcYMG25_5Drk3Pg", + "refresh_token": "44abe4de-d12d-4d28-a5a1-d18672e7163f", + "scope": "test_scp0 test_scp1", + "token_type": "Bearer" +} diff --git a/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_with_lifespan_augmented.json b/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_with_lifespan_augmented.json new file mode 100644 index 0000000..8c24315 --- /dev/null +++ b/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_with_lifespan_augmented.json @@ -0,0 +1,11 @@ +{ + "_comment": "_exp, _iat, and expires_in do not match the token. The token claims should take priority.", + "_exp": 2345, + "_iat": 1234, + "expires_in": 3, + "access_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Rfa2V5cGFpcjEiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJ0ZXN0X3ByaW1hcnlfaXNzdWVyIiwic3ViIjoidGVzdF91c2VyIiwiYXVkIjoidGVzdF9hdWRpZW5jZSIsImlhdCI6MTc1OTIwNjg5MSwiZXhwIjoxNzU5MjA2OTUxLCJqdGkiOiI5ODM5MDQ4MC0wNjczLTRmMjMtODRlNi0wNTZlYzYyZDg0ODciLCJ0ZXN0X2NsYXNzIjoiVGVzdE9pZGNDcmVkZW50aWFsIiwic2NvcGUiOiJ0ZXN0X3NjcDAgdGVzdF9zY3AxIn0.rJmP7KggXzYPvyKOZvE5m4XkR4H8tKdCxEUrUpJ66xfvx6nnbieFDO2sA5GtyNwkDgLcZhHkIILSq2WvfU5Wx1koyoFvaYTVJ3AaE4B3raJmrsSEroZR0uY89hlZ2f0bH5pTExizp0qpFs7f1HfvCyTkHIeks6xXXDHRFE8nTMR23BhIpl64sfYBBuXoWvFV6ptFrVUPZItX5iSkCZx-rAAA1MsoM0yoVXpaeY6yBN3z_jhq2AjeWsDD2LEjt4KoWrEvneqUKLbotp5SLqbin0qZ9xqHtxrXO1nUlpTvMhx8byE4wlw26dr93cbggvgEbqIprLs7-Jk44gwo4ilmNA", + "id_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Rfa2V5cGFpcjEiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJ0ZXN0X3ByaW1hcnlfaXNzdWVyIiwic3ViIjoiX19zdHViX2NsaWVudF9pZF9fIiwiYXVkIjoiX19zdHViX2NsaWVudF9pZF9fIiwiaWF0IjoxNzU5MjA2ODkxLCJleHAiOjE3NTkyMDY5NTEsImp0aSI6IjU3OWFlNGYwLWI1ZTUtNDRkNS1iNGM2LWY2MTVjMzY4Mjk4ZSIsInRlc3RfY2xhc3MiOiJUZXN0T2lkY0NyZWRlbnRpYWwiLCJzY29wZSI6InRlc3Rfc2NwMCB0ZXN0X3NjcDEifQ.RF8Hb8xf13yLPVedPJe3jqgjpD2kpjHz-RpP-vusW7ofuN1MLwZMmG7wbCriJbAk7OXUavfHytgivKGkCkkepQ0fwKACKNiveVCakQiYnEXIeLS9uu7wnOrLRbzCI6j0tm5996n8Wt-YuDFjbnpj5_isfIumjvB6oKYfLdTMZShlYccHDF8YG4Ug-8gHgZQktAVKGofMwbvabzOJdpBRn8-y5HIiC362mTVglG3zhZdiBEzBQpzgyDoDKl7CBjBOC4u7IjVsRsJ1pyuGALEbTOPnz-n7B_lr_C6-yp3ERFjd83e23gYPddn-GCe5_CkvGuaGf_dMcYMG25_5Drk3Pg", + "refresh_token": "44abe4de-d12d-4d28-a5a1-d18672e7163f", + "scope": "test_scp0 test_scp1", + "token_type": "Bearer" +} diff --git a/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_with_lifespan_non_augmented.json b/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_with_lifespan_non_augmented.json new file mode 100644 index 0000000..0ea00e0 --- /dev/null +++ b/tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_with_lifespan_non_augmented.json @@ -0,0 +1,9 @@ +{ + "_comment": "_exp, _iat, and expires_in do not match the token. The token claims should take priority.", + "expires_in": 3, + "access_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Rfa2V5cGFpcjEiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJ0ZXN0X3ByaW1hcnlfaXNzdWVyIiwic3ViIjoidGVzdF91c2VyIiwiYXVkIjoidGVzdF9hdWRpZW5jZSIsImlhdCI6MTc1OTIwNjg5MSwiZXhwIjoxNzU5MjA2OTUxLCJqdGkiOiI5ODM5MDQ4MC0wNjczLTRmMjMtODRlNi0wNTZlYzYyZDg0ODciLCJ0ZXN0X2NsYXNzIjoiVGVzdE9pZGNDcmVkZW50aWFsIiwic2NvcGUiOiJ0ZXN0X3NjcDAgdGVzdF9zY3AxIn0.rJmP7KggXzYPvyKOZvE5m4XkR4H8tKdCxEUrUpJ66xfvx6nnbieFDO2sA5GtyNwkDgLcZhHkIILSq2WvfU5Wx1koyoFvaYTVJ3AaE4B3raJmrsSEroZR0uY89hlZ2f0bH5pTExizp0qpFs7f1HfvCyTkHIeks6xXXDHRFE8nTMR23BhIpl64sfYBBuXoWvFV6ptFrVUPZItX5iSkCZx-rAAA1MsoM0yoVXpaeY6yBN3z_jhq2AjeWsDD2LEjt4KoWrEvneqUKLbotp5SLqbin0qZ9xqHtxrXO1nUlpTvMhx8byE4wlw26dr93cbggvgEbqIprLs7-Jk44gwo4ilmNA", + "id_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Rfa2V5cGFpcjEiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJ0ZXN0X3ByaW1hcnlfaXNzdWVyIiwic3ViIjoiX19zdHViX2NsaWVudF9pZF9fIiwiYXVkIjoiX19zdHViX2NsaWVudF9pZF9fIiwiaWF0IjoxNzU5MjA2ODkxLCJleHAiOjE3NTkyMDY5NTEsImp0aSI6IjU3OWFlNGYwLWI1ZTUtNDRkNS1iNGM2LWY2MTVjMzY4Mjk4ZSIsInRlc3RfY2xhc3MiOiJUZXN0T2lkY0NyZWRlbnRpYWwiLCJzY29wZSI6InRlc3Rfc2NwMCB0ZXN0X3NjcDEifQ.RF8Hb8xf13yLPVedPJe3jqgjpD2kpjHz-RpP-vusW7ofuN1MLwZMmG7wbCriJbAk7OXUavfHytgivKGkCkkepQ0fwKACKNiveVCakQiYnEXIeLS9uu7wnOrLRbzCI6j0tm5996n8Wt-YuDFjbnpj5_isfIumjvB6oKYfLdTMZShlYccHDF8YG4Ug-8gHgZQktAVKGofMwbvabzOJdpBRn8-y5HIiC362mTVglG3zhZdiBEzBQpzgyDoDKl7CBjBOC4u7IjVsRsJ1pyuGALEbTOPnz-n7B_lr_C6-yp3ERFjd83e23gYPddn-GCe5_CkvGuaGf_dMcYMG25_5Drk3Pg", + "refresh_token": "44abe4de-d12d-4d28-a5a1-d18672e7163f", + "scope": "test_scp0 test_scp1", + "token_type": "Bearer" +} diff --git a/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_no_lifespan_augmented.json b/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_no_lifespan_augmented.json new file mode 100644 index 0000000..8891b44 --- /dev/null +++ b/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_no_lifespan_augmented.json @@ -0,0 +1,10 @@ +{ + "_comment": "Opaque token. Externally specified _exp, _iat, and expires_in is all we have to go on.", + "_iat": 1, + "_exp": 101, + "token_type": "Bearer", + "access_token": "_dummy_access_token_", + "scope": "offline_access profile openid", + "refresh_token": "_dummy_refresh_token_", + "id_token": "_dummy_id_token_" +} diff --git a/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_no_lifespan_non_augmented.json b/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_no_lifespan_non_augmented.json new file mode 100644 index 0000000..625c5fc --- /dev/null +++ b/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_no_lifespan_non_augmented.json @@ -0,0 +1,8 @@ +{ + "_comment": "Opaque token. Externally specified _exp, _iat, and expires_in is all we have to go on.", + "token_type": "Bearer", + "access_token": "_dummy_access_token_", + "scope": "offline_access profile openid", + "refresh_token": "_dummy_refresh_token_", + "id_token": "_dummy_id_token_" +} diff --git a/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_with_lifespan_augmented.json b/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_with_lifespan_augmented.json new file mode 100644 index 0000000..fb0ede4 --- /dev/null +++ b/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_with_lifespan_augmented.json @@ -0,0 +1,11 @@ +{ + "_comment": "Opaque token. Externally specified _exp, _iat, and expires_in is all we have to go on.", + "_iat": 1, + "_exp": 101, + "expires_in": 3600, + "token_type": "Bearer", + "access_token": "_dummy_access_token_", + "scope": "offline_access profile openid", + "refresh_token": "_dummy_refresh_token_", + "id_token": "_dummy_id_token_" +} diff --git a/tests/test_planet_auth/data/keys/oidc_test_credential.json b/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_with_lifespan_non_augmented.json similarity index 67% rename from tests/test_planet_auth/data/keys/oidc_test_credential.json rename to tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_with_lifespan_non_augmented.json index 45d329c..f43c2a4 100644 --- a/tests/test_planet_auth/data/keys/oidc_test_credential.json +++ b/tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_with_lifespan_non_augmented.json @@ -1,6 +1,7 @@ { - "token_type": "Bearer", + "_comment": "Opaque token. Externally specified _exp, _iat, and expires_in is all we have to go on.", "expires_in": 3600, + "token_type": "Bearer", "access_token": "_dummy_access_token_", "scope": "offline_access profile openid", "refresh_token": "_dummy_refresh_token_", 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 22dc89d..b163f2b 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 @@ -41,6 +41,14 @@ stub_authority_pub_key_file=TEST_SIGNING_PUBKEY_FILE, scopes=["offline_access", "profile", "openid", "test_scope_1", "test_scope_2"], ) +TEST_STUB_CLIENT_WITH_NON_EXPIRING_TOKENS_CONFIG = StubOidcClientConfig( + auth_server=TEST_AUTH_SERVER, + stub_authority_ttl=None, + stub_authority_access_token_audience=TEST_TOKEN_AUDENCE, + stub_authority_signing_key_file=TEST_SIGNING_KEY_FILE, + stub_authority_pub_key_file=TEST_SIGNING_PUBKEY_FILE, + scopes=["offline_access", "profile", "openid", "test_scope_1", "test_scope_2"], +) class ORAUnitTestException(Exception): @@ -60,6 +68,8 @@ def setUp(self): self.wrapped_stub_auth_client = MagicMock(wraps=self.stub_auth_client) self.refresh_failing_auth_client = RefreshFailingStubOidcAuthClient(TEST_STUB_CLIENT_CONFIG) self.wrapper_refresh_failing_auth_client = MagicMock(wraps=self.refresh_failing_auth_client) + self.non_expiring_auth_client = StubOidcAuthClient(TEST_STUB_CLIENT_WITH_NON_EXPIRING_TOKENS_CONFIG) + self.wrapped_non_expiring_auth_client = MagicMock(wraps=self.non_expiring_auth_client) self.tmp_dir = tempfile.TemporaryDirectory() self.tmp_dir_path = pathlib.Path(self.tmp_dir.name) @@ -114,16 +124,33 @@ def under_test_refresh_fails(self): credential=test_credential, auth_client=self.wrapper_refresh_failing_auth_client ) + def under_test_non_expiring_token(self): + credential_path = self.tmp_dir_path / "refreshing_oidc_authenticator_test_token__non_expiring_token.json" + test_credential = self.mock_auth_login_and_command_initialize( + credential_path=credential_path, auth_client=self.non_expiring_auth_client, remove_claims=["iat", "exp"] + ) + return RefreshingOidcTokenRequestAuthenticator( + credential=test_credential, auth_client=self.wrapped_non_expiring_auth_client + ) + @staticmethod def mock_auth_login_and_command_initialize( - credential_path, auth_client, get_access_token=True, get_id_token=True, get_refresh_token=True + credential_path, + auth_client, + get_access_token=True, + get_id_token=True, + get_refresh_token=True, + remove_claims=None, ): # pretend to bootstrap client auth configuration on disk, the way # it may be used by a user in an interactive shell: # bash$ planet auth login initial_credential = auth_client.login( - get_access_token=get_access_token, get_refresh_token=get_refresh_token, get_id_token=get_id_token + get_access_token=get_access_token, + get_refresh_token=get_refresh_token, + get_id_token=get_id_token, + remove_claims=remove_claims, ) initial_credential.set_path(credential_path) initial_credential.save() @@ -232,6 +259,39 @@ def test_happy_path_2(self, frozen_time): self.assertIsNotNone(under_test._credential.data()) self.assertEqual(1, under_test._auth_client.refresh.call_count) + @freezegun.freeze_time(as_kwarg="frozen_time") + def test_happy_path_3_non_expiring_token(self, frozen_time): + under_test = self.under_test_non_expiring_token() + + self.assertIsNone(under_test._credential.data()) + self.mock_api_call(under_test) + self.assertIsNotNone(under_test._credential.data()) + self.assertEqual(0, under_test._auth_client.refresh.call_count) + access_token_t1 = under_test._credential.access_token() + + # inside the refresh window, more access should not refresh + self.mock_api_call(under_test) + self.mock_api_call(under_test) + self.mock_api_call(under_test) + self.assertEqual(0, under_test._auth_client.refresh.call_count) + access_token_t2 = under_test._credential.access_token() + self.assertEqual(access_token_t1, access_token_t2) + + # When the token reaches the 3/4 life, the authenticator should not + # attempt a token refresh. + frozen_time.tick(((3 * TEST_TOKEN_TTL) / 4) + 2) + self.mock_api_call(under_test) + self.assertEqual(0, under_test._auth_client.refresh.call_count) + access_token_t3 = under_test._credential.access_token() + self.assertEqual(access_token_t1, access_token_t3) + + # In the distant future, we should still not refresh the token. + frozen_time.tick(TEST_TOKEN_TTL * 100) + self.mock_api_call(under_test) + self.assertEqual(0, under_test._auth_client.refresh.call_count) + access_token_t4 = under_test._credential.access_token() + self.assertEqual(access_token_t1, access_token_t4) + @freezegun.freeze_time(as_kwarg="frozen_time") def test_refresh_fails(self, frozen_time): # Refresh could fail. If it does, this should be buried and we diff --git a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_oidc_token_credential.py b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_oidc_token_credential.py index 35ed159..d80c3bb 100644 --- a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_oidc_token_credential.py +++ b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_oidc_token_credential.py @@ -12,6 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import freezegun +import pathlib +import tempfile +import time import unittest from planet_auth.oidc.oidc_credential import FileBackedOidcCredential @@ -22,7 +26,10 @@ class TestOidcCredential(unittest.TestCase): def test_asserts_valid(self): under_test = FileBackedOidcCredential( - data=None, credential_file=tdata_resource_file_path("keys/oidc_test_credential.json") + data=None, + credential_file=tdata_resource_file_path( + "keys/oidc_test_credential_opaque_tokens_with_lifespan_non_augmented.json" + ), ) under_test.load() self.assertIsNotNone(under_test.data()) @@ -35,9 +42,203 @@ def test_asserts_valid(self): def test_getters(self): under_test = FileBackedOidcCredential( - data=None, credential_file=tdata_resource_file_path("keys/oidc_test_credential.json") + data=None, + credential_file=tdata_resource_file_path( + "keys/oidc_test_credential_opaque_tokens_with_lifespan_non_augmented.json" + ), ) under_test.load() self.assertEqual("_dummy_access_token_", under_test.access_token()) self.assertEqual("_dummy_refresh_token_", under_test.refresh_token()) self.assertEqual("_dummy_id_token_", under_test.id_token()) + + def test_load_credential_file__jwt_tokens__augmented__with_lifespan(self): + under_test = FileBackedOidcCredential( + data=None, + credential_file=tdata_resource_file_path( + "keys/oidc_test_credential_jwt_tokens_with_lifespan_augmented.json" + ), + ) + self.assertEqual(1759206891, under_test.issued_time()) + self.assertEqual(1759206951, under_test.expiry_time()) + self.assertTrue(under_test.is_expiring()) + self.assertFalse(under_test.is_non_expiring()) + + def test_load_credential_file__jwt_tokens__non_augmented__with_lifespan(self): + # Older versions of the code may have saved files without our computed fields. + # Check that this behaves as expected. + under_test = FileBackedOidcCredential( + data=None, + credential_file=tdata_resource_file_path( + "keys/oidc_test_credential_jwt_tokens_with_lifespan_non_augmented.json" + ), + ) + self.assertEqual(1759206891, under_test.issued_time()) + self.assertEqual(1759206951, under_test.expiry_time()) + self.assertTrue(under_test.is_expiring()) + self.assertFalse(under_test.is_non_expiring()) + + def test_load_credential_file__jwt_tokens__augmented__without_lifespan(self): + under_test = FileBackedOidcCredential( + data=None, + credential_file=tdata_resource_file_path( + "keys/oidc_test_credential_jwt_tokens_no_lifespan_augmented.json" + ), + ) + self.assertEqual(1759206891, under_test.issued_time()) + self.assertEqual(1759206951, under_test.expiry_time()) + self.assertTrue(under_test.is_expiring()) + self.assertFalse(under_test.is_non_expiring()) + + def test_load_credential_file__jwt_tokens__non_augmented__without_lifespan(self): + # Older versions of the code may have saved files without our computed fields. + # Check that this behaves as expected. + under_test = FileBackedOidcCredential( + data=None, + credential_file=tdata_resource_file_path( + "keys/oidc_test_credential_jwt_tokens_no_lifespan_non_augmented.json" + ), + ) + self.assertEqual(1759206891, under_test.issued_time()) + self.assertEqual(1759206951, under_test.expiry_time()) + self.assertTrue(under_test.is_expiring()) + self.assertFalse(under_test.is_non_expiring()) + + def test_load_credential_file__opaque_tokens__augmented__with_lifespan(self): + under_test = FileBackedOidcCredential( + data=None, + credential_file=tdata_resource_file_path( + "keys/oidc_test_credential_opaque_tokens_with_lifespan_augmented.json" + ), + ) + self.assertEqual(1, under_test.issued_time()) + self.assertEqual(101, under_test.expiry_time()) + self.assertTrue(under_test.is_expiring()) + self.assertFalse(under_test.is_non_expiring()) + + @freezegun.freeze_time(as_kwarg="frozen_time") + def test_load_credential_file__opaque_tokens__non_augmented__with_lifespan(self, frozen_time): + # Older versions of the code may have saved files without our computed fields. + # Check that this behaves as expected. + t0 = int(time.time()) + under_test = FileBackedOidcCredential( + data=None, + credential_file=tdata_resource_file_path( + "keys/oidc_test_credential_opaque_tokens_with_lifespan_non_augmented.json" + ), + ) + self.assertEqual(t0, under_test.issued_time()) + self.assertEqual(t0 + 3600, under_test.expiry_time()) + self.assertTrue(under_test.is_expiring()) + self.assertFalse(under_test.is_non_expiring()) + + def test_load_credential_file__opaque_tokens__augmented__without_lifespan(self): + under_test = FileBackedOidcCredential( + data=None, + credential_file=tdata_resource_file_path( + "keys/oidc_test_credential_opaque_tokens_no_lifespan_augmented.json" + ), + ) + self.assertEqual(1, under_test.issued_time()) + self.assertEqual(101, under_test.expiry_time()) + self.assertTrue(under_test.is_expiring()) + self.assertFalse(under_test.is_non_expiring()) + + @freezegun.freeze_time(as_kwarg="frozen_time") + def test_load_credential_file__opaque_tokens__non_augmented__without_lifespan(self, frozen_time): + # Older versions of the code may have saved files without our computed fields. + # Check that this behaves as expected. + t0 = int(time.time()) + under_test = FileBackedOidcCredential( + data=None, + credential_file=tdata_resource_file_path( + "keys/oidc_test_credential_opaque_tokens_no_lifespan_non_augmented.json" + ), + ) + self.assertEqual(t0, under_test.issued_time()) + self.assertIsNone(under_test.expiry_time()) + self.assertFalse(under_test.is_expiring()) + self.assertTrue(under_test.is_non_expiring()) + + +class TestBaseCredential(unittest.TestCase): + # Test the Credential base class functions using the OidcCredential derived class. + # The primary base class functionality under test is the IAT and EXP times + # and their persistence behavior. + + @freezegun.freeze_time(as_kwarg="frozen_time") + def test_save_persists_computed_values_with_lifespan(self, frozen_time): + tmp_dir = tempfile.TemporaryDirectory() + test_path = pathlib.Path(tmp_dir.name) / "test_save_computed_data.json" + t0 = int(time.time()) + under_test_1 = FileBackedOidcCredential( + data={ + "expires_in": 1000, + "token_type": "Bearer", + "scope": "offline_access profile openid", + "access_token": "_dummy_access_token_", + "refresh_token": "_dummy_refresh_token_", + "id_token": "_dummy_id_token_", + } + ) + self.assertEqual(t0, under_test_1.issued_time()) + self.assertEqual(t0 + 1000, under_test_1.expiry_time()) + self.assertTrue(under_test_1.is_expiring()) + self.assertFalse(under_test_1.is_non_expiring()) + self.assertFalse(under_test_1.is_expired()) + self.assertTrue(under_test_1.is_not_expired()) + under_test_1.set_path(test_path) + under_test_1.save() + + frozen_time.tick(100) + + under_test_2 = FileBackedOidcCredential(data=None, credential_file=test_path) + self.assertEqual(t0, under_test_2.issued_time()) + self.assertEqual(t0 + 1000, under_test_2.expiry_time()) + self.assertTrue(under_test_2.is_expiring()) + self.assertFalse(under_test_2.is_non_expiring()) + self.assertFalse(under_test_2.is_expired()) + self.assertTrue(under_test_2.is_not_expired()) + + frozen_time.tick(1000) + + self.assertTrue(under_test_2.is_expired()) + self.assertFalse(under_test_2.is_not_expired()) + + @freezegun.freeze_time(as_kwarg="frozen_time") + def test_save_persists_computed_values_without_lifespan(self, frozen_time): + tmp_dir = tempfile.TemporaryDirectory() + test_path = pathlib.Path(tmp_dir.name) / "test_save_computed_data.json" + t0 = int(time.time()) + under_test_1 = FileBackedOidcCredential( + data={ + "token_type": "Bearer", + "scope": "offline_access profile openid", + "access_token": "_dummy_access_token_", + "refresh_token": "_dummy_refresh_token_", + "id_token": "_dummy_id_token_", + } + ) + self.assertEqual(t0, under_test_1.issued_time()) + self.assertIsNone(under_test_1.expiry_time()) + self.assertFalse(under_test_1.is_expiring()) + self.assertTrue(under_test_1.is_non_expiring()) + self.assertFalse(under_test_1.is_expired()) + self.assertTrue(under_test_1.is_not_expired()) + under_test_1.set_path(test_path) + under_test_1.save() + + frozen_time.tick(100) + + under_test_2 = FileBackedOidcCredential(data=None, credential_file=test_path) + self.assertEqual(t0, under_test_2.issued_time()) + self.assertIsNone(under_test_2.expiry_time()) + self.assertFalse(under_test_2.is_expiring()) + self.assertTrue(under_test_2.is_non_expiring()) + self.assertFalse(under_test_2.is_expired()) + self.assertTrue(under_test_2.is_not_expired()) + + frozen_time.tick(1000) + + self.assertFalse(under_test_2.is_expired()) + self.assertTrue(under_test_2.is_not_expired()) diff --git a/tests/test_planet_auth/unit/auth/test_auth.py b/tests/test_planet_auth/unit/auth/test_auth.py index f7cedf5..62dfcfa 100644 --- a/tests/test_planet_auth/unit/auth/test_auth.py +++ b/tests/test_planet_auth/unit/auth/test_auth.py @@ -13,10 +13,14 @@ # limitations under the License. import pathlib +import time import unittest +from typing import List, Optional +from unittest.mock import MagicMock -from planet_auth.auth import Auth -from planet_auth.auth_client import AuthClientException, AuthClientConfig +from planet_auth.auth import Auth, Credential, CredentialRequestAuthenticator, AuthClientContextException +from planet_auth.auth_client import AuthClient, AuthClientConfig, AuthClientException +from planet_auth.auth_exception import AuthException from planet_auth.static_api_key.auth_client import StaticApiKeyAuthClient from planet_auth.static_api_key.request_authenticator import FileBackedApiKeyRequestAuthenticator from planet_auth.none.noop_auth import NoOpAuthClient @@ -66,3 +70,317 @@ def test_initialize_from_config_invalid_client(self): def test_initialize_from_config_none_client(self): with self.assertRaises(AuthClientException): Auth.initialize_from_config_dict(client_config=None, token_file="/dev/null/token.json") + + +class FakeCredential(Credential): + """Fake credential with controllable expiration state""" + + def __init__(self, token_ttl=None, is_expired=False, credential_file=None): + super().__init__( + data={ + "test_token_ttl": token_ttl, + "test_token_is_expired": is_expired, + }, + file_path=credential_file, + ) + self._augment_data() + + def _augment_data(self): + now = int(time.time()) + if self._data: + if self._data.get("test_token_ttl") is not None: + if self._data.get("test_token_is_expired"): + self._data["_iat"] = now - (2 * self._data["test_token_ttl"]) + self._data["_exp"] = now - self._data["test_token_ttl"] + else: + self._data["_iat"] = now - (self._data["test_token_ttl"] // 2) + self._data["_exp"] = now + (self._data["test_token_ttl"] // 2) + else: + self._data["_iat"] = now - 100 + self._data["_exp"] = None + else: + self._data["_iat"] = now + self._data["_exp"] = None + + def set_data(self, data, copy_data: bool = True): + super().set_data(data, copy_data) + self._augment_data() + + +class FakeAuthClientConfig(AuthClientConfig): + """Fake auth client config for testing""" + + def __init__( + self, + token_ttl=None, + can_login_unattended=False, + refresh_raises: Exception = None, + refresh_returns_credential: bool = True, + login_raises: Exception = None, + login_returns_credential: bool = True, + **kwargs, + ): + super().__init__(file_path=None, **kwargs) + self.token_ttl = token_ttl + self.can_login_unattended = can_login_unattended + self.refresh_raises = refresh_raises + self.refresh_returns_credential = refresh_returns_credential + self.login_raises = login_raises + self.login_returns_credential = login_returns_credential + + @classmethod + def meta(cls): + return { + "client_type": "fake_test_client", + "auth_client_class": "FakeAuthClient", + "display_name": "Fake Test Client", + "description": "Fake auth client for unit testing", + } + + +class FakeAuthClient(AuthClient): + """Fake auth client with controllable login behavior""" + + def __init__(self, auth_client_config: FakeAuthClientConfig): + super().__init__(auth_client_config) + self._fake_client_config = auth_client_config + + def can_login_unattended(self): + return self._fake_client_config.can_login_unattended + + def login(self, allow_open_browser=False, allow_tty_prompt=False, **kwargs) -> Credential: + if self._fake_client_config.login_raises: + raise self._fake_client_config.login_raises + if not self._fake_client_config.login_returns_credential: + # This is bad behavior. + # Clients are supposed to raise if they cannot login. + return None + return FakeCredential(self._fake_client_config.token_ttl) + + def refresh(self, refresh_token: str, requested_scopes: List[str]) -> Credential: + if self._fake_client_config.refresh_raises: + raise self._fake_client_config.refresh_raises + if not self._fake_client_config.refresh_returns_credential: + # This is bad behavior. + # Clients are supposed to raise if they cannot refresh. + return None + return FakeCredential(self._fake_client_config.token_ttl) + + def default_request_authenticator(self, credential): + """Override required abstract method""" + raise NotImplementedError("Not needed for these tests") + + +class FakeRequestAuthenticator(CredentialRequestAuthenticator): + """Fake request authenticator with controllable state""" + + def __init__(self, credential: FakeCredential, auth_client: FakeAuthClient = None, **kwargs): + super().__init__(credential=credential, **kwargs) + self._auth_client = auth_client + + def pre_request_hook(self): + pass + + def _refresh_needed(self): + if not self._credential: + return True + return self._credential.is_expired() + + def _refresh(self): + new_credentials = self._auth_client.refresh(refresh_token="dummy", requested_scopes=["test_scope"]) + new_credentials.set_path(self._credential.path()) + new_credentials.set_storage_provider(self._credential.storage_provider()) + new_credentials.save() + + # self.update_credential(new_credential=new_credentials) + self._credential = new_credentials + + def credential(self, refresh_if_needed: bool = False) -> Optional[Credential]: + if refresh_if_needed: + if self._refresh_needed(): + self._refresh() + return super().credential() + + +class TestLoginException(AuthException): + def __init__(self, **kwargs): + super().__init__(**kwargs) + + +class TestRefreshException(AuthException): + def __init__(self, **kwargs): + super().__init__(**kwargs) + + +class AuthTestEnsureAuthenticatorIsReady(unittest.TestCase): + """Tests for Auth.ensure_request_authenticator_is_ready()""" + + def _create_auth_with_state( + self, + has_credential=False, + is_expired=False, + token_ttl=200, + can_login_unattended=False, + refresh_raises: Exception = None, + refresh_returns_credential: bool = True, + login_raises: Exception = None, + login_returns_credential: bool = True, + ): + if has_credential: + credential = MagicMock(wraps=FakeCredential(token_ttl=token_ttl, is_expired=is_expired)) + else: + credential = None + + auth_client = MagicMock( + wraps=FakeAuthClient( + FakeAuthClientConfig( + can_login_unattended=can_login_unattended, + token_ttl=token_ttl, + refresh_raises=refresh_raises, + refresh_returns_credential=refresh_returns_credential, + login_raises=login_raises, + login_returns_credential=login_returns_credential, + ) + ) + ) + + request_authenticator = MagicMock( + wraps=FakeRequestAuthenticator( + credential=credential, + auth_client=auth_client, + ) + ) + + auth = MagicMock( + wraps=Auth( + auth_client=auth_client, + request_authenticator=request_authenticator, + token_file_path=None, + profile_name=None, + ) + ) + + return auth + + # Case #1: Already has valid (non-expired) credential + def test_case1_has_valid_credential_returns_immediately(self): + """When credential exists and is not expired, should return without any action""" + under_test = self._create_auth_with_state(has_credential=True, is_expired=False) + + under_test.ensure_request_authenticator_is_ready() + + self.assertEqual(0, under_test.login.call_count) + self.assertEqual(0, under_test.auth_client().login.call_count) + self.assertEqual(0, under_test.auth_client().refresh.call_count) + + # Case #2: No credential but can login unattended + def test_case2_no_credential_can_login_unattended(self): + """When no credential but can login unattended, should return without login""" + under_test = self._create_auth_with_state(has_credential=False, can_login_unattended=True) + + under_test.ensure_request_authenticator_is_ready() + + self.assertEqual(0, under_test.login.call_count) + self.assertEqual(0, under_test.auth_client().login.call_count) + self.assertEqual(0, under_test.auth_client().refresh.call_count) + + # Case #3a: Has expired credential and refresh succeeds + def test_case3a_expired_credential_refresh_succeeds(self): + """When credential is expired but refresh succeeds, should refresh without login""" + under_test = self._create_auth_with_state( + has_credential=True, + is_expired=True, + ) + + under_test.ensure_request_authenticator_is_ready() + + self.assertEqual(0, under_test.login.call_count) + self.assertEqual(0, under_test.auth_client().login.call_count) + self.assertEqual(1, under_test.auth_client().refresh.call_count) + + # Case #3b: Has expired credential and refresh fails + def test_case3b_expired_credential_refresh_fails_then_login(self): + """When credential is expired and refresh fails, should fall through to login""" + under_test = self._create_auth_with_state( + has_credential=True, + is_expired=True, + refresh_raises=Exception("Test Exception - Refresh failed"), + ) + + under_test.ensure_request_authenticator_is_ready() + + self.assertEqual(0, under_test.login.call_count) + self.assertEqual(1, under_test.auth_client().login.call_count) + self.assertEqual(1, under_test.auth_client().refresh.call_count) + + # Case #3c: Has expired credential and refresh and login fail. + def test_case3c_expired_credential_refresh_fails_and_login_fails(self): + """When credential is expired and refresh fails, should fall through to login""" + test_login_exception = TestLoginException(message="Test Exception - Login failed (3c)") + test_refresh_exception = TestRefreshException(message="Test Exception - Refresh failed (3c)") + under_test = self._create_auth_with_state( + has_credential=True, + is_expired=True, + can_login_unattended=False, + refresh_raises=test_refresh_exception, + login_raises=test_login_exception, + ) + + with self.assertRaises(TestLoginException) as raised: + under_test.ensure_request_authenticator_is_ready() + + self.assertEqual(test_login_exception, raised.exception) + self.assertEqual(0, under_test.login.call_count) + self.assertEqual(1, under_test.auth_client().login.call_count) + self.assertEqual(1, under_test.auth_client().refresh.call_count) + + # Case #4a: No credential and requires interactive login (succeeds) + def test_case4a_no_credential_interactive_login_succeeds(self): + """When no credential and cannot login unattended, should perform interactive login""" + under_test = self._create_auth_with_state( + has_credential=False, + can_login_unattended=False, + ) + + under_test.ensure_request_authenticator_is_ready() + + self.assertEqual(0, under_test.login.call_count) + self.assertEqual(1, under_test.auth_client().login.call_count) + self.assertEqual(0, under_test.auth_client().refresh.call_count) + + # Case #4b: Interactive login raises an exception. + def test_case4b_login_fails_with_raise(self): + """login raises exception that we should see propagated""" + test_exception = TestLoginException(message="Test Exception - Login failed (4b)") + under_test = self._create_auth_with_state( + has_credential=False, can_login_unattended=False, login_raises=test_exception + ) + + with self.assertRaises(TestLoginException) as raised: + under_test.ensure_request_authenticator_is_ready() + + self.assertEqual(test_exception, raised.exception) + self.assertEqual(0, under_test.login.call_count) + self.assertEqual(1, under_test.auth_client().login.call_count) + self.assertEqual(0, under_test.auth_client().refresh.call_count) + + # Case #4c: Interactive login returns None + def test_case4b_login_returns_none(self): + """When login returns None, should raise AuthClientContextException""" + under_test = self._create_auth_with_state( + has_credential=False, + can_login_unattended=False, + login_returns_credential=False, + ) + + expected_exception = AuthClientContextException( + message="Unknown login failure. No credentials and no error returned." + ) + + with self.assertRaises(AuthClientContextException) as raised: + under_test.ensure_request_authenticator_is_ready() + + self.assertEqual(str(expected_exception), str(raised.exception)) + self.assertEqual(0, under_test.login.call_count) + self.assertEqual(1, under_test.auth_client().login.call_count) + self.assertEqual(0, under_test.auth_client().refresh.call_count) diff --git a/version.txt b/version.txt index ccbccc3..276cbf9 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -2.2.0 +2.3.0