From 94aee469d1a1868ebf1670149250bb40802afb4d Mon Sep 17 00:00:00 2001 From: JS Date: Wed, 21 Oct 2026 06:21:23 +0800 Subject: [PATCH 1/2] =?UTF-8?q?feat(cli):=20`campus=20audit=20keys=20new?= =?UTF-8?q?=20--role`=20=E2=80=94=20mint=20a=20key=20by=20named=20role?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit service has a named role catalogue for API keys (campus#918, campus PR #935): a role name is accepted wherever a scope list is accepted, and resolves to exactly one scope list. `campus audit keys new` still took a raw --scope list, so the operator assembling scopes by hand is exactly what the catalogue exists to remove. --role now names one of the service's roles (traces-viewer, traces-searcher, traces-producer, key-operator), and --scope and --role are mutually exclusive — the service refuses both together, because it leaves it ambiguous which was meant, so the CLI refuses them first rather than sending a request that will be rejected. The catalogue is the service's, not the CLI's: the CLI passes the name through and lets the service refuse an unknown role with the names of the roles it does have. The CLI keeps no copy that could drift. `campus audit keys update --role` re-scopes a key the same way, and the dry-run output shows the role it would send. --- README.md | 14 +++++-- campus_cli/audit/keys.py | 63 ++++++++++++++++++++++++---- tests/unit/test_audit.py | 90 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 1f9be08..1fc4d66 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ campus grant grant --grantee-type user --grantee-id someone@school.edu \ # campus.audit: mint a student's trace-reading key (operator, #54) CAMPUS_AUDIT_API_KEY= campus audit login campus audit keys new --name "Alice" --owner alice@school.edu \ - --scope traces:read --rate-limit 60 + --role traces-viewer --rate-limit 60 campus audit keys list --owner alice@school.edu # …and the queries that key unlocks (from any machine the student logs in on) @@ -199,14 +199,22 @@ Two roles use the group: ```bash campus audit keys new --name "Alice" --owner alice@school.edu \ - --scope traces:read --rate-limit 60 --expires-at 2026-12-31T23:59:59Z + --role traces-viewer --rate-limit 60 --expires-at 2026-12-31T23:59:59Z campus audit keys list --owner alice@school.edu campus audit keys get --key-id - campus audit keys update --key-id --scope traces:read --rate-limit 120 + campus audit keys update --key-id --role traces-viewer --rate-limit 120 campus audit keys regenerate --key-id # rotation; old value dies campus audit keys revoke --key-id # record kept for audit ``` + **Roles vs scopes.** `--role` names one of the service's roles — a + bundle of scopes it owns (`traces-viewer`, `traces-searcher`, + `traces-producer`, `key-operator`), so the holder gets a reviewed + privilege set instead of a hand-assembled one. `--scope` is for + anything the roles do not cover. The two are mutually exclusive, and + the service owns the catalogue: an unknown role is refused with the + names of the roles it does have. + Plaintext key material is shown exactly once, at creation and at regeneration (the server keeps only the hash) — pass it to the student, who stores it with `campus audit login`. `owner_id` is a diff --git a/campus_cli/audit/keys.py b/campus_cli/audit/keys.py index 2a07a77..c45a877 100644 --- a/campus_cli/audit/keys.py +++ b/campus_cli/audit/keys.py @@ -53,6 +53,15 @@ " (the server requires at least one)" ), ) +RoleOption = typer.Option( + None, + "--role", + help=( + "Named role the key gets, e.g. traces-viewer. The role is a" + " bundle of scopes the service owns; --scope and --role are" + " mutually exclusive" + ), +) def _print_reveal_key(key_value: str) -> None: @@ -192,7 +201,15 @@ def keys_list( def keys_new( name: str = typer.Option(..., "--name", help="Key name, e.g. the student's name"), owner: str = OwnerOption, - scope: list[str] = ScopeOption, # noqa: B008 + scope: Optional[list[str]] = typer.Option( # noqa: B008 + None, + "--scope", + help=( + "Scope the key grants, e.g. traces:read; repeat for multiple" + " (mutually exclusive with --role)" + ), + ), + role: Optional[str] = RoleOption, rate_limit: Optional[int] = typer.Option( None, "--rate-limit", help="Requests per minute (optional)" ), @@ -205,16 +222,34 @@ def keys_new( """ Create an audit API key (mint a student key). + Give the key a privilege: --role for one of the service's + named roles (a bundle of scopes it owns), or --scope for a + hand-assembled scope list. The two are mutually exclusive — the + server refuses both together, because it leaves it ambiguous which + was meant. + The plaintext key is shown exactly once, at creation — the server keeps only its hash. Pass it to the student; they store it with 'campus audit login --api-key'. """ + if role and scope: + print_error("Give either --role or --scope, not both.") + raise typer.Exit(1) + if not role and not scope: + print_error( + "Give the key a privilege: --role or --scope ." + ) + raise typer.Exit(1) + if dry_run: new_kwargs = [ f" name={name!r},", f" owner_id={owner!r},", - f" scopes={list(scope)!r},", ] + if role is not None: + new_kwargs.append(f" role={role!r},") + else: + new_kwargs.append(f" scopes={list(scope)!r},") if rate_limit is not None: new_kwargs.append(f" rate_limit={rate_limit!r},") if expires_at is not None: @@ -236,7 +271,8 @@ def keys_new( record = api.apikeys.new( name=name, owner_id=owner, - scopes=list(scope), + scopes=list(scope) if scope else None, + role=role, rate_limit=rate_limit, expires_at=expires_at, ) @@ -297,9 +333,10 @@ def keys_update( "--scope", help=( "Replacement scope set; repeat for multiple. Omit to keep" - " the current scopes" + " the current scopes (mutually exclusive with --role)" ), ), + role: Optional[str] = RoleOption, rate_limit: Optional[int] = typer.Option( None, "--rate-limit", help="Requests per minute" ), @@ -307,14 +344,21 @@ def keys_update( dry_run: bool = dry_run_option(), ) -> None: """ - Update an audit API key's name, scopes, or rate limit. + Update an audit API key's name, scopes, role, or rate limit. Only the fields passed are changed. Expiry and revocation are immutable — mint a fresh key instead. + + --role and --scope are mutually exclusive: the server refuses both + together, because it leaves it ambiguous which was meant. """ - if name is None and scope is None and rate_limit is None: + if role and scope: + print_error("Give either --role or --scope, not both.") + raise typer.Exit(1) + if name is None and scope is None and role is None and rate_limit is None: print_error( - "Nothing to update: pass --name, --scope, or --rate-limit." + "Nothing to update: pass --name, --scope, --role, or" + " --rate-limit." ) raise typer.Exit(1) @@ -322,7 +366,9 @@ def keys_update( update_kwargs = [] if name is not None: update_kwargs.append(f" name={name!r},") - if scope is not None: + if role is not None: + update_kwargs.append(f" role={role!r},") + elif scope is not None: update_kwargs.append(f" scopes={list(scope)!r},") if rate_limit is not None: update_kwargs.append(f" rate_limit={rate_limit!r},") @@ -343,6 +389,7 @@ def keys_update( record = api.apikeys[key_id].update( name=name, scopes=list(scope) if scope is not None else None, + role=role, rate_limit=rate_limit, ) diff --git a/tests/unit/test_audit.py b/tests/unit/test_audit.py index a9b1db6..08f0639 100644 --- a/tests/unit/test_audit.py +++ b/tests/unit/test_audit.py @@ -766,3 +766,93 @@ def test_absent_journey_is_none(self): assert result.exit_code == 0, result.output assert api.traces.search.call_args.kwargs["journey_id"] is None + + +class TestKeysRole: + """`campus audit keys new/update --role` grants a named role (#67). + + The role is the service's own vocabulary (campus#918); the CLI + passes the name through and lets the server refuse an unknown one. + """ + + def _invoke_new(self, args): + api = mock.Mock() + api.apikeys.new.return_value = {"id": "k1", "api_key": "audit_v1_x"} + with ( + mock.patch("campus_cli.audit.keys.get_audit_client", return_value=api), + mock.patch.dict("os.environ", {"CAMPUS_AUDIT_URL": AUDIT_URL}), + ): + result = CliRunner().invoke(app, ["audit", "keys", "new", *args]) + return result, api + + def test_role_is_passed_and_no_scopes(self): + result, api = self._invoke_new([ + "--name", "Alice", "--owner", "alice@school.edu", + "--role", "traces-viewer", + ]) + + assert result.exit_code == 0, result.output + kwargs = api.apikeys.new.call_args.kwargs + assert kwargs["role"] == "traces-viewer" + assert kwargs["scopes"] is None + + def test_scopes_still_work(self): + result, api = self._invoke_new([ + "--name", "Alice", "--owner", "alice@school.edu", + "--scope", "traces:read", + ]) + + assert result.exit_code == 0, result.output + kwargs = api.apikeys.new.call_args.kwargs + assert kwargs["scopes"] == ["traces:read"] + assert kwargs["role"] is None + + def test_role_and_scopes_together_are_refused(self): + result, api = self._invoke_new([ + "--name", "Alice", "--owner", "alice@school.edu", + "--role", "traces-viewer", "--scope", "traces:read", + ]) + + assert result.exit_code == 1 + assert "not both" in result.output + api.apikeys.new.assert_not_called() + + def test_neither_role_nor_scopes_is_refused(self): + result, api = self._invoke_new([ + "--name", "Alice", "--owner", "alice@school.edu", + ]) + + assert result.exit_code == 1 + assert "--role" in result.output + api.apikeys.new.assert_not_called() + + def test_update_accepts_a_role(self): + api = mock.MagicMock() + api.apikeys.__getitem__.return_value.update.return_value = {"id": "k1"} + with ( + mock.patch("campus_cli.audit.keys.get_audit_client", return_value=api), + mock.patch.dict("os.environ", {"CAMPUS_AUDIT_URL": AUDIT_URL}), + ): + result = CliRunner().invoke(app, [ + "audit", "keys", "update", "--key-id", "k1", + "--role", "traces-searcher", + ]) + + assert result.exit_code == 0, result.output + kwargs = api.apikeys.__getitem__.return_value.update.call_args.kwargs + assert kwargs["role"] == "traces-searcher" + assert kwargs["scopes"] is None + + def test_update_refuses_role_and_scopes_together(self): + api = mock.MagicMock() + with ( + mock.patch("campus_cli.audit.keys.get_audit_client", return_value=api), + mock.patch.dict("os.environ", {"CAMPUS_AUDIT_URL": AUDIT_URL}), + ): + result = CliRunner().invoke(app, [ + "audit", "keys", "update", "--key-id", "k1", + "--role", "traces-viewer", "--scope", "traces:read", + ]) + + assert result.exit_code == 1 + assert "not both" in result.output From ba17dcc8addb1019bbe1a82eb6b26dae57aec2f0 Mon Sep 17 00:00:00 2001 From: JS Date: Wed, 21 Oct 2026 06:30:43 +0800 Subject: [PATCH 2/2] chore(deps): relock campus-python for the audit key role parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picks up campus-api-python#113, which adds the `role` parameter to the audit apikeys resource — the one this branch's `--role` flag depends on. Same shape as #52 (relock campus-python for the grants resource). --- poetry.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/poetry.lock b/poetry.lock index 5e78c04..e67faaf 100644 --- a/poetry.lock +++ b/poetry.lock @@ -176,7 +176,7 @@ flask = "^3.0.0" type = "git" url = "https://github.com/nyjc-computing/campus-api-python.git" reference = "main" -resolved_reference = "dad044252ec9256cf99e5a0f7d9b29ecf81e88fc" +resolved_reference = "26678465e55f737526fc28cc7ff9979dbfcd640a" [[package]] name = "campus-suite"