diff --git a/scripts/spark-authz-e2e-tests/MANUAL_SMOKE.md b/scripts/spark-authz-e2e-tests/MANUAL_SMOKE.md new file mode 100644 index 00000000..2efea002 --- /dev/null +++ b/scripts/spark-authz-e2e-tests/MANUAL_SMOKE.md @@ -0,0 +1,327 @@ +# Manual `agent_api_keys` smoke — pubsec-dev + +Copy-paste manual smoke for the `agent_api_keys` routes against +`agentex.sgp-pubsec-dev.scale.com`. **No e2e suite. No venv. Just `curl`.** + +Set the four env vars at the top, then paste any case block. + +Companion to the automated suite in this directory — same coverage, but +each case is one independent `curl` you can run from any shell. + +## Prereqs + +- A real pubsec-dev API key (`ssk_is_…`). +- The agentex-compatible `account_id` used for `x-selected-account-id`. +- An existing `agent_id` where you have `api_key.create` permission. +- (Optional) A second user's API key for tenant-isolation cases (15–17). + +```bash +export KEY="ssk_is_..." # your api key +export ACCT="69c69407ee5d19e1dce57d57" # x-selected-account-id +export AGENT="811a4c69-e86d-46b3-9293-0bc8443f599d" # parent agent (golden-agent works) +export BASE="https://agentex.sgp-pubsec-dev.scale.com" +H() { echo "-H"; echo "x-api-key: $KEY"; echo "-H"; echo "x-selected-account-id: $ACCT"; } +``` + +--- + +## Case 1 — Reachability + auth sanity + +Confirm the API is reachable and your headers authenticate. **Expected:** +`200` + JSON array of agents. + +```bash +curl -sS -w "\nHTTP %{http_code}\n" $(H) "$BASE/agents" +``` + +✅ Pass if `HTTP 200` and you see agents. + +--- + +## Case 2 — OpenAPI shape + +Confirm `agent_api_key` routes are published. **Expected:** five paths +under `/agent_api_keys`. + +```bash +curl -sS "$BASE/openapi.json" | python3 -c " +import json,sys +print('\n'.join(sorted(p for p in json.load(sys.stdin)['paths'] if 'api_key' in p))) +" +``` + +✅ Pass if you see `/agent_api_keys`, `/agent_api_keys/{id}`, +`/agent_api_keys/name/{name}`, `/agent_api_keys/name/{api_key_name}`. + +--- + +## Case 3 — Create happy path + +Mint an api_key. **Expected:** `200` with `id` + `api_key` in body. + +```bash +NAME="manual-smoke-$(date +%s)" +RESP=$(curl -sS -w "\nHTTP %{http_code}" -X POST $(H) \ + -H "Content-Type: application/json" \ + -d "{\"agent_id\":\"$AGENT\",\"name\":\"$NAME\",\"api_key_type\":\"external\"}" \ + "$BASE/agent_api_keys") +echo "$RESP" +export KID=$(echo "$RESP" | head -n -1 | python3 -c "import json,sys; print(json.load(sys.stdin)['id'])") +echo "KID=$KID" +``` + +✅ Pass if `HTTP 200` and `KID` is a UUID. Save `$KID` for later cases. + +--- + +## Case 4 — Validation: enum is lowercase + +Payload uses `EXTERNAL` (uppercase). **Expected:** `422` rejection. + +```bash +curl -sS -w "\nHTTP %{http_code}\n" -X POST $(H) \ + -H "Content-Type: application/json" \ + -d "{\"agent_id\":\"$AGENT\",\"name\":\"x\",\"api_key_type\":\"EXTERNAL\"}" \ + "$BASE/agent_api_keys" +``` + +✅ Pass if `HTTP 422`. + +--- + +## Case 5 — Validation: requires id OR name + +Payload omits both `agent_id` and `agent_name`. **Expected:** `400`. + +```bash +curl -sS -w "\nHTTP %{http_code}\n" -X POST $(H) \ + -H "Content-Type: application/json" \ + -d '{"name":"x","api_key_type":"external"}' \ + "$BASE/agent_api_keys" +``` + +✅ Pass if `HTTP 400`. + +--- + +## Case 6 — Validation: rejects both id AND name + +Payload includes both. **Expected:** `400`. + +```bash +curl -sS -w "\nHTTP %{http_code}\n" -X POST $(H) \ + -H "Content-Type: application/json" \ + -d "{\"agent_id\":\"$AGENT\",\"agent_name\":\"some-name\",\"name\":\"x\",\"api_key_type\":\"external\"}" \ + "$BASE/agent_api_keys" +``` + +✅ Pass if `HTTP 400`. + +--- + +## Case 7 — Duplicate-name conflict + +Create the same `(agent_id, name, type)` twice. **Expected:** second call +returns `409`. + +```bash +DUP="dup-$(date +%s)" +for i in 1 2; do + echo "--- attempt $i ---" + curl -sS -w "\nHTTP %{http_code}\n" -X POST $(H) \ + -H "Content-Type: application/json" \ + -d "{\"agent_id\":\"$AGENT\",\"name\":\"$DUP\",\"api_key_type\":\"external\"}" \ + "$BASE/agent_api_keys" +done +``` + +✅ Pass if attempt 1 = `200`, attempt 2 = `409`. + +--- + +## Case 8 — Missing auth + +No headers. **Expected:** `401`. + +```bash +curl -sS -w "\nHTTP %{http_code}\n" "$BASE/agent_api_keys/anything" +``` + +✅ Pass if `HTTP 401`. + +--- + +## Case 9 — Bogus API key + +Garbage `x-api-key`. **Expected:** `401`. + +```bash +curl -sS -w "\nHTTP %{http_code}\n" \ + -H "x-api-key: bogus" -H "x-selected-account-id: $ACCT" \ + "$BASE/agents" +``` + +✅ Pass if `HTTP 401`. + +--- + +## Case 10 — Owner GET by id + +Read the key you just created (Case 3). **Expected (healthy env):** `200` +with the same id. **Today on pubsec-dev:** `422` (agentex-auth is still +configured with `AUTH_PROVIDER=sgp`, so the route-level FGAC check routes +to legacy SGP-authz which doesn't know about `api_key`). + +```bash +curl -sS -w "\nHTTP %{http_code}\n" $(H) "$BASE/agent_api_keys/$KID" +``` + +Today: ❌ `422`. After the `AUTH_PROVIDER=spark` rollout: ✅ `200`. + +--- + +## Case 11 — Owner GET by name + +Same row, looked up by name. **Expected (healthy):** `200`. +**Today:** `422`. + +```bash +curl -sS -w "\nHTTP %{http_code}\n" $(H) \ + "$BASE/agent_api_keys/name/$NAME?agent_id=$AGENT&api_key_type=external" +``` + +--- + +## Case 12 — Owner LIST under agent + +Confirm the list includes `$KID`. **Expected (healthy):** `200` + array +containing it. **Today:** `422`. + +```bash +curl -sS -w "\nHTTP %{http_code}\n" $(H) \ + "$BASE/agent_api_keys?agent_id=$AGENT" | python3 -c " +import json, sys, os +body = sys.stdin.read() +try: + arr = json.loads(body) + print('count:', len(arr)) + print('contains target:', any(k.get('id') == os.environ['KID'] for k in arr)) +except Exception: + print(body) +" +``` + +--- + +## Case 13 — GET nonexistent + +Read a UUID that doesn't exist. **Expected (healthy):** `404`. +**Today:** likely `422` (same Gap-1 wrap). + +```bash +curl -sS -w "\nHTTP %{http_code}\n" $(H) \ + "$BASE/agent_api_keys/00000000-0000-0000-0000-000000000000" +``` + +--- + +## Case 14 — Owner DELETE + +Delete the key, then confirm it's gone. **Expected (healthy):** `200`, +then `404` on re-read. **Today:** both calls return `422`. + +```bash +curl -sS -w "\nHTTP %{http_code}\n" -X DELETE $(H) "$BASE/agent_api_keys/$KID" +echo "--- re-fetch ---" +curl -sS -w "\nHTTP %{http_code}\n" $(H) "$BASE/agent_api_keys/$KID" +``` + +--- + +## Case 15 — Non-owner GET → 404 (no existence leak) + +Requires `KEY_B` for a second user in the same tenant. + +```bash +export KEY_B="ssk_is_..." # second user's key +curl -sS -w "\nHTTP %{http_code}\n" \ + -H "x-api-key: $KEY_B" -H "x-selected-account-id: $ACCT" \ + "$BASE/agent_api_keys/$KID" +``` + +✅ Pass if `HTTP 404`. **Critical:** must NOT be `403` (leaks existence). + +--- + +## Case 16 — Non-owner LIST is filtered + +Requires `KEY_B`. + +```bash +curl -sS -H "x-api-key: $KEY_B" -H "x-selected-account-id: $ACCT" \ + "$BASE/agent_api_keys?agent_id=$AGENT" | python3 -c " +import json, sys, os +body = sys.stdin.read() +try: + arr = json.loads(body) + print('user_b list count:', len(arr)) + print('contains user_a key:', any(k.get('id') == os.environ['KID'] for k in arr)) +except Exception: + print(body) +" +``` + +✅ Pass if `contains user_a key: False`. + +--- + +## Case 17 — Non-owner DELETE preserves row + +Requires `KEY_B`. + +```bash +# user_b attempts delete +curl -sS -w "\nHTTP %{http_code}\n" -X DELETE \ + -H "x-api-key: $KEY_B" -H "x-selected-account-id: $ACCT" \ + "$BASE/agent_api_keys/$KID" +# user_a re-fetches +curl -sS -w "\nHTTP %{http_code}\n" $(H) "$BASE/agent_api_keys/$KID" +``` + +✅ Pass if user_b's call = `404` AND user_a's re-fetch = `200` +(row not deleted). + +--- + +## Quick scorecard + +| | Today on pubsec-dev | After `AUTH_PROVIDER=spark` rollout | +|---|---|---| +| **Cases 1–9** — HTTP + validation | ✅ All pass | ✅ Still pass | +| **Cases 10–14** — owner FGAC paths | ❌ All `422` | ✅ All pass | +| **Cases 15–17** — tenant isolation | ❌ Same `422` blocker; also needs second user | ✅ Pass with `KEY_B` | + +**Run Cases 1–9 today.** They confirm the route layer is healthy and the +contract (validation, auth, conflict handling) holds. + +**Cases 10–17 are the rollout-verification kit.** Re-run them after the +`AUTH_PROVIDER=spark` flip on pubsec-dev's `agentex-auth` deployment to +confirm the FGAC dual-write works end-to-end. + +## Background: the rollout blocker + +Pubsec-dev's `agentex-auth` deployment has `AUTH_PROVIDER=sgp`. Every +`register_resource` / `deregister_resource` call from scale-agentex's +dual-write code lands on legacy SGP-authz instead of `spark-authz` +(which IS deployed in the cluster — `ns/spark`, `svc/spark-authz`, ports +50052/8090 — verified reachable via port-forward + `/healthz` returning +`SERVING`). + +To unblock: redeploy `agentex-auth` in pubsec-dev with +`AUTH_PROVIDER=spark` plus +`SPARK_AUTHZ_GRPC_TARGET=spark-authz.spark.svc.cluster.local:50052`. No +new infra needed — just a value change in the Helm chart that +`sgp-system-manager` renders for pubsec-dev. + +The Jinja template that hardcodes this lives at +`sgp/services/sgp-system-manager/sgp_system_manager/packs/agentex-auth/values.yaml.jinja2:20`. diff --git a/scripts/spark-authz-e2e-tests/Makefile b/scripts/spark-authz-e2e-tests/Makefile index 460f22cf..aca3b054 100644 --- a/scripts/spark-authz-e2e-tests/Makefile +++ b/scripts/spark-authz-e2e-tests/Makefile @@ -28,16 +28,21 @@ PYTEST := $(VENV)/bin/pytest # Test-file globs. Update these (NOT individual case targets) when adding # a new test file for an existing resource. -EVENT_TESTS := tests/test_event_authz.py +API_KEY_TESTS := tests/test_api_key_create.py tests/test_api_key_get.py tests/test_api_key_list.py tests/test_api_key_delete.py +EVENT_TESTS := tests/test_event_authz.py # Aggregate groupings — extend these as resources are added. -# Direct-resource targets land with their own PRs (e.g. api_key). +DIRECT_RESOURCE_TESTS := $(API_KEY_TESTS) +# Future direct-resource additions go here: +# DIRECT_RESOURCE_TESTS += $(AGENT_TESTS) $(TASK_TESTS) + SUB_RESOURCE_TESTS := $(EVENT_TESTS) # Future sub-resource additions go here: # SUB_RESOURCE_TESTS += $(STATE_TESTS) $(MESSAGE_TESTS) $(TRACKER_TESTS) $(CHECKPOINT_TESTS) .PHONY: install test \ - test-sub-resources \ + test-direct-resources test-sub-resources \ + test-api-key test-api-key-create test-api-key-get test-api-key-list test-api-key-delete \ test-event \ clean help @@ -51,10 +56,15 @@ help: @echo " make test all tests" @echo "" @echo "Run a logical group:" + @echo " make test-direct-resources resources with their own SpiceDB type" @echo " make test-sub-resources resources that delegate to a parent" @echo "" @echo "Run one resource:" + @echo " make test-api-key all AGX1-325 cases" @echo " make test-event all AGX1-331 cases" + @echo "" + @echo "Run one case:" + @echo " make test-api-key-{create,get,list,delete}" install: $(PYTHON) -m venv $(VENV) @@ -67,9 +77,32 @@ test: # Logical groupings # --------------------------------------------------------------------------- +test-direct-resources: + $(PYTEST) $(DIRECT_RESOURCE_TESTS) -v + test-sub-resources: $(PYTEST) $(SUB_RESOURCE_TESTS) -v +# --------------------------------------------------------------------------- +# Direct resources +# --------------------------------------------------------------------------- + +# AGX1-325 — agent_api_keys +test-api-key: + $(PYTEST) $(API_KEY_TESTS) -v + +test-api-key-create: + $(PYTEST) tests/test_api_key_create.py -v + +test-api-key-get: + $(PYTEST) tests/test_api_key_get.py -v + +test-api-key-list: + $(PYTEST) tests/test_api_key_list.py -v + +test-api-key-delete: + $(PYTEST) tests/test_api_key_delete.py -v + # --------------------------------------------------------------------------- # Sub-resources (delegate authz to a parent) # --------------------------------------------------------------------------- diff --git a/scripts/spark-authz-e2e-tests/README.md b/scripts/spark-authz-e2e-tests/README.md index e4ff3296..d259a9a6 100644 --- a/scripts/spark-authz-e2e-tests/README.md +++ b/scripts/spark-authz-e2e-tests/README.md @@ -10,10 +10,14 @@ Modeled after the equivalent KB suite in ## Scope -This PR establishes the e2e test scaffolding (clients, conftest, factories, -cleanup) and ships the first resource: **events**. Subsequent PRs add new -resource test files on top of this infrastructure (e.g. AGX1-325 api_keys -in a follow-up). +### AGX1-325 — `agent_api_keys` + +Routes: create / get / get-by-name / list / delete / delete-by-name. + +- **create** dual-writes to SpiceDB with the `parent_agent` edge populated. +- **get** / **get-by-name** are gated by `api_key.read`; denial collapses to 404. +- **list** filters to the api_keys the caller has `read` on. +- **delete** / **delete-by-name** dual-write deregisters; denial collapses to 404. ### AGX1-331 — `events` (read-only, parent-agent-delegated) @@ -25,20 +29,10 @@ Routes: `GET /events/{id}` and `GET /events?task_id=...&agent_id=...`. `tests/test_event_authz.py`); only the denied paths are exercised, which is what the ticket asks for. -### Scaffolding shipped with this PR - -- `clients/agentex_client.py` — HTTP client for `scale-agentex` routes - (agent + api_key + event surfaces; api_key methods land here ahead of - AGX1-325's tests so the client doesn't grow in two places). -- `clients/spark_authz_client.py` — direct SpiceDB-state client (HTTP- - transcoded). Copied verbatim from the EGP suite — repo-agnostic. -- `conftest.py` — config loader, identity credentials, two `AgentexClient` - instances (user_a / user_b), a `SparkAuthzClient`, an `authz_reachable` - probe with graceful-skip semantics, `parent_agent` fixture that falls - back to a pre-existing `agentex.agent_id` when the test user lacks - `agent.create` on the tenant, function-scoped `cleanup` tracker. -- `helpers/cleanup.py` + `helpers/factories.py` — LIFO teardown + - unique-name generators. +This PR layers the api_key test files on top of the scaffolding established +in [#277](https://github.com/scaleapi/scale-agentex/pull/277). The clients, +conftest, factories, and cleanup tracker all live there; this PR is purely +the AGX1-325 test cases + `MANUAL_SMOKE.md`. ## Setup @@ -121,10 +115,18 @@ resource, all tests in a logical category, or the whole suite. make test # Logical groups +make test-direct-resources # resources with their own SpiceDB type (api_key, …) make test-sub-resources # resources that delegate to a parent (event, …) # One resource (all cases) +make test-api-key # AGX1-325 — all api_key cases make test-event # AGX1-331 — all event cases + +# One case +make test-api-key-create +make test-api-key-get +make test-api-key-list +make test-api-key-delete ``` Adding a new resource? Add a `_TESTS` variable in the Makefile, @@ -139,10 +141,10 @@ for direct permission assertions. The suite degrades gracefully: - Tests that **only hit scale-agentex HTTP routes** (most of the suite) run normally and assert on response codes + bodies. -- Tests that **assert directly against SpiceDB** skip with a clear reason - when `spark_authz.host` doesn't answer `/healthz` with 2xx. The event - suite this PR ships doesn't have any SpiceDB-asserting tests, but the - scaffolding is here so future resource PRs land cleanly. +- Tests that **assert directly against SpiceDB** (currently: + `test_api_key_create.py` and `test_owner_delete_deregisters_in_spicedb`) + skip with a clear reason when `spark_authz.host` doesn't answer `/healthz` + with 2xx. - The factory cleanup falls back to REST-only when spark-authz isn't reachable (no SpiceDB delete-resource call). Tuples may leak in this mode, but routes are the unit under test. @@ -160,6 +162,10 @@ helpers/ cleanup.py # LIFO cleanup tracker honoring config knobs factories.py # unique_agent_name, unique_api_key_name tests/ + test_api_key_create.py # AGX1-325: dual-write + parent_agent edge + test_api_key_get.py # AGX1-325: 200 owner / 404 non-owner on id + name + test_api_key_list.py # AGX1-325: FGAC list filter + test_api_key_delete.py # AGX1-325: deregister on delete + non-owner 404 test_event_authz.py # AGX1-331: GET /events/{id} + /events denied paths conftest.py # config, identities, clients, factories, cleanup config.json.example # template — copy to config.json and fill in diff --git a/scripts/spark-authz-e2e-tests/tests/test_api_key_create.py b/scripts/spark-authz-e2e-tests/tests/test_api_key_create.py new file mode 100644 index 00000000..5e6d88ca --- /dev/null +++ b/scripts/spark-authz-e2e-tests/tests/test_api_key_create.py @@ -0,0 +1,41 @@ +"""AGX1-325 — Create: dual-write to SpiceDB with parent_agent edge. + +Verifies that ``POST /agent_api_keys`` calls ``register_resource`` on the +authorization service, including the ``parent_agent`` edge so the cascade +``api_key.read = ... & parent_agent->read`` resolves end-to-end. +""" + +import pytest + +API_KEY_RESOURCE_TYPE = "api_key" + + +@pytest.mark.e2e +class TestApiKeyCreate: + def test_create_registers_owner_in_spicedb( + self, create_agent, create_api_key, authz_client, user_a + ): + """Owner has ``read`` and ``delete`` on the api_key right after create.""" + agent_id, _ = create_agent() + api_key_id, _, _ = create_api_key(agent_id) + + # Owner cascade: register_resource(parent=agent) wrote the parent_agent + # edge; without it, every read/delete check fails closed. + assert authz_client.check_permission_bool( + user_a.identity_id, API_KEY_RESOURCE_TYPE, api_key_id, "read" + ) + assert authz_client.check_permission_bool( + user_a.identity_id, API_KEY_RESOURCE_TYPE, api_key_id, "delete" + ) + + def test_non_owner_has_no_permissions( + self, create_agent, create_api_key, authz_client, user_b + ): + """A same-tenant user with no grant sees no permissions on the api_key.""" + agent_id, _ = create_agent() + api_key_id, _, _ = create_api_key(agent_id) + + for permission in ("read", "delete"): + assert not authz_client.check_permission_bool( + user_b.identity_id, API_KEY_RESOURCE_TYPE, api_key_id, permission + ), f"user_b should not have {permission} on api_key {api_key_id}" diff --git a/scripts/spark-authz-e2e-tests/tests/test_api_key_delete.py b/scripts/spark-authz-e2e-tests/tests/test_api_key_delete.py new file mode 100644 index 00000000..4a62bbd9 --- /dev/null +++ b/scripts/spark-authz-e2e-tests/tests/test_api_key_delete.py @@ -0,0 +1,88 @@ +"""AGX1-325 — Delete: dual-write deregisters; non-owner 404. + +Verifies ``DELETE /agent_api_keys/{id}``: + 1. Deregisters the api_key from SpiceDB (owner permissions vanish post-delete). + 2. Returns 404 for a non-owner — never 403, never deletes the row. +""" + +import pytest + +API_KEY_RESOURCE_TYPE = "api_key" + + +@pytest.mark.e2e +class TestApiKeyDelete: + def test_owner_delete_deregisters_in_spicedb( + self, + create_agent, + agentex_client_a, + authz_client, + user_a, + ): + """After delete, SpiceDB no longer reports the owner relationship. + + Built without the ``create_api_key`` factory because that fixture + registers its own teardown, which would race against the explicit + delete inside the test. + """ + from helpers.factories import unique_api_key_name + + agent_id, _ = create_agent() + resp = agentex_client_a.create_api_key( + agent_id=agent_id, name=unique_api_key_name() + ) + assert resp.status_code in (200, 201), resp.text + api_key_id = resp.json()["id"] + + # Sanity: owner has read pre-delete. + assert authz_client.check_permission_bool( + user_a.identity_id, API_KEY_RESOURCE_TYPE, api_key_id, "read" + ) + + del_resp = agentex_client_a.delete_api_key(api_key_id) + assert del_resp.status_code in (200, 204), del_resp.text + + # Deregister happened: owner no longer has any permission. + assert not authz_client.check_permission_bool( + user_a.identity_id, API_KEY_RESOURCE_TYPE, api_key_id, "read" + ), "owner still has read after delete — deregister did not fire" + assert not authz_client.check_permission_bool( + user_a.identity_id, API_KEY_RESOURCE_TYPE, api_key_id, "delete" + ) + + def test_non_owner_delete_returns_404_and_preserves_row( + self, + parent_agent, + create_api_key, + agentex_client_a, + agentex_client_b, + ): + """user_b's denied delete must collapse to 404 and leave the row intact.""" + agent_id, _ = parent_agent + api_key_id, _, _ = create_api_key(agent_id) + + denied = agentex_client_b.delete_api_key(api_key_id) + assert ( + denied.status_code == 404 + ), f"expected 404 (collapsed), got {denied.status_code}: {denied.text}" + + # Row still exists: owner can still fetch it. + owner_get = agentex_client_a.get_api_key(api_key_id) + assert owner_get.status_code == 200, ( + f"api_key was deleted despite the auth check failing — " + f"got {owner_get.status_code}: {owner_get.text}" + ) + + def test_non_owner_delete_by_name_returns_404( + self, + parent_agent, + create_api_key, + agentex_client_b, + ): + agent_id, _ = parent_agent + _, api_key_name, _ = create_api_key(agent_id) + + resp = agentex_client_b.delete_api_key_by_name(api_key_name, agent_id=agent_id) + assert ( + resp.status_code == 404 + ), f"expected 404 (collapsed), got {resp.status_code}: {resp.text}" diff --git a/scripts/spark-authz-e2e-tests/tests/test_api_key_get.py b/scripts/spark-authz-e2e-tests/tests/test_api_key_get.py new file mode 100644 index 00000000..c58a65df --- /dev/null +++ b/scripts/spark-authz-e2e-tests/tests/test_api_key_get.py @@ -0,0 +1,56 @@ +"""AGX1-325 — Get: owner 200, non-owner 404 (collapsed from 403). + +Verifies the id-path and name-path GET handlers route the auth check through +``_check_api_key_or_collapse_to_404`` so denials surface as 404, hiding +cross-tenant existence. +""" + +import pytest + + +@pytest.mark.e2e +class TestApiKeyGet: + def test_owner_get_by_id_returns_200( + self, parent_agent, create_api_key, agentex_client_a + ): + agent_id, _ = parent_agent + api_key_id, _, _ = create_api_key(agent_id) + + resp = agentex_client_a.get_api_key(api_key_id) + assert resp.status_code == 200, resp.text + assert resp.json()["id"] == api_key_id + + def test_owner_get_by_name_returns_200( + self, parent_agent, create_api_key, agentex_client_a + ): + agent_id, _ = parent_agent + api_key_id, api_key_name, _ = create_api_key(agent_id) + + resp = agentex_client_a.get_api_key_by_name(api_key_name, agent_id=agent_id) + assert resp.status_code == 200, resp.text + assert resp.json()["id"] == api_key_id + + def test_non_owner_get_by_id_returns_404( + self, parent_agent, create_api_key, agentex_client_b + ): + """Denial collapses to 404, not 403 — no existence leak.""" + agent_id, _ = parent_agent + api_key_id, _, _ = create_api_key(agent_id) + + resp = agentex_client_b.get_api_key(api_key_id) + assert ( + resp.status_code == 404 + ), f"expected 404 (collapsed), got {resp.status_code}: {resp.text}" + + def test_non_owner_get_by_name_returns_404( + self, parent_agent, create_api_key, agentex_client_b + ): + """Name-route denial must also collapse to 404 with the identifier-free + body so the absent-row and denied-row responses are indistinguishable.""" + agent_id, _ = parent_agent + _, api_key_name, _ = create_api_key(agent_id) + + resp = agentex_client_b.get_api_key_by_name(api_key_name, agent_id=agent_id) + assert ( + resp.status_code == 404 + ), f"expected 404 (collapsed), got {resp.status_code}: {resp.text}" diff --git a/scripts/spark-authz-e2e-tests/tests/test_api_key_list.py b/scripts/spark-authz-e2e-tests/tests/test_api_key_list.py new file mode 100644 index 00000000..7688fa68 --- /dev/null +++ b/scripts/spark-authz-e2e-tests/tests/test_api_key_list.py @@ -0,0 +1,48 @@ +"""AGX1-325 — List: owner sees own, non-owner sees filtered. + +``GET /agent_api_keys?agent_id=...`` forwards ``DAuthorizedResourceIds`` into +the use-case as an ``id`` filter, so the SQL ``WHERE id IN (...)`` clause +restricts results to api_keys the caller has ``read`` on. +""" + +import pytest + + +@pytest.mark.e2e +class TestApiKeyList: + def test_owner_list_includes_own_api_key( + self, parent_agent, create_api_key, agentex_client_a + ): + agent_id, _ = parent_agent + api_key_id, _, _ = create_api_key(agent_id) + + resp = agentex_client_a.list_api_keys(agent_id=agent_id) + assert resp.status_code == 200, resp.text + ids = [k["id"] for k in resp.json()] + assert api_key_id in ids, f"owner missing own api_key in list: {ids}" + + def test_non_owner_list_excludes_user_a_api_key( + self, parent_agent, create_api_key, agentex_client_b + ): + """user_b can hit the route (they have ``read`` on the agent in the + same tenant) but receives a list filtered to their own authorized + api_keys — which is empty for resources owned solely by user_a.""" + agent_id, _ = parent_agent + api_key_id, _, _ = create_api_key(agent_id) + + resp = agentex_client_b.list_api_keys(agent_id=agent_id) + # If user_b lacks ``read`` on the agent itself the route 404s before + # the list filter runs — the assertion below would never execute. Skip + # explicitly so this shows as SKIPPED (not PASSED) and the gap is + # visible in reports. To actually exercise the list filter, user_b + # needs ``read`` on the agent but no ``read`` on user_a's api_keys. + if resp.status_code == 404: + pytest.skip( + "user_b lacks read on the parent agent — route 404s before the " + "list filter runs; FGAC list-filter behavior not exercised." + ) + assert resp.status_code == 200, resp.text + ids = [k["id"] for k in resp.json()] + assert ( + api_key_id not in ids + ), f"user_b saw user_a's api_key in list — FGAC filter leak: {ids}"