Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
14 changes: 11 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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=<operator 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)
Expand Down Expand Up @@ -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 <id>
campus audit keys update --key-id <id> --scope traces:read --rate-limit 120
campus audit keys update --key-id <id> --role traces-viewer --rate-limit 120
campus audit keys regenerate --key-id <id> # rotation; old value dies
campus audit keys revoke --key-id <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
Expand Down
63 changes: 55 additions & 8 deletions campus_cli/audit/keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)"
),
Expand All @@ -205,16 +222,34 @@ def keys_new(
"""
Create an audit API key (mint a student key).

Give the key a privilege: --role <name> for one of the service's
named roles (a bundle of scopes it owns), or --scope <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 <name> or --scope <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:
Expand All @@ -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,
)
Expand Down Expand Up @@ -297,32 +333,42 @@ 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"
),
output_json: bool = typer.Option(False, "--json", help="Output as JSON"),
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)

if dry_run:
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},")
Expand All @@ -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,
)

Expand Down
2 changes: 1 addition & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

90 changes: 90 additions & 0 deletions tests/unit/test_audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading