fix(governance): reject body owner identity that conflicts with claims #433
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
|
|
@@ -31,9 +32,9 @@ public async Task BindingEndpoints_WhenAuthenticatedBoundServiceOmitsIdentity_Sh | |
| { | ||
| Content = JsonContent.Create(new | ||
| { | ||
| tenantId = "tenant-claim", | ||
| appId = "app-claim", | ||
| @namespace = "ns-claim", | ||
| bindingId = "binding-a", | ||
| displayName = "Dependency", | ||
| bindingKind = "service", | ||
|
|
@@ -68,6 +69,108 @@ 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", | ||
| }, | ||
| }), | ||
| }; | ||
| 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() | ||
| { | ||
| 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"); | ||
| 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(); | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. 两个新 conflict 测试都只断言了 status code 和 command/query 没被分发，但没断言 response body 里 var body = await response.Content.ReadFromJsonAsync<JsonElement>();
body.GetProperty("code").GetString().Should().Be("OWNER_SERVICE_IDENTITY_CONFLICT");（同样的缺口 另：这两个测试后面缺 PUT 和
Contributor
Author
There was a problem hiding this comment. Fixed in 6860b02. The two existing owner-conflict tests (POST body, GET query) now assert
Both assert 400 + |
||
|
|
||
| [Fact] | ||
| public async Task BindingEndpoints_WhenAuthenticatedBoundServiceIdentityConflictsWithClaims_ShouldReturnBadRequest() | ||
| { | ||
|
|
@@ -93,17 +196,29 @@ public async Task BindingEndpoints_WhenAuthenticatedBoundServiceIdentityConflict | |
| }, | ||
| }), | ||
| }; | ||
| 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<string?> ReadCodeAsync(HttpResponseMessage response) | ||
| { | ||
| var body = await response.Content.ReadFromJsonAsync<JsonElement>(); | ||
| return body.TryGetProperty("code", out var code) ? code.GetString() : null; | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task BindingEndpoints_ShouldMapServiceConnectorAndSecretBindings() | ||
| { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retire 和 Get 这里是先 validate 再 resolve，但 Create (line 43-44) 和 Update (line 78-79) 是反过来——先
TryResolveContext再 validate。四个 handler 的顺序不统一，读起来要回头比对。功能是等价的（malformed claims 时
Resolve()返回 null，validation 自然 no-op），但建议四个都对齐到 "先 validate 再 resolve"：具体的 400 OWNER_SERVICE_IDENTITY_CONFLICT 比通用的 403 SERVICE_IDENTITY_ACCESS_DENIED 更早返回，给客户端的诊断信息也更精确。顺带：
Resolve()现在在每个 handler 里被调两次（一次进 helper 内部，一次本地拿authenticatedContext）。可以让TryResolveContext/TryResolveIdentity顺手out ServiceIdentityContext? authenticatedContext，handler 直接用，少一次 claim 解析。There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 6860b02. All four handlers now share the same shape: (1) resolve authenticatedContext once, (2) validate body identity → 400 specific-code on mismatch, (3)
TryResolveContext/TryResolveIdentitypassing the already-resolved auth context (avoids the doubleResolve()call), (4) dispatch. SoHandleCreateAsyncandHandleUpdateAsyncnow matchHandleRetireAsyncandHandleGetAsync— validate-then-resolve, and 400OWNER_SERVICE_IDENTITY_CONFLICTprecedes 403SERVICE_IDENTITY_ACCESS_DENIEDfor malformed-claims-with-spoofed-body.Added overloads
TryResolveContext(resolver, authenticatedContext, ...)andTryResolveIdentity(resolver, authenticatedContext, ...)toServiceIdentityEndpointAccess. Old overloads delegate into the new ones, so other callers (ServiceServingEndpoints,ServiceEndpoints,ServicePolicyEndpoints,ServiceEndpointCatalogEndpoints, …) keep compiling unchanged.