From 57cc3c863dd2b10cf4134baf3002b099755e8960 Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Thu, 4 Sep 2025 14:05:49 -0700 Subject: [PATCH 01/33] Drop python 3.9 --- .github/workflows/test.yml | 2 +- docs/changelog.md | 3 +++ noxfile.py | 2 +- pyproject.toml | 2 +- version.txt | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 827ea21..6a4f434 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -33,7 +33,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ['3.9', '3.10', '3.11', '3.12', '3.13'] + python-version: ['3.10', '3.11', '3.12', '3.13', '3.14'] steps: - name: Checkout code uses: actions/checkout@v4 diff --git a/docs/changelog.md b/docs/changelog.md index 77f72e6..eee59eb 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,8 @@ # Changelog +## 2.2.0 - 2025-10-01 +- Update supported python versions + ## 2.1.1 - 2025-08-11 - Add py.typed to all top level packages diff --git a/noxfile.py b/noxfile.py index c97f384..9514e76 100644 --- a/noxfile.py +++ b/noxfile.py @@ -21,7 +21,7 @@ ] _DEFAULT_PYTHON = "3.13" -_ALL_PYTHON = ["3.9", "3.10", "3.11", "3.12", "3.13"] +_ALL_PYTHON = ["3.10", "3.11", "3.12", "3.13", "3.14"] @nox.session(python=_ALL_PYTHON) diff --git a/pyproject.toml b/pyproject.toml index 1b56d80..0799bc1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ ## ########################################################################### [project] name = "planet-auth" -requires-python = ">=3.9" +requires-python = ">=3.10" dynamic = ["version"] # version = "X.X.X" description = "Planet Auth Utility Code" diff --git a/version.txt b/version.txt index 3e3c2f1..ccbccc3 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -2.1.1 +2.2.0 From f54f5d70991e38aad35bbbccb5c3650c59e4cc5d Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Thu, 4 Sep 2025 14:09:23 -0700 Subject: [PATCH 02/33] Add support up through 3.15. --- .github/workflows/test.yml | 2 +- docs/changelog.md | 5 +++-- noxfile.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6a4f434..9f71293 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -33,7 +33,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ['3.10', '3.11', '3.12', '3.13', '3.14'] + python-version: ['3.10', '3.11', '3.12', '3.13', '3.14', '3.15'] steps: - name: Checkout code uses: actions/checkout@v4 diff --git a/docs/changelog.md b/docs/changelog.md index eee59eb..9108831 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,7 +1,8 @@ # Changelog -## 2.2.0 - 2025-10-01 -- Update supported python versions +## 2.2.0 - 2025-10-02 +- Update supported python versions. + Support for 3.9 dropped. Support for 3.15 added. ## 2.1.1 - 2025-08-11 - Add py.typed to all top level packages diff --git a/noxfile.py b/noxfile.py index 9514e76..74cc822 100644 --- a/noxfile.py +++ b/noxfile.py @@ -21,7 +21,7 @@ ] _DEFAULT_PYTHON = "3.13" -_ALL_PYTHON = ["3.10", "3.11", "3.12", "3.13", "3.14"] +_ALL_PYTHON = ["3.10", "3.11", "3.12", "3.13", "3.14", "3.15"] @nox.session(python=_ALL_PYTHON) From 000cd09aaa71d187d43665dbdcef95f43a83d45d Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Mon, 29 Sep 2025 08:24:20 -0700 Subject: [PATCH 03/33] WIP --- src/planet_auth/auth.py | 52 +++++++++++++ src/planet_auth/oidc/oidc_credential.py | 75 +++++++++++++++++++ src/planet_auth/oidc/request_authenticator.py | 38 ++++------ src/planet_auth/oidc/token_validator.py | 10 ++- src/planet_auth/request_authenticator.py | 1 + .../auth_clients/oidc/test_token_validator.py | 44 +++++++++-- 6 files changed, 191 insertions(+), 29 deletions(-) diff --git a/src/planet_auth/auth.py b/src/planet_auth/auth.py index 85d3ecf..345986c 100644 --- a/src/planet_auth/auth.py +++ b/src/planet_auth/auth.py @@ -113,9 +113,61 @@ 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 draft_insure_ready(self, allow_open_browser: Optional[bool] = False, allow_tty_prompt: Optional[bool] = False): + """ + Do everything necessary to insure the request authenticator is ready for use, + while still biasing towards not making unnecessary network requests + or prompts for user interaction. + + This can be more complex than it sounds, given the variety of capabilities + of authentication client types. + + There still may be conditions where we believe we are + ready, but requests will 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, + requests could fail for completely valid credentials if service + policy denies the request to the authenticated client. + """ + # TODO: be careful about when we trigger calls. + # Should we have a broader "is_ready()" or "insure_ready(allowed_tty, allowed_browser)"? + # making ourselves ready is more than checking if the authenticator is ready. + # if the authenticator isn't ready, making it ready may require the auth client. + + 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_not_expired() -> bool: + # Does not do any JIT check + return False + + if has_credential() and is_not_expired(): + return True + + if can_obtain_credentials_unattended(): + # Should we fetch one? we do not because the bias is towards JIT operations. + # This so programs can initialize and not fail unless the credential is actually needed. + # TODO: make caller configurable. "do_credential_prefetch" arg? + return True + + # TODO: attempt interaction-free refresh ... if has_refresh() ... or try refresh() + # TODO: check how we implemented "refresh" vs "login" for flow that do not require + # user interaction. We should make them the same. + + # TODO: attempt user-interactive login + return False + def login(self, **kwargs) -> Credential: """ Perform a login with the configured auth client. diff --git a/src/planet_auth/oidc/oidc_credential.py b/src/planet_auth/oidc/oidc_credential.py index ce5e43c..b1191d1 100644 --- a/src/planet_auth/oidc/oidc_credential.py +++ b/src/planet_auth/oidc/oidc_credential.py @@ -12,19 +12,26 @@ # See the License for the specific language governing permissions and # limitations under the License. +import jwt +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 +43,68 @@ 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): + # TODO - I am depending on saving an explicit None for "never expires". Check whether I allow this. + # TODO - Verify behavior when new fields do not exist. + # TODO - Check the none save-reload behavior + # TODO - unit test for all flavors of 0/null exp / iat in token data. + # 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. + if not self._data: + return + + if self._data.get("_iat") and self._data.get("_exp"): + return + + access_token_str = self.access_token() + if not access_token_str: + return + + try: + (_, hazmat_body, _) = TokenValidator.hazmat_unverified_decode(access_token_str) + except InvalidArgumentException as e: + # Proceed as if it's not a JWT. + hazmat_body = None + + if hazmat_body: + # For JWTs, inspect the token to get values + iat = hazmat_body.get("iat", None) + exp = hazmat_body.get("exp", None) + else: + # "expires_in" is only recommended by the RFC, not required. + # Without it, we can only guess when opaque tokens expire. + # Our guess is that it never expires. + now = int(time.time()) + iat = self._data.get("_iat", now) + lifespan = self._data.get("expires_in", 0) + if lifespan > 0: + exp = iat + lifespan + else: + exp = None + + # Edge case - It's possible that a JWT ID token has an expiration time + # that is different from the access token. + self._data["_iat"] = iat + self._data["_exp"] = exp + if exp is None: + self._data["_expiring"] = True + else: + self._data["_expiring"] = False + + def set_data(self, data): + """ + 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) + self._augment_rfc6749_data() + def access_token(self): """ Get the current access token. @@ -53,3 +122,9 @@ def refresh_token(self): Get the current refresh token. """ return self.lazy_get("refresh_token") + + def expiry_time(self): + return self.lazy_get("_exp") + + def issued_time(self): + return self.lazy_get("_iat") diff --git a/src/planet_auth/oidc/request_authenticator.py b/src/planet_auth/oidc/request_authenticator.py index 3ab06a2..44552fc 100644 --- a/src/planet_auth/oidc/request_authenticator.py +++ b/src/planet_auth/oidc/request_authenticator.py @@ -53,29 +53,23 @@ 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._token_body = self._credential.access_token() + # TODO - Continuity - Old token files won't have the augmented data, but they will expire. + # Since they are JWTs, will it just work? + # TODO - should treat the expiry of the access and ID token separately? + # TODO - handle non-expiring tokens + # TODO - Test all flavors of missing times: + # exp: None, iat: Any - Never expires + # exp: time, iat: None - Expires at time. Refresh per fallback grace period. + # exp: time: iat: time - Expires at time. Refresh at the 3/4 life. + iat = self._credential.issued_time() or 0 + exp = self._credential.expiry_time() or 0 self._refresh_at = int(iat + (3 * (exp - iat) / 4)) - self._token_body = access_token_str def _refresh(self): if self._auth_client: @@ -91,7 +85,7 @@ def _refresh(self): # self.update_credential(new_credential=new_credentials) self._credential = new_credentials - self._load() + self._load_and_prime() def _refresh_if_needed(self): # Reload the file before refreshing. Another process might @@ -110,7 +104,7 @@ def _refresh_if_needed(self): now = int(time.time()) if now > self._refresh_at: 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)}" @@ -158,7 +152,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 +195,4 @@ def _refresh(self): # self.update_credential(new_credential=new_credentials) self._credential = new_credentials - self._load() + self._load_and_prime() diff --git a/src/planet_auth/oidc/token_validator.py b/src/planet_auth/oidc/token_validator.py index 632bcee..4903825 100644 --- a/src/planet_auth/oidc/token_validator.py +++ b/src/planet_auth/oidc/token_validator.py @@ -257,9 +257,15 @@ def validate_token( return validated_claims @staticmethod + @InvalidArgumentException.recast(jwt.exceptions.DecodeError) def hazmat_unverified_decode(token_str): - # WARNING: Treat unverified token claims like toxic waste. - # Nothing can be trusted until the token is verified. + """ + Decide a JWT without verifying the signature or any claims. + + !!! Warning + Treat unverified token claims with extreme caution. + Nothing can be trusted until the token is verified. + """ unverified_complete = jwt.decode_complete(token_str, options={"verify_signature": False}) # nosemgrep return unverified_complete["header"], unverified_complete["payload"], unverified_complete["signature"] diff --git a/src/planet_auth/request_authenticator.py b/src/planet_auth/request_authenticator.py index d7bdbf6..3a0d57c 100644 --- a/src/planet_auth/request_authenticator.py +++ b/src/planet_auth/request_authenticator.py @@ -148,6 +148,7 @@ def is_initialized(self) -> bool: is checked for the existence of a credential, but it will not be loaded from storage at this time, preserving JIT loading behavior. """ + # TODO: Rename to "has_credential()"? if self._credential: return bool(self._credential.data()) or self._credential.is_persisted_to_storage() return False diff --git a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py index d67881d..1543ddb 100644 --- a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py +++ b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py @@ -104,6 +104,7 @@ def test_valid_access_token(self): access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) + validated_claims = under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, @@ -111,6 +112,11 @@ def test_valid_access_token(self): ) self.assertEqual(TEST_TOKEN_USER, validated_claims["sub"]) + u_header, u_body, _ = TokenValidator.hazmat_unverified_decode(access_token) + self.assertDictEqual(u_header, {"alg": "RS256", "kid": "test_keypair1", "typ": "JWT"}) + self.assertEqual(u_body.get("aud"), "test_token_audience_for_keypair1") + self.assertEqual(u_body.get("sub"), "unit_test_user") + def test_empty_access_token(self): # QE TC3, TC5 under_test = self.under_test_1 @@ -126,26 +132,36 @@ def test_empty_access_token(self): issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode("") + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode(None) def test_malformed_token_1(self): # QE TC6 - just some random garbage. under_test = self.under_test_1 + test_token = secrets.token_bytes(2048) with self.assertRaises(InvalidTokenException): under_test.validate_token( - token_str=secrets.token_bytes(2048), + token_str=test_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode(test_token) def test_malformed_token_2(self): # QE TC6 - just some random garbage, but URL safe. under_test = self.under_test_1 + test_token = secrets.token_urlsafe(2048) with self.assertRaises(InvalidTokenException): under_test.validate_token( - token_str=secrets.token_urlsafe(2048), + token_str=test_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode(test_token) def test_malformed_token_3(self): # QE TC6 - random garbage, but has JWT three dot structure. @@ -162,11 +178,14 @@ def test_malformed_token_3(self): issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode(fake_jwt) def test_malformed_token_4(self): # QE TC6 - random garbage, but looks more JWT like with encoded json. def _fake_token(header, body): - return "{}.{}.{}".format( + fake_sig_bytes = secrets.token_bytes(256) + fake_jwt = "{}.{}.{}".format( str( jwt.utils.base64url_encode( bytes( @@ -180,11 +199,12 @@ def _fake_token(header, body): jwt.utils.base64url_encode(bytes(json.dumps(body), "utf-8")), encoding="utf-8", ), - str(jwt.utils.base64url_encode(secrets.token_bytes(256)), encoding="utf-8"), + str(jwt.utils.base64url_encode(fake_sig_bytes), encoding="utf-8"), ) + return fake_jwt, fake_sig_bytes under_test = self.under_test_1 - fake_jwt = _fake_token( + fake_jwt, fake_sig_bytes = _fake_token( { "alg": self.token_builder_1.signing_key_algorithm, "kid": self.token_builder_1.signing_key_id, @@ -199,6 +219,13 @@ def _fake_token(header, body): audience=self.token_builder_1.audience, ) + # This is good enough for an unverified decode. It's still invalid, which is + # why you are careful with unverified decoded data. + u_header, u_body, u_sig = TokenValidator.hazmat_unverified_decode(fake_jwt) + self.assertDictEqual(u_header, {"alg": "RS256", "kid": "test_keypair1"}) + self.assertDictEqual(u_body, {"fake_claim": "test claim value"}) + self.assertEqual(u_sig, fake_sig_bytes) + def test_malformed_token_missing_issuer(self): # QE TC14 under_test = self.under_test_1 @@ -287,6 +314,11 @@ def _bad_token(header, body): audience=self.token_builder_1.audience, ) + u_header, u_body, u_sig = TokenValidator.hazmat_unverified_decode(bad_jwt) + self.assertIsNotNone(u_header) + self.assertIsNotNone(u_body) + self.assertEqual(u_sig, b"") + def test_missing_signature_2(self): # QE TC11 - JWT without a signature, and with no second "." def _bad_token(header, body): @@ -321,6 +353,8 @@ def _bad_token(header, body): issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode(bad_jwt) def test_empty_issuer_arg(self): under_test = self.under_test_1 From 7f9417097ef1ac164b8eb36060c97dbbea436139 Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Wed, 1 Oct 2025 10:36:22 -0700 Subject: [PATCH 04/33] WIP --- .gitignore | 1 + src/planet_auth/oidc/oidc_credential.py | 65 +++---- src/planet_auth/oidc/request_authenticator.py | 1 - src/planet_auth/storage_utils.py | 11 +- ...tial_jwt_tokens_no_lifespan_augmented.json | 10 + ..._jwt_tokens_no_lifespan_non_augmented.json | 8 + ...al_jwt_tokens_with_lifespan_augmented.json | 11 ++ ...wt_tokens_with_lifespan_non_augmented.json | 9 + ...l_opaque_tokens_no_lifespan_augmented.json | 10 + ...aque_tokens_no_lifespan_non_augmented.json | 8 + ...opaque_tokens_with_lifespan_augmented.json | 11 ++ ...e_tokens_with_lifespan_non_augmented.json} | 3 +- .../oidc/test_oidc_token_credential.py | 173 +++++++++++++++++- 13 files changed, 281 insertions(+), 40 deletions(-) create mode 100644 tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_no_lifespan_augmented.json create mode 100644 tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_no_lifespan_non_augmented.json create mode 100644 tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_with_lifespan_augmented.json create mode 100644 tests/test_planet_auth/data/keys/oidc_test_credential_jwt_tokens_with_lifespan_non_augmented.json create mode 100644 tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_no_lifespan_augmented.json create mode 100644 tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_no_lifespan_non_augmented.json create mode 100644 tests/test_planet_auth/data/keys/oidc_test_credential_opaque_tokens_with_lifespan_augmented.json rename tests/test_planet_auth/data/keys/{oidc_test_credential.json => oidc_test_credential_opaque_tokens_with_lifespan_non_augmented.json} (67%) diff --git a/.gitignore b/.gitignore index d80596f..7829556 100644 --- a/.gitignore +++ b/.gitignore @@ -16,4 +16,5 @@ site *.venv venv* version-with-buildnum.txt +.vscode __pycache__ diff --git a/src/planet_auth/oidc/oidc_credential.py b/src/planet_auth/oidc/oidc_credential.py index b1191d1..57464bc 100644 --- a/src/planet_auth/oidc/oidc_credential.py +++ b/src/planet_auth/oidc/oidc_credential.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 Optional @@ -59,50 +58,48 @@ def _augment_rfc6749_data(self): if not self._data: return - if self._data.get("_iat") and self._data.get("_exp"): - return - access_token_str = self.access_token() if not access_token_str: return try: - (_, hazmat_body, _) = TokenValidator.hazmat_unverified_decode(access_token_str) - except InvalidArgumentException as e: + (_, jwt_hazmat_body, _) = TokenValidator.hazmat_unverified_decode(access_token_str) + except InvalidArgumentException: # Proceed as if it's not a JWT. - hazmat_body = None - - if hazmat_body: - # For JWTs, inspect the token to get values - iat = hazmat_body.get("iat", None) - exp = hazmat_body.get("exp", None) + 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: - # "expires_in" is only recommended by the RFC, not required. - # Without it, we can only guess when opaque tokens expire. - # Our guess is that it never expires. - now = int(time.time()) - iat = self._data.get("_iat", now) - lifespan = self._data.get("expires_in", 0) - if lifespan > 0: - exp = iat + lifespan - else: - exp = None + _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 # Edge case - It's possible that a JWT ID token has an expiration time - # that is different from the access token. - self._data["_iat"] = iat - self._data["_exp"] = exp - if exp is None: - self._data["_expiring"] = True - else: - self._data["_expiring"] = False + # that is different from the access token. We are really only tracking + # access token expiration at this time. + self._data["_iat"] = _iat + self._data["_exp"] = _exp - def set_data(self, data): + 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) + super().set_data(data, copy_data) self._augment_rfc6749_data() def access_token(self): @@ -128,3 +125,9 @@ def expiry_time(self): def issued_time(self): return self.lazy_get("_iat") + + def is_expiring(self) -> bool: + return self.expiry_time() is not None + + def is_non_expiring(self) -> bool: + return not self.is_expiring() diff --git a/src/planet_auth/oidc/request_authenticator.py b/src/planet_auth/oidc/request_authenticator.py index 44552fc..0e42f6f 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 diff --git a/src/planet_auth/storage_utils.py b/src/planet_auth/storage_utils.py index f429aa7..7b39bab 100644 --- a/src/planet_auth/storage_utils.py +++ b/src/planet_auth/storage_utils.py @@ -364,14 +364,17 @@ def update_data(self, sparse_update_data): new_data = sparse_update_data self.set_data(new_data) - def set_data(self, data): + def set_data(self, data, copy_data: bool = True): """ Set the current in memory data. The data will be checked for validity before in memory values are set. Invalid data will result in an exception being thrown and no change being made to the in memory object. """ self.check_data(data) - self._data = data.copy() + if copy_data: + self._data = data.copy() + else: + self._data = data self._load_time = int(time.time()) def check_data(self, data): @@ -459,9 +462,7 @@ def load(self): return # we now allow in memory operation. Should we raise an error if the current data is invalid? new_data = self._object_storage_provider.load_obj(self._file_path) - self.check_data(new_data) - self._data = new_data - self._load_time = int(time.time()) + self.set_data(new_data, copy_data=False) def lazy_load(self): """ 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_token_credential.py b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_oidc_token_credential.py index 35ed159..21a7ad4 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,171 @@ 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()) + + @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()) + 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()) + + @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()) + 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()) From 9fba85fda99fac0865ad2d3a8326e87b002f24b4 Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Wed, 1 Oct 2025 10:42:48 -0700 Subject: [PATCH 05/33] doc fix --- src/planet_auth/logging/auth_logger.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/planet_auth/logging/auth_logger.py b/src/planet_auth/logging/auth_logger.py index 4107660..2563aa9 100644 --- a/src/planet_auth/logging/auth_logger.py +++ b/src/planet_auth/logging/auth_logger.py @@ -18,7 +18,7 @@ import importlib.metadata from contextlib import suppress -from typing import Dict +from typing import Dict, Optional from .events import AuthEvent from planet_auth.auth_exception import AuthException, InvalidTokenException @@ -234,14 +234,14 @@ def setPyLoggerForAuthLogger(py_logger: logging.Logger): _lib_global_py_logger = py_logger -def setStructuredLogging(nested_key=DEFAULT_NESTED_KEY): +def setStructuredLogging(nested_key: Optional[str] = DEFAULT_NESTED_KEY): """ Configure the library to emit structured log messages. When this mode is set, logs will be emitted specifying information using the logger's `extra` field. Parameters: - nested_key : dict key in which to wrap the library's data logged under + nested_key: dict key in which to wrap the library's data logged under the `extra` field. The default is to include all library logged extra fields encapsulated inside a dictionary with the single key `props`. This default was chosen to comform to the expectations of From 6842ed8b7aacba22e750ae03acd8fc7fa861b9bf Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Wed, 1 Oct 2025 10:47:05 -0700 Subject: [PATCH 06/33] version and changelog --- docs/changelog.md | 6 ++++++ version.txt | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/changelog.md b/docs/changelog.md index 77f72e6..1ed2b25 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,11 @@ # Changelog +## 2.2.0 - TBD +- Improve the user experience around old stale sessions that appear to be + initialized, but are actually expired. +- Save computed expiration time in token files. +- Support non-expiring tokens. + ## 2.1.1 - 2025-08-11 - Add py.typed to all top level packages diff --git a/version.txt b/version.txt index 3e3c2f1..ccbccc3 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -2.1.1 +2.2.0 From 8cd813b37d46e3599ed35b89398cf1cfe7e52ffe Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Wed, 1 Oct 2025 12:14:07 -0700 Subject: [PATCH 07/33] update to support through 3.14. Typing fixes for doc linting. --- docs/changelog.md | 2 +- noxfile.py | 2 +- src/planet_auth/logging/auth_logger.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 9108831..23123ef 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,7 +2,7 @@ ## 2.2.0 - 2025-10-02 - Update supported python versions. - Support for 3.9 dropped. Support for 3.15 added. + Support for 3.9 dropped. Support through 3.14 added. ## 2.1.1 - 2025-08-11 - Add py.typed to all top level packages diff --git a/noxfile.py b/noxfile.py index 74cc822..9514e76 100644 --- a/noxfile.py +++ b/noxfile.py @@ -21,7 +21,7 @@ ] _DEFAULT_PYTHON = "3.13" -_ALL_PYTHON = ["3.10", "3.11", "3.12", "3.13", "3.14", "3.15"] +_ALL_PYTHON = ["3.10", "3.11", "3.12", "3.13", "3.14"] @nox.session(python=_ALL_PYTHON) diff --git a/src/planet_auth/logging/auth_logger.py b/src/planet_auth/logging/auth_logger.py index 4107660..2563aa9 100644 --- a/src/planet_auth/logging/auth_logger.py +++ b/src/planet_auth/logging/auth_logger.py @@ -18,7 +18,7 @@ import importlib.metadata from contextlib import suppress -from typing import Dict +from typing import Dict, Optional from .events import AuthEvent from planet_auth.auth_exception import AuthException, InvalidTokenException @@ -234,14 +234,14 @@ def setPyLoggerForAuthLogger(py_logger: logging.Logger): _lib_global_py_logger = py_logger -def setStructuredLogging(nested_key=DEFAULT_NESTED_KEY): +def setStructuredLogging(nested_key: Optional[str] = DEFAULT_NESTED_KEY): """ Configure the library to emit structured log messages. When this mode is set, logs will be emitted specifying information using the logger's `extra` field. Parameters: - nested_key : dict key in which to wrap the library's data logged under + nested_key: dict key in which to wrap the library's data logged under the `extra` field. The default is to include all library logged extra fields encapsulated inside a dictionary with the single key `props`. This default was chosen to comform to the expectations of From 1b9da464dc4ebd2d0286e8f696f970546327767b Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Wed, 1 Oct 2025 12:15:51 -0700 Subject: [PATCH 08/33] drop python 3.15. --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9f71293..6a4f434 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -33,7 +33,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ['3.10', '3.11', '3.12', '3.13', '3.14', '3.15'] + python-version: ['3.10', '3.11', '3.12', '3.13', '3.14'] steps: - name: Checkout code uses: actions/checkout@v4 From b981809de2d02e40e6a0403660077e57bd2ad8f3 Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Wed, 1 Oct 2025 12:44:53 -0700 Subject: [PATCH 09/33] bump setup tools version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 0799bc1..1899c7d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -85,7 +85,7 @@ internal = [ plauth = "planet_auth_utils.commands.cli.main:cmd_plauth" [build-system] -requires = ["setuptools>=64", "setuptools_scm>=8", "wheel"] +requires = ["setuptools>=80", "setuptools_scm>=8", "wheel"] build-backend = "setuptools.build_meta" [tool.setuptools.dynamic] From d17e0d6ed3c0b03e4a6169074c3a095b313d4e81 Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Wed, 1 Oct 2025 14:28:07 -0700 Subject: [PATCH 10/33] Pull semgrep test dependencies into a different subpackage. It does not seem to install cleanly from pip under python 3.14, and this is breaking unrelated test pipelines. --- noxfile.py | 2 +- pyproject.toml | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/noxfile.py b/noxfile.py index 9514e76..49b7c24 100644 --- a/noxfile.py +++ b/noxfile.py @@ -40,7 +40,7 @@ def pytest(session): @nox.session(python=_DEFAULT_PYTHON) def semgrep_src(session): """Scan the code for security problems with semgrep""" - session.install("-e", ".[test]") + session.install("-e", ".[testsecurity]") # session.run("semgrep", "scan", "--strict", "--verbose", "--error", "--junit-xml", "--junit-xml-output=semgrep-src.xml", "src") session.run("semgrep", "scan", "--strict", "--verbose", "--error", "src") diff --git a/pyproject.toml b/pyproject.toml index 1899c7d..1f89537 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -69,9 +69,11 @@ test = [ "pytest", "pytest-cov", "pytest-xdist", - "semgrep", "validators", ] +testsecurity = [ + "semgrep", +] dev = [ "planet-auth[test, docs, build]", ] @@ -85,7 +87,7 @@ internal = [ plauth = "planet_auth_utils.commands.cli.main:cmd_plauth" [build-system] -requires = ["setuptools>=80", "setuptools_scm>=8", "wheel"] +requires = ["setuptools >= 77.0.3", "setuptools_scm >= 8"] build-backend = "setuptools.build_meta" [tool.setuptools.dynamic] From 1bd53b13f31983c0f38493398a4c549a27e17e69 Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Wed, 1 Oct 2025 15:19:47 -0700 Subject: [PATCH 11/33] version bump --- version.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 0b6c926672e10bbaead005dc341eed161740bef3 Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Thu, 2 Oct 2025 07:57:05 -0700 Subject: [PATCH 12/33] comments --- src/planet_auth/auth.py | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/planet_auth/auth.py b/src/planet_auth/auth.py index 345986c..b338437 100644 --- a/src/planet_auth/auth.py +++ b/src/planet_auth/auth.py @@ -122,11 +122,29 @@ def request_authenticator_is_ready(self) -> bool: def draft_insure_ready(self, allow_open_browser: Optional[bool] = False, allow_tty_prompt: Optional[bool] = False): """ Do everything necessary to insure the request authenticator is ready for use, - while still biasing towards not making unnecessary network requests - or prompts for user interaction. - - This can be more complex than it sounds, given the variety of capabilities - of authentication client types. + while still biasing towards just-in-time operations, 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. + Client 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. + 2. If the client has not been logged in and is a type that + can do so without prompting the user or probing the network. + 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. If this fails, the user will + be prompted to perform a fresh login. + 4. If the client has not 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 will still ultimately fail. For example, if @@ -166,6 +184,9 @@ def is_not_expired() -> bool: # user interaction. We should make them the same. # TODO: attempt user-interactive login + # TODO: document the allow_open_browser: Optional[bool] = False, allow_tty_prompt: Optional[bool] = False) parames + # through the login functions in this class. + # TODO: document parameters in pydocs return False def login(self, **kwargs) -> Credential: From 21dddaceb30503b25d9924bb4205d9bd09a434f7 Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Fri, 3 Oct 2025 10:47:28 -0700 Subject: [PATCH 13/33] WIP --- src/planet_auth/auth.py | 117 ++++++++++++------ src/planet_auth/credential.py | 65 ++++++++++ src/planet_auth/oidc/oidc_credential.py | 12 -- src/planet_auth/request_authenticator.py | 2 +- .../oidc/test_oidc_token_credential.py | 32 +++++ 5 files changed, 176 insertions(+), 52 deletions(-) diff --git a/src/planet_auth/auth.py b/src/planet_auth/auth.py index b338437..4391af0 100644 --- a/src/planet_auth/auth.py +++ b/src/planet_auth/auth.py @@ -119,10 +119,12 @@ def request_authenticator_is_ready(self) -> bool: """ return self._request_authenticator.is_initialized() or self._auth_client.can_login_unattended() - def draft_insure_ready(self, allow_open_browser: Optional[bool] = False, allow_tty_prompt: Optional[bool] = False): + def ensure_request_authenticator_is_ready( + self, allow_open_browser: Optional[bool] = False, allow_tty_prompt: Optional[bool] = False + ) -> None: """ - Do everything necessary to insure the request authenticator is ready for use, - while still biasing towards just-in-time operations, not making + 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 @@ -134,16 +136,21 @@ def draft_insure_ready(self, allow_open_browser: Optional[bool] = False, allow_t 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 + 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 or probing the network. - 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. If this fails, the user will - be prompted to perform a fresh login. - 4. If the client has not been logged in and is a type that - requires a user interactive login a user interactive + can do so without prompting the user or probing the network, + 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 @@ -152,44 +159,68 @@ def draft_insure_ready(self, allow_open_browser: Optional[bool] = False, allow_t assumed to be ready but the credentials could be bad. Even when ready, requests could fail for completely valid credentials if service policy denies the request to the authenticated client. + + 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. """ - # TODO: be careful about when we trigger calls. - # Should we have a broader "is_ready()" or "insure_ready(allowed_tty, allowed_browser)"? - # making ourselves ready is more than checking if the authenticator is ready. - # if the authenticator isn't ready, making it ready may require the auth client. - def has_credential() -> bool: + def _has_credential() -> bool: # Does not do any JIT checks return self._request_authenticator.is_initialized() - def can_obtain_credentials_unattended() -> bool: + def _can_obtain_credentials_unattended() -> bool: # Does not do any JIT checks return self._auth_client.can_login_unattended() - def is_not_expired() -> bool: + def _is_expired() -> bool: # Does not do any JIT check - return False - - if has_credential() and is_not_expired(): - return True - - if can_obtain_credentials_unattended(): - # Should we fetch one? we do not because the bias is towards JIT operations. - # This so programs can initialize and not fail unless the credential is actually needed. - # TODO: make caller configurable. "do_credential_prefetch" arg? + new_cred = self._request_authenticator.credential(refresh_if_needed=False) + if new_cred: + return new_cred.is_expired() return True - # TODO: attempt interaction-free refresh ... if has_refresh() ... or try refresh() - # TODO: check how we implemented "refresh" vs "login" for flow that do not require - # user interaction. We should make them the same. - - # TODO: attempt user-interactive login - # TODO: document the allow_open_browser: Optional[bool] = False, allow_tty_prompt: Optional[bool] = False) parames - # through the login functions in this class. - # TODO: document parameters in pydocs - return False - - def login(self, **kwargs) -> Credential: + # Case #1 above. + if _has_credential() and not _is_expired(): + return + + # Case #2 above. + if _can_obtain_credentials_unattended(): + # TODO? 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.") + if new_cred.is_expired(): + raise RuntimeError("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." + ) + # Fall through to case #4 below. + # self.login(allow_open_browser=allow_open_browser, allow_tty_prompt=allow_tty_prompt) + # return + + # Case #4 above. + new_cred = self.login(allow_open_browser=allow_open_browser, allow_tty_prompt=allow_tty_prompt) + if not new_cred: + # Most login failures should raise their own more details exceptions. + raise RuntimeError("Unable to ready request authenticator.") + + 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 @@ -197,8 +228,16 @@ 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 + ) 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..a7600a2 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 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 57464bc..195188e 100644 --- a/src/planet_auth/oidc/oidc_credential.py +++ b/src/planet_auth/oidc/oidc_credential.py @@ -119,15 +119,3 @@ def refresh_token(self): Get the current refresh token. """ return self.lazy_get("refresh_token") - - def expiry_time(self): - return self.lazy_get("_exp") - - def issued_time(self): - return self.lazy_get("_iat") - - def is_expiring(self) -> bool: - return self.expiry_time() is not None - - def is_non_expiring(self) -> bool: - return not self.is_expiring() diff --git a/src/planet_auth/request_authenticator.py b/src/planet_auth/request_authenticator.py index 3a0d57c..538d9f5 100644 --- a/src/planet_auth/request_authenticator.py +++ b/src/planet_auth/request_authenticator.py @@ -135,7 +135,7 @@ def credential(self, refresh_if_needed: bool = False) -> Optional[Credential]: Return the current credential. This may not be the credential the authenticator was constructed with. - Request Authenticators are free to refresh credentials depending in the + Request Authenticators are free to refresh credentials depending on the needs of the implementation. This may happen upon this request, or may happen as a side effect of RequestAuthenticator operations. """ 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 21a7ad4..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 @@ -160,6 +160,12 @@ def test_load_credential_file__opaque_tokens__non_augmented__without_lifespan(se 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() @@ -177,6 +183,10 @@ def test_save_persists_computed_values_with_lifespan(self, frozen_time): ) 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() @@ -185,6 +195,15 @@ def test_save_persists_computed_values_with_lifespan(self, frozen_time): 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): @@ -202,6 +221,10 @@ def test_save_persists_computed_values_without_lifespan(self, frozen_time): ) 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() @@ -210,3 +233,12 @@ def test_save_persists_computed_values_without_lifespan(self, frozen_time): 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()) From 72ecbaf822f22c414b136012f89fb4d0038577d4 Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Fri, 3 Oct 2025 11:02:25 -0700 Subject: [PATCH 14/33] examples --- .../oauth/make-oauth-authenticated-httpx-request.py | 1 + .../make-oauth-authenticated-requests-request.py | 1 + .../oauth/perform-oauth-initial-login-2.py | 13 +++++++++++++ 3 files changed, 15 insertions(+) create mode 100644 docs/examples/auth-client/oauth/perform-oauth-initial-login-2.py 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() From f70868eae6b6b745c36f69e4ffbf0b6d73ea8f36 Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Fri, 3 Oct 2025 11:09:27 -0700 Subject: [PATCH 15/33] update changelog --- docs/changelog.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 3e6b61e..5a090e6 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,8 +2,10 @@ ## 2.3.0 - TBD - Improve the user experience around old stale sessions that appear to be - initialized, but are actually expired. -- Save computed expiration time in token files. + 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. - Support non-expiring tokens. ## 2.2.0 - 2025-10-02 From db6bf3d8868ee6cd2d467cd0802bc73fc902de6b Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Fri, 3 Oct 2025 17:42:06 -0700 Subject: [PATCH 16/33] WIP --- src/planet_auth/oidc/oidc_credential.py | 23 ++++--- src/planet_auth/oidc/request_authenticator.py | 43 +++++++++---- .../oidc/test_oidc_request_authenticator.py | 64 ++++++++++++++++++- tests/test_planet_auth/unit/auth/util.py | 9 ++- 4 files changed, 109 insertions(+), 30 deletions(-) diff --git a/src/planet_auth/oidc/oidc_credential.py b/src/planet_auth/oidc/oidc_credential.py index 195188e..06cdab2 100644 --- a/src/planet_auth/oidc/oidc_credential.py +++ b/src/planet_auth/oidc/oidc_credential.py @@ -43,10 +43,6 @@ def check_data(self, data): ) def _augment_rfc6749_data(self): - # TODO - I am depending on saving an explicit None for "never expires". Check whether I allow this. - # TODO - Verify behavior when new fields do not exist. - # TODO - Check the none save-reload behavior - # TODO - unit test for all flavors of 0/null exp / iat in token data. # 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. @@ -55,15 +51,21 @@ def _augment_rfc6749_data(self): # 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 - access_token_str = self.access_token() - if not access_token_str: - return - try: - (_, jwt_hazmat_body, _) = TokenValidator.hazmat_unverified_decode(access_token_str) + 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 @@ -88,9 +90,6 @@ def _augment_rfc6749_data(self): if _exp is None and rfc6749_lifespan > 0: _exp = _iat + rfc6749_lifespan - # Edge case - It's possible that a JWT ID token has an expiration time - # that is different from the access token. We are really only tracking - # access token expiration at this time. self._data["_iat"] = _iat self._data["_exp"] = _exp diff --git a/src/planet_auth/oidc/request_authenticator.py b/src/planet_auth/oidc/request_authenticator.py index 0e42f6f..5dc3976 100644 --- a/src/planet_auth/oidc/request_authenticator.py +++ b/src/planet_auth/oidc/request_authenticator.py @@ -58,17 +58,17 @@ def _load_and_prime(self): self._credential.load() self._token_body = self._credential.access_token() - # TODO - Continuity - Old token files won't have the augmented data, but they will expire. - # Since they are JWTs, will it just work? - # TODO - should treat the expiry of the access and ID token separately? - # TODO - handle non-expiring tokens # TODO - Test all flavors of missing times: # exp: None, iat: Any - Never expires # exp: time, iat: None - Expires at time. Refresh per fallback grace period. # exp: time: iat: time - Expires at time. Refresh at the 3/4 life. iat = self._credential.issued_time() or 0 - exp = self._credential.expiry_time() or 0 - self._refresh_at = int(iat + (3 * (exp - iat) / 4)) + 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: @@ -86,13 +86,28 @@ def _refresh(self): self._credential = new_credentials self._load_and_prime() + def _refresh_needed(self, check_time: Optional[int] = None) -> bool: + if self._token_body is None: + # Always consider no token in need of a 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, + # Also, if refresh tokens may be configured to be one time use, # we want a fresh refresh token. Stale refresh tokens may be - # invalid depending on server configuration. + # invalid. # # Also, it's possible that we have a valid refresh token, # but not an access token. When that's true, we should @@ -100,15 +115,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_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 @@ -139,7 +156,7 @@ 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) - self._refresh_at = 0 + self._refresh_at = 0 # This mimics __init__. Check refresh JIT. # self._load() # Mimic __init__. Don't load, let that happen JIT. def update_credential_data(self, new_credential_data: Dict) -> None: 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/util.py b/tests/test_planet_auth/unit/auth/util.py index b77fcc7..12d8f40 100644 --- a/tests/test_planet_auth/unit/auth/util.py +++ b/tests/test_planet_auth/unit/auth/util.py @@ -69,9 +69,10 @@ def _construct_oidc_access_token(self, username: str, ttl: int, extra_claims, re "sub": username, "aud": self.audience, "iat": now, - "exp": now + ttl, "jti": str(uuid.uuid4()), } + if ttl: + unsigned_jwt["exp"] = now + ttl unsigned_jwt.update(extra_claims) if remove_claims: for remove_claim in remove_claims: @@ -118,9 +119,10 @@ def construct_oidc_id_token(self, client_id: str, ttl: int, extra_claims=None, r "sub": client_id, "aud": client_id, "iat": now, - "exp": now + ttl, "jti": str(uuid.uuid4()), } + if ttl: + unsigned_jwt["exp"] = now + ttl if extra_claims: # Note: this is clobbering of the claims above! might be fine # for this test class, but be warned if you copy-paste somewhere. @@ -322,9 +324,10 @@ def _construct_oidc_credential( credential_data = { "token_type": "Bearer", - "expires_in": self._mock_client_config.stub_authority_ttl, "scope": " ".join(requested_scopes), } + if self._mock_client_config.stub_authority_ttl: + credential_data["expires_in"] = self._mock_client_config.stub_authority_ttl if get_access_token: credential_data["access_token"] = jwt_access_token if get_id_token: From 260c208f36d9d60f4ad74654f7df0ea7d0d759e8 Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Sat, 4 Oct 2025 12:51:18 -0700 Subject: [PATCH 17/33] WIP - tests and test changes. --- src/planet_auth/__init__.py | 3 +- src/planet_auth/auth.py | 21 +- src/planet_auth/credential.py | 2 +- src/planet_auth/request_authenticator.py | 2 +- tests/test_planet_auth/unit/auth/test_auth.py | 323 +++++++++++++++++- 5 files changed, 337 insertions(+), 14 deletions(-) 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 4391af0..d0b5b8e 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: @@ -200,9 +202,9 @@ def _is_expired() -> bool: # 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.") + raise RuntimeError("Unable to refresh credentials - Unknown error") if new_cred.is_expired(): - raise RuntimeError("Refreshed credentials are still expired.") + raise RuntimeError("Unable to refresh credentials - Refreshed credentials are still expired.") return except Exception as e: auth_logger.warning( @@ -213,10 +215,7 @@ def _is_expired() -> bool: # return # Case #4 above. - new_cred = self.login(allow_open_browser=allow_open_browser, allow_tty_prompt=allow_tty_prompt) - if not new_cred: - # Most login failures should raise their own more details exceptions. - raise RuntimeError("Unable to ready request authenticator.") + 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 @@ -238,6 +237,10 @@ def login( 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 a7600a2..d1aa57c 100644 --- a/src/planet_auth/credential.py +++ b/src/planet_auth/credential.py @@ -82,7 +82,7 @@ def is_expired(self, at_time: Optional[int] = None) -> bool: at_time = int(time.time()) exp = self.expiry_time() - return at_time >= exp # type: ignore[operator] + return bool(at_time >= exp) # type: ignore[operator] def is_not_expired(self, at_time: Optional[int] = None) -> bool: """ diff --git a/src/planet_auth/request_authenticator.py b/src/planet_auth/request_authenticator.py index 538d9f5..9ab7b1e 100644 --- a/src/planet_auth/request_authenticator.py +++ b/src/planet_auth/request_authenticator.py @@ -126,7 +126,7 @@ def update_credential_data(self, new_credential_data: Dict) -> None: 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 + # Child classes are expected to populate it JIT for auth # requests. self._token_body = None diff --git a/tests/test_planet_auth/unit/auth/test_auth.py b/tests/test_planet_auth/unit/auth/test_auth.py index f7cedf5..e27ce39 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 patch, 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,318 @@ 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): + # self._refresh_if_needed() + super().pre_request_hook() + + 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 AuthTestNew(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) From 22433cc76138d72f985e5493645c85a2a26a9719 Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Sat, 4 Oct 2025 13:08:30 -0700 Subject: [PATCH 18/33] fixes --- tests/test_planet_auth/unit/auth/test_auth.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_planet_auth/unit/auth/test_auth.py b/tests/test_planet_auth/unit/auth/test_auth.py index e27ce39..5bef856 100644 --- a/tests/test_planet_auth/unit/auth/test_auth.py +++ b/tests/test_planet_auth/unit/auth/test_auth.py @@ -16,7 +16,7 @@ import time import unittest from typing import List, Optional -from unittest.mock import patch, MagicMock +from unittest.mock import MagicMock from planet_auth.auth import Auth, Credential, CredentialRequestAuthenticator, AuthClientContextException from planet_auth.auth_client import AuthClient, AuthClientConfig, AuthClientException @@ -179,8 +179,7 @@ def __init__(self, credential: FakeCredential, auth_client: FakeAuthClient = Non self._auth_client = auth_client def pre_request_hook(self): - # self._refresh_if_needed() - super().pre_request_hook() + pass def _refresh_needed(self): if not self._credential: From 97e995a9ac66b6b8369dbf1d73629dbf511e44c1 Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sat, 4 Oct 2025 18:42:53 -0700 Subject: [PATCH 19/33] update comments --- src/planet_auth/auth.py | 20 ++++++++++---------- src/planet_auth/request_authenticator.py | 1 - 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/planet_auth/auth.py b/src/planet_auth/auth.py index d0b5b8e..ba8e1a1 100644 --- a/src/planet_auth/auth.py +++ b/src/planet_auth/auth.py @@ -142,25 +142,25 @@ def ensure_request_authenticator_is_ready( 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 or probing the network, - 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. + 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. + 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 will still ultimately fail. For example, if + 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, - requests could fail for completely valid credentials if service - policy denies the request to the authenticated client. + 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 diff --git a/src/planet_auth/request_authenticator.py b/src/planet_auth/request_authenticator.py index 9ab7b1e..b4c80f2 100644 --- a/src/planet_auth/request_authenticator.py +++ b/src/planet_auth/request_authenticator.py @@ -148,7 +148,6 @@ def is_initialized(self) -> bool: is checked for the existence of a credential, but it will not be loaded from storage at this time, preserving JIT loading behavior. """ - # TODO: Rename to "has_credential()"? if self._credential: return bool(self._credential.data()) or self._credential.is_persisted_to_storage() return False From 5cdd7342716466012155d0f5fa423049935e61fb Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sat, 4 Oct 2025 18:43:37 -0700 Subject: [PATCH 20/33] update comments --- src/planet_auth/oidc/request_authenticator.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/planet_auth/oidc/request_authenticator.py b/src/planet_auth/oidc/request_authenticator.py index 5dc3976..60dc093 100644 --- a/src/planet_auth/oidc/request_authenticator.py +++ b/src/planet_auth/oidc/request_authenticator.py @@ -105,9 +105,8 @@ def _refresh_if_needed(self): # Another process might have done it for us, and save us the # network call. # - # Also, if refresh tokens may be configured to be one time use, - # we want a fresh refresh token. Stale refresh tokens may be - # invalid. + # 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 @@ -156,8 +155,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) - self._refresh_at = 0 # This mimics __init__. Check refresh JIT. - # self._load() # Mimic __init__. Don't load, let that happen JIT. + # 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_and_prime() def update_credential_data(self, new_credential_data: Dict) -> None: # This is more different than update_credential() than it may From 5dbd51052030a2ad7e97b5dbff998a7927fa2573 Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sat, 4 Oct 2025 20:25:48 -0700 Subject: [PATCH 21/33] update comments --- src/planet_auth/oidc/request_authenticator.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/planet_auth/oidc/request_authenticator.py b/src/planet_auth/oidc/request_authenticator.py index 60dc093..2cf9039 100644 --- a/src/planet_auth/oidc/request_authenticator.py +++ b/src/planet_auth/oidc/request_authenticator.py @@ -58,10 +58,6 @@ def _load_and_prime(self): self._credential.load() self._token_body = self._credential.access_token() - # TODO - Test all flavors of missing times: - # exp: None, iat: Any - Never expires - # exp: time, iat: None - Expires at time. Refresh per fallback grace period. - # exp: time: iat: time - Expires at time. Refresh at the 3/4 life. iat = self._credential.issued_time() or 0 exp = self._credential.expiry_time() if exp is None: From 5b19b552feafd145b40b055d9d0c201864ee62eb Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sat, 4 Oct 2025 20:26:11 -0700 Subject: [PATCH 22/33] update coverage report --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 1f89537..7d1f0a5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -106,6 +106,7 @@ build-backend = "setuptools.build_meta" source = [ "planet_auth", "planet_auth_utils", + "planet_auth_config_injection", # "tests", ] branch = true From 421a99d7b9d6976bf195082477e02e16000d3d76 Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sat, 4 Oct 2025 20:32:17 -0700 Subject: [PATCH 23/33] update comments --- src/planet_auth/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/planet_auth/auth.py b/src/planet_auth/auth.py index ba8e1a1..0a62e05 100644 --- a/src/planet_auth/auth.py +++ b/src/planet_auth/auth.py @@ -190,7 +190,7 @@ def _is_expired() -> bool: # Case #2 above. if _can_obtain_credentials_unattended(): - # TODO? Should we fetch one? We do not by default because the bias is towards + # 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 From f22ed32c2e5bda8fd7261da56c3b54c1330a3b8b Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sat, 4 Oct 2025 21:05:25 -0700 Subject: [PATCH 24/33] update docs --- src/planet_auth/oidc/token_validator.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/planet_auth/oidc/token_validator.py b/src/planet_auth/oidc/token_validator.py index 4903825..986fcbb 100644 --- a/src/planet_auth/oidc/token_validator.py +++ b/src/planet_auth/oidc/token_validator.py @@ -14,7 +14,7 @@ import jwt import time -from typing import Dict, List +from typing import Any, Dict, List, Tuple import planet_auth.logging.auth_logger from planet_auth.auth_exception import AuthException, InvalidTokenException @@ -258,13 +258,16 @@ def validate_token( @staticmethod @InvalidArgumentException.recast(jwt.exceptions.DecodeError) - def hazmat_unverified_decode(token_str): + def hazmat_unverified_decode(token_str) -> Tuple[Dict, Dict, Any]: """ Decide a JWT without verifying the signature or any claims. !!! Warning Treat unverified token claims with extreme caution. Nothing can be trusted until the token is verified. + + Returns: + Returns the decoded JWT header, payload, and signature """ unverified_complete = jwt.decode_complete(token_str, options={"verify_signature": False}) # nosemgrep return unverified_complete["header"], unverified_complete["payload"], unverified_complete["signature"] From 1040170ae812740d6bc5ce8771b5505df9c82262 Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sun, 5 Oct 2025 08:14:59 -0700 Subject: [PATCH 25/33] cherry-picking some small incidental changes from a larger feature branch to front-run the main MR. --- .gitignore | 1 + pyproject.toml | 1 + src/planet_auth/oidc/token_validator.py | 17 +++++-- src/planet_auth/request_authenticator.py | 4 +- .../auth_clients/oidc/test_token_validator.py | 44 ++++++++++++++++--- 5 files changed, 56 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index d80596f..7829556 100644 --- a/.gitignore +++ b/.gitignore @@ -16,4 +16,5 @@ site *.venv venv* version-with-buildnum.txt +.vscode __pycache__ diff --git a/pyproject.toml b/pyproject.toml index 1f89537..7d1f0a5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -106,6 +106,7 @@ build-backend = "setuptools.build_meta" source = [ "planet_auth", "planet_auth_utils", + "planet_auth_config_injection", # "tests", ] branch = true diff --git a/src/planet_auth/oidc/token_validator.py b/src/planet_auth/oidc/token_validator.py index 632bcee..986fcbb 100644 --- a/src/planet_auth/oidc/token_validator.py +++ b/src/planet_auth/oidc/token_validator.py @@ -14,7 +14,7 @@ import jwt import time -from typing import Dict, List +from typing import Any, Dict, List, Tuple import planet_auth.logging.auth_logger from planet_auth.auth_exception import AuthException, InvalidTokenException @@ -257,9 +257,18 @@ def validate_token( return validated_claims @staticmethod - def hazmat_unverified_decode(token_str): - # WARNING: Treat unverified token claims like toxic waste. - # Nothing can be trusted until the token is verified. + @InvalidArgumentException.recast(jwt.exceptions.DecodeError) + def hazmat_unverified_decode(token_str) -> Tuple[Dict, Dict, Any]: + """ + Decide a JWT without verifying the signature or any claims. + + !!! Warning + Treat unverified token claims with extreme caution. + Nothing can be trusted until the token is verified. + + Returns: + Returns the decoded JWT header, payload, and signature + """ unverified_complete = jwt.decode_complete(token_str, options={"verify_signature": False}) # nosemgrep return unverified_complete["header"], unverified_complete["payload"], unverified_complete["signature"] diff --git a/src/planet_auth/request_authenticator.py b/src/planet_auth/request_authenticator.py index d7bdbf6..b4c80f2 100644 --- a/src/planet_auth/request_authenticator.py +++ b/src/planet_auth/request_authenticator.py @@ -126,7 +126,7 @@ def update_credential_data(self, new_credential_data: Dict) -> None: 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 + # Child classes are expected to populate it JIT for auth # requests. self._token_body = None @@ -135,7 +135,7 @@ def credential(self, refresh_if_needed: bool = False) -> Optional[Credential]: Return the current credential. This may not be the credential the authenticator was constructed with. - Request Authenticators are free to refresh credentials depending in the + Request Authenticators are free to refresh credentials depending on the needs of the implementation. This may happen upon this request, or may happen as a side effect of RequestAuthenticator operations. """ diff --git a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py index d67881d..1543ddb 100644 --- a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py +++ b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py @@ -104,6 +104,7 @@ def test_valid_access_token(self): access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) + validated_claims = under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, @@ -111,6 +112,11 @@ def test_valid_access_token(self): ) self.assertEqual(TEST_TOKEN_USER, validated_claims["sub"]) + u_header, u_body, _ = TokenValidator.hazmat_unverified_decode(access_token) + self.assertDictEqual(u_header, {"alg": "RS256", "kid": "test_keypair1", "typ": "JWT"}) + self.assertEqual(u_body.get("aud"), "test_token_audience_for_keypair1") + self.assertEqual(u_body.get("sub"), "unit_test_user") + def test_empty_access_token(self): # QE TC3, TC5 under_test = self.under_test_1 @@ -126,26 +132,36 @@ def test_empty_access_token(self): issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode("") + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode(None) def test_malformed_token_1(self): # QE TC6 - just some random garbage. under_test = self.under_test_1 + test_token = secrets.token_bytes(2048) with self.assertRaises(InvalidTokenException): under_test.validate_token( - token_str=secrets.token_bytes(2048), + token_str=test_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode(test_token) def test_malformed_token_2(self): # QE TC6 - just some random garbage, but URL safe. under_test = self.under_test_1 + test_token = secrets.token_urlsafe(2048) with self.assertRaises(InvalidTokenException): under_test.validate_token( - token_str=secrets.token_urlsafe(2048), + token_str=test_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode(test_token) def test_malformed_token_3(self): # QE TC6 - random garbage, but has JWT three dot structure. @@ -162,11 +178,14 @@ def test_malformed_token_3(self): issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode(fake_jwt) def test_malformed_token_4(self): # QE TC6 - random garbage, but looks more JWT like with encoded json. def _fake_token(header, body): - return "{}.{}.{}".format( + fake_sig_bytes = secrets.token_bytes(256) + fake_jwt = "{}.{}.{}".format( str( jwt.utils.base64url_encode( bytes( @@ -180,11 +199,12 @@ def _fake_token(header, body): jwt.utils.base64url_encode(bytes(json.dumps(body), "utf-8")), encoding="utf-8", ), - str(jwt.utils.base64url_encode(secrets.token_bytes(256)), encoding="utf-8"), + str(jwt.utils.base64url_encode(fake_sig_bytes), encoding="utf-8"), ) + return fake_jwt, fake_sig_bytes under_test = self.under_test_1 - fake_jwt = _fake_token( + fake_jwt, fake_sig_bytes = _fake_token( { "alg": self.token_builder_1.signing_key_algorithm, "kid": self.token_builder_1.signing_key_id, @@ -199,6 +219,13 @@ def _fake_token(header, body): audience=self.token_builder_1.audience, ) + # This is good enough for an unverified decode. It's still invalid, which is + # why you are careful with unverified decoded data. + u_header, u_body, u_sig = TokenValidator.hazmat_unverified_decode(fake_jwt) + self.assertDictEqual(u_header, {"alg": "RS256", "kid": "test_keypair1"}) + self.assertDictEqual(u_body, {"fake_claim": "test claim value"}) + self.assertEqual(u_sig, fake_sig_bytes) + def test_malformed_token_missing_issuer(self): # QE TC14 under_test = self.under_test_1 @@ -287,6 +314,11 @@ def _bad_token(header, body): audience=self.token_builder_1.audience, ) + u_header, u_body, u_sig = TokenValidator.hazmat_unverified_decode(bad_jwt) + self.assertIsNotNone(u_header) + self.assertIsNotNone(u_body) + self.assertEqual(u_sig, b"") + def test_missing_signature_2(self): # QE TC11 - JWT without a signature, and with no second "." def _bad_token(header, body): @@ -321,6 +353,8 @@ def _bad_token(header, body): issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + with self.assertRaises(InvalidArgumentException): + TokenValidator.hazmat_unverified_decode(bad_jwt) def test_empty_issuer_arg(self): under_test = self.under_test_1 From 8ad43a6055d88fe0ab5eb54f2c397c08572ecf77 Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sun, 5 Oct 2025 09:48:27 -0700 Subject: [PATCH 26/33] more details test assertions for token validator --- src/planet_auth/oidc/multi_validator.py | 2 +- src/planet_auth/oidc/token_validator.py | 2 +- .../auth_clients/oidc/test_token_validator.py | 139 +++++++++++++----- 3 files changed, 103 insertions(+), 40 deletions(-) diff --git a/src/planet_auth/oidc/multi_validator.py b/src/planet_auth/oidc/multi_validator.py index 693c43b..a140a23 100644 --- a/src/planet_auth/oidc/multi_validator.py +++ b/src/planet_auth/oidc/multi_validator.py @@ -286,7 +286,7 @@ def validate_access_token( """ if not token: - raise InvalidArgumentException(message="Cannot validate empty string as a token") + raise InvalidArgumentException(message="Cannot decode empty string as a token") validator = self._select_validator(token) local_validation, remote_validation = self._check_access_token( diff --git a/src/planet_auth/oidc/token_validator.py b/src/planet_auth/oidc/token_validator.py index 986fcbb..0bf83bd 100644 --- a/src/planet_auth/oidc/token_validator.py +++ b/src/planet_auth/oidc/token_validator.py @@ -181,7 +181,7 @@ def validate_token( """ # PyJWT should enforce this, but we have unit tests in case... if not token_str: - raise InvalidArgumentException(message="Cannot validate empty string as a token") + raise InvalidArgumentException(message="Cannot decode empty string as a token") if not issuer: # PyJWT does not seem to raise if the issuer is explicitly None, even when # verify_iss was selected. diff --git a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py index 1543ddb..7fb9631 100644 --- a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py +++ b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py @@ -120,48 +120,65 @@ def test_valid_access_token(self): def test_empty_access_token(self): # QE TC3, TC5 under_test = self.under_test_1 - with self.assertRaises(InvalidArgumentException): + with self.assertRaises(InvalidArgumentException) as raised1: under_test.validate_token( "", issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Cannot decode empty string as a token", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: under_test.validate_token( None, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Cannot decode empty string as a token", str(raised2.exception)) + + with self.assertRaises(InvalidArgumentException) as raised3: TokenValidator.hazmat_unverified_decode("") - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Not enough segments (DecodeError)", str(raised3.exception)) + + with self.assertRaises(InvalidArgumentException) as raised4: TokenValidator.hazmat_unverified_decode(None) + self.assertEqual("Invalid token type. Token must be a (DecodeError)", str(raised4.exception)) def test_malformed_token_1(self): # QE TC6 - just some random garbage. under_test = self.under_test_1 test_token = secrets.token_bytes(2048) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=test_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + # Random garbage may throw different errors. + # self.assertEqual("TBD", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: TokenValidator.hazmat_unverified_decode(test_token) + # Random garbage may throw different errors + # self.assertEqual("TBD", str(raised2.exception)) def test_malformed_token_2(self): # QE TC6 - just some random garbage, but URL safe. under_test = self.under_test_1 test_token = secrets.token_urlsafe(2048) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=test_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + # Random garbage may throw different errors. + # self.assertEqual("TBD", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: TokenValidator.hazmat_unverified_decode(test_token) + # Random garbage may throw different errors. + # self.assertEqual("TBD", str(raised2.exception)) def test_malformed_token_3(self): # QE TC6 - random garbage, but has JWT three dot structure. @@ -172,14 +189,19 @@ def test_malformed_token_3(self): str(jwt.utils.base64url_encode(secrets.token_bytes(256)), encoding="utf-8"), ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=fake_jwt, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + # Random garbage may throw different errors. + # self.assertEqual("TBD", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: TokenValidator.hazmat_unverified_decode(fake_jwt) + # Random garbage may throw different errors. + # self.assertEqual("TBD", str(raised2.exception)) def test_malformed_token_4(self): # QE TC6 - random garbage, but looks more JWT like with encoded json. @@ -212,12 +234,14 @@ def _fake_token(header, body): {"fake_claim": "test claim value"}, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=fake_jwt, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + # Will random garbage always parse to this exact error? + self.assertEqual("Signature verification failed (InvalidSignatureError)", str(raised1.exception)) # This is good enough for an unverified decode. It's still invalid, which is # why you are careful with unverified decoded data. @@ -232,12 +256,13 @@ def test_malformed_token_missing_issuer(self): access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL, remove_claims=["iss"] ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + self.assertEqual('Token is missing the "iss" claim (MissingRequiredClaimError)', str(raised1.exception)) def test_altered_token(self): # QE TC10 @@ -272,12 +297,13 @@ def _alter_token(access_token, old_claim_value, new_claim_value): # Now, actually change the claim value, and look for the exception altered_access_token = _alter_token(access_token, "sensitive_value_A", "sensitive_value_B") - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( altered_access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + self.assertEqual("Signature verification failed (InvalidSignatureError)", str(raised1.exception)) def test_missing_signature_1(self): # QE TC11 - JWT without a signature, but with the "." @@ -307,12 +333,13 @@ def _bad_token(header, body): {"fake_claim": "test claim value"}, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=bad_jwt, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + self.assertEqual("Signature verification failed (InvalidSignatureError)", str(raised1.exception)) u_header, u_body, u_sig = TokenValidator.hazmat_unverified_decode(bad_jwt) self.assertIsNotNone(u_header) @@ -347,14 +374,17 @@ def _bad_token(header, body): {"fake_claim": "test claim value"}, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=bad_jwt, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Not enough segments (DecodeError)", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: TokenValidator.hazmat_unverified_decode(bad_jwt) + self.assertEqual("Not enough segments (DecodeError)", str(raised2.exception)) def test_empty_issuer_arg(self): under_test = self.under_test_1 @@ -367,18 +397,21 @@ def test_empty_issuer_arg(self): audience=self.token_builder_1.audience, ) self.assertEqual(TEST_TOKEN_USER, validated_claims["sub"]) - with self.assertRaises(InvalidArgumentException): + with self.assertRaises(InvalidArgumentException) as raised1: validated_claims = under_test.validate_token( access_token, issuer="", audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Cannot validate token with no required issuer provided", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: validated_claims = under_test.validate_token( access_token, issuer=None, audience=self.token_builder_1.audience, ) + self.assertEqual("Cannot validate token with no required issuer provided", str(raised2.exception)) def test_empty_audience_arg(self): under_test = self.under_test_1 @@ -391,18 +424,21 @@ def test_empty_audience_arg(self): audience=self.token_builder_1.audience, ) self.assertEqual(TEST_TOKEN_USER, validated_claims["sub"]) - with self.assertRaises(InvalidArgumentException): + with self.assertRaises(InvalidArgumentException) as raised1: validated_claims = under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience="", ) - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Cannot validate token with no required audience provided", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: validated_claims = under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=None, ) + self.assertEqual("Cannot validate token with no required audience provided", str(raised2.exception)) def test_access_token_unknown_signing_key(self): # QE TC12 @@ -410,12 +446,13 @@ def test_access_token_unknown_signing_key(self): access_token = self.token_builder_2.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(UnknownSigningKeyTokenException): + with self.assertRaises(UnknownSigningKeyTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_2.issuer, audience=self.token_builder_2.audience, ) + self.assertEqual("Could not find signing key for key ID test_keypair2", str(raised1.exception)) def test_access_token_issuer_mismatch(self): # QE TC15 untrusted issuer. (See also multi-validator tests) @@ -423,12 +460,13 @@ def test_access_token_issuer_mismatch(self): access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer + "_make_it_mismatch", audience=self.token_builder_1.audience, ) + self.assertEqual("Invalid issuer (InvalidIssuerError)", str(raised1.exception)) def test_access_token_incorrect_audience(self): # QE TC17 @@ -436,12 +474,13 @@ def test_access_token_incorrect_audience(self): access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience + "_make_it_mismatch", ) + self.assertEqual("Audience doesn't match (InvalidAudienceError)", str(raised1.exception)) def test_access_token_multiple_audiences(self): # QE TC17 @@ -461,10 +500,11 @@ def test_access_token_multiple_audiences(self): under_test.validate_token( token_str=access_token, issuer=self.token_builder_1.issuer, audience="extra_audience_2" ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=access_token, issuer=self.token_builder_1.issuer, audience="extra_audience_3" ) + self.assertEqual("Audience doesn't match (InvalidAudienceError)", str(raised1.exception)) @freezegun.freeze_time(as_kwarg="frozen_time") def test_access_token_expired(self, frozen_time): @@ -474,25 +514,29 @@ def test_access_token_expired(self, frozen_time): username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=3 ) frozen_time.tick(5) - with self.assertRaises(ExpiredTokenException): + with self.assertRaises(ExpiredTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + self.assertEqual("Signature has expired (ExpiredSignatureError)", str(raised1.exception)) def test_access_token_missing_claim(self): under_test = self.under_test_1 access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, required_claims=["missing_claim_1", "missing_claim_2"], ) + self.assertEqual( + 'Token is missing the "missing_claim_1" claim (MissingRequiredClaimError)', str(raised1.exception) + ) def test_access_token_scope_validation__no_scopes_required__rfc8692(self): under_test = self.under_test_1 @@ -548,13 +592,14 @@ def _scope_validation_assertions(self, under_test, access_token): audience=self.token_builder_1.audience, scopes_anyof=[TEST_TOKEN_SCOPE_1, TEST_TOKEN_SCOPE_2], ) - with self.assertRaises(ScopeNotGrantedTokenException): + with self.assertRaises(ScopeNotGrantedTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, scopes_anyof=[TEST_TOKEN_SCOPE_3], ) + # self.assertRegexpMatches(str(raised1.exception), r"Access token did not grant sufficient scope.") def test_access_token_scope_validation__scope_required__rfc8693(self): # QE TC16 @@ -567,8 +612,9 @@ def test_access_token_scope_validation__scope_required__rfc8693(self): access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=[], ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: self._scope_validation_assertions(under_test, access_token) + self.assertEqual("No OAuth2 Scopes claim could be found in the access token", str(raised1.exception)) def test_access_token_scope_validation__scope_required__okta(self): # QE TC16 @@ -581,8 +627,9 @@ def test_access_token_scope_validation__scope_required__okta(self): access_token = self.token_builder_1.construct_oidc_access_token_okta( username=TEST_TOKEN_USER, requested_scopes=[], ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: self._scope_validation_assertions(under_test, access_token) + self.assertEqual("No OAuth2 Scopes claim could be found in the access token", str(raised1.exception)) def test_id_token_nonce(self): under_test = self.under_test_1 @@ -596,10 +643,11 @@ def test_id_token_nonce(self): under_test.validate_id_token( id_token_with_nonce, issuer=self.token_builder_1.issuer, client_id="test_client_id", nonce="12345" ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_id_token( id_token_with_nonce, issuer=self.token_builder_1.issuer, client_id="test_client_id", nonce="67890" ) + self.assertEqual("Token nonce did not match expected value", str(raised1.exception)) # TODO: See comments in the class. Unsure if we should make missing # nonce check's fatal when there is a nonce. It would be very strict. @@ -607,13 +655,15 @@ def test_id_token_nonce(self): id_token_with_nonce, issuer=self.token_builder_1.issuer, client_id="test_client_id", nonce=None ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised2: under_test.validate_id_token( id_token_without_nonce, issuer=self.token_builder_1.issuer, client_id="test_client_id", nonce="12345", ) + self.assertEqual('Token is missing the "nonce" claim (MissingRequiredClaimError)', str(raised2.exception)) + under_test.validate_id_token( id_token_with_nonce, issuer=self.token_builder_1.issuer, client_id="test_client_id" ) @@ -638,8 +688,11 @@ def test_validate_id_token_multiple_audiences(self): client_id="test_client_id", extra_claims={"aud": ["test_client_id", "extra_audience_1", "extra_audience_2"]}, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_id_token(id_token, issuer=self.token_builder_1.issuer, client_id="test_client_id") + self.assertEqual( + '"azp" claim mut be present when ID token contains multiple audiences.', str(raised1.exception) + ) # azp claim doesn't contain expected value when multiple audiences # are present @@ -648,8 +701,12 @@ def test_validate_id_token_multiple_audiences(self): client_id="test_client_id", extra_claims={"aud": ["test_client_id", "extra_audience_1", "extra_audience_2"], "azp": "mismatch_azp"}, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised2: under_test.validate_id_token(id_token, issuer=self.token_builder_1.issuer, client_id="test_client_id") + self.assertEqual( + 'ID token "azp" claim expected to match the client ID "test_client_id", but was "mismatch_azp"', + str(raised2.exception), + ) @freezegun.freeze_time(as_kwarg="frozen_time") def test_min_jwks_fetch_interval(self, frozen_time): @@ -665,12 +722,13 @@ def test_min_jwks_fetch_interval(self, frozen_time): self.assertEqual(0, under_test._jwks_client.jwks.call_count) # t1 - key miss loads keys - with self.assertRaises(UnknownSigningKeyTokenException): + with self.assertRaises(UnknownSigningKeyTokenException) as raised1: under_test.validate_token( token_unknown_signer, issuer=self.token_builder_2.issuer, audience=self.token_builder_2.audience, ) + self.assertEqual("Could not find signing key for key ID test_keypair2", str(raised1.exception)) self.assertEqual(1, under_test._jwks_client.jwks.call_count) # t2 - key hit should pull from hot cache. @@ -684,12 +742,14 @@ def test_min_jwks_fetch_interval(self, frozen_time): # t3 - Repeated key hits and misses inside the fetch interval should # not trigger a reload of the jwks verification keys for n in range(5): - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised_repeatedly: under_test.validate_token( token_unknown_signer, issuer=self.token_builder_2.issuer, audience=self.token_builder_2.audience, ) + self.assertEqual("Could not find signing key for key ID test_keypair2", str(raised_repeatedly.exception)) + under_test.validate_token( token_known_signer, issuer=self.token_builder_1.issuer, @@ -708,12 +768,13 @@ def test_min_jwks_fetch_interval(self, frozen_time): issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised3: under_test.validate_token( token_unknown_signer, issuer=self.token_builder_2.issuer, audience=self.token_builder_2.audience, ) + self.assertEqual("Could not find signing key for key ID test_keypair2", str(raised3.exception)) self.assertEqual(2, under_test._jwks_client.jwks.call_count) def test_untrusted_token_algorithm(self): @@ -722,12 +783,13 @@ def test_untrusted_token_algorithm(self): test_token = self.token_builder_3.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidAlgorithmTokenException): + with self.assertRaises(InvalidAlgorithmTokenException) as raised1: under_test.validate_token( test_token, issuer=self.token_builder_3.issuer, audience=self.token_builder_3.audience, ) + self.assertEqual("Unknown or unsupported token algorithm RS512", str(raised1.exception)) def test_jwks_endpoint_lists_unsupported_algorithm(self): # See CG-867 @@ -762,12 +824,13 @@ def test_jwks_endpoint_lists_unsupported_algorithm(self): access_token = self.token_builder_4.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(UnknownSigningKeyTokenException): + with self.assertRaises(UnknownSigningKeyTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_4.issuer, audience=self.token_builder_4.audience, ) + self.assertEqual("Could not find signing key for key ID test_keypair4", str(raised1.exception)) # def test_max_jwks_age(self): # # Feature not implemented. From 6f7463664659d523e8834aed88396822bc3251ff Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sun, 5 Oct 2025 09:55:16 -0700 Subject: [PATCH 27/33] flake fixes --- .../auth_clients/oidc/test_token_validator.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py index 7fb9631..30b83fe 100644 --- a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py +++ b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py @@ -148,7 +148,7 @@ def test_malformed_token_1(self): # QE TC6 - just some random garbage. under_test = self.under_test_1 test_token = secrets.token_bytes(2048) - with self.assertRaises(InvalidTokenException) as raised1: + with self.assertRaises(InvalidTokenException): # as raised1: under_test.validate_token( token_str=test_token, issuer=self.token_builder_1.issuer, @@ -157,7 +157,7 @@ def test_malformed_token_1(self): # Random garbage may throw different errors. # self.assertEqual("TBD", str(raised1.exception)) - with self.assertRaises(InvalidArgumentException) as raised2: + with self.assertRaises(InvalidArgumentException): # as raised2: TokenValidator.hazmat_unverified_decode(test_token) # Random garbage may throw different errors # self.assertEqual("TBD", str(raised2.exception)) @@ -166,7 +166,7 @@ def test_malformed_token_2(self): # QE TC6 - just some random garbage, but URL safe. under_test = self.under_test_1 test_token = secrets.token_urlsafe(2048) - with self.assertRaises(InvalidTokenException) as raised1: + with self.assertRaises(InvalidTokenException): # as raised1: under_test.validate_token( token_str=test_token, issuer=self.token_builder_1.issuer, @@ -175,7 +175,7 @@ def test_malformed_token_2(self): # Random garbage may throw different errors. # self.assertEqual("TBD", str(raised1.exception)) - with self.assertRaises(InvalidArgumentException) as raised2: + with self.assertRaises(InvalidArgumentException): # as raised2: TokenValidator.hazmat_unverified_decode(test_token) # Random garbage may throw different errors. # self.assertEqual("TBD", str(raised2.exception)) @@ -189,7 +189,7 @@ def test_malformed_token_3(self): str(jwt.utils.base64url_encode(secrets.token_bytes(256)), encoding="utf-8"), ) - with self.assertRaises(InvalidTokenException) as raised1: + with self.assertRaises(InvalidTokenException): # as raised1: under_test.validate_token( token_str=fake_jwt, issuer=self.token_builder_1.issuer, @@ -198,7 +198,7 @@ def test_malformed_token_3(self): # Random garbage may throw different errors. # self.assertEqual("TBD", str(raised1.exception)) - with self.assertRaises(InvalidArgumentException) as raised2: + with self.assertRaises(InvalidArgumentException): # as raised2: TokenValidator.hazmat_unverified_decode(fake_jwt) # Random garbage may throw different errors. # self.assertEqual("TBD", str(raised2.exception)) @@ -592,14 +592,13 @@ def _scope_validation_assertions(self, under_test, access_token): audience=self.token_builder_1.audience, scopes_anyof=[TEST_TOKEN_SCOPE_1, TEST_TOKEN_SCOPE_2], ) - with self.assertRaises(ScopeNotGrantedTokenException) as raised1: + with self.assertRaises(ScopeNotGrantedTokenException): under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, scopes_anyof=[TEST_TOKEN_SCOPE_3], ) - # self.assertRegexpMatches(str(raised1.exception), r"Access token did not grant sufficient scope.") def test_access_token_scope_validation__scope_required__rfc8693(self): # QE TC16 From 0fc7fa4bbb8b8fd4e48d63ad87ae58564ba0db1b Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sun, 5 Oct 2025 10:15:21 -0700 Subject: [PATCH 28/33] merge from minor tweaks branch --- .../auth_clients/oidc/test_token_validator.py | 136 +++++++++++++----- 1 file changed, 99 insertions(+), 37 deletions(-) diff --git a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py index 1543ddb..30b83fe 100644 --- a/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py +++ b/tests/test_planet_auth/unit/auth/auth_clients/oidc/test_token_validator.py @@ -120,48 +120,65 @@ def test_valid_access_token(self): def test_empty_access_token(self): # QE TC3, TC5 under_test = self.under_test_1 - with self.assertRaises(InvalidArgumentException): + with self.assertRaises(InvalidArgumentException) as raised1: under_test.validate_token( "", issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Cannot decode empty string as a token", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: under_test.validate_token( None, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Cannot decode empty string as a token", str(raised2.exception)) + + with self.assertRaises(InvalidArgumentException) as raised3: TokenValidator.hazmat_unverified_decode("") - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Not enough segments (DecodeError)", str(raised3.exception)) + + with self.assertRaises(InvalidArgumentException) as raised4: TokenValidator.hazmat_unverified_decode(None) + self.assertEqual("Invalid token type. Token must be a (DecodeError)", str(raised4.exception)) def test_malformed_token_1(self): # QE TC6 - just some random garbage. under_test = self.under_test_1 test_token = secrets.token_bytes(2048) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException): # as raised1: under_test.validate_token( token_str=test_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + # Random garbage may throw different errors. + # self.assertEqual("TBD", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException): # as raised2: TokenValidator.hazmat_unverified_decode(test_token) + # Random garbage may throw different errors + # self.assertEqual("TBD", str(raised2.exception)) def test_malformed_token_2(self): # QE TC6 - just some random garbage, but URL safe. under_test = self.under_test_1 test_token = secrets.token_urlsafe(2048) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException): # as raised1: under_test.validate_token( token_str=test_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + # Random garbage may throw different errors. + # self.assertEqual("TBD", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException): # as raised2: TokenValidator.hazmat_unverified_decode(test_token) + # Random garbage may throw different errors. + # self.assertEqual("TBD", str(raised2.exception)) def test_malformed_token_3(self): # QE TC6 - random garbage, but has JWT three dot structure. @@ -172,14 +189,19 @@ def test_malformed_token_3(self): str(jwt.utils.base64url_encode(secrets.token_bytes(256)), encoding="utf-8"), ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException): # as raised1: under_test.validate_token( token_str=fake_jwt, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + # Random garbage may throw different errors. + # self.assertEqual("TBD", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException): # as raised2: TokenValidator.hazmat_unverified_decode(fake_jwt) + # Random garbage may throw different errors. + # self.assertEqual("TBD", str(raised2.exception)) def test_malformed_token_4(self): # QE TC6 - random garbage, but looks more JWT like with encoded json. @@ -212,12 +234,14 @@ def _fake_token(header, body): {"fake_claim": "test claim value"}, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=fake_jwt, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + # Will random garbage always parse to this exact error? + self.assertEqual("Signature verification failed (InvalidSignatureError)", str(raised1.exception)) # This is good enough for an unverified decode. It's still invalid, which is # why you are careful with unverified decoded data. @@ -232,12 +256,13 @@ def test_malformed_token_missing_issuer(self): access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL, remove_claims=["iss"] ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + self.assertEqual('Token is missing the "iss" claim (MissingRequiredClaimError)', str(raised1.exception)) def test_altered_token(self): # QE TC10 @@ -272,12 +297,13 @@ def _alter_token(access_token, old_claim_value, new_claim_value): # Now, actually change the claim value, and look for the exception altered_access_token = _alter_token(access_token, "sensitive_value_A", "sensitive_value_B") - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( altered_access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + self.assertEqual("Signature verification failed (InvalidSignatureError)", str(raised1.exception)) def test_missing_signature_1(self): # QE TC11 - JWT without a signature, but with the "." @@ -307,12 +333,13 @@ def _bad_token(header, body): {"fake_claim": "test claim value"}, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=bad_jwt, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + self.assertEqual("Signature verification failed (InvalidSignatureError)", str(raised1.exception)) u_header, u_body, u_sig = TokenValidator.hazmat_unverified_decode(bad_jwt) self.assertIsNotNone(u_header) @@ -347,14 +374,17 @@ def _bad_token(header, body): {"fake_claim": "test claim value"}, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=bad_jwt, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Not enough segments (DecodeError)", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: TokenValidator.hazmat_unverified_decode(bad_jwt) + self.assertEqual("Not enough segments (DecodeError)", str(raised2.exception)) def test_empty_issuer_arg(self): under_test = self.under_test_1 @@ -367,18 +397,21 @@ def test_empty_issuer_arg(self): audience=self.token_builder_1.audience, ) self.assertEqual(TEST_TOKEN_USER, validated_claims["sub"]) - with self.assertRaises(InvalidArgumentException): + with self.assertRaises(InvalidArgumentException) as raised1: validated_claims = under_test.validate_token( access_token, issuer="", audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Cannot validate token with no required issuer provided", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: validated_claims = under_test.validate_token( access_token, issuer=None, audience=self.token_builder_1.audience, ) + self.assertEqual("Cannot validate token with no required issuer provided", str(raised2.exception)) def test_empty_audience_arg(self): under_test = self.under_test_1 @@ -391,18 +424,21 @@ def test_empty_audience_arg(self): audience=self.token_builder_1.audience, ) self.assertEqual(TEST_TOKEN_USER, validated_claims["sub"]) - with self.assertRaises(InvalidArgumentException): + with self.assertRaises(InvalidArgumentException) as raised1: validated_claims = under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience="", ) - with self.assertRaises(InvalidArgumentException): + self.assertEqual("Cannot validate token with no required audience provided", str(raised1.exception)) + + with self.assertRaises(InvalidArgumentException) as raised2: validated_claims = under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=None, ) + self.assertEqual("Cannot validate token with no required audience provided", str(raised2.exception)) def test_access_token_unknown_signing_key(self): # QE TC12 @@ -410,12 +446,13 @@ def test_access_token_unknown_signing_key(self): access_token = self.token_builder_2.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(UnknownSigningKeyTokenException): + with self.assertRaises(UnknownSigningKeyTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_2.issuer, audience=self.token_builder_2.audience, ) + self.assertEqual("Could not find signing key for key ID test_keypair2", str(raised1.exception)) def test_access_token_issuer_mismatch(self): # QE TC15 untrusted issuer. (See also multi-validator tests) @@ -423,12 +460,13 @@ def test_access_token_issuer_mismatch(self): access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer + "_make_it_mismatch", audience=self.token_builder_1.audience, ) + self.assertEqual("Invalid issuer (InvalidIssuerError)", str(raised1.exception)) def test_access_token_incorrect_audience(self): # QE TC17 @@ -436,12 +474,13 @@ def test_access_token_incorrect_audience(self): access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience + "_make_it_mismatch", ) + self.assertEqual("Audience doesn't match (InvalidAudienceError)", str(raised1.exception)) def test_access_token_multiple_audiences(self): # QE TC17 @@ -461,10 +500,11 @@ def test_access_token_multiple_audiences(self): under_test.validate_token( token_str=access_token, issuer=self.token_builder_1.issuer, audience="extra_audience_2" ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( token_str=access_token, issuer=self.token_builder_1.issuer, audience="extra_audience_3" ) + self.assertEqual("Audience doesn't match (InvalidAudienceError)", str(raised1.exception)) @freezegun.freeze_time(as_kwarg="frozen_time") def test_access_token_expired(self, frozen_time): @@ -474,25 +514,29 @@ def test_access_token_expired(self, frozen_time): username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=3 ) frozen_time.tick(5) - with self.assertRaises(ExpiredTokenException): + with self.assertRaises(ExpiredTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) + self.assertEqual("Signature has expired (ExpiredSignatureError)", str(raised1.exception)) def test_access_token_missing_claim(self): under_test = self.under_test_1 access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, required_claims=["missing_claim_1", "missing_claim_2"], ) + self.assertEqual( + 'Token is missing the "missing_claim_1" claim (MissingRequiredClaimError)', str(raised1.exception) + ) def test_access_token_scope_validation__no_scopes_required__rfc8692(self): under_test = self.under_test_1 @@ -567,8 +611,9 @@ def test_access_token_scope_validation__scope_required__rfc8693(self): access_token = self.token_builder_1.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=[], ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: self._scope_validation_assertions(under_test, access_token) + self.assertEqual("No OAuth2 Scopes claim could be found in the access token", str(raised1.exception)) def test_access_token_scope_validation__scope_required__okta(self): # QE TC16 @@ -581,8 +626,9 @@ def test_access_token_scope_validation__scope_required__okta(self): access_token = self.token_builder_1.construct_oidc_access_token_okta( username=TEST_TOKEN_USER, requested_scopes=[], ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: self._scope_validation_assertions(under_test, access_token) + self.assertEqual("No OAuth2 Scopes claim could be found in the access token", str(raised1.exception)) def test_id_token_nonce(self): under_test = self.under_test_1 @@ -596,10 +642,11 @@ def test_id_token_nonce(self): under_test.validate_id_token( id_token_with_nonce, issuer=self.token_builder_1.issuer, client_id="test_client_id", nonce="12345" ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_id_token( id_token_with_nonce, issuer=self.token_builder_1.issuer, client_id="test_client_id", nonce="67890" ) + self.assertEqual("Token nonce did not match expected value", str(raised1.exception)) # TODO: See comments in the class. Unsure if we should make missing # nonce check's fatal when there is a nonce. It would be very strict. @@ -607,13 +654,15 @@ def test_id_token_nonce(self): id_token_with_nonce, issuer=self.token_builder_1.issuer, client_id="test_client_id", nonce=None ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised2: under_test.validate_id_token( id_token_without_nonce, issuer=self.token_builder_1.issuer, client_id="test_client_id", nonce="12345", ) + self.assertEqual('Token is missing the "nonce" claim (MissingRequiredClaimError)', str(raised2.exception)) + under_test.validate_id_token( id_token_with_nonce, issuer=self.token_builder_1.issuer, client_id="test_client_id" ) @@ -638,8 +687,11 @@ def test_validate_id_token_multiple_audiences(self): client_id="test_client_id", extra_claims={"aud": ["test_client_id", "extra_audience_1", "extra_audience_2"]}, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised1: under_test.validate_id_token(id_token, issuer=self.token_builder_1.issuer, client_id="test_client_id") + self.assertEqual( + '"azp" claim mut be present when ID token contains multiple audiences.', str(raised1.exception) + ) # azp claim doesn't contain expected value when multiple audiences # are present @@ -648,8 +700,12 @@ def test_validate_id_token_multiple_audiences(self): client_id="test_client_id", extra_claims={"aud": ["test_client_id", "extra_audience_1", "extra_audience_2"], "azp": "mismatch_azp"}, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised2: under_test.validate_id_token(id_token, issuer=self.token_builder_1.issuer, client_id="test_client_id") + self.assertEqual( + 'ID token "azp" claim expected to match the client ID "test_client_id", but was "mismatch_azp"', + str(raised2.exception), + ) @freezegun.freeze_time(as_kwarg="frozen_time") def test_min_jwks_fetch_interval(self, frozen_time): @@ -665,12 +721,13 @@ def test_min_jwks_fetch_interval(self, frozen_time): self.assertEqual(0, under_test._jwks_client.jwks.call_count) # t1 - key miss loads keys - with self.assertRaises(UnknownSigningKeyTokenException): + with self.assertRaises(UnknownSigningKeyTokenException) as raised1: under_test.validate_token( token_unknown_signer, issuer=self.token_builder_2.issuer, audience=self.token_builder_2.audience, ) + self.assertEqual("Could not find signing key for key ID test_keypair2", str(raised1.exception)) self.assertEqual(1, under_test._jwks_client.jwks.call_count) # t2 - key hit should pull from hot cache. @@ -684,12 +741,14 @@ def test_min_jwks_fetch_interval(self, frozen_time): # t3 - Repeated key hits and misses inside the fetch interval should # not trigger a reload of the jwks verification keys for n in range(5): - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised_repeatedly: under_test.validate_token( token_unknown_signer, issuer=self.token_builder_2.issuer, audience=self.token_builder_2.audience, ) + self.assertEqual("Could not find signing key for key ID test_keypair2", str(raised_repeatedly.exception)) + under_test.validate_token( token_known_signer, issuer=self.token_builder_1.issuer, @@ -708,12 +767,13 @@ def test_min_jwks_fetch_interval(self, frozen_time): issuer=self.token_builder_1.issuer, audience=self.token_builder_1.audience, ) - with self.assertRaises(InvalidTokenException): + with self.assertRaises(InvalidTokenException) as raised3: under_test.validate_token( token_unknown_signer, issuer=self.token_builder_2.issuer, audience=self.token_builder_2.audience, ) + self.assertEqual("Could not find signing key for key ID test_keypair2", str(raised3.exception)) self.assertEqual(2, under_test._jwks_client.jwks.call_count) def test_untrusted_token_algorithm(self): @@ -722,12 +782,13 @@ def test_untrusted_token_algorithm(self): test_token = self.token_builder_3.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(InvalidAlgorithmTokenException): + with self.assertRaises(InvalidAlgorithmTokenException) as raised1: under_test.validate_token( test_token, issuer=self.token_builder_3.issuer, audience=self.token_builder_3.audience, ) + self.assertEqual("Unknown or unsupported token algorithm RS512", str(raised1.exception)) def test_jwks_endpoint_lists_unsupported_algorithm(self): # See CG-867 @@ -762,12 +823,13 @@ def test_jwks_endpoint_lists_unsupported_algorithm(self): access_token = self.token_builder_4.construct_oidc_access_token_rfc8693( username=TEST_TOKEN_USER, requested_scopes=TEST_TOKEN_SCOPES, ttl=TEST_TOKEN_TTL ) - with self.assertRaises(UnknownSigningKeyTokenException): + with self.assertRaises(UnknownSigningKeyTokenException) as raised1: under_test.validate_token( access_token, issuer=self.token_builder_4.issuer, audience=self.token_builder_4.audience, ) + self.assertEqual("Could not find signing key for key ID test_keypair4", str(raised1.exception)) # def test_max_jwks_age(self): # # Feature not implemented. From 53b8cd98a2397bd52c1119ca6ef82c6a100ef0cf Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sun, 5 Oct 2025 10:21:13 -0700 Subject: [PATCH 29/33] rename tests --- tests/test_planet_auth/unit/auth/test_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_planet_auth/unit/auth/test_auth.py b/tests/test_planet_auth/unit/auth/test_auth.py index 5bef856..62dfcfa 100644 --- a/tests/test_planet_auth/unit/auth/test_auth.py +++ b/tests/test_planet_auth/unit/auth/test_auth.py @@ -212,7 +212,7 @@ def __init__(self, **kwargs): super().__init__(**kwargs) -class AuthTestNew(unittest.TestCase): +class AuthTestEnsureAuthenticatorIsReady(unittest.TestCase): """Tests for Auth.ensure_request_authenticator_is_ready()""" def _create_auth_with_state( From cfa5e72f30516872656629f8b7be058b1f5d4276 Mon Sep 17 00:00:00 2001 From: Carl Alexander Adams Date: Sun, 5 Oct 2025 15:59:58 -0700 Subject: [PATCH 30/33] fix typo --- src/planet_auth/oidc/token_validator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/planet_auth/oidc/token_validator.py b/src/planet_auth/oidc/token_validator.py index 0bf83bd..44b6f19 100644 --- a/src/planet_auth/oidc/token_validator.py +++ b/src/planet_auth/oidc/token_validator.py @@ -260,7 +260,7 @@ def validate_token( @InvalidArgumentException.recast(jwt.exceptions.DecodeError) def hazmat_unverified_decode(token_str) -> Tuple[Dict, Dict, Any]: """ - Decide a JWT without verifying the signature or any claims. + Decode a JWT without verifying the signature or any claims. !!! Warning Treat unverified token claims with extreme caution. From 36d63716c4ad29ebea55ea3690e6c2c6177e3ddd Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Tue, 7 Oct 2025 12:48:59 -0700 Subject: [PATCH 31/33] update comments --- src/planet_auth/auth.py | 3 --- src/planet_auth/oidc/request_authenticator.py | 4 +++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/planet_auth/auth.py b/src/planet_auth/auth.py index 0a62e05..bf35b5e 100644 --- a/src/planet_auth/auth.py +++ b/src/planet_auth/auth.py @@ -210,9 +210,6 @@ def _is_expired() -> bool: auth_logger.warning( msg=f"Failed to refresh expired credentials (Error: {str(e)}). Attempting interactive login." ) - # Fall through to case #4 below. - # self.login(allow_open_browser=allow_open_browser, allow_tty_prompt=allow_tty_prompt) - # return # Case #4 above. self.login(allow_open_browser=allow_open_browser, allow_tty_prompt=allow_tty_prompt) diff --git a/src/planet_auth/oidc/request_authenticator.py b/src/planet_auth/oidc/request_authenticator.py index 2cf9039..12b2977 100644 --- a/src/planet_auth/oidc/request_authenticator.py +++ b/src/planet_auth/oidc/request_authenticator.py @@ -84,7 +84,9 @@ def _refresh(self): def _refresh_needed(self, check_time: Optional[int] = None) -> bool: if self._token_body is None: - # Always consider no token in need of a refresh. + # 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: From 476f05d9a259eeb43135b4f5b06d25a0f83435ff Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Tue, 7 Oct 2025 18:26:20 -0700 Subject: [PATCH 32/33] update changelog --- docs/changelog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.md b/docs/changelog.md index 5a090e6..826645b 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -6,6 +6,10 @@ 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 From 73176aa20c55ada7ec8cf9928b720726355a5d3d Mon Sep 17 00:00:00 2001 From: "Carl A. Adams" Date: Wed, 8 Oct 2025 10:08:03 -0700 Subject: [PATCH 33/33] update grammar --- src/planet_auth/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/planet_auth/auth.py b/src/planet_auth/auth.py index bf35b5e..216c417 100644 --- a/src/planet_auth/auth.py +++ b/src/planet_auth/auth.py @@ -131,7 +131,7 @@ def ensure_request_authenticator_is_ready( This can be more complex than it sounds given the variations in the capabilities of authentication clients and possible session states. - Client may be initialized with active sessions, initialized with stale + 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