From 9e1737eb4466c02d2b2b3a31706612bd8f227eae Mon Sep 17 00:00:00 2001 From: eanzhao Date: Mon, 27 Apr 2026 14:06:33 +0800 Subject: [PATCH 1/2] fix(governance): reject body owner identity that conflicts with claims Issue #362 residue C-1. Bound-service identity already returns 400 BOUND_SERVICE_IDENTITY_CONFLICT on body/claim mismatch (commit 2ba2b2ae), but the owner identity on the same binding endpoints was still claim-silent-overrides-body. Make it symmetric: when authenticated, reject body owner tenantId / appId / namespace that disagree with claims with 400 OWNER_SERVICE_IDENTITY_CONFLICT. Applies to all four ServiceBindingEndpoints handlers: Create, Update, Retire, Get. Other identity-resolving endpoints are out of scope; their tests still assert silent override. Test changes: - BindingEndpoints_WhenAuthenticatedBoundServiceOmitsIdentity_ShouldUseClaimIdentityForOwnerAndBoundService: body owner switched from "spoof-*" to claim-matching, since it now would 400 on the spoof. Test purpose unchanged (bound-service omission still derives from claims). - New: BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsWithClaims_ShouldReturnBadRequest (POST) - New: BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsWithQuery_ShouldReturnBadRequest (GET query) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Endpoints/ServiceBindingEndpoints.cs | 37 ++++++++++++ .../GovernanceEndpointsTests.cs | 58 ++++++++++++++++++- 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/src/platform/Aevatar.GAgentService.Governance.Hosting/Endpoints/ServiceBindingEndpoints.cs b/src/platform/Aevatar.GAgentService.Governance.Hosting/Endpoints/ServiceBindingEndpoints.cs index 47b46e7fa..95ac0cd88 100644 --- a/src/platform/Aevatar.GAgentService.Governance.Hosting/Endpoints/ServiceBindingEndpoints.cs +++ b/src/platform/Aevatar.GAgentService.Governance.Hosting/Endpoints/ServiceBindingEndpoints.cs @@ -41,6 +41,9 @@ private static async Task HandleCreateAsync( } var authenticatedContext = identityResolver.Resolve(); + if (TryValidateOwnerIdentity(request.TenantId, request.AppId, request.Namespace, authenticatedContext) is { } ownerInvalid) + return ownerInvalid; + var bindingKind = ParseBindingKind(request.BindingKind); if (TryValidateBoundServiceIdentity(bindingKind, request, authenticatedContext) is { } invalid) return invalid; @@ -73,6 +76,9 @@ private static async Task HandleUpdateAsync( } var authenticatedContext = identityResolver.Resolve(); + if (TryValidateOwnerIdentity(request.TenantId, request.AppId, request.Namespace, authenticatedContext) is { } ownerInvalid) + return ownerInvalid; + var bindingKind = ParseBindingKind(request.BindingKind); if (TryValidateBoundServiceIdentity(bindingKind, request, authenticatedContext) is { } invalid) return invalid; @@ -93,6 +99,10 @@ private static async Task HandleRetireAsync( [FromServices] IServiceGovernanceCommandPort commandPort, CancellationToken ct) { + var authenticatedContext = identityResolver.Resolve(); + if (TryValidateOwnerIdentity(request.TenantId, request.AppId, request.Namespace, authenticatedContext) is { } ownerInvalid) + return ownerInvalid; + if (!ServiceIdentityEndpointAccess.TryResolveIdentity( identityResolver, request.TenantId, @@ -121,6 +131,10 @@ private static async Task HandleGetAsync( [FromServices] IServiceGovernanceQueryPort queryPort, CancellationToken ct) { + var authenticatedContext = identityResolver.Resolve(); + if (TryValidateOwnerIdentity(query.TenantId, query.AppId, query.Namespace, authenticatedContext) is { } ownerInvalid) + return ownerInvalid; + if (!ServiceIdentityEndpointAccess.TryResolveIdentity( identityResolver, query.TenantId, @@ -190,6 +204,29 @@ private static ServiceBindingSpec ToSpec( return spec; } + private static IResult? TryValidateOwnerIdentity( + string? requestedTenantId, + string? requestedAppId, + string? requestedNamespace, + ServiceIdentityContext? authenticatedContext) + { + if (authenticatedContext is null) + return null; + + if (!MatchesAuthenticatedValue(requestedTenantId, authenticatedContext.TenantId) || + !MatchesAuthenticatedValue(requestedAppId, authenticatedContext.AppId) || + !MatchesAuthenticatedValue(requestedNamespace, authenticatedContext.Namespace)) + { + return Results.BadRequest(new + { + code = "OWNER_SERVICE_IDENTITY_CONFLICT", + message = "Authenticated service identity does not allow overriding owner tenantId, appId, or namespace.", + }); + } + + return null; + } + private static IResult? TryValidateBoundServiceIdentity( ServiceBindingKind bindingKind, ServiceBindingHttpRequest request, diff --git a/test/Aevatar.GAgentService.Integration.Tests/GovernanceEndpointsTests.cs b/test/Aevatar.GAgentService.Integration.Tests/GovernanceEndpointsTests.cs index 5f0ec52ca..3ea1244ec 100644 --- a/test/Aevatar.GAgentService.Integration.Tests/GovernanceEndpointsTests.cs +++ b/test/Aevatar.GAgentService.Integration.Tests/GovernanceEndpointsTests.cs @@ -31,9 +31,9 @@ public async Task BindingEndpoints_WhenAuthenticatedBoundServiceOmitsIdentity_Sh { Content = JsonContent.Create(new { - tenantId = "spoof-tenant", - appId = "spoof-app", - @namespace = "spoof-ns", + tenantId = "tenant-claim", + appId = "app-claim", + @namespace = "ns-claim", bindingId = "binding-a", displayName = "Dependency", bindingKind = "service", @@ -68,6 +68,58 @@ public async Task BindingEndpoints_WhenAuthenticatedBoundServiceOmitsIdentity_Sh }); } + [Fact] + public async Task BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsWithClaims_ShouldReturnBadRequest() + { + await using var host = await GovernanceEndpointTestHost.StartAsync(); + + using var request = new HttpRequestMessage(HttpMethod.Post, "/api/services/orders/bindings") + { + Content = JsonContent.Create(new + { + tenantId = "spoof-tenant", + appId = "spoof-app", + @namespace = "spoof-ns", + bindingId = "binding-a", + displayName = "Dependency", + bindingKind = "service", + service = new + { + serviceId = "dependency", + endpointId = "run", + }, + }), + }; + request.Headers.Add("X-Test-Authenticated", "true"); + request.Headers.Add("X-Test-Tenant-Id", "tenant-claim"); + request.Headers.Add("X-Test-App-Id", "app-claim"); + request.Headers.Add("X-Test-Namespace", "ns-claim"); + + var response = await host.Client.SendAsync(request); + + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + host.CommandPort.CreateBindingCommand.Should().BeNull(); + } + + [Fact] + public async Task BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsWithQuery_ShouldReturnBadRequest() + { + await using var host = await GovernanceEndpointTestHost.StartAsync(); + + using var request = new HttpRequestMessage( + HttpMethod.Get, + "/api/services/orders/bindings?tenantId=spoof-tenant&appId=spoof-app&namespace=spoof-ns"); + request.Headers.Add("X-Test-Authenticated", "true"); + request.Headers.Add("X-Test-Tenant-Id", "tenant-claim"); + request.Headers.Add("X-Test-App-Id", "app-claim"); + request.Headers.Add("X-Test-Namespace", "ns-claim"); + + var response = await host.Client.SendAsync(request); + + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + host.QueryPort.LastBindingsIdentity.Should().BeNull(); + } + [Fact] public async Task BindingEndpoints_WhenAuthenticatedBoundServiceIdentityConflictsWithClaims_ShouldReturnBadRequest() { From 6860b02e104fbfafbc1dbacdd03fec18b263aa69 Mon Sep 17 00:00:00 2001 From: eanzhao Date: Mon, 27 Apr 2026 15:50:23 +0800 Subject: [PATCH 2/2] fix(governance): unify binding handler order + assert response code in conflict tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two PR #433 review comments. 1. ServiceBindingEndpoints handler order is now consistent across all four handlers (Create / Update / Retire / Get): validate-then-resolve. Previously Create and Update did TryResolveContext (which internally calls Resolve()) before TryValidateOwnerIdentity, so a malformed-claims request that should have produced 400 OWNER_SERVICE_IDENTITY_CONFLICT would short-circuit on 403 SERVICE_IDENTITY_ACCESS_DENIED instead. The four handlers now share the same shape: a) Resolve authenticated context once. b) Validate body identity → 400 with specific code on mismatch. c) TryResolveContext / TryResolveIdentity, passing the already-resolved authenticated context (avoids the double Resolve() call). d) Dispatch. Added new overloads to ServiceIdentityEndpointAccess that accept a pre-resolved ServiceIdentityContext, so the handler does not pay for Resolve() twice. Old overloads delegate into the new ones, keeping every other caller (ServiceServingEndpoints, ServiceEndpoints, ServicePolicyEndpoints, ServiceEndpointCatalogEndpoints, etc.) compiling unchanged. 2. Conflict tests now assert the response body's `code` JSON property, which is the API contract clients switch on. Added forward-fix coverage to BindingEndpoints_WhenAuthenticatedBoundServiceIdentity- ConflictsWithClaims as well. Plus two missing integration scenarios for the new owner-identity guard: - BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsOnUpdate → 400 OWNER_SERVICE_IDENTITY_CONFLICT, no UpdateBindingCommand - BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsOnRetire → 400 OWNER_SERVICE_IDENTITY_CONFLICT, no RetireBindingCommand Test helpers AddAuthenticatedClaims and ReadCodeAsync collapse the shared setup that was duplicated across each scenario. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Endpoints/ServiceBindingEndpoints.cs | 51 ++++++----- .../Identity/ServiceIdentityEndpointAccess.cs | 48 +++++++++- .../GovernanceEndpointsTests.cs | 87 ++++++++++++++++--- 3 files changed, 153 insertions(+), 33 deletions(-) diff --git a/src/platform/Aevatar.GAgentService.Governance.Hosting/Endpoints/ServiceBindingEndpoints.cs b/src/platform/Aevatar.GAgentService.Governance.Hosting/Endpoints/ServiceBindingEndpoints.cs index 95ac0cd88..35ab1b34f 100644 --- a/src/platform/Aevatar.GAgentService.Governance.Hosting/Endpoints/ServiceBindingEndpoints.cs +++ b/src/platform/Aevatar.GAgentService.Governance.Hosting/Endpoints/ServiceBindingEndpoints.cs @@ -21,6 +21,13 @@ public static void Map(RouteGroupBuilder group) group.MapGet("/{serviceId}/bindings", HandleGetAsync); } + // All four handlers share the same shape: + // 1. Resolve authenticated context once. + // 2. Validate body identity against claims (returns 400 OWNER_*_CONFLICT + // / BOUND_SERVICE_IDENTITY_CONFLICT before the more generic 403). + // 3. TryResolveContext / TryResolveIdentity using the already-resolved auth context + // (avoids the double-Resolve cost). + // 4. Dispatch the command / query. private static async Task HandleCreateAsync( HttpContext http, string serviceId, @@ -29,25 +36,26 @@ private static async Task HandleCreateAsync( [FromServices] IServiceGovernanceCommandPort commandPort, CancellationToken ct) { - if (ServiceIdentityEndpointAccess.TryResolveContext( + var authenticatedContext = identityResolver.Resolve(); + if (TryValidateOwnerIdentity(request.TenantId, request.AppId, request.Namespace, authenticatedContext) is { } ownerInvalid) + return ownerInvalid; + + var bindingKind = ParseBindingKind(request.BindingKind); + if (TryValidateBoundServiceIdentity(bindingKind, request, authenticatedContext) is { } invalid) + return invalid; + + if (!ServiceIdentityEndpointAccess.TryResolveContext( identityResolver, + authenticatedContext, request.TenantId, request.AppId, request.Namespace, out var ownerContext, - out var denied) == false) + out var denied)) { return denied; } - var authenticatedContext = identityResolver.Resolve(); - if (TryValidateOwnerIdentity(request.TenantId, request.AppId, request.Namespace, authenticatedContext) is { } ownerInvalid) - return ownerInvalid; - - var bindingKind = ParseBindingKind(request.BindingKind); - if (TryValidateBoundServiceIdentity(bindingKind, request, authenticatedContext) is { } invalid) - return invalid; - var receipt = await commandPort.CreateBindingAsync(new CreateServiceBindingCommand { Spec = ToSpec(serviceId, request, request.BindingId ?? string.Empty, bindingKind, ownerContext, authenticatedContext), @@ -64,25 +72,26 @@ private static async Task HandleUpdateAsync( [FromServices] IServiceGovernanceCommandPort commandPort, CancellationToken ct) { - if (ServiceIdentityEndpointAccess.TryResolveContext( + var authenticatedContext = identityResolver.Resolve(); + if (TryValidateOwnerIdentity(request.TenantId, request.AppId, request.Namespace, authenticatedContext) is { } ownerInvalid) + return ownerInvalid; + + var bindingKind = ParseBindingKind(request.BindingKind); + if (TryValidateBoundServiceIdentity(bindingKind, request, authenticatedContext) is { } invalid) + return invalid; + + if (!ServiceIdentityEndpointAccess.TryResolveContext( identityResolver, + authenticatedContext, request.TenantId, request.AppId, request.Namespace, out var ownerContext, - out var denied) == false) + out var denied)) { return denied; } - var authenticatedContext = identityResolver.Resolve(); - if (TryValidateOwnerIdentity(request.TenantId, request.AppId, request.Namespace, authenticatedContext) is { } ownerInvalid) - return ownerInvalid; - - var bindingKind = ParseBindingKind(request.BindingKind); - if (TryValidateBoundServiceIdentity(bindingKind, request, authenticatedContext) is { } invalid) - return invalid; - var receipt = await commandPort.UpdateBindingAsync(new UpdateServiceBindingCommand { Spec = ToSpec(serviceId, request, bindingId, bindingKind, ownerContext, authenticatedContext), @@ -105,6 +114,7 @@ private static async Task HandleRetireAsync( if (!ServiceIdentityEndpointAccess.TryResolveIdentity( identityResolver, + authenticatedContext, request.TenantId, request.AppId, request.Namespace, @@ -137,6 +147,7 @@ private static async Task HandleGetAsync( if (!ServiceIdentityEndpointAccess.TryResolveIdentity( identityResolver, + authenticatedContext, query.TenantId, query.AppId, query.Namespace, diff --git a/src/platform/Aevatar.GAgentService.Governance.Hosting/Identity/ServiceIdentityEndpointAccess.cs b/src/platform/Aevatar.GAgentService.Governance.Hosting/Identity/ServiceIdentityEndpointAccess.cs index 359f84e4a..691c64ae0 100644 --- a/src/platform/Aevatar.GAgentService.Governance.Hosting/Identity/ServiceIdentityEndpointAccess.cs +++ b/src/platform/Aevatar.GAgentService.Governance.Hosting/Identity/ServiceIdentityEndpointAccess.cs @@ -93,15 +93,25 @@ private static bool TryGetSingleClaimValue( public static class ServiceIdentityEndpointAccess { + /// + /// Resolves the owner identity context. When is + /// supplied (the caller already invoked resolver.Resolve()), claim resolution is + /// reused — avoiding the double-Resolve cost when the handler also needs the authenticated + /// context for validation (e.g., TryValidateOwnerIdentity). Pass null to fall + /// through to the original behaviour: when authenticated but claims are missing/ambiguous, + /// returns 403 SERVICE_IDENTITY_ACCESS_DENIED; when unauthenticated, falls back to + /// the request's tenant/app/namespace fields. + /// public static bool TryResolveContext( IServiceIdentityContextResolver resolver, + ServiceIdentityContext? authenticatedContext, string? fallbackTenantId, string? fallbackAppId, string? fallbackNamespace, out ServiceIdentityContext context, out IResult denied) { - if (resolver.Resolve() is { } resolved) + if (authenticatedContext is { } resolved) { context = resolved; denied = Results.Empty; @@ -130,8 +140,25 @@ public static bool TryResolveContext( return true; } + public static bool TryResolveContext( + IServiceIdentityContextResolver resolver, + string? fallbackTenantId, + string? fallbackAppId, + string? fallbackNamespace, + out ServiceIdentityContext context, + out IResult denied) + => TryResolveContext( + resolver, + resolver.Resolve(), + fallbackTenantId, + fallbackAppId, + fallbackNamespace, + out context, + out denied); + public static bool TryResolveIdentity( IServiceIdentityContextResolver resolver, + ServiceIdentityContext? authenticatedContext, string? fallbackTenantId, string? fallbackAppId, string? fallbackNamespace, @@ -141,6 +168,7 @@ public static bool TryResolveIdentity( { if (!TryResolveContext( resolver, + authenticatedContext, fallbackTenantId, fallbackAppId, fallbackNamespace, @@ -160,4 +188,22 @@ public static bool TryResolveIdentity( }; return true; } + + public static bool TryResolveIdentity( + IServiceIdentityContextResolver resolver, + string? fallbackTenantId, + string? fallbackAppId, + string? fallbackNamespace, + string serviceId, + out ServiceIdentity identity, + out IResult denied) + => TryResolveIdentity( + resolver, + resolver.Resolve(), + fallbackTenantId, + fallbackAppId, + fallbackNamespace, + serviceId, + out identity, + out denied); } diff --git a/test/Aevatar.GAgentService.Integration.Tests/GovernanceEndpointsTests.cs b/test/Aevatar.GAgentService.Integration.Tests/GovernanceEndpointsTests.cs index 3ea1244ec..291194564 100644 --- a/test/Aevatar.GAgentService.Integration.Tests/GovernanceEndpointsTests.cs +++ b/test/Aevatar.GAgentService.Integration.Tests/GovernanceEndpointsTests.cs @@ -1,6 +1,7 @@ using System.Security.Claims; using System.Net; using System.Net.Http.Json; +using System.Text.Json; using Aevatar.Authentication.Abstractions; using Aevatar.GAgentService.Abstractions; using Aevatar.GAgentService.Abstractions.Commands; @@ -90,17 +91,69 @@ public async Task BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsWithCl }, }), }; - request.Headers.Add("X-Test-Authenticated", "true"); - request.Headers.Add("X-Test-Tenant-Id", "tenant-claim"); - request.Headers.Add("X-Test-App-Id", "app-claim"); - request.Headers.Add("X-Test-Namespace", "ns-claim"); + AddAuthenticatedClaims(request); var response = await host.Client.SendAsync(request); response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + (await ReadCodeAsync(response)).Should().Be("OWNER_SERVICE_IDENTITY_CONFLICT"); host.CommandPort.CreateBindingCommand.Should().BeNull(); } + [Fact] + public async Task BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsOnUpdate_ShouldReturnBadRequest() + { + await using var host = await GovernanceEndpointTestHost.StartAsync(); + + using var request = new HttpRequestMessage(HttpMethod.Put, "/api/services/orders/bindings/binding-a") + { + Content = JsonContent.Create(new + { + tenantId = "spoof-tenant", + appId = "spoof-app", + @namespace = "spoof-ns", + bindingId = "binding-a", + displayName = "Dependency", + bindingKind = "service", + service = new + { + serviceId = "dependency", + endpointId = "run", + }, + }), + }; + AddAuthenticatedClaims(request); + + var response = await host.Client.SendAsync(request); + + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + (await ReadCodeAsync(response)).Should().Be("OWNER_SERVICE_IDENTITY_CONFLICT"); + host.CommandPort.UpdateBindingCommand.Should().BeNull(); + } + + [Fact] + public async Task BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsOnRetire_ShouldReturnBadRequest() + { + await using var host = await GovernanceEndpointTestHost.StartAsync(); + + using var request = new HttpRequestMessage(HttpMethod.Post, "/api/services/orders/bindings/binding-a:retire") + { + Content = JsonContent.Create(new + { + tenantId = "spoof-tenant", + appId = "spoof-app", + @namespace = "spoof-ns", + }), + }; + AddAuthenticatedClaims(request); + + var response = await host.Client.SendAsync(request); + + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + (await ReadCodeAsync(response)).Should().Be("OWNER_SERVICE_IDENTITY_CONFLICT"); + host.CommandPort.RetireBindingCommand.Should().BeNull(); + } + [Fact] public async Task BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsWithQuery_ShouldReturnBadRequest() { @@ -109,14 +162,12 @@ public async Task BindingEndpoints_WhenAuthenticatedOwnerIdentityConflictsWithQu using var request = new HttpRequestMessage( HttpMethod.Get, "/api/services/orders/bindings?tenantId=spoof-tenant&appId=spoof-app&namespace=spoof-ns"); - request.Headers.Add("X-Test-Authenticated", "true"); - request.Headers.Add("X-Test-Tenant-Id", "tenant-claim"); - request.Headers.Add("X-Test-App-Id", "app-claim"); - request.Headers.Add("X-Test-Namespace", "ns-claim"); + AddAuthenticatedClaims(request); var response = await host.Client.SendAsync(request); response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + (await ReadCodeAsync(response)).Should().Be("OWNER_SERVICE_IDENTITY_CONFLICT"); host.QueryPort.LastBindingsIdentity.Should().BeNull(); } @@ -145,17 +196,29 @@ public async Task BindingEndpoints_WhenAuthenticatedBoundServiceIdentityConflict }, }), }; - request.Headers.Add("X-Test-Authenticated", "true"); - request.Headers.Add("X-Test-Tenant-Id", "tenant-claim"); - request.Headers.Add("X-Test-App-Id", "app-claim"); - request.Headers.Add("X-Test-Namespace", "ns-claim"); + AddAuthenticatedClaims(request); var response = await host.Client.SendAsync(request); response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + (await ReadCodeAsync(response)).Should().Be("BOUND_SERVICE_IDENTITY_CONFLICT"); host.CommandPort.CreateBindingCommand.Should().BeNull(); } + private static void AddAuthenticatedClaims(HttpRequestMessage request) + { + request.Headers.Add("X-Test-Authenticated", "true"); + request.Headers.Add("X-Test-Tenant-Id", "tenant-claim"); + request.Headers.Add("X-Test-App-Id", "app-claim"); + request.Headers.Add("X-Test-Namespace", "ns-claim"); + } + + private static async Task ReadCodeAsync(HttpResponseMessage response) + { + var body = await response.Content.ReadFromJsonAsync(); + return body.TryGetProperty("code", out var code) ? code.GetString() : null; + } + [Fact] public async Task BindingEndpoints_ShouldMapServiceConnectorAndSecretBindings() {