Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

globus api --scope-string (for clients) #906

Merged
merged 5 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions changelog.d/20231206_130249_derek_api_scopes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@

### Enhancements

* The ``globus api <service>`` command now supports a ``--scope-string`` parameter.

* If supplied, the CLI will enforce that any specified scope strings are included
in consent requirements *in addition to* standard service scope requirements.

* This parameter may be supplied multiple times to specify multiple scope strings.

* This parameter is only supported in the context of Client Credentials-based authentication.
([Client Credentials with GLOBUS_CLI_CLIENT_ID](https://docs.globus.org/cli/environment_variables/#client_credentials_with_globus_cli_client_id))
36 changes: 35 additions & 1 deletion src/globus_cli/commands/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import globus_sdk

from globus_cli import termio, version
from globus_cli.login_manager import LoginManager
from globus_cli.login_manager import LoginManager, is_client_login
from globus_cli.login_manager.scopes import CLI_SCOPE_REQUIREMENTS
from globus_cli.parsing import command, group, mutex_option_group
from globus_cli.termio import display
Expand Down Expand Up @@ -124,6 +124,20 @@ def print_error_or_response(
display(data, simple_text=data.text)


def _get_resource_server(service_name: str) -> str:
_resource_server = {
"auth": globus_sdk.AuthClient.resource_server,
"flows": globus_sdk.FlowsClient.resource_server,
"groups": globus_sdk.GroupsClient.resource_server,
"search": globus_sdk.SearchClient.resource_server,
"transfer": globus_sdk.TransferClient.resource_server,
"timer": globus_sdk.TimerClient.resource_server,
}.get(service_name)
if _resource_server is None:
raise NotImplementedError(f"unrecognized service: {service_name}")
return _resource_server


def _get_client(
login_manager: LoginManager, service_name: str
) -> globus_sdk.BaseClient:
Expand Down Expand Up @@ -240,6 +254,16 @@ def build_command(service_name: ServiceNameLiteral) -> click.Command:
),
)
@click.option("--no-retry", is_flag=True, help="Disable built-in request retries")
@click.option(
"--scope-string",
type=str,
multiple=True,
help=(
"A scope string to consent to for this command (only supported for "
"confidential client usage). "
"Pass this option multiple times to specify multiple scopes."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommending removal of "for this command". I've rewrapped the lines as well.

Suggested change
"A scope string to consent to for this command (only supported for "
"confidential client usage). "
"Pass this option multiple times to specify multiple scopes."
"A scope string to consent to "
"(only supported for confidential client usage). "
"Pass this option multiple times to specify multiple scopes."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this based on the standup conversation yesterday around "we'll keep using this cached token which now has expanded consents"? Or is this more of just trying to make it read cleaner?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only about readability -- "to consent to for" caught my eye.

),
)
@mutex_option_group("--body", "--body-file")
def service_command(
login_manager: LoginManager,
Expand All @@ -254,6 +278,7 @@ def service_command(
allow_errors: bool,
allow_redirects: bool,
no_retry: bool,
scope_string: tuple[str, ...],
) -> None:
# the overall flow of this command will be as follows:
# - prepare a client
Expand All @@ -264,6 +289,15 @@ def service_command(
# - on success or error with --allow-errors, print
# - on error without --allow-errors, reraise

if scope_string:
if not is_client_login():
raise click.UsageError(
"Scope requirements (--scope-string) are currently only "
"supported for confidential-client authorized calls."
)
resource_server = _get_resource_server(service_name)
login_manager.add_requirement(resource_server, scope_string)

client = _get_client(login_manager, service_name)
client.app_name = version.app_name + " raw-api-command"
if no_retry:
Expand Down
9 changes: 8 additions & 1 deletion src/globus_cli/login_manager/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,13 @@ def has_login(self, resource_server: str) -> bool:

return self._validate_token(tokens["refresh_token"])

def _tokens_meet_auth_requirements(
self, resource_server: str, tokens: dict[str, t.Any]
) -> bool:
return self._tokens_meet_static_requirements(
resource_server, tokens
) and self._tokens_meet_nonstatic_requirements(resource_server, tokens)
derek-globus marked this conversation as resolved.
Show resolved Hide resolved

def _tokens_meet_static_requirements(
self, resource_server: str, tokens: dict[str, t.Any]
) -> bool:
Expand Down Expand Up @@ -309,7 +316,7 @@ def _get_client_authorizer(
# or for another client, but automatic retries will handle that
access_token = None
expires_at = None
if tokens:
if tokens and self._tokens_meet_auth_requirements(resource_server, tokens):
access_token = tokens["access_token"]
expires_at = tokens["expires_at_seconds"]

Expand Down
27 changes: 27 additions & 0 deletions tests/functional/test_api.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import urllib.parse

import pytest
import responses
from globus_sdk._testing import (
RegisteredResponse,
get_last_request,
Expand Down Expand Up @@ -105,3 +106,29 @@ def test_api_command_query_params_multiple_become_list(run_line):
parsed_query_string = urllib.parse.parse_qs(parsed_url.query)
assert list(parsed_query_string.keys()) == ["filter"]
assert set(parsed_query_string["filter"]) == {"frobulated", "demuddled", "reversed"}


def test_api_command_with_scope_strings(monkeypatch, client_login, run_line):
load_response("cli.api.transfer_stub")
load_response("auth.oauth2_client_credentials_tokens")

run_line("globus api transfer get /foo --scope-string foobarjohn")

token_grant = [
kall for kall in responses.calls if kall.request.url.endswith("/token")
derek-globus marked this conversation as resolved.
Show resolved Hide resolved
][0]
request_params = urllib.parse.parse_qs(token_grant.request.body)
assert request_params["grant_type"][0] == "client_credentials"
scopes = request_params["scope"][0].split(" ")
# This is the default transfer scope, inherited through the service name.
assert "urn:globus:auth:scope:transfer.api.globus.org:all" in scopes
# This is the scope string we explicitly passed in.
assert "foobarjohn" in scopes


def test_api_command_rejects_non_client_based_scope_strings(run_line):
result = run_line(
"globus api auth GET /v2/api/projects --scope-string foobarjohn",
assert_exit_code=2,
)
assert "only supported for confidential-client authorized calls" in result.stderr