diff --git a/campus_cli/auth/client.py b/campus_cli/auth/client.py index 8cd5cb1..bb825bd 100644 --- a/campus_cli/auth/client.py +++ b/campus_cli/auth/client.py @@ -41,6 +41,11 @@ def _format_client(client) -> dict: # so admins need to see which clients have none registered. "redirect_uris": client.redirect_uris or [], "is_public": client.is_public, + # Fail-closed admin fields (docs/auth-token-invariants.md A1/B3/C1): + # admins vetting a client need to see exactly what it may hold. + "allowed_scopes": client.allowed_scopes or [], + "upstream_scopes": client.upstream_scopes or {}, + "token_bridge": bool(client.token_bridge), } @@ -63,6 +68,7 @@ def _print_client_details(client_data: dict) -> None: console.print(f" - {vault}: {access}") _print_client_redirect_uris(client_data) + _print_client_scopes(client_data) def _print_client_redirect_uris(client_data: dict, indent: str = "") -> None: @@ -80,6 +86,30 @@ def _print_client_redirect_uris(client_data: dict, indent: str = "") -> None: ) +def _print_client_scopes(client_data: dict, indent: str = "") -> None: + """Print the scope allowlists and token-bridge flag.""" + allowed = client_data.get("allowed_scopes") or [] + console.print(f"{indent}[bold]Allowed scopes:[/bold]") + if allowed: + for scope in allowed: + console.print(f"{indent} - {scope}") + else: + console.print(f"{indent} (none)") + upstream = client_data.get("upstream_scopes") or {} + console.print(f"{indent}[bold]Upstream scopes:[/bold]") + if upstream: + for provider, scopes in upstream.items(): + console.print(f"{indent} {provider}:") + for scope in scopes: + console.print(f"{indent} - {scope}") + else: + console.print(f"{indent} (none)") + console.print( + f"{indent}[bold]Token bridge:[/bold] " + f"{'yes' if client_data.get('token_bridge') else 'no'}" + ) + + @client_app.command("list") def client_list( output_json: bool = typer.Option(False, "--json", help="Output as JSON"), @@ -256,6 +286,26 @@ def client_get( raise typer.Exit(1) from e +def _group_upstream_scopes(values: list[str]) -> dict[str, list[str]]: + """Group repeated --upstream-scope values by provider. + + Each value must be `provider=scope`; the scope URL may contain '='. + Providers keep first-seen order; scopes keep flag order. + """ + grouped: dict[str, list[str]] = {} + for value in values: + provider, sep, scope = value.partition("=") + if not sep or not provider or not scope: + print_error( + f"Invalid --upstream-scope {value!r}: expected" + " provider=scope (e.g." + " google=https://www.googleapis.com/auth/classroom.rosters)." + ) + raise typer.Exit(1) + grouped.setdefault(provider, []).append(scope) + return grouped + + @client_app.command("update") def client_update( client_id: str = typer.Option(..., "--client-id", "-i", help="Client ID"), @@ -269,21 +319,60 @@ def client_update( help="OAuth redirect URI (repeat for multiple; REPLACES the existing" " list, so re-pass the full list when adding one)", ), + allowed_scope: list[str] = typer.Option( # noqa: B008 + [], + "--allowed-scope", + help="Scope this client may be granted on campus tokens (repeat for" + " multiple; REPLACES the whole allowlist, so re-pass every scope the" + " client must keep). Fail-closed: an empty allowlist grants nothing", + ), + clear_allowed_scopes: bool = typer.Option( + False, + "--clear-allowed-scopes", + help="Empty the scope allowlist (fail-closed: grants nothing)", + ), + upstream_scope: list[str] = typer.Option( # noqa: B008 + [], + "--upstream-scope", + help="Upstream scope as provider=scope (repeat for multiple;" + " scopes of the same provider are grouped; REPLACES the whole" + " upstream scope map, so re-pass every provider=scope pair the" + " client must keep)", + ), + token_bridge: bool | None = typer.Option( # noqa: B008 + None, + "--token-bridge/--no-token-bridge", + help="Grant/revoke broker token-bridge access (confidential clients" + " only; the server rejects it for public clients)", + ), output_json: bool = typer.Option(False, "--json", help="Output as JSON"), dry_run: bool = dry_run_option(), ) -> None: """ Update an OAuth client. - Updates the name, description and/or redirect URIs of an existing - client. --redirect-uri replaces the whole redirect URI list: the - authorization endpoint will require an exact match (RFC 6749 - §3.1.2.2), so re-pass every URI the client must keep. + Updates the name, description, redirect URIs, scope allowlists and/or + token-bridge access of an existing client. List-valued options + (--redirect-uri, --allowed-scope, --upstream-scope) REPLACE the whole + stored value: the API layer full-replaces each provided field, so + re-pass every entry the client must keep. Fields you omit are left + untouched. """ - if not name and not description and not redirect_uri: + if clear_allowed_scopes and allowed_scope: + print_error( + "--clear-allowed-scopes cannot be combined with --allowed-scope." + ) + raise typer.Exit(1) + + upstream_scopes = _group_upstream_scopes(list(upstream_scope)) + + if not (name or description or redirect_uri or allowed_scope + or clear_allowed_scopes or upstream_scope + or token_bridge is not None): print_error( - "At least one of --name, --description or --redirect-uri " - "must be provided." + "At least one of --name, --description, --redirect-uri," + " --allowed-scope, --clear-allowed-scopes, --upstream-scope or" + " --token-bridge must be provided." ) raise typer.Exit(1) @@ -295,6 +384,14 @@ def client_update( update_kwargs.append(f"description={description!r}") if redirect_uri: update_kwargs.append(f"redirect_uris={list(redirect_uri)!r}") + if allowed_scope: + update_kwargs.append(f"allowed_scopes={list(allowed_scope)!r}") + if clear_allowed_scopes: + update_kwargs.append("allowed_scopes=[]") + if upstream_scope: + update_kwargs.append(f"upstream_scopes={upstream_scopes!r}") + if token_bridge is not None: + update_kwargs.append(f"token_bridge={token_bridge}") print_python_api( "campus client update", [ @@ -315,6 +412,14 @@ def client_update( update_kwargs["description"] = description if redirect_uri: update_kwargs["redirect_uris"] = list(redirect_uri) + if allowed_scope: + update_kwargs["allowed_scopes"] = list(allowed_scope) + elif clear_allowed_scopes: + update_kwargs["allowed_scopes"] = [] + if upstream_scope: + update_kwargs["upstream_scopes"] = upstream_scopes + if token_bridge is not None: + update_kwargs["token_bridge"] = token_bridge client = api.auth_clients[client_id].update(**update_kwargs) result = _format_client(client) diff --git a/poetry.lock b/poetry.lock index 0ccf821..4194c02 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 2.4.1 and should not be changed by hand. +# This file is automatically @generated by Poetry 2.5.1 and should not be changed by hand. [[package]] name = "annotated-doc" @@ -140,7 +140,7 @@ files = [ [package.dependencies] jmespath = ">=0.7.1,<2.0.0" python-dateutil = ">=2.1,<3.0.0" -urllib3 = ">=1.25.4,<2.2.0 || >2.2.0,<3" +urllib3 = ">=1.25.4,!=2.2.0,<3" [package.extras] crt = ["awscrt (==0.32.2)"] @@ -163,7 +163,7 @@ flask = "^3.0.0" type = "git" url = "https://github.com/nyjc-computing/campus-api-python.git" reference = "main" -resolved_reference = "74da7c0ac378cd0b21cb9bc3cbb8d5fcb563fad3" +resolved_reference = "aece1067471cbc27c83b29ebc96db3e666a4e62a" [[package]] name = "campus-suite" @@ -190,7 +190,7 @@ werkzeug = "^3.0.0" type = "git" url = "https://github.com/nyjc-computing/campus.git" reference = "weekly" -resolved_reference = "52fc3bb85088817e519ecf989c9e42b957a06b64" +resolved_reference = "08d901cefe49f560324745415e9428d550f7cb3e" [[package]] name = "certifi" diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py index 3139977..5342668 100644 --- a/tests/integration/test_cli.py +++ b/tests/integration/test_cli.py @@ -24,6 +24,9 @@ def _mock_client(**overrides): permissions={}, is_public=False, redirect_uris=[], + allowed_scopes=[], + upstream_scopes={}, + token_bridge=False, ) # `name` as a Mock kwarg names the mock itself; the attribute needs # setting explicitly or .name is a child Mock (breaks JSON output). @@ -251,6 +254,51 @@ def test_client_get_json_includes_redirect_uris_and_is_public(): assert payload["is_public"] is True +def test_client_get_json_includes_scope_and_bridge_fields(): + """campus-cli#24: client get --json carries the fail-closed fields.""" + with patch("campus_cli.auth.client.get_api_client") as mock_get: + mock_get.return_value.auth_clients["uid-client-test1234"].get.return_value = ( + _mock_client( + allowed_scopes=["openid"], + upstream_scopes={"google": ["https://a.example/calendar"]}, + token_bridge=True, + ) + ) + + result = runner.invoke( + app, ["client", "get", "--client-id", "uid-client-test1234", "--json"] + ) + + assert result.exit_code == 0 + payload = json_loads(result.stdout) + assert payload["allowed_scopes"] == ["openid"] + assert payload["upstream_scopes"] == {"google": ["https://a.example/calendar"]} + assert payload["token_bridge"] is True + + +def test_client_get_table_shows_scope_and_bridge_fields(): + """campus-cli#24: table mode prints allowlists and the bridge flag.""" + with patch("campus_cli.auth.client.get_api_client") as mock_get: + mock_get.return_value.auth_clients["uid-client-test1234"].get.return_value = ( + _mock_client( + allowed_scopes=["openid"], + upstream_scopes={"google": ["https://a.example/calendar"]}, + token_bridge=True, + ) + ) + + result = runner.invoke( + app, ["client", "get", "--client-id", "uid-client-test1234"] + ) + + assert result.exit_code == 0 + assert "Allowed scopes:" in result.stdout + assert "- openid" in result.stdout + assert "Upstream scopes:" in result.stdout + assert "google:" in result.stdout + assert "Token bridge: yes" in result.stdout + + def test_client_list_json_includes_redirect_uris_and_is_public(): """issue #21: client list --json no longer drops redirect_uris/is_public.""" with patch("campus_cli.auth.client.get_api_client") as mock_get: diff --git a/tests/integration/test_client_update.py b/tests/integration/test_client_update.py index d37d66b..4d6741d 100644 --- a/tests/integration/test_client_update.py +++ b/tests/integration/test_client_update.py @@ -2,7 +2,9 @@ The command must forward only the fields the admin passed — including --redirect-uri, which the API layer replaces wholesale — and reject a -call that would update nothing. +call that would update nothing. campus-cli#24 adds the fail-closed +scope/bridge fields (allowed_scopes, upstream_scopes, token_bridge) +under the same contract. """ from json import loads as json_loads @@ -25,6 +27,9 @@ def _updated_client(**overrides): "created_at": "2026-09-30T06:31:24+00:00", "permissions": {"campus.api": 1}, "redirect_uris": [], + "allowed_scopes": [], + "upstream_scopes": {}, + "token_bridge": False, "is_public": False, } fields.update(overrides) @@ -93,3 +98,117 @@ def test_update_json_output_includes_redirect_uris(): payload = json_loads(result.stdout) assert payload["redirect_uris"] == ["https://a/cb"] assert payload["is_public"] is False + + +def test_update_forwards_allowed_scopes(): + """campus-cli#24: --allowed-scope forwards the full replacement list.""" + result, api = _invoke_update([ + "--client-id", "uid-x", + "--allowed-scope", "openid", + "--allowed-scope", "campus.api", + ]) + + assert result.exit_code == 0 + api.auth_clients["uid-x"].update.assert_called_once_with( + allowed_scopes=["openid", "campus.api"] + ) + + +def test_update_clear_allowed_scopes_sends_empty_list(): + """--clear-allowed-scopes must send allowed_scopes=[] (fail-closed + A1: an empty allowlist grants nothing, so clearing is a real op).""" + result, api = _invoke_update([ + "--client-id", "uid-x", "--clear-allowed-scopes", + ]) + + assert result.exit_code == 0 + api.auth_clients["uid-x"].update.assert_called_once_with( + allowed_scopes=[] + ) + + +def test_update_rejects_clear_combined_with_allowed_scope(): + """--clear-allowed-scopes and --allowed-scope are mutually exclusive.""" + result, api = _invoke_update([ + "--client-id", "uid-x", + "--clear-allowed-scopes", + "--allowed-scope", "openid", + ]) + + assert result.exit_code == 1 + api.auth_clients["uid-x"].update.assert_not_called() + + +def test_update_groups_upstream_scopes_by_provider(): + """--upstream-scope values group per provider, in flag order.""" + result, api = _invoke_update([ + "--client-id", "uid-x", + "--upstream-scope", "google=https://a.example/calendar", + "--upstream-scope", + "google.classroom=https://a.example/classroom.rosters", + "--upstream-scope", "google=https://a.example/classroom.profile", + ]) + + assert result.exit_code == 0 + api.auth_clients["uid-x"].update.assert_called_once_with( + upstream_scopes={ + "google": [ + "https://a.example/calendar", + "https://a.example/classroom.profile", + ], + "google.classroom": ["https://a.example/classroom.rosters"], + } + ) + + +def test_update_rejects_malformed_upstream_scope(): + """A --upstream-scope without provider=scope errors and calls nothing.""" + result, api = _invoke_update([ + "--client-id", "uid-x", + "--upstream-scope", "google-https://a.example/calendar", + ]) + + assert result.exit_code == 1 + assert "provider=scope" in result.stderr + api.auth_clients["uid-x"].update.assert_not_called() + + +def test_update_token_bridge_tri_state(): + """--token-bridge/--no-token-bridge forward bools; omitting sends nothing.""" + for flag, expected in [("--token-bridge", True), + ("--no-token-bridge", False)]: + result, api = _invoke_update(["--client-id", "uid-x", flag]) + assert result.exit_code == 0, flag + api.auth_clients["uid-x"].update.assert_called_once_with( + token_bridge=expected + ) + + result, api = _invoke_update([ + "--client-id", "uid-x", "--name", "New Name", + ]) + assert result.exit_code == 0 + kwargs = api.auth_clients["uid-x"].update.call_args.kwargs + assert "token_bridge" not in kwargs + + +def test_update_json_output_includes_scope_fields(): + """campus-cli#24: update --json surfaces the scope/bridge fields.""" + api = MagicMock() + api.auth_clients["uid-x"].update.return_value = _updated_client( + allowed_scopes=["openid"], + upstream_scopes={"google": ["https://a.example/calendar"]}, + token_bridge=True, + ) + with patch("campus_cli.auth.client.get_api_client", return_value=api): + result = runner.invoke(app, [ + "client", "update", "--client-id", "uid-x", + "--allowed-scope", "openid", "--token-bridge", "--json", + ]) + + assert result.exit_code == 0 + payload = json_loads(result.stdout) + assert payload["allowed_scopes"] == ["openid"] + assert payload["upstream_scopes"] == { + "google": ["https://a.example/calendar"] + } + assert payload["token_bridge"] is True diff --git a/tests/integration/test_dry_run.py b/tests/integration/test_dry_run.py index f6406ee..7fd9cdc 100644 --- a/tests/integration/test_dry_run.py +++ b/tests/integration/test_dry_run.py @@ -106,6 +106,39 @@ def test_client_update_dry_run_redirect_uris(): assert "redirect_uris" not in plain.stdout +def test_client_update_dry_run_scope_bridge_fields(): + """campus-cli#24: dry-run previews the scope/bridge kwargs it would send.""" + result = _invoke_dry_run( + "client", "update", "--client-id", "uid-x", + "--allowed-scope", "openid", + "--upstream-scope", "google=https://a.example/calendar", + "--token-bridge", + ) + + assert "campus.auth.clients['uid-x'].update(" in result.stdout + assert "allowed_scopes=['openid']" in result.stdout + assert "upstream_scopes={'google': ['https://a.example/calendar']}" in ( + result.stdout + ) + assert "token_bridge=True" in result.stdout + + plain = _invoke_dry_run( + "client", "update", "--client-id", "uid-x", "--name", "N" + ) + assert "allowed_scopes" not in plain.stdout + assert "upstream_scopes" not in plain.stdout + assert "token_bridge" not in plain.stdout + + +def test_client_update_dry_run_clear_allowed_scopes(): + """--clear-allowed-scopes previews an explicit empty allowlist.""" + result = _invoke_dry_run( + "client", "update", "--client-id", "uid-x", "--clear-allowed-scopes", + ) + + assert "allowed_scopes=[]" in result.stdout + + def test_client_delete_dry_run_skips_confirmation(): """client delete --dry-run shows the delete call without prompting.""" result = _invoke_dry_run("client", "delete", "--client-id", "uid-x") diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 7e5d135..97d5af3 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -14,6 +14,9 @@ def _client(created_at, **overrides): "created_at": created_at, "permissions": {"campus.api": 1}, "redirect_uris": [], + "allowed_scopes": [], + "upstream_scopes": {}, + "token_bridge": False, "is_public": False, } fields.update(overrides) @@ -57,3 +60,31 @@ def test_format_client_coerces_none_redirect_uris_to_empty_list(): )) assert result["redirect_uris"] == [] assert result["is_public"] is False + + +def test_format_client_includes_scope_and_bridge_fields(): + """campus-cli#24: JSON output must carry the fail-closed admin fields.""" + result = _format_client(_client( + "2026-09-29T10:00:00+00:00", + allowed_scopes=["openid", "campus.api"], + upstream_scopes={"google": ["https://www.googleapis.com/auth/calendar"]}, + token_bridge=True, + )) + assert result["allowed_scopes"] == ["openid", "campus.api"] + assert result["upstream_scopes"] == { + "google": ["https://www.googleapis.com/auth/calendar"] + } + assert result["token_bridge"] is True + + +def test_format_client_defaults_scope_fields_when_none(): + """Null scope/bridge fields project as empty values, always present.""" + result = _format_client(_client( + "2026-09-29T10:00:00+00:00", + allowed_scopes=None, + upstream_scopes=None, + token_bridge=None, + )) + assert result["allowed_scopes"] == [] + assert result["upstream_scopes"] == {} + assert result["token_bridge"] is False