From 7fdc7179e145b189682eb71a286e98f8927ed049 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Fri, 4 Nov 2022 03:39:27 +0000 Subject: [PATCH 01/10] Add scope parsing to Scope object The core of this change is that `MutableScope` has been renamed to `Scope` and now has a pair of parsing functions as classmethods: `Scope.parse` and `Scope.deserialize`. The first parses any valid scope string to request, meaning it may be a space delimited list of scopes, and the second parses any valid scope string with a single top-level scope. As an inverse of `deserialize()`, the stringifier has been renamed to `serialize()`, and `__str__()` now just calls `serialize()`. The parsing itself requires several new internal components. A tokenizer handles converting strings into lists of tokens, e.g. converting `foo[bar *baz]` into `["foo", "[", "bar", "*", "baz", "]"]`. The tokens are small objects pairing strings with a type, which makes it simpler for the parser to handle various cases. The parser is minimally stateful, has no recursion or backtracking, operates with limited lookahead, and is generally meant to be the smallest and simplest thing which can do the job that it does. The parse result, a `Scope`, can now represent the true tree-like structure of scopes. `Scope.dependencies` is a list of child nodes in the tree, a list of `Scope`s. This has implicitations for `Scope.add_dependency` in that a `Scope` is now a valid input. As a result of this introduction, the use of `add_dependency("foo", optional=True)` is considered deprecated and now emits a DeprecationWarning. Building dependencies in the current version should be done either with (newly parseable) strings or with `Scope` objects. i.e. The aforementioned usage can be replaced with either `add_dependency("*foo")` or `add_dependency(Scope("foo", optional=True))`. Some related changes included here: - `globus_sdk.scopes` is now a subpackage (it was getting a bit long) - the internal type alias for ScopeCollectionType is now kept in `_types` for simpler universal access within the SDK - many more unit tests are added for the scope parsing and serialization functionality - documentation is updated to refer to these as "Scope objects" rather than "MutableScopes" - various doc fixes, including missing scope data in the scopes docs and a bug in the listknownscopes custom directive --- ...21104_145601_sirosen_add_scope_parsing.rst | 17 + docs/scopes.rst | 141 +++++- src/globus_sdk/_types.py | 12 + .../authorizers/client_credentials.py | 5 +- src/globus_sdk/scopes.py | 429 ------------------ src/globus_sdk/scopes/__init__.py | 39 ++ src/globus_sdk/scopes/builder.py | 159 +++++++ src/globus_sdk/scopes/data.py | 176 +++++++ src/globus_sdk/scopes/scope_definition.py | 241 ++++++++++ .../auth/flow_managers/authorization_code.py | 3 +- .../services/auth/flow_managers/native_app.py | 3 +- tests/unit/test_scopes.py | 172 +++++-- 12 files changed, 895 insertions(+), 502 deletions(-) create mode 100644 changelog.d/20221104_145601_sirosen_add_scope_parsing.rst delete mode 100644 src/globus_sdk/scopes.py create mode 100644 src/globus_sdk/scopes/__init__.py create mode 100644 src/globus_sdk/scopes/builder.py create mode 100644 src/globus_sdk/scopes/data.py create mode 100644 src/globus_sdk/scopes/scope_definition.py diff --git a/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst b/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst new file mode 100644 index 000000000..080042cb7 --- /dev/null +++ b/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst @@ -0,0 +1,17 @@ +* Enhance scope utilities with scope parsing, attached to + ``globus_sdk.scopes.Scope`` (:pr:`NUMBER`) + + * ``MutableScope`` has been renamed to ``Scope``. Both names remain available + for backwards compatibility, but the preferred name is now ``Scope`` + + * ``Scope.parse`` and ``Scope.deserialize`` can now be used to parse strings + into ``Scope``\s + + * ``Scope(...).serialize()`` is added, and ``str(Scope(...))`` uses it + + * ``Scope.add_dependency`` now supports ``Scope`` objects as inputs + + * The ``optional`` argument to ``add_dependency`` is deprecated. + ``Scope(...).add_dependency("*foo")`` can be used to add an optional + dependency as a string, or equivalently + ``Scope(...).add_dependency(Scope("foo", optional=True))`` diff --git a/docs/scopes.rst b/docs/scopes.rst index d81c865a1..ccf37ffd0 100644 --- a/docs/scopes.rst +++ b/docs/scopes.rst @@ -1,5 +1,7 @@ .. _scopes: +.. currentmodule:: globus_sdk.scopes + Scopes and ScopeBuilders ======================== @@ -76,16 +78,16 @@ To elaborate on the above example: # data from the response tokendata = token_response.by_resource_server[TransferScopes.resource_server] -MutableScope objects --------------------- +Scope objects +------------- In order to support optional and dependent scopes, an additional type is -provided by ``globus_sdk.scopes``: the ``MutableScope`` class. +provided by ``globus_sdk.scopes``: the ``Scope`` class. -``MutableScope`` can be constructed directly, from full scope strings, or via a +``Scope`` can be constructed directly, from full scope strings, or via a ``ScopeBuilder``'s ``make_mutable`` method, given a scope's short name. -For example, one can create a ``MutableScope`` from the Groups "all" scope as +For example, one can create a ``Scope`` from the Groups "all" scope as follows: .. code-block:: python @@ -94,11 +96,65 @@ follows: scope = GroupsScopes.make_mutable("all") -MutableScopes provide the most value when handling scope dependencies. For -example, given a Globus Connect Server Mapped Collection, it may be desirable -to construct a "data_access" scope as an optional dependency for the Transfer -Scope. To do so, one first creates the mutable scope object, then adds the -dependency to it: +``Scope`` objects primarily provide three main pieces of functionality: +parsing from a string, dynamically building a scope tree, and serializing to a +string. + +Scope Parsing +~~~~~~~~~~~~~ + +:meth:`Scope.parse` is the primary parsing method. Given a string, parsing may +produce a list of scopes. The reason for this is that a scope string being +requested may be a space-delimited set of scopes. For example, the following +parse is desirable: + +.. code-block:: pycon + + >>> Scope.parse("openid urn:globus:auth:scopes:transfer.api.globus.org:all") + [ + Scope("openid"), + Scope("urn:globus:auth:scopes:transfer.api.globus.org:all"), + ] + +Additionally, scopes can be deserialized from strings with +:meth:`Scope.deserialize`. This is similar to ``parse``, but it must return +exactly one scope. For example, + +.. code-block:: pycon + + >>> Scope.deserialize("urn:globus:auth:scopes:transfer.api.globus.org:all") + Scope("urn:globus:auth:scopes:transfer.api.globus.org:all") + +Parsing supports scopes with dependencies and optional scopes denoted by the +``*`` marker. Therefore, the following is also a valid parse: + +.. code-block:: pycon + + >>> transfer_scope = "urn:globus:auth:scopes:transfer.api.globus.org:all" + >>> collection_scope = ( + ... "https://auth.globus.org/scopes/c855676f-7840-4630-9b16-ef260aaf02c3/data_access" + ... ) + >>> Scope.deserialize(f"{transfer_scope}[*{collection_scope}]") + Scope( + "urn:globus:auth:scopes:transfer.api.globus.org:all", + dependencies=[ + Scope( + "https://auth.globus.org/scopes/c855676f-7840-4630-9b16-ef260aaf02c3/data_access", + optional=True + ) + ] + ) + +Dynamic Scope Construction +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +In the parsing example above, a scope string was constructed as a format string +which was then parsed into a complex dependent scope structure. This can be +done directly, without needing to encode the scope as a string beforehand. + +For example, the same transfer scope dependent upon a collection scope may be +constructed by means of ``Scope`` methods and the ``make_mutable`` method of +scope builders: .. code-block:: python @@ -106,19 +162,51 @@ dependency to it: MAPPED_COLLECTION_ID = "...ID HERE..." + # create the scopes with make_mutable transfer_scope = TransferScopes.make_mutable("all") - transfer_scope.add_dependency( - GCSCollectionScopeBuilder(MAPPED_COLLECTION_ID).data_access, optional=True + data_access_scope = GCSCollectionScopeBuilder(MAPPED_COLLECTION_ID).make_mutable( + "data_access" ) + # set the data_access scope to be optional + data_access_scope.optional = True + # add data_access as a dependency + transfer_scope.add_dependency(data_access_scope) -``MutableScope``\s can be used in most of the same locations where scope +``Scope``\s can be used in most of the same locations where scope strings can be used, but you can also call ``str()`` on them to get a stringified representation. -ScopeBuilder Types and Constants --------------------------------- +Serializing Scopes +~~~~~~~~~~~~~~~~~~ -.. module:: globus_sdk.scopes +Whenever scopes are being sent to Globus services, they need to be encoded as +strings. All scope objects support this by means of their defined ``serialize`` +method. Note that ``__str__`` for a ``Scope`` is just an alias for +``serialize``. For example, the following is valid usage to demonstrate +``str()``, ``repr()``, and ``serialize()``: + +.. code-block:: pycon + + >>> from globus_sdk.scopes import Scope + >>> str(Scope("foo").add_dependency("bar").add_dependency("*baz")) + foo[bar *baz] + >>> Scope("foo").add_dependency("bar").serialize() + foo[bar] + >>> repr(Scope("foo").add_dependency("*bar")) + Scope("foo", dependencies=[Scope("bar", optional=True)]) + +Scope Reference +~~~~~~~~~~~~~~~ + +.. autoclass:: Scope + :members: + :show-inheritance: + +ScopeBuilders +------------- + +ScopeBuilder Types +~~~~~~~~~~~~~~~~~~ .. autoclass:: ScopeBuilder :members: @@ -132,21 +220,26 @@ ScopeBuilder Types and Constants :members: :show-inheritance: -.. autoclass:: MutableScope - :members: - :show-inheritance: +ScopeBuilder Constants +~~~~~~~~~~~~~~~~~~~~~~ + +.. autodata:: globus_sdk.scopes.data.AuthScopes + :annotation: + +.. autodata:: globus_sdk.scopes.data.FlowsScopes + :annotation: -.. autodata:: AuthScopes +.. autodata:: globus_sdk.scopes.data.GroupsScopes :annotation: -.. autodata:: GroupsScopes +.. autodata:: globus_sdk.scopes.data.NexusScopes :annotation: -.. autodata:: NexusScopes +.. autodata:: globus_sdk.scopes.data.SearchScopes :annotation: -.. autodata:: SearchScopes +.. autodata:: globus_sdk.scopes.data.TimerScopes :annotation: -.. autodata:: globus_sdk.scopes.TransferScopes +.. autodata:: globus_sdk.scopes.data.TransferScopes :annotation: diff --git a/src/globus_sdk/_types.py b/src/globus_sdk/_types.py index 3f362ed11..d11270314 100644 --- a/src/globus_sdk/_types.py +++ b/src/globus_sdk/_types.py @@ -2,6 +2,18 @@ import typing as t import uuid +if t.TYPE_CHECKING: + from globus_sdk.scopes import Scope + +# these types are aliases meant for internal use IntLike = t.Union[int, str] UUIDLike = t.Union[uuid.UUID, str] DateLike = t.Union[str, datetime.datetime] + +ScopeCollectionType = t.Union[ + str, + "Scope", + t.Iterable[str], + t.Iterable["Scope"], + t.Iterable[t.Union[str, "Scope"]], +] diff --git a/src/globus_sdk/authorizers/client_credentials.py b/src/globus_sdk/authorizers/client_credentials.py index cf37d13cc..2345a7f22 100644 --- a/src/globus_sdk/authorizers/client_credentials.py +++ b/src/globus_sdk/authorizers/client_credentials.py @@ -1,7 +1,8 @@ import logging import typing as t -from globus_sdk.scopes import MutableScope, _ScopeCollectionType +from globus_sdk._types import ScopeCollectionType +from globus_sdk.scopes import MutableScope if t.TYPE_CHECKING: from globus_sdk.services.auth import ConfidentialAppAuthClient, OAuthTokenResponse @@ -61,7 +62,7 @@ class ClientCredentialsAuthorizer(RenewingAuthorizer): def __init__( self, confidential_client: "ConfidentialAppAuthClient", - scopes: _ScopeCollectionType, + scopes: ScopeCollectionType, *, access_token: t.Optional[str] = None, expires_at: t.Optional[int] = None, diff --git a/src/globus_sdk/scopes.py b/src/globus_sdk/scopes.py deleted file mode 100644 index 480d41029..000000000 --- a/src/globus_sdk/scopes.py +++ /dev/null @@ -1,429 +0,0 @@ -import typing as t - -# this type alias is meant for internal use, which is why it's named with an underscore -_ScopeCollectionType = t.Union[ - str, - "MutableScope", - t.Iterable[str], - t.Iterable["MutableScope"], - t.Iterable[t.Union[str, "MutableScope"]], -] - - -def _iter_scope_collection(obj: _ScopeCollectionType) -> t.Iterator[str]: - if isinstance(obj, str): - yield obj - elif isinstance(obj, MutableScope): - yield str(obj) - else: - for item in obj: - yield str(item) - - -class MutableScope: - """ - A mutable scope is a representation of a scope which allows modifications to be - made. In particular, it supports handling scope dependencies via - ``add_dependency``. A MutableScope can be created - - `str(MutableScope(...))` produces a valid scope string for use in various methods. - - :param scope_string: The scope which will be used as the basis for this MutableScope - :type scope_string: str - :param optional: The scope may be marked as optional. This means that the scope can - be declined by the user without declining consent for other scopes - :type optional: bool - """ - - def __init__(self, scope_string: str, *, optional: bool = False) -> None: - self.optional = optional - self._scope_string = scope_string - # map from scope name to optional=t/f - # this means that dependencies are not ordered, but that adding the same - # dependency twice is a no-op - self._dependencies: t.Dict[str, bool] = {} - - def add_dependency(self, scope: str, *, optional: bool = False) -> "MutableScope": - """ - Add a scope dependency. The dependent scope relationship will be stored in the - MutableScope and will be evident in its string representation. - - :param scope: The scope upon which the current scope depends - :type scope: str - :param optional: Mark the dependency an optional one. By default it is not. An - optional scope dependency can be declined by the user without declining - consent for the primary scope - :type optional: bool, optional - """ - if scope.startswith("*"): - raise ValueError( - "Scope strings for add_dependency cannot contain a leading '*'. " - "To pass an optional scope, first strip any leading '*', and use " - "'optional=True' if the scope must be marked optional." - ) - self._dependencies[scope] = optional - return self - - def __repr__(self) -> str: - parts: t.List[str] = [f"'{self._scope_string}'"] - if self.optional: - parts.append("optional=True") - if self._dependencies: - parts.append(f"dependencies={self._dependencies}") - return "MutableScope(" + ", ".join(parts) + ")" - - def _formatted_dependencies(self) -> t.Iterator[str]: - for scope in self._dependencies: - optional_prefix = "*" if self._dependencies[scope] else "" - yield optional_prefix + scope - - def __str__(self) -> str: - base_scope = ("*" if self.optional else "") + self._scope_string - if not self._dependencies: - return base_scope - return base_scope + "[" + " ".join(self._formatted_dependencies()) + "]" - - @staticmethod - def scopes2str(obj: _ScopeCollectionType) -> str: - """ - Given a scope string, a collection of scope strings, a MutableScope object, a - collection of MutableScope objects, or a mixed collection of strings and - MutableScopes, convert to a string which can be used in a request. - """ - return " ".join(_iter_scope_collection(obj)) - - -ScopeBuilderScopes = t.Union[ - None, str, t.Tuple[str, str], t.List[t.Union[str, t.Tuple[str, str]]] -] - - -class ScopeBuilder: - """ - Utility class for creating scope strings for a specified resource server. - - :param resource_server: The identifier, usually a domain name or a UUID, for the - resource server to return scopes for. - :type resource_server: str - :param known_scopes: A list of scope values or named scope values to pre-populate on - this instance. This will set attributes on the instance using the URN scope - format. - :type known_scopes: list of str or k/v tuples, optional - :param known_url_scopes: A list of scope values or named scope values to - pre-populate on this instance. This will set attributes on the instance using - the URL scope format. - :type known_url_scopes: list of str or k/v tuples, optional - """ - - _classattr_scope_names: t.List[str] = [] - - def __init__( - self, - resource_server: str, - *, - known_scopes: ScopeBuilderScopes = None, - known_url_scopes: ScopeBuilderScopes = None, - ) -> None: - self.resource_server = resource_server - - self._registered_scope_names: t.List[str] = [] - self._register_scopes(known_scopes, self.urn_scope_string) - self._register_scopes(known_url_scopes, self.url_scope_string) - - def _register_scopes( - self, scopes: ScopeBuilderScopes, transform_func: t.Callable[[str], str] - ) -> None: - scopes_dict = self._scopes_input_to_dict(scopes) - for scope_name, scope_val in scopes_dict.items(): - self._registered_scope_names.append(scope_name) - setattr(self, scope_name, transform_func(scope_val)) - - def _scopes_input_to_dict(self, items: ScopeBuilderScopes) -> t.Dict[str, str]: - """ - ScopeBuilders accepts many collection-style types of scopes. This function - normalizes all of those types into a standard {scope_name: scope_val} dict - - Translation Map: - None => {} - "my-str" => {"my-str": "my-str"} - ["my-list"] => {"my-list": "my-list"} - ("my-tuple-key", "my-tuple-val") => {"my-tuple-key": "my-tuple-val"} - """ - if items is None: - return {} - elif isinstance(items, str): - return {items: items} - elif isinstance(items, tuple): - return {items[0]: items[1]} - else: - items_dict = {} - for item in items: - if isinstance(item, str): - items_dict[item] = item - else: - items_dict[item[0]] = item[1] - return items_dict - - @property - def scope_names(self) -> t.List[str]: - return self._classattr_scope_names + self._registered_scope_names - - # custom __getattr__ instructs `mypy` that unknown attributes of a ScopeBuilder are - # of type `str`, allowing for dynamic attribute names - # to test, try creating a module with - # - # from globus_sdk.scopes import TransferScopes - # x = TransferScopes.all - # - # without this method, the assignment to `x` would fail type checking - # because `all` is unknown to mypy - # - # note that the implementation just raises AttributeError; this is okay because - # __getattr__ is only called as a last resort, when __getattribute__ has failed - # normal attribute access will not be disrupted - def __getattr__(self, name: str) -> str: - raise AttributeError(f"Unrecognized Attribute '{name}'") - - def urn_scope_string(self, scope_name: str) -> str: - """ - Return a complete string representing the scope with a given name for this - client, in the Globus Auth URN format. - - Note that this module already provides many such scope strings for use with - Globus services. - - **Examples** - - >>> sb = ScopeBuilder("transfer.api.globus.org") - >>> sb.urn_scope_string("transfer.api.globus.org", "all") - "urn:globus:auth:scope:transfer.api.globus.org:all" - - :param scope_name: The short name for the scope involved. - :type scope_name: str - """ - return f"urn:globus:auth:scope:{self.resource_server}:{scope_name}" - - def url_scope_string(self, scope_name: str) -> str: - """ - Return a complete string representing the scope with a given name for this - client, in URL format. - - **Examples** - - >>> sb = ScopeBuilder("actions.globus.org") - >>> sb.url_scope_string("actions.globus.org", "hello_world") - "https://auth.globus.org/scopes/actions.globus.org/hello_world" - - :param scope_name: The short name for the scope involved. - :type scope_name: str - """ - return f"https://auth.globus.org/scopes/{self.resource_server}/{scope_name}" - - def make_mutable(self, scope: str) -> MutableScope: - """ - For a given scope, create a MutableScope object. - - The ``scope`` name given refers to the name of a scope attached to the - ScopeBuilder. It is given by attribute name, not by the full scope string. - - **Examples** - - Using the ``TransferScopes`` object, one could reference ``all`` as follows: - - >>> TransferScopes.all - 'urn:globus:auth:scope:transfer.api.globus.org:all' - >>> TransferScopes.make_mutable("all") - MutableScope('urn:globus:auth:scope:transfer.api.globus.org:all') - - This is equivalent to constructing a MutableScope object from the resolved - scope string, as in - - >>> MutableScope(TransferScopes.all) - MutableScope('urn:globus:auth:scope:transfer.api.globus.org:all') - - :param scope: The name of the scope to convert to a MutableScope - :type scope: str - """ - return MutableScope(getattr(self, scope)) - - def __str__(self) -> str: - return f"ScopeBuilder[{self.resource_server}]\n" + "\n".join( - f" {name}:\n {getattr(self, name)}" for name in self.scope_names - ) - - -class GCSEndpointScopeBuilder(ScopeBuilder): - """ - A ScopeBuilder with a named property for the GCS manage_collections scope. - "manage_collections" is a scope on GCS Endpoints. The resource_server string should - be the GCS Endpoint ID. - - **Examples** - - >>> sb = GCSEndpointScopeBuilder("xyz") - >>> mc_scope = sb.manage_collections - """ - - _classattr_scope_names = ["manage_collections"] - - @property - def manage_collections(self) -> str: - return self.urn_scope_string("manage_collections") - - -class GCSCollectionScopeBuilder(ScopeBuilder): - """ - A ScopeBuilder with a named property for the GCS data_access scope. - "data_access" is a scope on GCS Collections. The resource_server string should - be the GCS Collection ID. - - **Examples** - - >>> sb = GCSCollectionScopeBuilder("xyz") - >>> da_scope = sb.data_access - >>> https_scope = sb.https - """ - - _classattr_scope_names = ["data_access", "https"] - - @property - def data_access(self) -> str: - return self.url_scope_string("data_access") - - @property - def https(self) -> str: - return self.url_scope_string("https") - - -class _AuthScopesBuilder(ScopeBuilder): - _classattr_scope_names = ["openid", "email", "profile"] - - openid: str = "openid" - email: str = "email" - profile: str = "profile" - - -AuthScopes = _AuthScopesBuilder( - "auth.globus.org", - known_scopes=[ - "view_authentications", - "view_clients", - "view_clients_and_scopes", - "view_identities", - "view_identity_set", - ], -) -"""Globus Auth scopes. - -.. listknownscopes:: globus_sdk.scopes.AuthScopes - example_scope=view_identity_set -""" - - -class _FlowsScopeBuilder(ScopeBuilder): - """ - The Flows Service breaks the scopes/resource server convention: - its resource server is a domain name but its scopes are built around the client id - Given that there isn't a simple way to support this more generally - (and we shouldn't encourage supporting this more generally), this class serves to - build out the scopes accurately specifically for Flows - """ - - def __init__( - self, - domain_name: str, - client_id: str, - known_scopes: t.Union[ - t.List[t.Union[str, t.Tuple[str, str]]], str, None - ] = None, - known_url_scopes: t.Union[ - t.List[t.Union[str, t.Tuple[str, str]]], str, None - ] = None, - ) -> None: - self._client_id = client_id - super().__init__( - domain_name, known_scopes=known_scopes, known_url_scopes=known_url_scopes - ) - - def urn_scope_string(self, scope_name: str) -> str: - return f"urn:globus:auth:scope:{self._client_id}:{scope_name}" - - def url_scope_string(self, scope_name: str) -> str: - return f"https://auth.globus.org/scopes/{self._client_id}/{scope_name}" - - -FlowsScopes = _FlowsScopeBuilder( - "flows.globus.org", - "eec9b274-0c81-4334-bdc2-54e90e689b9a", - known_url_scopes=[ - "manage_flows", - "view_flows", - "run", - "run_status", - "run_manage", - ], -) -"""Globus Flows scopes. -.. listknownscopes:: globus_sdk.scopes.FlowsScopes -""" - - -GroupsScopes = ScopeBuilder( - "groups.api.globus.org", - known_scopes=[ - "all", - "view_my_groups_and_memberships", - ], -) -"""Groups scopes. - -.. listknownscopes:: globus_sdk.scopes.GroupsScopes -""" - - -NexusScopes = ScopeBuilder( - "nexus.api.globus.org", - known_scopes=[ - "groups", - ], -) -"""Nexus scopes (internal use only). - -.. listknownscopes:: globus_sdk.scopes.NexusScopes -""" - -SearchScopes = ScopeBuilder( - "search.api.globus.org", - known_scopes=[ - "all", - "globus_connect_server", - "ingest", - "search", - ], -) -"""Globus Search scopes. - -.. listknownscopes:: globus_sdk.scopes.SearchScopes -""" - -TimerScopes = ScopeBuilder( - "524230d7-ea86-4a52-8312-86065a9e0417", - known_url_scopes=[ - "timer", - ], -) -"""Globus Timer scopes. -.. listknownscopes:: globus_sdk.scopes.TimerScopes -""" - -TransferScopes = ScopeBuilder( - "transfer.api.globus.org", - known_scopes=[ - "all", - "gcp_install", - ], -) -"""Globus Transfer scopes. - -.. listknownscopes:: globus_sdk.scopes.TransferScopes -""" diff --git a/src/globus_sdk/scopes/__init__.py b/src/globus_sdk/scopes/__init__.py new file mode 100644 index 000000000..c69bafd9c --- /dev/null +++ b/src/globus_sdk/scopes/__init__.py @@ -0,0 +1,39 @@ +from .builder import ScopeBuilder +from .data import ( + AuthScopes, + FlowsScopes, + GCSCollectionScopeBuilder, + GCSEndpointScopeBuilder, + GroupsScopes, + NexusScopes, + SearchScopes, + TimerScopes, + TransferScopes, +) +from .scope_definition import Scope, ScopeParseError + +# alias to old name +# +# deprecation TODO: +# - add a `__getattr__` on this module which raises a deprecation warning on access to +# `MutableScope` +# - ensure removal in SDK v4.0 +# (add a test case which checks the version and fails if this is present and the +# version is >=4 ?) +MutableScope = Scope + + +__all__ = ( + "ScopeBuilder", + "Scope", + "ScopeParseError", + "GCSCollectionScopeBuilder", + "GCSEndpointScopeBuilder", + "AuthScopes", + "FlowsScopes", + "GroupsScopes", + "NexusScopes", + "SearchScopes", + "TimerScopes", + "TransferScopes", +) diff --git a/src/globus_sdk/scopes/builder.py b/src/globus_sdk/scopes/builder.py new file mode 100644 index 000000000..13d99d8d9 --- /dev/null +++ b/src/globus_sdk/scopes/builder.py @@ -0,0 +1,159 @@ +import typing as t + +from .scope_definition import Scope + +ScopeBuilderScopes = t.Union[ + None, str, t.Tuple[str, str], t.List[t.Union[str, t.Tuple[str, str]]] +] + + +class ScopeBuilder: + """ + Utility class for creating scope strings for a specified resource server. + + :param resource_server: The identifier, usually a domain name or a UUID, for the + resource server to return scopes for. + :type resource_server: str + :param known_scopes: A list of scope names to pre-populate on this instance. This + will set attributes on the instance using the URN scope format. + :type known_scopes: list of str, optional + :param known_url_scopes: A list of scope names to pre-populate on this instance. + This will set attributes on the instance using the URL scope format. + :type known_url_scopes: list of str, optional + """ + + _classattr_scope_names: t.List[str] = [] + + def __init__( + self, + resource_server: str, + *, + known_scopes: ScopeBuilderScopes = None, + known_url_scopes: ScopeBuilderScopes = None, + ) -> None: + self.resource_server = resource_server + + self._registered_scope_names: t.List[str] = [] + self._register_scopes(known_scopes, self.urn_scope_string) + self._register_scopes(known_url_scopes, self.url_scope_string) + + def _register_scopes( + self, scopes: ScopeBuilderScopes, transform_func: t.Callable[[str], str] + ) -> None: + scopes_dict = self._scopes_input_to_dict(scopes) + for scope_name, scope_val in scopes_dict.items(): + self._registered_scope_names.append(scope_name) + setattr(self, scope_name, transform_func(scope_val)) + + def _scopes_input_to_dict(self, items: ScopeBuilderScopes) -> t.Dict[str, str]: + """ + ScopeBuilders accepts many collection-style types of scopes. This function + normalizes all of those types into a standard {scope_name: scope_val} dict + + Translation Map: + None => {} + "my-str" => {"my-str": "my-str"} + ["my-list"] => {"my-list": "my-list"} + ("my-tuple-key", "my-tuple-val") => {"my-tuple-key": "my-tuple-val"} + """ + if items is None: + return {} + elif isinstance(items, str): + return {items: items} + elif isinstance(items, tuple): + return {items[0]: items[1]} + else: + items_dict = {} + for item in items: + if isinstance(item, str): + items_dict[item] = item + else: + items_dict[item[0]] = item[1] + return items_dict + + @property + def scope_names(self) -> t.List[str]: + return self._classattr_scope_names + self._registered_scope_names + + # custom __getattr__ instructs `mypy` that unknown attributes of a ScopeBuilder are + # of type `str`, allowing for dynamic attribute names + # to test, try creating a module with + # + # from globus_sdk.scopes import TransferScopes + # x = TransferScopes.all + # + # without this method, the assignment to `x` would fail type checking + # because `all` is unknown to mypy + # + # note that the implementation just raises AttributeError; this is okay because + # __getattr__ is only called as a last resort, when __getattribute__ has failed + # normal attribute access will not be disrupted + def __getattr__(self, name: str) -> str: + raise AttributeError(f"Unrecognized Attribute '{name}'") + + def urn_scope_string(self, scope_name: str) -> str: + """ + Return a complete string representing the scope with a given name for this + client, in the Globus Auth URN format. + + Note that this module already provides many such scope strings for use with + Globus services. + + **Examples** + + >>> sb = ScopeBuilder("transfer.api.globus.org") + >>> sb.urn_scope_string("transfer.api.globus.org", "all") + "urn:globus:auth:scope:transfer.api.globus.org:all" + + :param scope_name: The short name for the scope involved. + :type scope_name: str + """ + return f"urn:globus:auth:scope:{self.resource_server}:{scope_name}" + + def url_scope_string(self, scope_name: str) -> str: + """ + Return a complete string representing the scope with a given name for this + client, in URL format. + + **Examples** + + >>> sb = ScopeBuilder("actions.globus.org") + >>> sb.url_scope_string("actions.globus.org", "hello_world") + "https://auth.globus.org/scopes/actions.globus.org/hello_world" + + :param scope_name: The short name for the scope involved. + :type scope_name: str + """ + return f"https://auth.globus.org/scopes/{self.resource_server}/{scope_name}" + + def make_mutable(self, scope: str) -> Scope: + """ + For a given scope, create a Scope object. + + The ``scope`` name given refers to the name of a scope attached to the + ScopeBuilder. It is given by attribute name, not by the full scope string. + + **Examples** + + Using the ``TransferScopes`` object, one could reference ``all`` as follows: + + >>> TransferScopes.all + 'urn:globus:auth:scope:transfer.api.globus.org:all' + >>> TransferScopes.make_mutable("all") + Scope('urn:globus:auth:scope:transfer.api.globus.org:all') + + This is equivalent to constructing a Scope object from the resolved + scope string, as in + + >>> Scope(TransferScopes.all) + Scope('urn:globus:auth:scope:transfer.api.globus.org:all') + + :param scope: The name of the scope to convert to a Scope + :type scope: str + """ + return Scope(getattr(self, scope)) + + def __str__(self) -> str: + return f"ScopeBuilder[{self.resource_server}]\n" + "\n".join( + f" {name}:\n {getattr(self, name)}" for name in self.scope_names + ) diff --git a/src/globus_sdk/scopes/data.py b/src/globus_sdk/scopes/data.py new file mode 100644 index 000000000..6a48aabab --- /dev/null +++ b/src/globus_sdk/scopes/data.py @@ -0,0 +1,176 @@ +from .builder import ScopeBuilder, ScopeBuilderScopes + + +class GCSEndpointScopeBuilder(ScopeBuilder): + """ + A ScopeBuilder with a named property for the GCS manage_collections scope. + "manage_collections" is a scope on GCS Endpoints. The resource_server string should + be the GCS Endpoint ID. + + **Examples** + + >>> sb = GCSEndpointScopeBuilder("xyz") + >>> mc_scope = sb.manage_collections + """ + + _classattr_scope_names = ["manage_collections"] + + @property + def manage_collections(self) -> str: + return self.urn_scope_string("manage_collections") + + +class GCSCollectionScopeBuilder(ScopeBuilder): + """ + A ScopeBuilder with a named property for the GCS data_access scope. + "data_access" is a scope on GCS Collections. The resource_server string should + be the GCS Collection ID. + + **Examples** + + >>> sb = GCSCollectionScopeBuilder("xyz") + >>> da_scope = sb.data_access + >>> https_scope = sb.https + """ + + _classattr_scope_names = ["data_access", "https"] + + @property + def data_access(self) -> str: + return self.url_scope_string("data_access") + + @property + def https(self) -> str: + return self.url_scope_string("https") + + +class _AuthScopesBuilder(ScopeBuilder): + _classattr_scope_names = ["openid", "email", "profile"] + + openid: str = "openid" + email: str = "email" + profile: str = "profile" + + +AuthScopes = _AuthScopesBuilder( + "auth.globus.org", + known_scopes=[ + "view_authentications", + "view_clients", + "view_clients_and_scopes", + "view_identities", + "view_identity_set", + ], +) +"""Globus Auth scopes. + +.. listknownscopes:: globus_sdk.scopes.AuthScopes + example_scope=view_identity_set +""" + + +class _FlowsScopeBuilder(ScopeBuilder): + """ + The Flows Service breaks the scopes/resource server convention: + its resource server is a domain name but its scopes are built around the client id + Given that there isn't a simple way to support this more generally + (and we shouldn't encourage supporting this more generally), this class serves to + build out the scopes accurately specifically for Flows + """ + + def __init__( + self, + domain_name: str, + client_id: str, + known_scopes: ScopeBuilderScopes = None, + known_url_scopes: ScopeBuilderScopes = None, + ) -> None: + self._client_id = client_id + super().__init__( + domain_name, known_scopes=known_scopes, known_url_scopes=known_url_scopes + ) + + def urn_scope_string(self, scope_name: str) -> str: + return f"urn:globus:auth:scope:{self._client_id}:{scope_name}" + + def url_scope_string(self, scope_name: str) -> str: + return f"https://auth.globus.org/scopes/{self._client_id}/{scope_name}" + + +FlowsScopes = _FlowsScopeBuilder( + "flows.globus.org", + "eec9b274-0c81-4334-bdc2-54e90e689b9a", + known_url_scopes=[ + "manage_flows", + "view_flows", + "run", + "run_status", + "run_manage", + ], +) +"""Globus Flows scopes. + +.. listknownscopes:: globus_sdk.scopes.FlowsScopes +""" + + +GroupsScopes = ScopeBuilder( + "groups.api.globus.org", + known_scopes=[ + "all", + "view_my_groups_and_memberships", + ], +) +"""Groups scopes. + +.. listknownscopes:: globus_sdk.scopes.GroupsScopes +""" + + +NexusScopes = ScopeBuilder( + "nexus.api.globus.org", + known_scopes=[ + "groups", + ], +) +"""Nexus scopes (internal use only). + +.. listknownscopes:: globus_sdk.scopes.NexusScopes +""" + +SearchScopes = ScopeBuilder( + "search.api.globus.org", + known_scopes=[ + "all", + "globus_connect_server", + "ingest", + "search", + ], +) +"""Globus Search scopes. + +.. listknownscopes:: globus_sdk.scopes.SearchScopes +""" + +TimerScopes = ScopeBuilder( + "524230d7-ea86-4a52-8312-86065a9e0417", + known_url_scopes=[ + "timer", + ], +) +"""Globus Timer scopes. + +.. listknownscopes:: globus_sdk.scopes.TimerScopes +""" + +TransferScopes = ScopeBuilder( + "transfer.api.globus.org", + known_scopes=[ + "all", + "gcp_install", + ], +) +"""Globus Transfer scopes. + +.. listknownscopes:: globus_sdk.scopes.TransferScopes +""" diff --git a/src/globus_sdk/scopes/scope_definition.py b/src/globus_sdk/scopes/scope_definition.py new file mode 100644 index 000000000..4f49b5831 --- /dev/null +++ b/src/globus_sdk/scopes/scope_definition.py @@ -0,0 +1,241 @@ +""" +This defines the Scope object and the scope parser. +Because these components are mutually dependent, it's easiest if they're kept in a +single module. +""" +import enum +import typing as t +import warnings + +from globus_sdk._types import ScopeCollectionType + + +class ScopeParseError(ValueError): + pass + + +def _iter_scope_collection(obj: ScopeCollectionType) -> t.Iterator[str]: + if isinstance(obj, str): + yield obj + elif isinstance(obj, Scope): + yield str(obj) + else: + for item in obj: + yield str(item) + + +class ParseTokenType(enum.Enum): + # a string like 'urn:globus:auth:scopes:transfer.api.globus.org:all' + scope_string = enum.auto() + # the optional marker, '*' + opt_marker = enum.auto() + # '[' and ']' + lbracket = enum.auto() + rbracket = enum.auto() + + +class ParseToken: + def __init__(self, value: str, token_type: ParseTokenType) -> None: + self.value = value + self.token_type = token_type + + +def _tokenize(scope_string: str) -> t.List[ParseToken]: + tokens: t.List[ParseToken] = [] + current_token: t.List[str] = [] + for c in scope_string: + if c in "[]* ": + if current_token: + tokens.append( + ParseToken("".join(current_token), ParseTokenType.scope_string) + ) + current_token = [] + if c == "*": + tokens.append(ParseToken(c, ParseTokenType.opt_marker)) + elif c == "[": + tokens.append(ParseToken(c, ParseTokenType.lbracket)) + elif c == "]": + tokens.append(ParseToken(c, ParseTokenType.rbracket)) + elif c == " ": + if tokens and tokens[-1].token_type == ParseTokenType.opt_marker: + raise ScopeParseError( + "optional marker must not be followed by a space" + ) + else: + raise NotImplementedError + else: + current_token.append(c) + if current_token: + tokens.append(ParseToken("".join(current_token), ParseTokenType.scope_string)) + return tokens + + +def _parse_tokens(tokens: t.List[ParseToken]) -> t.List["Scope"]: + # value to return + ret: t.List[Scope] = [] + # track whether or not the current scope is optional (has a preceding *) + current_optional = False + # keep a stack of "parents", each time we enter a `[` context, push the last scope + # and each time we exit via a `]`, pop from the stack + parents: t.List[Scope] = [] + # track the current (or, by similar terminology, "last") complete scope seen + current_scope: t.Optional[Scope] = None + + for idx in range(len(tokens)): + token = tokens[idx] + try: + peek: t.Optional[ParseToken] = tokens[idx + 1] + except IndexError: + peek = None + + if token.token_type == ParseTokenType.opt_marker: + current_optional = True + if peek is None: + raise ScopeParseError("ended in optional marker") + if peek.token_type != ParseTokenType.scope_string: + raise ScopeParseError( + "a scope string must always follow an optional marker" + ) + + elif token.token_type == ParseTokenType.lbracket: + if peek is None: + raise ScopeParseError("ended in left bracket") + if peek.token_type == ParseTokenType.rbracket: + raise ScopeParseError("found empty brackets") + if peek.token_type == ParseTokenType.lbracket: + raise ScopeParseError("found double left-bracket") + if not current_scope: + raise ScopeParseError("found '[' without a preceding scope string") + + parents.append(current_scope) + elif token.token_type == ParseTokenType.rbracket: + if not parents: + raise ScopeParseError("found ']' with no matching '[' preceding it") + parents.pop() + else: + current_scope = Scope(token.value, optional=current_optional) + current_optional = False + if parents: + parents[-1].dependencies.append(current_scope) + else: + ret.append(current_scope) + if parents: + raise ScopeParseError("unclosed brackets, missing ']'") + + return ret + + +class Scope: + """ + A mutable scope is a representation of a scope which allows modifications to be + made. In particular, it supports handling scope dependencies via + ``add_dependency``. + + `str(Scope(...))` produces a valid scope string for use in various methods. + + :param scope_string: The string which will be used as the basis for this Scope + :type scope_string: str + :param optional: The scope may be marked as optional. This means that the scope can + be declined by the user without declining consent for other scopes + :type optional: bool + """ + + def __init__( + self, + scope_string: str, + *, + optional: bool = False, + dependencies: t.Optional[t.List["Scope"]] = None, + ) -> None: + self._scope_string = scope_string + self.optional = optional + self.dependencies: t.List[Scope] = [] if dependencies is None else dependencies + + @staticmethod + def parse(scope_string: str) -> t.List["Scope"]: + """ + Parse an arbitrary scope string to a list of scopes. + + Zero or more than one scope may be returned, as in the case of an empty string + or space-delimited scopes. + """ + tokens = _tokenize(scope_string) + return _parse_tokens(tokens) + + @classmethod + def deserialize(cls, scope_string: str) -> "Scope": + """ + Deserialize a scope string to a scope object. + + This is the special case of parsing in which exactly one scope must be returned + by the parse. + """ + data = Scope.parse(scope_string) + if len(data) != 1: + raise ValueError( + "Deserializing a scope from string did not get exactly one scope. " + f"Instead got data={data}" + ) + return data[0] + + def serialize(self) -> str: + base_scope = ("*" if self.optional else "") + self._scope_string + if not self.dependencies: + return base_scope + return ( + base_scope + "[" + " ".join(c.serialize() for c in self.dependencies) + "]" + ) + + def add_dependency( + self, scope: t.Union[str, "Scope"], *, optional: t.Optional[bool] = None + ) -> "Scope": + """ + Add a scope dependency. The dependent scope relationship will be stored in the + Scope and will be evident in its string representation. + + :param scope: The scope upon which the current scope depends + :type scope: str + :param optional: Mark the dependency an optional one. By default it is not. An + optional scope dependency can be declined by the user without declining + consent for the primary scope + :type optional: bool, optional + """ + if optional is not None: + if isinstance(scope, Scope): + raise ValueError( + "cannot use optional=... with a Scope object as the argument to " + "add_dependency" + ) + warnings.warn( + "Passing 'optional' to add_dependency is deprecated. " + "Construct an optional Scope object instead.", + DeprecationWarning, + ) + scopeobj = Scope(scope, optional=optional) + else: + if isinstance(scope, str): + scopeobj = Scope.deserialize(scope) + else: + scopeobj = scope + self.dependencies.append(scopeobj) + return self + + def __repr__(self) -> str: + parts: t.List[str] = [f"'{self._scope_string}'"] + if self.optional: + parts.append("optional=True") + if self.dependencies: + parts.append(f"dependencies={self.dependencies!r}") + return "Scope(" + ", ".join(parts) + ")" + + def __str__(self) -> str: + return self.serialize() + + @staticmethod + def scopes2str(obj: ScopeCollectionType) -> str: + """ + Given a scope string, a collection of scope strings, a Scope object, a + collection of Scope objects, or a mixed collection of strings and + Scopes, convert to a string which can be used in a request. + """ + return " ".join(_iter_scope_collection(obj)) diff --git a/src/globus_sdk/services/auth/flow_managers/authorization_code.py b/src/globus_sdk/services/auth/flow_managers/authorization_code.py index 6e1163398..516ac9862 100644 --- a/src/globus_sdk/services/auth/flow_managers/authorization_code.py +++ b/src/globus_sdk/services/auth/flow_managers/authorization_code.py @@ -3,6 +3,7 @@ import urllib.parse from globus_sdk import scopes, utils +from globus_sdk._types import ScopeCollectionType from ..oauth2_constants import DEFAULT_REQUESTED_SCOPES from ..response import OAuthTokenResponse @@ -56,7 +57,7 @@ def __init__( self, auth_client: "globus_sdk.AuthClient", redirect_uri: str, - requested_scopes: t.Optional[scopes._ScopeCollectionType] = None, + requested_scopes: t.Optional[ScopeCollectionType] = None, state: str = "_default", refresh_tokens: bool = False, ): diff --git a/src/globus_sdk/services/auth/flow_managers/native_app.py b/src/globus_sdk/services/auth/flow_managers/native_app.py index eb10a0ed1..e20375e5a 100644 --- a/src/globus_sdk/services/auth/flow_managers/native_app.py +++ b/src/globus_sdk/services/auth/flow_managers/native_app.py @@ -7,6 +7,7 @@ import urllib.parse from globus_sdk import scopes, utils +from globus_sdk._types import ScopeCollectionType from globus_sdk.exc import GlobusSDKUsageError from ..oauth2_constants import DEFAULT_REQUESTED_SCOPES @@ -106,7 +107,7 @@ class GlobusNativeAppFlowManager(GlobusOAuthFlowManager): def __init__( self, auth_client: "globus_sdk.AuthClient", - requested_scopes: t.Optional[scopes._ScopeCollectionType] = None, + requested_scopes: t.Optional[ScopeCollectionType] = None, redirect_uri: t.Optional[str] = None, state: str = "_default", verifier: t.Optional[str] = None, diff --git a/tests/unit/test_scopes.py b/tests/unit/test_scopes.py index 911ea6bc7..36fe11e75 100644 --- a/tests/unit/test_scopes.py +++ b/tests/unit/test_scopes.py @@ -2,7 +2,7 @@ import pytest -from globus_sdk.scopes import FlowsScopes, MutableScope, ScopeBuilder +from globus_sdk.scopes import FlowsScopes, Scope, ScopeBuilder, ScopeParseError def test_url_scope_string(): @@ -79,67 +79,73 @@ def test_sb_allowed_inputs_types(): assert list_sb.do_a_thing == scope_1_urn -def test_mutable_scope_str_and_repr_simple(): - s = MutableScope("simple") +def test_scope_str_and_repr_simple(): + s = Scope("simple") assert str(s) == "simple" - assert repr(s) == "MutableScope('simple')" + assert repr(s) == "Scope('simple')" - s2 = MutableScope("simple", optional=True) - assert str(s2) == "*simple" - assert repr(s2) == "MutableScope('simple', optional=True)" +def test_scope_str_and_repr_optional(): + s = Scope("simple", optional=True) + assert str(s) == "*simple" + assert repr(s) == "Scope('simple', optional=True)" -def test_mutable_scope_str_and_repr_with_dependencies(): - s = MutableScope("top") + +def test_scope_str_and_repr_with_dependencies(): + s = Scope("top") s.add_dependency("foo") assert str(s) == "top[foo]" - s.add_dependency("bar", optional=True) - # NOTE: order is not guaranteed for dict() until python3.7 - assert str(s) in ("top[foo *bar]", "top[*bar foo]") - assert repr(s) in ( - "MutableScope('top', dependencies={'foo': False, 'bar': True})", - "MutableScope('top', dependencies={'bar': True, 'foo': False})", - ) + s.add_dependency("bar") + assert str(s) == "top[foo bar]" + assert repr(s) == "Scope('top', dependencies=[Scope('foo'), Scope('bar')])" - s2 = MutableScope("top", optional=True) - s2.add_dependency("foo") - assert str(s2) == "*top[foo]" - s2.add_dependency("bar", optional=True) - # NOTE: order is not guaranteed for dict() until python3.7 - assert str(s2) in ("*top[foo *bar]", "*top[*bar foo]") - assert repr(s2) in ( - "MutableScope('top', optional=True, dependencies={'foo': False, 'bar': True})", - "MutableScope('top', optional=True, dependencies={'bar': True, 'foo': False})", - ) + +def test_add_dependency_warns_on_optional_but_still_has_good_str_and_repr(): + s = Scope("top") + # this should warn, the use of `optional=...` rather than adding a Scope object + # when optional dependencies are wanted is deprecated + with pytest.warns(DeprecationWarning): + s.add_dependency("foo", optional=True) + + # confirm the str representation and repr for good measure + assert str(s) == "top[*foo]" + assert repr(s) == "Scope('top', dependencies=[Scope('foo', optional=True)])" -def test_mutable_scope_str_nested(): - top = MutableScope("top") - mid = MutableScope("mid") - bottom = MutableScope("bottom") - mid.add_dependency(str(bottom)) - top.add_dependency(str(mid)) +@pytest.mark.parametrize("optional_arg", (True, False)) +def test_add_dependency_fails_if_optional_is_combined_with_scope(optional_arg): + s = Scope("top") + s2 = Scope("bottom") + with pytest.raises(ValueError): + s.add_dependency(s2, optional=optional_arg) + + +def test_scope_str_nested(): + top = Scope("top") + mid = Scope("mid") + bottom = Scope("bottom") + mid.add_dependency(bottom) + top.add_dependency(mid) assert str(bottom) == "bottom" assert str(mid) == "mid[bottom]" assert str(top) == "top[mid[bottom]]" -def test_mutable_scope_collection_to_str(): - foo = MutableScope("foo") - bar = MutableScope("bar") +def test_scope_collection_to_str(): + foo = Scope("foo") + bar = Scope("bar") baz = "baz" - assert MutableScope.scopes2str(foo) == "foo" - assert MutableScope.scopes2str([foo, bar]) == "foo bar" - assert MutableScope.scopes2str(baz) == "baz" - assert MutableScope.scopes2str([foo, baz]) == "foo baz" + assert Scope.scopes2str(foo) == "foo" + assert Scope.scopes2str([foo, bar]) == "foo bar" + assert Scope.scopes2str(baz) == "baz" + assert Scope.scopes2str([foo, baz]) == "foo baz" -def test_mutable_scope_rejects_scope_with_optional_marker(): - s = MutableScope("top") - with pytest.raises(ValueError) as excinfo: - s.add_dependency("*subscope") - - assert "add_dependency cannot contain a leading '*'" in str(excinfo.value) +def test_add_dependency_parses_scope_with_optional_marker(): + s = Scope("top") + s.add_dependency("*subscope") + assert str(s) == "top[*subscope]" + assert repr(s) == "Scope('top', dependencies=[Scope('subscope', optional=True)])" def test_scopebuilder_make_mutable_produces_same_strings(): @@ -154,3 +160,79 @@ def test_flows_scopes_creation(): FlowsScopes.run == "https://auth.globus.org/scopes/eec9b274-0c81-4334-bdc2-54e90e689b9a/run" ) + + +def test_scope_parsing_rejects_optional_marker_followed_by_space(): + Scope.parse("*foo") # okay + with pytest.raises(ScopeParseError): + Scope.parse("* foo") # not okay + + +def test_scope_parsing_rejects_ending_in_optional_marker(): + with pytest.raises(ScopeParseError): + Scope.parse("foo*") + + +def test_scope_parsing_allows_empty_string(): + scopes = Scope.parse("") + assert scopes == [] + + +def test_scope_parsing_does_not_allow_optional_marker_before_brackets(): + with pytest.raises(ScopeParseError): + Scope.parse("foo*[bar]") + with pytest.raises(ScopeParseError): + Scope.parse("foo *[bar]") + with pytest.raises(ScopeParseError): + Scope.parse("foo [bar*]") + + +def test_scope_parsing_does_not_allow_ending_in_open_bracket(): + with pytest.raises(ScopeParseError): + Scope.parse("foo[") + + +def test_scope_parsing_does_not_allow_empty_brackets(): + with pytest.raises(ScopeParseError): + Scope.parse("foo[]") + + +def test_scope_parsing_does_not_allow_starting_with_open_bracket(): + with pytest.raises(ScopeParseError): + Scope.parse("[foo]") + + +def test_scope_parsing_does_not_allow_double_open_bracket(): + with pytest.raises(ScopeParseError): + Scope.parse("foo[[bar]]") + + +def test_scope_parsing_does_not_allow_unbalanced_brackets_closing(): + with pytest.raises(ScopeParseError): + Scope.parse("foo[bar]]") + + +def test_scope_parsing_does_not_allow_unbalanced_brackets_missing_close(): + with pytest.raises(ScopeParseError): + Scope.parse("foo[bar") + + +def test_scope_deserialize_simple(): + scope = Scope.deserialize("foo") + assert str(scope) == "foo" + + +def test_scope_deserialize_with_dependencies(): + # oh, while we're here, let's also check that our whitespace insensitivity works + scope = Scope.deserialize("foo[ bar *baz ]") + assert str(scope) == "foo[bar *baz]" + + +def test_scope_deserialize_fails_on_empty(): + with pytest.raises(ValueError): + Scope.deserialize(" ") + + +def test_scope_deserialize_fails_on_multiple_top_level_scopes(): + with pytest.raises(ValueError): + Scope.deserialize("foo bar") From bc1fc5c7f445740eedda418bb7594922a70329e8 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Fri, 4 Nov 2022 19:46:10 +0000 Subject: [PATCH 02/10] Forbid special characters in Scope.__init__ This avoids having to consider parsing or otherwise handling `Scope("*foo")`. Although this would ideally be an alias for `Scope.deserialize("*foo")`, the interface gets messy if we allow this. --- src/globus_sdk/scopes/scope_definition.py | 5 +++++ tests/unit/test_scopes.py | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/src/globus_sdk/scopes/scope_definition.py b/src/globus_sdk/scopes/scope_definition.py index 4f49b5831..b00581351 100644 --- a/src/globus_sdk/scopes/scope_definition.py +++ b/src/globus_sdk/scopes/scope_definition.py @@ -147,6 +147,11 @@ def __init__( optional: bool = False, dependencies: t.Optional[t.List["Scope"]] = None, ) -> None: + if any(c in scope_string for c in "[]* "): + raise ValueError( + "Scope instances may not contain the special characters '[]* '. " + "Use either Scope.deserialize or Scope.parse instead" + ) self._scope_string = scope_string self.optional = optional self.dependencies: t.List[Scope] = [] if dependencies is None else dependencies diff --git a/tests/unit/test_scopes.py b/tests/unit/test_scopes.py index 36fe11e75..b8e283da8 100644 --- a/tests/unit/test_scopes.py +++ b/tests/unit/test_scopes.py @@ -236,3 +236,9 @@ def test_scope_deserialize_fails_on_empty(): def test_scope_deserialize_fails_on_multiple_top_level_scopes(): with pytest.raises(ValueError): Scope.deserialize("foo bar") + + +@pytest.mark.parametrize("scope_str", ("*foo", "foo[bar]", "foo[", "foo]", "foo bar")) +def test_scope_init_forbids_special_chars(scope_str): + with pytest.raises(ValueError): + Scope(scope_str) From 26d94ba630668cbb77ca2b29d072a3078033d8e9 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Fri, 4 Nov 2022 20:39:30 +0000 Subject: [PATCH 03/10] Add `Scope.__contains__` with tests `Scope(...) in Scope(...)` is meant to indicate "permissions containment", meaning that the permissions requested by the first scope are covered by the permissions requested by the second scope. That notion of containment leads to the following defined behaviors: - the top-level scope strings must match - either both have matching optional=t/false OR only the "contained" scope is optional - each dependency needs to be contained in the "containing" scope's dependencies - it doesn't make sense to ask if a non-Scope value is in a Scope This definition feeds directly into an implementation and a large suite of tests to check correctness. --- ...21104_145601_sirosen_add_scope_parsing.rst | 6 + src/globus_sdk/scopes/scope_definition.py | 55 +++++++++ tests/unit/test_scopes.py | 105 ++++++++++++++++++ 3 files changed, 166 insertions(+) diff --git a/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst b/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst index 080042cb7..36e11c144 100644 --- a/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst +++ b/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst @@ -15,3 +15,9 @@ ``Scope(...).add_dependency("*foo")`` can be used to add an optional dependency as a string, or equivalently ``Scope(...).add_dependency(Scope("foo", optional=True))`` + + * ``Scope``\s support containment checking (``Scope("foo") in Scope("bar")``), + which means that all of the consents for one scope are a strict subset of + the consents of another scope. Containment is therefore an indication that + the associated permissions of a token for a scope string are covered by + the set of permissions associated with another scope string. diff --git a/src/globus_sdk/scopes/scope_definition.py b/src/globus_sdk/scopes/scope_definition.py index b00581351..fd61d78ad 100644 --- a/src/globus_sdk/scopes/scope_definition.py +++ b/src/globus_sdk/scopes/scope_definition.py @@ -236,6 +236,61 @@ def __repr__(self) -> str: def __str__(self) -> str: return self.serialize() + def __contains__(self, other: t.Any) -> bool: + """ + scope1 in scope2 is defined as subtree matching. + + A scope contains another scope if + - the top level strings match + - the optional-ness matches OR only the contained scope is optional + - the dependencies of the contained scope are all contained in dependencies of + the containing scope + + Therefore, the following are true: + + .. code-block:: pycon + + # self inclusion works + >>> Scope.deserialize("foo") in Scope.deserialize("foo") + # optional mismatches in either direction do not indicate containment + >>> Scope.deserialize("foo") not in Scope.deserialize("*foo") + >>> Scope.deserialize("*foo") not in Scope.deserialize("foo") + # dependencies have the expected meanings + >>> Scope.deserialize("foo") in Scope.deserialize("foo[bar]") + >>> Scope.deserialize("foo[bar]") not in Scope.deserialize("foo") + >>> Scope.deserialize("foo[bar]") in Scope.deserialize("foo[bar[baz]]") + # dependencies are not transitive and obey "optionalness" matching + >>> Scope.deserialize("foo[bar]") not in Scope.deserialize("foo[fizz[bar]]") + >>> Scope.deserialize("foo[bar]") not in Scope.deserialize("foo[*bar]") + """ + # scopes can only contain other scopes + if not isinstance(other, Scope): + return False + + # top-level scope must match + if self._scope_string != other._scope_string: + return False + # if both are optional, okay + # if neither is optional, okay + # but if only one is optional... + if self.optional != other.optional: + # ... then make sure it is 'other' + if self.optional: + return False + + # dependencies must all be contained -- search for a contrary example + for other_dep in other.dependencies: + found_match = False + for dep in self.dependencies: + if other_dep in dep: + found_match = True + break + if not found_match: + return False + + # all criteria were met -- True! + return True + @staticmethod def scopes2str(obj: ScopeCollectionType) -> str: """ diff --git a/tests/unit/test_scopes.py b/tests/unit/test_scopes.py index b8e283da8..4f8f95399 100644 --- a/tests/unit/test_scopes.py +++ b/tests/unit/test_scopes.py @@ -242,3 +242,108 @@ def test_scope_deserialize_fails_on_multiple_top_level_scopes(): def test_scope_init_forbids_special_chars(scope_str): with pytest.raises(ValueError): Scope(scope_str) + + +def test_scope_contains_requires_scope_objects(): + s = Scope("foo") + assert "foo" not in s + + +@pytest.mark.parametrize( + "contained, containing, expect", + [ + # string matching, including optional on both sides + ("foo", "foo", True), # identity + ("*foo", "*foo", True), # identity + ("foo", "bar", False), + ("foo", "*bar", False), + ("*foo", "bar", False), + # optional-ness is one-way when mismatched + ("foo", "*foo", False), + ("*foo", "foo", True), + # dependency matching is also one-way when mismatched + ("foo[bar]", "foo[bar]", True), # identity + ("foo[bar]", "foo", False), + ("foo", "foo[bar]", True), + ("foo", "foo[bar[baz]]", True), + ("foo[bar]", "foo[bar[baz]]", True), + ("foo[bar[baz]]", "foo[bar[baz]]", True), # identity + # and the combination of dependencies with optional also works + ("foo[*bar]", "foo[bar[baz]]", True), + ("foo[*bar]", "foo[*bar[baz]]", True), + ("foo[bar]", "foo[bar[*baz]]", True), + ("foo[*bar]", "foo[bar[*baz]]", True), + ], +) +def test_scope_contains_simple_cases(contained, containing, expect): + outer_s = Scope.deserialize(containing) + inner_s = Scope.deserialize(contained) + if expect: + assert inner_s in outer_s + else: + assert inner_s not in outer_s + + +@pytest.mark.parametrize( + "contained, containing, expect", + [ + # "simple" cases for multiple dependencies + ("foo[bar baz]", "foo[bar[baz] baz]", True), + ("foo[bar baz]", "foo[bar[baz]]", False), + ("foo[baz bar]", "foo[bar[baz] baz]", True), + ("foo[bar baz]", "foo[bar[baz] baz buzz]", True), + # these scenarios will mirror some "realistic" usage + ( + "timer[transfer_ap[transfer]]", + "timer[transfer_ap[transfer[*foo/data_access]]]", + True, + ), + ( + "timer[transfer_ap[transfer[*foo/data_access]]]", + "timer[transfer_ap[transfer[*foo/data_access *bar/data_access]]]", + True, + ), + ( + "timer[transfer_ap[transfer[*bar *foo]]]", + "timer[transfer_ap[transfer[*foo *bar *baz]]]", + True, + ), + ( + "timer[transfer_ap[transfer[*foo/data_access]]]", + "timer[transfer_ap[transfer]]", + False, + ), + ( + "timer[transfer_ap[transfer[*foo/data_access *bar/data_access]]]", + "timer[transfer_ap[transfer[*foo/data_access]]]", + False, + ), + ( + "timer[transfer_ap[transfer[foo/data_access]]]", + "timer[transfer_ap[transfer[*foo/data_access]]]", + False, + ), + ( + "timer[transfer_ap[transfer[*foo/data_access]]]", + "timer[transfer_ap[transfer[foo/data_access]]]", + True, + ), + ( + "timer[transfer_ap[*transfer[*foo/data_access]]]", + "timer[transfer_ap[transfer[foo/data_access]]]", + True, + ), + ( + "timer[transfer_ap[transfer[*foo/data_access]]]", + "timer[transfer_ap[*transfer[foo/data_access]]]", + False, + ), + ], +) +def test_scope_contains_complex_usages(contained, containing, expect): + outer_s = Scope.deserialize(containing) + inner_s = Scope.deserialize(contained) + if expect: + assert inner_s in outer_s + else: + assert inner_s not in outer_s From d9a4190338a2171babbb0c44cd6112224190e9f4 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 7 Nov 2022 22:28:45 +0000 Subject: [PATCH 04/10] Allow optional=t/f on make_mutable This allows better scope constructor behavior from `ScopeBuilder.make_mutable`, as an optional scope can be constructed in a single step. --- changelog.d/20221104_145601_sirosen_add_scope_parsing.rst | 4 ++++ docs/scopes.rst | 4 +--- src/globus_sdk/scopes/builder.py | 6 ++++-- tests/unit/test_scopes.py | 5 +++++ 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst b/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst index 36e11c144..f3ae490b9 100644 --- a/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst +++ b/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst @@ -21,3 +21,7 @@ the consents of another scope. Containment is therefore an indication that the associated permissions of a token for a scope string are covered by the set of permissions associated with another scope string. + + * ``ScopeBuilder.make_mutable`` now accepts a keyword argument ``optional``. + This allows, for example, + ``TransferScopes.make_mutable("all", optional=True)`` diff --git a/docs/scopes.rst b/docs/scopes.rst index ccf37ffd0..3bd534ef6 100644 --- a/docs/scopes.rst +++ b/docs/scopes.rst @@ -165,10 +165,8 @@ scope builders: # create the scopes with make_mutable transfer_scope = TransferScopes.make_mutable("all") data_access_scope = GCSCollectionScopeBuilder(MAPPED_COLLECTION_ID).make_mutable( - "data_access" + "data_access", optional=True ) - # set the data_access scope to be optional - data_access_scope.optional = True # add data_access as a dependency transfer_scope.add_dependency(data_access_scope) diff --git a/src/globus_sdk/scopes/builder.py b/src/globus_sdk/scopes/builder.py index 13d99d8d9..cdf0d061a 100644 --- a/src/globus_sdk/scopes/builder.py +++ b/src/globus_sdk/scopes/builder.py @@ -126,7 +126,7 @@ def url_scope_string(self, scope_name: str) -> str: """ return f"https://auth.globus.org/scopes/{self.resource_server}/{scope_name}" - def make_mutable(self, scope: str) -> Scope: + def make_mutable(self, scope: str, *, optional: bool = False) -> Scope: """ For a given scope, create a Scope object. @@ -150,8 +150,10 @@ def make_mutable(self, scope: str) -> Scope: :param scope: The name of the scope to convert to a Scope :type scope: str + :param optional: If true, the created Scope object will be marked optional + :type optional: bool """ - return Scope(getattr(self, scope)) + return Scope(getattr(self, scope), optional=optional) def __str__(self) -> str: return f"ScopeBuilder[{self.resource_server}]\n" + "\n".join( diff --git a/tests/unit/test_scopes.py b/tests/unit/test_scopes.py index 4f8f95399..23d06c3ee 100644 --- a/tests/unit/test_scopes.py +++ b/tests/unit/test_scopes.py @@ -154,6 +154,11 @@ def test_scopebuilder_make_mutable_produces_same_strings(): assert str(sb.make_mutable("bar")) == sb.bar +def test_scopebuilder_make_mutable_can_be_optional(): + sb = ScopeBuilder(str(uuid.UUID(int=0)), known_scopes="foo") + assert str(sb.make_mutable("foo", optional=True)) == "*" + sb.foo + + def test_flows_scopes_creation(): assert FlowsScopes.resource_server == "flows.globus.org" assert ( From 71318cdc390cac9fbbb40ad124ab78525f2c0ba9 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 7 Nov 2022 23:23:20 +0000 Subject: [PATCH 05/10] Refine `Scope.__contains__` and update doc - Ensure `__contains__` is included in docs - Rebrand the `__contains__` check as "permission coverage" throughout relevant doc - Update the structure of the check to use simpler control-flow (a "reminder" comment should help make the for-else readable to those not familiar with the construct) --- ...21104_145601_sirosen_add_scope_parsing.rst | 10 ++-- docs/scopes.rst | 2 + src/globus_sdk/scopes/scope_definition.py | 50 ++++++++++++------- 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst b/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst index f3ae490b9..b6ceabf34 100644 --- a/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst +++ b/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst @@ -16,11 +16,11 @@ dependency as a string, or equivalently ``Scope(...).add_dependency(Scope("foo", optional=True))`` - * ``Scope``\s support containment checking (``Scope("foo") in Scope("bar")``), - which means that all of the consents for one scope are a strict subset of - the consents of another scope. Containment is therefore an indication that - the associated permissions of a token for a scope string are covered by - the set of permissions associated with another scope string. + * ``Scope``\s support permission coverage checking with the ``in`` and + ``not in`` operators. ``scope1 in scope2`` means that a token issued for + ``scope2`` has all of the permissions of a token issued for ``scope1``. The + check therefore can be used to validate that an existing token has the + permissions associated with a prospective request. * ``ScopeBuilder.make_mutable`` now accepts a keyword argument ``optional``. This allows, for example, diff --git a/docs/scopes.rst b/docs/scopes.rst index 3bd534ef6..5da2d4de6 100644 --- a/docs/scopes.rst +++ b/docs/scopes.rst @@ -198,6 +198,8 @@ Scope Reference .. autoclass:: Scope :members: + :special-members: + :exclude-members: __init__,__repr__,__weakref__,__str__ :show-inheritance: ScopeBuilders diff --git a/src/globus_sdk/scopes/scope_definition.py b/src/globus_sdk/scopes/scope_definition.py index fd61d78ad..b55a51853 100644 --- a/src/globus_sdk/scopes/scope_definition.py +++ b/src/globus_sdk/scopes/scope_definition.py @@ -238,23 +238,28 @@ def __str__(self) -> str: def __contains__(self, other: t.Any) -> bool: """ - scope1 in scope2 is defined as subtree matching. + ``in`` and ``not in`` are defined as permission coverage checks + + ``scope1 in scope2`` means that a token scoped for + ``scope2`` has all of the permissions of a token scoped for ``scope1``. + + A scope is covered by another scope if - A scope contains another scope if - the top level strings match - - the optional-ness matches OR only the contained scope is optional - - the dependencies of the contained scope are all contained in dependencies of - the containing scope + - the optional-ness matches OR only the covered scope is optional + - the dependencies of the covered scope are all covered by dependencies of + the covering scope Therefore, the following are true: .. code-block:: pycon - # self inclusion works + # self inclusion works, including when optional >>> Scope.deserialize("foo") in Scope.deserialize("foo") - # optional mismatches in either direction do not indicate containment + >>> Scope.deserialize("*foo") in Scope.deserialize("*foo") + # an optional scope is covered by a non-optional one, but not the reverse >>> Scope.deserialize("foo") not in Scope.deserialize("*foo") - >>> Scope.deserialize("*foo") not in Scope.deserialize("foo") + >>> Scope.deserialize("*foo") in Scope.deserialize("foo") # dependencies have the expected meanings >>> Scope.deserialize("foo") in Scope.deserialize("foo[bar]") >>> Scope.deserialize("foo[bar]") not in Scope.deserialize("foo") @@ -270,22 +275,31 @@ def __contains__(self, other: t.Any) -> bool: # top-level scope must match if self._scope_string != other._scope_string: return False - # if both are optional, okay - # if neither is optional, okay - # but if only one is optional... - if self.optional != other.optional: - # ... then make sure it is 'other' - if self.optional: - return False + + # between self.optional and other.optional, there are four possibilities, + # of which three are acceptable and one is not + # both optional and neither optional are okay, + # 'self' being non-optional and 'other' being optional is okay + # the failing case is 'other in self' when 'self' is optional and 'other' is not + # + # self.optional | other.optional | (other in self) is possible + # --------------|----------------|---------------------------- + # true | true | true + # false | false | true + # false | true | true + # true | false | false + # + # so only check for that one case + if self.optional and not other.optional: + return False # dependencies must all be contained -- search for a contrary example for other_dep in other.dependencies: - found_match = False for dep in self.dependencies: if other_dep in dep: - found_match = True break - if not found_match: + # reminder: the else branch of a for-else means that the break was never hit + else: return False # all criteria were met -- True! From b730a559fe366b161d0bf8eb2918c7a5c3ef9aaa Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 7 Nov 2022 23:51:38 +0000 Subject: [PATCH 06/10] Consolidate scope parsing tests and add more cases Several cases of scopes rejected by Globus Auth but previously accepted by this parser have been added. In particular: `foo[bar]baz` is not allowed, nor is `foo [bar]`. Both of these strings were previously allowed by the parser (but would re-serialize correctly for Globus Auth consumption). In order to support this, the lexer now also implements a "peek" value for lookahead and checks it against various possible values. This enables simpler checks than would otherwise be possible when iterating over the input string. --- src/globus_sdk/scopes/scope_definition.py | 18 ++++-- tests/unit/test_scopes.py | 68 +++++++++-------------- 2 files changed, 40 insertions(+), 46 deletions(-) diff --git a/src/globus_sdk/scopes/scope_definition.py b/src/globus_sdk/scopes/scope_definition.py index b55a51853..d73584367 100644 --- a/src/globus_sdk/scopes/scope_definition.py +++ b/src/globus_sdk/scopes/scope_definition.py @@ -43,24 +43,32 @@ def __init__(self, value: str, token_type: ParseTokenType) -> None: def _tokenize(scope_string: str) -> t.List[ParseToken]: tokens: t.List[ParseToken] = [] current_token: t.List[str] = [] - for c in scope_string: + for idx, c in enumerate(scope_string): + try: + peek: t.Optional[str] = scope_string[idx + 1] + except IndexError: + peek = None + if c in "[]* ": if current_token: tokens.append( ParseToken("".join(current_token), ParseTokenType.scope_string) ) current_token = [] + if c == "*": + if peek == " ": + raise ScopeParseError("'*' must not be followed by a space") tokens.append(ParseToken(c, ParseTokenType.opt_marker)) elif c == "[": tokens.append(ParseToken(c, ParseTokenType.lbracket)) elif c == "]": + if peek is not None and peek not in (" ", "]"): + raise ScopeParseError("']' may only be followed by a space or ']'") tokens.append(ParseToken(c, ParseTokenType.rbracket)) elif c == " ": - if tokens and tokens[-1].token_type == ParseTokenType.opt_marker: - raise ScopeParseError( - "optional marker must not be followed by a space" - ) + if peek == "[": + raise ScopeParseError("'[' cannot have a preceding space") else: raise NotImplementedError else: diff --git a/tests/unit/test_scopes.py b/tests/unit/test_scopes.py index 23d06c3ee..8f2582f3f 100644 --- a/tests/unit/test_scopes.py +++ b/tests/unit/test_scopes.py @@ -173,53 +173,39 @@ def test_scope_parsing_rejects_optional_marker_followed_by_space(): Scope.parse("* foo") # not okay -def test_scope_parsing_rejects_ending_in_optional_marker(): - with pytest.raises(ScopeParseError): - Scope.parse("foo*") - - def test_scope_parsing_allows_empty_string(): scopes = Scope.parse("") assert scopes == [] -def test_scope_parsing_does_not_allow_optional_marker_before_brackets(): - with pytest.raises(ScopeParseError): - Scope.parse("foo*[bar]") - with pytest.raises(ScopeParseError): - Scope.parse("foo *[bar]") - with pytest.raises(ScopeParseError): - Scope.parse("foo [bar*]") - - -def test_scope_parsing_does_not_allow_ending_in_open_bracket(): - with pytest.raises(ScopeParseError): - Scope.parse("foo[") - - -def test_scope_parsing_does_not_allow_empty_brackets(): - with pytest.raises(ScopeParseError): - Scope.parse("foo[]") - - -def test_scope_parsing_does_not_allow_starting_with_open_bracket(): - with pytest.raises(ScopeParseError): - Scope.parse("[foo]") - - -def test_scope_parsing_does_not_allow_double_open_bracket(): - with pytest.raises(ScopeParseError): - Scope.parse("foo[[bar]]") - - -def test_scope_parsing_does_not_allow_unbalanced_brackets_closing(): - with pytest.raises(ScopeParseError): - Scope.parse("foo[bar]]") - - -def test_scope_parsing_does_not_allow_unbalanced_brackets_missing_close(): +@pytest.mark.parametrize( + "scopestring", + [ + # ending in '*' + "foo*", + # '*' followed by '[]' + "foo*[bar]", + "foo *[bar]", + "foo [bar*]", + # empty brackets + "foo[]", + # starting with open bracket + "[foo]", + # double brackets + "foo[[bar]]", + # unbalanced brackets + "foo[", + "foo[bar", + "foo[bar]]", + # space before brackets + "foo [bar]", + # missing space before next scope string after ']' + "foo[bar]baz", + ], +) +def test_scope_parsing_rejects_bad_inputs(scopestring): with pytest.raises(ScopeParseError): - Scope.parse("foo[bar") + Scope.parse(scopestring) def test_scope_deserialize_simple(): From c2dcebaf0b6681d61881e79fcd390b6324a81863 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Tue, 8 Nov 2022 17:42:51 +0000 Subject: [PATCH 07/10] Add more test cases for scope parsing --- tests/unit/test_scopes.py | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_scopes.py b/tests/unit/test_scopes.py index 8f2582f3f..df761a4c1 100644 --- a/tests/unit/test_scopes.py +++ b/tests/unit/test_scopes.py @@ -167,36 +167,59 @@ def test_flows_scopes_creation(): ) -def test_scope_parsing_rejects_optional_marker_followed_by_space(): - Scope.parse("*foo") # okay - with pytest.raises(ScopeParseError): - Scope.parse("* foo") # not okay - - def test_scope_parsing_allows_empty_string(): scopes = Scope.parse("") assert scopes == [] +@pytest.mark.parametrize( + "scope_string1,scope_string2", + [ + ("foo ", "foo"), + (" foo", "foo"), + ("foo[ bar]", "foo[bar]"), + ], +) +def test_scope_parsing_ignores_non_semantic_whitespace(scope_string1, scope_string2): + list1 = Scope.parse(scope_string1) + list2 = Scope.parse(scope_string2) + assert len(list1) == len(list2) == 1 + s1, s2 = list1[0], list2[0] + # Scope.__eq__ is not defined, so equivalence checking is manual (and somewhat error + # prone) for now + assert s1._scope_string == s2._scope_string + assert s1.optional == s2.optional + for i in range(len(s1.dependencies)): + assert s1.dependencies[i]._scope_string == s2.dependencies[i]._scope_string + assert s1.dependencies[i].optional == s2.dependencies[i].optional + + @pytest.mark.parametrize( "scopestring", [ # ending in '*' "foo*", - # '*' followed by '[]' + "foo *", + # '*' followed by '[] ' "foo*[bar]", "foo *[bar]", "foo [bar*]", + "foo * ", + "* foo", # empty brackets "foo[]", # starting with open bracket "[foo]", # double brackets "foo[[bar]]", - # unbalanced brackets + # unbalanced open brackets "foo[", "foo[bar", + # unbalanced close brackets + "foo]", + "foo bar]", "foo[bar]]", + "foo[bar] baz]", # space before brackets "foo [bar]", # missing space before next scope string after ']' From 6e493ee68f32b20972d00743b0696966b2b2a53d Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Fri, 11 Nov 2022 19:04:14 +0000 Subject: [PATCH 08/10] Make Scope "contains" a private helper method We retain all of the tests, but the method is renamed to `_contains`, removed from public docs, and commented as a "non-authoritative convenience function". In particular, after discussion we do not believe that it is valuable to try to build fully accurate client-side scope comparison when this cannot be authoratative with respect to a user's current consents in Auth. For now, the method will exist and it will be available for our own use-cases (e.g. in globus-cli), but not for general use. Also, improve documentation of `add_dependency` to avoid (confusing) chaining usage. --- ...21104_145601_sirosen_add_scope_parsing.rst | 6 ---- docs/scopes.rst | 18 ++++++----- src/globus_sdk/scopes/scope_definition.py | 31 ++++++++++++------- tests/unit/test_scopes.py | 12 ++----- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst b/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst index b6ceabf34..0fb3a7ea3 100644 --- a/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst +++ b/changelog.d/20221104_145601_sirosen_add_scope_parsing.rst @@ -16,12 +16,6 @@ dependency as a string, or equivalently ``Scope(...).add_dependency(Scope("foo", optional=True))`` - * ``Scope``\s support permission coverage checking with the ``in`` and - ``not in`` operators. ``scope1 in scope2`` means that a token issued for - ``scope2`` has all of the permissions of a token issued for ``scope1``. The - check therefore can be used to validate that an existing token has the - permissions associated with a prospective request. - * ``ScopeBuilder.make_mutable`` now accepts a keyword argument ``optional``. This allows, for example, ``TransferScopes.make_mutable("all", optional=True)`` diff --git a/docs/scopes.rst b/docs/scopes.rst index 5da2d4de6..c36c96b83 100644 --- a/docs/scopes.rst +++ b/docs/scopes.rst @@ -186,20 +186,24 @@ method. Note that ``__str__`` for a ``Scope`` is just an alias for .. code-block:: pycon >>> from globus_sdk.scopes import Scope - >>> str(Scope("foo").add_dependency("bar").add_dependency("*baz")) + >>> foo = Scope("foo") + >>> bar = Scope("bar") + >>> bar.add_dependency("baz") + >>> foo.add_dependency(bar) + >>> print(str(Scope("foo"))) foo[bar *baz] - >>> Scope("foo").add_dependency("bar").serialize() - foo[bar] - >>> repr(Scope("foo").add_dependency("*bar")) - Scope("foo", dependencies=[Scope("bar", optional=True)]) + >>> print(bar.serialize()) + bar[baz] + >>> alpha = Scope("alpha") + >>> alpha.add_dependency("*beta") + >>> print(repr(alpha)) + Scope("alpha", dependencies=[Scope("beta", optional=True)]) Scope Reference ~~~~~~~~~~~~~~~ .. autoclass:: Scope :members: - :special-members: - :exclude-members: __init__,__repr__,__weakref__,__str__ :show-inheritance: ScopeBuilders diff --git a/src/globus_sdk/scopes/scope_definition.py b/src/globus_sdk/scopes/scope_definition.py index d73584367..992b45469 100644 --- a/src/globus_sdk/scopes/scope_definition.py +++ b/src/globus_sdk/scopes/scope_definition.py @@ -244,8 +244,16 @@ def __repr__(self) -> str: def __str__(self) -> str: return self.serialize() - def __contains__(self, other: t.Any) -> bool: + def _contains(self, other: t.Any) -> bool: """ + .. warning:: + + The ``_contains`` method is a non-authoritative convenience for comparing + parsed scopes. Although the essence and intent of the check is summarized + below, there is no guarantee that it correctly reflects the permissions of a + token or tokens. The structure of the data for a given consent in Globus + Auth is not perfectly reflected in the parse tree. + ``in`` and ``not in`` are defined as permission coverage checks ``scope1 in scope2`` means that a token scoped for @@ -262,19 +270,20 @@ def __contains__(self, other: t.Any) -> bool: .. code-block:: pycon + >>> s = lambda x: Scope.deserialize(x) # define this for brevity below # self inclusion works, including when optional - >>> Scope.deserialize("foo") in Scope.deserialize("foo") - >>> Scope.deserialize("*foo") in Scope.deserialize("*foo") + >>> s("foo")._contains(s("foo")) + >>> s("*foo")._contains(s("*foo")) # an optional scope is covered by a non-optional one, but not the reverse - >>> Scope.deserialize("foo") not in Scope.deserialize("*foo") - >>> Scope.deserialize("*foo") in Scope.deserialize("foo") + >>> not s("foo")._contains(s("*foo")) + >>> s("*foo")._contains(s("foo")) # dependencies have the expected meanings - >>> Scope.deserialize("foo") in Scope.deserialize("foo[bar]") - >>> Scope.deserialize("foo[bar]") not in Scope.deserialize("foo") - >>> Scope.deserialize("foo[bar]") in Scope.deserialize("foo[bar[baz]]") + >>> s("foo")._contains(s("foo[bar]")) + >>> not s("foo[bar]")._contains(s("foo")) + >>> s("foo[bar]")._contains(s("foo[bar[baz]]")) # dependencies are not transitive and obey "optionalness" matching - >>> Scope.deserialize("foo[bar]") not in Scope.deserialize("foo[fizz[bar]]") - >>> Scope.deserialize("foo[bar]") not in Scope.deserialize("foo[*bar]") + >>> not s("foo[bar]")._contains(s("foo[fizz[bar]]")) + >>> not s("foo[bar]")._contains(s("foo[*bar]")) """ # scopes can only contain other scopes if not isinstance(other, Scope): @@ -304,7 +313,7 @@ def __contains__(self, other: t.Any) -> bool: # dependencies must all be contained -- search for a contrary example for other_dep in other.dependencies: for dep in self.dependencies: - if other_dep in dep: + if dep._contains(other_dep): break # reminder: the else branch of a for-else means that the break was never hit else: diff --git a/tests/unit/test_scopes.py b/tests/unit/test_scopes.py index df761a4c1..e210f5592 100644 --- a/tests/unit/test_scopes.py +++ b/tests/unit/test_scopes.py @@ -260,7 +260,7 @@ def test_scope_init_forbids_special_chars(scope_str): def test_scope_contains_requires_scope_objects(): s = Scope("foo") - assert "foo" not in s + assert not s._contains("foo") @pytest.mark.parametrize( @@ -292,10 +292,7 @@ def test_scope_contains_requires_scope_objects(): def test_scope_contains_simple_cases(contained, containing, expect): outer_s = Scope.deserialize(containing) inner_s = Scope.deserialize(contained) - if expect: - assert inner_s in outer_s - else: - assert inner_s not in outer_s + assert outer_s._contains(inner_s) == expect @pytest.mark.parametrize( @@ -357,7 +354,4 @@ def test_scope_contains_simple_cases(contained, containing, expect): def test_scope_contains_complex_usages(contained, containing, expect): outer_s = Scope.deserialize(containing) inner_s = Scope.deserialize(contained) - if expect: - assert inner_s in outer_s - else: - assert inner_s not in outer_s + assert outer_s._contains(inner_s) == expect From 6a97871b24709d4d381a9f838a67ad3858e6c4f3 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 14 Nov 2022 16:35:56 +0000 Subject: [PATCH 09/10] Add scope parsing tests which only assert validity These tests do not check anything *about* the parsed scopes. Only that the strings which are being fed to `parse()` are valid. --- tests/unit/test_scopes.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/unit/test_scopes.py b/tests/unit/test_scopes.py index e210f5592..16c396baa 100644 --- a/tests/unit/test_scopes.py +++ b/tests/unit/test_scopes.py @@ -231,6 +231,18 @@ def test_scope_parsing_rejects_bad_inputs(scopestring): Scope.parse(scopestring) +@pytest.mark.parametrize( + "scopestring", + ("foo", "*foo", "foo[bar]", "foo[*bar]", "foo bar", "foo[bar[baz]]"), +) +def test_scope_parsing_accepts_valid_inputs(scopestring): + # test *only* that parsing does not error and returns a non-empty list of scopes + scopes = Scope.parse(scopestring) + assert isinstance(scopes, list) + assert len(scopes) > 0 + assert isinstance(scopes[0], Scope) + + def test_scope_deserialize_simple(): scope = Scope.deserialize("foo") assert str(scope) == "foo" From e6f6dd6518f9769a6288118fb99ee18557dbed88 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 14 Nov 2022 23:41:34 +0000 Subject: [PATCH 10/10] Test scope parsing on service formatted scopes --- src/globus_sdk/scopes/scope_definition.py | 2 +- tests/unit/test_scopes.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/globus_sdk/scopes/scope_definition.py b/src/globus_sdk/scopes/scope_definition.py index 992b45469..c1112b152 100644 --- a/src/globus_sdk/scopes/scope_definition.py +++ b/src/globus_sdk/scopes/scope_definition.py @@ -135,7 +135,7 @@ def _parse_tokens(tokens: t.List[ParseToken]) -> t.List["Scope"]: class Scope: """ - A mutable scope is a representation of a scope which allows modifications to be + A scope object is a representation of a scope which allows modifications to be made. In particular, it supports handling scope dependencies via ``add_dependency``. diff --git a/tests/unit/test_scopes.py b/tests/unit/test_scopes.py index 16c396baa..a53ba8c93 100644 --- a/tests/unit/test_scopes.py +++ b/tests/unit/test_scopes.py @@ -243,6 +243,23 @@ def test_scope_parsing_accepts_valid_inputs(scopestring): assert isinstance(scopes[0], Scope) +@pytest.mark.parametrize("rs_name", (str(uuid.UUID(int=0)), "example.globus.org")) +@pytest.mark.parametrize("scope_format", ("urn", "url")) +def test_scope_parsing_can_consume_scopebuilder_results(rs_name, scope_format): + sb = ScopeBuilder(rs_name) + if scope_format == "urn": + scope_string = sb.urn_scope_string("foo") + expect_string = f"urn:globus:auth:scope:{rs_name}:foo" + elif scope_format == "url": + scope_string = sb.url_scope_string("foo") + expect_string = f"https://auth.globus.org/scopes/{rs_name}/foo" + else: + raise NotImplementedError + + scope = Scope.deserialize(scope_string) + assert str(scope) == expect_string + + def test_scope_deserialize_simple(): scope = Scope.deserialize("foo") assert str(scope) == "foo"