From b6eb71abb5f3c2743cde260d5cd371492d753781 Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Fri, 18 Oct 2024 08:47:36 -0500 Subject: [PATCH 01/15] Reduce the prevalence of `None` as a return value --- ...kurtmckee_eliminate_none_return_values.rst | 15 ++++ .../authentication.py | 78 +++++++++---------- .../flask/apt_blueprint.py | 4 - tests/test_authentication.py | 13 +--- 4 files changed, 54 insertions(+), 56 deletions(-) create mode 100644 changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst diff --git a/changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst b/changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst new file mode 100644 index 0000000..f77b231 --- /dev/null +++ b/changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst @@ -0,0 +1,15 @@ +Changes +------- + +- ``AuthState.introspect_token()`` will no longer return ``None`` if the token is not active. + + Instead, a new exception, ``InactiveTokenError``, will be raised. + ``InactiveTokenError`` is a subclass of ``ValueError``. + + Existing code that calls ``AuthState.introspect_token()`` no longer returns ``None``, either, + but will instead raise ``ValueError``, a subclass of ``ValueError``, + or a ``globus_sdk.GlobusAPIError``: + + * ``AuthState.get_authorizer_for_scope`` + * ``AuthState.effective_identity`` + * ``AuthState.identities`` diff --git a/src/globus_action_provider_tools/authentication.py b/src/globus_action_provider_tools/authentication.py index 00ab490..7d858a6 100644 --- a/src/globus_action_provider_tools/authentication.py +++ b/src/globus_action_provider_tools/authentication.py @@ -48,6 +48,10 @@ def __init__( ) +class InactiveTokenError(ValueError): + """Indicates that the token is not valid (its 'active' field is False).""" + + class AuthState: # Cache for introspection operations, max lifetime: 30 seconds introspect_cache: TypedTTLCache[globus_sdk.GlobusHTTPResponse] = TypedTTLCache( @@ -95,7 +99,7 @@ def _cached_introspect_call(self) -> GlobusHTTPResponse: introspect_result = AuthState.introspect_cache.get(self._token_hash) if introspect_result is not None: log.debug( - f"Using cached introspection introspect_resultonse for " + f"Using cached introspection response for token_hash={self._token_hash}" ) return introspect_result @@ -107,14 +111,8 @@ def _cached_introspect_call(self) -> GlobusHTTPResponse: return introspect_result - def introspect_token(self) -> GlobusHTTPResponse | None: + def introspect_token(self) -> GlobusHTTPResponse: introspect_result = self._cached_introspect_call() - - # FIXME: convert this to an exception, rather than 'None' - # the exception could be raised in _verify_introspect_result - if not introspect_result["active"]: - return None - self._verify_introspect_result(introspect_result) return introspect_result @@ -122,6 +120,10 @@ def _verify_introspect_result(self, introspect_result: GlobusHTTPResponse) -> No """ A helper which checks token introspect properties and raises exceptions on failure. """ + + if not introspect_result["active"]: + raise InactiveTokenError("The token is invalid or has been revoked") + # validate scopes, ensuring that the token provided accords with the service's # notion of what operations exist and are supported scopes = set(introspect_result.get("scope", "").split()) @@ -129,18 +131,14 @@ def _verify_introspect_result(self, introspect_result: GlobusHTTPResponse) -> No raise InvalidTokenScopesError(self.expected_scopes, frozenset(scopes)) @property - def effective_identity(self) -> str | None: + def effective_identity(self) -> str: tkn_details = self.introspect_token() - if tkn_details is None: - return None effective = identity_principal(tkn_details["sub"]) return effective @property def identities(self) -> frozenset[str]: tkn_details = self.introspect_token() - if tkn_details is None: - return frozenset() return frozenset(map(identity_principal, tkn_details["identity_set"])) @property @@ -221,7 +219,7 @@ def get_authorizer_for_scope( self, scope: str, required_authorizer_expiration_time: int = 60, - ) -> RefreshTokenAuthorizer | AccessTokenAuthorizer | None: + ) -> RefreshTokenAuthorizer | AccessTokenAuthorizer: """ Get dependent tokens for the caller's token, then retrieve token data for the requested scope and attempt to build an authorizer from that data. @@ -240,39 +238,33 @@ def get_authorizer_for_scope( If the access token is or will be expired within the ``required_authorizer_expiration_time``, it is treated as a failure. """ - # this block is intentionally nested under a single exception handler - # if either dependent token callout fails, the entire operation is treated as a failure - try: - had_cached_value, dependent_tokens = self._get_cached_dependent_tokens() - # if the dependent token data (which could have been cached) failed to meet - # the requirements... + had_cached_value, dependent_tokens = self._get_cached_dependent_tokens() + + # if the dependent token data (which could have been cached) failed to meet + # the requirements... + if not self._dependent_token_response_satisfies_scope_request( + dependent_tokens, scope, required_authorizer_expiration_time + ): + # if there was no cached value, we just got fresh dependent tokens + # to do this work but they weren't sufficient + # there's no reason to expect new tokens would do better + # fail, but do not clear the cache -- it could be satisfactory + # for some other scope request + if not had_cached_value: + raise ValueError("No matching cached dependent tokens found") + + # otherwise, the cached value was bad -- fetch and check again, + # by clearing the cache and asking for the same data + del self.dependent_tokens_cache[self._dependent_token_cache_key] + _, dependent_tokens = self._get_cached_dependent_tokens() + + # check again against requirements -- + # this is guaranteed to be fresh data if not self._dependent_token_response_satisfies_scope_request( dependent_tokens, scope, required_authorizer_expiration_time ): - # if there was no cached value, we just got fresh dependent tokens to do this work - # and they weren't sufficient - # there's no reason to expect new tokens would do better - # fail, but do not clear the cache -- it could be satisfactory for some other scope request - if not had_cached_value: - return None # FIXME: raise an exception instead - - # otherwise, the cached value was bad -- fetch and check again, by clearing the cache and - # asking for the same data - del self.dependent_tokens_cache[self._dependent_token_cache_key] - _, dependent_tokens = self._get_cached_dependent_tokens() - - # check again against requirements -- this is guaranteed to be fresh data - if not self._dependent_token_response_satisfies_scope_request( - dependent_tokens, scope, required_authorizer_expiration_time - ): - return None # FIXME: raise an exception instead - except globus_sdk.AuthAPIError: - log.warning( - f"Unable to create GlobusAuthorizer for scope {scope}. Using 'None'", - exc_info=True, - ) - return None + raise ValueError("Refreshed dependent tokens still don't match") token_data = dependent_tokens.by_scopes[scope] diff --git a/src/globus_action_provider_tools/flask/apt_blueprint.py b/src/globus_action_provider_tools/flask/apt_blueprint.py index 99f3d34..672f76d 100644 --- a/src/globus_action_provider_tools/flask/apt_blueprint.py +++ b/src/globus_action_provider_tools/flask/apt_blueprint.py @@ -543,7 +543,3 @@ def _check_token(self) -> None: return g.auth_state = self.state_builder.build_from_request() - if g.auth_state.effective_identity is None: - current_app.logger.info( - f"Request failed authentication due to: {g.auth_state.errors}" - ) diff --git a/tests/test_authentication.py b/tests/test_authentication.py index 9a78345..49d849a 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -101,24 +101,19 @@ def test_auth_state_caching_across_instances(auth_state, freeze_time, mocked_res def test_invalid_grant_exception(auth_state): load_response("token-introspect", case="success") load_response("token", case="invalid-grant") - assert auth_state.get_authorizer_for_scope("doesn't matter") is None + with pytest.raises(globus_sdk.GlobusAPIError): + auth_state.get_authorizer_for_scope("doesn't matter") def test_dependent_token_callout_500_fails_dependent_authorization(auth_state): - """ - On a 5xx response, getting an authorizer fails. - - FIXME: currently this simply emits 'None' -- in the future the error should propagate - """ + """On a 5xx response, getting an authorizer fails.""" RegisteredResponse( service="auth", path="/v2/oauth2/token", method="POST", status=500 ).add() - assert ( + with pytest.raises(globus_sdk.GlobusAPIError): auth_state.get_authorizer_for_scope( "urn:globus:auth:scope:groups.api.globus.org:view_my_groups_and_memberships" ) - is None - ) def test_dependent_token_callout_success_fixes_bad_cache(auth_state): From a31111348ab1efa3edfc9ba6d556b6c7c55c90c4 Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Fri, 18 Oct 2024 12:00:09 -0500 Subject: [PATCH 02/15] Incorporate PR feedback Co-authored-by: Stephen Rosen <1300022+sirosen@users.noreply.github.com> --- ...8_082221_kurtmckee_eliminate_none_return_values.rst | 3 +-- src/globus_action_provider_tools/authentication.py | 10 +++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst b/changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst index f77b231..e8ac24d 100644 --- a/changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst +++ b/changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst @@ -7,8 +7,7 @@ Changes ``InactiveTokenError`` is a subclass of ``ValueError``. Existing code that calls ``AuthState.introspect_token()`` no longer returns ``None``, either, - but will instead raise ``ValueError``, a subclass of ``ValueError``, - or a ``globus_sdk.GlobusAPIError``: + but will instead raise ``ValueError`` (or a subclass) or a ``globus_sdk.GlobusAPIError``: * ``AuthState.get_authorizer_for_scope`` * ``AuthState.effective_identity`` diff --git a/src/globus_action_provider_tools/authentication.py b/src/globus_action_provider_tools/authentication.py index 7d858a6..2ed6e28 100644 --- a/src/globus_action_provider_tools/authentication.py +++ b/src/globus_action_provider_tools/authentication.py @@ -122,7 +122,7 @@ def _verify_introspect_result(self, introspect_result: GlobusHTTPResponse) -> No """ if not introspect_result["active"]: - raise InactiveTokenError("The token is invalid or has been revoked") + raise InactiveTokenError("The token is invalid.") # validate scopes, ensuring that the token provided accords with the service's # notion of what operations exist and are supported @@ -239,7 +239,7 @@ def get_authorizer_for_scope( ``required_authorizer_expiration_time``, it is treated as a failure. """ - had_cached_value, dependent_tokens = self._get_cached_dependent_tokens() + retrieved_from_cache, dependent_tokens = self._get_cached_dependent_tokens() # if the dependent token data (which could have been cached) failed to meet # the requirements... @@ -251,8 +251,8 @@ def get_authorizer_for_scope( # there's no reason to expect new tokens would do better # fail, but do not clear the cache -- it could be satisfactory # for some other scope request - if not had_cached_value: - raise ValueError("No matching cached dependent tokens found") + if not retrieved_from_cache: + raise ValueError("Dependent tokens do not match request.") # otherwise, the cached value was bad -- fetch and check again, # by clearing the cache and asking for the same data @@ -264,7 +264,7 @@ def get_authorizer_for_scope( if not self._dependent_token_response_satisfies_scope_request( dependent_tokens, scope, required_authorizer_expiration_time ): - raise ValueError("Refreshed dependent tokens still don't match") + raise ValueError("Dependent tokens do not match request.") token_data = dependent_tokens.by_scopes[scope] From b3bc2160edda8262d386a744b406a026bc32180a Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Fri, 25 Oct 2024 17:28:45 -0500 Subject: [PATCH 03/15] Refactor groups caching for simplicity and perf This order of evaluation is cleaner/simpler to reason about and has improved performance in specific cases, when the cache is warm and a dependent token is absent (e.g., the group cache was populated at the end of the lifetime for a cached dependent token). This changeset refrains from changing the established behavior of converting groups API errors to the empty group set, but it does make this behavior more visible. --- ..._172707_sirosen_improve_groups_caching.rst | 7 ++ .../authentication.py | 72 +++++++------------ tests/api-fixtures/groups-my_groups.yaml | 7 ++ tests/test_authentication.py | 24 +++++++ 4 files changed, 64 insertions(+), 46 deletions(-) create mode 100644 changelog.d/20241025_172707_sirosen_improve_groups_caching.rst diff --git a/changelog.d/20241025_172707_sirosen_improve_groups_caching.rst b/changelog.d/20241025_172707_sirosen_improve_groups_caching.rst new file mode 100644 index 0000000..f54feb5 --- /dev/null +++ b/changelog.d/20241025_172707_sirosen_improve_groups_caching.rst @@ -0,0 +1,7 @@ +Changes +------- + +- Group caching behavior in the ``AuthState`` class has been improved to ensure + that the cache is checked before any external operations (e.g., dependent + token callouts) are required. The cache now uses the token hash as its key, + rather than a dependent token. diff --git a/src/globus_action_provider_tools/authentication.py b/src/globus_action_provider_tools/authentication.py index 8663d9f..93b3530 100644 --- a/src/globus_action_provider_tools/authentication.py +++ b/src/globus_action_provider_tools/authentication.py @@ -10,10 +10,7 @@ from globus_sdk import ( AccessTokenAuthorizer, ConfidentialAppAuthClient, - GlobusAPIError, - GlobusError, GlobusHTTPResponse, - GroupsClient, RefreshTokenAuthorizer, ) @@ -81,7 +78,7 @@ def __init__( self.expected_scopes = expected_scopes self.errors: list[Exception] = [] - self._groups_client: GroupsClient | None = None + self._groups_client: globus_sdk.GroupsClient | None = None @functools.cached_property def _token_hash(self) -> str: @@ -147,44 +144,33 @@ def principals(self) -> frozenset[str]: @property def groups(self) -> frozenset[str]: - try: - groups_client = self._get_groups_client() - except (GlobusAPIError, KeyError, ValueError) as err: - # Only warning level, because this could be normal state of - # affairs for a system that doesn't use or care about groups. - log.warning( - "Unable to determine groups membership. Setting groups to {}", - exc_info=True, - ) - self.errors.append(err) - return frozenset() - else: + group_set: frozenset[str] | None = self.group_membership_cache.get( + self._token_hash + ) + if group_set is None: try: - groups_token = groups_client.authorizer.access_token - except AttributeError as err: - log.error("Missing access token to use for groups service") + groups_client = self._get_groups_client() + except (globus_sdk.GlobusAPIError, KeyError, ValueError) as err: + # Only warning level, because this could be normal state of + # affairs for a system that doesn't use or care about groups. + log.warning( + "Unable to determine groups membership. Setting groups to {}", + exc_info=True, + ) self.errors.append(err) return frozenset() - safe_groups_token = groups_token[-7:] - groups_set = AuthState.group_membership_cache.get(groups_token) - if groups_set is not None: - log.info( - f"Using cached group membership for groups token ***{safe_groups_token}" + + try: + group_data = groups_client.get_my_groups() + except globus_sdk.GlobusAPIError: + log.warning( + "failed to get groups, treating groups as '{}'", exc_info=True ) - return groups_set + return frozenset() - try: - log.info(f"Querying groups for groups token ***{safe_groups_token}") - groups = groups_client.get_my_groups() - except GlobusError as err: - log.exception("Error getting groups", exc_info=True) - self.errors.append(err) - return frozenset() - else: - groups_set = frozenset(map(group_principal, (grp["id"] for grp in groups))) - log.info(f"Caching groups for token **{safe_groups_token}") - AuthState.group_membership_cache[groups_token] = groups_set - return groups_set + group_set = frozenset(group_principal(g["id"]) for g in group_data) + self.group_membership_cache[self._token_hash] = group_set + return group_set def get_dependent_tokens( self, *, bypass_cache_lookup: bool = False @@ -335,19 +321,13 @@ def _dependent_token_response_satisfies_scope_request( return access_token is not None - def _get_groups_client(self) -> GroupsClient: + def _get_groups_client(self) -> globus_sdk.GroupsClient: if self._groups_client is not None: return self._groups_client authorizer = self.get_authorizer_for_scope( - GroupsClient.scopes.view_my_groups_and_memberships + globus_sdk.GroupsClient.scopes.view_my_groups_and_memberships ) - if authorizer is None: - raise ValueError( - "Unable to get authorizer for " - + GroupsClient.scopes.view_my_groups_and_memberships - ) - - self._groups_client = GroupsClient(authorizer=authorizer) + self._groups_client = globus_sdk.GroupsClient(authorizer=authorizer) return self._groups_client @staticmethod diff --git a/tests/api-fixtures/groups-my_groups.yaml b/tests/api-fixtures/groups-my_groups.yaml index 7545e25..ce049fc 100644 --- a/tests/api-fixtures/groups-my_groups.yaml +++ b/tests/api-fixtures/groups-my_groups.yaml @@ -42,3 +42,10 @@ success: ], }, ] + +failure: + service: groups + path: /groups/my_groups + method: GET + json: {"message": "Internal server error."} + status: 500 diff --git a/tests/test_authentication.py b/tests/test_authentication.py index 896ff4e..fd8886b 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -179,3 +179,27 @@ def test_required_scopes_may_be_a_subset_of_token_scopes(): auth_state_instance = get_auth_state_instance(["expected-scope"]) load_response("token-introspect", case="success") auth_state_instance.introspect_token() + + +def test_no_groups_client_is_constructed_when_cache_is_warm(auth_state): + """ + Confirm that if the group cache is warm, then the AuthState will refer to it without + even trying to collect credentials to callout to groups. + """ + load_response("token", case="success") + load_response("token-introspect", case="success") + load_response("groups-my_groups", case="failure") + + # put a mock in place so that we can see whether or not a client was built + auth_state._get_groups_client = mock.Mock() + + # write a value directly into the cache + auth_state.group_membership_cache[auth_state._token_hash] = frozenset() + + # now, fetch the groups property -- it should populate properly with an empty set + # even though there would be an error if we called out to groups + assert len(auth_state.groups) == 0 + + # finally, confirm via our mock that there was no attempt to instantiate a + # groups client + auth_state._get_groups_client.assert_not_called() From df32294a98cf7c283933958bdd2503dbabb17ffb Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Tue, 29 Oct 2024 14:04:09 -0500 Subject: [PATCH 04/15] Fix erroneous use of dependent refresh tokens (#184) * Fix erroneous use of dependent refresh tokens The previous code (here, almost entirely removed) requested dependent refresh tokens unconditionally. These were then stored in a 47 hour cache, and there was never an update to the cache to refresh these credentials. Access tokens last for 48 hours, meaning that with the prior cache parameters, it was not realistically possible to reach the point of requiring a refresh via expiration. (A refresh triggered by token revocation would fail, and therefore is not worthy of special attention.) Similar to the impossibility of a normal refresh, the timing requirement expressed by the `required_authorizer_expiration_time` parameter was more or less impossible to trigger in normal usage. With the default value of 60 seconds, the only way it could fire requires an hour to pass between the request being received and the dependent token for that request being retrieved. Therefore, this parameter is no longer checked, and emits a DeprecationWarning if used explicitly. * Update src/globus_action_provider_tools/authentication.py Co-authored-by: Kurt McKee * Add test for deprecated param Confirm that 'required_authorizer_expiration_time' causes a deprecation warning to be emitted. --------- Co-authored-by: Kurt McKee --- ...5333_sirosen_do_not_use_refresh_tokens.rst | 14 +++ .../authentication.py | 101 ++++-------------- tests/test_authentication.py | 19 +++- 3 files changed, 49 insertions(+), 85 deletions(-) create mode 100644 changelog.d/20241025_185333_sirosen_do_not_use_refresh_tokens.rst diff --git a/changelog.d/20241025_185333_sirosen_do_not_use_refresh_tokens.rst b/changelog.d/20241025_185333_sirosen_do_not_use_refresh_tokens.rst new file mode 100644 index 0000000..1e35e76 --- /dev/null +++ b/changelog.d/20241025_185333_sirosen_do_not_use_refresh_tokens.rst @@ -0,0 +1,14 @@ +Bugfixes +-------- + +- Action Provider Tools no longer requests Dependent Refresh Tokens in + contexts where Access Tokens are sufficient. As a result of this fix, the + AuthState dependent token cache will never contain dependent refresh tokens. + +Deprecations +------------ + +- The ``required_authorizer_expiration_time`` parameter to + ``get_authorizer_for_scope`` is deprecated. Given token expiration and + caching lifetimes, it was not possible for this parameter to have any effect + based on its prior documented usage. diff --git a/src/globus_action_provider_tools/authentication.py b/src/globus_action_provider_tools/authentication.py index 93b3530..9450055 100644 --- a/src/globus_action_provider_tools/authentication.py +++ b/src/globus_action_provider_tools/authentication.py @@ -3,7 +3,7 @@ import functools import hashlib import logging -import time +import warnings from typing import Iterable import globus_sdk @@ -11,7 +11,6 @@ AccessTokenAuthorizer, ConfidentialAppAuthClient, GlobusHTTPResponse, - RefreshTokenAuthorizer, ) from .utils import TypedTTLCache @@ -56,7 +55,7 @@ class AuthState: ) # Cache for dependent tokens, max lifetime: 47 hours: a bit less than the 48 hours - # until a refresh would be required anyway + # for which an access token is valid dependent_tokens_cache: TypedTTLCache[globus_sdk.OAuthDependentTokenResponse] = ( TypedTTLCache(maxsize=100, ttl=47 * 3600) ) @@ -204,8 +203,8 @@ def get_dependent_tokens( def get_authorizer_for_scope( self, scope: str, - required_authorizer_expiration_time: int = 60, - ) -> RefreshTokenAuthorizer | AccessTokenAuthorizer: + required_authorizer_expiration_time: int | None = None, + ) -> AccessTokenAuthorizer: """ Get dependent tokens for the caller's token, then retrieve token data for the requested scope and attempt to build an authorizer from that data. @@ -213,30 +212,25 @@ def get_authorizer_for_scope( The class caches dependent token results, regardless of whether or not building authorizers succeeds. - .. warning:: - - This call converts *all* errors to `None` results. - - If a dependent refresh token is available in the response, a - RefreshTokenAuthorizer will be built and returned. - Otherwise, this will attempt to build an AccessTokenAuthorizer. - - If the access token is or will be expired within the - ``required_authorizer_expiration_time``, it is treated as a failure. + :param scope: The scope for which an authorizer is being requested + :param required_authorizer_expiration_time: Deprecated parameter. Has no effect. """ + if required_authorizer_expiration_time is not None: + warnings.warn( + "`required_authorizer_expiration_time` has no effect and will be removed in a future version.", + DeprecationWarning, + stacklevel=2, + ) retrieved_from_cache, dependent_tokens = self._get_cached_dependent_tokens() # if the dependent token data (which could have been cached) failed to meet - # the requirements... - if not self._dependent_token_response_satisfies_scope_request( - dependent_tokens, scope, required_authorizer_expiration_time - ): + # the scope requirement... + if scope not in dependent_tokens.by_scopes: # if there was no cached value, we just got fresh dependent tokens - # to do this work but they weren't sufficient + # to do this work but the scope was missing # there's no reason to expect new tokens would do better - # fail, but do not clear the cache -- it could be satisfactory - # for some other scope request + # fail, but do not clear the cache if not retrieved_from_cache: raise ValueError("Dependent tokens do not match request.") @@ -245,25 +239,12 @@ def get_authorizer_for_scope( del self.dependent_tokens_cache[self._dependent_token_cache_key] _, dependent_tokens = self._get_cached_dependent_tokens() - # check again against requirements -- - # this is guaranteed to be fresh data - if not self._dependent_token_response_satisfies_scope_request( - dependent_tokens, scope, required_authorizer_expiration_time - ): + # check scope again -- this is guaranteed to be fresh data + if scope not in dependent_tokens.by_scopes: raise ValueError("Dependent tokens do not match request.") token_data = dependent_tokens.by_scopes[scope] - - refresh_token: str | None = token_data.get("refresh_token") - if refresh_token is not None: - return RefreshTokenAuthorizer( - refresh_token, - self.auth_client, - access_token=token_data["access_token"], - expires_at=token_data["expires_at_seconds"], - ) - else: - return AccessTokenAuthorizer(token_data["access_token"]) + return AccessTokenAuthorizer(token_data["access_token"]) def _get_cached_dependent_tokens( self, @@ -275,52 +256,10 @@ def _get_cached_dependent_tokens( """ if self._dependent_token_cache_key in self.dependent_tokens_cache: return (True, self.dependent_tokens_cache[self._dependent_token_cache_key]) - - # FIXME: switch from dependent refresh tokens to dependent access tokens - # - # dependent refresh tokens in an ephemeral execution context are not appropriate - # because the dependent refresh tokens are cached in-memory, not in a persistent store of state, - # this means we're fetching long-lived credentials which are then not persisted into long term - # storage - # - # the discrepancy between the credential type and the storage strategy reveals that these tokens - # are only used in a synchronous context in which the refresh token is not needed or wanted - # - token_response = self.auth_client.oauth2_get_dependent_tokens( - self.bearer_token, additional_params={"access_type": "offline"} - ) + token_response = self.auth_client.oauth2_get_dependent_tokens(self.bearer_token) self.dependent_tokens_cache[self._dependent_token_cache_key] = token_response return (False, token_response) - def _dependent_token_response_satisfies_scope_request( - self, - r: globus_sdk.OAuthDependentTokenResponse, - scope: str, - required_authorizer_expiration_time: int, - ) -> bool: - """ - Check a token response to see if it appears to satisfy the request for tokens - with a specific scope. - - :returns: True if the data is good, False if the data is bad - """ - try: - token_data = r.by_scopes[scope] - except LookupError: - return False - - refresh_token: str | None = token_data.get("refresh_token") - access_token: str | None = token_data.get("access_token") - token_expiration: int = token_data.get("expires_at_seconds", 0) - - if refresh_token is not None: - return True - - if token_expiration <= (int(time.time()) + required_authorizer_expiration_time): - return False - - return access_token is not None - def _get_groups_client(self) -> globus_sdk.GroupsClient: if self._groups_client is not None: return self._groups_client diff --git a/tests/test_authentication.py b/tests/test_authentication.py index fd8886b..892e56a 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -98,6 +98,19 @@ def test_auth_state_caching_across_instances(auth_state, freeze_time, mocked_res assert len(mocked_responses.calls) == 1 +def test_(auth_state): + load_response("token-introspect", case="success") + load_response("token", case="success") + with pytest.warns( + DeprecationWarning, + match="`required_authorizer_expiration_time` has no effect and will be removed", + ): + auth_state.get_authorizer_for_scope( + "urn:globus:auth:scope:groups.api.globus.org:view_my_groups_and_memberships", + required_authorizer_expiration_time=60, + ) + + def test_invalid_grant_exception(auth_state): load_response("token-introspect", case="success") load_response("token", case="invalid-grant") @@ -128,7 +141,6 @@ def test_dependent_token_callout_success_fixes_bad_cache(auth_state): "foo_scope": { "expires_at_seconds": time.time() + 100, "access_token": "foo_AT", - "refresh_token": "foo_RT", } } auth_state.dependent_tokens_cache[auth_state._dependent_token_cache_key] = ( @@ -146,15 +158,14 @@ def test_dependent_token_callout_success_fixes_bad_cache(auth_state): "scope": "bar_scope", "expires_at_seconds": time.time() + 100, "access_token": "bar_AT", - "refresh_token": "bar_RT", } ], ).add() # now get the 'bar_scope' authorizer authorizer = auth_state.get_authorizer_for_scope("bar_scope") - # it should be a refresh token authorizer and the cache should be updated - assert isinstance(authorizer, globus_sdk.RefreshTokenAuthorizer) + # it should be an access token authorizer and the cache should be updated + assert isinstance(authorizer, globus_sdk.AccessTokenAuthorizer) cache_value = auth_state.dependent_tokens_cache[ auth_state._dependent_token_cache_key ] From 46a5961a1847c0ea29f23cb2e989164e5b21e52b Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Wed, 30 Oct 2024 11:21:23 -0500 Subject: [PATCH 05/15] Remove flask 'api_helpers' and examples This module is never used by consumers of the package, based on an examination of known users. It was slated for removal in a much earlier version 0.12.0, but the deprecation warning which was put in place for this was incorrectly applied, so that all usage of the library emitted the deprecation warning. The warning was removed in 0.13.0 in order to fix this, and never re-applied for simplicity. After removing the module and tracing references to its functions, we find that two example applications (unmaintained) in the `examples/` directory were using these. Remove them. Update the examples pages in the docs to accord with this removal. Also, fix any description of the custom Flask Blueprint as "Flask Decorators". This is a narrow characteristic of the Blueprint object, and not an appropriate name for the feature. --- ...1030_100211_sirosen_remove_api_helpers.rst | 9 + docs/source/examples.rst | 57 --- docs/source/examples/apt_blueprint.rst | 11 +- docs/source/examples/watchasay.rst | 12 - docs/source/examples/whattimeisitrightnow.rst | 12 - examples/apt_blueprint/README.rst | 18 +- examples/watchasay/README.rst | 92 ----- examples/watchasay/app/__init__.py | 0 examples/watchasay/app/config.py | 5 - examples/watchasay/app/provider.py | 258 ------------- examples/watchasay/app/schema.json | 16 - examples/watchasay/requirements.txt | 3 - examples/whattimeisitrightnow/README.rst | 98 ----- examples/whattimeisitrightnow/app/__init__.py | 0 examples/whattimeisitrightnow/app/app.py | 363 ------------------ examples/whattimeisitrightnow/app/config.py | 3 - examples/whattimeisitrightnow/app/database.py | 19 - examples/whattimeisitrightnow/app/error.py | 39 -- examples/whattimeisitrightnow/app/schema.json | 16 - .../whattimeisitrightnow/requirements.txt | 4 - .../flask/__init__.py | 10 - .../flask/api_helpers.py | 324 ---------------- tests/test_flask_helpers/app_utils.py | 7 - tests/test_flask_helpers/conftest.py | 44 +-- tests/test_flask_helpers/test_routes.py | 16 +- 25 files changed, 37 insertions(+), 1399 deletions(-) create mode 100644 changelog.d/20241030_100211_sirosen_remove_api_helpers.rst delete mode 100644 docs/source/examples/watchasay.rst delete mode 100644 docs/source/examples/whattimeisitrightnow.rst delete mode 100644 examples/watchasay/README.rst delete mode 100644 examples/watchasay/app/__init__.py delete mode 100644 examples/watchasay/app/config.py delete mode 100644 examples/watchasay/app/provider.py delete mode 100644 examples/watchasay/app/schema.json delete mode 100644 examples/watchasay/requirements.txt delete mode 100644 examples/whattimeisitrightnow/README.rst delete mode 100644 examples/whattimeisitrightnow/app/__init__.py delete mode 100644 examples/whattimeisitrightnow/app/app.py delete mode 100644 examples/whattimeisitrightnow/app/config.py delete mode 100644 examples/whattimeisitrightnow/app/database.py delete mode 100644 examples/whattimeisitrightnow/app/error.py delete mode 100644 examples/whattimeisitrightnow/app/schema.json delete mode 100644 examples/whattimeisitrightnow/requirements.txt delete mode 100644 src/globus_action_provider_tools/flask/api_helpers.py diff --git a/changelog.d/20241030_100211_sirosen_remove_api_helpers.rst b/changelog.d/20241030_100211_sirosen_remove_api_helpers.rst new file mode 100644 index 0000000..6139fd8 --- /dev/null +++ b/changelog.d/20241030_100211_sirosen_remove_api_helpers.rst @@ -0,0 +1,9 @@ +Changes +------- + +- Remove the ``globus_action_provider_tools.flask.api_helpers`` module, and the + helpers it provided. Most users are not using these tools and should + experience no impact. + +- Remove examples from documentation which relied upon the ``api_helpers`` + module. diff --git a/docs/source/examples.rst b/docs/source/examples.rst index e1e2f84..64cbaf5 100644 --- a/docs/source/examples.rst +++ b/docs/source/examples.rst @@ -1,65 +1,8 @@ Examples ======== -We demonstrate how to use the different components in the toolkit by providing a -few sample Action Provider implementations. Each implementation leverages a -different part of the toolkit to implement a Globus Automate-compatible Action -Provider. - -Flask Decorators -^^^^^^^^^^^^^^^^ - -`Flask `__ is a popular framework for -creating APIs. This toolkit provides a custom `Flask Blueprint -`_ object that -provides decorators for registering functions that will implement the operations -for the Action Provider Interface and also perform most of the authentication and -validation. All the developer needs to do is create a series of -functions that will execute as the Action Provider's endpoints and register the -Blueprint with a Flask application. - -:doc:`Flask Decorators Example` - -Flask API Helpers -^^^^^^^^^^^^^^^^^ - -Another `Flask `__ targeted helper, this -part of the toolkit provides a different way of creating an Action Provider -which also implements most of the authentication and validation required. Users -of this helper need only implement callback functions that will be used as the -Action Provider routes. - -:doc:`Flask API Helpers Example` - -ActionProvider Input Schema Definition -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -This example demonstrates the supported methods of defining an ActionProvider's -input schema. An input schema serves as a standardized definition for the -input to an ActionProvider and provides input validation. - -:doc:`Input Schema Definitions` - -Framework Agnostic Tools -^^^^^^^^^^^^^^^^^^^^^^^^ - -Finally, if you would like to use your own Python microservice framework, you -can use the toolkit's components individually. The `Flask -`_ based components of the toolkit are good -examples of how you can compose the individual components. We also provide an -example implementation demonstrating how you can create routes implementing the -Action Provider interface, how you can create a TokenChecker instance to -validate tokens, how to create validation objects to validate incoming -ActionRequests and more. - -:doc:`Python Helpers Example` - - .. toctree:: :maxdepth: 1 - :hidden: - examples/whattimeisitrightnow - examples/watchasay examples/apt_blueprint examples/input_schemas diff --git a/docs/source/examples/apt_blueprint.rst b/docs/source/examples/apt_blueprint.rst index 1b3dc08..d1abb23 100644 --- a/docs/source/examples/apt_blueprint.rst +++ b/docs/source/examples/apt_blueprint.rst @@ -1,10 +1,15 @@ -Flask Decorators -================ +Flask Blueprint +=============== + +``globus-action-provider-tools`` provides a custom +`Flask Blueprint `_ object +with decorators for registering functions which implement Action Provider +interfaces. .. include:: ../../../examples/apt_blueprint/README.rst Action Provider Implementation -============================== +------------------------------ .. literalinclude:: ../../../examples/apt_blueprint/config.py :language: python diff --git a/docs/source/examples/watchasay.rst b/docs/source/examples/watchasay.rst deleted file mode 100644 index 49ccbd4..0000000 --- a/docs/source/examples/watchasay.rst +++ /dev/null @@ -1,12 +0,0 @@ -watchasay Action Provider -========================= - -.. include:: ../../../examples/watchasay/README.rst - - -Action Provider Implementation -============================== - -.. literalinclude:: ../../../examples/watchasay/app/provider.py - :language: python - :name: watchasay-provider-py diff --git a/docs/source/examples/whattimeisitrightnow.rst b/docs/source/examples/whattimeisitrightnow.rst deleted file mode 100644 index 08b5396..0000000 --- a/docs/source/examples/whattimeisitrightnow.rst +++ /dev/null @@ -1,12 +0,0 @@ -whattimeisitrightnow Action Provider -==================================== - -.. include:: ../../../examples/whattimeisitrightnow/README.rst - - -Action Provider Implementation -============================== - -.. literalinclude:: ../../../examples/whattimeisitrightnow/app/app.py - :language: python - :name: watchasay-provider-py diff --git a/examples/apt_blueprint/README.rst b/examples/apt_blueprint/README.rst index 1a26c97..1d588a1 100644 --- a/examples/apt_blueprint/README.rst +++ b/examples/apt_blueprint/README.rst @@ -1,6 +1,7 @@ Overview -^^^^^^^^ -This is a sample Flask application implemented using the Flask Decorators in the +-------- + +This is a sample Flask application implemented using the Flask Blueprint in the Action Provider Toolkit. The Toolkit provides an `ActionProviderBlueprint` with five decorators which are used to decorate functions that will be run when the Action Provider is invoked. Each decorator corresponds to one of the Action Provider @@ -14,11 +15,12 @@ Interface endpoints. The decorators available are: Using this Tool -^^^^^^^^^^^^^^^ -The `ActionProviderBlueprint` is exactly like a Flask `Blueprint`, except it has -been customized to implement the *Action Provider Interface* and ties together -much of the tooling available in the rest of the Toolkit to provide a -streamlined development experience. The ActionProviderBlueprint will: +--------------- + +The `ActionProviderBlueprint` is a Flask `Blueprint` which has +been customized to implement the *Action Provider Interface*. + +The ActionProviderBlueprint will: - Validate that incoming requests to your ActionProvider adhere to the ActionRequest schema @@ -165,7 +167,7 @@ would register any other Flask Blueprint and run your ActionProvider: application's "CLIENT_ID" and "CLIENT_SECRET" values. Example Configuration -===================== +--------------------- To run this example Action Provider, you need to generate your own CLIENT_ID, CLIENT_SECRET, and SCOPE. It may be useful to follow the directions diff --git a/examples/watchasay/README.rst b/examples/watchasay/README.rst deleted file mode 100644 index 7ef5955..0000000 --- a/examples/watchasay/README.rst +++ /dev/null @@ -1,92 +0,0 @@ -######### -watchasay -######### - -This is a sample Flask application implementing the simple "echo" Unix command -line utility as an ActionProvider. This example uses the Flask API helpers in -globus_action_provider_tools. Rather than defining each of the -Action Provider Interface routes in the Flask application, the helpers declare -the necessary routes to Flask, perform the serialization, validation and -authentication on the request, and pass only those requests which have satisfied -these conditions on to a user-defined implementation of the routes. - -To run a *watchasay* Action, provide a "input_string" parameter indicating the -text to have "echoed" back. Since this ActionProvider is synchronous, each -Action has its status set to "SUCCEEDED" immediately. Note that this Action -Provider is configured to run at the */skeleton* endpoint. - -Presteps -======== -To run this example Action Provider, you will need to generate your own -CLIENT_ID, CLIENT_SECRET, and SCOPE. It may be useful to follow the directions -for generating each of these located at README.rst. Once you have those three -values, place them into the example Action Provider's config.py. - -Starting the Action Provider -============================ -We recommend creating a virtualenvironment to install project dependencies and -run the Action Provider. Once the virtualenvironment has been created and -activated, run the following: - - .. code-block:: BASH - - cd examples/watchasay - pip install -r requirements.txt - python app/provider.py - -Testing the Action Provider -=========================== -We provide example tests to validate that your Action Provider is working and -enable some form of continuous integration. To run the example test suite, once -again activate the project's virtualenvironment and run the following: - - .. code-block:: BASH - - cd examples/watchasay - pytest - -Within these tests, we provide examples of how to use a patch that is useful for -testing your Action Provider without using a valid CLIENT_ID, CLIENT_SECRET or -request Tokens. Only use this patch during testing. - -Actually using the Action Provider -================================== -You'll notice that the only endpoint we can reach without a valid token is the -introspect endpoint (*/skeleton*). Issuing the below command will report the -expected request schema and the required scope for using the Provider: - - .. code-block:: BASH - - curl http://localhost:5000/skeleton/ - -Why? It's because the watchasay Provider has been set to be publicly visible. -Setting the introspection endpoint to be publicly visible is useful way of -providing documentation on how to interact with the ActionProvider. -All other operations on the Action Provider will require a valid token: - - .. code-block:: BASH - - curl --request POST \ - --url http://localhost:5000/skeleton/run \ - --header 'authorization: Bearer token' \ - --data '{"request_id": "some-id","body": {"input_string": "hey"}}' - - -But how to get the token? The recommended route to retrieve a token is to use -the globus-automate-client CLI tool. Conveniently, the globus-automate-client -CLI tool removes the need to create curl requests and the need to manually -format Action request bodies. See the doc on downloading the CLI tool. Once -downloaded, issue a command similar to to the one below. The first time you -run the command, you will need to follow a flow to request the necessary grants -for your Action Provider's scopes. Later attempts to use the -globus-automate-client tool will use locally cached tokens and transparently -refresh expired tokens. - - .. code-block:: BASH - - globus-automate action-run \ - --action-url http://localhost:5000/skeleton/run \ - --action-scope $YOUR_PROVIDERS_SCOPE \ - --body '{"input_string":"hi"}' - -Run the CLI tool with the *--help* option for more information. diff --git a/examples/watchasay/app/__init__.py b/examples/watchasay/app/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/examples/watchasay/app/config.py b/examples/watchasay/app/config.py deleted file mode 100644 index 3f8c89d..0000000 --- a/examples/watchasay/app/config.py +++ /dev/null @@ -1,5 +0,0 @@ -client_id = "" -client_secret = "" -our_scope = ( - "https://auth.globus.org/scopes/d3a66776-759f-4316-ba55-21725fe37323/action_all" -) diff --git a/examples/watchasay/app/provider.py b/examples/watchasay/app/provider.py deleted file mode 100644 index ac789b2..0000000 --- a/examples/watchasay/app/provider.py +++ /dev/null @@ -1,258 +0,0 @@ -import datetime -import json -import os -from typing import Dict, Optional, Set, Tuple - -from flask import Blueprint, Flask - -from examples.watchasay.app import config -from globus_action_provider_tools import ( - ActionProviderDescription, - ActionRequest, - ActionStatus, - ActionStatusValue, - AuthState, -) -from globus_action_provider_tools.authorization import ( - authorize_action_access_or_404, - authorize_action_management_or_404, -) -from globus_action_provider_tools.flask import add_action_routes_to_blueprint -from globus_action_provider_tools.flask.exceptions import ActionConflict, ActionNotFound -from globus_action_provider_tools.flask.helpers import assign_json_provider -from globus_action_provider_tools.flask.types import ActionCallbackReturn - -# A simulated database mapping input user action requests identifiers to a previously -# seen request id and the corresponding action id -_fake_request_db: Dict[str, Tuple[ActionRequest, str]] = {} - -_fake_action_db: Dict[str, ActionStatus] = {} - - -def _retrieve_action_status(action_id: str) -> ActionStatus: - status = _fake_action_db.get(action_id) - if status is None: - raise ActionNotFound(f"No Action with id {action_id}") - return status - - -def load_schema(): - with open( - os.path.join(os.path.dirname(os.path.abspath(__file__)), "schema.json") - ) as f: - schema = json.load(f) - return schema - - -def action_enumerate(auth: AuthState, params: Dict[str, Set]): - """ - This is an optional endpoint, useful for allowing requesters to enumerate - actions filtered by ActionStatus and role. - - The params argument will always be a dict containing the incoming request's - validated query arguments. There will be two keys, 'statuses' and 'roles', - where each maps to a set containing the filter values for the key. A typical - params object will look like: - - { - "statuses": {}, - "roles": {"creator_id"} - } - - Notice that the value for the "statuses" key is an Enum value. - """ - statuses = params["statuses"] - roles = params["roles"] - matches = [] - - for _, action in _fake_action_db.items(): - if action.status in statuses: - # Create a set of identities that are allowed to access this action, - # based on the roles being queried for - allowed_set = set() - for role in roles: - identities = getattr(action, role) - if isinstance(identities, str): - allowed_set.add(identities) - else: - allowed_set.update(identities) - - # Determine if this request's auth allows access based on the - # allowed_set - authorized = auth.check_authorization(allowed_set) - if authorized: - matches.append(action) - - return matches - - -def action_run(request: ActionRequest, auth: AuthState) -> ActionCallbackReturn: - """ - Asynchronous actions most likely need to implement retry logic here to - prevent duplicate requests with matching request_ids from launching - another job. In the event that a request with an existing request_id - and creator_id arrives, this function should simply return the action's - status via the action_status function. - - Synchronous actions or actions where it makes sense to execute repeated - runs with the same parameters need not implement retry logic. - """ - - caller_id = auth.effective_identity - request_id = request.request_id - full_request_id = f"{caller_id}:{request_id}" - prev_request = _fake_request_db.get(full_request_id) - if prev_request is not None: - # If the a matching ActionRequest has been found, deduplicate the - # requests and return the Action's status - if prev_request[0] == request: - return action_status(prev_request[1], auth) - # If a pre-existing ActionRequest with different parameters has been - # found, throw an error as we can't modify an already running Action - else: - raise ActionConflict( - f"Request with id {request_id} already present with different parameters " - ) - - # Local processing happens here - result_details = { - # This is safe because the input has been validated against the input schema - "you_said": request.body["input_string"] - } - - # Create an ActionStatus that contains the computed results - status = ActionStatus( - status=ActionStatusValue.SUCCEEDED, - creator_id=caller_id or "UNKNOWN", - monitor_by=request.monitor_by, - manage_by=request.manage_by, - start_time=str(datetime.datetime.now().isoformat()), - completion_time=str(datetime.datetime.now().isoformat()), - release_after=request.release_after or "P30D", - display_status=ActionStatusValue.SUCCEEDED, - details=result_details, - ) - - # Store the request and action_status - _fake_request_db[full_request_id] = (request, status.action_id) - _fake_action_db[status.action_id] = status - return status - - -def action_status(action_id: str, auth: AuthState) -> ActionCallbackReturn: - """ - action_status retrieves the most recent state of the action. This endpoint - requires the user authenticate with a principal value which is in the - monitor_by list established when the Action was started. - """ - status = _retrieve_action_status(action_id) - authorize_action_access_or_404(status, auth) - return status, 200 - - -def action_cancel(action_id: str, auth: AuthState) -> ActionCallbackReturn: - """ - Asynchronous actions need not ensure a running action is immediately - completed or terminated. In this scenario, action_cancel should return - an action in a non-completion state. If it has completed, return the action's - status. - - Synchronous actions need not implement any logic in action_cancel. All - processing happens in the action_run callback so that action_cancel - simply returns the action_id's status. - - This endpoint requires the user authenticate with a principal value which is - in the manage_by list established when the Action was started. - """ - status = _retrieve_action_status(action_id) - authorize_action_management_or_404(status, auth) - - # If action is already in complete state, return completion details - if status.status in (ActionStatusValue.SUCCEEDED, ActionStatusValue.FAILED): - return status - - # Process Action cancellation - status.status = ActionStatusValue.FAILED - status.display_status = "Canceled by user request" - return status - - -def action_release(action_id: str, auth: AuthState) -> ActionCallbackReturn: - """ - If the Action is not already in a completion state, action_release should - return an error as this operation does not attempt to stop execution. - Synchronous actions need not determine if the action_id is still in a - processing state. All processing starts and completes in the action_run - callback so that action_release simply removes the action_id and request_id - from history and returns the action_id's completion status. - - This endpoint requires the user authenticate with a principal value which is - in the manage_by list established when the Action was started. - """ - status = _retrieve_action_status(action_id) - authorize_action_management_or_404(status, auth) - - # Error if attempt to release an active Action - if status.status not in (ActionStatusValue.SUCCEEDED, ActionStatusValue.FAILED): - raise ActionConflict("Action is not complete") - - _fake_action_db.pop(action_id) - # Both fake and badly inefficient - remove_req_id: Optional[str] = None - for req_id, req_and_action_id in _fake_request_db.items(): - if req_and_action_id[1] == action_id: - remove_req_id = req_id - break - if remove_req_id is not None: - _fake_request_db.pop(remove_req_id) - return status, 200 - - -def create_app(): - app = Flask(__name__) - assign_json_provider(app) - app.url_map.strict_slashes = False - - # Create and define a blueprint onto which the routes will be added - skeleton_blueprint = Blueprint("skeleton", __name__, url_prefix="/skeleton") - - # Create the ActionProviderDescription with the correct scope and schema - provider_description = ActionProviderDescription( - globus_auth_scope=config.our_scope, - title="skeleton_action_provider", - admin_contact="support@globus.org", - synchronous=True, - input_schema=load_schema(), - log_supported=False, # This provider doesn't implement the log callback - visible_to=["public"], - ) - - # Use the flask helper function to register the endpoint callbacks onto the - # blueprint - add_action_routes_to_blueprint( - blueprint=skeleton_blueprint, - client_id=config.client_id, - client_secret=config.client_secret, - provider_description=provider_description, - action_run_callback=action_run, - action_status_callback=action_status, - action_cancel_callback=action_cancel, - action_release_callback=action_release, - action_enumeration_callback=action_enumerate, - additional_scopes=[ - "https://auth.globus.org/scopes/d3a66776-759f-4316-ba55-21725fe37323/secondary_scope" - ], - ) - - # Register the blueprint with your flask app before returning it - app.register_blueprint(skeleton_blueprint) - return app - - -def main(): - app = create_app() - app.run(debug=True) - - -if __name__ == "__main__": - main() diff --git a/examples/watchasay/app/schema.json b/examples/watchasay/app/schema.json deleted file mode 100644 index c7c87d6..0000000 --- a/examples/watchasay/app/schema.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "$id": "https://automate.globus.org/skeleton_action_provider.input.schema.json", - "$schema": "http://json-schema.org/draft-07/schema#", - "title": "Skeleton Action Provider Input Schema", - "type": "object", - "properties": { - "input_string": { - "description": "A string to be replayed back to the user", - "type": "string" - } - }, - "additionalProperties": false, - "required": [ - "input_string" - ] -} diff --git a/examples/watchasay/requirements.txt b/examples/watchasay/requirements.txt deleted file mode 100644 index 612e60a..0000000 --- a/examples/watchasay/requirements.txt +++ /dev/null @@ -1,3 +0,0 @@ -flask -globus_action_provider_tools -pytest diff --git a/examples/whattimeisitrightnow/README.rst b/examples/whattimeisitrightnow/README.rst deleted file mode 100644 index ca2dc52..0000000 --- a/examples/whattimeisitrightnow/README.rst +++ /dev/null @@ -1,98 +0,0 @@ -#################### -whattimeisitrightnow -#################### - -From the makers of Philbert: Another exciting promotional tie-in for whattimeisitrightnow.com -############################################################################################# - -This is a sample Flask application implementing the ActionProvider interface. It -accepts requests with a "utc_offset" parameter indicating which timezone to -return the current UTC time as. To demonstrate some degree of complexity, the -application randomly assigns each request an *estimated_completion_time* so that -an action's results will not be available until the *estimated_completion_time*. -To do this, we store and make use of "private" data fields which are never -displayed to any requester. Additionally, some percentage of requests to the -ActionProvider fail to demonstrate how to report errors back to the requesters. - -Presteps -======== -To run this example Action Provider, you will need to generate your own -CLIENT_ID, CLIENT_SECRET, and SCOPE. It may be useful to follow the directions -for generating each of these located at README.rst. Once you have those three -values, place them into the example Action Provider's config.py. - -Starting the Action Provider -============================ -We recommend creating a virtualenvironment to install project dependencies and -run the Action Provider. Once the virtualenvironment has been created and -activated, run the following: - - .. code-block:: BASH - - cd examples/whattimeisitrightnow - pip install -r requirements.txt - python app/app.py - -Testing the Action Provider -=========================== -We provide example tests to validate that your Action Provider is working and -enable some form of continuous integration. To run the example test suite, once -again activate the project's virtualenvironment and run the following: - - .. code-block:: BASH - - cd examples/whattimeisitrightnow - pytest - -Within these tests, we provide examples of how to use a patch that is useful for -testing your Action Provider without using a valid CLIENT_ID, CLIENT_SECRET or -request Tokens. Only use this patch during testing. - -Actually using the Action Provider -================================== -You'll notice that just because its running doesn't mean we can actually use the -Action Provider. In particular, once the whattimeisitrightnow Action Provider is -run, this will fail: - - .. code-block:: BASH - - curl http://localhost:5000/ - -Why? It's because the whattimeisitrightnow Provider has been set to be visible -to only authenticated users (see the ActionProviderDescription initialization -values). Therefore, requests need proper HTTP authorization headers (i.e. -a token needs to be provided): - - .. code-block:: BASH - - curl --request GET \ - --url http://localhost:5000/ \ - --header 'authorization: Bearer token' - -But how to get the token? The recommended route to retrieve a token is to use -the globus-automate-client CLI tool. Conveniently, the globus-automate-client -CLI tool removes the need to create curl requests and the need to manually -format Action request bodies. See the doc on downloading the CLI tool. Once -downloaded, issue a command similar to to the one below. The first time you -run the command, you will need to follow a flow to request the necessary grants -for your Action Provider's scopes. Later attempts to use the -globus-automate-client tool will use locally cached tokens and transparently -refresh expired tokens. - - .. code-block:: BASH - - globus-automate action-provider-introspect \ - --action-url http://localhost:5000/ \ - --action-scope $YOUR_PROVIDERS_SCOPE - -The globus-automate-client CLI tool can also make requests to endpoints besides -the introspection endpoint, for example: - - .. code-block:: BASH - - globus-automate action-run \ - --action-url http://localhost:5000/ \ - --action-scope $YOUR_PROVIDERS_SCOPE \ - --body '{"utc_offset": 1}' - -Run the CLI tool with the *--help* option for more information. diff --git a/examples/whattimeisitrightnow/app/__init__.py b/examples/whattimeisitrightnow/app/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/examples/whattimeisitrightnow/app/app.py b/examples/whattimeisitrightnow/app/app.py deleted file mode 100644 index 83eaeb9..0000000 --- a/examples/whattimeisitrightnow/app/app.py +++ /dev/null @@ -1,363 +0,0 @@ -import json -import logging -import os -from datetime import datetime, timedelta, timezone -from random import randint -from typing import Any, Dict, Tuple - -import globus_sdk -from flask import Flask, Response, jsonify, request - -from examples.whattimeisitrightnow.app import config -from examples.whattimeisitrightnow.app import error as err -from examples.whattimeisitrightnow.app.database import db -from globus_action_provider_tools.authentication import AuthStateBuilder -from globus_action_provider_tools.authorization import ( - authorize_action_access_or_404, - authorize_action_management_or_404, -) -from globus_action_provider_tools.data_types import ( - ActionProviderDescription, - ActionStatus, - ActionStatusValue, -) -from globus_action_provider_tools.flask import flask_validate_request -from globus_action_provider_tools.flask.helpers import assign_json_provider - -app = Flask(__name__) -assign_json_provider(app) - -state_builder = AuthStateBuilder( - globus_sdk.ConfidentialAppAuthClient(config.client_id, config.client_secret), - [config.our_scope], -) - -COMPLETE_STATES = (ActionStatusValue.SUCCEEDED, ActionStatusValue.FAILED) -INCOMPLETE_STATES = (ActionStatusValue.ACTIVE, ActionStatusValue.INACTIVE) - -with open(os.path.join(os.path.dirname(os.path.abspath(__file__)), "schema.json")) as f: - schema = json.load(f) - - -@app.errorhandler(err.ApiError) -def handle_invalid_usage(error) -> Response: - response = jsonify(error.to_dict()) - response.status_code = error.status - return response - - -@app.before_request -def before_request() -> None: - """ - Here we handle some authorization and request validation before the request - ever makes it to our ActionProvider. We also attach authentication - information to the request to make it easier to inspect. - - flask_validate_request ensures that we are receiving a valid request - body from the user. - """ - validation_result = flask_validate_request(request) - if validation_result.errors: - raise err.InvalidRequest(*validation_result.errors) - - token = request.headers.get("Authorization", "").replace("Bearer ", "") - auth_state = state_builder.build(token) - if not auth_state.identities: - # Returning these authentication errors to the caller will make debugging - # easier for this example. Consider whether this is appropriate - # for your production use case or not. - raise err.NoAuthentication(*auth_state.errors) - request.auth = auth_state # type: ignore - - -@app.route("/", methods=["GET"]) -def introspect() -> Tuple[Response, int]: - """ - The base endpoint of an ActionProvider should serve as documentation, - enabling users to introspect the JSON schema required to launch an action. - This endpoint can be publicly accessible or not. The - ActionProviderDescription visible_to field public access by default. - """ - description = ActionProviderDescription( - globus_auth_scope=config.our_scope, - title="What Time Is It Right Now?", - admin_contact="support@whattimeisrightnow.example", - synchronous=False, - input_schema=schema, - api_version="1.0", - subtitle=( - "From the makers of Philbert: " - "Another exciting promotional tie-in for whattimeisitrightnow.com" - ), - description="", - keywords=["time", "whattimeisitnow", "productivity"], - visible_to=["all_authenticated_users"], - runnable_by=["all_authenticated_users"], - administered_by=["support@whattimeisrightnow.example"], - ) - if not request.auth.check_authorization( # type: ignore - description.visible_to, allow_all_authenticated_users=True - ): - raise err.NotAuthorized("Not visible_to this access token.") - return jsonify(description), 200 - - -@app.route("/run", methods=["POST"]) -def run() -> Tuple[Response, int]: - """ - This function implements Action Provider interface for launching an - action instance. - - This function parses the request_id from a request body and from it - determines whether a new action should be launched or whether to - return the status of a previously launched action. To accomplish this, - it is necessary to store a mapping of request_ids to action_ids in - addition to a history of actions. - """ - req = request.get_json(force=True) - - # Deduplicate multiple requests based on request_id - action_id = db.query(req["request_id"]) - if action_id is not None: - return status(action_id) - else: - action_status = run_action(req) - # Remove any private data from the ActionStatus before - # returning it to the requester - action_status = _filter_private_fields(action_status) - return jsonify(action_status), 202 - - -def run_action(req) -> ActionStatus: - """ - A wrapper function to handle executing 'business logic', creating the - ActionStatus and storing both the request_id:action_id mapping and the - action_id:action_status mapping. - """ - now = datetime.now(tz=timezone.utc) - - # Kickoff whatever the action is actually doing - results = _magic_business_logic(now, req["body"]) - - # Create an ActionStatus object with result information - action_status = ActionStatus( - status=ActionStatusValue.ACTIVE, - creator_id=request.auth.effective_identity, # type: ignore - label=req.get("label", None), - monitor_by=req.get("monitor_by", request.auth.identities), # type: ignore - manage_by=req.get("manage_by", request.auth.identities), # type: ignore - start_time=str(now), - completion_time=None, - release_after=req.get("release_after", "P30D"), - display_status=ActionStatusValue.ACTIVE, - details=results, - ) - - # Store the request_id for deduplication - db.persist(req["request_id"], action_status.action_id) - - # Store the details on the running job - db.persist(action_status.action_id, action_status) - return action_status - - -def _filter_private_fields(action_status: ActionStatus) -> ActionStatus: - """ - Helper function to demonstrate how an ActionStatus object can - hold private data in its details field and how to filter this - data before returning an ActionStatus to the requester - """ - if action_status.details is not None: - assert isinstance(action_status.details, dict) - action_status.details.pop("private", None) - return action_status - - -def _magic_business_logic(now, request_body) -> Dict[str, Any]: - """ - This function computes the current time in a different timezone. - It accepts "utc_offset" to determine the target timezone before - converting the time and storing it into the results dictionary. - - This function demonstrates how private data can be computed and - stored. - """ - try: - tz = timezone(timedelta(hours=request_body["utc_offset"])) - except (KeyError, ValueError) as exc: - raise err.InvalidRequest("Invalid or missing 'utc_offset'", exc) - - # Simulate some amount of processing time - estimated_processing_time = randint(5, 900) - estimated_completion_time = now + timedelta(seconds=estimated_processing_time) - - # 30% of our jobs fail because the universe is an imperfect place - # and we want clients to understand some providers may fail - success = randint(1, 100) >= 30 - if success: - results = { - "estimated_completion_time": estimated_completion_time, - "private": { - "success": True, - "details": {"whattimeisit": now.astimezone(tz)}, - }, - } - else: - results = { - "estimated_completion_time": estimated_completion_time, - "private": { - "success": False, - "details": { - "message": "We didn't know what time it was.", - "error": "WATCHLESS", - }, - }, - } - return results - - -@app.route("//status", methods=["GET"]) -def status(action_id) -> Tuple[Response, int]: - """ - This function implements Action Provider interface for looking up an - action's status. This endpoint is used to query actions that may - still be executing or may have completed. - """ - # Ensure the requested action_id exists - action_status = _get_action_status_or_404(action_id) - - # Ensure the user is authorized to view the action status - authorize_action_access_or_404(action_status, request.auth) # type: ignore - - action_status = _reconcile_action_status(action_status) - - # Remove any private data from the ActionStatus before - # returning it to the requester - action_status = _filter_private_fields(action_status) - return jsonify(action_status), 200 - - -def _get_action_status_or_404(action_id: str) -> ActionStatus: - """ - Retrieves an action_status from the database - """ - action_status = db.query(action_id) - - # Since we're using the same database to store action_ids and - # request_ids, it's possible a user may make a request for an - # action_id's status using the request_id. In that event, the - # db lookup for a request_id will return a str. - if action_status is None or isinstance(action_status, str): - raise err.NotFound(f"No action instance found for {action_id}") - return action_status - - -def _reconcile_action_status(action_status: ActionStatus) -> ActionStatus: - """ - Helper function to determine if an Action should have completed, and to - update its status if necessary. If the Action is already in a completed - state, its record is returned. If the action is still not scheduled to - complete, its record is returned unmodified. If the record was scheduled - to complete, its status is updated, stored, and returned. - """ - # If status is in a completion state, return - if action_status.status in COMPLETE_STATES: - return action_status - - # Make mypy happy... - if action_status.details is None: - raise err.DeveloperError(f"{action_status.action_id} has no details.") - - # If it is not yet time for the action to complete, return - now = datetime.now(tz=timezone.utc) - assert isinstance(action_status.details, dict) - if action_status.details["estimated_completion_time"] > now: - return action_status - - # If the action was scheduled to complete by now, update the ActionStatus - # object with completion data - assert isinstance(action_status.details, dict) - private = action_status.details.pop("private", {}) - action_status.completion_time = action_status.details["estimated_completion_time"] - action_status.details = private["details"] - - if private["success"]: - action_status.status = ActionStatusValue.SUCCEEDED - action_status.display_status = ActionStatusValue.SUCCEEDED - else: - action_status.status = ActionStatusValue.FAILED - action_status.display_status = ActionStatusValue.FAILED - - # Persist updates to the ActionStatus - db.persist(action_status.action_id, action_status) - return action_status - - -@app.route("//cancel", methods=["POST"]) -def cancel(action_id: str) -> Tuple[Response, int]: - """ - This function implements the ActionProvider interface for cancelling an - action. As noted in the documentation, this operation does not need to - force the action to immediately cancel. - """ - # Ensure the requested action_id exists - action_status = _get_action_status_or_404(action_id) - - # Ensure the user is authorized to manage the action's state - authorize_action_management_or_404(action_status, request.auth) # type: ignore - - # Reconcile before cancelling to determine if action already completed - action_status = _reconcile_action_status(action_status) - - if action_status.status in COMPLETE_STATES: - raise err.InvalidState(f"Cannot cancel, {action_id} already completed.") - - # Interrupt / cancel the job if it's still running - action_status = _cancel_job(action_status) - return jsonify(action_status), 200 - - -def _cancel_job(action_status) -> ActionStatus: - """ - Helper function used to set an action's status fields to cancel. - Once cancelled, updates are persisted to the database. - """ - action_status.status = ActionStatusValue.FAILED - action_status.display_status = ActionStatusValue.FAILED - action_status.completion_time = datetime.now(tz=timezone.utc) - action_status.details = {"message": "Job cancelled", "error": "CANCELLED"} - - db.persist(action_status.action_id, action_status) - return action_status - - -@app.route("//release", methods=["POST"]) -def release(action_id: str) -> Tuple[Response, int]: - """ - Releasing an Action erases all records of its execution from the - Provider's history. Subsequent lookups for the Action's execution - will fail. - """ - # Ensure the requested action_id exists - action_status = _get_action_status_or_404(action_id) - - # Ensure the user is authorized to manage the action's state - authorize_action_management_or_404(action_status, request.auth) # type: ignore - - # Reconcile before cancelling to determine if action already completed - action_status = _reconcile_action_status(action_status) - - if action_status.status in INCOMPLETE_STATES: - raise err.InvalidState(f"Cannot release, {action_id} has not completed.") - - db.delete(action_id) - return jsonify(action_status), 200 - - -def main(): - logging.basicConfig(level=logging.INFO) - app.run(debug=True) - - -if __name__ == "__main__": - main() diff --git a/examples/whattimeisitrightnow/app/config.py b/examples/whattimeisitrightnow/app/config.py deleted file mode 100644 index 065c333..0000000 --- a/examples/whattimeisitrightnow/app/config.py +++ /dev/null @@ -1,3 +0,0 @@ -client_id = "16e16447-209a-4825-ae19-25e279d91642" -client_secret = "SECRET" -our_scope = "https://auth.globus.org/scopes/16e16447-209a-4825-ae19-25e279d91642/action_all_with_groups" diff --git a/examples/whattimeisitrightnow/app/database.py b/examples/whattimeisitrightnow/app/database.py deleted file mode 100644 index f45586d..0000000 --- a/examples/whattimeisitrightnow/app/database.py +++ /dev/null @@ -1,19 +0,0 @@ -import pickle - - -class DummyDatabase: - def __init__(self): - self.db = {} - - def persist(self, key, value): - self.db[key] = pickle.dumps(value) - - def query(self, key): - val = self.db.get(key, pickle.dumps(None)) - return pickle.loads(val) - - def delete(self, key): - del self.db[key] - - -db = DummyDatabase() diff --git a/examples/whattimeisitrightnow/app/error.py b/examples/whattimeisitrightnow/app/error.py deleted file mode 100644 index 1ff851a..0000000 --- a/examples/whattimeisitrightnow/app/error.py +++ /dev/null @@ -1,39 +0,0 @@ -# http://flask.pocoo.org/docs/1.0/patterns/apierrors/ -class ApiError(Exception): - status = 500 - - def __init__(self, *errors, status=None): - self.status = status if status is not None else self.status - self.errors = errors - - def to_dict(self): - errs = [{"detail": str(err)} for err in self.errors] - return {"errors": errs} - - -class DeveloperError(ApiError): - """ - Your action provider implementation is trying - to return a response that conflicts with the API - specification. - """ - - -class InvalidRequest(ApiError): - status = 400 - - -class NoAuthentication(ApiError): - status = 401 - - -class NotAuthorized(ApiError): - status = 403 - - -class NotFound(ApiError): - status = 404 - - -class InvalidState(ApiError): - status = 409 diff --git a/examples/whattimeisitrightnow/app/schema.json b/examples/whattimeisitrightnow/app/schema.json deleted file mode 100644 index 5e6d14a..0000000 --- a/examples/whattimeisitrightnow/app/schema.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "$id": "whattimeisitnow.provider.input.schema.json", - "$schema": "http://json-schema.org/draft-07/schema#", - "title": "whattimeisitnow Provider Input Schema", - "type": "object", - "properties": { - "utc_offset": { - "description": "The utc_offset to return the time right now as", - "type": "string" - } - }, - "required": [ - "utc_offset" - ], - "additionalProperties": false -} diff --git a/examples/whattimeisitrightnow/requirements.txt b/examples/whattimeisitrightnow/requirements.txt deleted file mode 100644 index e9c3753..0000000 --- a/examples/whattimeisitrightnow/requirements.txt +++ /dev/null @@ -1,4 +0,0 @@ -flask -globus_action_provider_tools -isodate -pytest diff --git a/src/globus_action_provider_tools/flask/__init__.py b/src/globus_action_provider_tools/flask/__init__.py index ee12ad5..188ce8b 100644 --- a/src/globus_action_provider_tools/flask/__init__.py +++ b/src/globus_action_provider_tools/flask/__init__.py @@ -1,17 +1,7 @@ -from .api_helpers import ( - add_action_routes_to_blueprint, - blueprint_error_handler, - flask_validate_request, - flask_validate_response, -) from .apt_blueprint import ActionProviderBlueprint from .config import DEFAULT_CONFIG, ActionProviderConfig __all__ = ( - "add_action_routes_to_blueprint", - "flask_validate_request", - "flask_validate_response", - "blueprint_error_handler", "ActionProviderBlueprint", "ActionProviderConfig", "DEFAULT_CONFIG", diff --git a/src/globus_action_provider_tools/flask/api_helpers.py b/src/globus_action_provider_tools/flask/api_helpers.py deleted file mode 100644 index e9b761e..0000000 --- a/src/globus_action_provider_tools/flask/api_helpers.py +++ /dev/null @@ -1,324 +0,0 @@ -from __future__ import annotations - -import logging - -import flask -import globus_sdk -from flask import jsonify, request -from pydantic import ValidationError - -from globus_action_provider_tools.data_types import ( - ActionProviderDescription, - ActionStatusValue, -) -from globus_action_provider_tools.flask.exceptions import ( - ActionNotFound, - ActionProviderError, - UnauthorizedRequest, -) -from globus_action_provider_tools.flask.helpers import ( - FlaskAuthStateBuilder, - action_status_return_to_view_return, - assign_json_provider, - blueprint_error_handler, - get_input_body_validator, - parse_query_args, - query_args_to_enum, - validate_input, -) -from globus_action_provider_tools.flask.types import ( - ActionCancelCallback, - ActionEnumerationCallback, - ActionLogCallback, - ActionReleaseCallback, - ActionRunCallback, - ActionStatusCallback, - ViewReturn, -) -from globus_action_provider_tools.validation import ( - ValidationRequest, - ValidationResult, - request_validator, -) - -# this deprecation warning was added in a past version, but never acted upon -# remove the warning until we have clarity on the maintenance status of this module -# -# warnings.warn( -# ( -# "The globus_action_provider_tools.flask.api_helpers module is deprecated " -# "and will be removed in 0.12.0. Please consider using the " -# "globus_action_provider_tools.flask.apt_blueprint module instead." -# ), -# DeprecationWarning, -# stacklevel=2, -# ) - - -_request_schema_types = {"run": "ActionRequest"} - -_response_schema_types = { - "run": "ActionStatus", - "status": "ActionStatus", - "cancel": "ActionStatus", - "release": "ActionStatus", -} - -log = logging.getLogger(__name__) - - -def _api_operation_for_request(request: flask.Request) -> str: - method: str = request.path.rsplit("/", 1) - op_name: str = method[-1] - return op_name - - -def flask_validate_request(request: flask.Request) -> ValidationResult: - request_dict = request.get_json(force=True, silent=True) - op_name = _api_operation_for_request(request) - doc_type = _request_schema_types.get(op_name) - if doc_type is not None: - validation_request = ValidationRequest( - provider_doc_type=doc_type, request_data=request_dict - ) - return request_validator(validation_request) - else: - return ValidationResult(errors=[], error_msg=None) - - -def flask_validate_response( - request: flask.Request, response: flask.Response -) -> ValidationResult: - response_data = response.get_json(force=True, silent=True) - op_name = _api_operation_for_request(request) - doc_type = _response_schema_types.get(op_name) - # Only do response validation if it is a successful response status - if doc_type is not None and (200 <= response.status_code < 299): - validation_request = ValidationRequest( - provider_doc_type=doc_type, request_data=response_data - ) - return request_validator(validation_request) - else: - return ValidationResult(errors=[], error_msg=None) - - -def add_action_routes_to_blueprint( - blueprint: flask.Blueprint, - client_id: str, - client_secret: str, - provider_description: ActionProviderDescription, - action_run_callback: ActionRunCallback, - action_status_callback: ActionStatusCallback, - action_cancel_callback: ActionCancelCallback, - action_release_callback: ActionReleaseCallback, - action_log_callback: ActionLogCallback | None = None, - additional_scopes: list[str] | None = None, - action_enumeration_callback: ActionEnumerationCallback | None = None, -) -> None: - """ - Add routes to a Flask Blueprint to implement the required operations of the Action - Provider Interface: Introspect, Run, Status, Cancel and Release. The route handlers - added to the blueprint perform basic functionality such as input validation and - authorization checks where appropriate, and use the provided callbacks to implement - the action provider specific functionality. See description of each callback below - for a description of functionality performed prior to invoking the callback. - - **Parameters** - - ``blueprint`` (*Flask.Blueprint*) - A flask blueprint to which routes for the URL paths '/', '/run', '/status', - '/cancel', and '/release' will be added. Optionally, (see below) '/log' may be added - as well. The blueprint should define a ``url_prefix`` to define a root to the paths - where these new paths will be added. In addition to the new URL paths, the blueprint - will also have a custom JSONEncoder associated with it to aid in the serialization - of data-types associated with these operations. - - ``client_id`` (*string*) - A Globus Auth registered ``client_id`` which will be used when validating input - request tokens. - - ``client_secret`` (*string*) - A Globus Auth generated ``client_secret`` which will be used when validating input - request tokens. - - ``provider_description`` (:class:`ActionProviderDescription\ - `) - A structure describing the provider to be returned by the provider introspection - operation (`GET /`). Some fields are also used in processing requests: the - `input_schema` field is used to validate the `body` of incoming action requests on - the `/run` operation. The `globus_auth_scope` value is used to validate the incoming - tokens on all requests. The `visible_to` and `runnable_by` lists are used to - authorization operations on the introspect (GET '/') and run (POST '/run') - operations respectively. The `log_supported` field should be `True` only if the - `action_log_callback` parameter is provided a value. - - ``action_run_callback`` (* Callable[[ActionRequest, AuthState], Union[ActionStatus, - Tuple[ActionStatus, int]]] *) - A function which will be called when an action /run invocation is called. Prior to - invoking the callback, the handler will validate the input conforms to the Action - Provider defined request format *and* that the input `body` matches the - `input_schema` defined in the `provider_description`. It will also authorize the - caller against the `runnable_by` property of the `provider_description`. In the case - of any validation or authorization errors, the corresponding werkzeug defined - exception will be raised. When validation and authorization succeed, the callback - will be invoked providing the `ActionRequest` structure corresponding to the request - and the authorization state (`AuthState`) of the caller. The callback should return - an `ActionStatus` value to be returned on the invocation. Optionally, a status - integer can be added to the return (making the return a (ActionStatus, int) tuple) - which defines the HTTP status code to be returned. This is useful in the case where - an existing request with the same id and body are seen which should return a 200 - HTTP status rather than the normal 201 HTTP status (which is the default when the - status code is not returned). - - """ - if additional_scopes: - all_accepted_scopes = additional_scopes + [ - provider_description.globus_auth_scope - ] - else: - all_accepted_scopes = [provider_description.globus_auth_scope] - - # FIXME: - # it needs to be possible to parametrize this client to control its network callout - # behavior, tuning retries and timeouts - auth_client = globus_sdk.ConfidentialAppAuthClient( - client_id=client_id, - client_secret=client_secret, - ) - state_builder = FlaskAuthStateBuilder( - auth_client=auth_client, expected_scopes=all_accepted_scopes - ) - - assign_json_provider(blueprint) - input_body_validator = get_input_body_validator(provider_description) - blueprint.register_error_handler(Exception, blueprint_error_handler) - - @blueprint.route("/", methods=["GET", "OPTIONS"], strict_slashes=False) - def action_introspect() -> ViewReturn: - # Short-circuit CORS requests. - if request.method == "OPTIONS": - response = flask.make_response("") - response.headers["Access-Control-Allow-Methods"] = "GET, OPTIONS" - response.headers["Access-Control-Allow-Origin"] = "*" - response.headers["Access-Control-Expose-Headers"] = "*" - return response, 204 - - # Check tokens if "public" is not in the *visible_to* list. - if "public" not in provider_description.visible_to: - auth_state = state_builder.build_from_request() - if not auth_state.check_authorization( - provider_description.visible_to, - allow_public=True, - allow_all_authenticated_users=True, - ): - raise ActionNotFound - - response = flask.make_response(jsonify(provider_description)) - response.headers["Access-Control-Allow-Origin"] = "*" - return response, 200 - - @blueprint.route("/actions", methods=["POST"]) - @blueprint.route("/run", methods=["POST"]) - def action_run() -> ViewReturn: - auth_state = state_builder.build_from_request() - if not auth_state.check_authorization( - provider_description.runnable_by, allow_all_authenticated_users=True - ): - log.info(f"Unauthorized call to action run, errors: {auth_state.errors}") - raise UnauthorizedRequest - if blueprint.url_prefix: - request.path = request.path.lstrip(blueprint.url_prefix) - if request.url_rule is not None: - request.url_rule.rule = request.url_rule.rule.lstrip( - blueprint.url_prefix - ) - - action_request = validate_input( - request.get_json(force=True), input_body_validator - ) - - # It's possible the user will attempt to make a malformed ActionStatus - - # pydantic won't like that. So log and handle the error with a 500 - try: - status = action_run_callback(action_request, auth_state) - except ValidationError as ve: - log.error( - f"ActionProvider attempted to create a non-conformant ActionStatus" - f" in {action_run_callback.__name__}: {ve.errors()}" - ) - raise ActionProviderError - - return action_status_return_to_view_return(status, 202) - - @blueprint.route("//status", methods=["GET"]) - @blueprint.route("/actions/", methods=["GET"]) - def action_status(action_id: str) -> ViewReturn: - auth_state = state_builder.build_from_request() - try: - status = action_status_callback(action_id, auth_state) # type: ignore - except ValidationError as ve: - log.error( - f"ActionProvider attempted to create a non-conformant ActionStatus" - f" in {action_status_callback.__name__}: {ve.errors()}" - ) - raise ActionProviderError - return action_status_return_to_view_return(status, 200) - - @blueprint.route("//cancel", methods=["POST"]) - @blueprint.route("/actions//cancel", methods=["POST"]) - def action_cancel(action_id: str) -> ViewReturn: - auth_state = state_builder.build_from_request() - try: - status = action_cancel_callback(action_id, auth_state) # type: ignore - except ValidationError as ve: - log.error( - f"ActionProvider attempted to create a non-conformant ActionStatus" - f" in {action_cancel_callback.__name__}: {ve.errors()}" - ) - raise ActionProviderError - return action_status_return_to_view_return(status, 200) - - @blueprint.route("//release", methods=["POST"]) - @blueprint.route("/actions/", methods=["DELETE"]) - def action_release(action_id: str) -> ViewReturn: - auth_state = state_builder.build_from_request() - try: - status = action_release_callback(action_id, auth_state) # type: ignore - except ValidationError as ve: - log.error( - f"ActionProvider attempted to create a non-conformant ActionStatus" - f" in {action_release_callback.__name__}: {ve.errors()}" - ) - raise ActionProviderError - return action_status_return_to_view_return(status, 200) - - if action_log_callback is not None: - - @blueprint.route("/actions//log", methods=["GET"]) - @blueprint.route("//log", methods=["GET"]) - def action_log(action_id: str) -> ViewReturn: - state_builder.build_from_request() - return jsonify({"log": "message"}), 200 - - if action_enumeration_callback is not None: - - @blueprint.route("/actions", methods=["GET"]) - def action_enumeration(): - auth_state = state_builder.build_from_request() - - valid_statuses = {e.name.casefold() for e in ActionStatusValue} - statuses = parse_query_args( - request, - arg_name="status", - default_value="active", - valid_vals=valid_statuses, - ) - statuses = query_args_to_enum(statuses, ActionStatusValue) - roles = parse_query_args( - request, - arg_name="roles", - default_value="creator_id", - valid_vals={"creator_id", "monitor_by", "manage_by"}, - ) - query_params = {"statuses": statuses, "roles": roles} - return jsonify(action_enumeration_callback(auth_state, query_params)), 200 diff --git a/tests/test_flask_helpers/app_utils.py b/tests/test_flask_helpers/app_utils.py index 29505d4..b884056 100644 --- a/tests/test_flask_helpers/app_utils.py +++ b/tests/test_flask_helpers/app_utils.py @@ -1,10 +1,3 @@ -""" -This module essentially contains the pieces required for creating a Flask app -using either the Decorator helper or the add_action_routes_to_blueprint helper. -These pieces are meant to be imported and used in a fixture to assemble the -Flask app according to what needs to be tested. -""" - from datetime import datetime, timezone from typing import Dict, List, Set diff --git a/tests/test_flask_helpers/conftest.py b/tests/test_flask_helpers/conftest.py index cb0c065..44e90b9 100644 --- a/tests/test_flask_helpers/conftest.py +++ b/tests/test_flask_helpers/conftest.py @@ -1,18 +1,14 @@ """ -This module contains fixtures for creating Flask app instances using the Flask -helpers with authentication mocked out. Each fixture creates an identical app, -the only difference being in the helper that is used to create the app. +This module contains fixtures for creating a Flask app using the +ActionProviderBlueprint, but with authentication mocked out. """ from unittest import mock import pytest -from flask import Blueprint, Flask +from flask import Flask -from globus_action_provider_tools.flask import ( - ActionProviderBlueprint, - add_action_routes_to_blueprint, -) +from globus_action_provider_tools.flask import ActionProviderBlueprint from globus_action_provider_tools.flask.helpers import assign_json_provider from .app_utils import ( @@ -63,35 +59,3 @@ def _create_app_from_blueprint(blueprint: ActionProviderBlueprint) -> Flask: return app return _create_app_from_blueprint - - -@pytest.fixture() -def add_routes_app(auth_state): - """ - This fixture creates a Flask app with routes loaded via the - add_action_routes_to_blueprint Flask helper. - """ - app = Flask(__name__) - assign_json_provider(app) - bp = Blueprint("func_helper", __name__, url_prefix="/func_helper") - add_action_routes_to_blueprint( - blueprint=bp, - client_id="bogus", - client_secret="bogus", - provider_description=ap_description, - action_run_callback=mock_action_run_func, - action_status_callback=mock_action_status_func, - action_cancel_callback=mock_action_cancel_func, - action_release_callback=mock_action_release_func, - action_log_callback=mock_action_log_func, - action_enumeration_callback=mock_action_enumeration_func, - additional_scopes=[ - "https://auth.globus.org/scopes/d3a66776-759f-4316-ba55-21725fe37323/secondary_scope" - ], - ) - app.register_blueprint(bp) - with mock.patch( - "globus_action_provider_tools.authentication.AuthStateBuilder.build", - return_value=auth_state, - ): - yield app diff --git a/tests/test_flask_helpers/test_routes.py b/tests/test_flask_helpers/test_routes.py index 766e4e9..4ed9986 100644 --- a/tests/test_flask_helpers/test_routes.py +++ b/tests/test_flask_helpers/test_routes.py @@ -19,22 +19,20 @@ ) -@pytest.mark.parametrize("app_fixture", ["aptb_app", "add_routes_app"]) @pytest.mark.parametrize("api_version", ["1.0", "1.1"]) @pytest.mark.parametrize("use_pydantic_schema", [True, False]) def test_routes_conform_to_api( - freeze_time, request, app_fixture: str, api_version: str, use_pydantic_schema: bool + freeze_time, request, aptb_app, api_version: str, use_pydantic_schema: bool ): freeze_time(load_response("token-introspect", case="success")) - app: flask.Flask = request.getfixturevalue(app_fixture) - _, bp = list(app.blueprints.items())[0] + _, bp = list(aptb_app.blueprints.items())[0] if use_pydantic_schema: bp.input_schema = ActionProviderPydanticInputSchema else: bp.input_schema = action_provider_json_input_schema client = ActionProviderClient( - app.test_client(), bp.url_prefix, api_version=api_version + aptb_app.test_client(), bp.url_prefix, api_version=api_version ) introspect_resp = client.introspect() @@ -47,13 +45,11 @@ def test_routes_conform_to_api( client.release(action_id) -@pytest.mark.parametrize("app_fixture", ["aptb_app", "add_routes_app"]) -def test_introspect_cors_requests(request, app_fixture): +def test_introspect_cors_requests(request, aptb_app): """Verify that CORS requests are allowed on introspect routes.""" - app: flask.Flask = request.getfixturevalue(app_fixture) - client: flask.testing.FlaskClient = app.test_client() - _, bp = list(app.blueprints.items())[0] + client: flask.testing.FlaskClient = aptb_app.test_client() + _, bp = list(aptb_app.blueprints.items())[0] introspection_cors_response = client.options(bp.url_prefix) assert introspection_cors_response.status_code == 204 From 411942339e8cd2e9ab7d0e6f136414db2aabf301 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Wed, 30 Oct 2024 11:58:20 -0500 Subject: [PATCH 06/15] Remove 'check-json' hook (no matching files) --- .pre-commit-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3e30b9d..9f5c0ee 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -16,7 +16,6 @@ repos: - id: trailing-whitespace - id: end-of-file-fixer - id: check-yaml - - id: check-json - id: check-added-large-files - repo: https://github.com/sirosen/texthooks From 756cfedfd125963bf95e1eee2762143a9077ed3d Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Wed, 30 Oct 2024 12:09:24 -0500 Subject: [PATCH 07/15] Update changelog.d/20241030_100211_sirosen_remove_api_helpers.rst Co-authored-by: Kurt McKee --- changelog.d/20241030_100211_sirosen_remove_api_helpers.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/changelog.d/20241030_100211_sirosen_remove_api_helpers.rst b/changelog.d/20241030_100211_sirosen_remove_api_helpers.rst index 6139fd8..07ff16a 100644 --- a/changelog.d/20241030_100211_sirosen_remove_api_helpers.rst +++ b/changelog.d/20241030_100211_sirosen_remove_api_helpers.rst @@ -2,8 +2,7 @@ Changes ------- - Remove the ``globus_action_provider_tools.flask.api_helpers`` module, and the - helpers it provided. Most users are not using these tools and should - experience no impact. + helpers it provided. - Remove examples from documentation which relied upon the ``api_helpers`` module. From 552c0c5e8150eb052c0aa8c1a8f3c321ae86e1c9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 1 Nov 2024 07:23:34 +0000 Subject: [PATCH 08/15] Bump the github-actions group with 4 updates Bumps the github-actions group with 4 updates: [actions/checkout](https://github.com/actions/checkout), [actions/setup-python](https://github.com/actions/setup-python), [actions/cache](https://github.com/actions/cache) and [actions/upload-artifact](https://github.com/actions/upload-artifact). Updates `actions/checkout` from 4.2.0 to 4.2.2 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/d632683dd7b4114ad314bca15554477dd762a938...11bd71901bbe5b1630ceea73d27597364c9af683) Updates `actions/setup-python` from 5.2.0 to 5.3.0 - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](https://github.com/actions/setup-python/compare/f677139bbe7f9c59b41e40162b753c062f5d49a3...0b93645e9fea7318ecaed2b359559ac225c90a2b) Updates `actions/cache` from 4.0.2 to 4.1.2 - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](https://github.com/actions/cache/compare/0c45773b623bea8c8e75f6c82b208c3cf94ea4f9...6849a6489940f00c2f30c0fb92c6274307ccb58a) Updates `actions/upload-artifact` from 4.4.0 to 4.4.3 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/50769540e7f4bd5e21e526ee35c689e35e0d6874...b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: actions/setup-python dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: actions/cache dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions ... Signed-off-by: dependabot[bot] --- .github/workflows/ci.yaml | 18 +++++++++--------- .github/workflows/release.yml | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5490201..667f972 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,11 +15,11 @@ jobs: runs-on: "ubuntu-latest" steps: - name: "Checkout the repo" - uses: "actions/checkout@d632683dd7b4114ad314bca15554477dd762a938" # v4.2.0 + uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683" # v4.2.2 - name: "Setup Python" id: "setup-python" - uses: "actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3" # v5.2.0 + uses: "actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b" # v5.3.0 with: python-version: "3.12" @@ -28,7 +28,7 @@ jobs: - name: "Restore the cache" id: "restore-cache" - uses: "actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9" # v4.0.2 + uses: "actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a" # v4.1.2 with: path: | .mypy_cache/ @@ -58,7 +58,7 @@ jobs: wheel-filename: "${{ steps.build-wheel.outputs.wheel-filename }}" steps: - name: "Checkout the repo" - uses: "actions/checkout@d632683dd7b4114ad314bca15554477dd762a938" # v4.2.0 + uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683" # v4.2.2 - name: "Identify the week number" run: | @@ -67,7 +67,7 @@ jobs: - name: "Setup Python" id: "setup-python" - uses: "actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3" # v5.2.0 + uses: "actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b" # v5.3.0 with: python-version: "3.11" cache: "pip" @@ -84,7 +84,7 @@ jobs: echo "wheel-filename=$(find globus_action_provider_tools-*.whl | head -n 1)" >> "$GITHUB_OUTPUT" - name: "Upload the artifact" - uses: "actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874" # v4.4.0 + uses: "actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882" # v4.4.3 with: name: "globus_action_provider_tools-${{ github.sha }}.whl" path: "${{ steps.build-wheel.outputs.wheel-filename }}" @@ -138,7 +138,7 @@ jobs: steps: - name: "Checkout the repo" - uses: "actions/checkout@d632683dd7b4114ad314bca15554477dd762a938" # v4.2.0 + uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683" # v4.2.2 - name: "Identify the week number" shell: "bash" @@ -148,7 +148,7 @@ jobs: - name: "Setup Python" id: "setup-python" - uses: "actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3" # v5.2.0 + uses: "actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b" # v5.3.0 with: python-version: "${{ matrix.python-version }}" cache: "pip" @@ -160,7 +160,7 @@ jobs: - name: "Restore cache" id: "restore-cache" - uses: "actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9" # v4.0.2 + uses: "actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a" # v4.1.2 with: path: | .tox/ diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 684e33b..b98183f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -11,12 +11,12 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout source code - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: fetch-depth: 1 - name: Set target python version - uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0 + uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0 with: python-version: "3.11" From 82ff19a5da6010e36d10869f44f2b3baf2a13447 Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Wed, 6 Nov 2024 11:23:56 -0600 Subject: [PATCH 09/15] Pin werkzeug when testing the minimum Flask version --- pyproject.toml | 4 ++-- tox.ini | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 0c0249a..62cf71e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,8 +84,8 @@ fail_under = 83 filterwarnings = [ "error", - # Minimum Flask versions interact with werkzeug in a now-deprecated manner. - "ignore:The '__version__' attribute is deprecated:DeprecationWarning", + # Minimum Flask versions use a werkzeug version that triggers AST warnings. + "ignore:.+? is deprecated and will be removed:DeprecationWarning:werkzeug|ast", # dateutil, used by freezegun during testing, has a Python 3.12 compatibility issue. "ignore:datetime.datetime.utcfromtimestamp\\(\\) is deprecated:DeprecationWarning", diff --git a/tox.ini b/tox.ini index 96f7319..2752a9a 100644 --- a/tox.ini +++ b/tox.ini @@ -28,6 +28,7 @@ extras = deps = -r requirements/test/requirements.txt minimum_flask: flask==2.3.0 + minimum_flask: werkzeug==2.3.0 commands = coverage run -m pytest {posargs} From 69b11700b36bf1d93f576631db37cabbe6a44f88 Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Wed, 6 Nov 2024 11:29:06 -0600 Subject: [PATCH 10/15] Rename the test workflow to better reflect its purpose --- .github/workflows/{ci.yaml => test.yaml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .github/workflows/{ci.yaml => test.yaml} (100%) diff --git a/.github/workflows/ci.yaml b/.github/workflows/test.yaml similarity index 100% rename from .github/workflows/ci.yaml rename to .github/workflows/test.yaml From 5a45735fc3a7af378dff5a36d9b2baa23a19a67f Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Wed, 6 Nov 2024 11:46:13 -0600 Subject: [PATCH 11/15] Migrate to a reusable workflow --- .github/workflows/test.yaml | 241 +++++++++--------------------------- .pre-commit-config.yaml | 2 +- tox.ini | 8 +- 3 files changed, 62 insertions(+), 189 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 667f972..1e79f69 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -1,196 +1,69 @@ -name: "CI" +name: "๐Ÿงช Test" + on: - pull_request: + pull_request: null push: branches: - "main" - "production" -env: - PRE_COMMIT_HOME: ".tox/pre-commit-home" - jobs: - lint: - name: "Lint" - runs-on: "ubuntu-latest" - steps: - - name: "Checkout the repo" - uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683" # v4.2.2 - - - name: "Setup Python" - id: "setup-python" - uses: "actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b" # v5.3.0 - with: - python-version: "3.12" - - - name: "Detect Pythons" - uses: "kurtmckee/detect-pythons@38187a5464f266e93e5c1467699f7be2bf521d2e" # v1.1.0 - - - name: "Restore the cache" - id: "restore-cache" - uses: "actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a" # v4.1.2 - with: - path: | - .mypy_cache/ - .tox/ - .venv/ - key: "lint-hash=${{ hashFiles('.github/workflows/ci.yaml', '.python-identifiers', 'pyproject.toml', 'tox.ini', 'requirements/*/*.txt') }}" - - - name: "Create virtual environment" - if: "steps.restore-cache.outputs.cache-hit == false" - run: | - python -m venv .venv - .venv/bin/pip install --upgrade pip setuptools wheel - .venv/bin/pip install tox - - - name: "Lint type annotations" - run: | - .venv/bin/tox -e mypy - - - name: "Lint documentation" - run: | - .venv/bin/tox -e docs - - build: - name: "Build a shared wheel" - runs-on: "ubuntu-latest" - outputs: - wheel-filename: "${{ steps.build-wheel.outputs.wheel-filename }}" - steps: - - name: "Checkout the repo" - uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683" # v4.2.2 - - - name: "Identify the week number" - run: | - date +'%U' > week-number.txt - date +'week-number=%U' >> "$GITHUB_ENV" - - - name: "Setup Python" - id: "setup-python" - uses: "actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b" # v5.3.0 - with: - python-version: "3.11" - cache: "pip" - cache-dependency-path: | - .github/workflows/ci.yaml - pyproject.toml - tox.ini - week-number.txt - - - name: "Build the wheel" - id: "build-wheel" - run: | - pip wheel . - echo "wheel-filename=$(find globus_action_provider_tools-*.whl | head -n 1)" >> "$GITHUB_OUTPUT" - - - name: "Upload the artifact" - uses: "actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882" # v4.4.3 - with: - name: "globus_action_provider_tools-${{ github.sha }}.whl" - path: "${{ steps.build-wheel.outputs.wheel-filename }}" - retention-days: 1 - test: - name: "Test ${{ matrix.python-version }}${{ matrix.tox-extras }} on ${{ matrix.os.name }}" - needs: ["build"] - runs-on: "${{ matrix.os.value }}" - + name: "${{ matrix.name }}" strategy: + fail-fast: false matrix: - # Broadly test supported Python versions on Linux. - os: - - name: "Linux" - value: "ubuntu-latest" - python-version: - - "3.8" - - "3.9" - - "3.10" - - "3.11" - - "3.12" - tox-extras: [""] + # These names establish the number of jobs that will run, + # and the actual job configurations will be added by matching "name" keys + # in the "include" section below. + name: + - "Linux" + - "macOS" + - "Windows" + - "Quality" + + # The nested list in this single-item list will be added to each job above. + cache-key-hash-files: + - + - "pyproject.toml" + - "requirements/*/requirements.txt" include: - # Test Windows. - - os: - name: "Windows" - value: "windows-latest" - python-version: "3.12" - tox-extras: "" - - # Test Mac. - - os: - name: "Mac" - value: "macos-latest" - python-version: "3.12" - tox-extras: "" - - # Test minimum dependencies. - - os: - name: "Linux" - value: "ubuntu-latest" - python-version: "3.8" - tox-extras: "-minimum_flask" - - os: - name: "Linux" - value: "ubuntu-latest" - python-version: "3.12" - tox-extras: "-minimum_flask" - - steps: - - name: "Checkout the repo" - uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683" # v4.2.2 - - - name: "Identify the week number" - shell: "bash" - run: | - date +'%U' > week-number.txt - date +'week-number=%U' >> "$GITHUB_ENV" - - - name: "Setup Python" - id: "setup-python" - uses: "actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b" # v5.3.0 - with: - python-version: "${{ matrix.python-version }}" - cache: "pip" - cache-dependency-path: | - .github/workflows/ci.yaml - pyproject.toml - tox.ini - week-number.txt - - - name: "Restore cache" - id: "restore-cache" - uses: "actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a" # v4.1.2 - with: - path: | - .tox/ - .venv/ - key: > - test - week=${{ env.week-number }} - os=${{ matrix.os.value }} - python=${{ steps.setup-python.outputs.python-version }} - hash=${{ hashFiles('.github/workflows/ci.yaml', 'pyproject.toml', 'tox.ini') }} - - - name: "Identify virtual environment path" - shell: "bash" - run: | - echo 'venv-path=${{ runner.os == 'Windows' && '.venv/Scripts' || '.venv/bin' }}' >> "$GITHUB_ENV" - - - name: "Install tox" - if: "steps.restore-cache.outputs.cache-hit == false" - run: | - python -m venv .venv - ${{ env.venv-path }}/pip install --upgrade pip setuptools wheel - ${{ env.venv-path }}/pip install tox - - - name: "Download the artifact" - uses: "actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16" # v4.1.8 - with: - name: "globus_action_provider_tools-${{ github.sha }}.whl" - - - name: "Test" - run: > - ${{ env.venv-path }}/tox run - --installpkg="${{ needs.build.outputs.wheel-filename }}" - -e py${{ matrix.python-version}}${{ matrix.tox-extras }} + - name: "Linux" + runner: "ubuntu-latest" + cpythons: + - "3.8" + - "3.9" + - "3.10" + - "3.11" + - "3.12" + tox-environments-from-pythons: true + tox-post-environments: + - "py3.8-minimum_flask" + - "py3.12-minimum_flask" + + - name: "macOS" + runner: "macos-latest" + cpythons: + - "3.12" + tox-environments-from-pythons: true + + - name: "Windows" + runner: "windows-latest" + cpythons: + - "3.12" + tox-environments-from-pythons: true + + - name: "Quality" + runner: "ubuntu-latest" + cpythons: + - "3.12" # This must match the Read the Docs interpreter version. + tox-environments: + - "docs" + - "mypy" + cache-paths: + - ".mypy_cache/" + + uses: "globus/workflows/.github/workflows/tox.yaml@04b4abd6fcb9b4be7263bc9d6994ae2ada220739" # v1.1 + with: + config: "${{ toJSON(matrix) }}" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9f5c0ee..fe957af 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -53,7 +53,7 @@ repos: - id: slyp - repo: https://github.com/rhysd/actionlint - rev: v1.7.3 + rev: v1.7.4 hooks: - id: actionlint diff --git a/tox.ini b/tox.ini index 2752a9a..302fb3b 100644 --- a/tox.ini +++ b/tox.ini @@ -4,8 +4,8 @@ envlist = mypy coverage_erase - py{38, 39, 310, 311, 312} - py{38, 312}-minimum_flask + py{3.8, 3.9, 3.10, 3.11, 3.12} + py{3.8, 3.12}-minimum_flask coverage_report docs labels = @@ -22,7 +22,7 @@ commands = coverage erase package = wheel wheel_build_env = build_wheel depends = - py{312, 311, 310, 39, 38}{-minimum_flask,}: coverage_erase + py{3.8, 3.9, 3.10, 3.11, 3.12}{-minimum_flask,}: coverage_erase extras = !minimum_flask: flask deps = @@ -34,7 +34,7 @@ commands = coverage run -m pytest {posargs} [testenv:coverage_report] depends = - py{312, 311, 310, 39, 38}{-minimum_flask,} + py{3.8, 3.9, 3.10, 3.11, 3.12}{-minimum_flask,} skip_install = true deps = coverage[toml] commands_pre = From 6173ebfa05b7bdd676cd90a7010afaaaa1801b4b Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Thu, 7 Nov 2024 11:09:07 -0600 Subject: [PATCH 12/15] Re-order the tool configurations; add headers for visibility The tool configs are now alphabetized, with the exception of poetry. Poetry defines the project metadata, so it is listed first. --- pyproject.toml | 84 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 30 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 62cf71e..8dea76e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,11 @@ +[build-system] +requires = ["poetry-core>=1.0.0"] +build-backend = "poetry.core.masonry.api" + + +# poetry +# ------ + [tool.poetry] name = "globus-action-provider-tools" version = "0.19.1" @@ -39,29 +47,9 @@ flask = {version = "^2.3.0", optional = true} [tool.poetry.extras] flask = ["flask"] -[tool.isort] -multi_line_output = 3 -include_trailing_comma = true -force_grid_wrap = 0 -use_parentheses = true -line_length = 88 - -[build-system] -requires = ["poetry-core>=1.0.0"] -build-backend = "poetry.core.masonry.api" -[tool.scriv] -categories = [ - "Python support", - "Features", - "Bugfixes", - "Changes", - "Documentation", - "Deprecations", - "Development", - "Dependencies", -] -version = "literal: pyproject.toml: tool.poetry.version" +# coverage +# -------- [tool.coverage.run] branch = true @@ -80,16 +68,20 @@ source = [ # When the test coverage increases, this bar should also raise. fail_under = 83 -[tool.pytest.ini_options] -filterwarnings = [ - "error", - # Minimum Flask versions use a werkzeug version that triggers AST warnings. - "ignore:.+? is deprecated and will be removed:DeprecationWarning:werkzeug|ast", +# isort +# ----- + +[tool.isort] +multi_line_output = 3 +include_trailing_comma = true +force_grid_wrap = 0 +use_parentheses = true +line_length = 88 - # dateutil, used by freezegun during testing, has a Python 3.12 compatibility issue. - "ignore:datetime.datetime.utcfromtimestamp\\(\\) is deprecated:DeprecationWarning", -] + +# mypy +# ---- [tool.mypy] sqlite_cache = true @@ -102,3 +94,35 @@ warn_return_any = true warn_no_return = true no_implicit_optional = true # disallow_untyped_defs = true + + +# pytest +# ------ + +[tool.pytest.ini_options] +filterwarnings = [ + "error", + + # Minimum Flask versions use a werkzeug version that triggers AST warnings. + "ignore:.+? is deprecated and will be removed:DeprecationWarning:werkzeug|ast", + + # dateutil, used by freezegun during testing, has a Python 3.12 compatibility issue. + "ignore:datetime.datetime.utcfromtimestamp\\(\\) is deprecated:DeprecationWarning", +] + + +# scriv +# ----- + +[tool.scriv] +categories = [ + "Python support", + "Features", + "Bugfixes", + "Changes", + "Documentation", + "Deprecations", + "Development", + "Dependencies", +] +version = "literal: pyproject.toml: tool.poetry.version" From e26ef95f9a07ff59ff67887ff46e84089b6bcd2f Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Thu, 7 Nov 2024 11:12:58 -0600 Subject: [PATCH 13/15] Add new scriv categories and order them by importance --- .../20241107_111041_kurtmckee_update_scriv_categories.rst | 7 +++++++ pyproject.toml | 6 ++++-- 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 changelog.d/20241107_111041_kurtmckee_update_scriv_categories.rst diff --git a/changelog.d/20241107_111041_kurtmckee_update_scriv_categories.rst b/changelog.d/20241107_111041_kurtmckee_update_scriv_categories.rst new file mode 100644 index 0000000..fd3c2f2 --- /dev/null +++ b/changelog.d/20241107_111041_kurtmckee_update_scriv_categories.rst @@ -0,0 +1,7 @@ +Development +----------- + +- Introduce new scriv categories to better communicate how the project evolves. + + The categories are also re-ordered, + which defines how fragments will be ordered in the CHANGELOG. diff --git a/pyproject.toml b/pyproject.toml index 8dea76e..a4b6562 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -116,13 +116,15 @@ filterwarnings = [ [tool.scriv] categories = [ + "Breaking changes", + "Security", + "Deprecations", "Python support", + "Dependencies", "Features", "Bugfixes", "Changes", "Documentation", - "Deprecations", "Development", - "Dependencies", ] version = "literal: pyproject.toml: tool.poetry.version" From 0a88efb368fadc781a25aa6785661aa6dd41ad67 Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Thu, 7 Nov 2024 12:28:21 -0600 Subject: [PATCH 14/15] Add a changelog fragment template The template adds instructive text to be filled out for "Breaking changes". --- ...2830_kurtmckee_update_scriv_categories.rst | 4 ++++ changelog.d/fragment-template.rst.txt | 20 +++++++++++++++++++ pyproject.toml | 1 + 3 files changed, 25 insertions(+) create mode 100644 changelog.d/20241107_122830_kurtmckee_update_scriv_categories.rst create mode 100644 changelog.d/fragment-template.rst.txt diff --git a/changelog.d/20241107_122830_kurtmckee_update_scriv_categories.rst b/changelog.d/20241107_122830_kurtmckee_update_scriv_categories.rst new file mode 100644 index 0000000..f315986 --- /dev/null +++ b/changelog.d/20241107_122830_kurtmckee_update_scriv_categories.rst @@ -0,0 +1,4 @@ +Development +----------- + +* Add a changelog fragment template. diff --git a/changelog.d/fragment-template.rst.txt b/changelog.d/fragment-template.rst.txt new file mode 100644 index 0000000..462ada2 --- /dev/null +++ b/changelog.d/fragment-template.rst.txt @@ -0,0 +1,20 @@ +.. + + De-dent what you need. + Delete everything you don't. +{% for category in config.categories %} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + {{ category }} + {{ config.rst_header_chars[1] * (category|length) }} + + * Describe your "{{ category }}" change here. EDIT ME! + +{%- if category.title() == "Breaking Changes" %} + + Document what steps must be taken to account for the breaking change. + If there are multiple breaking changes, + list them in separate bullet points or create additional changelog fragments. + +{%- endif %} +{% endfor %} diff --git a/pyproject.toml b/pyproject.toml index a4b6562..ea3a3e6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -128,3 +128,4 @@ categories = [ "Development", ] version = "literal: pyproject.toml: tool.poetry.version" +new_fragment_template = "file: fragment-template.rst.txt" From 84734fa2f6b6c65d5c3aeec95930d6530b2c98c8 Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Thu, 7 Nov 2024 13:35:51 -0600 Subject: [PATCH 15/15] Bump version for release v0.20.0 --- CHANGELOG.rst | 68 +++++++++++++++++++ ...kurtmckee_eliminate_none_return_values.rst | 14 ---- ..._172707_sirosen_improve_groups_caching.rst | 7 -- ...5333_sirosen_do_not_use_refresh_tokens.rst | 14 ---- ...1030_100211_sirosen_remove_api_helpers.rst | 8 --- ...1041_kurtmckee_update_scriv_categories.rst | 7 -- ...2830_kurtmckee_update_scriv_categories.rst | 4 -- pyproject.toml | 2 +- 8 files changed, 69 insertions(+), 55 deletions(-) delete mode 100644 changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst delete mode 100644 changelog.d/20241025_172707_sirosen_improve_groups_caching.rst delete mode 100644 changelog.d/20241025_185333_sirosen_do_not_use_refresh_tokens.rst delete mode 100644 changelog.d/20241030_100211_sirosen_remove_api_helpers.rst delete mode 100644 changelog.d/20241107_111041_kurtmckee_update_scriv_categories.rst delete mode 100644 changelog.d/20241107_122830_kurtmckee_update_scriv_categories.rst diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 462e923..3cee68c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,6 +10,74 @@ Unreleased changes are documented in files in the `changelog.d`_ directory. .. scriv-insert-here +.. _changelog-0.20.0: + +0.20.0 โ€” 2024-11-07 +=================== + +Breaking changes +---------------- + +* Remove the ``globus_action_provider_tools.flask.api_helpers`` module, + and the helpers it provided. + + If possible, it is recommended to immediately migrate Action Providers + off of the code in the Flask API helpers module. + + If this cannot be done immediately, it is recommended to pin + the Action Provider Tools dependency to ``0.19.1``. + +Deprecations +------------ + +* The ``required_authorizer_expiration_time`` parameter to ``get_authorizer_for_scope`` is deprecated. + + Given token expiration and caching lifetimes, + it was not possible for this parameter to have any effect based on its prior documented usage. + +Bugfixes +-------- + +* Action Provider Tools no longer requests Dependent Refresh Tokens + if Access Tokens are sufficient. As a result of this fix, + the AuthState dependent token cache will never contain dependent refresh tokens. + +Changes +------- + +* ``AuthState.introspect_token()`` will no longer return ``None`` + if the token is not active. + + Instead, a new exception, ``InactiveTokenError``, will be raised. + ``InactiveTokenError`` is a subclass of ``ValueError``. + + Code that calls ``AuthState.introspect_token()`` no longer returns ``None``, either, + but will instead raise ``ValueError`` (or a subclass) or a ``globus_sdk.GlobusAPIError``: + + * ``AuthState.get_authorizer_for_scope`` + * ``AuthState.effective_identity`` + * ``AuthState.identities`` + +* Group caching behavior in the ``AuthState`` class has been improved + to ensure that the cache is checked before any external operations + (e.g., dependent token callouts) are required. + The cache now uses the token hash as its key, rather than a dependent token. + +Documentation +------------- + +* Remove examples from documentation which relied upon the ``api_helpers`` module. + +Development +----------- + +* Introduce new scriv categories to better communicate how the project evolves. + + The categories are also re-ordered, + which defines how fragments will be ordered in the CHANGELOG. + +* Add a changelog fragment template. + .. _changelog-0.19.1: 0.19.1 โ€” 2024-10-22 diff --git a/changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst b/changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst deleted file mode 100644 index e8ac24d..0000000 --- a/changelog.d/20241018_082221_kurtmckee_eliminate_none_return_values.rst +++ /dev/null @@ -1,14 +0,0 @@ -Changes -------- - -- ``AuthState.introspect_token()`` will no longer return ``None`` if the token is not active. - - Instead, a new exception, ``InactiveTokenError``, will be raised. - ``InactiveTokenError`` is a subclass of ``ValueError``. - - Existing code that calls ``AuthState.introspect_token()`` no longer returns ``None``, either, - but will instead raise ``ValueError`` (or a subclass) or a ``globus_sdk.GlobusAPIError``: - - * ``AuthState.get_authorizer_for_scope`` - * ``AuthState.effective_identity`` - * ``AuthState.identities`` diff --git a/changelog.d/20241025_172707_sirosen_improve_groups_caching.rst b/changelog.d/20241025_172707_sirosen_improve_groups_caching.rst deleted file mode 100644 index f54feb5..0000000 --- a/changelog.d/20241025_172707_sirosen_improve_groups_caching.rst +++ /dev/null @@ -1,7 +0,0 @@ -Changes -------- - -- Group caching behavior in the ``AuthState`` class has been improved to ensure - that the cache is checked before any external operations (e.g., dependent - token callouts) are required. The cache now uses the token hash as its key, - rather than a dependent token. diff --git a/changelog.d/20241025_185333_sirosen_do_not_use_refresh_tokens.rst b/changelog.d/20241025_185333_sirosen_do_not_use_refresh_tokens.rst deleted file mode 100644 index 1e35e76..0000000 --- a/changelog.d/20241025_185333_sirosen_do_not_use_refresh_tokens.rst +++ /dev/null @@ -1,14 +0,0 @@ -Bugfixes --------- - -- Action Provider Tools no longer requests Dependent Refresh Tokens in - contexts where Access Tokens are sufficient. As a result of this fix, the - AuthState dependent token cache will never contain dependent refresh tokens. - -Deprecations ------------- - -- The ``required_authorizer_expiration_time`` parameter to - ``get_authorizer_for_scope`` is deprecated. Given token expiration and - caching lifetimes, it was not possible for this parameter to have any effect - based on its prior documented usage. diff --git a/changelog.d/20241030_100211_sirosen_remove_api_helpers.rst b/changelog.d/20241030_100211_sirosen_remove_api_helpers.rst deleted file mode 100644 index 07ff16a..0000000 --- a/changelog.d/20241030_100211_sirosen_remove_api_helpers.rst +++ /dev/null @@ -1,8 +0,0 @@ -Changes -------- - -- Remove the ``globus_action_provider_tools.flask.api_helpers`` module, and the - helpers it provided. - -- Remove examples from documentation which relied upon the ``api_helpers`` - module. diff --git a/changelog.d/20241107_111041_kurtmckee_update_scriv_categories.rst b/changelog.d/20241107_111041_kurtmckee_update_scriv_categories.rst deleted file mode 100644 index fd3c2f2..0000000 --- a/changelog.d/20241107_111041_kurtmckee_update_scriv_categories.rst +++ /dev/null @@ -1,7 +0,0 @@ -Development ------------ - -- Introduce new scriv categories to better communicate how the project evolves. - - The categories are also re-ordered, - which defines how fragments will be ordered in the CHANGELOG. diff --git a/changelog.d/20241107_122830_kurtmckee_update_scriv_categories.rst b/changelog.d/20241107_122830_kurtmckee_update_scriv_categories.rst deleted file mode 100644 index f315986..0000000 --- a/changelog.d/20241107_122830_kurtmckee_update_scriv_categories.rst +++ /dev/null @@ -1,4 +0,0 @@ -Development ------------ - -* Add a changelog fragment template. diff --git a/pyproject.toml b/pyproject.toml index ea3a3e6..2470290 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ build-backend = "poetry.core.masonry.api" [tool.poetry] name = "globus-action-provider-tools" -version = "0.19.1" +version = "0.20.0" description = "Tools to help developers build services that implement the Action Provider specification." authors = [ "Globus Team ",