Skip to content

Commit 9668f1a

Browse files
dm36claude
andcommitted
feat: register_resource with parent edge for agent_api_keys dual-write
Closes the parent_agent cascade gap surfaced on scaleapi/agentex#354. The api_key dual-write (AGX1-272, PR #248) currently calls grant() which writes the owner edge in SpiceDB but NOT the parent_agent edge. The agent_api_key schema requires `read = ... & parent_agent->read & ...`, so every downstream read/update fails closed without that edge. This PR adds register_resource/deregister_resource (Port + adapter + service) and swaps the api_keys use case from grant→register_resource with parent=AgentexResource.agent(agent_id). Now the owner edge and parent_agent edge are written atomically. Stack: - scaleapi/scaleapi#144657 — sgp-authz 0.7.0 (parent_resource kwarg). - scaleapi/agentex#355 — agentex-auth Port + adapter + HTTP routes. - #248 — original AGX1-272 dual-write (this stacks on it). - THIS PR — extends #248 to use the parent-aware path. Changes: - Port: abstract register_resource(resource, parent=None) and deregister_resource(resource). - Adapter proxy: POST /v1/authz/register and /v1/authz/deregister. - Service: mirror existing grant/revoke pattern (principal_context override, _bypass support, parent in log line for cascade debugging). - Use case: swap grant→register_resource passing parent=agent; swap revoke→deregister_resource. except Exception wrappers preserved (fail-closed on register, best-effort on deregister). - Tests: rename mocks to register_resource/deregister_resource; assert the parent edge is passed correctly. Test plan: - pytest agentex/tests/integration/services/test_agent_api_key_service_dual_write.py → 8 / 8 pass. - New test ``test_create_api_key_calls_grant_when_flag_on`` asserts parent.type == AgentexResourceType.agent and parent.selector == agent.id. Other resource types' grant→register_resource swap is out of scope. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 7e4d50e commit 9668f1a

5 files changed

Lines changed: 215 additions & 66 deletions

File tree

agentex/src/adapters/authorization/adapter_agentex_authz_proxy.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,34 @@ async def list_resources(
8585
)
8686
return response["items"]
8787

88+
async def register_resource(
89+
self,
90+
principal: AgentexAuthPrincipalContext,
91+
resource: AgentexResource,
92+
parent: AgentexResource | None = None,
93+
) -> None:
94+
payload = {
95+
"principal": principal,
96+
"resource": resource.model_dump(),
97+
"parent": parent.model_dump() if parent is not None else None,
98+
}
99+
await HttpRequestHandler.post_with_error_handling(
100+
self.agentex_auth_url, "/v1/authz/register", json=payload
101+
)
102+
103+
async def deregister_resource(
104+
self,
105+
principal: AgentexAuthPrincipalContext,
106+
resource: AgentexResource,
107+
) -> None:
108+
payload = {
109+
"principal": principal,
110+
"resource": resource.model_dump(),
111+
}
112+
await HttpRequestHandler.post_with_error_handling(
113+
self.agentex_auth_url, "/v1/authz/deregister", json=payload
114+
)
115+
88116

89117
DAgentexAuthorization = Annotated[
90118
AgentexAuthorizationProxy, Depends(AgentexAuthorizationProxy)

agentex/src/adapters/authorization/port.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,29 @@ async def list_resources(
4949
filter_operation: AuthorizedOperationType = AuthorizedOperationType.read,
5050
) -> Iterable[str]:
5151
"""List resource_ids for a given principal"""
52+
53+
@abstractmethod
54+
async def register_resource(
55+
self,
56+
principal: PrincipalT,
57+
resource: AgentexResource,
58+
parent: AgentexResource | None = None,
59+
) -> None:
60+
"""Register a newly created resource in SpiceDB with the principal as
61+
owner. Optionally writes a lifecycle parent edge.
62+
63+
Use this on resource create instead of ``grant`` when the resource
64+
type's SpiceDB definition has a parent relation that permission
65+
checks cascade through (e.g. ``agent_api_key`` declares
66+
``parent_agent``). Without writing that edge here the cascade fails
67+
closed.
68+
"""
69+
70+
@abstractmethod
71+
async def deregister_resource(
72+
self,
73+
principal: PrincipalT,
74+
resource: AgentexResource,
75+
) -> None:
76+
"""Deregister a deleted resource and all of its relationships
77+
(owner, parent, grantees) in a single atomic call."""

agentex/src/domain/services/authorization_service.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,5 +188,62 @@ async def list_resources(
188188
)
189189
return result
190190

191+
async def register_resource(
192+
self,
193+
resource: AgentexResource,
194+
parent: AgentexResource | None = None,
195+
*,
196+
principal_context=...,
197+
) -> None:
198+
"""Register a newly created resource with the principal as owner.
199+
200+
Prefer this over ``grant`` when the resource's SpiceDB schema has
201+
a parent relation that permissions cascade through (e.g.
202+
``agent_api_key`` declares ``parent_agent``). Pass ``parent`` to
203+
link the child to its parent atomically; without it the cascade
204+
fails closed.
205+
"""
206+
if self._bypass():
207+
logger.info(f"Authorization bypassed for register_resource on {resource}")
208+
return None
209+
210+
effective_principal = (
211+
principal_context
212+
if principal_context is not ...
213+
else self.principal_context
214+
)
215+
logger.info(
216+
"[authorization_service] Registering %s:%s for principal %s (parent=%s)",
217+
resource.type,
218+
resource.selector,
219+
effective_principal,
220+
f"{parent.type}:{parent.selector}" if parent is not None else None,
221+
)
222+
await self.gateway.register_resource(effective_principal, resource, parent)
223+
224+
async def deregister_resource(
225+
self,
226+
resource: AgentexResource,
227+
*,
228+
principal_context=...,
229+
) -> None:
230+
"""Deregister a deleted resource and all of its relationships."""
231+
if self._bypass():
232+
logger.info(f"Authorization bypassed for deregister_resource on {resource}")
233+
return None
234+
235+
effective_principal = (
236+
principal_context
237+
if principal_context is not ...
238+
else self.principal_context
239+
)
240+
logger.info(
241+
"[authorization_service] Deregistering %s:%s for principal %s",
242+
resource.type,
243+
resource.selector,
244+
effective_principal,
245+
)
246+
await self.gateway.deregister_resource(effective_principal, resource)
247+
191248

192249
DAuthorizationService = Annotated[AuthorizationService, Depends(AuthorizationService)]

agentex/src/domain/use_cases/agent_api_keys_use_case.py

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -135,23 +135,24 @@ async def _register_api_key_in_spark_authz(
135135
creator_user_id: str | None,
136136
creator_service_account_id: str | None,
137137
) -> str | None:
138-
"""Register a new agent_api_key in Spark AuthZ with creator as owner.
138+
"""Register a new agent_api_key in Spark AuthZ with creator as owner
139+
AND the parent_agent edge to the owning agent.
139140
140141
Called BEFORE the Postgres write — a failure raises and prevents the
141-
row from being persisted, so there is no compensating action to take.
142-
Mirrors the dual-write pattern used for tasks (AGX1-274).
143-
144-
The current ``Provider.spark`` adapter returns ``{}`` from ``grant``;
145-
no ZedToken is surfaced today, so we always return ``None`` for the
146-
new-write-isolation column. A follow-up will plumb the token through
142+
row from being persisted, so there is no compensating action.
143+
144+
The ``agent_api_key`` SpiceDB schema has a ``parent_agent`` relation
145+
that read/update/delete permissions cascade through:
146+
``permission read = internal_effective_viewer & parent_agent->read &
147+
internal_tenant_gate``. We MUST write the parent edge here or every
148+
downstream permission check fails closed. ``register_resource``
149+
(added in agentex-auth and sgp-authz 0.7.0) writes both the owner
150+
edge and the parent edge atomically in one round-trip.
151+
152+
The current ``register_resource`` returns ``None``; no ZedToken is
153+
surfaced today, so we always return ``None`` for the
154+
spark_authz_zedtoken column. A follow-up will plumb the token through
147155
once the adapter exposes it.
148-
149-
Note: the ``agent_api_key`` SpiceDB schema has a ``parent_agent``
150-
relation that read/delete permissions cascade through. The current
151-
``AuthorizationGateway.grant`` signature does not accept a parent
152-
relation — the agentex-auth adapter is expected to set
153-
``parent_agent`` based on the resource shape. This is the same
154-
gap Asher's task PR has and is tracked as a follow-up.
155156
"""
156157
if creator_user_id is None and creator_service_account_id is None:
157158
logger.warning(
@@ -163,16 +164,36 @@ async def _register_api_key_in_spark_authz(
163164
},
164165
)
165166
return None
166-
await self.authorization_service.grant(
167-
resource=AgentexResource.api_key(api_key_id),
168-
)
167+
try:
168+
await self.authorization_service.register_resource(
169+
resource=AgentexResource.api_key(api_key_id),
170+
parent=AgentexResource.agent(agent_id),
171+
)
172+
except Exception as exc:
173+
# Fail closed: log + re-raise so the Postgres row is never written.
174+
# The dual-write contract requires the SpiceDB tuple (and parent
175+
# edge) to exist before the row does.
176+
logger.exception(
177+
"Spark AuthZ register_resource failed for agent_api_key; aborting create",
178+
extra={
179+
"api_key_id": api_key_id,
180+
"agent_id": agent_id,
181+
"account_id": account_id,
182+
"creator_user_id": creator_user_id,
183+
"creator_service_account_id": creator_service_account_id,
184+
"error_type": type(exc).__name__,
185+
},
186+
)
187+
raise
169188
return None
170189

171190
async def _deregister_api_key_from_spark_authz(
172191
self, *, api_key_id: str, account_id: str | None
173192
) -> None:
174-
"""Best-effort revocation of an api_key's Spark AuthZ tuples on delete.
193+
"""Best-effort deregistration of an api_key's Spark AuthZ tuples on delete.
175194
195+
``deregister_resource`` removes the resource and all of its
196+
relationships (owner, parent_agent, any grantees) atomically.
176197
Only invoked when the FGAC_AGENT_API_KEYS_DUAL_WRITE flag is enabled
177198
for the caller's account. Failures are logged but do not block the
178199
delete.
@@ -182,13 +203,18 @@ async def _deregister_api_key_from_spark_authz(
182203
):
183204
return
184205
try:
185-
await self.authorization_service.revoke(
206+
await self.authorization_service.deregister_resource(
186207
resource=AgentexResource.api_key(api_key_id),
187208
)
188-
except Exception:
209+
except Exception as exc:
210+
# Best-effort: log and continue. Postgres row already deleted.
189211
logger.warning(
190-
"Spark AuthZ revoke failed for agent_api_key",
191-
extra={"api_key_id": api_key_id, "account_id": account_id},
212+
"Spark AuthZ deregister failed for agent_api_key; tuple may be orphaned",
213+
extra={
214+
"api_key_id": api_key_id,
215+
"account_id": account_id,
216+
"error_type": type(exc).__name__,
217+
},
192218
exc_info=True,
193219
)
194220

0 commit comments

Comments
 (0)