Eagerly check token validity and handle invalid tokens in flask helpers

changelog.d/20250207_122449_sirosen_eagerly_check_authn.rst

@@ -0,0 +1,18 @@
+Breaking changes
+----------------
+
+*   The ``AuthState`` object now introspects its token when initialized. This
+    results in more eager error behaviors, as a failed introspect will now
+    raise an error immediately, rather than on the first usage which triggers
+    an implicit introspect.
+
+    Callers who are explicitly handling invalid token errors, like
+    ``InactiveTokenError``, should put their handling around ``AuthState``
+    construction rather than around ``AuthState`` attribute and method usage.
+
+*   The provided ``FlaskAuthStateBuilder`` used by the provided flask blueprint
+    now handles ``InactiveTokenError`` and ``InvalidTokenScopesError`` and will
+    raise an ``AuthenticationError`` if these are encountered.
+    The error handler in the provided blueprint translates these into
+    ``UnauthorizedRequest`` exceptions, which render as HTTP 401 Unauthorized
+    responses.

src/globus_action_provider_tools/authentication.py

@@ -81,6 +81,8 @@ def __init__(
 
         self.errors: list[Exception] = []
 
+        self._token_data = self.introspect_token()
+
     @functools.cached_property
     def _token_hash(self) -> str:
         return _hash_token(self.bearer_token)
@@ -143,14 +145,12 @@ def _verify_introspect_result(self, introspect_result: GlobusHTTPResponse) -> No
 
     @property
     def effective_identity(self) -> str:
-        tkn_details = self.introspect_token()
-        effective = identity_principal(tkn_details["sub"])
+        effective = identity_principal(self._token_data["sub"])
         return effective
 
     @property
     def identities(self) -> frozenset[str]:
-        tkn_details = self.introspect_token()
-        return frozenset(map(identity_principal, tkn_details["identity_set"]))
+        return frozenset(map(identity_principal, self._token_data["identity_set"]))
 
     @property
     def principals(self) -> frozenset[str]:

src/globus_action_provider_tools/flask/helpers.py

@@ -12,7 +12,12 @@
 from flask import Request, current_app, jsonify
 from pydantic import BaseModel, ValidationError
 
-from globus_action_provider_tools.authentication import AuthState, AuthStateBuilder
+from globus_action_provider_tools.authentication import (
+    AuthState,
+    AuthStateBuilder,
+    InactiveTokenError,
+    InvalidTokenScopesError,
+)
 from globus_action_provider_tools.data_types import (
     ActionProviderDescription,
     ActionProviderJsonEncoder,
@@ -74,7 +79,12 @@ def build_from_request(self, *, request: Request | None = None) -> AuthState:
         if not 10 <= len(access_token) <= 2048:
             raise UnverifiedAuthenticationError("Bearer token length is unexpected")
 
-        return super().build(access_token)
+        try:
+            return super().build(access_token)
+        except InvalidTokenScopesError as err:
+            raise AuthenticationError("Token has invalid scopes") from err
+        except InactiveTokenError as err:
+            raise AuthenticationError("Token is invalid, expired, or revoked") from err
 
 
 def parse_query_args(

tests/api-fixtures/token-introspect.yaml

@@ -23,6 +23,7 @@ success:
       - 84b6fc41-022a-42c2-ae31-ab41cd94d08f
       - ecad897d-eb0b-4970-aa12-ac523c3567d9
       - 02d1f36c-a798-410b-95fe-3a9ea1a83610
+      - f7e81526-1610-47e2-a7c5-b071db77ca47
     effective-id: &success-effective-id
       f7e81526-1610-47e2-a7c5-b071db77ca47
 

tests/conftest.py

@@ -6,13 +6,13 @@
 from unittest import mock
 
 import freezegun
-import globus_sdk
 import pytest
 import responses
 import yaml
-from globus_sdk._testing import RegisteredResponse, register_response_set
+from globus_sdk._testing import RegisteredResponse, load_response, register_response_set
 
-from globus_action_provider_tools.authentication import AuthState, AuthStateBuilder
+from globus_action_provider_tools.authentication import AuthState
+from globus_action_provider_tools.client_factory import ClientFactory
 
 from .data import canned_responses
 
@@ -22,6 +22,14 @@
     collect_ignore = ["flask"]
 
 
+class NoRetryClientFactory(ClientFactory):
+    DEFAULT_AUTH_TRANSPORT_PARAMS = (("max_retries", 0),)
+    DEFAULT_GROUPS_TRANSPORT_PARAMS = (("max_retries", 0),)
+
+
+_NO_RETRY_FACTORY = NoRetryClientFactory()
+
+
 @pytest.fixture
 def config():
     return {
@@ -31,38 +39,69 @@ def config():
     }
 
 
+@pytest.fixture(autouse=True)
+def mocked_responses() -> responses.RequestsMock:
+    """Mock all requests.
+
+    The default `responses.mock` object is returned,
+    which allows tests to access various properties of the mock.
+    For example, they might check the number of intercepted `.calls`.
+    """
+
+    with responses.mock:
+        yield responses.mock
+
+
+@pytest.fixture
+def introspect_success_response(mocked_responses):
+    return load_response("token-introspect", case="success")
+
+
+@pytest.fixture
+def dependent_token_success_response(mocked_responses):
+    return load_response("token", case="success")
+
+
 @pytest.fixture
-@mock.patch("globus_sdk.ConfidentialAppAuthClient")
-def auth_state(MockAuthClient, config, monkeypatch) -> AuthState:
-    # FIXME: the comment below is a lie, assess and figure out what is being said
-    #
-    # Mock the introspection first because that gets called as soon as we create
-    # an AuthStateBuilder
-    client = MockAuthClient.return_value
-    client.oauth2_token_introspect.return_value = (
-        canned_responses.introspect_response()()
-    )
-
-    # Mock the dependent_tokens and list_groups functions bc they get used when
-    # creating a GroupsClient
-    client.oauth2_get_dependent_tokens.return_value = (
-        canned_responses.dependent_token_response()()
-    )
-    monkeypatch.setattr(
-        globus_sdk.GroupsClient, "get_my_groups", canned_responses.groups_response()
-    )
-
-    # Create an AuthStateBuilder to be used to create a mocked auth_state object
-    builder = AuthStateBuilder(client, expected_scopes=config["expected_scopes"])
-    auth_state = builder.build("NOT_A_TOKEN")
-
-    # Reset the call count because check_token implicitly calls oauth2_token_introspect
-    client.oauth2_token_introspect.call_count = 0
-
-    # Mock out this AuthState instance's GroupClient
-    # auth_state._groups_client = GroupsClient(authorizer=None)
-    # auth_state._groups_client.list_groups = canned_responses.groups_response()
-    return auth_state
+def groups_success_response(mocked_responses):
+    return load_response("groups-my_groups", case="success")
+
+
+@pytest.fixture
+def get_auth_state_instance() -> t.Callable[..., AuthState]:
+    def _func(
+        expected_scopes: t.Iterable[str],
+        client_factory: ClientFactory = _NO_RETRY_FACTORY,
+    ) -> AuthState:
+        client = client_factory.make_confidential_app_auth_client("bogus", "bogus")
+        return AuthState(
+            auth_client=client,
+            bearer_token="bogus",
+            expected_scopes=frozenset(expected_scopes),
+            client_factory=client_factory,
+        )
+
+    return _func
+
+
+@pytest.fixture(autouse=True)
+def _clear_auth_state_cache():
+    AuthState.dependent_tokens_cache.clear()
+    AuthState.group_membership_cache.clear()
+    AuthState.introspect_cache.clear()
+
+
+@pytest.fixture
+def auth_state(
+    mocked_responses,
+    get_auth_state_instance: t.Callable[..., AuthState],
+    introspect_success_response,
+    dependent_token_success_response,
+    groups_success_response,
+) -> AuthState:
+    """Create an AuthState instance."""
+    # note that expected-scope MUST match the fixture data
+    return get_auth_state_instance(["expected-scope"])
 
 
 @pytest.fixture
@@ -102,19 +141,6 @@ def register_api_fixtures():
         register_response_set(yaml_file.stem, response_set)
 
 
-@pytest.fixture(autouse=True)
-def mocked_responses() -> responses.RequestsMock:
-    """Mock all requests.
-
-    The default `responses.mock` object is returned,
-    which allows tests to access various properties of the mock.
-    For example, they might check the number of intercepted `.calls`.
-    """
-
-    with responses.mock:
-        yield responses.mock
-
-
 @pytest.fixture
 def freeze_time() -> (
     t.Generator[t.Callable[[RegisteredResponse], RegisteredResponse], None, None]

tests/flask/test_api_errors.py

@@ -1,12 +1,15 @@
 import typing as t
+from unittest import mock
 
 import pytest
 from pydantic import BaseModel
 
+from globus_action_provider_tools.authentication import InactiveTokenError
 from globus_action_provider_tools.flask import (
     ActionProviderBlueprint,
     ActionProviderConfig,
 )
+from globus_action_provider_tools.flask.helpers import FlaskAuthStateBuilder
 from tests.flask.ap_client import ActionProviderClient
 from tests.flask.app_utils import ap_description
 
@@ -97,3 +100,35 @@ def test_validation_errors__WHEN_scrubbing_is_disabled(create_app_from_blueprint
 
     expected = "Field '$.foo' (category: 'type'): 'not-an-int' is not of type 'integer'"
     assert resp.json["description"] == expected
+
+
+def test_invalid_token_results_in_401_unauthorized(create_app_from_blueprint):
+    blueprint = ActionProviderBlueprint(
+        name="TestBlueprint",
+        import_name=__name__,
+        url_prefix="/my_cool_ap",
+        provider_description=ap_description,
+    )
+    app = create_app_from_blueprint(blueprint)
+    client = ActionProviderClient(app.test_client(), blueprint.url_prefix)
+
+    # create a dummy builder but ensure the real class is used
+    # (`create_app_from_blueprint` mocks over this)
+    #
+    # instead, inject an error when an AuthState is being constructed
+    # narrowly, this just needs to be during the init-time call to introspect_token
+    blueprint.state_builder = FlaskAuthStateBuilder(mock.Mock(), ("foo-scope",))
+    with mock.patch(
+        "globus_action_provider_tools.authentication.AuthState.introspect_token",
+        side_effect=InactiveTokenError("token wuz bad"),
+    ):
+        resp = client.run(
+            body={"echo_string": "hello badly authenticated world"}, assert_status=401
+        )
+
+    # confirm that the customized 401 renders as desired as JSON
+    assert resp.json["code"] == "UnauthorizedRequest"
+    assert resp.json["description"] == (
+        "The server could not verify that you are authorized "
+        "to access the URL requested."
+    )

tests/flask/test_flask_auth_state_builder.py

@@ -2,7 +2,14 @@
 
 import pytest
 
-from globus_action_provider_tools.errors import UnverifiedAuthenticationError
+from globus_action_provider_tools.authentication import (
+    InactiveTokenError,
+    InvalidTokenScopesError,
+)
+from globus_action_provider_tools.errors import (
+    AuthenticationError,
+    UnverifiedAuthenticationError,
+)
 from globus_action_provider_tools.flask.helpers import FlaskAuthStateBuilder
 
 
@@ -37,3 +44,29 @@ def test_bogus_authorization_headers_are_rejected_without_io(authorization_heade
     ):
         with pytest.raises(UnverifiedAuthenticationError):
             builder.build_from_request(request=mock_request_object)
+
+
+@pytest.mark.parametrize(
+    "underlying_error, expect_message",
+    (
+        (InactiveTokenError("foo"), "Token is invalid, expired, or revoked"),
+        (
+            InvalidTokenScopesError(frozenset(["foo"]), frozenset(["bar"])),
+            "Token has invalid scopes",
+        ),
+    ),
+)
+def test_invalid_token_data_results_in_authn_errors(underlying_error, expect_message):
+    # stub the Auth Client and scopes
+    builder = FlaskAuthStateBuilder(mock.Mock(), [])
+
+    mock_request_object = mock.Mock()
+    mock_request_object.headers = {"Authorization": "Bearer AbcDefGhiJklmnop"}
+
+    # mock in the underlying authentication related error
+    with mock.patch(
+        "globus_action_provider_tools.authentication.AuthState",
+        side_effect=underlying_error,
+    ):
+        with pytest.raises(AuthenticationError, match=expect_message):
+            builder.build_from_request(request=mock_request_object)

tests/flask/test_routes.py

@@ -10,7 +10,6 @@
 import flask
 import flask.testing
 import pytest
-from globus_sdk._testing import load_response
 
 from .ap_client import ActionProviderClient
 from .app_utils import (
@@ -22,9 +21,12 @@
 @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, aptb_app, api_version: str, use_pydantic_schema: bool
+    introspect_success_response,
+    request,
+    aptb_app,
+    api_version: str,
+    use_pydantic_schema: bool,
 ):
-    freeze_time(load_response("token-introspect", case="success"))
     _, bp = list(aptb_app.blueprints.items())[0]
     if use_pydantic_schema:
         bp.input_schema = ActionProviderPydanticInputSchema