Skip to content

Commit 06dc211

Browse files
authored
fix(server): repair Orbit binding ID auth and preserve legacy upstreams [SAO-17527] (#273)
## Problem [SAO-17527](https://splunk.atlassian.net/browse/SAO-17527) reports 10/10 failures for `DELETE /api/v1/control-bindings/{binding_id}` in demo-v2-poc. The ID route sent `control_bindings.write` without target context. Orbit rejected that request with 400, which Agent Control surfaced as 502. GET and PATCH use the same by-ID authorization pattern by source inspection; their failure was not reproduced live for this ticket. ## Change - For target-aware authorizers, resolve the caller namespace, load the binding within that namespace, then authorize `control_bindings.read` or `control_bindings.write` with its stored target. The handler reads or mutates the row in the authorized namespace again. - At startup, Agent Control derives Orbit's identity URL from the known management authorization URL and passes it to the generic HTTP provider. Local JWT authorizers also use stored-target authorization. - Custom HTTP upstreams with neither an explicit nor a derivable identity URL, and other legacy authorizers, retain their previous single namespace-wide authorization call. `AGENT_CONTROL_AUTH_UPSTREAM_IDENTITY_URL` is optional; compatible custom upstreams can set it to opt into stored-target authorization. Those deployments do not need a new setting to keep their prior behavior. - On the target-aware path, invalid credentials retain 401; missing or cross-namespace IDs and target denials return the binding 404. Legacy provider error behavior is unchanged. Request and response schemas are unchanged; generated TypeScript route comments reflect the provider-specific behavior. ## Validation - On the previous PR commit (`a63d331`), the full local server suite passed (936 tests); focused auth and control-binding suites passed (193 tests), including GET/PATCH/DELETE compatibility checks for a custom upstream that rejects target context. - On `dcf5901`, local Ruff lint and server mypy passed. DB-free configuration and mock HTTP checks covered Orbit URL derivation, custom upstream fallback, explicit URL precedence, and identity/target authorization requests. - On reviewer follow-up `2edb64d`, a GET/PATCH/DELETE regression test covers an identity/target-authorization namespace mismatch and checks that the binding remains unchanged. Local `make prepush` passed after installing declared optional test dependencies. The DB-backed endpoint test could not run locally because PostgreSQL and Docker were unavailable. - GitHub Python CI, UI CI, TypeScript SDK CI, server build, title validation, and Codecov patch checks passed on `2edb64d`. - The earlier PR commit was exercised on an isolated GCP DevStack for successful ID operations, missing IDs, and invalid credentials. This commit has not been manually exercised on the shared demo stack. ## Rollout and remaining risk - Before deploying Agent Control, verify that the target Orbit environment exposes `POST /internal/auth/resolve_tenant_context` and that its management authorization URL matches the known path. Target-aware ID calls add one identity request before the stored-target authorization request. - Monitor by-ID binding responses and upstream authorization errors. Roll back by reverting this PR and redeploying the prior Agent Control version. - A same-namespace caller may still infer that an inaccessible ID exists from latency or a transient upstream error. Normal target denials return 404. Targetless list authorization is outside this change.
1 parent a0d2660 commit 06dc211

12 files changed

Lines changed: 995 additions & 96 deletions

File tree

‎sdks/typescript/src/generated/funcs/control-bindings-delete.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,14 @@ import { APICall, APIPromise } from "../types/async.js";
2828
import { Result } from "../types/fp.js";
2929

3030
/**
31-
* Delete a control binding (namespace-wide)
31+
* Delete a control binding
3232
*
3333
* @remarks
3434
* Delete a control binding by surrogate ID.
3535
*
36-
* See the GET-by-id docstring for the authorization scope: this route
37-
* is namespace-wide because the target identifiers are not available
38-
* before the binding is loaded. Use ``POST /by-key:delete`` for
39-
* target-scoped detach that includes the target in the request context.
36+
* Target-aware authorizers use the binding's stored target identifiers.
37+
* Other authorizers retain namespace-wide authorization. The deletion
38+
* remains scoped to the authorized namespace.
4039
*/
4140
export function controlBindingsDelete(
4241
client: AgentControlSDKCore,

‎sdks/typescript/src/generated/funcs/control-bindings-get.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,14 @@ import { APICall, APIPromise } from "../types/async.js";
2828
import { Result } from "../types/fp.js";
2929

3030
/**
31-
* Get a control binding (namespace-wide)
31+
* Get a control binding
3232
*
3333
* @remarks
3434
* Read a single control binding by surrogate ID.
3535
*
36-
* Authorization is namespace-wide: the binding's target identifiers
37-
* are not available until after the row is loaded.
38-
* Callers whose authorization model requires per-target permissions
39-
* should use the natural-key endpoints (``PUT /by-key``,
40-
* ``POST /by-key:delete``) and the target-filtered list endpoint, all
41-
* of which include ``(target_type, target_id)`` in the request context.
36+
* Target-aware authorizers use the binding's stored target identifiers.
37+
* Other authorizers retain namespace-wide authorization. The row is loaded
38+
* using the authorized namespace before any binding data is returned.
4239
*/
4340
export function controlBindingsGet(
4441
client: AgentControlSDKCore,

‎sdks/typescript/src/generated/funcs/control-bindings-update.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,14 @@ import { APICall, APIPromise } from "../types/async.js";
2828
import { Result } from "../types/fp.js";
2929

3030
/**
31-
* Update a control binding (namespace-wide)
31+
* Update a control binding
3232
*
3333
* @remarks
3434
* Update the ``enabled`` flag on a control binding.
3535
*
36-
* See the GET-by-id docstring for the authorization scope: this route
37-
* is namespace-wide because the target identifiers are not available
38-
* before the binding is loaded. Use ``PUT /by-key`` for target-scoped
39-
* upserts that include the target in the request context.
36+
* Target-aware authorizers use the binding's stored target identifiers.
37+
* Other authorizers retain namespace-wide authorization. The mutation
38+
* remains scoped to the authorized namespace.
4039
*/
4140
export function controlBindingsUpdate(
4241
client: AgentControlSDKCore,

‎sdks/typescript/src/generated/sdk/control-bindings.ts‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,14 @@ export class ControlBindings extends ClientSDK {
118118
}
119119

120120
/**
121-
* Delete a control binding (namespace-wide)
121+
* Delete a control binding
122122
*
123123
* @remarks
124124
* Delete a control binding by surrogate ID.
125125
*
126-
* See the GET-by-id docstring for the authorization scope: this route
127-
* is namespace-wide because the target identifiers are not available
128-
* before the binding is loaded. Use ``POST /by-key:delete`` for
129-
* target-scoped detach that includes the target in the request context.
126+
* Target-aware authorizers use the binding's stored target identifiers.
127+
* Other authorizers retain namespace-wide authorization. The deletion
128+
* remains scoped to the authorized namespace.
130129
*/
131130
async delete(
132131
request:
@@ -141,17 +140,14 @@ export class ControlBindings extends ClientSDK {
141140
}
142141

143142
/**
144-
* Get a control binding (namespace-wide)
143+
* Get a control binding
145144
*
146145
* @remarks
147146
* Read a single control binding by surrogate ID.
148147
*
149-
* Authorization is namespace-wide: the binding's target identifiers
150-
* are not available until after the row is loaded.
151-
* Callers whose authorization model requires per-target permissions
152-
* should use the natural-key endpoints (``PUT /by-key``,
153-
* ``POST /by-key:delete``) and the target-filtered list endpoint, all
154-
* of which include ``(target_type, target_id)`` in the request context.
148+
* Target-aware authorizers use the binding's stored target identifiers.
149+
* Other authorizers retain namespace-wide authorization. The row is loaded
150+
* using the authorized namespace before any binding data is returned.
155151
*/
156152
async get(
157153
request:
@@ -166,15 +162,14 @@ export class ControlBindings extends ClientSDK {
166162
}
167163

168164
/**
169-
* Update a control binding (namespace-wide)
165+
* Update a control binding
170166
*
171167
* @remarks
172168
* Update the ``enabled`` flag on a control binding.
173169
*
174-
* See the GET-by-id docstring for the authorization scope: this route
175-
* is namespace-wide because the target identifiers are not available
176-
* before the binding is loaded. Use ``PUT /by-key`` for target-scoped
177-
* upserts that include the target in the request context.
170+
* Target-aware authorizers use the binding's stored target identifiers.
171+
* Other authorizers retain namespace-wide authorization. The mutation
172+
* remains scoped to the authorized namespace.
178173
*/
179174
async update(
180175
request:

‎server/src/agent_control_server/auth_framework/config.py‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import os
3333
import ssl
3434
from dataclasses import dataclass
35+
from urllib.parse import urlsplit, urlunsplit
3536

3637
from ..config import auth_settings
3738
from ..logging_utils import get_logger
@@ -53,6 +54,7 @@
5354
# Default flow.
5455
_MODE_ENV = "AGENT_CONTROL_AUTH_MODE"
5556
_UPSTREAM_URL_ENV = "AGENT_CONTROL_AUTH_UPSTREAM_URL"
57+
_UPSTREAM_IDENTITY_URL_ENV = "AGENT_CONTROL_AUTH_UPSTREAM_IDENTITY_URL"
5658
_UPSTREAM_TIMEOUT_ENV = "AGENT_CONTROL_AUTH_UPSTREAM_TIMEOUT_SECONDS"
5759
_UPSTREAM_TOKEN_ENV = "AGENT_CONTROL_AUTH_UPSTREAM_SERVICE_TOKEN"
5860
_UPSTREAM_TOKEN_HEADER_ENV = "AGENT_CONTROL_AUTH_UPSTREAM_SERVICE_TOKEN_HEADER"
@@ -63,6 +65,8 @@
6365
_UPSTREAM_MAX_KEEPALIVE_CONNECTIONS_ENV = (
6466
"AGENT_CONTROL_AUTH_UPSTREAM_MAX_KEEPALIVE_CONNECTIONS"
6567
)
68+
_ORBIT_MANAGEMENT_PATH = "/internal/auth/agent_control/check_management_access"
69+
_ORBIT_IDENTITY_PATH = "/internal/auth/resolve_tenant_context"
6670

6771
# Runtime flow.
6872
_RUNTIME_MODE_ENV = "AGENT_CONTROL_RUNTIME_AUTH_MODE"
@@ -110,7 +114,10 @@ def configure_auth_from_env() -> None:
110114
is unset, startup selects ``api_key`` only if local API-key validation is
111115
enabled; otherwise it selects ``none``.
112116
- ``AGENT_CONTROL_AUTH_MODE=http_upstream``: :class:`HttpUpstreamAuthProvider`
113-
pointed at ``AGENT_CONTROL_AUTH_UPSTREAM_URL``.
117+
pointed at ``AGENT_CONTROL_AUTH_UPSTREAM_URL``. By-ID binding routes use
118+
stored-target authorization when an identity URL can be derived from
119+
Orbit's management URL or ``AGENT_CONTROL_AUTH_UPSTREAM_IDENTITY_URL`` is
120+
set. Other upstreams retain the prior namespace-wide authorization flow.
114121
115122
Runtime flow:
116123
@@ -242,9 +249,14 @@ def _build_default_provider() -> RequestAuthorizer:
242249
max_keepalive_connections=max_keepalive_connections,
243250
)
244251
_logger.info("Default auth provider: http_upstream url=%s", url)
252+
# Only the known Orbit management route has a predictable identity route.
253+
# Other upstreams keep their existing namespace-wide authorization flow.
254+
explicit_identity_url = (os.environ.get(_UPSTREAM_IDENTITY_URL_ENV) or "").strip()
255+
identity_url = explicit_identity_url or _derive_orbit_identity_url(url)
245256
try:
246257
upstream_config = HttpUpstreamConfig(
247258
url=url,
259+
identity_url=identity_url,
248260
timeout_seconds=timeout,
249261
service_token=token,
250262
service_token_header=token_header,
@@ -272,6 +284,17 @@ def _build_default_provider() -> RequestAuthorizer:
272284
)
273285

274286

287+
def _derive_orbit_identity_url(url: str) -> str | None:
288+
"""Resolve Orbit's existing identity route from its management route."""
289+
parts = urlsplit(url)
290+
if not parts.path.endswith(_ORBIT_MANAGEMENT_PATH):
291+
return None
292+
prefix = parts.path[: -len(_ORBIT_MANAGEMENT_PATH)]
293+
return urlunsplit(
294+
(parts.scheme, parts.netloc, prefix + _ORBIT_IDENTITY_PATH, parts.query, parts.fragment)
295+
)
296+
297+
275298
def _validate_local_api_key_mode(mode_env: str = _MODE_ENV) -> None:
276299
"""Fail startup when local API-key mode has no local key validator."""
277300
if not auth_settings.api_key_enabled:

‎server/src/agent_control_server/auth_framework/core.py‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
from dataclasses import dataclass
2727
from datetime import datetime
2828
from enum import StrEnum
29-
from typing import Any, Protocol
29+
from typing import Any, Protocol, runtime_checkable
3030

3131
from fastapi import Request
3232

@@ -115,6 +115,22 @@ async def authorize(
115115
) -> Principal: ...
116116

117117

118+
@runtime_checkable
119+
class IdentityResolver(Protocol):
120+
"""Opt-in credential and namespace lookup for target-bound authorizers.
121+
122+
Providers opt in only when they can resolve an identity without changing
123+
the authorization behavior of existing namespace-wide binding ID routes.
124+
The route then calls ``authorize`` with the stored target after the
125+
namespace-scoped lookup.
126+
"""
127+
128+
@property
129+
def binding_target_authorization(self) -> bool: ...
130+
131+
async def resolve_identity(self, request: Request, operation: Operation) -> Principal: ...
132+
133+
118134
_default_authorizer: RequestAuthorizer | None = None
119135
_operation_authorizers: dict[Operation, RequestAuthorizer] = {}
120136

‎server/src/agent_control_server/auth_framework/providers/http_upstream.py‎

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@
6767
_logger = get_logger(__name__)
6868

6969
_DEFAULT_FORWARDED_HEADERS = ("X-API-Key", "Authorization", "Cookie")
70+
# Diagnostic label only; the identity POST has no operation payload.
71+
_IDENTITY_LOOKUP_LABEL = "identity.resolve"
7072

7173
_AUTH_UPSTREAM_ATTEMPTS = Counter(
7274
"agent_control_server_auth_upstream_attempts_total",
@@ -180,6 +182,9 @@ class HttpUpstreamConfig:
180182
max_keepalive_connections: int = 20
181183
"""Maximum idle connections retained for the auth upstream."""
182184

185+
identity_url: str | None = None
186+
"""Optional URL for credential and namespace resolution before target authorization."""
187+
183188
def __post_init__(self) -> None:
184189
if self.keepalive_expiry_seconds < 0:
185190
raise ValueError("keepalive_expiry_seconds must be greater than or equal to 0")
@@ -213,6 +218,7 @@ def __init__(
213218
client: httpx.AsyncClient | None = None,
214219
) -> None:
215220
self._config = config
221+
self._identity_url = config.identity_url
216222
self._owns_client = client is None
217223
if client is not None:
218224
self._client = client
@@ -229,6 +235,11 @@ def __init__(
229235
client_kwargs["verify"] = ssl.create_default_context(cafile=config.ca_file)
230236
self._client = httpx.AsyncClient(**client_kwargs)
231237

238+
@property
239+
def binding_target_authorization(self) -> bool:
240+
"""Use stored-target checks only when a separate identity URL exists."""
241+
return self._identity_url is not None
242+
232243
async def aclose(self) -> None:
233244
"""Release the HTTP client if this provider created it."""
234245
if self._owns_client:
@@ -245,19 +256,49 @@ async def authorize(
245256
if context:
246257
payload["context"] = context
247258

248-
response = await self._post_upstream(operation, payload, headers)
249-
return self._handle_response(response, operation, context)
259+
response = await self._post_upstream(operation.value, payload, headers)
260+
return self._handle_response(response, operation.value, context)
261+
262+
async def resolve_identity(self, request: Request, operation: Operation) -> Principal:
263+
"""Authenticate with the upstream before a namespace-scoped lookup."""
264+
del operation # Target-specific permission is checked by authorize afterward.
265+
if self._identity_url is None:
266+
raise APIError(
267+
status_code=500,
268+
error_code=ErrorCode.AUTH_MISCONFIGURED,
269+
reason=ErrorReason.INTERNAL_ERROR,
270+
detail="Authorization identity endpoint is not configured.",
271+
hint="Set AGENT_CONTROL_AUTH_UPSTREAM_IDENTITY_URL.",
272+
)
273+
response = await self._post_upstream(
274+
_IDENTITY_LOOKUP_LABEL,
275+
None,
276+
self._forward_headers(request),
277+
url=self._identity_url,
278+
)
279+
if response.status_code == 404:
280+
raise APIError(
281+
status_code=502,
282+
error_code=ErrorCode.AUTH_UPSTREAM_REJECTED,
283+
reason=ErrorReason.INTERNAL_ERROR,
284+
detail="Authorization identity endpoint was not found.",
285+
hint="Check the configured authorization identity URL.",
286+
)
287+
principal = self._handle_response(response, _IDENTITY_LOOKUP_LABEL, None)
288+
return Principal(namespace_key=principal.namespace_key, caller_id=principal.caller_id)
250289

251290
async def _post_upstream(
252291
self,
253-
operation: Operation,
254-
payload: dict[str, Any],
292+
operation: str,
293+
payload: dict[str, Any] | None,
255294
headers: dict[str, str],
295+
*,
296+
url: str | None = None,
256297
) -> httpx.Response:
257298
started = perf_counter()
258299
try:
259300
response = await self._client.post(
260-
self._config.url,
301+
url or self._config.url,
261302
json=payload,
262303
headers=headers,
263304
)
@@ -270,7 +311,7 @@ async def _post_upstream(
270311
)
271312
_logger.warning(
272313
"Auth upstream unreachable for operation %s: %s",
273-
operation.value,
314+
operation,
274315
exc,
275316
)
276317
raise _authorization_service_unavailable_error() from exc
@@ -301,7 +342,7 @@ def _forward_headers(self, request: Request) -> dict[str, str]:
301342
def _handle_response(
302343
self,
303344
response: httpx.Response,
304-
operation: Operation,
345+
operation: str,
305346
context: dict[str, Any] | None,
306347
) -> Principal:
307348
status = response.status_code
@@ -318,7 +359,7 @@ def _handle_response(
318359
if status == 403:
319360
raise ForbiddenError(
320361
error_code=ErrorCode.AUTH_INSUFFICIENT_PRIVILEGES,
321-
detail=f"Not authorized to perform {operation.value!r}.",
362+
detail=f"Not authorized to perform {operation!r}.",
322363
hint="Contact your administrator if you expected access.",
323364
)
324365
if status == 404:
@@ -343,7 +384,7 @@ def _handle_response(
343384
hint = f"{hint} Retry-After: {retry_after}."
344385
_logger.warning(
345386
"Upstream returned 429 for operation %s",
346-
operation.value,
387+
operation,
347388
)
348389
raise APIError(
349390
status_code=503,
@@ -355,7 +396,7 @@ def _handle_response(
355396
if 400 <= status < 500:
356397
_logger.warning(
357398
"Authorization upstream rejected operation %s with status %d",
358-
operation.value,
399+
operation,
359400
status,
360401
)
361402
raise APIError(
@@ -375,7 +416,7 @@ def _handle_response(
375416
_logger.warning(
376417
"Unexpected upstream status %d for operation %s",
377418
status,
378-
operation.value,
419+
operation,
379420
)
380421
raise APIError(
381422
status_code=503,
@@ -420,21 +461,21 @@ def _parse_principal(self, response: httpx.Response) -> Principal:
420461

421462

422463
def _observe_upstream_attempt(
423-
operation: Operation,
464+
operation: str,
424465
duration_seconds: float,
425466
*,
426467
outcome: str,
427468
status_code: int | None = None,
428469
error: httpx.HTTPError | None = None,
429470
) -> None:
430471
_AUTH_UPSTREAM_ATTEMPTS.labels(
431-
operation=operation.value,
472+
operation=operation,
432473
outcome=outcome,
433474
status_code=str(status_code) if status_code is not None else "none",
434475
error_type=type(error).__name__ if error is not None else "none",
435476
).inc()
436477
_AUTH_UPSTREAM_ATTEMPT_DURATION.labels(
437-
operation=operation.value,
478+
operation=operation,
438479
outcome=outcome,
439480
).observe(duration_seconds)
440481

0 commit comments

Comments
 (0)