Skip to content

Commit

Permalink
Make Scope "contains" a private helper method
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sirosen committed Nov 11, 2022
1 parent c2dceba commit 6e493ee
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 33 deletions.
6 changes: 0 additions & 6 deletions changelog.d/20221104_145601_sirosen_add_scope_parsing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)``
18 changes: 11 additions & 7 deletions docs/scopes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 20 additions & 11 deletions src/globus_sdk/scopes/scope_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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:
Expand Down
12 changes: 3 additions & 9 deletions tests/unit/test_scopes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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

0 comments on commit 6e493ee

Please sign in to comment.