From 307e1a59de8fa7dcdda332a6d6133ac30c3ec9b1 Mon Sep 17 00:00:00 2001 From: Ada Date: Thu, 29 Jun 2023 09:45:22 -0700 Subject: [PATCH 01/32] Add support for Globus Session Error responses Signed-off-by: Ada --- ...9_093104_ada_add_session_error_support.rst | 2 + docs/experimental/session_errors.rst | 122 +++++ .../experimental/session_error/__init__.py | 16 + .../session_error/session_error.py | 193 ++++++++ .../experimental/session_error/utils.py | 114 +++++ .../experimental/session_error/variants.py | 275 ++++++++++++ tests/unit/experimental/test_session_error.py | 415 ++++++++++++++++++ 7 files changed, 1137 insertions(+) create mode 100644 changelog.d/20230629_093104_ada_add_session_error_support.rst create mode 100644 docs/experimental/session_errors.rst create mode 100644 src/globus_sdk/experimental/session_error/__init__.py create mode 100644 src/globus_sdk/experimental/session_error/session_error.py create mode 100644 src/globus_sdk/experimental/session_error/utils.py create mode 100644 src/globus_sdk/experimental/session_error/variants.py create mode 100644 tests/unit/experimental/test_session_error.py diff --git a/changelog.d/20230629_093104_ada_add_session_error_support.rst b/changelog.d/20230629_093104_ada_add_session_error_support.rst new file mode 100644 index 000000000..b30e1b705 --- /dev/null +++ b/changelog.d/20230629_093104_ada_add_session_error_support.rst @@ -0,0 +1,2 @@ +* Add a new class ``GlobusSessionError`` and utility functions + to support handling of the Globus Session Error response format (:pr:`NUMBER`) diff --git a/docs/experimental/session_errors.rst b/docs/experimental/session_errors.rst new file mode 100644 index 000000000..c2090a72d --- /dev/null +++ b/docs/experimental/session_errors.rst @@ -0,0 +1,122 @@ +Session Errors +============== + +Globus Session Error is a response format that conveys to a client any +modifications to a session (i.e., "boosting") that will be required +in order to complete a particular request. + +The ``session_errors`` module provides a number of tools to make it easier to +identify and handle these errors when they occur. + +GlobusSessionError +------------------ + +The ``GlobusSessionError`` class provides a model for working with Globus +Session Error responses in the Python SDK. The shape of an instance closely +matches that of the JSON response, such that in order to access a +response's session_required_scopes one could use, e.g.,: + +.. code-block:: python + + from globus_sdk.experimental import session_errors + + error = session_errors.GlobusSessionError(response) + error.authorization_parameters.session_required_scopes + +``GlobusSessionError`` enforces types strictly when parsing a Globus Session +Error response dictionary, and will raise a ``ValueError`` if a supported +field is supplied with value of the wrong type. ``GlobusSessionError`` does +not, however, attempt to mimic or itself enforce any logic specific to the +Globus Auth service with regard to what represents a coherent combination +of fields (e.g., that ``session_required_mfa`` requires either +``session_required_identities`` or ``session_required_single_domain`` +in order to be properly handled). + +If you are writing a service that needs to respond with a session error, you can +instantiate the ``GlobusSessionError`` class directly to emit session errors +in your application, e.g.: + +.. code-block:: python + + from globus_sdk.experimental import session_errors + + error = session_errors.GlobusSessionError( + code="ConsentRequired", + authorization_parameters=GlobusSessionErrorAuthorizationParameters( + session_required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], + session_message="Missing required 'foo' consent", + ), + ) + + # Render a strict dictionary + error.to_dict() + +If non-canonical fields were supplied on creation (either as keyword arguments +during instantiation or as fields in a dictionary supplied to ``from_dict()``), +you can preserve these fields in the rendered output dictionary +by specifying ``include_extra=True``. + +.. code-block:: python + + from globus_sdk.experimental import session_errors + + error = session_errors.GlobusSessionError( + code="ConsentRequired", + message="Missing required 'foo' consent", + request_id="WmMV97A1w", + required_scopes=["urn:globus:auth:scope:transfer.api.globus.org:all[*foo]"], + resource="/transfer", + authorization_parameters=GlobusSessionErrorAuthorizationParameters( + session_required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], + session_message="Missing required 'foo' consent", + ), + ) + + # Render a dictionary with extra fields + error.to_dict(include_extra=True) + +These fields are stored by both the ``GlobusSessionError`` and +``GlobusSessionErrorAuthenticationParameters`` classes in an ``extra_fields`` +attribute. + +Parsing Responses +----------------- + +If you are writing a client to a Globus API, the ``session_errors`` subpackage +in the Python SDK provides utilities to detect legacy session error response +formats and normalize them. + +To detect if a ``GlobusAPIError``, ``ErrorSubdocument``, or JSON response +dictionary represents an error that can be converted to a Globus Session Error, +you can use, e.g.,: + +.. code-block:: python + + from globus_sdk.experimental import session_error + + error_dict = { + "code": "ConsentRequired", + "message": "Missing required foo consent", + } + session_error.utils.is_session_error(error_dict) # False + session_error.utils.to_session_error(error_dict) # None + + error_dict = { + "code": "ConsentRequired", + "message": "Missing required foo consent", + "required_scopes": ["urn:globus:auth:scope:transfer.api.globus.org:all[*foo]"], + } + session_error.utils.is_session_error(error_dict) # True + session_error.utils.to_session_error(error_dict) # GlobusSessionError + +.. note:: + If a GlobusAPIError represents multiple errors that were returned in an + array, this only returns the first error in that array that can be + converted to the Globus Session Error response format. In this case (and, + in general) it's preferable to use ``to_session_error()`` (which also + accepts a list of ``GlobusAPIErrors``, ``ErrorSubdocuments``, and JSON + response dictionaries): + +.. code-block:: python + session_error.utils.to_session_error(other_error) # GlobusSessionError + session_error.utils.to_session_errors([other_error]) # [GlobusSessionError, ...] diff --git a/src/globus_sdk/experimental/session_error/__init__.py b/src/globus_sdk/experimental/session_error/__init__.py new file mode 100644 index 000000000..6053eb50f --- /dev/null +++ b/src/globus_sdk/experimental/session_error/__init__.py @@ -0,0 +1,16 @@ +from .session_error import GlobusSessionError, GlobusSessionErrorAuthorizationParameters +from .utils import ( + has_session_errors, + is_session_error, + to_session_error, + to_session_errors, +) + +__all__ = [ + "GlobusSessionError", + "GlobusSessionErrorAuthorizationParameters", + "to_session_error", + "to_session_errors", + "is_session_error", + "has_session_errors", +] diff --git a/src/globus_sdk/experimental/session_error/session_error.py b/src/globus_sdk/experimental/session_error/session_error.py new file mode 100644 index 000000000..4c78a0271 --- /dev/null +++ b/src/globus_sdk/experimental/session_error/session_error.py @@ -0,0 +1,193 @@ +import copy +import typing as t + +from globus_sdk.exc import GlobusError + + +class GlobusSessionErrorAuthorizationParameters: + """ + Represents authorization parameters that can be used to instruct a client + which additional authorizations are needed in order to complete a request. + + :ivar session_message: A message to be displayed to the user. + :vartype session_message: str, optional + + :ivar session_required_identities: A list of identities required for the + session. + :vartype session_required_identities: list of str, optional + + :ivar session_required_policies: A list of policies required for the + session. + :vartype session_required_policies: list of str, optional + + :ivar session_required_single_domain: A list of domains required for the + session. + :vartype session_required_single_domain: list of str, optional + + :ivar session_required_mfa: Whether MFA is required for the session. + :vartype session_required_mfa: bool, optional + + :ivar session_required_scopes: A list of scopes for which consent is required. + :vartype session_required_scopes: list of str, optional + + :ivar extra_fields: A dictionary of additional fields that were provided. May + be used for forward/backward compatibility. + :vartype extra_fields: dict + """ + + SUPPORTED_FIELDS = { + "session_message": str, + "session_required_identities": list, + "session_required_policies": list, + "session_required_single_domain": list, + "session_required_mfa": bool, + "session_required_scopes": list, + } + + def __init__( + self, + session_message: t.Optional[str] = None, + session_required_identities: t.Optional[t.List[str]] = None, + session_required_policies: t.Optional[t.List[str]] = None, + session_required_single_domain: t.Optional[t.List[str]] = None, + session_required_mfa: t.Optional[bool] = None, + session_required_scopes: t.Optional[t.List[str]] = None, + **kwargs: t.Any, + ): + self.session_message: t.Optional[str] = session_message + self.session_required_identities = session_required_identities + self.session_required_policies = session_required_policies + self.session_required_single_domain: t.Optional[ + t.List[str] + ] = session_required_single_domain + self.session_required_mfa: t.Optional[bool] = session_required_mfa + self.session_required_scopes: t.Optional[t.List[str]] = session_required_scopes + self.extra_fields: t.Dict[str, t.Any] = kwargs + + @classmethod + def from_dict(cls, param_dict: t.Dict[str, t.Any]) -> "GlobusSessionError": + """ + Instantiate from a session error authorization parameters dictionary. Raises + a ValueError if the dictionary does not contain a valid GlobusSessionError. + + :param param_dict: The dictionary to create the error from. + :type param_dict: dict + """ + + # Enforce that the error_dict contains at least one of the fields we expect + if not any(field in param_dict for field in cls.SUPPORTED_FIELDS.keys()): + raise ValueError( + "Must include at least one supported authorization parameter: " + ", ".join(cls.SUPPORTED_FIELDS.keys()) + ) + + # Enforce the field types + for field_name, field_type in cls.SUPPORTED_FIELDS.items(): + if field_name in param_dict and not isinstance( + param_dict[field_name], field_type + ): + raise ValueError(f"{field_name} must be of type {field_type.__name__}") + + return cls(**param_dict) + + def to_dict(self, include_extra: bool = False) -> t.Dict[str, t.Any]: + """ + Return a session error authorization parameters dictionary. + + :param include_extra: Whether to include stored extra (non-standard) fields in + the returned dictionary. + :type include_extra: bool + """ + session_error_dict = {} + + # Set any authorization parameters + for field in self.SUPPORTED_FIELDS.keys(): + if getattr(self, field) is not None: + session_error_dict[field] = getattr(self, field) + + # Set any extra fields + if include_extra: + session_error_dict.update(self.extra_fields) + + return session_error_dict + + +class GlobusSessionError(GlobusError): + """ + Represents a Globus Session Error. + + A Session Error is a class of error that is returned by Globus services to + indicate that additional authorization is required in order to complete a request + and contains information that can be used to request the appropriate authorization. + + :ivar code: The error code for this error. + :vartype code: str + + :ivar authorization_parameters: The authorization parameters for this error. + :vartype authorization_parameters: GlobusAuthorizationParameters + + :ivar extra_fields: A dictionary of additional fields that were provided. May + be used for forward/backward compatibility. + :vartype extra_fields: dict + """ + + def __init__( + self, + code: str, + authorization_parameters: GlobusSessionErrorAuthorizationParameters, + **kwargs: t.Any, + ): + self.code: str = code + self.authorization_parameters: GlobusSessionErrorAuthorizationParameters = ( + authorization_parameters + ) + self.extra_fields = kwargs + + @classmethod + def from_dict(cls, error_dict: t.Dict[str, t.Any]) -> "GlobusSessionError": + """ + Instantiate a GlobusSessionError from a dictionary. + + :param error_dict: The dictionary to create the error from. + :type error_dict: dict + """ + + if "code" not in error_dict: + raise ValueError("GlobusSessionError must have a code") + + # Enforce that authorization_parameters is in the error_dict and + # contains at least one of the fields we expect + if "authorization_parameters" not in error_dict: + raise ValueError("GlobusSessionError must have authorization_parameters") + + kwargs = copy.deepcopy(error_dict) + + # Create our GlobusAuthorizationParameters object + kwargs[ + "authorization_parameters" + ] = GlobusSessionErrorAuthorizationParameters.from_dict( + param_dict=kwargs.pop("authorization_parameters") + ) + + return cls(**kwargs) + + def to_dict(self, include_extra: bool = False) -> t.Dict[str, t.Any]: + """ + Return a session error response dictionary. + + :param include_extra: Whether to include stored extra (non-standard) fields + in the dictionary. + :type include_extra: bool, optional (default: False) + """ + session_error_dict = { + "code": self.code, + "authorization_parameters": self.authorization_parameters.to_dict( + include_extra=include_extra + ), + } + + # Set any extra fields + if include_extra: + session_error_dict.update(self.extra_fields) + + return session_error_dict diff --git a/src/globus_sdk/experimental/session_error/utils.py b/src/globus_sdk/experimental/session_error/utils.py new file mode 100644 index 000000000..8419afb0e --- /dev/null +++ b/src/globus_sdk/experimental/session_error/utils.py @@ -0,0 +1,114 @@ +import typing as t +from contextlib import suppress + +from globus_sdk.exc import ErrorSubdocument, GlobusAPIError + +from .session_error import GlobusSessionError +from .variants import ( + LegacyAuthorizationParametersError, + LegacyConsentRequiredAPError, + LegacyConsentRequiredTransferError, +) + + +def to_session_error( + error: t.Union[GlobusAPIError, ErrorSubdocument, t.Dict] +) -> t.Optional[GlobusSessionError]: + """ + Converts a GlobusAPIError, ErrorSubdocument, or dict into a GlobusSessionError by + attempting to match to GlobusSessionError (preferred) or legacy variants. + + .. note:: + a GlobusAPIError may contain multiple errors, and in this case only a single + session error is returned for the first error that matches a known format. + + + If the provided error does not match a known format, None is returned. + + :param error: The error to convert. + :type error: a GlobusAPIError, ErrorSubdocument, or dict + """ + + # GlobusAPIErrors may contain more than one error, so we consider all of them + # even though we only return the first. + if isinstance(error, GlobusAPIError): + # Iterate over ErrorSubdocuments + for subdoc in error.errors: + session_error = to_session_error(subdoc) + if session_error is not None: + # Return only the first session error we encounter + return session_error + # We failed to find a session error + return None + elif isinstance(error, ErrorSubdocument): + error_dict = error.raw + else: + error_dict = error + + # Prefer a proper session error, if possible + with suppress(ValueError): + return GlobusSessionError.from_dict(error_dict) + + for variant in [ + LegacyAuthorizationParametersError, + LegacyConsentRequiredTransferError, + LegacyConsentRequiredAPError, + ]: + with suppress(ValueError): + return variant.from_dict(error_dict).to_session_error() + + return None + + +def to_session_errors( + errors: t.List[t.Union[GlobusAPIError, ErrorSubdocument, t.Dict]] +) -> t.List[GlobusSessionError]: + """ + Converts a list of GlobusAPIErrors, ErrorSubdocuments, or dicts into a list of + GlobusSessionErrors by attempting to match each error to + GlobusSessionError (preferred) or legacy variants. + + .. note:: + A GlobusAPIError may contain multiple errors, so the result + list could be longer than the provided list. + + If no errors match any known formats, an empty list is returned. + + :param errors: The errors to convert. + :type errors: a list of GlobusAPIErrors, ErrorSubdocuments, or dicts + """ + candidate_errors = [] + for error in errors: + if isinstance(error, GlobusAPIError): + # Use the ErrorSubdocuments + candidate_errors.extend(error.errors) + else: + candidate_errors.append(error) + + # Try to convert all candidate errors to session errors + all_errors = [to_session_error(error) for error in candidate_errors] + + # Remove any errors that did not resolve to a session error + return [error for error in all_errors if error is not None] + + +def is_session_error(error: t.Union[GlobusAPIError, ErrorSubdocument, t.Dict]) -> bool: + """ + Return True if the provided error matches a known session error format. + + :param error: The error to check. + :type error: a GlobusAPIError, ErrorSubdocument, or dict + """ + return to_session_error(error) is not None + + +def has_session_errors( + errors: t.List[t.Union[GlobusAPIError, ErrorSubdocument, t.Dict]] +) -> bool: + """ + Return True if any of the provided errors match a known session error format. + + :param errors: The errors to check. + :type errors: a list of GlobusAPIErrors, ErrorSubdocuments, or dicts + """ + return any(is_session_error(error) for error in errors) diff --git a/src/globus_sdk/experimental/session_error/variants.py b/src/globus_sdk/experimental/session_error/variants.py new file mode 100644 index 000000000..2540ee589 --- /dev/null +++ b/src/globus_sdk/experimental/session_error/variants.py @@ -0,0 +1,275 @@ +import typing as t + +from .session_error import GlobusSessionError, GlobusSessionErrorAuthorizationParameters + + +class LegacyConsentRequiredTransferError: + """ + The ConsentRequired error format emitted by the Globus Transfer service. + """ + + def __init__( + self, + code: t.Literal["ConsentRequired"], + required_scopes: t.List[str], + message: t.Optional[str] = None, + request_id: t.Optional[str] = None, + resource: t.Optional[str] = None, + **kwargs: t.Any, + ): + self.code: t.Literal["ConsentRequired"] = code + self.required_scopes: t.List[str] = required_scopes + self.message: t.Optional[str] = message + self.request_id: t.Optional[str] = request_id + self.resource: t.Optional[str] = resource + self.extra_fields: t.Dict[str, t.Any] = kwargs + + def to_session_error(self) -> GlobusSessionError: + """ + Return a GlobusSessionError representing this error. + """ + return GlobusSessionError( + code=self.code, + authorization_parameters=GlobusSessionErrorAuthorizationParameters( + session_required_scopes=self.required_scopes, + session_message=self.message, + ), + ) + + @classmethod + def from_dict( + cls, error_dict: t.Dict[str, t.Any] + ) -> "LegacyConsentRequiredTransferError": + """ + Instantiate from an error dictionary. Raises a ValueError if the dictionary + does not contain a recognized LegacyConsentRequiredTransferError. + + :param error_dict: The dictionary to instantiate the error from. + :type error_dict: dict + """ + if error_dict.get("code") != "ConsentRequired": + raise ValueError("Must be a ConsentRequired error") + + if not error_dict.get("required_scopes"): + raise ValueError("Must include required_scopes") + + return cls(**error_dict) + + +class LegacyConsentRequiredAPError: + """ + The ConsentRequired error format emitted by the legacy Globus Transfer + Action Providers. + """ + + def __init__( + self, + code: t.Literal["ConsentRequired"], + required_scope: str, + description: t.Optional[str] = None, + **kwargs: t.Any, + ): + self.code: t.Literal["ConsentRequired"] = code + self.required_scope: str = required_scope + self.description: t.Optional[str] = description + self.extra_fields: t.Dict[str, t.Any] = kwargs + + def to_session_error(self) -> GlobusSessionError: + """ + Return a GlobusSessionError representing this error. + + Normalizes the required_scope field to a list and uses the description + to set the session message. + """ + return GlobusSessionError( + code=self.code, + authorization_parameters=GlobusSessionErrorAuthorizationParameters( + session_required_scopes=[self.required_scope], + session_message=self.description, + ), + ) + + @classmethod + def from_dict( + cls, error_dict: t.Dict[str, t.Any] + ) -> "LegacyConsentRequiredAPError": + """ + Instantiate from an error dictionary. Raises a ValueError if the dictionary + does not contain a recognized LegacyConsentRequiredAPError. + + :param error_dict: The dictionary to create the error from. + :type error_dict: dict + """ + if error_dict.get("code") != "ConsentRequired": + raise ValueError("Must be a ConsentRequired error") + + if not error_dict.get("required_scope"): + raise ValueError("Must include required_scope") + + return cls(**error_dict) + + +class LegacyAuthorizationParameters: + """ + An Authorization Parameters object that describes all known variants in use by + Globus services. + """ + + DEFAULT_CODE = "AuthorizationRequired" + + SUPPORTED_FIELDS = { + "session_message": (str,), + "session_required_identities": (list,), + "session_required_policies": (list, str), + "session_required_single_domain": (list, str), + "session_required_mfa": (bool,), + "session_required_scopes": (list,), + } + + def __init__( + self, + session_message: t.Optional[str] = None, + session_required_identities: t.Optional[t.List[str]] = None, + session_required_policies: t.Optional[t.Union[str, t.List[str]]] = None, + session_required_single_domain: t.Optional[t.Union[str, t.List[str]]] = None, + session_required_mfa: t.Optional[bool] = None, + session_required_scopes: t.Optional[t.List[str]] = None, + **kwargs: t.Any, + ): + self.session_message: t.Optional[str] = session_message + self.session_required_identities: t.Optional[ + t.List[str] + ] = session_required_identities + self.session_required_policies: t.Optional[ + t.Union[str, t.List[str]] + ] = session_required_policies + self.session_required_single_domain: t.Optional[ + t.Union[str, t.List[str]] + ] = session_required_single_domain + self.session_required_mfa: t.Optional[bool] = session_required_mfa + # Declared here for compatibility with mixed legacy payloads + self.session_required_scopes: t.Optional[t.List[str]] = session_required_scopes + # Retain any additional fields + self.extra_fields: t.Dict[str, t.Any] = kwargs + + def to_session_error_authorization_parameters( + self, + ) -> GlobusSessionErrorAuthorizationParameters: + """ + Return a GlobusSessionError representing this error. + + Normalizes fields that may have been provided + as comma-delimited strings to lists of strings. + """ + required_policies = self.session_required_policies + if isinstance(required_policies, str): + required_policies = required_policies.split(",") + + # TODO: Unnecessary? + required_single_domain = self.session_required_single_domain + if isinstance(required_single_domain, str): + required_single_domain = required_single_domain.split(",") + + return GlobusSessionErrorAuthorizationParameters( + session_message=self.session_message, + session_required_identities=self.session_required_identities, + session_required_mfa=self.session_required_mfa, + session_required_policies=required_policies, + session_required_single_domain=required_single_domain, + session_required_scopes=self.session_required_scopes, + **self.extra_fields, + ) + + @classmethod + def from_dict( + cls, param_dict: t.Dict[str, t.Any] + ) -> "LegacyAuthorizationParameters": + """ + Create from a dictionary. Raises a ValueError if the dictionary does not contain + a recognized LegacyAuthorizationParameters format. + + :param param_dict: The dictionary to create the AuthorizationParameters from. + :type param_dict: dict + """ + if not any(field in param_dict for field in cls.SUPPORTED_FIELDS.keys()): + raise ValueError( + "Must include at least one supported authorization parameter: " + ", ".join(cls.SUPPORTED_FIELDS.keys()) + ) + + for field, field_types in cls.SUPPORTED_FIELDS.items(): + if field not in param_dict: + continue + if not isinstance(param_dict[field], field_types): + raise ValueError( + f"Field {field} must be one of {field_types}, " + "got {error_dict[field]}" + ) + + return cls(**param_dict) + + +class LegacyAuthorizationParametersError: + """ + Defines an Authorization Parameters error that describes all known variants + in use by Globus services. + """ + + DEFAULT_CODE = "AuthorizationRequired" + + def __init__( + self, + authorization_parameters: LegacyAuthorizationParameters, + code: t.Optional[str] = None, + **kwargs: t.Any, + ): + self.authorization_parameters: LegacyAuthorizationParameters = ( + authorization_parameters + ) + self.code: str = code or self.DEFAULT_CODE + # Retain any additional fields + self.extra_fields: t.Dict[str, t.Any] = kwargs + + @classmethod + def from_dict( + cls, error_dict: t.Dict[str, t.Any] + ) -> "LegacyAuthorizationParametersError": + """ + Instantiate a LegacyAuthorizationParametersError from a dictionary. + """ + + # Enforce that authorization_parameters is present in the error_dict + if not isinstance(error_dict, dict) or not isinstance( + error_dict.get("authorization_parameters"), dict + ): + raise ValueError( + "LegacyAuthorizationParametersError must be a dict that contains an " + "'authorization_parameters' dict" + ) + + extra_fields = { + key: value + for key, value in error_dict.items() + if key not in ("authorization_parameters", "code") + } + + return cls( + authorization_parameters=LegacyAuthorizationParameters.from_dict( + error_dict["authorization_parameters"] + ), + code=error_dict.get("code"), + **extra_fields, + ) + + def to_session_error(self) -> GlobusSessionError: + """ + Return a GlobusSessionError representing this error. + """ + authorization_parameters = ( + self.authorization_parameters.to_session_error_authorization_parameters() + ) + return GlobusSessionError( + authorization_parameters=authorization_parameters, + code=self.code, + **self.extra_fields, + ) diff --git a/tests/unit/experimental/test_session_error.py b/tests/unit/experimental/test_session_error.py new file mode 100644 index 000000000..cbfa537c1 --- /dev/null +++ b/tests/unit/experimental/test_session_error.py @@ -0,0 +1,415 @@ +import json + +import pytest + +from globus_sdk.exc import ErrorSubdocument, GlobusAPIError +from globus_sdk.experimental.session_error import ( + GlobusSessionError, + has_session_errors, + is_session_error, + to_session_error, + to_session_errors, +) + +# TODO: We should move this to a common location +from tests.unit.errors.conftest import _mk_response + + +def _error_for_json_dict(error_dict, status=403, method="POST", headers=None): + combined_headers = {"Content-Type": "application/json"} + if headers: + combined_headers.update(headers) + + response = _mk_response( + data=error_dict, + status=status, + method=method, + headers=combined_headers, + data_transform=json.dumps, + ) + return GlobusAPIError(response.r) + + +@pytest.mark.parametrize( + "error_dict, status", + ( + ( + { + "code": "ConsentRequired", + "message": "Missing required foo_bar consent", + "request_id": "WmMV97A1w", + "required_scopes": [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*foo *bar]" + ], + "resource": "/transfer", + }, + 403, + ), + ( + { + "code": "ConsentRequired", + "required_scope": ( + "urn:globus:auth:scope:transfer.api.globus.org:all[*foo *bar]" + ), + "description": "Missing required foo_bar consent", + }, + 401, + ), + ), +) +def test_create_session_error_from_consent_error(error_dict, status): + """ + Test that various ConsentRequired error shapes can be detected and converted + to a GlobusSessionError. + """ + # Create various supplementary objects representing this error + error_subdoc = ErrorSubdocument(error_dict) + api_error = _error_for_json_dict(error_dict, status=status) + + for error in (error_dict, error_subdoc, api_error): + # Test boolean utility functions + assert is_session_error(error) + assert has_session_errors([error]) + + # Check that this only produces one error + assert len(to_session_errors([error])) == 1 + + # Create a session error from a Transfer format error + session_error = to_session_error(error) + assert isinstance(session_error, GlobusSessionError) + assert session_error.code == "ConsentRequired" + assert session_error.authorization_parameters.session_required_scopes == [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*foo *bar]" + ] + assert ( + session_error.authorization_parameters.session_message + == "Missing required foo_bar consent" + ) + + +@pytest.mark.parametrize( + "authorization_parameters", + ( + { + "session_message": ( + "To gain access you need to authenticate with your baz identity" + ), + "session_required_identities": ["urn:globus:auth:identity:baz"], + "session_required_mfa": True, + }, + { + "session_message": ( + "You need to authenticate with an identity that " + "matches the required policies" + ), + "session_required_policies": ["foo", "baz"], + }, + { + "session_message": ( + "You need to authenticate with an identity that " + "belongs to an authorized domain" + ), + "session_required_single_domain": ["foo.com", "baz.org"], + }, + ), +) +def test_create_session_error_from_authorization_error(authorization_parameters): + """ + Test that various AuthorizationRequired error shapes can be detected and converted + to a GlobusSessionError. + """ + # Create various supplementary objects representing this error + error_dict = {"authorization_parameters": authorization_parameters} + error_subdoc = ErrorSubdocument(error_dict) + api_error = _error_for_json_dict(error_dict) + + for error in (error_dict, error_subdoc, api_error): + # Test boolean utility functions + assert is_session_error(error) + assert has_session_errors([error]) + + # Check that this only produces one error + assert len(to_session_errors([error])) == 1 + + # Create a session error from a legacy authorization parameters format error + session_error = to_session_error(error) + assert isinstance(session_error, GlobusSessionError) + + # Check that the default error code is set + assert session_error.code == "AuthorizationRequired" + + # Iterate over the expected attributes and check that they match + for name, value in authorization_parameters.items(): + assert getattr(session_error.authorization_parameters, name) == value + + +@pytest.mark.parametrize( + "authorization_parameters", + ( + { + "session_message": ( + "You need to authenticate with an identity that " + "matches the required policies" + ), + "session_required_policies": ["foo", "baz"], + }, + { + "session_message": ( + "You need to authenticate with an identity that " + "belongs to an authorized domain" + ), + "session_required_single_domain": ["foo.com", "baz.org"], + }, + ), +) +def test_create_session_error_from_authorization_error_csv(authorization_parameters): + """ + Test that AuthorizationRequired error shapes that provide lists as comma-delimited + values can be detected and converted to a GlobusSessionError normalizing to + lists of strings for those values. + """ + # Create various supplementary objects representing this error + error_dict = {"authorization_parameters": {}} + for key, value in authorization_parameters.items(): + if key in ("session_required_policies", "session_required_single_domain"): + # Convert the list to a comma-separated string for known variants + error_dict["authorization_parameters"][key] = ",".join(value) + else: + error_dict["authorization_parameters"][key] = value + + error_subdoc = ErrorSubdocument(error_dict) + api_error = _error_for_json_dict(error_dict) + + for error in (error_dict, error_subdoc, api_error): + # Test boolean utility functions + assert is_session_error(error) + assert has_session_errors([error]) + + # Check that this only produces one error + assert len(to_session_errors([error])) == 1 + + # Create a session error from a legacy authorization parameters format error + session_error = to_session_error(error) + assert isinstance(session_error, GlobusSessionError) + + # Check that the default error code is set + assert session_error.code == "AuthorizationRequired" + + # Iterate over the expected attributes and check that they match + for name, value in authorization_parameters.items(): + assert getattr(session_error.authorization_parameters, name) == value + + +def test_create_session_errors_from_multiple_errors(): + """ + Test that a GlobusAPIError with multiple subdocuments is converted to multiple + GlobusSessionErrors, and additionally test that this is correct even when mingled + with other accepted data types. + """ + consent_errors = _error_for_json_dict( + { + "errors": [ + { + "code": "ConsentRequired", + "message": "Missing required foo_bar consent", + "authorization_parameters": { + "session_required_scopes": [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*bar]" + ], + "session_message": "Missing required foo_bar consent", + }, + }, + { + "code": "ConsentRequired", + "message": "Missing required foo_baz consent", + "authorization_parameters": { + "session_required_scopes": [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" + ], + "session_message": "Missing required foo_baz consent", + }, + }, + ] + } + ) + + authorization_error = _error_for_json_dict( + { + "authorization_parameters": { + "session_message": ( + "You need to authenticate with an identity that " + "matches the required policies" + ), + "session_required_policies": ["foo", "baz"], + } + } + ) + + not_an_error = _error_for_json_dict( + { + "code": "NotAnError", + "message": "This is not an error", + } + ) + + all_errors = [consent_errors, not_an_error, authorization_error] + + # Test boolean utility function + assert has_session_errors(all_errors) + + # Create session errors from a all errors + session_errors = to_session_errors(all_errors) + assert isinstance(session_errors, list) + assert len(session_errors) == 3 + + # Check that errors properly converted + for session_error in session_errors: + assert isinstance(session_error, GlobusSessionError) + + # Check that the proper session errors were produced + assert session_errors[0].code == "ConsentRequired" + assert session_errors[0].authorization_parameters.session_required_scopes == [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*bar]" + ] + assert ( + session_errors[0].authorization_parameters.session_message + == "Missing required foo_bar consent" + ) + assert session_errors[1].code == "ConsentRequired" + assert session_errors[1].authorization_parameters.session_required_scopes == [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" + ] + assert ( + session_errors[1].authorization_parameters.session_message + == "Missing required foo_baz consent" + ) + assert session_errors[2].code == "AuthorizationRequired" + assert session_errors[2].authorization_parameters.session_required_policies == [ + "foo", + "baz", + ] + assert session_errors[2].authorization_parameters.session_message == ( + "You need to authenticate with an identity that matches the required policies" + ) + + +def test_create_session_error_from_legacy_authorization_error_with_code(): + """ + Test that legacy AuthorizationRequired error shapes that provide a `code` can be + detected and converted to a GlobusSessionError while retaining the `code`. + """ + # Create a legacy authorization parameters error with a code + error_dict = { + "code": "UnsatisfiedPolicy", + "authorization_parameters": { + "session_message": ( + "You need to authenticate with an identity that " + "matches the required policies" + ), + "session_required_policies": "foo,baz", + }, + } + + # Create various supplementary objects representing this error + error_subdoc = ErrorSubdocument(error_dict) + api_error = _error_for_json_dict(error_dict) + + for error in (error_dict, error_subdoc, api_error): + # Test boolean utility functions + assert is_session_error(error) + assert has_session_errors([error]) + + # Check that this only produces one error + assert len(to_session_errors([error])) == 1 + + # Create a session error from a legacy authorization parameters format error + session_error = to_session_error(error) + assert isinstance(session_error, GlobusSessionError) + + # Check that the custom error code is set + assert session_error.code == "UnsatisfiedPolicy" + + # Iterate over the expected attributes and check that they match + assert session_error.authorization_parameters.session_required_policies == [ + "foo", + "baz", + ] + + +def test_backward_compatibility_consent_required_error(): + """ + Test that a consent required error with a comingled backward-compatible + data schema is converted to a GlobusSessionError. + """ + # Create an API error with a backward compatible data schema using + # distinct values for duplicative fields to facilitate testing + # (in practice these would be the same) + error = _error_for_json_dict( + { + "code": "ConsentRequired", + "message": "Missing required foo_bar consent", + "request_id": "WmMV97A1w", + "required_scopes": [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*foo *bar]" + ], + "resource": "/transfer", + "authorization_parameters": { + "session_message": "Missing baz consent", + "session_required_scopes": [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" + ], + "session_required_policies": "foo,bar", + "optional": "A non-canonical field", + }, + }, + status=403, + ) + + # Test boolean utility functions + assert is_session_error(error) + assert has_session_errors([error]) + + # Check that this only produces one error + assert len(to_session_errors([error])) == 1 + + # Create a session error + session_error = to_session_error(error) + assert isinstance(session_error, GlobusSessionError) + assert session_error.code == "ConsentRequired" + assert session_error.authorization_parameters.session_required_scopes == [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" + ] + assert ( + session_error.authorization_parameters.session_message == "Missing baz consent" + ) + + # Test that only suppotred fields are present in the dict + assert session_error.to_dict() == { + "code": "ConsentRequired", + "authorization_parameters": { + "session_message": "Missing baz consent", + "session_required_scopes": [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" + ], + "session_required_policies": ["foo", "bar"], + }, + } + + # Test that extra fields are present in the dict + assert session_error.to_dict(include_extra=True) == { + "code": "ConsentRequired", + "message": "Missing required foo_bar consent", + "request_id": "WmMV97A1w", + "required_scopes": [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*foo *bar]" + ], + "resource": "/transfer", + "authorization_parameters": { + "session_message": "Missing baz consent", + "session_required_scopes": [ + "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" + ], + "session_required_policies": ["foo", "bar"], + "optional": "A non-canonical field", + }, + } From 88f94511e19d6084cbd46b6555d1205b36dcf45e Mon Sep 17 00:00:00 2001 From: Ada Date: Thu, 29 Jun 2023 15:07:46 -0700 Subject: [PATCH 02/32] Placate mypy Signed-off-by: Ada --- docs/experimental/session_errors.rst | 1 + .../session_error/session_error.py | 4 ++- .../experimental/session_error/utils.py | 18 +++++++----- .../experimental/session_error/variants.py | 29 ++++++++++++++----- 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/docs/experimental/session_errors.rst b/docs/experimental/session_errors.rst index c2090a72d..3be25fd67 100644 --- a/docs/experimental/session_errors.rst +++ b/docs/experimental/session_errors.rst @@ -118,5 +118,6 @@ you can use, e.g.,: response dictionaries): .. code-block:: python + session_error.utils.to_session_error(other_error) # GlobusSessionError session_error.utils.to_session_errors([other_error]) # [GlobusSessionError, ...] diff --git a/src/globus_sdk/experimental/session_error/session_error.py b/src/globus_sdk/experimental/session_error/session_error.py index 4c78a0271..6f3e80da6 100644 --- a/src/globus_sdk/experimental/session_error/session_error.py +++ b/src/globus_sdk/experimental/session_error/session_error.py @@ -65,7 +65,9 @@ def __init__( self.extra_fields: t.Dict[str, t.Any] = kwargs @classmethod - def from_dict(cls, param_dict: t.Dict[str, t.Any]) -> "GlobusSessionError": + def from_dict( + cls, param_dict: t.Dict[str, t.Any] + ) -> "GlobusSessionErrorAuthorizationParameters": """ Instantiate from a session error authorization parameters dictionary. Raises a ValueError if the dictionary does not contain a valid GlobusSessionError. diff --git a/src/globus_sdk/experimental/session_error/utils.py b/src/globus_sdk/experimental/session_error/utils.py index 8419afb0e..ff94e36a7 100644 --- a/src/globus_sdk/experimental/session_error/utils.py +++ b/src/globus_sdk/experimental/session_error/utils.py @@ -8,11 +8,12 @@ LegacyAuthorizationParametersError, LegacyConsentRequiredAPError, LegacyConsentRequiredTransferError, + LegacySessionErrorVariant, ) def to_session_error( - error: t.Union[GlobusAPIError, ErrorSubdocument, t.Dict] + error: t.Union[GlobusAPIError, ErrorSubdocument, t.Dict[str, t.Any]] ) -> t.Optional[GlobusSessionError]: """ Converts a GlobusAPIError, ErrorSubdocument, or dict into a GlobusSessionError by @@ -49,11 +50,12 @@ def to_session_error( with suppress(ValueError): return GlobusSessionError.from_dict(error_dict) - for variant in [ + supported_variants: t.List[t.Type[LegacySessionErrorVariant]] = [ LegacyAuthorizationParametersError, LegacyConsentRequiredTransferError, LegacyConsentRequiredAPError, - ]: + ] + for variant in supported_variants: with suppress(ValueError): return variant.from_dict(error_dict).to_session_error() @@ -61,7 +63,7 @@ def to_session_error( def to_session_errors( - errors: t.List[t.Union[GlobusAPIError, ErrorSubdocument, t.Dict]] + errors: t.List[t.Union[GlobusAPIError, ErrorSubdocument, t.Dict[str, t.Any]]] ) -> t.List[GlobusSessionError]: """ Converts a list of GlobusAPIErrors, ErrorSubdocuments, or dicts into a list of @@ -77,7 +79,7 @@ def to_session_errors( :param errors: The errors to convert. :type errors: a list of GlobusAPIErrors, ErrorSubdocuments, or dicts """ - candidate_errors = [] + candidate_errors: t.List[t.Union[ErrorSubdocument, t.Dict[str, t.Any]]] = [] for error in errors: if isinstance(error, GlobusAPIError): # Use the ErrorSubdocuments @@ -92,7 +94,9 @@ def to_session_errors( return [error for error in all_errors if error is not None] -def is_session_error(error: t.Union[GlobusAPIError, ErrorSubdocument, t.Dict]) -> bool: +def is_session_error( + error: t.Union[GlobusAPIError, ErrorSubdocument, t.Dict[str, t.Any]] +) -> bool: """ Return True if the provided error matches a known session error format. @@ -103,7 +107,7 @@ def is_session_error(error: t.Union[GlobusAPIError, ErrorSubdocument, t.Dict]) - def has_session_errors( - errors: t.List[t.Union[GlobusAPIError, ErrorSubdocument, t.Dict]] + errors: t.List[t.Union[GlobusAPIError, ErrorSubdocument, t.Dict[str, t.Any]]] ) -> bool: """ Return True if any of the provided errors match a known session error format. diff --git a/src/globus_sdk/experimental/session_error/variants.py b/src/globus_sdk/experimental/session_error/variants.py index 2540ee589..e5efd74eb 100644 --- a/src/globus_sdk/experimental/session_error/variants.py +++ b/src/globus_sdk/experimental/session_error/variants.py @@ -2,22 +2,37 @@ from .session_error import GlobusSessionError, GlobusSessionErrorAuthorizationParameters +T = t.TypeVar("T", bound="LegacySessionErrorVariant") -class LegacyConsentRequiredTransferError: + +class LegacySessionErrorVariant: + """ + Abstract base class for errors which can be converted to a Globus Session Error. + """ + + @classmethod + def from_dict(cls: t.Type[T], error_dict: t.Dict[str, t.Any]) -> T: + raise NotImplementedError() + + def to_session_error(self) -> GlobusSessionError: + raise NotImplementedError() + + +class LegacyConsentRequiredTransferError(LegacySessionErrorVariant): """ The ConsentRequired error format emitted by the Globus Transfer service. """ def __init__( self, - code: t.Literal["ConsentRequired"], + code: str, required_scopes: t.List[str], message: t.Optional[str] = None, request_id: t.Optional[str] = None, resource: t.Optional[str] = None, **kwargs: t.Any, ): - self.code: t.Literal["ConsentRequired"] = code + self.code: str = code self.required_scopes: t.List[str] = required_scopes self.message: t.Optional[str] = message self.request_id: t.Optional[str] = request_id @@ -56,7 +71,7 @@ def from_dict( return cls(**error_dict) -class LegacyConsentRequiredAPError: +class LegacyConsentRequiredAPError(LegacySessionErrorVariant): """ The ConsentRequired error format emitted by the legacy Globus Transfer Action Providers. @@ -64,12 +79,12 @@ class LegacyConsentRequiredAPError: def __init__( self, - code: t.Literal["ConsentRequired"], + code: str, required_scope: str, description: t.Optional[str] = None, **kwargs: t.Any, ): - self.code: t.Literal["ConsentRequired"] = code + self.code: str = code self.required_scope: str = required_scope self.description: t.Optional[str] = description self.extra_fields: t.Dict[str, t.Any] = kwargs @@ -209,7 +224,7 @@ def from_dict( return cls(**param_dict) -class LegacyAuthorizationParametersError: +class LegacyAuthorizationParametersError(LegacySessionErrorVariant): """ Defines an Authorization Parameters error that describes all known variants in use by Globus services. From 04ecbab40e0035bb6fd9aca4db202bf51615ea73 Mon Sep 17 00:00:00 2001 From: Ada Date: Thu, 29 Jun 2023 15:23:08 -0700 Subject: [PATCH 03/32] Fix unlisted docs page Signed-off-by: Ada --- docs/experimental/index.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/experimental/index.rst b/docs/experimental/index.rst index a1a5597ee..c3e9b1674 100644 --- a/docs/experimental/index.rst +++ b/docs/experimental/index.rst @@ -17,3 +17,4 @@ Globus SDK Experimental Components :maxdepth: 1 scope_parser + session_errors From 1bacb8cdc9d6290e915b62567ce2d6b490c8dc90 Mon Sep 17 00:00:00 2001 From: Ada <107940310+ada-globus@users.noreply.github.com> Date: Thu, 6 Jul 2023 10:18:40 -0700 Subject: [PATCH 04/32] Apply documentation improvements from code review Co-authored-by: Kurt McKee --- docs/experimental/session_errors.rst | 18 +++++++++++------- .../experimental/session_error/utils.py | 2 ++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/experimental/session_errors.rst b/docs/experimental/session_errors.rst index 3be25fd67..a7a5591d9 100644 --- a/docs/experimental/session_errors.rst +++ b/docs/experimental/session_errors.rst @@ -3,7 +3,7 @@ Session Errors Globus Session Error is a response format that conveys to a client any modifications to a session (i.e., "boosting") that will be required -in order to complete a particular request. +to complete a particular request. The ``session_errors`` module provides a number of tools to make it easier to identify and handle these errors when they occur. @@ -25,7 +25,7 @@ response's session_required_scopes one could use, e.g.,: ``GlobusSessionError`` enforces types strictly when parsing a Globus Session Error response dictionary, and will raise a ``ValueError`` if a supported -field is supplied with value of the wrong type. ``GlobusSessionError`` does +field is supplied with a value of the wrong type. ``GlobusSessionError`` does not, however, attempt to mimic or itself enforce any logic specific to the Globus Auth service with regard to what represents a coherent combination of fields (e.g., that ``session_required_mfa`` requires either @@ -51,7 +51,7 @@ in your application, e.g.: # Render a strict dictionary error.to_dict() -If non-canonical fields were supplied on creation (either as keyword arguments +If non-canonical fields are supplied on creation (either as keyword arguments during instantiation or as fields in a dictionary supplied to ``from_dict()``), you can preserve these fields in the rendered output dictionary by specifying ``include_extra=True``. @@ -83,7 +83,7 @@ Parsing Responses ----------------- If you are writing a client to a Globus API, the ``session_errors`` subpackage -in the Python SDK provides utilities to detect legacy session error response +provides utilities to detect legacy session error response formats and normalize them. To detect if a ``GlobusAPIError``, ``ErrorSubdocument``, or JSON response @@ -98,7 +98,10 @@ you can use, e.g.,: "code": "ConsentRequired", "message": "Missing required foo consent", } - session_error.utils.is_session_error(error_dict) # False + # The dict is not a session error, so `False` is returned. + session_error.utils.is_session_error(error_dict) + + # The dict is not a session error and cannot be converted. session_error.utils.to_session_error(error_dict) # None error_dict = { @@ -110,11 +113,12 @@ you can use, e.g.,: session_error.utils.to_session_error(error_dict) # GlobusSessionError .. note:: + If a GlobusAPIError represents multiple errors that were returned in an array, this only returns the first error in that array that can be converted to the Globus Session Error response format. In this case (and, - in general) it's preferable to use ``to_session_error()`` (which also - accepts a list of ``GlobusAPIErrors``, ``ErrorSubdocuments``, and JSON + in general) it's preferable to use ``to_session_errors()`` (which also + accepts a list of ``GlobusAPIError``s, ``ErrorSubdocument``s, and JSON response dictionaries): .. code-block:: python diff --git a/src/globus_sdk/experimental/session_error/utils.py b/src/globus_sdk/experimental/session_error/utils.py index ff94e36a7..9cef5e0bb 100644 --- a/src/globus_sdk/experimental/session_error/utils.py +++ b/src/globus_sdk/experimental/session_error/utils.py @@ -20,6 +20,7 @@ def to_session_error( attempting to match to GlobusSessionError (preferred) or legacy variants. .. note:: + a GlobusAPIError may contain multiple errors, and in this case only a single session error is returned for the first error that matches a known format. @@ -71,6 +72,7 @@ def to_session_errors( GlobusSessionError (preferred) or legacy variants. .. note:: + A GlobusAPIError may contain multiple errors, so the result list could be longer than the provided list. From 1458b4d320d6ad9fc60b09982a2edb3debda13e2 Mon Sep 17 00:00:00 2001 From: Ada Date: Thu, 6 Jul 2023 10:35:02 -0700 Subject: [PATCH 05/32] Adopt `construct_error` from `_testing` Signed-off-by: Ada --- tests/unit/experimental/test_session_error.py | 58 +++++++------------ 1 file changed, 21 insertions(+), 37 deletions(-) diff --git a/tests/unit/experimental/test_session_error.py b/tests/unit/experimental/test_session_error.py index cbfa537c1..1e9c17355 100644 --- a/tests/unit/experimental/test_session_error.py +++ b/tests/unit/experimental/test_session_error.py @@ -1,8 +1,7 @@ -import json - import pytest -from globus_sdk.exc import ErrorSubdocument, GlobusAPIError +from globus_sdk._testing import construct_error +from globus_sdk.exc import ErrorSubdocument from globus_sdk.experimental.session_error import ( GlobusSessionError, has_session_errors, @@ -11,24 +10,6 @@ to_session_errors, ) -# TODO: We should move this to a common location -from tests.unit.errors.conftest import _mk_response - - -def _error_for_json_dict(error_dict, status=403, method="POST", headers=None): - combined_headers = {"Content-Type": "application/json"} - if headers: - combined_headers.update(headers) - - response = _mk_response( - data=error_dict, - status=status, - method=method, - headers=combined_headers, - data_transform=json.dumps, - ) - return GlobusAPIError(response.r) - @pytest.mark.parametrize( "error_dict, status", @@ -64,7 +45,7 @@ def test_create_session_error_from_consent_error(error_dict, status): """ # Create various supplementary objects representing this error error_subdoc = ErrorSubdocument(error_dict) - api_error = _error_for_json_dict(error_dict, status=status) + api_error = construct_error(body=error_dict, http_status=status) for error in (error_dict, error_subdoc, api_error): # Test boolean utility functions @@ -121,7 +102,7 @@ def test_create_session_error_from_authorization_error(authorization_parameters) # Create various supplementary objects representing this error error_dict = {"authorization_parameters": authorization_parameters} error_subdoc = ErrorSubdocument(error_dict) - api_error = _error_for_json_dict(error_dict) + api_error = construct_error(body=error_dict, http_status=403) for error in (error_dict, error_subdoc, api_error): # Test boolean utility functions @@ -178,7 +159,7 @@ def test_create_session_error_from_authorization_error_csv(authorization_paramet error_dict["authorization_parameters"][key] = value error_subdoc = ErrorSubdocument(error_dict) - api_error = _error_for_json_dict(error_dict) + api_error = construct_error(body=error_dict, http_status=403) for error in (error_dict, error_subdoc, api_error): # Test boolean utility functions @@ -206,8 +187,8 @@ def test_create_session_errors_from_multiple_errors(): GlobusSessionErrors, and additionally test that this is correct even when mingled with other accepted data types. """ - consent_errors = _error_for_json_dict( - { + consent_errors = construct_error( + body={ "errors": [ { "code": "ConsentRequired", @@ -230,11 +211,12 @@ def test_create_session_errors_from_multiple_errors(): }, }, ] - } + }, + http_status=403, ) - authorization_error = _error_for_json_dict( - { + authorization_error = construct_error( + body={ "authorization_parameters": { "session_message": ( "You need to authenticate with an identity that " @@ -242,14 +224,16 @@ def test_create_session_errors_from_multiple_errors(): ), "session_required_policies": ["foo", "baz"], } - } + }, + http_status=403, ) - not_an_error = _error_for_json_dict( - { + not_an_error = construct_error( + body={ "code": "NotAnError", "message": "This is not an error", - } + }, + http_status=403, ) all_errors = [consent_errors, not_an_error, authorization_error] @@ -312,7 +296,7 @@ def test_create_session_error_from_legacy_authorization_error_with_code(): # Create various supplementary objects representing this error error_subdoc = ErrorSubdocument(error_dict) - api_error = _error_for_json_dict(error_dict) + api_error = construct_error(body=error_dict, http_status=403) for error in (error_dict, error_subdoc, api_error): # Test boolean utility functions @@ -344,8 +328,8 @@ def test_backward_compatibility_consent_required_error(): # Create an API error with a backward compatible data schema using # distinct values for duplicative fields to facilitate testing # (in practice these would be the same) - error = _error_for_json_dict( - { + error = construct_error( + body={ "code": "ConsentRequired", "message": "Missing required foo_bar consent", "request_id": "WmMV97A1w", @@ -362,7 +346,7 @@ def test_backward_compatibility_consent_required_error(): "optional": "A non-canonical field", }, }, - status=403, + http_status=403, ) # Test boolean utility functions From 520477957bfc6c8e256ee00d679a1e9e9942c9ed Mon Sep 17 00:00:00 2001 From: Ada Date: Thu, 6 Jul 2023 10:55:23 -0700 Subject: [PATCH 06/32] Add debug logging to session error parsing Signed-off-by: Ada --- .../experimental/session_error/session_error.py | 8 +++++--- .../experimental/session_error/utils.py | 16 +++++++++++++--- .../experimental/session_error/variants.py | 13 +++++-------- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/globus_sdk/experimental/session_error/session_error.py b/src/globus_sdk/experimental/session_error/session_error.py index 6f3e80da6..8b5cf4a57 100644 --- a/src/globus_sdk/experimental/session_error/session_error.py +++ b/src/globus_sdk/experimental/session_error/session_error.py @@ -88,7 +88,9 @@ def from_dict( if field_name in param_dict and not isinstance( param_dict[field_name], field_type ): - raise ValueError(f"{field_name} must be of type {field_type.__name__}") + raise ValueError( + f"'{field_name}' must be of type {field_type.__name__}" + ) return cls(**param_dict) @@ -155,12 +157,12 @@ def from_dict(cls, error_dict: t.Dict[str, t.Any]) -> "GlobusSessionError": """ if "code" not in error_dict: - raise ValueError("GlobusSessionError must have a code") + raise ValueError("Must have a 'code'") # Enforce that authorization_parameters is in the error_dict and # contains at least one of the fields we expect if "authorization_parameters" not in error_dict: - raise ValueError("GlobusSessionError must have authorization_parameters") + raise ValueError("Must have 'authorization_parameters'") kwargs = copy.deepcopy(error_dict) diff --git a/src/globus_sdk/experimental/session_error/utils.py b/src/globus_sdk/experimental/session_error/utils.py index 9cef5e0bb..ffd9aaf31 100644 --- a/src/globus_sdk/experimental/session_error/utils.py +++ b/src/globus_sdk/experimental/session_error/utils.py @@ -1,5 +1,5 @@ +import logging import typing as t -from contextlib import suppress from globus_sdk.exc import ErrorSubdocument, GlobusAPIError @@ -11,6 +11,8 @@ LegacySessionErrorVariant, ) +log = logging.getLogger(__name__) + def to_session_error( error: t.Union[GlobusAPIError, ErrorSubdocument, t.Dict[str, t.Any]] @@ -48,8 +50,12 @@ def to_session_error( error_dict = error # Prefer a proper session error, if possible - with suppress(ValueError): + try: return GlobusSessionError.from_dict(error_dict) + except ValueError as err: + log.debug( + f"Failed to parse error as 'GlobusSessionError' because: {err.args[0]}" + ) supported_variants: t.List[t.Type[LegacySessionErrorVariant]] = [ LegacyAuthorizationParametersError, @@ -57,8 +63,12 @@ def to_session_error( LegacyConsentRequiredAPError, ] for variant in supported_variants: - with suppress(ValueError): + try: return variant.from_dict(error_dict).to_session_error() + except ValueError as err: + log.debug( + f"Failed to parse error as '{variant.__name__}' because: {err.args[0]}" + ) return None diff --git a/src/globus_sdk/experimental/session_error/variants.py b/src/globus_sdk/experimental/session_error/variants.py index e5efd74eb..3367f58f4 100644 --- a/src/globus_sdk/experimental/session_error/variants.py +++ b/src/globus_sdk/experimental/session_error/variants.py @@ -63,10 +63,10 @@ def from_dict( :type error_dict: dict """ if error_dict.get("code") != "ConsentRequired": - raise ValueError("Must be a ConsentRequired error") + raise ValueError("'code' must be 'ConsentRequired'") if not error_dict.get("required_scopes"): - raise ValueError("Must include required_scopes") + raise ValueError("Must include 'required_scopes'") return cls(**error_dict) @@ -116,10 +116,10 @@ def from_dict( :type error_dict: dict """ if error_dict.get("code") != "ConsentRequired": - raise ValueError("Must be a ConsentRequired error") + raise ValueError("'code' must be 'ConsentRequired'") if not error_dict.get("required_scope"): - raise ValueError("Must include required_scope") + raise ValueError("Must include 'required_scope'") return cls(**error_dict) @@ -257,10 +257,7 @@ def from_dict( if not isinstance(error_dict, dict) or not isinstance( error_dict.get("authorization_parameters"), dict ): - raise ValueError( - "LegacyAuthorizationParametersError must be a dict that contains an " - "'authorization_parameters' dict" - ) + raise ValueError("Must contain an 'authorization_parameters' dict") extra_fields = { key: value From 0fae66808a88be2139c89af278ed2254d5e9bc9e Mon Sep 17 00:00:00 2001 From: Ada Date: Thu, 6 Jul 2023 11:01:00 -0700 Subject: [PATCH 07/32] Suggest more strongly that variants are not public Signed-off-by: Ada --- .../experimental/session_error/{variants.py => _variants.py} | 0 src/globus_sdk/experimental/session_error/utils.py | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) rename src/globus_sdk/experimental/session_error/{variants.py => _variants.py} (100%) diff --git a/src/globus_sdk/experimental/session_error/variants.py b/src/globus_sdk/experimental/session_error/_variants.py similarity index 100% rename from src/globus_sdk/experimental/session_error/variants.py rename to src/globus_sdk/experimental/session_error/_variants.py diff --git a/src/globus_sdk/experimental/session_error/utils.py b/src/globus_sdk/experimental/session_error/utils.py index ffd9aaf31..64adacc06 100644 --- a/src/globus_sdk/experimental/session_error/utils.py +++ b/src/globus_sdk/experimental/session_error/utils.py @@ -3,13 +3,13 @@ from globus_sdk.exc import ErrorSubdocument, GlobusAPIError -from .session_error import GlobusSessionError -from .variants import ( +from ._variants import ( LegacyAuthorizationParametersError, LegacyConsentRequiredAPError, LegacyConsentRequiredTransferError, LegacySessionErrorVariant, ) +from .session_error import GlobusSessionError log = logging.getLogger(__name__) From 1aab07a74cca8aa0ef429af898e53d9d0ae70350 Mon Sep 17 00:00:00 2001 From: Ada Date: Fri, 7 Jul 2023 11:03:37 -0700 Subject: [PATCH 08/32] Update type annotations to modern style Signed-off-by: Ada --- .../experimental/session_error/_variants.py | 84 +++++++++---------- .../session_error/session_error.py | 38 +++++---- .../experimental/session_error/utils.py | 18 ++-- 3 files changed, 70 insertions(+), 70 deletions(-) diff --git a/src/globus_sdk/experimental/session_error/_variants.py b/src/globus_sdk/experimental/session_error/_variants.py index 3367f58f4..61a80f4a0 100644 --- a/src/globus_sdk/experimental/session_error/_variants.py +++ b/src/globus_sdk/experimental/session_error/_variants.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import typing as t from .session_error import GlobusSessionError, GlobusSessionErrorAuthorizationParameters @@ -11,7 +13,7 @@ class LegacySessionErrorVariant: """ @classmethod - def from_dict(cls: t.Type[T], error_dict: t.Dict[str, t.Any]) -> T: + def from_dict(cls: t.Type[T], error_dict: dict[str, t.Any]) -> T: raise NotImplementedError() def to_session_error(self) -> GlobusSessionError: @@ -26,18 +28,18 @@ class LegacyConsentRequiredTransferError(LegacySessionErrorVariant): def __init__( self, code: str, - required_scopes: t.List[str], - message: t.Optional[str] = None, - request_id: t.Optional[str] = None, - resource: t.Optional[str] = None, + required_scopes: list[str], + message: str | None = None, + request_id: str | None = None, + resource: str | None = None, **kwargs: t.Any, ): self.code: str = code - self.required_scopes: t.List[str] = required_scopes - self.message: t.Optional[str] = message - self.request_id: t.Optional[str] = request_id - self.resource: t.Optional[str] = resource - self.extra_fields: t.Dict[str, t.Any] = kwargs + self.required_scopes: list[str] = required_scopes + self.message: str | None = message + self.request_id: str | None = request_id + self.resource: str | None = resource + self.extra_fields: dict[str, t.Any] = kwargs def to_session_error(self) -> GlobusSessionError: """ @@ -53,8 +55,8 @@ def to_session_error(self) -> GlobusSessionError: @classmethod def from_dict( - cls, error_dict: t.Dict[str, t.Any] - ) -> "LegacyConsentRequiredTransferError": + cls, error_dict: dict[str, t.Any] + ) -> LegacyConsentRequiredTransferError: """ Instantiate from an error dictionary. Raises a ValueError if the dictionary does not contain a recognized LegacyConsentRequiredTransferError. @@ -81,13 +83,13 @@ def __init__( self, code: str, required_scope: str, - description: t.Optional[str] = None, + description: str | None = None, **kwargs: t.Any, ): self.code: str = code self.required_scope: str = required_scope - self.description: t.Optional[str] = description - self.extra_fields: t.Dict[str, t.Any] = kwargs + self.description: str | None = description + self.extra_fields: dict[str, t.Any] = kwargs def to_session_error(self) -> GlobusSessionError: """ @@ -105,9 +107,7 @@ def to_session_error(self) -> GlobusSessionError: ) @classmethod - def from_dict( - cls, error_dict: t.Dict[str, t.Any] - ) -> "LegacyConsentRequiredAPError": + def from_dict(cls, error_dict: dict[str, t.Any]) -> LegacyConsentRequiredAPError: """ Instantiate from an error dictionary. Raises a ValueError if the dictionary does not contain a recognized LegacyConsentRequiredAPError. @@ -143,29 +143,27 @@ class LegacyAuthorizationParameters: def __init__( self, - session_message: t.Optional[str] = None, - session_required_identities: t.Optional[t.List[str]] = None, - session_required_policies: t.Optional[t.Union[str, t.List[str]]] = None, - session_required_single_domain: t.Optional[t.Union[str, t.List[str]]] = None, - session_required_mfa: t.Optional[bool] = None, - session_required_scopes: t.Optional[t.List[str]] = None, + session_message: str | None = None, + session_required_identities: list[str] | None = None, + session_required_policies: str | list[str] | None = None, + session_required_single_domain: str | list[str] | None = None, + session_required_mfa: bool | None = None, + session_required_scopes: list[str] | None = None, **kwargs: t.Any, ): - self.session_message: t.Optional[str] = session_message - self.session_required_identities: t.Optional[ - t.List[str] - ] = session_required_identities - self.session_required_policies: t.Optional[ - t.Union[str, t.List[str]] - ] = session_required_policies - self.session_required_single_domain: t.Optional[ - t.Union[str, t.List[str]] - ] = session_required_single_domain - self.session_required_mfa: t.Optional[bool] = session_required_mfa + self.session_message: str | None = session_message + self.session_required_identities: list[str] | None = session_required_identities + self.session_required_policies: str | list[ + str + ] | None = session_required_policies + self.session_required_single_domain: str | list[ + str + ] | None = session_required_single_domain + self.session_required_mfa: bool | None = session_required_mfa # Declared here for compatibility with mixed legacy payloads - self.session_required_scopes: t.Optional[t.List[str]] = session_required_scopes + self.session_required_scopes: list[str] | None = session_required_scopes # Retain any additional fields - self.extra_fields: t.Dict[str, t.Any] = kwargs + self.extra_fields: dict[str, t.Any] = kwargs def to_session_error_authorization_parameters( self, @@ -196,9 +194,7 @@ def to_session_error_authorization_parameters( ) @classmethod - def from_dict( - cls, param_dict: t.Dict[str, t.Any] - ) -> "LegacyAuthorizationParameters": + def from_dict(cls, param_dict: dict[str, t.Any]) -> LegacyAuthorizationParameters: """ Create from a dictionary. Raises a ValueError if the dictionary does not contain a recognized LegacyAuthorizationParameters format. @@ -235,7 +231,7 @@ class LegacyAuthorizationParametersError(LegacySessionErrorVariant): def __init__( self, authorization_parameters: LegacyAuthorizationParameters, - code: t.Optional[str] = None, + code: str | None = None, **kwargs: t.Any, ): self.authorization_parameters: LegacyAuthorizationParameters = ( @@ -243,12 +239,12 @@ def __init__( ) self.code: str = code or self.DEFAULT_CODE # Retain any additional fields - self.extra_fields: t.Dict[str, t.Any] = kwargs + self.extra_fields: dict[str, t.Any] = kwargs @classmethod def from_dict( - cls, error_dict: t.Dict[str, t.Any] - ) -> "LegacyAuthorizationParametersError": + cls, error_dict: dict[str, t.Any] + ) -> LegacyAuthorizationParametersError: """ Instantiate a LegacyAuthorizationParametersError from a dictionary. """ diff --git a/src/globus_sdk/experimental/session_error/session_error.py b/src/globus_sdk/experimental/session_error/session_error.py index 8b5cf4a57..16d0eb273 100644 --- a/src/globus_sdk/experimental/session_error/session_error.py +++ b/src/globus_sdk/experimental/session_error/session_error.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import copy import typing as t @@ -46,28 +48,28 @@ class GlobusSessionErrorAuthorizationParameters: def __init__( self, - session_message: t.Optional[str] = None, - session_required_identities: t.Optional[t.List[str]] = None, - session_required_policies: t.Optional[t.List[str]] = None, - session_required_single_domain: t.Optional[t.List[str]] = None, - session_required_mfa: t.Optional[bool] = None, - session_required_scopes: t.Optional[t.List[str]] = None, + session_message: str | None = None, + session_required_identities: list[str] | None = None, + session_required_policies: list[str] | None = None, + session_required_single_domain: list[str] | None = None, + session_required_mfa: bool | None = None, + session_required_scopes: list[str] | None = None, **kwargs: t.Any, ): - self.session_message: t.Optional[str] = session_message + self.session_message: str | None = session_message self.session_required_identities = session_required_identities self.session_required_policies = session_required_policies - self.session_required_single_domain: t.Optional[ - t.List[str] - ] = session_required_single_domain - self.session_required_mfa: t.Optional[bool] = session_required_mfa - self.session_required_scopes: t.Optional[t.List[str]] = session_required_scopes - self.extra_fields: t.Dict[str, t.Any] = kwargs + self.session_required_single_domain: list[ + str + ] | None = session_required_single_domain + self.session_required_mfa: bool | None = session_required_mfa + self.session_required_scopes: list[str] | None = session_required_scopes + self.extra_fields: dict[str, t.Any] = kwargs @classmethod def from_dict( - cls, param_dict: t.Dict[str, t.Any] - ) -> "GlobusSessionErrorAuthorizationParameters": + cls, param_dict: dict[str, t.Any] + ) -> GlobusSessionErrorAuthorizationParameters: """ Instantiate from a session error authorization parameters dictionary. Raises a ValueError if the dictionary does not contain a valid GlobusSessionError. @@ -94,7 +96,7 @@ def from_dict( return cls(**param_dict) - def to_dict(self, include_extra: bool = False) -> t.Dict[str, t.Any]: + def to_dict(self, include_extra: bool = False) -> dict[str, t.Any]: """ Return a session error authorization parameters dictionary. @@ -148,7 +150,7 @@ def __init__( self.extra_fields = kwargs @classmethod - def from_dict(cls, error_dict: t.Dict[str, t.Any]) -> "GlobusSessionError": + def from_dict(cls, error_dict: dict[str, t.Any]) -> GlobusSessionError: """ Instantiate a GlobusSessionError from a dictionary. @@ -175,7 +177,7 @@ def from_dict(cls, error_dict: t.Dict[str, t.Any]) -> "GlobusSessionError": return cls(**kwargs) - def to_dict(self, include_extra: bool = False) -> t.Dict[str, t.Any]: + def to_dict(self, include_extra: bool = False) -> dict[str, t.Any]: """ Return a session error response dictionary. diff --git a/src/globus_sdk/experimental/session_error/utils.py b/src/globus_sdk/experimental/session_error/utils.py index 64adacc06..a56229ed4 100644 --- a/src/globus_sdk/experimental/session_error/utils.py +++ b/src/globus_sdk/experimental/session_error/utils.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import logging import typing as t @@ -15,8 +17,8 @@ def to_session_error( - error: t.Union[GlobusAPIError, ErrorSubdocument, t.Dict[str, t.Any]] -) -> t.Optional[GlobusSessionError]: + error: GlobusAPIError | ErrorSubdocument | dict[str, t.Any] +) -> GlobusSessionError | None: """ Converts a GlobusAPIError, ErrorSubdocument, or dict into a GlobusSessionError by attempting to match to GlobusSessionError (preferred) or legacy variants. @@ -57,7 +59,7 @@ def to_session_error( f"Failed to parse error as 'GlobusSessionError' because: {err.args[0]}" ) - supported_variants: t.List[t.Type[LegacySessionErrorVariant]] = [ + supported_variants: list[t.Type[LegacySessionErrorVariant]] = [ LegacyAuthorizationParametersError, LegacyConsentRequiredTransferError, LegacyConsentRequiredAPError, @@ -74,8 +76,8 @@ def to_session_error( def to_session_errors( - errors: t.List[t.Union[GlobusAPIError, ErrorSubdocument, t.Dict[str, t.Any]]] -) -> t.List[GlobusSessionError]: + errors: list[GlobusAPIError | ErrorSubdocument | dict[str, t.Any]] +) -> list[GlobusSessionError]: """ Converts a list of GlobusAPIErrors, ErrorSubdocuments, or dicts into a list of GlobusSessionErrors by attempting to match each error to @@ -91,7 +93,7 @@ def to_session_errors( :param errors: The errors to convert. :type errors: a list of GlobusAPIErrors, ErrorSubdocuments, or dicts """ - candidate_errors: t.List[t.Union[ErrorSubdocument, t.Dict[str, t.Any]]] = [] + candidate_errors: list[ErrorSubdocument | dict[str, t.Any]] = [] for error in errors: if isinstance(error, GlobusAPIError): # Use the ErrorSubdocuments @@ -107,7 +109,7 @@ def to_session_errors( def is_session_error( - error: t.Union[GlobusAPIError, ErrorSubdocument, t.Dict[str, t.Any]] + error: GlobusAPIError | ErrorSubdocument | dict[str, t.Any] ) -> bool: """ Return True if the provided error matches a known session error format. @@ -119,7 +121,7 @@ def is_session_error( def has_session_errors( - errors: t.List[t.Union[GlobusAPIError, ErrorSubdocument, t.Dict[str, t.Any]]] + errors: list[GlobusAPIError | ErrorSubdocument | dict[str, t.Any]] ) -> bool: """ Return True if any of the provided errors match a known session error format. From 445d9ab772a12c3c8cdf049d7706bad2e2cd8cd1 Mon Sep 17 00:00:00 2001 From: Ada Date: Tue, 11 Jul 2023 09:40:07 -0700 Subject: [PATCH 09/32] Move validation to constructors Signed-off-by: Ada --- .../experimental/session_error/_variants.py | 229 +++++++++++------- .../session_error/session_error.py | 128 ++++++---- 2 files changed, 222 insertions(+), 135 deletions(-) diff --git a/src/globus_sdk/experimental/session_error/_variants.py b/src/globus_sdk/experimental/session_error/_variants.py index 61a80f4a0..759e6763f 100644 --- a/src/globus_sdk/experimental/session_error/_variants.py +++ b/src/globus_sdk/experimental/session_error/_variants.py @@ -25,21 +25,32 @@ class LegacyConsentRequiredTransferError(LegacySessionErrorVariant): The ConsentRequired error format emitted by the Globus Transfer service. """ + code: str + required_scopes: list[str] + extra_fields: dict[str, t.Any] + + SUPPORTED_FIELDS = { + "code": (str,), + "required_scopes": (list,), + } + def __init__( self, - code: str, - required_scopes: list[str], - message: str | None = None, - request_id: str | None = None, - resource: str | None = None, - **kwargs: t.Any, + code: str | None, + required_scopes: list[str] | None, + extra: dict[str, t.Any] | None = None, ): - self.code: str = code - self.required_scopes: list[str] = required_scopes - self.message: str | None = message - self.request_id: str | None = request_id - self.resource: str | None = resource - self.extra_fields: dict[str, t.Any] = kwargs + # Needed to make mypy happy w/ stricter types on instance vars. + # Will clean up in a subsequent commit + if not isinstance(code, str) or code != "ConsentRequired": + raise ValueError("'code' must be the string 'ConsentRequired'") + + if not isinstance(required_scopes, list): + raise ValueError("'required_scopes' must be a list") + + self.code = code + self.required_scopes = required_scopes + self.extra_fields = extra or {} def to_session_error(self) -> GlobusSessionError: """ @@ -49,8 +60,9 @@ def to_session_error(self) -> GlobusSessionError: code=self.code, authorization_parameters=GlobusSessionErrorAuthorizationParameters( session_required_scopes=self.required_scopes, - session_message=self.message, + session_message=self.extra_fields.get("message"), ), + extra=self.extra_fields, ) @classmethod @@ -64,13 +76,14 @@ def from_dict( :param error_dict: The dictionary to instantiate the error from. :type error_dict: dict """ - if error_dict.get("code") != "ConsentRequired": - raise ValueError("'code' must be 'ConsentRequired'") - - if not error_dict.get("required_scopes"): - raise ValueError("Must include 'required_scopes'") + # Extract any extra fields + extras = {k: v for k, v in error_dict.items() if k not in cls.SUPPORTED_FIELDS} + kwargs: dict[str, t.Any] = {"extra": extras} + # Ensure required fields are supplied + for field_name in cls.SUPPORTED_FIELDS.keys(): + kwargs[field_name] = error_dict.get(field_name) - return cls(**error_dict) + return cls(**kwargs) class LegacyConsentRequiredAPError(LegacySessionErrorVariant): @@ -79,17 +92,32 @@ class LegacyConsentRequiredAPError(LegacySessionErrorVariant): Action Providers. """ + code: str + required_scope: str + extra_fields: dict[str, t.Any] + + SUPPORTED_FIELDS = { + "code": (str,), + "required_scope": (str,), + } + def __init__( self, - code: str, - required_scope: str, - description: str | None = None, - **kwargs: t.Any, + code: str | None, + required_scope: str | None, + extra: dict[str, t.Any] | None, ): - self.code: str = code - self.required_scope: str = required_scope - self.description: str | None = description - self.extra_fields: dict[str, t.Any] = kwargs + # Needed to make mypy happy w/ stricter types on instance vars. + # Will clean up in a subsequent commit + if not isinstance(code, str) or code != "ConsentRequired": + raise ValueError("'code' must be the string 'ConsentRequired'") + + if not isinstance(required_scope, str): + raise ValueError("'required_scope' must be a list") + + self.code = code + self.required_scope = required_scope + self.extra_fields = extra or {} def to_session_error(self) -> GlobusSessionError: """ @@ -102,8 +130,14 @@ def to_session_error(self) -> GlobusSessionError: code=self.code, authorization_parameters=GlobusSessionErrorAuthorizationParameters( session_required_scopes=[self.required_scope], - session_message=self.description, + session_message=self.extra_fields.get("description"), + extra=self.extra_fields.get("authorization_parameters"), ), + extra={ + k: v + for k, v in self.extra_fields.items() + if k != "authorization_parameters" + }, ) @classmethod @@ -115,13 +149,15 @@ def from_dict(cls, error_dict: dict[str, t.Any]) -> LegacyConsentRequiredAPError :param error_dict: The dictionary to create the error from. :type error_dict: dict """ - if error_dict.get("code") != "ConsentRequired": - raise ValueError("'code' must be 'ConsentRequired'") - if not error_dict.get("required_scope"): - raise ValueError("Must include 'required_scope'") + # Extract any extra fields + extras = {k: v for k, v in error_dict.items() if k not in cls.SUPPORTED_FIELDS} + kwargs: dict[str, t.Any] = {"extra": extras} + # Ensure required fields are supplied + for field_name in cls.SUPPORTED_FIELDS.keys(): + kwargs[field_name] = error_dict.get(field_name) - return cls(**error_dict) + return cls(**kwargs) class LegacyAuthorizationParameters: @@ -130,6 +166,15 @@ class LegacyAuthorizationParameters: Globus services. """ + session_message: str | None + session_required_identities: list[str] | None + session_required_policies: str | list[str] | None + session_required_single_domain: str | list[str] | None + session_required_mfa: bool | None + # Declared here for compatibility with mixed legacy payloads + session_required_scopes: list[str] | None + extra_fields: dict[str, t.Any] + DEFAULT_CODE = "AuthorizationRequired" SUPPORTED_FIELDS = { @@ -149,21 +194,33 @@ def __init__( session_required_single_domain: str | list[str] | None = None, session_required_mfa: bool | None = None, session_required_scopes: list[str] | None = None, - **kwargs: t.Any, + extra: dict[str, t.Any] | None = None, ): - self.session_message: str | None = session_message - self.session_required_identities: list[str] | None = session_required_identities - self.session_required_policies: str | list[ - str - ] | None = session_required_policies - self.session_required_single_domain: str | list[ - str - ] | None = session_required_single_domain - self.session_required_mfa: bool | None = session_required_mfa + self.session_message = session_message + self.session_required_identities = session_required_identities + self.session_required_policies = session_required_policies + self.session_required_single_domain = session_required_single_domain + self.session_required_mfa = session_required_mfa # Declared here for compatibility with mixed legacy payloads - self.session_required_scopes: list[str] | None = session_required_scopes + self.session_required_scopes = session_required_scopes # Retain any additional fields - self.extra_fields: dict[str, t.Any] = kwargs + self.extra_fields = extra or {} + + # Enforce that the error contains at least one of the fields we expect + if not any( + (getattr(self, field_name) is not None) + for field_name in self.SUPPORTED_FIELDS.keys() + ): + raise ValueError( + "Must include at least one supported authorization parameter: " + ", ".join(self.SUPPORTED_FIELDS.keys()) + ) + + # Enforce the field types + for field_name, field_types in self.SUPPORTED_FIELDS.items(): + field_value = getattr(self, field_name) + if field_value is not None and not isinstance(field_value, field_types): + raise ValueError(f"'{field_name}' must be one of {field_types}") def to_session_error_authorization_parameters( self, @@ -190,7 +247,7 @@ def to_session_error_authorization_parameters( session_required_policies=required_policies, session_required_single_domain=required_single_domain, session_required_scopes=self.session_required_scopes, - **self.extra_fields, + extra=self.extra_fields, ) @classmethod @@ -202,22 +259,15 @@ def from_dict(cls, param_dict: dict[str, t.Any]) -> LegacyAuthorizationParameter :param param_dict: The dictionary to create the AuthorizationParameters from. :type param_dict: dict """ - if not any(field in param_dict for field in cls.SUPPORTED_FIELDS.keys()): - raise ValueError( - "Must include at least one supported authorization parameter: " - ", ".join(cls.SUPPORTED_FIELDS.keys()) - ) - for field, field_types in cls.SUPPORTED_FIELDS.items(): - if field not in param_dict: - continue - if not isinstance(param_dict[field], field_types): - raise ValueError( - f"Field {field} must be one of {field_types}, " - "got {error_dict[field]}" - ) + # Extract any extra fields + extras = {k: v for k, v in param_dict.items() if k not in cls.SUPPORTED_FIELDS} + kwargs: dict[str, t.Any] = {"extra": extras} + # Ensure required fields are supplied + for field_name in cls.SUPPORTED_FIELDS.keys(): + kwargs[field_name] = param_dict.get(field_name) - return cls(**param_dict) + return cls(**kwargs) class LegacyAuthorizationParametersError(LegacySessionErrorVariant): @@ -226,20 +276,38 @@ class LegacyAuthorizationParametersError(LegacySessionErrorVariant): in use by Globus services. """ + authorization_parameters: LegacyAuthorizationParameters + code: str + extra_fields: dict[str, t.Any] + DEFAULT_CODE = "AuthorizationRequired" + SUPPORTED_FIELDS = { + "code": (str,), + "authorization_parameters": (LegacyAuthorizationParameters,), + } + def __init__( self, - authorization_parameters: LegacyAuthorizationParameters, + authorization_parameters: dict[str, t.Any] + | LegacyAuthorizationParameters + | None, code: str | None = None, - **kwargs: t.Any, + extra: dict[str, t.Any] | None = None, ): - self.authorization_parameters: LegacyAuthorizationParameters = ( - authorization_parameters - ) - self.code: str = code or self.DEFAULT_CODE + self.code = code or self.DEFAULT_CODE + + if isinstance(authorization_parameters, LegacyAuthorizationParameters): + self.authorization_parameters = authorization_parameters + elif isinstance(authorization_parameters, dict): + self.authorization_parameters = LegacyAuthorizationParameters.from_dict( + param_dict=authorization_parameters + ) + else: + raise ValueError("Must have 'authorization_parameters'") + # Retain any additional fields - self.extra_fields: dict[str, t.Any] = kwargs + self.extra_fields = extra or {} @classmethod def from_dict( @@ -249,25 +317,14 @@ def from_dict( Instantiate a LegacyAuthorizationParametersError from a dictionary. """ - # Enforce that authorization_parameters is present in the error_dict - if not isinstance(error_dict, dict) or not isinstance( - error_dict.get("authorization_parameters"), dict - ): - raise ValueError("Must contain an 'authorization_parameters' dict") - - extra_fields = { - key: value - for key, value in error_dict.items() - if key not in ("authorization_parameters", "code") - } + # Extract any extra fields + extras = {k: v for k, v in error_dict.items() if k not in cls.SUPPORTED_FIELDS} + kwargs: dict[str, t.Any] = {"extra": extras} + # Ensure required fields are supplied + for field_name in cls.SUPPORTED_FIELDS.keys(): + kwargs[field_name] = error_dict.get(field_name) - return cls( - authorization_parameters=LegacyAuthorizationParameters.from_dict( - error_dict["authorization_parameters"] - ), - code=error_dict.get("code"), - **extra_fields, - ) + return cls(**kwargs) def to_session_error(self) -> GlobusSessionError: """ @@ -279,5 +336,5 @@ def to_session_error(self) -> GlobusSessionError: return GlobusSessionError( authorization_parameters=authorization_parameters, code=self.code, - **self.extra_fields, + extra=self.extra_fields, ) diff --git a/src/globus_sdk/experimental/session_error/session_error.py b/src/globus_sdk/experimental/session_error/session_error.py index 16d0eb273..c0e32d439 100644 --- a/src/globus_sdk/experimental/session_error/session_error.py +++ b/src/globus_sdk/experimental/session_error/session_error.py @@ -1,6 +1,5 @@ from __future__ import annotations -import copy import typing as t from globus_sdk.exc import GlobusError @@ -37,6 +36,14 @@ class GlobusSessionErrorAuthorizationParameters: :vartype extra_fields: dict """ + session_message: str | None + session_required_identities: list[str] | None + session_required_policies: list[str] | None + session_required_single_domain: list[str] | None + session_required_mfa: bool | None + session_required_scopes: list[str] | None + extra_fields: dict[str, t.Any] + SUPPORTED_FIELDS = { "session_message": str, "session_required_identities": list, @@ -54,17 +61,33 @@ def __init__( session_required_single_domain: list[str] | None = None, session_required_mfa: bool | None = None, session_required_scopes: list[str] | None = None, - **kwargs: t.Any, + extra: dict[str, t.Any] | None = None, ): - self.session_message: str | None = session_message + self.session_message = session_message self.session_required_identities = session_required_identities self.session_required_policies = session_required_policies - self.session_required_single_domain: list[ - str - ] | None = session_required_single_domain - self.session_required_mfa: bool | None = session_required_mfa - self.session_required_scopes: list[str] | None = session_required_scopes - self.extra_fields: dict[str, t.Any] = kwargs + self.session_required_single_domain = session_required_single_domain + self.session_required_mfa = session_required_mfa + self.session_required_scopes = session_required_scopes + self.extra_fields = extra or {} + + # Enforce that the error contains at least one of the fields we expect + if not any( + (getattr(self, field_name) is not None) + for field_name in self.SUPPORTED_FIELDS.keys() + ): + raise ValueError( + "Must include at least one supported authorization parameter: " + ", ".join(self.SUPPORTED_FIELDS.keys()) + ) + + # Enforce the field types + for field_name, field_type in self.SUPPORTED_FIELDS.items(): + field_value = getattr(self, field_name) + if field_value is not None and not isinstance(field_value, field_type): + raise ValueError( + f"'{field_name}' must be of type {field_type.__name__}" + ) @classmethod def from_dict( @@ -78,23 +101,14 @@ def from_dict( :type param_dict: dict """ - # Enforce that the error_dict contains at least one of the fields we expect - if not any(field in param_dict for field in cls.SUPPORTED_FIELDS.keys()): - raise ValueError( - "Must include at least one supported authorization parameter: " - ", ".join(cls.SUPPORTED_FIELDS.keys()) - ) + # Extract any extra fields + extras = {k: v for k, v in param_dict.items() if k not in cls.SUPPORTED_FIELDS} + kwargs: dict[str, t.Any] = {"extra": extras} + # Ensure required fields are supplied + for field_name in cls.SUPPORTED_FIELDS.keys(): + kwargs[field_name] = param_dict.get(field_name) - # Enforce the field types - for field_name, field_type in cls.SUPPORTED_FIELDS.items(): - if field_name in param_dict and not isinstance( - param_dict[field_name], field_type - ): - raise ValueError( - f"'{field_name}' must be of type {field_type.__name__}" - ) - - return cls(**param_dict) + return cls(**kwargs) def to_dict(self, include_extra: bool = False) -> dict[str, t.Any]: """ @@ -137,17 +151,43 @@ class GlobusSessionError(GlobusError): :vartype extra_fields: dict """ + code: str + authorization_parameters: GlobusSessionErrorAuthorizationParameters + extra_fields: dict[str, t.Any] + + SUPPORTED_FIELDS = { + "code": str, + "authorization_parameters": GlobusSessionErrorAuthorizationParameters, + } + def __init__( self, - code: str, - authorization_parameters: GlobusSessionErrorAuthorizationParameters, - **kwargs: t.Any, + code: str | None, + authorization_parameters: dict[str, t.Any] + | GlobusSessionErrorAuthorizationParameters + | None, + extra: dict[str, t.Any] | None, ): - self.code: str = code - self.authorization_parameters: GlobusSessionErrorAuthorizationParameters = ( - authorization_parameters - ) - self.extra_fields = kwargs + if code is None: + raise ValueError("Must have a 'code'") + self.code = code + + self.extra_fields = extra or {} + + # Enforce that authorization_parameters is in the error_dict and + # contains at least one of the fields we expect + if isinstance( + authorization_parameters, GlobusSessionErrorAuthorizationParameters + ): + self.authorization_parameters = authorization_parameters + elif isinstance(authorization_parameters, dict): + self.authorization_parameters = ( + GlobusSessionErrorAuthorizationParameters.from_dict( + param_dict=authorization_parameters + ) + ) + else: + raise ValueError("Must have 'authorization_parameters'") @classmethod def from_dict(cls, error_dict: dict[str, t.Any]) -> GlobusSessionError: @@ -158,22 +198,12 @@ def from_dict(cls, error_dict: dict[str, t.Any]) -> GlobusSessionError: :type error_dict: dict """ - if "code" not in error_dict: - raise ValueError("Must have a 'code'") - - # Enforce that authorization_parameters is in the error_dict and - # contains at least one of the fields we expect - if "authorization_parameters" not in error_dict: - raise ValueError("Must have 'authorization_parameters'") - - kwargs = copy.deepcopy(error_dict) - - # Create our GlobusAuthorizationParameters object - kwargs[ - "authorization_parameters" - ] = GlobusSessionErrorAuthorizationParameters.from_dict( - param_dict=kwargs.pop("authorization_parameters") - ) + # Extract any extra fields + extras = {k: v for k, v in error_dict.items() if k not in cls.SUPPORTED_FIELDS} + kwargs: dict[str, t.Any] = {"extra": extras} + # Ensure required fields are supplied + for field_name in cls.SUPPORTED_FIELDS.keys(): + kwargs[field_name] = error_dict.get(field_name) return cls(**kwargs) From 587117aebe4a718a6d6ea28486d0c091cf6bfba9 Mon Sep 17 00:00:00 2001 From: Ada Date: Thu, 3 Aug 2023 16:49:16 -0700 Subject: [PATCH 10/32] Rename to Globus Auth Requirements Error Signed-off-by: Ada --- ...9_093104_ada_add_session_error_support.rst | 5 +- .../experimental/auth_requirements_errors.rst | 133 ++++++++++++++ docs/experimental/index.rst | 2 +- docs/experimental/session_errors.rst | 127 ------------- .../auth_requirements_error/__init__.py | 19 ++ .../_variants.py | 52 +++--- .../auth_requirements_error.py} | 58 +++--- .../utils.py | 65 ++++--- .../experimental/session_error/__init__.py | 16 -- ...ror.py => test_auth_requirements_error.py} | 173 +++++++++--------- 10 files changed, 335 insertions(+), 315 deletions(-) create mode 100644 docs/experimental/auth_requirements_errors.rst delete mode 100644 docs/experimental/session_errors.rst create mode 100644 src/globus_sdk/experimental/auth_requirements_error/__init__.py rename src/globus_sdk/experimental/{session_error => auth_requirements_error}/_variants.py (87%) rename src/globus_sdk/experimental/{session_error/session_error.py => auth_requirements_error/auth_requirements_error.py} (79%) rename src/globus_sdk/experimental/{session_error => auth_requirements_error}/utils.py (58%) delete mode 100644 src/globus_sdk/experimental/session_error/__init__.py rename tests/unit/experimental/{test_session_error.py => test_auth_requirements_error.py} (66%) diff --git a/changelog.d/20230629_093104_ada_add_session_error_support.rst b/changelog.d/20230629_093104_ada_add_session_error_support.rst index b30e1b705..c8f9b6dd4 100644 --- a/changelog.d/20230629_093104_ada_add_session_error_support.rst +++ b/changelog.d/20230629_093104_ada_add_session_error_support.rst @@ -1,2 +1,3 @@ -* Add a new class ``GlobusSessionError`` and utility functions - to support handling of the Globus Session Error response format (:pr:`NUMBER`) +* Add a new class ``GlobusAuthRequirementsError`` and utility functions to the + experimental package to support handling of the Globus Auth Requirements Error + response format (:pr:`NUMBER`) diff --git a/docs/experimental/auth_requirements_errors.rst b/docs/experimental/auth_requirements_errors.rst new file mode 100644 index 000000000..ee293cd25 --- /dev/null +++ b/docs/experimental/auth_requirements_errors.rst @@ -0,0 +1,133 @@ +Auth Requirements Errors +======================== + +Globus Auth Requirements Error is a response format that conveys to a client any +modifications to a session (i.e., "boosting") that will be required +to complete a particular request. + +The ``auth_requirements_error`` module provides a number of tools to make it easier to +identify and handle these errors when they occur. + +GlobusAuthRequirementsError +--------------------------- + +The ``GlobusAuthRequirementsError`` class provides a model for working with Globus +Auth Requirements Error responses in the Python SDK. The shape of an instance closely +matches that of the JSON response, such that in order to access a +response's session_required_scopes one could use, e.g.,: + +.. code-block:: python + + from globus_sdk.experimental import auth_requirements_error + + error = auth_requirements_error.GlobusAuthRequirementsError(response) + error.authorization_parameters.session_required_scopes + +``GlobusAuthRequirementsError`` enforces types strictly when parsing a Globus +Auth Requirements Error response dictionary, and will raise a ``ValueError`` if a +supported field is supplied with a value of the wrong type. +``GlobusAuthRequirementsError`` does not, however, attempt to mimic or itself enforce +any logic specific to the Globus Auth service with regard to what represents a coherent +combination of fields (e.g., that ``session_required_mfa`` requires either +``session_required_identities`` or ``session_required_single_domain`` +in order to be properly handled). + +If you are writing a service that needs to respond with a Globus Auth Requirements Error, you can +instantiate the ``GlobusAuthRequirementsError`` class directly to emit auth requirements errors +in your application, e.g.: + +.. code-block:: python + + from globus_sdk.experimental import auth_requirements_error + + error = auth_requirements_error.GlobusAuthRequirementsError( + code="ConsentRequired", + authorization_parameters=GlobusAuthorizationParameters( + session_required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], + session_message="Missing required 'foo' consent", + ), + ) + + # Render a strict dictionary + error.to_dict() + +If non-canonical fields are supplied on creation (either as keyword arguments +during instantiation or as fields in a dictionary supplied to ``from_dict()``), +you can preserve these fields in the rendered output dictionary +by specifying ``include_extra=True``. + +.. code-block:: python + + from globus_sdk.experimental import auth_requirements_error + + error = auth_requirements_error.GlobusAuthRequirementsError( + code="ConsentRequired", + message="Missing required 'foo' consent", + request_id="WmMV97A1w", + required_scopes=["urn:globus:auth:scope:transfer.api.globus.org:all[*foo]"], + resource="/transfer", + authorization_parameters=GlobusAuthorizationParameters( + session_required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], + session_message="Missing required 'foo' consent", + ), + ) + + # Render a dictionary with extra fields + error.to_dict(include_extra=True) + +These fields are stored by both the ``GlobusAuthRequirementsError`` and +``GlobusAuthenticationParameters`` classes in an ``extra_fields`` +attribute. + +Parsing Responses +----------------- + +If you are writing a client to a Globus API, the ``auth_requirements_error`` subpackage +provides utilities to detect legacy auth requirements error response +formats and normalize them. + +To detect if a ``GlobusAPIError``, ``ErrorSubdocument``, or JSON response +dictionary represents an error that can be converted to a Globus Auth +Requirements Error, you can use, e.g.,: + +.. code-block:: python + + from globus_sdk.experimental import auth_requirements_error + + error_dict = { + "code": "ConsentRequired", + "message": "Missing required foo consent", + } + # The dict is not a Globus Auth Requirements Error, so `False` is returned. + auth_requirements_error.utils.is_auth_requirements_error(error_dict) + + # The dict is not a Globus Auth Requirements Error and cannot be converted. + auth_requirements_error.utils.to_auth_requirements_error(error_dict) # None + + error_dict = { + "code": "ConsentRequired", + "message": "Missing required foo consent", + "required_scopes": ["urn:globus:auth:scope:transfer.api.globus.org:all[*foo]"], + } + auth_requirements_error.utils.is_auth_requirements_error(error_dict) # True + auth_requirements_error.utils.to_auth_requirements_error( + error_dict + ) # GlobusAuthRequirementsError + +.. note:: + + If a GlobusAPIError represents multiple errors that were returned in an + array, this only returns the first error in that array that can be + converted to the Globus Auth Requirements Error response format. In this case (and, + in general) it's preferable to use ``to_auth_requirements_errors()`` (which also + accepts a list of ``GlobusAPIError``\ s, ``ErrorSubdocument``\ s, and JSON + response dictionaries): + +.. code-block:: python + + auth_requirements_error.utils.to_auth_requirements_error( + other_error + ) # GlobusAuthRequirementsError + auth_requirements_error.utils.to_auth_requirements_errors( + [other_error] + ) # [GlobusAuthRequirementsError, ...] diff --git a/docs/experimental/index.rst b/docs/experimental/index.rst index c3e9b1674..4025d385f 100644 --- a/docs/experimental/index.rst +++ b/docs/experimental/index.rst @@ -16,5 +16,5 @@ Globus SDK Experimental Components :caption: Contents :maxdepth: 1 + auth_requirements_errors scope_parser - session_errors diff --git a/docs/experimental/session_errors.rst b/docs/experimental/session_errors.rst deleted file mode 100644 index a7a5591d9..000000000 --- a/docs/experimental/session_errors.rst +++ /dev/null @@ -1,127 +0,0 @@ -Session Errors -============== - -Globus Session Error is a response format that conveys to a client any -modifications to a session (i.e., "boosting") that will be required -to complete a particular request. - -The ``session_errors`` module provides a number of tools to make it easier to -identify and handle these errors when they occur. - -GlobusSessionError ------------------- - -The ``GlobusSessionError`` class provides a model for working with Globus -Session Error responses in the Python SDK. The shape of an instance closely -matches that of the JSON response, such that in order to access a -response's session_required_scopes one could use, e.g.,: - -.. code-block:: python - - from globus_sdk.experimental import session_errors - - error = session_errors.GlobusSessionError(response) - error.authorization_parameters.session_required_scopes - -``GlobusSessionError`` enforces types strictly when parsing a Globus Session -Error response dictionary, and will raise a ``ValueError`` if a supported -field is supplied with a value of the wrong type. ``GlobusSessionError`` does -not, however, attempt to mimic or itself enforce any logic specific to the -Globus Auth service with regard to what represents a coherent combination -of fields (e.g., that ``session_required_mfa`` requires either -``session_required_identities`` or ``session_required_single_domain`` -in order to be properly handled). - -If you are writing a service that needs to respond with a session error, you can -instantiate the ``GlobusSessionError`` class directly to emit session errors -in your application, e.g.: - -.. code-block:: python - - from globus_sdk.experimental import session_errors - - error = session_errors.GlobusSessionError( - code="ConsentRequired", - authorization_parameters=GlobusSessionErrorAuthorizationParameters( - session_required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], - session_message="Missing required 'foo' consent", - ), - ) - - # Render a strict dictionary - error.to_dict() - -If non-canonical fields are supplied on creation (either as keyword arguments -during instantiation or as fields in a dictionary supplied to ``from_dict()``), -you can preserve these fields in the rendered output dictionary -by specifying ``include_extra=True``. - -.. code-block:: python - - from globus_sdk.experimental import session_errors - - error = session_errors.GlobusSessionError( - code="ConsentRequired", - message="Missing required 'foo' consent", - request_id="WmMV97A1w", - required_scopes=["urn:globus:auth:scope:transfer.api.globus.org:all[*foo]"], - resource="/transfer", - authorization_parameters=GlobusSessionErrorAuthorizationParameters( - session_required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], - session_message="Missing required 'foo' consent", - ), - ) - - # Render a dictionary with extra fields - error.to_dict(include_extra=True) - -These fields are stored by both the ``GlobusSessionError`` and -``GlobusSessionErrorAuthenticationParameters`` classes in an ``extra_fields`` -attribute. - -Parsing Responses ------------------ - -If you are writing a client to a Globus API, the ``session_errors`` subpackage -provides utilities to detect legacy session error response -formats and normalize them. - -To detect if a ``GlobusAPIError``, ``ErrorSubdocument``, or JSON response -dictionary represents an error that can be converted to a Globus Session Error, -you can use, e.g.,: - -.. code-block:: python - - from globus_sdk.experimental import session_error - - error_dict = { - "code": "ConsentRequired", - "message": "Missing required foo consent", - } - # The dict is not a session error, so `False` is returned. - session_error.utils.is_session_error(error_dict) - - # The dict is not a session error and cannot be converted. - session_error.utils.to_session_error(error_dict) # None - - error_dict = { - "code": "ConsentRequired", - "message": "Missing required foo consent", - "required_scopes": ["urn:globus:auth:scope:transfer.api.globus.org:all[*foo]"], - } - session_error.utils.is_session_error(error_dict) # True - session_error.utils.to_session_error(error_dict) # GlobusSessionError - -.. note:: - - If a GlobusAPIError represents multiple errors that were returned in an - array, this only returns the first error in that array that can be - converted to the Globus Session Error response format. In this case (and, - in general) it's preferable to use ``to_session_errors()`` (which also - accepts a list of ``GlobusAPIError``s, ``ErrorSubdocument``s, and JSON - response dictionaries): - -.. code-block:: python - - session_error.utils.to_session_error(other_error) # GlobusSessionError - session_error.utils.to_session_errors([other_error]) # [GlobusSessionError, ...] diff --git a/src/globus_sdk/experimental/auth_requirements_error/__init__.py b/src/globus_sdk/experimental/auth_requirements_error/__init__.py new file mode 100644 index 000000000..7376a9b2d --- /dev/null +++ b/src/globus_sdk/experimental/auth_requirements_error/__init__.py @@ -0,0 +1,19 @@ +from .auth_requirements_error import ( + GlobusAuthorizationParameters, + GlobusAuthRequirementsError, +) +from .utils import ( + has_auth_requirements_errors, + is_auth_requirements_error, + to_auth_requirements_error, + to_auth_requirements_errors, +) + +__all__ = [ + "GlobusAuthRequirementsError", + "GlobusAuthorizationParameters", + "to_auth_requirements_error", + "to_auth_requirements_errors", + "is_auth_requirements_error", + "has_auth_requirements_errors", +] diff --git a/src/globus_sdk/experimental/session_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py similarity index 87% rename from src/globus_sdk/experimental/session_error/_variants.py rename to src/globus_sdk/experimental/auth_requirements_error/_variants.py index 759e6763f..151371d6c 100644 --- a/src/globus_sdk/experimental/session_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -2,25 +2,29 @@ import typing as t -from .session_error import GlobusSessionError, GlobusSessionErrorAuthorizationParameters +from .auth_requirements_error import ( + GlobusAuthorizationParameters, + GlobusAuthRequirementsError, +) -T = t.TypeVar("T", bound="LegacySessionErrorVariant") +T = t.TypeVar("T", bound="LegacyAuthRequirementsErrorVariant") -class LegacySessionErrorVariant: +class LegacyAuthRequirementsErrorVariant: """ - Abstract base class for errors which can be converted to a Globus Session Error. + Abstract base class for errors which can be converted to a + Globus Auth Requirements Error. """ @classmethod def from_dict(cls: t.Type[T], error_dict: dict[str, t.Any]) -> T: raise NotImplementedError() - def to_session_error(self) -> GlobusSessionError: + def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: raise NotImplementedError() -class LegacyConsentRequiredTransferError(LegacySessionErrorVariant): +class LegacyConsentRequiredTransferError(LegacyAuthRequirementsErrorVariant): """ The ConsentRequired error format emitted by the Globus Transfer service. """ @@ -52,13 +56,13 @@ def __init__( self.required_scopes = required_scopes self.extra_fields = extra or {} - def to_session_error(self) -> GlobusSessionError: + def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: """ - Return a GlobusSessionError representing this error. + Return a GlobusAuthRequirementsError representing this error. """ - return GlobusSessionError( + return GlobusAuthRequirementsError( code=self.code, - authorization_parameters=GlobusSessionErrorAuthorizationParameters( + authorization_parameters=GlobusAuthorizationParameters( session_required_scopes=self.required_scopes, session_message=self.extra_fields.get("message"), ), @@ -86,7 +90,7 @@ def from_dict( return cls(**kwargs) -class LegacyConsentRequiredAPError(LegacySessionErrorVariant): +class LegacyConsentRequiredAPError(LegacyAuthRequirementsErrorVariant): """ The ConsentRequired error format emitted by the legacy Globus Transfer Action Providers. @@ -119,16 +123,16 @@ def __init__( self.required_scope = required_scope self.extra_fields = extra or {} - def to_session_error(self) -> GlobusSessionError: + def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: """ - Return a GlobusSessionError representing this error. + Return a GlobusAuthRequirementsError representing this error. Normalizes the required_scope field to a list and uses the description to set the session message. """ - return GlobusSessionError( + return GlobusAuthRequirementsError( code=self.code, - authorization_parameters=GlobusSessionErrorAuthorizationParameters( + authorization_parameters=GlobusAuthorizationParameters( session_required_scopes=[self.required_scope], session_message=self.extra_fields.get("description"), extra=self.extra_fields.get("authorization_parameters"), @@ -222,11 +226,11 @@ def __init__( if field_value is not None and not isinstance(field_value, field_types): raise ValueError(f"'{field_name}' must be one of {field_types}") - def to_session_error_authorization_parameters( + def to_authorization_parameters( self, - ) -> GlobusSessionErrorAuthorizationParameters: + ) -> GlobusAuthorizationParameters: """ - Return a GlobusSessionError representing this error. + Return a GlobusAuthRequirementsError representing this error. Normalizes fields that may have been provided as comma-delimited strings to lists of strings. @@ -240,7 +244,7 @@ def to_session_error_authorization_parameters( if isinstance(required_single_domain, str): required_single_domain = required_single_domain.split(",") - return GlobusSessionErrorAuthorizationParameters( + return GlobusAuthorizationParameters( session_message=self.session_message, session_required_identities=self.session_required_identities, session_required_mfa=self.session_required_mfa, @@ -270,7 +274,7 @@ def from_dict(cls, param_dict: dict[str, t.Any]) -> LegacyAuthorizationParameter return cls(**kwargs) -class LegacyAuthorizationParametersError(LegacySessionErrorVariant): +class LegacyAuthorizationParametersError(LegacyAuthRequirementsErrorVariant): """ Defines an Authorization Parameters error that describes all known variants in use by Globus services. @@ -326,14 +330,14 @@ def from_dict( return cls(**kwargs) - def to_session_error(self) -> GlobusSessionError: + def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: """ - Return a GlobusSessionError representing this error. + Return a GlobusAuthRequirementsError representing this error. """ authorization_parameters = ( - self.authorization_parameters.to_session_error_authorization_parameters() + self.authorization_parameters.to_authorization_parameters() ) - return GlobusSessionError( + return GlobusAuthRequirementsError( authorization_parameters=authorization_parameters, code=self.code, extra=self.extra_fields, diff --git a/src/globus_sdk/experimental/session_error/session_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py similarity index 79% rename from src/globus_sdk/experimental/session_error/session_error.py rename to src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index c0e32d439..35a06551c 100644 --- a/src/globus_sdk/experimental/session_error/session_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -5,7 +5,7 @@ from globus_sdk.exc import GlobusError -class GlobusSessionErrorAuthorizationParameters: +class GlobusAuthorizationParameters: """ Represents authorization parameters that can be used to instruct a client which additional authorizations are needed in order to complete a request. @@ -90,12 +90,9 @@ def __init__( ) @classmethod - def from_dict( - cls, param_dict: dict[str, t.Any] - ) -> GlobusSessionErrorAuthorizationParameters: + def from_dict(cls, param_dict: dict[str, t.Any]) -> GlobusAuthorizationParameters: """ - Instantiate from a session error authorization parameters dictionary. Raises - a ValueError if the dictionary does not contain a valid GlobusSessionError. + Instantiate from an authorization parameters dictionary. :param param_dict: The dictionary to create the error from. :type param_dict: dict @@ -112,33 +109,34 @@ def from_dict( def to_dict(self, include_extra: bool = False) -> dict[str, t.Any]: """ - Return a session error authorization parameters dictionary. + Return an authorization parameters dictionary. :param include_extra: Whether to include stored extra (non-standard) fields in the returned dictionary. :type include_extra: bool """ - session_error_dict = {} + error_dict = {} # Set any authorization parameters for field in self.SUPPORTED_FIELDS.keys(): if getattr(self, field) is not None: - session_error_dict[field] = getattr(self, field) + error_dict[field] = getattr(self, field) # Set any extra fields if include_extra: - session_error_dict.update(self.extra_fields) + error_dict.update(self.extra_fields) - return session_error_dict + return error_dict -class GlobusSessionError(GlobusError): +class GlobusAuthRequirementsError(GlobusError): """ - Represents a Globus Session Error. + Represents a Globus Auth Requirements Error. - A Session Error is a class of error that is returned by Globus services to - indicate that additional authorization is required in order to complete a request - and contains information that can be used to request the appropriate authorization. + A Globus Auth Requirements Error is a class of error that is returned by Globus + services to indicate that additional authorization is required in order to complete + a request and contains information that can be used to request the appropriate + authorization. :ivar code: The error code for this error. :vartype code: str @@ -152,19 +150,19 @@ class GlobusSessionError(GlobusError): """ code: str - authorization_parameters: GlobusSessionErrorAuthorizationParameters + authorization_parameters: GlobusAuthorizationParameters extra_fields: dict[str, t.Any] SUPPORTED_FIELDS = { "code": str, - "authorization_parameters": GlobusSessionErrorAuthorizationParameters, + "authorization_parameters": GlobusAuthorizationParameters, } def __init__( self, code: str | None, authorization_parameters: dict[str, t.Any] - | GlobusSessionErrorAuthorizationParameters + | GlobusAuthorizationParameters | None, extra: dict[str, t.Any] | None, ): @@ -176,23 +174,19 @@ def __init__( # Enforce that authorization_parameters is in the error_dict and # contains at least one of the fields we expect - if isinstance( - authorization_parameters, GlobusSessionErrorAuthorizationParameters - ): + if isinstance(authorization_parameters, GlobusAuthorizationParameters): self.authorization_parameters = authorization_parameters elif isinstance(authorization_parameters, dict): - self.authorization_parameters = ( - GlobusSessionErrorAuthorizationParameters.from_dict( - param_dict=authorization_parameters - ) + self.authorization_parameters = GlobusAuthorizationParameters.from_dict( + param_dict=authorization_parameters ) else: raise ValueError("Must have 'authorization_parameters'") @classmethod - def from_dict(cls, error_dict: dict[str, t.Any]) -> GlobusSessionError: + def from_dict(cls, error_dict: dict[str, t.Any]) -> GlobusAuthRequirementsError: """ - Instantiate a GlobusSessionError from a dictionary. + Instantiate a GlobusAuthRequirementsError from a dictionary. :param error_dict: The dictionary to create the error from. :type error_dict: dict @@ -209,13 +203,13 @@ def from_dict(cls, error_dict: dict[str, t.Any]) -> GlobusSessionError: def to_dict(self, include_extra: bool = False) -> dict[str, t.Any]: """ - Return a session error response dictionary. + Return a Globus Auth Requirements Error response dictionary. :param include_extra: Whether to include stored extra (non-standard) fields in the dictionary. :type include_extra: bool, optional (default: False) """ - session_error_dict = { + error_dict = { "code": self.code, "authorization_parameters": self.authorization_parameters.to_dict( include_extra=include_extra @@ -224,6 +218,6 @@ def to_dict(self, include_extra: bool = False) -> dict[str, t.Any]: # Set any extra fields if include_extra: - session_error_dict.update(self.extra_fields) + error_dict.update(self.extra_fields) - return session_error_dict + return error_dict diff --git a/src/globus_sdk/experimental/session_error/utils.py b/src/globus_sdk/experimental/auth_requirements_error/utils.py similarity index 58% rename from src/globus_sdk/experimental/session_error/utils.py rename to src/globus_sdk/experimental/auth_requirements_error/utils.py index a56229ed4..6576a4ab9 100644 --- a/src/globus_sdk/experimental/session_error/utils.py +++ b/src/globus_sdk/experimental/auth_requirements_error/utils.py @@ -7,26 +7,28 @@ from ._variants import ( LegacyAuthorizationParametersError, + LegacyAuthRequirementsErrorVariant, LegacyConsentRequiredAPError, LegacyConsentRequiredTransferError, - LegacySessionErrorVariant, ) -from .session_error import GlobusSessionError +from .auth_requirements_error import GlobusAuthRequirementsError log = logging.getLogger(__name__) -def to_session_error( +def to_auth_requirements_error( error: GlobusAPIError | ErrorSubdocument | dict[str, t.Any] -) -> GlobusSessionError | None: +) -> GlobusAuthRequirementsError | None: """ - Converts a GlobusAPIError, ErrorSubdocument, or dict into a GlobusSessionError by - attempting to match to GlobusSessionError (preferred) or legacy variants. + Converts a GlobusAPIError, ErrorSubdocument, or dict into a + GlobusAuthRequirementsError by attempting to match to + GlobusAuthRequirementsError (preferred) or legacy variants. .. note:: a GlobusAPIError may contain multiple errors, and in this case only a single - session error is returned for the first error that matches a known format. + GlobusAuthRequirementsError is returned for the first error that matches + a known format. If the provided error does not match a known format, None is returned. @@ -40,33 +42,34 @@ def to_session_error( if isinstance(error, GlobusAPIError): # Iterate over ErrorSubdocuments for subdoc in error.errors: - session_error = to_session_error(subdoc) - if session_error is not None: - # Return only the first session error we encounter - return session_error - # We failed to find a session error + authreq_error = to_auth_requirements_error(subdoc) + if authreq_error is not None: + # Return only the first auth requirements error we encounter + return authreq_error + # We failed to find a Globus Auth Requirements Error return None elif isinstance(error, ErrorSubdocument): error_dict = error.raw else: error_dict = error - # Prefer a proper session error, if possible + # Prefer a proper auth requirements error, if possible try: - return GlobusSessionError.from_dict(error_dict) + return GlobusAuthRequirementsError.from_dict(error_dict) except ValueError as err: log.debug( - f"Failed to parse error as 'GlobusSessionError' because: {err.args[0]}" + "Failed to parse error as " + f"'GlobusAuthRequirementsError' because: {err.args[0]}" ) - supported_variants: list[t.Type[LegacySessionErrorVariant]] = [ + supported_variants: list[t.Type[LegacyAuthRequirementsErrorVariant]] = [ LegacyAuthorizationParametersError, LegacyConsentRequiredTransferError, LegacyConsentRequiredAPError, ] for variant in supported_variants: try: - return variant.from_dict(error_dict).to_session_error() + return variant.from_dict(error_dict).to_auth_requirements_error() except ValueError as err: log.debug( f"Failed to parse error as '{variant.__name__}' because: {err.args[0]}" @@ -75,13 +78,13 @@ def to_session_error( return None -def to_session_errors( +def to_auth_requirements_errors( errors: list[GlobusAPIError | ErrorSubdocument | dict[str, t.Any]] -) -> list[GlobusSessionError]: +) -> list[GlobusAuthRequirementsError]: """ Converts a list of GlobusAPIErrors, ErrorSubdocuments, or dicts into a list of - GlobusSessionErrors by attempting to match each error to - GlobusSessionError (preferred) or legacy variants. + GlobusAuthRequirementsErrors by attempting to match each error to + GlobusAuthRequirementsError (preferred) or legacy variants. .. note:: @@ -101,32 +104,34 @@ def to_session_errors( else: candidate_errors.append(error) - # Try to convert all candidate errors to session errors - all_errors = [to_session_error(error) for error in candidate_errors] + # Try to convert all candidate errors to auth requirements errors + all_errors = [to_auth_requirements_error(error) for error in candidate_errors] - # Remove any errors that did not resolve to a session error + # Remove any errors that did not resolve to a Globus Auth Requirements Error return [error for error in all_errors if error is not None] -def is_session_error( +def is_auth_requirements_error( error: GlobusAPIError | ErrorSubdocument | dict[str, t.Any] ) -> bool: """ - Return True if the provided error matches a known session error format. + Return True if the provided error matches a known + Globus Auth Requirements Error format. :param error: The error to check. :type error: a GlobusAPIError, ErrorSubdocument, or dict """ - return to_session_error(error) is not None + return to_auth_requirements_error(error) is not None -def has_session_errors( +def has_auth_requirements_errors( errors: list[GlobusAPIError | ErrorSubdocument | dict[str, t.Any]] ) -> bool: """ - Return True if any of the provided errors match a known session error format. + Return True if any of the provided errors match a known + Globus Auth Requirements Error format. :param errors: The errors to check. :type errors: a list of GlobusAPIErrors, ErrorSubdocuments, or dicts """ - return any(is_session_error(error) for error in errors) + return any(is_auth_requirements_error(error) for error in errors) diff --git a/src/globus_sdk/experimental/session_error/__init__.py b/src/globus_sdk/experimental/session_error/__init__.py deleted file mode 100644 index 6053eb50f..000000000 --- a/src/globus_sdk/experimental/session_error/__init__.py +++ /dev/null @@ -1,16 +0,0 @@ -from .session_error import GlobusSessionError, GlobusSessionErrorAuthorizationParameters -from .utils import ( - has_session_errors, - is_session_error, - to_session_error, - to_session_errors, -) - -__all__ = [ - "GlobusSessionError", - "GlobusSessionErrorAuthorizationParameters", - "to_session_error", - "to_session_errors", - "is_session_error", - "has_session_errors", -] diff --git a/tests/unit/experimental/test_session_error.py b/tests/unit/experimental/test_auth_requirements_error.py similarity index 66% rename from tests/unit/experimental/test_session_error.py rename to tests/unit/experimental/test_auth_requirements_error.py index 1e9c17355..b9a071d3d 100644 --- a/tests/unit/experimental/test_session_error.py +++ b/tests/unit/experimental/test_auth_requirements_error.py @@ -2,12 +2,12 @@ from globus_sdk._testing import construct_error from globus_sdk.exc import ErrorSubdocument -from globus_sdk.experimental.session_error import ( - GlobusSessionError, - has_session_errors, - is_session_error, - to_session_error, - to_session_errors, +from globus_sdk.experimental.auth_requirements_error import ( + GlobusAuthRequirementsError, + has_auth_requirements_errors, + is_auth_requirements_error, + to_auth_requirements_error, + to_auth_requirements_errors, ) @@ -38,10 +38,10 @@ ), ), ) -def test_create_session_error_from_consent_error(error_dict, status): +def test_create_auth_requirements_error_from_consent_error(error_dict, status): """ Test that various ConsentRequired error shapes can be detected and converted - to a GlobusSessionError. + to a GlobusAuthRequirementsError. """ # Create various supplementary objects representing this error error_subdoc = ErrorSubdocument(error_dict) @@ -49,21 +49,21 @@ def test_create_session_error_from_consent_error(error_dict, status): for error in (error_dict, error_subdoc, api_error): # Test boolean utility functions - assert is_session_error(error) - assert has_session_errors([error]) + assert is_auth_requirements_error(error) + assert has_auth_requirements_errors([error]) # Check that this only produces one error - assert len(to_session_errors([error])) == 1 + assert len(to_auth_requirements_errors([error])) == 1 - # Create a session error from a Transfer format error - session_error = to_session_error(error) - assert isinstance(session_error, GlobusSessionError) - assert session_error.code == "ConsentRequired" - assert session_error.authorization_parameters.session_required_scopes == [ + # Create a Globus Auth requirements error from a Transfer format error + authreq_error = to_auth_requirements_error(error) + assert isinstance(authreq_error, GlobusAuthRequirementsError) + assert authreq_error.code == "ConsentRequired" + assert authreq_error.authorization_parameters.session_required_scopes == [ "urn:globus:auth:scope:transfer.api.globus.org:all[*foo *bar]" ] assert ( - session_error.authorization_parameters.session_message + authreq_error.authorization_parameters.session_message == "Missing required foo_bar consent" ) @@ -94,10 +94,12 @@ def test_create_session_error_from_consent_error(error_dict, status): }, ), ) -def test_create_session_error_from_authorization_error(authorization_parameters): +def test_create_auth_requirements_error_from_authorization_error( + authorization_parameters, +): """ - Test that various AuthorizationRequired error shapes can be detected and converted - to a GlobusSessionError. + Test that various authorization parameters error shapes can be detected and + converted to a GlobusAuthRequirementsError. """ # Create various supplementary objects representing this error error_dict = {"authorization_parameters": authorization_parameters} @@ -106,22 +108,23 @@ def test_create_session_error_from_authorization_error(authorization_parameters) for error in (error_dict, error_subdoc, api_error): # Test boolean utility functions - assert is_session_error(error) - assert has_session_errors([error]) + assert is_auth_requirements_error(error) + assert has_auth_requirements_errors([error]) # Check that this only produces one error - assert len(to_session_errors([error])) == 1 + assert len(to_auth_requirements_errors([error])) == 1 - # Create a session error from a legacy authorization parameters format error - session_error = to_session_error(error) - assert isinstance(session_error, GlobusSessionError) + # Create a Globus Auth requirements error from a legacy + # authorization parameters format error + authreq_error = to_auth_requirements_error(error) + assert isinstance(authreq_error, GlobusAuthRequirementsError) # Check that the default error code is set - assert session_error.code == "AuthorizationRequired" + assert authreq_error.code == "AuthorizationRequired" # Iterate over the expected attributes and check that they match for name, value in authorization_parameters.items(): - assert getattr(session_error.authorization_parameters, name) == value + assert getattr(authreq_error.authorization_parameters, name) == value @pytest.mark.parametrize( @@ -143,11 +146,13 @@ def test_create_session_error_from_authorization_error(authorization_parameters) }, ), ) -def test_create_session_error_from_authorization_error_csv(authorization_parameters): +def test_create_auth_requirements_error_from_authorization_error_csv( + authorization_parameters, +): """ - Test that AuthorizationRequired error shapes that provide lists as comma-delimited - values can be detected and converted to a GlobusSessionError normalizing to - lists of strings for those values. + Test that authorization parameters error shapes that provide lists as comma- + delimited values can be detected and converted to a GlobusAuthRequirementsError + normalizing to lists of strings for those values. """ # Create various supplementary objects representing this error error_dict = {"authorization_parameters": {}} @@ -163,29 +168,30 @@ def test_create_session_error_from_authorization_error_csv(authorization_paramet for error in (error_dict, error_subdoc, api_error): # Test boolean utility functions - assert is_session_error(error) - assert has_session_errors([error]) + assert is_auth_requirements_error(error) + assert has_auth_requirements_errors([error]) # Check that this only produces one error - assert len(to_session_errors([error])) == 1 + assert len(to_auth_requirements_errors([error])) == 1 - # Create a session error from a legacy authorization parameters format error - session_error = to_session_error(error) - assert isinstance(session_error, GlobusSessionError) + # Create a Globus Auth requirements error from a legacy + # authorization parameters format error + authreq_error = to_auth_requirements_error(error) + assert isinstance(authreq_error, GlobusAuthRequirementsError) # Check that the default error code is set - assert session_error.code == "AuthorizationRequired" + assert authreq_error.code == "AuthorizationRequired" # Iterate over the expected attributes and check that they match for name, value in authorization_parameters.items(): - assert getattr(session_error.authorization_parameters, name) == value + assert getattr(authreq_error.authorization_parameters, name) == value -def test_create_session_errors_from_multiple_errors(): +def test_create_auth_requirements_errors_from_multiple_errors(): """ Test that a GlobusAPIError with multiple subdocuments is converted to multiple - GlobusSessionErrors, and additionally test that this is correct even when mingled - with other accepted data types. + GlobusAuthRequirementsErrors, and additionally test that this is correct even + when mingled with other accepted data types. """ consent_errors = construct_error( body={ @@ -239,48 +245,48 @@ def test_create_session_errors_from_multiple_errors(): all_errors = [consent_errors, not_an_error, authorization_error] # Test boolean utility function - assert has_session_errors(all_errors) + assert has_auth_requirements_errors(all_errors) - # Create session errors from a all errors - session_errors = to_session_errors(all_errors) - assert isinstance(session_errors, list) - assert len(session_errors) == 3 + # Create auth requirements errors from a all errors + authreq_errors = to_auth_requirements_errors(all_errors) + assert isinstance(authreq_errors, list) + assert len(authreq_errors) == 3 # Check that errors properly converted - for session_error in session_errors: - assert isinstance(session_error, GlobusSessionError) + for authreq_error in authreq_errors: + assert isinstance(authreq_error, GlobusAuthRequirementsError) - # Check that the proper session errors were produced - assert session_errors[0].code == "ConsentRequired" - assert session_errors[0].authorization_parameters.session_required_scopes == [ + # Check that the proper auth requirements errors were produced + assert authreq_errors[0].code == "ConsentRequired" + assert authreq_errors[0].authorization_parameters.session_required_scopes == [ "urn:globus:auth:scope:transfer.api.globus.org:all[*bar]" ] assert ( - session_errors[0].authorization_parameters.session_message + authreq_errors[0].authorization_parameters.session_message == "Missing required foo_bar consent" ) - assert session_errors[1].code == "ConsentRequired" - assert session_errors[1].authorization_parameters.session_required_scopes == [ + assert authreq_errors[1].code == "ConsentRequired" + assert authreq_errors[1].authorization_parameters.session_required_scopes == [ "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" ] assert ( - session_errors[1].authorization_parameters.session_message + authreq_errors[1].authorization_parameters.session_message == "Missing required foo_baz consent" ) - assert session_errors[2].code == "AuthorizationRequired" - assert session_errors[2].authorization_parameters.session_required_policies == [ + assert authreq_errors[2].code == "AuthorizationRequired" + assert authreq_errors[2].authorization_parameters.session_required_policies == [ "foo", "baz", ] - assert session_errors[2].authorization_parameters.session_message == ( + assert authreq_errors[2].authorization_parameters.session_message == ( "You need to authenticate with an identity that matches the required policies" ) -def test_create_session_error_from_legacy_authorization_error_with_code(): +def test_create_auth_requirements_error_from_legacy_authorization_error_with_code(): """ - Test that legacy AuthorizationRequired error shapes that provide a `code` can be - detected and converted to a GlobusSessionError while retaining the `code`. + Test that legacy authorization parameters error shapes that provide a `code` can be + detected and converted to a GlobusAuthRequirementsError while retaining the `code`. """ # Create a legacy authorization parameters error with a code error_dict = { @@ -300,21 +306,22 @@ def test_create_session_error_from_legacy_authorization_error_with_code(): for error in (error_dict, error_subdoc, api_error): # Test boolean utility functions - assert is_session_error(error) - assert has_session_errors([error]) + assert is_auth_requirements_error(error) + assert has_auth_requirements_errors([error]) # Check that this only produces one error - assert len(to_session_errors([error])) == 1 + assert len(to_auth_requirements_errors([error])) == 1 - # Create a session error from a legacy authorization parameters format error - session_error = to_session_error(error) - assert isinstance(session_error, GlobusSessionError) + # Create a Globus Auth requirements error from a legacy + # authorization parameters format error + authreq_error = to_auth_requirements_error(error) + assert isinstance(authreq_error, GlobusAuthRequirementsError) # Check that the custom error code is set - assert session_error.code == "UnsatisfiedPolicy" + assert authreq_error.code == "UnsatisfiedPolicy" # Iterate over the expected attributes and check that they match - assert session_error.authorization_parameters.session_required_policies == [ + assert authreq_error.authorization_parameters.session_required_policies == [ "foo", "baz", ] @@ -323,7 +330,7 @@ def test_create_session_error_from_legacy_authorization_error_with_code(): def test_backward_compatibility_consent_required_error(): """ Test that a consent required error with a comingled backward-compatible - data schema is converted to a GlobusSessionError. + data schema is converted to a GlobusAuthRequirementsError. """ # Create an API error with a backward compatible data schema using # distinct values for duplicative fields to facilitate testing @@ -350,25 +357,25 @@ def test_backward_compatibility_consent_required_error(): ) # Test boolean utility functions - assert is_session_error(error) - assert has_session_errors([error]) + assert is_auth_requirements_error(error) + assert has_auth_requirements_errors([error]) # Check that this only produces one error - assert len(to_session_errors([error])) == 1 + assert len(to_auth_requirements_errors([error])) == 1 - # Create a session error - session_error = to_session_error(error) - assert isinstance(session_error, GlobusSessionError) - assert session_error.code == "ConsentRequired" - assert session_error.authorization_parameters.session_required_scopes == [ + # Create a Globus Auth requirements error + authreq_error = to_auth_requirements_error(error) + assert isinstance(authreq_error, GlobusAuthRequirementsError) + assert authreq_error.code == "ConsentRequired" + assert authreq_error.authorization_parameters.session_required_scopes == [ "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" ] assert ( - session_error.authorization_parameters.session_message == "Missing baz consent" + authreq_error.authorization_parameters.session_message == "Missing baz consent" ) # Test that only suppotred fields are present in the dict - assert session_error.to_dict() == { + assert authreq_error.to_dict() == { "code": "ConsentRequired", "authorization_parameters": { "session_message": "Missing baz consent", @@ -380,7 +387,7 @@ def test_backward_compatibility_consent_required_error(): } # Test that extra fields are present in the dict - assert session_error.to_dict(include_extra=True) == { + assert authreq_error.to_dict(include_extra=True) == { "code": "ConsentRequired", "message": "Missing required foo_bar consent", "request_id": "WmMV97A1w", From ceb5aea227a3fffaf51265cf568f9bd63d3923eb Mon Sep 17 00:00:00 2001 From: Ada Date: Tue, 11 Jul 2023 11:51:27 -0700 Subject: [PATCH 11/32] Move `.from_dict()` implementation to shared base Signed-off-by: Ada --- .../auth_requirements_error/_variants.py | 76 +++++-------------- 1 file changed, 17 insertions(+), 59 deletions(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index 151371d6c..1b57d3fdb 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -16,9 +16,24 @@ class LegacyAuthRequirementsErrorVariant: Globus Auth Requirements Error. """ + SUPPORTED_FIELDS: dict[str, t.Tuple[t.Type[t.Any], ...]] = {} + @classmethod def from_dict(cls: t.Type[T], error_dict: dict[str, t.Any]) -> T: - raise NotImplementedError() + """ + Instantiate from an error dictionary. + + :param error_dict: The dictionary to instantiate the error from. + :type error_dict: dict + """ + # Extract any extra fields + extras = {k: v for k, v in error_dict.items() if k not in cls.SUPPORTED_FIELDS} + kwargs: dict[str, t.Any] = {"extra": extras} + # Ensure required fields are supplied + for field_name in cls.SUPPORTED_FIELDS.keys(): + kwargs[field_name] = error_dict.get(field_name) + + return cls(**kwargs) def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: raise NotImplementedError() @@ -69,26 +84,6 @@ def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: extra=self.extra_fields, ) - @classmethod - def from_dict( - cls, error_dict: dict[str, t.Any] - ) -> LegacyConsentRequiredTransferError: - """ - Instantiate from an error dictionary. Raises a ValueError if the dictionary - does not contain a recognized LegacyConsentRequiredTransferError. - - :param error_dict: The dictionary to instantiate the error from. - :type error_dict: dict - """ - # Extract any extra fields - extras = {k: v for k, v in error_dict.items() if k not in cls.SUPPORTED_FIELDS} - kwargs: dict[str, t.Any] = {"extra": extras} - # Ensure required fields are supplied - for field_name in cls.SUPPORTED_FIELDS.keys(): - kwargs[field_name] = error_dict.get(field_name) - - return cls(**kwargs) - class LegacyConsentRequiredAPError(LegacyAuthRequirementsErrorVariant): """ @@ -144,25 +139,6 @@ def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: }, ) - @classmethod - def from_dict(cls, error_dict: dict[str, t.Any]) -> LegacyConsentRequiredAPError: - """ - Instantiate from an error dictionary. Raises a ValueError if the dictionary - does not contain a recognized LegacyConsentRequiredAPError. - - :param error_dict: The dictionary to create the error from. - :type error_dict: dict - """ - - # Extract any extra fields - extras = {k: v for k, v in error_dict.items() if k not in cls.SUPPORTED_FIELDS} - kwargs: dict[str, t.Any] = {"extra": extras} - # Ensure required fields are supplied - for field_name in cls.SUPPORTED_FIELDS.keys(): - kwargs[field_name] = error_dict.get(field_name) - - return cls(**kwargs) - class LegacyAuthorizationParameters: """ @@ -257,8 +233,7 @@ def to_authorization_parameters( @classmethod def from_dict(cls, param_dict: dict[str, t.Any]) -> LegacyAuthorizationParameters: """ - Create from a dictionary. Raises a ValueError if the dictionary does not contain - a recognized LegacyAuthorizationParameters format. + Instantiate from an authorization_parameters dictionary. :param param_dict: The dictionary to create the AuthorizationParameters from. :type param_dict: dict @@ -313,23 +288,6 @@ def __init__( # Retain any additional fields self.extra_fields = extra or {} - @classmethod - def from_dict( - cls, error_dict: dict[str, t.Any] - ) -> LegacyAuthorizationParametersError: - """ - Instantiate a LegacyAuthorizationParametersError from a dictionary. - """ - - # Extract any extra fields - extras = {k: v for k, v in error_dict.items() if k not in cls.SUPPORTED_FIELDS} - kwargs: dict[str, t.Any] = {"extra": extras} - # Ensure required fields are supplied - for field_name in cls.SUPPORTED_FIELDS.keys(): - kwargs[field_name] = error_dict.get(field_name) - - return cls(**kwargs) - def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: """ Return a GlobusAuthRequirementsError representing this error. From 41263d7916a85cfa0be302f6804620d1a88a051c Mon Sep 17 00:00:00 2001 From: Ada Date: Tue, 11 Jul 2023 14:10:36 -0700 Subject: [PATCH 12/32] Adopt validators to improve strictness Signed-off-by: Ada --- .../auth_requirements_error/_variants.py | 94 ++++++++-------- .../auth_requirements_error.py | 63 +++++------ .../auth_requirements_error/validators.py | 103 ++++++++++++++++++ .../test_auth_requirements_error.py | 26 +++++ 4 files changed, 199 insertions(+), 87 deletions(-) create mode 100644 src/globus_sdk/experimental/auth_requirements_error/validators.py diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index 1b57d3fdb..a83818c34 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -2,6 +2,7 @@ import typing as t +from . import validators from .auth_requirements_error import ( GlobusAuthorizationParameters, GlobusAuthRequirementsError, @@ -16,7 +17,7 @@ class LegacyAuthRequirementsErrorVariant: Globus Auth Requirements Error. """ - SUPPORTED_FIELDS: dict[str, t.Tuple[t.Type[t.Any], ...]] = {} + SUPPORTED_FIELDS: dict[str, t.Callable[[t.Any], t.Any]] = {} @classmethod def from_dict(cls: t.Type[T], error_dict: dict[str, t.Any]) -> T: @@ -49,8 +50,8 @@ class LegacyConsentRequiredTransferError(LegacyAuthRequirementsErrorVariant): extra_fields: dict[str, t.Any] SUPPORTED_FIELDS = { - "code": (str,), - "required_scopes": (list,), + "code": validators.StringLiteral("ConsentRequired"), + "required_scopes": validators.ListOfStrings, } def __init__( @@ -59,16 +60,11 @@ def __init__( required_scopes: list[str] | None, extra: dict[str, t.Any] | None = None, ): - # Needed to make mypy happy w/ stricter types on instance vars. - # Will clean up in a subsequent commit - if not isinstance(code, str) or code != "ConsentRequired": - raise ValueError("'code' must be the string 'ConsentRequired'") + # Validate and assign supported fields + for field_name, validator in self.SUPPORTED_FIELDS.items(): + field_value = validator(locals()[field_name]) + setattr(self, field_name, field_value) - if not isinstance(required_scopes, list): - raise ValueError("'required_scopes' must be a list") - - self.code = code - self.required_scopes = required_scopes self.extra_fields = extra or {} def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: @@ -96,8 +92,8 @@ class LegacyConsentRequiredAPError(LegacyAuthRequirementsErrorVariant): extra_fields: dict[str, t.Any] SUPPORTED_FIELDS = { - "code": (str,), - "required_scope": (str,), + "code": validators.StringLiteral("ConsentRequired"), + "required_scope": validators.String, } def __init__( @@ -106,16 +102,11 @@ def __init__( required_scope: str | None, extra: dict[str, t.Any] | None, ): - # Needed to make mypy happy w/ stricter types on instance vars. - # Will clean up in a subsequent commit - if not isinstance(code, str) or code != "ConsentRequired": - raise ValueError("'code' must be the string 'ConsentRequired'") - - if not isinstance(required_scope, str): - raise ValueError("'required_scope' must be a list") + # Validate and assign supported fields + for field_name, validator in self.SUPPORTED_FIELDS.items(): + field_value = validator(locals()[field_name]) + setattr(self, field_name, field_value) - self.code = code - self.required_scope = required_scope self.extra_fields = extra or {} def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: @@ -158,12 +149,16 @@ class LegacyAuthorizationParameters: DEFAULT_CODE = "AuthorizationRequired" SUPPORTED_FIELDS = { - "session_message": (str,), - "session_required_identities": (list,), - "session_required_policies": (list, str), - "session_required_single_domain": (list, str), - "session_required_mfa": (bool,), - "session_required_scopes": (list,), + "session_message": validators.OptionalString, + "session_required_identities": validators.OptionalListOfStrings, + "session_required_policies": ( + validators.OptionalListOfStringsOrCommaDelimitedStrings + ), + "session_required_single_domain": ( + validators.OptionalListOfStringsOrCommaDelimitedStrings + ), + "session_required_mfa": validators.OptionalBool, + "session_required_scopes": validators.OptionalListOfStrings, } def __init__( @@ -176,13 +171,11 @@ def __init__( session_required_scopes: list[str] | None = None, extra: dict[str, t.Any] | None = None, ): - self.session_message = session_message - self.session_required_identities = session_required_identities - self.session_required_policies = session_required_policies - self.session_required_single_domain = session_required_single_domain - self.session_required_mfa = session_required_mfa - # Declared here for compatibility with mixed legacy payloads - self.session_required_scopes = session_required_scopes + # Validate and assign supported fields + for field_name, validator in self.SUPPORTED_FIELDS.items(): + field_value = validator(locals()[field_name]) + setattr(self, field_name, field_value) + # Retain any additional fields self.extra_fields = extra or {} @@ -196,12 +189,6 @@ def __init__( ", ".join(self.SUPPORTED_FIELDS.keys()) ) - # Enforce the field types - for field_name, field_types in self.SUPPORTED_FIELDS.items(): - field_value = getattr(self, field_name) - if field_value is not None and not isinstance(field_value, field_types): - raise ValueError(f"'{field_name}' must be one of {field_types}") - def to_authorization_parameters( self, ) -> GlobusAuthorizationParameters: @@ -262,8 +249,10 @@ class LegacyAuthorizationParametersError(LegacyAuthRequirementsErrorVariant): DEFAULT_CODE = "AuthorizationRequired" SUPPORTED_FIELDS = { - "code": (str,), - "authorization_parameters": (LegacyAuthorizationParameters,), + "code": validators.String, + "authorization_parameters": validators.ClassInstance( + LegacyAuthorizationParameters + ), } def __init__( @@ -274,16 +263,19 @@ def __init__( code: str | None = None, extra: dict[str, t.Any] | None = None, ): - self.code = code or self.DEFAULT_CODE + # Apply default, if necessary + code = code or self.DEFAULT_CODE - if isinstance(authorization_parameters, LegacyAuthorizationParameters): - self.authorization_parameters = authorization_parameters - elif isinstance(authorization_parameters, dict): - self.authorization_parameters = LegacyAuthorizationParameters.from_dict( + # Convert authorization_parameters if it's a dict + if isinstance(authorization_parameters, dict): + authorization_parameters = LegacyAuthorizationParameters.from_dict( param_dict=authorization_parameters ) - else: - raise ValueError("Must have 'authorization_parameters'") + + # Validate and assign supported fields + for field_name, validator in self.SUPPORTED_FIELDS.items(): + field_value = validator(locals()[field_name]) + setattr(self, field_name, field_value) # Retain any additional fields self.extra_fields = extra or {} diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index 35a06551c..b96cf970d 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -4,6 +4,8 @@ from globus_sdk.exc import GlobusError +from . import validators + class GlobusAuthorizationParameters: """ @@ -45,12 +47,12 @@ class GlobusAuthorizationParameters: extra_fields: dict[str, t.Any] SUPPORTED_FIELDS = { - "session_message": str, - "session_required_identities": list, - "session_required_policies": list, - "session_required_single_domain": list, - "session_required_mfa": bool, - "session_required_scopes": list, + "session_message": validators.OptionalString, + "session_required_identities": validators.OptionalListOfStrings, + "session_required_policies": validators.OptionalListOfStrings, + "session_required_single_domain": validators.OptionalListOfStrings, + "session_required_mfa": validators.OptionalBool, + "session_required_scopes": validators.OptionalListOfStrings, } def __init__( @@ -63,12 +65,11 @@ def __init__( session_required_scopes: list[str] | None = None, extra: dict[str, t.Any] | None = None, ): - self.session_message = session_message - self.session_required_identities = session_required_identities - self.session_required_policies = session_required_policies - self.session_required_single_domain = session_required_single_domain - self.session_required_mfa = session_required_mfa - self.session_required_scopes = session_required_scopes + # Validate and assign supported fields + for field_name, validator in self.SUPPORTED_FIELDS.items(): + field_value = validator(locals()[field_name]) + setattr(self, field_name, field_value) + self.extra_fields = extra or {} # Enforce that the error contains at least one of the fields we expect @@ -81,14 +82,6 @@ def __init__( ", ".join(self.SUPPORTED_FIELDS.keys()) ) - # Enforce the field types - for field_name, field_type in self.SUPPORTED_FIELDS.items(): - field_value = getattr(self, field_name) - if field_value is not None and not isinstance(field_value, field_type): - raise ValueError( - f"'{field_name}' must be of type {field_type.__name__}" - ) - @classmethod def from_dict(cls, param_dict: dict[str, t.Any]) -> GlobusAuthorizationParameters: """ @@ -154,8 +147,10 @@ class GlobusAuthRequirementsError(GlobusError): extra_fields: dict[str, t.Any] SUPPORTED_FIELDS = { - "code": str, - "authorization_parameters": GlobusAuthorizationParameters, + "code": validators.String, + "authorization_parameters": validators.ClassInstance( + GlobusAuthorizationParameters + ), } def __init__( @@ -166,22 +161,18 @@ def __init__( | None, extra: dict[str, t.Any] | None, ): - if code is None: - raise ValueError("Must have a 'code'") - self.code = code - - self.extra_fields = extra or {} - - # Enforce that authorization_parameters is in the error_dict and - # contains at least one of the fields we expect - if isinstance(authorization_parameters, GlobusAuthorizationParameters): - self.authorization_parameters = authorization_parameters - elif isinstance(authorization_parameters, dict): - self.authorization_parameters = GlobusAuthorizationParameters.from_dict( + # Convert authorization_parameters if it's a dict + if isinstance(authorization_parameters, dict): + authorization_parameters = GlobusAuthorizationParameters.from_dict( param_dict=authorization_parameters ) - else: - raise ValueError("Must have 'authorization_parameters'") + + # Validate and assign supported fields + for field_name, validator in self.SUPPORTED_FIELDS.items(): + field_value = validator(locals()[field_name]) + setattr(self, field_name, field_value) + + self.extra_fields = extra or {} @classmethod def from_dict(cls, error_dict: dict[str, t.Any]) -> GlobusAuthRequirementsError: diff --git a/src/globus_sdk/experimental/auth_requirements_error/validators.py b/src/globus_sdk/experimental/auth_requirements_error/validators.py new file mode 100644 index 000000000..3e824d363 --- /dev/null +++ b/src/globus_sdk/experimental/auth_requirements_error/validators.py @@ -0,0 +1,103 @@ +from __future__ import annotations + +import typing as t +from contextlib import suppress + +T = t.TypeVar("T") + + +def _string(value: t.Any) -> str: + if not isinstance(value, str): + raise ValueError("Must be a string") + + return value + + +def _string_literal(literal: str) -> t.Callable[[t.Any], str]: + def validator(value: t.Any) -> str: + value = _string(value) + if value != literal: + raise ValueError(f"Must be '{literal}'") + + return t.cast(str, value) + + return validator + + +def _class_instance(cls: t.Type[T]) -> t.Callable[[t.Any], T]: + def validator(value: t.Any) -> T: + if not isinstance(value, cls): + raise ValueError(f"Must be an instance of {cls.__name__}") + + return value + + return validator + + +def _list_of_strings(value: t.Any) -> list[str]: + if not (isinstance(value, list) and all(isinstance(v, str) for v in value)): + raise ValueError("Must be a list of strings") + + return value + + +def _comma_delimited_strings(value: t.Any) -> list[str]: + if not isinstance(value, str): + raise ValueError("Must be a comma-delimited string") + + return value.split(",") + + +def _boolean(value: t.Any) -> bool: + if not isinstance(value, bool): + raise ValueError("Must be a bool") + + return value + + +def _null(value: t.Any) -> None: + if value is not None: + raise ValueError("Must be null") + + return None + + +def _anyof( + validators: tuple[t.Callable[..., t.Any], ...], + description: str, +) -> t.Callable[..., t.Any]: + def _anyof_validator(value: t.Any) -> t.Any: + for validator in validators: + with suppress(ValueError): + return validator(value) + + raise ValueError(f"Must be {description}") + + return _anyof_validator + + +String: t.Callable[[t.Any], str] = _string +StringLiteral: t.Callable[[str], t.Callable[[t.Any], str]] = _string_literal +ClassInstance: t.Callable[[t.Any], t.Any] = _class_instance +ListOfStrings: t.Callable[[t.Any], list[str]] = _list_of_strings +CommaDelimitedStrings: t.Callable[[t.Any], list[str]] = _comma_delimited_strings +Bool: t.Callable[[t.Any], bool] = _boolean +Null: t.Callable[[t.Any], None] = _null +OptionalString: t.Callable[[t.Any], str | None] = _anyof( + (_string, _null), description="a string or null" +) +OptionalListOfStrings: t.Callable[[t.Any], list[str] | None] = _anyof( + (_list_of_strings, _null), description="a list of strings or null" +) +OptionalCommaDelimitedStrings: t.Callable[[t.Any], list[str] | None] = _anyof( + (_comma_delimited_strings, _null), description="a comma-delimited string or null" +) +OptionalListOfStringsOrCommaDelimitedStrings: t.Callable[ + [t.Any], list[str] | None +] = _anyof( + (_list_of_strings, _comma_delimited_strings, _null), + description="a list of strings, a comma-delimited string, or null", +) +OptionalBool: t.Callable[[t.Any], bool | None] = _anyof( + (_boolean, _null), description="a bool or null" +) diff --git a/tests/unit/experimental/test_auth_requirements_error.py b/tests/unit/experimental/test_auth_requirements_error.py index b9a071d3d..e8925c5cf 100644 --- a/tests/unit/experimental/test_auth_requirements_error.py +++ b/tests/unit/experimental/test_auth_requirements_error.py @@ -1,9 +1,13 @@ +import inspect + import pytest from globus_sdk._testing import construct_error from globus_sdk.exc import ErrorSubdocument from globus_sdk.experimental.auth_requirements_error import ( + GlobusAuthorizationParameters, GlobusAuthRequirementsError, + _variants, has_auth_requirements_errors, is_auth_requirements_error, to_auth_requirements_error, @@ -404,3 +408,25 @@ def test_backward_compatibility_consent_required_error(): "optional": "A non-canonical field", }, } + + +@pytest.mark.parametrize( + "target_class", + [ + GlobusAuthRequirementsError, + GlobusAuthorizationParameters, + _variants.LegacyAuthorizationParameters, + _variants.LegacyAuthorizationParametersError, + _variants.LegacyConsentRequiredTransferError, + _variants.LegacyConsentRequiredAPError, + ], +) +def test_constructors_include_all_supported_fields(target_class): + """ + Test that all supported fields are included in the constructors. + """ + + method_sig = inspect.signature(target_class.__init__) + for field_name in target_class.SUPPORTED_FIELDS: + # Make sure the constructor has a parameter for this field + assert field_name in method_sig.parameters From f30b179101d1353e188ac1766509bdf4da1d2f1f Mon Sep 17 00:00:00 2001 From: Ada Date: Wed, 12 Jul 2023 07:12:43 -0700 Subject: [PATCH 13/32] Change field name to `required_scopes` Signed-off-by: Ada --- docs/experimental/auth_requirements_errors.rst | 8 ++++---- .../auth_requirements_error/_variants.py | 12 +++++------- .../auth_requirements_error.py | 10 +++++----- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/docs/experimental/auth_requirements_errors.rst b/docs/experimental/auth_requirements_errors.rst index ee293cd25..045129d19 100644 --- a/docs/experimental/auth_requirements_errors.rst +++ b/docs/experimental/auth_requirements_errors.rst @@ -14,14 +14,14 @@ GlobusAuthRequirementsError The ``GlobusAuthRequirementsError`` class provides a model for working with Globus Auth Requirements Error responses in the Python SDK. The shape of an instance closely matches that of the JSON response, such that in order to access a -response's session_required_scopes one could use, e.g.,: +response's required_scopes one could use, e.g.,: .. code-block:: python from globus_sdk.experimental import auth_requirements_error error = auth_requirements_error.GlobusAuthRequirementsError(response) - error.authorization_parameters.session_required_scopes + error.authorization_parameters.required_scopes ``GlobusAuthRequirementsError`` enforces types strictly when parsing a Globus Auth Requirements Error response dictionary, and will raise a ``ValueError`` if a @@ -43,7 +43,7 @@ in your application, e.g.: error = auth_requirements_error.GlobusAuthRequirementsError( code="ConsentRequired", authorization_parameters=GlobusAuthorizationParameters( - session_required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], + required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], session_message="Missing required 'foo' consent", ), ) @@ -67,7 +67,7 @@ by specifying ``include_extra=True``. required_scopes=["urn:globus:auth:scope:transfer.api.globus.org:all[*foo]"], resource="/transfer", authorization_parameters=GlobusAuthorizationParameters( - session_required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], + required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], session_message="Missing required 'foo' consent", ), ) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index a83818c34..546f12b9f 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -74,7 +74,7 @@ def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: return GlobusAuthRequirementsError( code=self.code, authorization_parameters=GlobusAuthorizationParameters( - session_required_scopes=self.required_scopes, + required_scopes=self.required_scopes, session_message=self.extra_fields.get("message"), ), extra=self.extra_fields, @@ -119,7 +119,7 @@ def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: return GlobusAuthRequirementsError( code=self.code, authorization_parameters=GlobusAuthorizationParameters( - session_required_scopes=[self.required_scope], + required_scopes=[self.required_scope], session_message=self.extra_fields.get("description"), extra=self.extra_fields.get("authorization_parameters"), ), @@ -143,7 +143,7 @@ class LegacyAuthorizationParameters: session_required_single_domain: str | list[str] | None session_required_mfa: bool | None # Declared here for compatibility with mixed legacy payloads - session_required_scopes: list[str] | None + required_scopes: list[str] | None extra_fields: dict[str, t.Any] DEFAULT_CODE = "AuthorizationRequired" @@ -158,7 +158,6 @@ class LegacyAuthorizationParameters: validators.OptionalListOfStringsOrCommaDelimitedStrings ), "session_required_mfa": validators.OptionalBool, - "session_required_scopes": validators.OptionalListOfStrings, } def __init__( @@ -168,7 +167,6 @@ def __init__( session_required_policies: str | list[str] | None = None, session_required_single_domain: str | list[str] | None = None, session_required_mfa: bool | None = None, - session_required_scopes: list[str] | None = None, extra: dict[str, t.Any] | None = None, ): # Validate and assign supported fields @@ -193,7 +191,8 @@ def to_authorization_parameters( self, ) -> GlobusAuthorizationParameters: """ - Return a GlobusAuthRequirementsError representing this error. + Return a normalized GlobusAuthorizationParameters instance representing + these parameters. Normalizes fields that may have been provided as comma-delimited strings to lists of strings. @@ -213,7 +212,6 @@ def to_authorization_parameters( session_required_mfa=self.session_required_mfa, session_required_policies=required_policies, session_required_single_domain=required_single_domain, - session_required_scopes=self.session_required_scopes, extra=self.extra_fields, ) diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index b96cf970d..4dbfd0c43 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -30,8 +30,8 @@ class GlobusAuthorizationParameters: :ivar session_required_mfa: Whether MFA is required for the session. :vartype session_required_mfa: bool, optional - :ivar session_required_scopes: A list of scopes for which consent is required. - :vartype session_required_scopes: list of str, optional + :ivar required_scopes: A list of scopes for which consent is required. + :vartype required_scopes: list of str, optional :ivar extra_fields: A dictionary of additional fields that were provided. May be used for forward/backward compatibility. @@ -43,7 +43,7 @@ class GlobusAuthorizationParameters: session_required_policies: list[str] | None session_required_single_domain: list[str] | None session_required_mfa: bool | None - session_required_scopes: list[str] | None + required_scopes: list[str] | None extra_fields: dict[str, t.Any] SUPPORTED_FIELDS = { @@ -52,7 +52,7 @@ class GlobusAuthorizationParameters: "session_required_policies": validators.OptionalListOfStrings, "session_required_single_domain": validators.OptionalListOfStrings, "session_required_mfa": validators.OptionalBool, - "session_required_scopes": validators.OptionalListOfStrings, + "required_scopes": validators.OptionalListOfStrings, } def __init__( @@ -62,7 +62,7 @@ def __init__( session_required_policies: list[str] | None = None, session_required_single_domain: list[str] | None = None, session_required_mfa: bool | None = None, - session_required_scopes: list[str] | None = None, + required_scopes: list[str] | None = None, extra: dict[str, t.Any] | None = None, ): # Validate and assign supported fields From ec5d7813e494fdcb869540f2cf3e4c35a7386d73 Mon Sep 17 00:00:00 2001 From: Ada Date: Wed, 12 Jul 2023 07:18:05 -0700 Subject: [PATCH 14/32] Add missing default for `extra` Signed-off-by: Ada --- .../auth_requirements_error/auth_requirements_error.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index 4dbfd0c43..527cc9a16 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -159,7 +159,7 @@ def __init__( authorization_parameters: dict[str, t.Any] | GlobusAuthorizationParameters | None, - extra: dict[str, t.Any] | None, + extra: dict[str, t.Any] | None = None, ): # Convert authorization_parameters if it's a dict if isinstance(authorization_parameters, dict): From 2b9504b88f6d9150fdff12cf6a38016af6d571cf Mon Sep 17 00:00:00 2001 From: Ada Date: Wed, 12 Jul 2023 07:58:41 -0700 Subject: [PATCH 15/32] Fix tests for required_scopes Missed in a previous commit. Signed-off-by: Ada --- .../test_auth_requirements_error.py | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/tests/unit/experimental/test_auth_requirements_error.py b/tests/unit/experimental/test_auth_requirements_error.py index e8925c5cf..d165ce3f4 100644 --- a/tests/unit/experimental/test_auth_requirements_error.py +++ b/tests/unit/experimental/test_auth_requirements_error.py @@ -63,7 +63,7 @@ def test_create_auth_requirements_error_from_consent_error(error_dict, status): authreq_error = to_auth_requirements_error(error) assert isinstance(authreq_error, GlobusAuthRequirementsError) assert authreq_error.code == "ConsentRequired" - assert authreq_error.authorization_parameters.session_required_scopes == [ + assert authreq_error.authorization_parameters.required_scopes == [ "urn:globus:auth:scope:transfer.api.globus.org:all[*foo *bar]" ] assert ( @@ -204,7 +204,7 @@ def test_create_auth_requirements_errors_from_multiple_errors(): "code": "ConsentRequired", "message": "Missing required foo_bar consent", "authorization_parameters": { - "session_required_scopes": [ + "required_scopes": [ "urn:globus:auth:scope:transfer.api.globus.org:all[*bar]" ], "session_message": "Missing required foo_bar consent", @@ -214,7 +214,7 @@ def test_create_auth_requirements_errors_from_multiple_errors(): "code": "ConsentRequired", "message": "Missing required foo_baz consent", "authorization_parameters": { - "session_required_scopes": [ + "required_scopes": [ "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" ], "session_message": "Missing required foo_baz consent", @@ -262,7 +262,7 @@ def test_create_auth_requirements_errors_from_multiple_errors(): # Check that the proper auth requirements errors were produced assert authreq_errors[0].code == "ConsentRequired" - assert authreq_errors[0].authorization_parameters.session_required_scopes == [ + assert authreq_errors[0].authorization_parameters.required_scopes == [ "urn:globus:auth:scope:transfer.api.globus.org:all[*bar]" ] assert ( @@ -270,7 +270,7 @@ def test_create_auth_requirements_errors_from_multiple_errors(): == "Missing required foo_bar consent" ) assert authreq_errors[1].code == "ConsentRequired" - assert authreq_errors[1].authorization_parameters.session_required_scopes == [ + assert authreq_errors[1].authorization_parameters.required_scopes == [ "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" ] assert ( @@ -350,10 +350,9 @@ def test_backward_compatibility_consent_required_error(): "resource": "/transfer", "authorization_parameters": { "session_message": "Missing baz consent", - "session_required_scopes": [ + "required_scopes": [ "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" ], - "session_required_policies": "foo,bar", "optional": "A non-canonical field", }, }, @@ -371,7 +370,7 @@ def test_backward_compatibility_consent_required_error(): authreq_error = to_auth_requirements_error(error) assert isinstance(authreq_error, GlobusAuthRequirementsError) assert authreq_error.code == "ConsentRequired" - assert authreq_error.authorization_parameters.session_required_scopes == [ + assert authreq_error.authorization_parameters.required_scopes == [ "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" ] assert ( @@ -383,10 +382,9 @@ def test_backward_compatibility_consent_required_error(): "code": "ConsentRequired", "authorization_parameters": { "session_message": "Missing baz consent", - "session_required_scopes": [ + "required_scopes": [ "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" ], - "session_required_policies": ["foo", "bar"], }, } @@ -401,10 +399,9 @@ def test_backward_compatibility_consent_required_error(): "resource": "/transfer", "authorization_parameters": { "session_message": "Missing baz consent", - "session_required_scopes": [ + "required_scopes": [ "urn:globus:auth:scope:transfer.api.globus.org:all[*baz]" ], - "session_required_policies": ["foo", "bar"], "optional": "A non-canonical field", }, } From 2e149a21a9c08f8042f0f80d9c263d813d7e4781 Mon Sep 17 00:00:00 2001 From: Ada Date: Wed, 12 Jul 2023 07:59:26 -0700 Subject: [PATCH 16/32] Improve error details with field name Signed-off-by: Ada --- .../auth_requirements_error/_variants.py | 24 ++++++++++--- .../auth_requirements_error.py | 14 ++++++-- .../auth_requirements_error/utils.py | 9 ++--- .../test_auth_requirements_error.py | 34 +++++++++++++++++++ 4 files changed, 67 insertions(+), 14 deletions(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index 546f12b9f..d39f1e8d2 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -62,7 +62,11 @@ def __init__( ): # Validate and assign supported fields for field_name, validator in self.SUPPORTED_FIELDS.items(): - field_value = validator(locals()[field_name]) + try: + field_value = validator(locals()[field_name]) + except ValueError as e: + raise ValueError(f"Error validating field '{field_name}': {e}") from e + setattr(self, field_name, field_value) self.extra_fields = extra or {} @@ -104,7 +108,11 @@ def __init__( ): # Validate and assign supported fields for field_name, validator in self.SUPPORTED_FIELDS.items(): - field_value = validator(locals()[field_name]) + try: + field_value = validator(locals()[field_name]) + except ValueError as e: + raise ValueError(f"Error validating field '{field_name}': {e}") from e + setattr(self, field_name, field_value) self.extra_fields = extra or {} @@ -171,7 +179,11 @@ def __init__( ): # Validate and assign supported fields for field_name, validator in self.SUPPORTED_FIELDS.items(): - field_value = validator(locals()[field_name]) + try: + field_value = validator(locals()[field_name]) + except ValueError as e: + raise ValueError(f"Error validating field '{field_name}': {e}") from e + setattr(self, field_name, field_value) # Retain any additional fields @@ -272,7 +284,11 @@ def __init__( # Validate and assign supported fields for field_name, validator in self.SUPPORTED_FIELDS.items(): - field_value = validator(locals()[field_name]) + try: + field_value = validator(locals()[field_name]) + except ValueError as e: + raise ValueError(f"Error validating field '{field_name}': {e}") from e + setattr(self, field_name, field_value) # Retain any additional fields diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index 527cc9a16..959554449 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -67,7 +67,11 @@ def __init__( ): # Validate and assign supported fields for field_name, validator in self.SUPPORTED_FIELDS.items(): - field_value = validator(locals()[field_name]) + try: + field_value = validator(locals()[field_name]) + except ValueError as e: + raise ValueError(f"Error validating field '{field_name}': {e}") from e + setattr(self, field_name, field_value) self.extra_fields = extra or {} @@ -79,7 +83,7 @@ def __init__( ): raise ValueError( "Must include at least one supported authorization parameter: " - ", ".join(self.SUPPORTED_FIELDS.keys()) + + ", ".join(self.SUPPORTED_FIELDS.keys()) ) @classmethod @@ -169,7 +173,11 @@ def __init__( # Validate and assign supported fields for field_name, validator in self.SUPPORTED_FIELDS.items(): - field_value = validator(locals()[field_name]) + try: + field_value = validator(locals()[field_name]) + except ValueError as e: + raise ValueError(f"Error validating field '{field_name}': {e}") from e + setattr(self, field_name, field_value) self.extra_fields = extra or {} diff --git a/src/globus_sdk/experimental/auth_requirements_error/utils.py b/src/globus_sdk/experimental/auth_requirements_error/utils.py index 6576a4ab9..d1503efa9 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/utils.py +++ b/src/globus_sdk/experimental/auth_requirements_error/utils.py @@ -57,10 +57,7 @@ def to_auth_requirements_error( try: return GlobusAuthRequirementsError.from_dict(error_dict) except ValueError as err: - log.debug( - "Failed to parse error as " - f"'GlobusAuthRequirementsError' because: {err.args[0]}" - ) + log.debug(f"Failed to parse error as 'GlobusAuthRequirementsError' ({err})") supported_variants: list[t.Type[LegacyAuthRequirementsErrorVariant]] = [ LegacyAuthorizationParametersError, @@ -71,9 +68,7 @@ def to_auth_requirements_error( try: return variant.from_dict(error_dict).to_auth_requirements_error() except ValueError as err: - log.debug( - f"Failed to parse error as '{variant.__name__}' because: {err.args[0]}" - ) + log.debug(f"Failed to parse error as '{variant.__name__}' ({err})") return None diff --git a/tests/unit/experimental/test_auth_requirements_error.py b/tests/unit/experimental/test_auth_requirements_error.py index d165ce3f4..9c0463bd6 100644 --- a/tests/unit/experimental/test_auth_requirements_error.py +++ b/tests/unit/experimental/test_auth_requirements_error.py @@ -427,3 +427,37 @@ def test_constructors_include_all_supported_fields(target_class): for field_name in target_class.SUPPORTED_FIELDS: # Make sure the constructor has a parameter for this field assert field_name in method_sig.parameters + + +@pytest.mark.parametrize( + "target_class, field_name", + [ + (GlobusAuthRequirementsError, "code"), + (_variants.LegacyAuthorizationParametersError, "authorization_parameters"), + (_variants.LegacyConsentRequiredTransferError, "code"), + (_variants.LegacyConsentRequiredAPError, "code"), + ], +) +def test_error_from_dict_insufficient_input(target_class, field_name): + """ """ + with pytest.raises(ValueError) as exc_info: + target_class.from_dict({}) + + assert f"Error validating field '{field_name}'" in str(exc_info.value) + + +@pytest.mark.parametrize( + "target_class", + [ + GlobusAuthorizationParameters, + _variants.LegacyAuthorizationParameters, + ], +) +def test_authorization_parameters_from_dict_insufficient_input(target_class): + """ """ + with pytest.raises(ValueError) as exc_info: + target_class.from_dict({}) + + assert "Must include at least one supported authorization parameter" in str( + exc_info.value + ) From 951fa39c107f3eda29a95de977b288e342617d7c Mon Sep 17 00:00:00 2001 From: Ada Date: Wed, 12 Jul 2023 08:27:00 -0700 Subject: [PATCH 17/32] Tweak language in docs Signed-off-by: Ada --- .../experimental/auth_requirements_errors.rst | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/docs/experimental/auth_requirements_errors.rst b/docs/experimental/auth_requirements_errors.rst index 045129d19..7e30d2d73 100644 --- a/docs/experimental/auth_requirements_errors.rst +++ b/docs/experimental/auth_requirements_errors.rst @@ -27,8 +27,8 @@ response's required_scopes one could use, e.g.,: Auth Requirements Error response dictionary, and will raise a ``ValueError`` if a supported field is supplied with a value of the wrong type. ``GlobusAuthRequirementsError`` does not, however, attempt to mimic or itself enforce -any logic specific to the Globus Auth service with regard to what represents a coherent -combination of fields (e.g., that ``session_required_mfa`` requires either +any logic specific to the Globus Auth service with regard to what represents a valid +combination of fields (e.g., ``session_required_mfa`` requires either ``session_required_identities`` or ``session_required_single_domain`` in order to be properly handled). @@ -38,9 +38,9 @@ in your application, e.g.: .. code-block:: python - from globus_sdk.experimental import auth_requirements_error + from globus_sdk.experimental.auth_requirements_error import GlobusAuthRequirementsError - error = auth_requirements_error.GlobusAuthRequirementsError( + error = GlobusAuthRequirementsError( code="ConsentRequired", authorization_parameters=GlobusAuthorizationParameters( required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], @@ -51,25 +51,28 @@ in your application, e.g.: # Render a strict dictionary error.to_dict() -If non-canonical fields are supplied on creation (either as keyword arguments -during instantiation or as fields in a dictionary supplied to ``from_dict()``), -you can preserve these fields in the rendered output dictionary -by specifying ``include_extra=True``. +If non-canonical fields are needed, the ``extra`` argument can be used to +supply a dictionary of additional fields to include. Non-canonical fields present +in the provided dictionary when calling ``from_dict()`` are stored similarly. +You can include these fields in the rendered output dictionary +by specifying ``include_extra=True`` when calling ``to_dict()``. .. code-block:: python - from globus_sdk.experimental import auth_requirements_error + from globus_sdk.experimental.auth_requirements_error import GlobusAuthRequirementsError - error = auth_requirements_error.GlobusAuthRequirementsError( + error = GlobusAuthRequirementsError( code="ConsentRequired", - message="Missing required 'foo' consent", - request_id="WmMV97A1w", - required_scopes=["urn:globus:auth:scope:transfer.api.globus.org:all[*foo]"], - resource="/transfer", authorization_parameters=GlobusAuthorizationParameters( required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], session_message="Missing required 'foo' consent", ), + extra={ + "message": "Missing required 'foo' consent", + "request_id": "WmMV97A1w", + "required_scopes": ["urn:globus:auth:scope:transfer.api.globus.org:all[*foo]"], + "resource": "/transfer", + }, ) # Render a dictionary with extra fields @@ -83,7 +86,7 @@ Parsing Responses ----------------- If you are writing a client to a Globus API, the ``auth_requirements_error`` subpackage -provides utilities to detect legacy auth requirements error response +provides utilities to detect legacy Globus Auth requirements error response formats and normalize them. To detect if a ``GlobusAPIError``, ``ErrorSubdocument``, or JSON response @@ -116,7 +119,7 @@ Requirements Error, you can use, e.g.,: .. note:: - If a GlobusAPIError represents multiple errors that were returned in an + If a ``GlobusAPIError`` represents multiple errors that were returned in an array, this only returns the first error in that array that can be converted to the Globus Auth Requirements Error response format. In this case (and, in general) it's preferable to use ``to_auth_requirements_errors()`` (which also From 65b6fc6d4735bf240b505e0ede51fd2b63cb2c4d Mon Sep 17 00:00:00 2001 From: Ada Date: Thu, 3 Aug 2023 16:35:36 -0700 Subject: [PATCH 18/32] Ignore unused-arguments warning Signed-off-by: Ada --- .../experimental/auth_requirements_error/_variants.py | 6 +++--- .../auth_requirements_error/auth_requirements_error.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index d39f1e8d2..39feb91e4 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -59,7 +59,7 @@ def __init__( code: str | None, required_scopes: list[str] | None, extra: dict[str, t.Any] | None = None, - ): + ): # pylint: disable=unused-argument # Validate and assign supported fields for field_name, validator in self.SUPPORTED_FIELDS.items(): try: @@ -105,7 +105,7 @@ def __init__( code: str | None, required_scope: str | None, extra: dict[str, t.Any] | None, - ): + ): # pylint: disable=unused-argument # Validate and assign supported fields for field_name, validator in self.SUPPORTED_FIELDS.items(): try: @@ -176,7 +176,7 @@ def __init__( session_required_single_domain: str | list[str] | None = None, session_required_mfa: bool | None = None, extra: dict[str, t.Any] | None = None, - ): + ): # pylint: disable=unused-argument # Validate and assign supported fields for field_name, validator in self.SUPPORTED_FIELDS.items(): try: diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index 959554449..c5a45b02a 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -64,7 +64,7 @@ def __init__( session_required_mfa: bool | None = None, required_scopes: list[str] | None = None, extra: dict[str, t.Any] | None = None, - ): + ): # pylint: disable=unused-argument # Validate and assign supported fields for field_name, validator in self.SUPPORTED_FIELDS.items(): try: @@ -159,7 +159,7 @@ class GlobusAuthRequirementsError(GlobusError): def __init__( self, - code: str | None, + code: str | None, # pylint: disable=unused-argument authorization_parameters: dict[str, t.Any] | GlobusAuthorizationParameters | None, From bbbe459d15ee49236d4b353ae939ffed0f14a200 Mon Sep 17 00:00:00 2001 From: Ada Date: Fri, 4 Aug 2023 07:58:49 -0700 Subject: [PATCH 19/32] Tweak changelog Signed-off-by: Ada --- changelog.d/20230629_093104_ada_add_session_error_support.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/20230629_093104_ada_add_session_error_support.rst b/changelog.d/20230629_093104_ada_add_session_error_support.rst index c8f9b6dd4..6efa594a2 100644 --- a/changelog.d/20230629_093104_ada_add_session_error_support.rst +++ b/changelog.d/20230629_093104_ada_add_session_error_support.rst @@ -1,3 +1,3 @@ * Add a new class ``GlobusAuthRequirementsError`` and utility functions to the - experimental package to support handling of the Globus Auth Requirements Error + experimental subpackage to support handling of the Globus Auth Requirements Error response format (:pr:`NUMBER`) From e0d4d700f802840892330847c493eb6119863cb5 Mon Sep 17 00:00:00 2001 From: Ada Date: Fri, 4 Aug 2023 08:02:03 -0700 Subject: [PATCH 20/32] Remove "Python SDK" from documentation copy Signed-off-by: Ada --- docs/experimental/auth_requirements_errors.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/experimental/auth_requirements_errors.rst b/docs/experimental/auth_requirements_errors.rst index 7e30d2d73..3a1eaa98b 100644 --- a/docs/experimental/auth_requirements_errors.rst +++ b/docs/experimental/auth_requirements_errors.rst @@ -12,7 +12,7 @@ GlobusAuthRequirementsError --------------------------- The ``GlobusAuthRequirementsError`` class provides a model for working with Globus -Auth Requirements Error responses in the Python SDK. The shape of an instance closely +Auth Requirements Error responses. The shape of an instance closely matches that of the JSON response, such that in order to access a response's required_scopes one could use, e.g.,: From 5c180fd35fb3959e549b5dfe20abfb973b72e799 Mon Sep 17 00:00:00 2001 From: Ada Date: Fri, 4 Aug 2023 08:05:20 -0700 Subject: [PATCH 21/32] Require keyword arguments for GARE and variants Signed-off-by: Ada --- .../experimental/auth_requirements_error/_variants.py | 4 ++++ .../auth_requirements_error/auth_requirements_error.py | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index 39feb91e4..87c712d1b 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -56,6 +56,7 @@ class LegacyConsentRequiredTransferError(LegacyAuthRequirementsErrorVariant): def __init__( self, + *, code: str | None, required_scopes: list[str] | None, extra: dict[str, t.Any] | None = None, @@ -102,6 +103,7 @@ class LegacyConsentRequiredAPError(LegacyAuthRequirementsErrorVariant): def __init__( self, + *, code: str | None, required_scope: str | None, extra: dict[str, t.Any] | None, @@ -170,6 +172,7 @@ class LegacyAuthorizationParameters: def __init__( self, + *, session_message: str | None = None, session_required_identities: list[str] | None = None, session_required_policies: str | list[str] | None = None, @@ -267,6 +270,7 @@ class LegacyAuthorizationParametersError(LegacyAuthRequirementsErrorVariant): def __init__( self, + *, authorization_parameters: dict[str, t.Any] | LegacyAuthorizationParameters | None, diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index c5a45b02a..2da9570c8 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -57,6 +57,7 @@ class GlobusAuthorizationParameters: def __init__( self, + *, session_message: str | None = None, session_required_identities: list[str] | None = None, session_required_policies: list[str] | None = None, @@ -163,6 +164,7 @@ def __init__( authorization_parameters: dict[str, t.Any] | GlobusAuthorizationParameters | None, + *, extra: dict[str, t.Any] | None = None, ): # Convert authorization_parameters if it's a dict From ce90eb8741bec0847fdc4c5dc24eb3d8e46a7cd6 Mon Sep 17 00:00:00 2001 From: Ada <107940310+ada-globus@users.noreply.github.com> Date: Sun, 6 Aug 2023 10:06:15 -0700 Subject: [PATCH 22/32] Simplify double-negative constructions in check Co-authored-by: Kurt McKee --- .../experimental/auth_requirements_error/_variants.py | 6 +++--- .../auth_requirements_error/auth_requirements_error.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index 87c712d1b..7ccbcb73b 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -193,9 +193,9 @@ def __init__( self.extra_fields = extra or {} # Enforce that the error contains at least one of the fields we expect - if not any( - (getattr(self, field_name) is not None) - for field_name in self.SUPPORTED_FIELDS.keys() + if all( + getattr(self, field_name) is None + for field_name in self.SUPPORTED_FIELDS ): raise ValueError( "Must include at least one supported authorization parameter: " diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index 2da9570c8..cbc04d27e 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -78,9 +78,9 @@ def __init__( self.extra_fields = extra or {} # Enforce that the error contains at least one of the fields we expect - if not any( - (getattr(self, field_name) is not None) - for field_name in self.SUPPORTED_FIELDS.keys() + if all( + getattr(self, field_name) is None + for field_name in self.SUPPORTED_FIELDS ): raise ValueError( "Must include at least one supported authorization parameter: " From dede384c787a5577465fbd393aa182973beef9b4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 6 Aug 2023 17:06:30 +0000 Subject: [PATCH 23/32] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../experimental/auth_requirements_error/_variants.py | 3 +-- .../auth_requirements_error/auth_requirements_error.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index 7ccbcb73b..a63b40333 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -194,8 +194,7 @@ def __init__( # Enforce that the error contains at least one of the fields we expect if all( - getattr(self, field_name) is None - for field_name in self.SUPPORTED_FIELDS + getattr(self, field_name) is None for field_name in self.SUPPORTED_FIELDS ): raise ValueError( "Must include at least one supported authorization parameter: " diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index cbc04d27e..ad77c45c1 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -79,8 +79,7 @@ def __init__( # Enforce that the error contains at least one of the fields we expect if all( - getattr(self, field_name) is None - for field_name in self.SUPPORTED_FIELDS + getattr(self, field_name) is None for field_name in self.SUPPORTED_FIELDS ): raise ValueError( "Must include at least one supported authorization parameter: " From 14887761bf5b6fc2f361153ed0436856707498fc Mon Sep 17 00:00:00 2001 From: Ada Date: Sun, 6 Aug 2023 13:58:11 -0700 Subject: [PATCH 24/32] Apply docs suggestions from code review Co-authored-by: Kurt McKee Co-authored-by: Stephen Rosen Signed-off-by: Ada --- .../experimental/auth_requirements_errors.rst | 71 ++++++++++--------- .../auth_requirements_error/utils.py | 2 +- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/docs/experimental/auth_requirements_errors.rst b/docs/experimental/auth_requirements_errors.rst index 3a1eaa98b..1809b8a3c 100644 --- a/docs/experimental/auth_requirements_errors.rst +++ b/docs/experimental/auth_requirements_errors.rst @@ -5,36 +5,19 @@ Globus Auth Requirements Error is a response format that conveys to a client any modifications to a session (i.e., "boosting") that will be required to complete a particular request. -The ``auth_requirements_error`` module provides a number of tools to make it easier to -identify and handle these errors when they occur. +The ``globus_sdk.experimental.auth_requirements_error`` module provides a +number of tools to make it easier to identify and handle these errors when they occur. GlobusAuthRequirementsError --------------------------- The ``GlobusAuthRequirementsError`` class provides a model for working with Globus -Auth Requirements Error responses. The shape of an instance closely -matches that of the JSON response, such that in order to access a -response's required_scopes one could use, e.g.,: +Auth Requirements Error responses. -.. code-block:: python - - from globus_sdk.experimental import auth_requirements_error - - error = auth_requirements_error.GlobusAuthRequirementsError(response) - error.authorization_parameters.required_scopes - -``GlobusAuthRequirementsError`` enforces types strictly when parsing a Globus -Auth Requirements Error response dictionary, and will raise a ``ValueError`` if a -supported field is supplied with a value of the wrong type. -``GlobusAuthRequirementsError`` does not, however, attempt to mimic or itself enforce -any logic specific to the Globus Auth service with regard to what represents a valid -combination of fields (e.g., ``session_required_mfa`` requires either -``session_required_identities`` or ``session_required_single_domain`` -in order to be properly handled). - -If you are writing a service that needs to respond with a Globus Auth Requirements Error, you can -instantiate the ``GlobusAuthRequirementsError`` class directly to emit auth requirements errors -in your application, e.g.: +Services in the Globus ecosystem may need to communicate authorization requirements +to their consumers. For example, a service may need to instruct clients to have the user +consent to an additional scope, ``"foo"``. In such a case, ``GlobusAuthRequirementsError`` +can provide serialization into the well-known Globus Auth Requirements Error format: .. code-block:: python @@ -43,7 +26,7 @@ in your application, e.g.: error = GlobusAuthRequirementsError( code="ConsentRequired", authorization_parameters=GlobusAuthorizationParameters( - required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], + required_scopes=["foo"], session_message="Missing required 'foo' consent", ), ) @@ -64,13 +47,13 @@ by specifying ``include_extra=True`` when calling ``to_dict()``. error = GlobusAuthRequirementsError( code="ConsentRequired", authorization_parameters=GlobusAuthorizationParameters( - required_scopes=["urn:globus:auth:scope:transfer.api.globus.org"], + required_scopes=["foo"], session_message="Missing required 'foo' consent", ), extra={ "message": "Missing required 'foo' consent", "request_id": "WmMV97A1w", - "required_scopes": ["urn:globus:auth:scope:transfer.api.globus.org:all[*foo]"], + "required_scopes": ["foo"], "resource": "/transfer", }, ) @@ -79,8 +62,15 @@ by specifying ``include_extra=True`` when calling ``to_dict()``. error.to_dict(include_extra=True) These fields are stored by both the ``GlobusAuthRequirementsError`` and -``GlobusAuthenticationParameters`` classes in an ``extra_fields`` -attribute. +``GlobusAuthenticationParameters`` classes in an ``extra_fields`` attribute. + +.. note:: + + Non-canonical fields in a Globus Auth Requirements Error are primarily intended + to make it easier for services to provide backward-compatibile error responses + to clients that have not adopted the Globus Auth Requirements Error format. Avoid + using non-canonical fields for any data that should be generically understood by + a consumer of the error response. Parsing Responses ----------------- @@ -120,11 +110,11 @@ Requirements Error, you can use, e.g.,: .. note:: If a ``GlobusAPIError`` represents multiple errors that were returned in an - array, this only returns the first error in that array that can be - converted to the Globus Auth Requirements Error response format. In this case (and, - in general) it's preferable to use ``to_auth_requirements_errors()`` (which also - accepts a list of ``GlobusAPIError``\ s, ``ErrorSubdocument``\ s, and JSON - response dictionaries): + array, ``to_auth_requirements_error()`` only returns the first error in that + array that can be converted to the Globus Auth Requirements Error response format. + In this case (and in general) it's preferable to use + ``to_auth_requirements_errors()`` (which also accepts a list of + ``GlobusAPIError``\ s, ``ErrorSubdocument``\ s, and JSON response dictionaries): .. code-block:: python @@ -134,3 +124,16 @@ Requirements Error, you can use, e.g.,: auth_requirements_error.utils.to_auth_requirements_errors( [other_error] ) # [GlobusAuthRequirementsError, ...] + +Notes +----- + +``GlobusAuthRequirementsError`` enforces types strictly when parsing a Globus +Auth Requirements Error response dictionary, and will raise a ``ValueError`` if a +supported field is supplied with a value of the wrong type. + +``GlobusAuthRequirementsError`` does not attempt to mimic or itself enforce +any logic specific to the Globus Auth service with regard to what represents a valid +combination of fields (e.g., ``session_required_mfa`` requires either +``session_required_identities`` or ``session_required_single_domain`` +in order to be properly handled). \ No newline at end of file diff --git a/src/globus_sdk/experimental/auth_requirements_error/utils.py b/src/globus_sdk/experimental/auth_requirements_error/utils.py index d1503efa9..92d9c6bb1 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/utils.py +++ b/src/globus_sdk/experimental/auth_requirements_error/utils.py @@ -26,7 +26,7 @@ def to_auth_requirements_error( .. note:: - a GlobusAPIError may contain multiple errors, and in this case only a single + A GlobusAPIError may contain multiple errors, and in this case only a single GlobusAuthRequirementsError is returned for the first error that matches a known format. From 5562209ebb0ab2a960ddf1f19aec249560a67f8e Mon Sep 17 00:00:00 2001 From: Ada Date: Sun, 6 Aug 2023 14:03:14 -0700 Subject: [PATCH 25/32] Narrow types in constructor signature Per a conversation with Stephen about evaluation of hints in the __init__() signature, so therefore, Co-authored-by: Stephen Rosen Signed-off-by: Ada --- .../auth_requirements_error/_variants.py | 12 +++++------- .../auth_requirements_error.py | 6 ++---- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index a63b40333..c1254d5ee 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -57,8 +57,8 @@ class LegacyConsentRequiredTransferError(LegacyAuthRequirementsErrorVariant): def __init__( self, *, - code: str | None, - required_scopes: list[str] | None, + code: str, + required_scopes: list[str], extra: dict[str, t.Any] | None = None, ): # pylint: disable=unused-argument # Validate and assign supported fields @@ -104,8 +104,8 @@ class LegacyConsentRequiredAPError(LegacyAuthRequirementsErrorVariant): def __init__( self, *, - code: str | None, - required_scope: str | None, + code: str, + required_scope: str, extra: dict[str, t.Any] | None, ): # pylint: disable=unused-argument # Validate and assign supported fields @@ -270,9 +270,7 @@ class LegacyAuthorizationParametersError(LegacyAuthRequirementsErrorVariant): def __init__( self, *, - authorization_parameters: dict[str, t.Any] - | LegacyAuthorizationParameters - | None, + authorization_parameters: dict[str, t.Any] | LegacyAuthorizationParameters, code: str | None = None, extra: dict[str, t.Any] | None = None, ): diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index ad77c45c1..42118413b 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -159,10 +159,8 @@ class GlobusAuthRequirementsError(GlobusError): def __init__( self, - code: str | None, # pylint: disable=unused-argument - authorization_parameters: dict[str, t.Any] - | GlobusAuthorizationParameters - | None, + code: str, # pylint: disable=unused-argument + authorization_parameters: dict[str, t.Any] | GlobusAuthorizationParameters, *, extra: dict[str, t.Any] | None = None, ): From 5905d8615887b22d277855e985e4d8da5b73e64e Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Sun, 6 Aug 2023 14:20:28 -0500 Subject: [PATCH 26/32] Make GARE validation as simple as possible Rather than trying to build a framework, this is the "no framework" solution, in which we define very concrete helpers for very concrete purposes, including a dedicated one for the "ConsentRequired" Literal. All of the GARE classes now inherit from an internal helper class, Serializable, which defines from_dict, to_dict, and _supported_fields in a generic way. The last of these *does* use some signature inspection magic, but nothing too abstruse. The basic transformation is to replace each combination of a class-level annotation + a `SUPPORTED_FIELDS` entry with relevant assignment in `__init__`. Some tangentially related simplifications and minor improvements are included: - `extra_fields -> extra` - Removal of unnecessary str splitting (after object is initialized) - `isinstance` checking can also handle deserialization of dicts - checking for non-null values will not accept `session_message`-only GARE data -- at least one of the semantic fields is required by the rewritten check - GlobusAuthRequirementsError does not inherit from `GlobusError` or `Exception` -- it is not clear that this inheritance is useful or instructive to any user, since it mixes Exception hierarchies with data-representing hierarchies - replace direct `ValueError` usage with a custom ValidationError class -- this avoids any messy scenarios in which a ValueError is accidentally introduced and incorrectly caught (e.g. from a stdlib call) There are also some more significant improvements included. Most notably: - Annotations explicitly do not accept `None` unless it is a valid value (i.e. annotations align with validation requirements) - to_dict can now look for a concrete type (`Serializable`) and therefore can automatically invoke `to_dict` down a tree of objects Although brevity is a non-goal of this changeset -- more verbose but clearer would be acceptable -- the result is almost 200 LOC lighter in the `src/` tree. The primary ways in which things became shorter appear to be: - explicit version is often much shorter than the framework-ized version (e.g. `LegacyConsentRequiredAPError`), and even where the two are close, the explicit version is shorter - `to_dict` and `from_dict` centralization --- .../auth_requirements_error/_serializable.py | 60 +++++ .../auth_requirements_error/_validators.py | 77 ++++++ .../auth_requirements_error/_variants.py | 239 +++++------------- .../auth_requirements_error.py | 185 +++----------- .../auth_requirements_error/utils.py | 7 +- .../auth_requirements_error/validators.py | 103 -------- .../test_auth_requirements_error.py | 57 ++--- 7 files changed, 269 insertions(+), 459 deletions(-) create mode 100644 src/globus_sdk/experimental/auth_requirements_error/_serializable.py create mode 100644 src/globus_sdk/experimental/auth_requirements_error/_validators.py delete mode 100644 src/globus_sdk/experimental/auth_requirements_error/validators.py diff --git a/src/globus_sdk/experimental/auth_requirements_error/_serializable.py b/src/globus_sdk/experimental/auth_requirements_error/_serializable.py new file mode 100644 index 000000000..c1931fc1f --- /dev/null +++ b/src/globus_sdk/experimental/auth_requirements_error/_serializable.py @@ -0,0 +1,60 @@ +from __future__ import annotations + +import inspect +import typing as t + +T = t.TypeVar("T", bound="Serializable") + + +class Serializable: + _EXLUDE_VARS: t.ClassVar[tuple[str, ...]] = ("self", "extra_fields", "extra") + extra: dict[str, t.Any] + + @classmethod + def _supported_fields(cls) -> list[str]: + signature = inspect.signature(cls.__init__) + return [ + name for name in signature.parameters.keys() if name not in cls._EXLUDE_VARS + ] + + @classmethod + def from_dict(cls: type[T], data: dict[str, t.Any]) -> T: + """ + Instantiate from a dictionary. + + :param data: The dictionary to create the error from. + :type data: dict + """ + + # Extract any extra fields + extras = {k: v for k, v in data.items() if k not in cls._supported_fields()} + kwargs: dict[str, t.Any] = {"extra": extras} + # Ensure required fields are supplied + for field_name in cls._supported_fields(): + kwargs[field_name] = data.get(field_name) + + return cls(**kwargs) + + def to_dict(self, include_extra: bool = False) -> dict[str, t.Any]: + """ + Render to a dictionary. + + :param include_extra: Whether to include stored extra (non-standard) fields in + the returned dictionary. + :type include_extra: bool + """ + result = {} + + # Set any authorization parameters + for field in self._supported_fields(): + value = getattr(self, field) + if value is not None: + if isinstance(value, Serializable): + value = value.to_dict(include_extra=include_extra) + result[field] = value + + # Set any extra fields + if include_extra: + result.update(self.extra) + + return result diff --git a/src/globus_sdk/experimental/auth_requirements_error/_validators.py b/src/globus_sdk/experimental/auth_requirements_error/_validators.py new file mode 100644 index 000000000..69a054cb6 --- /dev/null +++ b/src/globus_sdk/experimental/auth_requirements_error/_validators.py @@ -0,0 +1,77 @@ +from __future__ import annotations + +import sys +import typing as t + +from ._serializable import Serializable + +if sys.version_info >= (3, 8): + from typing import Literal +else: + from typing_extensions import Literal + +S = t.TypeVar("S", bound=Serializable) + + +class ValidationError(ValueError): + pass + + +def str_(name: str, value: t.Any) -> str: + if not isinstance(value, str): + raise ValidationError(f"'{name}' must be a string") + return value + + +def opt_str(name: str, value: t.Any) -> str | None: + if value is None: + return None + if not isinstance(value, str): + raise ValidationError(f"'{name}' must be a string") + return value + + +def consent_required_literal(name: str, value: t.Any) -> Literal["ConsentRequired"]: + if not isinstance(value, str) or value != "ConsentRequired": + raise ValidationError(f"'{name}' must be the string 'ConsentRequired'") + return t.cast(Literal["ConsentRequired"], value) + + +def opt_bool(name: str, value: t.Any) -> bool | None: + if value is None: + return None + if not isinstance(value, bool): + raise ValidationError(f"'{name}' must be a bool") + return value + + +def str_list(name: str, value: t.Any) -> list[str]: + if not (isinstance(value, list) and all(isinstance(s, str) for s in value)): + raise ValidationError(f"'{name}' must be a list of strings") + return value + + +def opt_str_list(name: str, value: t.Any) -> list[str] | None: + if value is None: + return None + if not (isinstance(value, list) and all(isinstance(s, str) for s in value)): + raise ValidationError(f"'{name}' must be a list of strings") + return value + + +def opt_str_list_or_commasep(name: str, value: t.Any) -> list[str] | None: + if value is None: + return None + if isinstance(value, str): + value = value.split(",") + if not (isinstance(value, list) and all(isinstance(s, str) for s in value)): + raise ValidationError(f"'{name}' must be a list of strings") + return value + + +def instance_or_dict(name: str, value: t.Any, cls: type[S]) -> S: + if isinstance(value, cls): + return value + if isinstance(value, dict): + return cls.from_dict(value) + raise ValidationError(f"'{name}' must be a '{cls.__name__}' object or a dictionary") diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index c1254d5ee..3bf21ec40 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -1,76 +1,50 @@ from __future__ import annotations +import sys import typing as t -from . import validators +from . import _serializable, _validators from .auth_requirements_error import ( GlobusAuthorizationParameters, GlobusAuthRequirementsError, ) -T = t.TypeVar("T", bound="LegacyAuthRequirementsErrorVariant") +if sys.version_info >= (3, 8): + from typing import Literal, Protocol +else: + from typing_extensions import Literal, Protocol +V = t.TypeVar("V", bound="LegacyAuthRequirementsErrorVariant") -class LegacyAuthRequirementsErrorVariant: + +class LegacyAuthRequirementsErrorVariant(Protocol): """ - Abstract base class for errors which can be converted to a - Globus Auth Requirements Error. + Protocol for errors which can be converted to a Globus Auth Requirements Error. """ - SUPPORTED_FIELDS: dict[str, t.Callable[[t.Any], t.Any]] = {} - @classmethod - def from_dict(cls: t.Type[T], error_dict: dict[str, t.Any]) -> T: - """ - Instantiate from an error dictionary. - - :param error_dict: The dictionary to instantiate the error from. - :type error_dict: dict - """ - # Extract any extra fields - extras = {k: v for k, v in error_dict.items() if k not in cls.SUPPORTED_FIELDS} - kwargs: dict[str, t.Any] = {"extra": extras} - # Ensure required fields are supplied - for field_name in cls.SUPPORTED_FIELDS.keys(): - kwargs[field_name] = error_dict.get(field_name) - - return cls(**kwargs) + def from_dict(cls: type[V], data: dict[str, t.Any]) -> V: + pass def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: - raise NotImplementedError() + ... -class LegacyConsentRequiredTransferError(LegacyAuthRequirementsErrorVariant): +class LegacyConsentRequiredTransferError(_serializable.Serializable): """ The ConsentRequired error format emitted by the Globus Transfer service. """ - code: str - required_scopes: list[str] - extra_fields: dict[str, t.Any] - - SUPPORTED_FIELDS = { - "code": validators.StringLiteral("ConsentRequired"), - "required_scopes": validators.ListOfStrings, - } - def __init__( self, *, - code: str, + code: Literal["ConsentRequired"], required_scopes: list[str], extra: dict[str, t.Any] | None = None, - ): # pylint: disable=unused-argument - # Validate and assign supported fields - for field_name, validator in self.SUPPORTED_FIELDS.items(): - try: - field_value = validator(locals()[field_name]) - except ValueError as e: - raise ValueError(f"Error validating field '{field_name}': {e}") from e - - setattr(self, field_name, field_value) - - self.extra_fields = extra or {} + ): + self.code = _validators.consent_required_literal("code", code) + self.required_scopes = _validators.str_list("required_scopes", required_scopes) + self.extra = extra or {} def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: """ @@ -80,44 +54,28 @@ def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: code=self.code, authorization_parameters=GlobusAuthorizationParameters( required_scopes=self.required_scopes, - session_message=self.extra_fields.get("message"), + session_message=self.extra.get("message"), ), - extra=self.extra_fields, + extra=self.extra, ) -class LegacyConsentRequiredAPError(LegacyAuthRequirementsErrorVariant): +class LegacyConsentRequiredAPError(_serializable.Serializable): """ The ConsentRequired error format emitted by the legacy Globus Transfer Action Providers. """ - code: str - required_scope: str - extra_fields: dict[str, t.Any] - - SUPPORTED_FIELDS = { - "code": validators.StringLiteral("ConsentRequired"), - "required_scope": validators.String, - } - def __init__( self, *, - code: str, + code: Literal["ConsentRequired"], required_scope: str, extra: dict[str, t.Any] | None, - ): # pylint: disable=unused-argument - # Validate and assign supported fields - for field_name, validator in self.SUPPORTED_FIELDS.items(): - try: - field_value = validator(locals()[field_name]) - except ValueError as e: - raise ValueError(f"Error validating field '{field_name}': {e}") from e - - setattr(self, field_name, field_value) - - self.extra_fields = extra or {} + ): + self.code = _validators.consent_required_literal("code", code) + self.required_scope = _validators.str_("required_scope", required_scope) + self.extra = extra or {} def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: """ @@ -130,46 +88,21 @@ def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: code=self.code, authorization_parameters=GlobusAuthorizationParameters( required_scopes=[self.required_scope], - session_message=self.extra_fields.get("description"), - extra=self.extra_fields.get("authorization_parameters"), + session_message=self.extra.get("description"), + extra=self.extra.get("authorization_parameters"), ), extra={ - k: v - for k, v in self.extra_fields.items() - if k != "authorization_parameters" + k: v for k, v in self.extra.items() if k != "authorization_parameters" }, ) -class LegacyAuthorizationParameters: +class LegacyAuthorizationParameters(_serializable.Serializable): """ An Authorization Parameters object that describes all known variants in use by Globus services. """ - session_message: str | None - session_required_identities: list[str] | None - session_required_policies: str | list[str] | None - session_required_single_domain: str | list[str] | None - session_required_mfa: bool | None - # Declared here for compatibility with mixed legacy payloads - required_scopes: list[str] | None - extra_fields: dict[str, t.Any] - - DEFAULT_CODE = "AuthorizationRequired" - - SUPPORTED_FIELDS = { - "session_message": validators.OptionalString, - "session_required_identities": validators.OptionalListOfStrings, - "session_required_policies": ( - validators.OptionalListOfStringsOrCommaDelimitedStrings - ), - "session_required_single_domain": ( - validators.OptionalListOfStringsOrCommaDelimitedStrings - ), - "session_required_mfa": validators.OptionalBool, - } - def __init__( self, *, @@ -179,26 +112,32 @@ def __init__( session_required_single_domain: str | list[str] | None = None, session_required_mfa: bool | None = None, extra: dict[str, t.Any] | None = None, - ): # pylint: disable=unused-argument - # Validate and assign supported fields - for field_name, validator in self.SUPPORTED_FIELDS.items(): - try: - field_value = validator(locals()[field_name]) - except ValueError as e: - raise ValueError(f"Error validating field '{field_name}': {e}") from e - - setattr(self, field_name, field_value) - - # Retain any additional fields - self.extra_fields = extra or {} + ): + self.session_message = _validators.opt_str("session_message", session_message) + self.session_required_identities = _validators.opt_str_list( + "session_required_identities", session_required_identities + ) + self.session_required_policies = _validators.opt_str_list_or_commasep( + "session_required_policies", session_required_policies + ) + self.session_required_single_domain = _validators.opt_str_list_or_commasep( + "session_required_single_domain", session_required_single_domain + ) + self.session_required_mfa = _validators.opt_bool( + "session_required_mfa", session_required_mfa + ) + self.extra = extra or {} # Enforce that the error contains at least one of the fields we expect + requires_at_least_one = [ + name for name in self._supported_fields() if name != "session_message" + ] if all( - getattr(self, field_name) is None for field_name in self.SUPPORTED_FIELDS + getattr(self, field_name) is None for field_name in requires_at_least_one ): - raise ValueError( + raise _validators.ValidationError( "Must include at least one supported authorization parameter: " - ", ".join(self.SUPPORTED_FIELDS.keys()) + + ", ".join(requires_at_least_one) ) def to_authorization_parameters( @@ -211,62 +150,24 @@ def to_authorization_parameters( Normalizes fields that may have been provided as comma-delimited strings to lists of strings. """ - required_policies = self.session_required_policies - if isinstance(required_policies, str): - required_policies = required_policies.split(",") - - # TODO: Unnecessary? - required_single_domain = self.session_required_single_domain - if isinstance(required_single_domain, str): - required_single_domain = required_single_domain.split(",") - return GlobusAuthorizationParameters( session_message=self.session_message, session_required_identities=self.session_required_identities, session_required_mfa=self.session_required_mfa, - session_required_policies=required_policies, - session_required_single_domain=required_single_domain, - extra=self.extra_fields, + session_required_policies=self.session_required_policies, + session_required_single_domain=self.session_required_single_domain, + extra=self.extra, ) - @classmethod - def from_dict(cls, param_dict: dict[str, t.Any]) -> LegacyAuthorizationParameters: - """ - Instantiate from an authorization_parameters dictionary. - - :param param_dict: The dictionary to create the AuthorizationParameters from. - :type param_dict: dict - """ - - # Extract any extra fields - extras = {k: v for k, v in param_dict.items() if k not in cls.SUPPORTED_FIELDS} - kwargs: dict[str, t.Any] = {"extra": extras} - # Ensure required fields are supplied - for field_name in cls.SUPPORTED_FIELDS.keys(): - kwargs[field_name] = param_dict.get(field_name) - - return cls(**kwargs) - -class LegacyAuthorizationParametersError(LegacyAuthRequirementsErrorVariant): +class LegacyAuthorizationParametersError(_serializable.Serializable): """ Defines an Authorization Parameters error that describes all known variants in use by Globus services. """ - authorization_parameters: LegacyAuthorizationParameters - code: str - extra_fields: dict[str, t.Any] - DEFAULT_CODE = "AuthorizationRequired" - SUPPORTED_FIELDS = { - "code": validators.String, - "authorization_parameters": validators.ClassInstance( - LegacyAuthorizationParameters - ), - } - def __init__( self, *, @@ -275,25 +176,13 @@ def __init__( extra: dict[str, t.Any] | None = None, ): # Apply default, if necessary - code = code or self.DEFAULT_CODE - - # Convert authorization_parameters if it's a dict - if isinstance(authorization_parameters, dict): - authorization_parameters = LegacyAuthorizationParameters.from_dict( - param_dict=authorization_parameters - ) - - # Validate and assign supported fields - for field_name, validator in self.SUPPORTED_FIELDS.items(): - try: - field_value = validator(locals()[field_name]) - except ValueError as e: - raise ValueError(f"Error validating field '{field_name}': {e}") from e - - setattr(self, field_name, field_value) - - # Retain any additional fields - self.extra_fields = extra or {} + self.code = _validators.str_("code", code or self.DEFAULT_CODE) + self.authorization_parameters = _validators.instance_or_dict( + "authorization_parameters", + authorization_parameters, + LegacyAuthorizationParameters, + ) + self.extra = extra or {} def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: """ @@ -305,5 +194,5 @@ def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: return GlobusAuthRequirementsError( authorization_parameters=authorization_parameters, code=self.code, - extra=self.extra_fields, + extra=self.extra, ) diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py index 42118413b..f47781d38 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py +++ b/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py @@ -2,12 +2,10 @@ import typing as t -from globus_sdk.exc import GlobusError +from . import _serializable, _validators -from . import validators - -class GlobusAuthorizationParameters: +class GlobusAuthorizationParameters(_serializable.Serializable): """ Represents authorization parameters that can be used to instruct a client which additional authorizations are needed in order to complete a request. @@ -33,28 +31,11 @@ class GlobusAuthorizationParameters: :ivar required_scopes: A list of scopes for which consent is required. :vartype required_scopes: list of str, optional - :ivar extra_fields: A dictionary of additional fields that were provided. May + :ivar extra: A dictionary of additional fields that were provided. May be used for forward/backward compatibility. - :vartype extra_fields: dict + :vartype extra: dict """ - session_message: str | None - session_required_identities: list[str] | None - session_required_policies: list[str] | None - session_required_single_domain: list[str] | None - session_required_mfa: bool | None - required_scopes: list[str] | None - extra_fields: dict[str, t.Any] - - SUPPORTED_FIELDS = { - "session_message": validators.OptionalString, - "session_required_identities": validators.OptionalListOfStrings, - "session_required_policies": validators.OptionalListOfStrings, - "session_required_single_domain": validators.OptionalListOfStrings, - "session_required_mfa": validators.OptionalBool, - "required_scopes": validators.OptionalListOfStrings, - } - def __init__( self, *, @@ -65,68 +46,39 @@ def __init__( session_required_mfa: bool | None = None, required_scopes: list[str] | None = None, extra: dict[str, t.Any] | None = None, - ): # pylint: disable=unused-argument - # Validate and assign supported fields - for field_name, validator in self.SUPPORTED_FIELDS.items(): - try: - field_value = validator(locals()[field_name]) - except ValueError as e: - raise ValueError(f"Error validating field '{field_name}': {e}") from e - - setattr(self, field_name, field_value) - - self.extra_fields = extra or {} + ): + self.session_message = _validators.opt_str("session_message", session_message) + self.session_required_identities = _validators.opt_str_list( + "session_required_identities", session_required_identities + ) + self.session_required_policies = _validators.opt_str_list( + "session_required_policies", session_required_policies + ) + self.session_required_single_domain = _validators.opt_str_list( + "session_required_single_domain", session_required_single_domain + ) + self.session_required_mfa = _validators.opt_bool( + "session_required_mfa", session_required_mfa + ) + self.required_scopes = _validators.opt_str_list( + "required_scopes", required_scopes + ) + self.extra = extra or {} # Enforce that the error contains at least one of the fields we expect + requires_at_least_one = [ + name for name in self._supported_fields() if name != "session_message" + ] if all( - getattr(self, field_name) is None for field_name in self.SUPPORTED_FIELDS + getattr(self, field_name) is None for field_name in requires_at_least_one ): - raise ValueError( + raise _validators.ValidationError( "Must include at least one supported authorization parameter: " - + ", ".join(self.SUPPORTED_FIELDS.keys()) + + ", ".join(requires_at_least_one) ) - @classmethod - def from_dict(cls, param_dict: dict[str, t.Any]) -> GlobusAuthorizationParameters: - """ - Instantiate from an authorization parameters dictionary. - - :param param_dict: The dictionary to create the error from. - :type param_dict: dict - """ - - # Extract any extra fields - extras = {k: v for k, v in param_dict.items() if k not in cls.SUPPORTED_FIELDS} - kwargs: dict[str, t.Any] = {"extra": extras} - # Ensure required fields are supplied - for field_name in cls.SUPPORTED_FIELDS.keys(): - kwargs[field_name] = param_dict.get(field_name) - - return cls(**kwargs) - - def to_dict(self, include_extra: bool = False) -> dict[str, t.Any]: - """ - Return an authorization parameters dictionary. - :param include_extra: Whether to include stored extra (non-standard) fields in - the returned dictionary. - :type include_extra: bool - """ - error_dict = {} - - # Set any authorization parameters - for field in self.SUPPORTED_FIELDS.keys(): - if getattr(self, field) is not None: - error_dict[field] = getattr(self, field) - - # Set any extra fields - if include_extra: - error_dict.update(self.extra_fields) - - return error_dict - - -class GlobusAuthRequirementsError(GlobusError): +class GlobusAuthRequirementsError(_serializable.Serializable): """ Represents a Globus Auth Requirements Error. @@ -141,81 +93,22 @@ class GlobusAuthRequirementsError(GlobusError): :ivar authorization_parameters: The authorization parameters for this error. :vartype authorization_parameters: GlobusAuthorizationParameters - :ivar extra_fields: A dictionary of additional fields that were provided. May + :ivar extra: A dictionary of additional fields that were provided. May be used for forward/backward compatibility. - :vartype extra_fields: dict + :vartype extra: dict """ - code: str - authorization_parameters: GlobusAuthorizationParameters - extra_fields: dict[str, t.Any] - - SUPPORTED_FIELDS = { - "code": validators.String, - "authorization_parameters": validators.ClassInstance( - GlobusAuthorizationParameters - ), - } - def __init__( self, - code: str, # pylint: disable=unused-argument + code: str, authorization_parameters: dict[str, t.Any] | GlobusAuthorizationParameters, *, extra: dict[str, t.Any] | None = None, ): - # Convert authorization_parameters if it's a dict - if isinstance(authorization_parameters, dict): - authorization_parameters = GlobusAuthorizationParameters.from_dict( - param_dict=authorization_parameters - ) - - # Validate and assign supported fields - for field_name, validator in self.SUPPORTED_FIELDS.items(): - try: - field_value = validator(locals()[field_name]) - except ValueError as e: - raise ValueError(f"Error validating field '{field_name}': {e}") from e - - setattr(self, field_name, field_value) - - self.extra_fields = extra or {} - - @classmethod - def from_dict(cls, error_dict: dict[str, t.Any]) -> GlobusAuthRequirementsError: - """ - Instantiate a GlobusAuthRequirementsError from a dictionary. - - :param error_dict: The dictionary to create the error from. - :type error_dict: dict - """ - - # Extract any extra fields - extras = {k: v for k, v in error_dict.items() if k not in cls.SUPPORTED_FIELDS} - kwargs: dict[str, t.Any] = {"extra": extras} - # Ensure required fields are supplied - for field_name in cls.SUPPORTED_FIELDS.keys(): - kwargs[field_name] = error_dict.get(field_name) - - return cls(**kwargs) - - def to_dict(self, include_extra: bool = False) -> dict[str, t.Any]: - """ - Return a Globus Auth Requirements Error response dictionary. - - :param include_extra: Whether to include stored extra (non-standard) fields - in the dictionary. - :type include_extra: bool, optional (default: False) - """ - error_dict = { - "code": self.code, - "authorization_parameters": self.authorization_parameters.to_dict( - include_extra=include_extra - ), - } - - # Set any extra fields - if include_extra: - error_dict.update(self.extra_fields) - - return error_dict + self.code = _validators.str_("code", code) + self.authorization_parameters = _validators.instance_or_dict( + "authorization_parameters", + authorization_parameters, + GlobusAuthorizationParameters, + ) + self.extra = extra or {} diff --git a/src/globus_sdk/experimental/auth_requirements_error/utils.py b/src/globus_sdk/experimental/auth_requirements_error/utils.py index 92d9c6bb1..8d2536599 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/utils.py +++ b/src/globus_sdk/experimental/auth_requirements_error/utils.py @@ -5,6 +5,7 @@ from globus_sdk.exc import ErrorSubdocument, GlobusAPIError +from . import _validators from ._variants import ( LegacyAuthorizationParametersError, LegacyAuthRequirementsErrorVariant, @@ -56,10 +57,10 @@ def to_auth_requirements_error( # Prefer a proper auth requirements error, if possible try: return GlobusAuthRequirementsError.from_dict(error_dict) - except ValueError as err: + except _validators.ValidationError as err: log.debug(f"Failed to parse error as 'GlobusAuthRequirementsError' ({err})") - supported_variants: list[t.Type[LegacyAuthRequirementsErrorVariant]] = [ + supported_variants: list[type[LegacyAuthRequirementsErrorVariant]] = [ LegacyAuthorizationParametersError, LegacyConsentRequiredTransferError, LegacyConsentRequiredAPError, @@ -67,7 +68,7 @@ def to_auth_requirements_error( for variant in supported_variants: try: return variant.from_dict(error_dict).to_auth_requirements_error() - except ValueError as err: + except _validators.ValidationError as err: log.debug(f"Failed to parse error as '{variant.__name__}' ({err})") return None diff --git a/src/globus_sdk/experimental/auth_requirements_error/validators.py b/src/globus_sdk/experimental/auth_requirements_error/validators.py deleted file mode 100644 index 3e824d363..000000000 --- a/src/globus_sdk/experimental/auth_requirements_error/validators.py +++ /dev/null @@ -1,103 +0,0 @@ -from __future__ import annotations - -import typing as t -from contextlib import suppress - -T = t.TypeVar("T") - - -def _string(value: t.Any) -> str: - if not isinstance(value, str): - raise ValueError("Must be a string") - - return value - - -def _string_literal(literal: str) -> t.Callable[[t.Any], str]: - def validator(value: t.Any) -> str: - value = _string(value) - if value != literal: - raise ValueError(f"Must be '{literal}'") - - return t.cast(str, value) - - return validator - - -def _class_instance(cls: t.Type[T]) -> t.Callable[[t.Any], T]: - def validator(value: t.Any) -> T: - if not isinstance(value, cls): - raise ValueError(f"Must be an instance of {cls.__name__}") - - return value - - return validator - - -def _list_of_strings(value: t.Any) -> list[str]: - if not (isinstance(value, list) and all(isinstance(v, str) for v in value)): - raise ValueError("Must be a list of strings") - - return value - - -def _comma_delimited_strings(value: t.Any) -> list[str]: - if not isinstance(value, str): - raise ValueError("Must be a comma-delimited string") - - return value.split(",") - - -def _boolean(value: t.Any) -> bool: - if not isinstance(value, bool): - raise ValueError("Must be a bool") - - return value - - -def _null(value: t.Any) -> None: - if value is not None: - raise ValueError("Must be null") - - return None - - -def _anyof( - validators: tuple[t.Callable[..., t.Any], ...], - description: str, -) -> t.Callable[..., t.Any]: - def _anyof_validator(value: t.Any) -> t.Any: - for validator in validators: - with suppress(ValueError): - return validator(value) - - raise ValueError(f"Must be {description}") - - return _anyof_validator - - -String: t.Callable[[t.Any], str] = _string -StringLiteral: t.Callable[[str], t.Callable[[t.Any], str]] = _string_literal -ClassInstance: t.Callable[[t.Any], t.Any] = _class_instance -ListOfStrings: t.Callable[[t.Any], list[str]] = _list_of_strings -CommaDelimitedStrings: t.Callable[[t.Any], list[str]] = _comma_delimited_strings -Bool: t.Callable[[t.Any], bool] = _boolean -Null: t.Callable[[t.Any], None] = _null -OptionalString: t.Callable[[t.Any], str | None] = _anyof( - (_string, _null), description="a string or null" -) -OptionalListOfStrings: t.Callable[[t.Any], list[str] | None] = _anyof( - (_list_of_strings, _null), description="a list of strings or null" -) -OptionalCommaDelimitedStrings: t.Callable[[t.Any], list[str] | None] = _anyof( - (_comma_delimited_strings, _null), description="a comma-delimited string or null" -) -OptionalListOfStringsOrCommaDelimitedStrings: t.Callable[ - [t.Any], list[str] | None -] = _anyof( - (_list_of_strings, _comma_delimited_strings, _null), - description="a list of strings, a comma-delimited string, or null", -) -OptionalBool: t.Callable[[t.Any], bool | None] = _anyof( - (_boolean, _null), description="a bool or null" -) diff --git a/tests/unit/experimental/test_auth_requirements_error.py b/tests/unit/experimental/test_auth_requirements_error.py index 9c0463bd6..31298b64f 100644 --- a/tests/unit/experimental/test_auth_requirements_error.py +++ b/tests/unit/experimental/test_auth_requirements_error.py @@ -1,5 +1,3 @@ -import inspect - import pytest from globus_sdk._testing import construct_error @@ -408,42 +406,37 @@ def test_backward_compatibility_consent_required_error(): @pytest.mark.parametrize( - "target_class", - [ - GlobusAuthRequirementsError, - GlobusAuthorizationParameters, - _variants.LegacyAuthorizationParameters, - _variants.LegacyAuthorizationParametersError, - _variants.LegacyConsentRequiredTransferError, - _variants.LegacyConsentRequiredAPError, - ], -) -def test_constructors_include_all_supported_fields(target_class): - """ - Test that all supported fields are included in the constructors. - """ - - method_sig = inspect.signature(target_class.__init__) - for field_name in target_class.SUPPORTED_FIELDS: - # Make sure the constructor has a parameter for this field - assert field_name in method_sig.parameters - - -@pytest.mark.parametrize( - "target_class, field_name", + "target_class, data, expect_message", [ - (GlobusAuthRequirementsError, "code"), - (_variants.LegacyAuthorizationParametersError, "authorization_parameters"), - (_variants.LegacyConsentRequiredTransferError, "code"), - (_variants.LegacyConsentRequiredAPError, "code"), + ( # missing 'code' + GlobusAuthRequirementsError, + {"authorization_parameters": {"session_required_policies": "foo"}}, + "'code' must be a string", + ), + ( # missing 'authorization_parameters' + _variants.LegacyAuthorizationParametersError, + {}, + "'authorization_parameters' must be a 'LegacyAuthorizationParameters' " + "object or a dictionary", + ), + ( # missing 'code' + _variants.LegacyConsentRequiredTransferError, + {"required_scopes": []}, + "'code' must be the string 'ConsentRequired'", + ), + ( # missing 'code' + _variants.LegacyConsentRequiredAPError, + {"required_scope": "foo"}, + "'code' must be the string 'ConsentRequired'", + ), ], ) -def test_error_from_dict_insufficient_input(target_class, field_name): +def test_error_from_dict_insufficient_input(target_class, data, expect_message): """ """ with pytest.raises(ValueError) as exc_info: - target_class.from_dict({}) + target_class.from_dict(data) - assert f"Error validating field '{field_name}'" in str(exc_info.value) + assert str(exc_info.value) == expect_message @pytest.mark.parametrize( From bd735e1d2a2c7a5a9f33fd6f098dd7699b8a39e2 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 7 Aug 2023 13:49:56 -0500 Subject: [PATCH 27/32] Update src/globus_sdk/experimental/auth_requirements_error/_serializable.py Co-authored-by: Ada <107940310+ada-globus@users.noreply.github.com> --- .../experimental/auth_requirements_error/_serializable.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_serializable.py b/src/globus_sdk/experimental/auth_requirements_error/_serializable.py index c1931fc1f..4ae2a7034 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_serializable.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_serializable.py @@ -7,14 +7,14 @@ class Serializable: - _EXLUDE_VARS: t.ClassVar[tuple[str, ...]] = ("self", "extra_fields", "extra") + _EXCLUDE_VARS: t.ClassVar[tuple[str, ...]] = ("self", "extra") extra: dict[str, t.Any] @classmethod def _supported_fields(cls) -> list[str]: signature = inspect.signature(cls.__init__) return [ - name for name in signature.parameters.keys() if name not in cls._EXLUDE_VARS + name for name in signature.parameters.keys() if name not in cls._EXCLUDE_VARS ] @classmethod From ac86722acd2f17d9a92ff23eb3a9aed69e209aaf Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 7 Aug 2023 18:50:10 +0000 Subject: [PATCH 28/32] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../experimental/auth_requirements_error/_serializable.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_serializable.py b/src/globus_sdk/experimental/auth_requirements_error/_serializable.py index 4ae2a7034..c976e7e61 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_serializable.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_serializable.py @@ -14,7 +14,9 @@ class Serializable: def _supported_fields(cls) -> list[str]: signature = inspect.signature(cls.__init__) return [ - name for name in signature.parameters.keys() if name not in cls._EXCLUDE_VARS + name + for name in signature.parameters.keys() + if name not in cls._EXCLUDE_VARS ] @classmethod From 8b6ca104405de782a37555716d9c4c369c9fbad9 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 7 Aug 2023 14:29:48 -0500 Subject: [PATCH 29/32] Minor doc adjustment extra_fields->extra --- docs/experimental/auth_requirements_errors.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/experimental/auth_requirements_errors.rst b/docs/experimental/auth_requirements_errors.rst index 1809b8a3c..b2605bde6 100644 --- a/docs/experimental/auth_requirements_errors.rst +++ b/docs/experimental/auth_requirements_errors.rst @@ -62,7 +62,7 @@ by specifying ``include_extra=True`` when calling ``to_dict()``. error.to_dict(include_extra=True) These fields are stored by both the ``GlobusAuthRequirementsError`` and -``GlobusAuthenticationParameters`` classes in an ``extra_fields`` attribute. +``GlobusAuthenticationParameters`` classes in an ``extra`` attribute. .. note:: @@ -136,4 +136,4 @@ supported field is supplied with a value of the wrong type. any logic specific to the Globus Auth service with regard to what represents a valid combination of fields (e.g., ``session_required_mfa`` requires either ``session_required_identities`` or ``session_required_single_domain`` -in order to be properly handled). \ No newline at end of file +in order to be properly handled). From ca2b0b36886e47c2a7ef282256a911b1ccfb513b Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 7 Aug 2023 14:38:09 -0500 Subject: [PATCH 30/32] Relocate validation helper --- .../auth_requirements_error/_validators.py | 12 ------------ .../auth_requirements_error/_variants.py | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_validators.py b/src/globus_sdk/experimental/auth_requirements_error/_validators.py index 69a054cb6..04987a2af 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_validators.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_validators.py @@ -1,15 +1,9 @@ from __future__ import annotations -import sys import typing as t from ._serializable import Serializable -if sys.version_info >= (3, 8): - from typing import Literal -else: - from typing_extensions import Literal - S = t.TypeVar("S", bound=Serializable) @@ -31,12 +25,6 @@ def opt_str(name: str, value: t.Any) -> str | None: return value -def consent_required_literal(name: str, value: t.Any) -> Literal["ConsentRequired"]: - if not isinstance(value, str) or value != "ConsentRequired": - raise ValidationError(f"'{name}' must be the string 'ConsentRequired'") - return t.cast(Literal["ConsentRequired"], value) - - def opt_bool(name: str, value: t.Any) -> bool | None: if value is None: return None diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index 3bf21ec40..cd81da1c9 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -42,7 +42,7 @@ def __init__( required_scopes: list[str], extra: dict[str, t.Any] | None = None, ): - self.code = _validators.consent_required_literal("code", code) + self.code = _validate_consent_required_literal("code", code) self.required_scopes = _validators.str_list("required_scopes", required_scopes) self.extra = extra or {} @@ -73,7 +73,7 @@ def __init__( required_scope: str, extra: dict[str, t.Any] | None, ): - self.code = _validators.consent_required_literal("code", code) + self.code = _validate_consent_required_literal("code", code) self.required_scope = _validators.str_("required_scope", required_scope) self.extra = extra or {} @@ -196,3 +196,13 @@ def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: code=self.code, extra=self.extra, ) + + +def _validate_consent_required_literal( + name: str, value: t.Any +) -> Literal["ConsentRequired"]: + if not isinstance(value, str) or value != "ConsentRequired": + raise _validators.ValidationError( + f"'{name}' must be the string 'ConsentRequired'" + ) + return t.cast(Literal["ConsentRequired"], value) From 9c2fe1acd64da5dd0322e767091b1f5c595acbc2 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 7 Aug 2023 15:06:01 -0500 Subject: [PATCH 31/32] Invert ordering of GARE validators This makes these more uniform. --- .../auth_requirements_error/__init__.py | 2 + .../auth_requirements_error/_validators.py | 40 +++++++++---------- .../auth_requirements_error/_variants.py | 8 ++-- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/globus_sdk/experimental/auth_requirements_error/__init__.py b/src/globus_sdk/experimental/auth_requirements_error/__init__.py index 7376a9b2d..012feedf7 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/__init__.py +++ b/src/globus_sdk/experimental/auth_requirements_error/__init__.py @@ -1,3 +1,4 @@ +from ._validators import ValidationError from .auth_requirements_error import ( GlobusAuthorizationParameters, GlobusAuthRequirementsError, @@ -10,6 +11,7 @@ ) __all__ = [ + "ValidationError", "GlobusAuthRequirementsError", "GlobusAuthorizationParameters", "to_auth_requirements_error", diff --git a/src/globus_sdk/experimental/auth_requirements_error/_validators.py b/src/globus_sdk/experimental/auth_requirements_error/_validators.py index 04987a2af..c1873e62d 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_validators.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_validators.py @@ -12,39 +12,37 @@ class ValidationError(ValueError): def str_(name: str, value: t.Any) -> str: - if not isinstance(value, str): - raise ValidationError(f"'{name}' must be a string") - return value + if isinstance(value, str): + return value + raise ValidationError(f"'{name}' must be a string") def opt_str(name: str, value: t.Any) -> str | None: if value is None: return None - if not isinstance(value, str): - raise ValidationError(f"'{name}' must be a string") - return value + if isinstance(value, str): + return value + raise ValidationError(f"'{name}' must be a string or null") def opt_bool(name: str, value: t.Any) -> bool | None: - if value is None: - return None - if not isinstance(value, bool): - raise ValidationError(f"'{name}' must be a bool") - return value + if value is None or isinstance(value, bool): + return value + raise ValidationError(f"'{name}' must be a bool or null") def str_list(name: str, value: t.Any) -> list[str]: - if not (isinstance(value, list) and all(isinstance(s, str) for s in value)): - raise ValidationError(f"'{name}' must be a list of strings") - return value + if isinstance(value, list) and all(isinstance(s, str) for s in value): + return value + raise ValidationError(f"'{name}' must be a list of strings") def opt_str_list(name: str, value: t.Any) -> list[str] | None: if value is None: return None - if not (isinstance(value, list) and all(isinstance(s, str) for s in value)): - raise ValidationError(f"'{name}' must be a list of strings") - return value + if isinstance(value, list) and all(isinstance(s, str) for s in value): + return value + raise ValidationError(f"'{name}' must be a list of strings or null") def opt_str_list_or_commasep(name: str, value: t.Any) -> list[str] | None: @@ -52,9 +50,11 @@ def opt_str_list_or_commasep(name: str, value: t.Any) -> list[str] | None: return None if isinstance(value, str): value = value.split(",") - if not (isinstance(value, list) and all(isinstance(s, str) for s in value)): - raise ValidationError(f"'{name}' must be a list of strings") - return value + if isinstance(value, list) and all(isinstance(s, str) for s in value): + return value + raise ValidationError( + f"'{name}' must be a list of strings or a comma-delimited string or null" + ) def instance_or_dict(name: str, value: t.Any, cls: type[S]) -> S: diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index cd81da1c9..488b74476 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -201,8 +201,6 @@ def to_auth_requirements_error(self) -> GlobusAuthRequirementsError: def _validate_consent_required_literal( name: str, value: t.Any ) -> Literal["ConsentRequired"]: - if not isinstance(value, str) or value != "ConsentRequired": - raise _validators.ValidationError( - f"'{name}' must be the string 'ConsentRequired'" - ) - return t.cast(Literal["ConsentRequired"], value) + if value == "ConsentRequired": + return "ConsentRequired" + raise _validators.ValidationError(f"'{name}' must be the string 'ConsentRequired'") From 70cd92907b9cac5ca08a838d7fa568f16e9fc6b1 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 7 Aug 2023 15:26:48 -0500 Subject: [PATCH 32/32] Rename modules per discussion --- .../experimental/auth_requirements_error/__init__.py | 6 +++--- ...th_requirements_error.py => _auth_requirements_error.py} | 0 .../{utils.py => _functional_api.py} | 2 +- .../experimental/auth_requirements_error/_variants.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename src/globus_sdk/experimental/auth_requirements_error/{auth_requirements_error.py => _auth_requirements_error.py} (100%) rename src/globus_sdk/experimental/auth_requirements_error/{utils.py => _functional_api.py} (98%) diff --git a/src/globus_sdk/experimental/auth_requirements_error/__init__.py b/src/globus_sdk/experimental/auth_requirements_error/__init__.py index 012feedf7..a5ca35507 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/__init__.py +++ b/src/globus_sdk/experimental/auth_requirements_error/__init__.py @@ -1,14 +1,14 @@ -from ._validators import ValidationError -from .auth_requirements_error import ( +from ._auth_requirements_error import ( GlobusAuthorizationParameters, GlobusAuthRequirementsError, ) -from .utils import ( +from ._functional_api import ( has_auth_requirements_errors, is_auth_requirements_error, to_auth_requirements_error, to_auth_requirements_errors, ) +from ._validators import ValidationError __all__ = [ "ValidationError", diff --git a/src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py b/src/globus_sdk/experimental/auth_requirements_error/_auth_requirements_error.py similarity index 100% rename from src/globus_sdk/experimental/auth_requirements_error/auth_requirements_error.py rename to src/globus_sdk/experimental/auth_requirements_error/_auth_requirements_error.py diff --git a/src/globus_sdk/experimental/auth_requirements_error/utils.py b/src/globus_sdk/experimental/auth_requirements_error/_functional_api.py similarity index 98% rename from src/globus_sdk/experimental/auth_requirements_error/utils.py rename to src/globus_sdk/experimental/auth_requirements_error/_functional_api.py index 8d2536599..07a3f4ef2 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/utils.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_functional_api.py @@ -6,13 +6,13 @@ from globus_sdk.exc import ErrorSubdocument, GlobusAPIError from . import _validators +from ._auth_requirements_error import GlobusAuthRequirementsError from ._variants import ( LegacyAuthorizationParametersError, LegacyAuthRequirementsErrorVariant, LegacyConsentRequiredAPError, LegacyConsentRequiredTransferError, ) -from .auth_requirements_error import GlobusAuthRequirementsError log = logging.getLogger(__name__) diff --git a/src/globus_sdk/experimental/auth_requirements_error/_variants.py b/src/globus_sdk/experimental/auth_requirements_error/_variants.py index 488b74476..ac029fb6f 100644 --- a/src/globus_sdk/experimental/auth_requirements_error/_variants.py +++ b/src/globus_sdk/experimental/auth_requirements_error/_variants.py @@ -4,7 +4,7 @@ import typing as t from . import _serializable, _validators -from .auth_requirements_error import ( +from ._auth_requirements_error import ( GlobusAuthorizationParameters, GlobusAuthRequirementsError, )