Skip to content

Commit

Permalink
Remove legacy Automate Transfer AP scope from timers scope dependenci…
Browse files Browse the repository at this point in the history
…es (#1050)

* removed legacy automate transfer scope

* updated tests with new scope requirements
  • Loading branch information
MaxTueckeGlobus authored Nov 8, 2024
1 parent 27b3a64 commit 8de1f23
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 31 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Bugfixes

* Removed the legacy Automate Transfer AP scope from Timers' scope dependencies.
* Updated `CURRENT_SCOPE_CONTRACT_VERSION` and Timers' `min_contract_version` from 1 to 2.
1 change: 0 additions & 1 deletion src/globus_cli/commands/timer/create/transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,6 @@ def _derive_needed_scopes(
target_scope = GCSCollectionScopeBuilder(target).data_access
scopes_needed[target] = _ez_make_nested_scope(
globus_sdk.TimerClient.scopes.timer,
"https://auth.globus.org/scopes/actions.globus.org/transfer/transfer",
globus_sdk.TransferClient.scopes.all,
target_scope,
)
Expand Down
13 changes: 3 additions & 10 deletions src/globus_cli/login_manager/scopes.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@

from globus_cli.types import ServiceNameLiteral

TRANSFER_AP_SCOPE_STR: str = (
"https://auth.globus.org/scopes/actions.globus.org/transfer/transfer"
)


def compute_timer_scope(
*, data_access_collection_ids: t.Sequence[str] | None = None
Expand All @@ -29,11 +25,8 @@ def compute_timer_scope(
Scope(GCSCollectionScopeBuilder(cid).data_access, optional=True)
)

transfer_ap_scope = Scope(TRANSFER_AP_SCOPE_STR)
transfer_ap_scope.add_dependency(transfer_scope)

timer_scope = Scope(TimersScopes.timer)
timer_scope.add_dependency(transfer_ap_scope)
timer_scope.add_dependency(transfer_scope)
return timer_scope


Expand Down Expand Up @@ -87,7 +80,7 @@ def __init__(self) -> None:
],
}
self["timer"] = {
"min_contract_version": 1,
"min_contract_version": 2,
"resource_server": TimersScopes.resource_server,
"nice_server_name": "Globus Timers",
"scopes": [
Expand Down Expand Up @@ -125,4 +118,4 @@ def get_by_resource_server(self, rs_name: str) -> _ServiceRequirement:
# version we were at when we got a token
# it should be the max of the version numbers required by the various different
# services
CURRENT_SCOPE_CONTRACT_VERSION: t.Final[int] = 1
CURRENT_SCOPE_CONTRACT_VERSION: t.Final[int] = 2
14 changes: 2 additions & 12 deletions tests/functional/timer/test_transfer_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,28 +61,18 @@ def setup_timer_consent_tree_response(identity_id, *data_access_collection_ids):
**_dummy_consent_fields,
},
{
"scope_name": (
"https://auth.globus.org/scopes/"
"actions.globus.org/transfer/transfer"
),
"scope_name": globus_sdk.TransferClient.scopes.all,
"scope": str(uuid.uuid1()),
"dependency_path": [100, 101],
"id": 101,
**_dummy_consent_fields,
},
{
"scope_name": globus_sdk.TransferClient.scopes.all,
"scope": str(uuid.uuid1()),
"dependency_path": [100, 101, 102],
"id": 102,
**_dummy_consent_fields,
},
]
+ [
{
"scope_name": GCSCollectionScopeBuilder(name).data_access,
"scope": str(uuid.uuid1()),
"dependency_path": [100, 101, 102, 1000 + idx],
"dependency_path": [100, 101, 1000 + idx],
"id": 1000 + idx,
**_dummy_consent_fields,
}
Expand Down
12 changes: 4 additions & 8 deletions tests/unit/test_login_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ def urlfmt_scope(rs: str, name: str) -> str:


BASE_TIMER_SCOPE = urlfmt_scope("524230d7-ea86-4a52-8312-86065a9e0417", "timer")
TRANSFER_AP_SCOPE = urlfmt_scope("actions.globus.org", "transfer/transfer")


def test_requires_login_success(patch_scope_requirements, patched_tokenstorage):
Expand Down Expand Up @@ -297,7 +296,7 @@ def test_compute_timer_scope_no_data_access():

computed = str(compute_timer_scope())
assert computed.startswith(BASE_TIMER_SCOPE)
assert computed == f"{BASE_TIMER_SCOPE}[{TRANSFER_AP_SCOPE}[{transfer_scope}]]"
assert computed == f"{BASE_TIMER_SCOPE}[{transfer_scope}]"


def test_compute_timer_scope_one_data_access():
Expand All @@ -307,10 +306,7 @@ def test_compute_timer_scope_one_data_access():
computed = str(compute_timer_scope(data_access_collection_ids=["foo"]))
assert computed.startswith(BASE_TIMER_SCOPE)
assert foo_scope in computed
assert (
computed
== f"{BASE_TIMER_SCOPE}[{TRANSFER_AP_SCOPE}[{transfer_scope}[*{foo_scope}]]]"
)
assert computed == f"{BASE_TIMER_SCOPE}[{transfer_scope}[*{foo_scope}]]"


def test_compute_timer_scope_multiple_data_access():
Expand All @@ -326,8 +322,8 @@ def test_compute_timer_scope_multiple_data_access():
assert foo_scope in computed
assert bar_scope in computed
assert baz_scope in computed
start_part = f"{BASE_TIMER_SCOPE}[{TRANSFER_AP_SCOPE}[{transfer_scope}["
end_part = "]]]"
start_part = f"{BASE_TIMER_SCOPE}[{transfer_scope}["
end_part = "]]"
assert computed == f"{start_part}*{foo_scope} *{bar_scope} *{baz_scope}{end_part}"


Expand Down

0 comments on commit 8de1f23

Please sign in to comment.