From eb3def11f90e7c6c0c1cd60fd77acd8e4237f56e Mon Sep 17 00:00:00 2001 From: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> Date: Thu, 24 Sep 2026 10:38:13 +0100 Subject: [PATCH 1/3] Add optional OAuth consent origin allowlist to Foundry hosting Add a host-owned exact-origin allowlist for OAuth consent links in .NET and Python Foundry hosting. When configured, consent links from toolbox enumeration, per-call consent errors, and final output emission must match an allowed HTTPS origin. When omitted, the existing safe-HTTPS validation is unchanged. --- .../AgentFrameworkResponseHandler.cs | 19 ++- .../ConsentAwareMcpClientAIFunction.cs | 16 ++- .../FoundryToolboxOptions.cs | 11 ++ .../FoundryToolboxService.cs | 11 +- .../ServiceCollectionExtensions.cs | 6 +- .../ToolboxConsentParser.cs | 97 +++++++++++++++ .../FoundryToolboxServiceTests.cs | 18 +++ .../OAuthConsentEmissionTests.cs | 43 +++++++ .../ToolboxConsentParserTests.cs | 78 ++++++++++++ python/packages/foundry_hosting/README.md | 19 +++ .../_responses.py | 60 ++++++++- .../foundry_hosting/tests/test_responses.py | 115 ++++++++++++++++++ 12 files changed, 479 insertions(+), 14 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs index efe820a3691..6eed78d848e 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs @@ -309,7 +309,8 @@ await this._toolboxService foreach (var consentEvent in EmitOAuthConsentRequest( stream, consent.ToolName, - consent.ConsentUrl)) + consent.ConsentUrl, + this._toolboxService.ConsentLinkPolicy)) { yield return consentEvent; } @@ -396,7 +397,8 @@ await this._toolboxService foreach (var consentEvent in EmitOAuthConsentRequest( stream, consent.ToolName, - consent.ConsentUrl)) + consent.ConsentUrl, + this._toolboxService.ConsentLinkPolicy)) { yield return consentEvent; } @@ -566,7 +568,8 @@ await PersistWorkflowCheckpointAsync( foreach (var consentEvent in EmitOAuthConsentRequest( stream, consentInfo.ToolName, - consentInfo.ConsentUrl)) + consentInfo.ConsentUrl, + this._toolboxService?.ConsentLinkPolicy)) { yield return consentEvent; } @@ -782,12 +785,20 @@ bool CheckNotAllowedStoreUsage() => /// The response event stream to emit on. /// The tool source / server label that requires consent. /// The OAuth consent URL the user must visit. + /// Optional host-owned origin allowlist policy. /// An enumerable of events: output_item.added → output_item.done. internal static IEnumerable EmitOAuthConsentRequest( ResponseEventStream stream, string serverLabel, - string consentUrl) + string consentUrl, + OAuthConsentLinkPolicy? consentLinkPolicy = null) { + if (consentLinkPolicy?.IsAllowed(consentUrl) == false) + { + throw new InvalidOperationException( + "OAuth consent request did not match the configured allowed origins."); + } + var item = new OAuthConsentRequestOutputItem(NewOAuthConsentItemId(), consentUrl, serverLabel); var builder = stream.AddOutputItem(item.Id); yield return builder.EmitAdded(item); diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ConsentAwareMcpClientAIFunction.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ConsentAwareMcpClientAIFunction.cs index 5f3ec0ed9b1..2283fed6a23 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ConsentAwareMcpClientAIFunction.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ConsentAwareMcpClientAIFunction.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. +using System; using System.Text.Json; using System.Threading; using System.Threading.Tasks; @@ -21,7 +22,7 @@ namespace Microsoft.Agents.AI.Foundry.Hosting; /// and the per-request is cancelled. This causes /// to stop the tool loop (it guards /// exceptions with when (!ct.IsCancellationRequested)) and surfaces an -/// to the handler. The handler then emits the +/// to the handler. The handler then emits the /// mcp_approval_request output item and marks the response as incomplete. /// /// @@ -29,11 +30,16 @@ internal sealed class ConsentAwareMcpClientAIFunction : AIFunction { private readonly McpClientTool _inner; private readonly string _toolboxName; + private readonly OAuthConsentLinkPolicy _consentLinkPolicy; - internal ConsentAwareMcpClientAIFunction(McpClientTool inner, string toolboxName) + internal ConsentAwareMcpClientAIFunction( + McpClientTool inner, + string toolboxName, + OAuthConsentLinkPolicy consentLinkPolicy) { this._inner = inner; this._toolboxName = toolboxName; + this._consentLinkPolicy = consentLinkPolicy; } public override string Name => this._inner.Name; @@ -56,6 +62,12 @@ internal ConsentAwareMcpClientAIFunction(McpClientTool inner, string toolboxName } catch (McpProtocolException ex) when ((int)ex.ErrorCode == -32006) { + if (!this._consentLinkPolicy.IsAllowed(ex.Message)) + { + throw new InvalidOperationException( + "The OAuth consent response did not match the configured allowed origins."); + } + var state = McpConsentContext.Current.Value; if (state is not null) { diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxOptions.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxOptions.cs index 46d09b83203..784f6ad6beb 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxOptions.cs @@ -36,6 +36,17 @@ public sealed class FoundryToolboxOptions /// public bool StrictMode { get; set; } = true; + /// + /// Gets the optional exact HTTPS origins allowed for OAuth consent links. + /// + /// + /// Leave this property to preserve existing behavior without restricting consent-link origins. + /// When a collection is provided, every surfaced consent link must match one of its normalized origins. + /// An empty collection therefore rejects every consent link. Duplicate entries are ignored after normalization. + /// Configure origins such as https://auth.example.com; paths, queries, and fragments are rejected. + /// + public IList? AllowedOAuthConsentOrigins { get; set; } + /// /// For testing only: overrides the toolbox proxy base URL (skipping the /// FOUNDRY_PROJECT_ENDPOINT-derived default). When set, the proxy URL diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxService.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxService.cs index a64ba352d19..bc46f44084c 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxService.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxService.cs @@ -60,6 +60,8 @@ public sealed class FoundryToolboxService : IHostedService, IAsyncDisposable private string _agentName = "hosted-agent"; private string _agentVersion = "1.0.0"; + internal OAuthConsentLinkPolicy ConsentLinkPolicy { get; } + /// /// Gets the cached list of instances discovered from all /// pre-registered toolboxes. Always non-null after startup. @@ -117,6 +119,7 @@ public FoundryToolboxService( this._options = options.Value; this._credential = credential; this._logger = logger ?? NullLogger.Instance; + this.ConsentLinkPolicy = new OAuthConsentLinkPolicy(this._options.AllowedOAuthConsentOrigins); } /// @@ -616,7 +619,11 @@ private async Task OpenToolboxAsync( mcpTools = await client.ListToolsAsync(cancellationToken: cancellationToken).ConfigureAwait(false); } catch (McpProtocolException ex) when ( - ToolboxConsentParser.TryParseConsentRequired(toolboxName, ex.Message, out var consents)) + ToolboxConsentParser.TryParseConsentRequired( + toolboxName, + ex.Message, + this.ConsentLinkPolicy, + out var consents)) { // A tool source needs user OAuth consent before it can be enumerated. Dispose the // half-open client and signal the caller, which keeps the container routable and @@ -651,7 +658,7 @@ private async Task OpenToolboxAsync( var wrapped = new List(mcpTools.Count); foreach (var tool in mcpTools) { - wrapped.Add(new ConsentAwareMcpClientAIFunction(tool, toolboxName)); + wrapped.Add(new ConsentAwareMcpClientAIFunction(tool, toolboxName, this.ConsentLinkPolicy)); } _ = version; // reserved for future version-specific routing; currently handled server-side by the proxy. diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ServiceCollectionExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ServiceCollectionExtensions.cs index 640784d798d..22e3a61bc2a 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ServiceCollectionExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ServiceCollectionExtensions.cs @@ -263,7 +263,11 @@ public static IServiceCollection AddFoundryToolboxes( /// /// The service collection. /// The used to authenticate with the Foundry Toolboxes MCP proxy. - /// Callback to further configure (e.g. set ). + /// + /// Callback to configure , such as + /// or + /// . + /// /// Names of the Foundry toolboxes to pre-register at startup. /// The service collection for chaining. public static IServiceCollection AddFoundryToolboxes( diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolboxConsentParser.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolboxConsentParser.cs index 806a613b2be..c9e1b845df7 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolboxConsentParser.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolboxConsentParser.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Text.Json; namespace Microsoft.Agents.AI.Foundry.Hosting; @@ -51,6 +52,17 @@ public static bool TryParseConsentRequired( string toolboxName, string? exceptionMessage, out IReadOnlyList consents) + => TryParseConsentRequired( + toolboxName, + exceptionMessage, + OAuthConsentLinkPolicy.AllowAll, + out consents); + + internal static bool TryParseConsentRequired( + string toolboxName, + string? exceptionMessage, + OAuthConsentLinkPolicy consentLinkPolicy, + out IReadOnlyList consents) { consents = []; @@ -102,6 +114,11 @@ public static bool TryParseConsentRequired( return false; } + if (!consentLinkPolicy.IsAllowed(consentUrl)) + { + return false; + } + string toolName = error.TryGetProperty("name", out var name) && name.ValueKind == JsonValueKind.String ? name.GetString() ?? toolboxName @@ -124,3 +141,83 @@ public static bool TryParseConsentRequired( return true; } } + +/// +/// Applies an optional host-owned exact-origin allowlist to OAuth consent links. +/// +internal sealed class OAuthConsentLinkPolicy +{ + private readonly HashSet? _allowedOrigins; + + internal static OAuthConsentLinkPolicy AllowAll { get; } = new(null); + + internal OAuthConsentLinkPolicy(IEnumerable? allowedOrigins) + { + if (allowedOrigins is null) + { + return; + } + + this._allowedOrigins = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (string origin in allowedOrigins) + { + if (!TryNormalizeOrigin(origin, requireOriginOnly: true, out string? normalizedOrigin)) + { + throw new ArgumentException( + $"OAuth consent allowlist entry '{origin}' must be an absolute HTTPS origin without a path, query, or fragment.", + nameof(allowedOrigins)); + } + + this._allowedOrigins.Add(normalizedOrigin); + } + } + + internal bool IsAllowed(string? consentUrl) + { + if (this._allowedOrigins is null) + { + return true; + } + + return TryNormalizeOrigin(consentUrl, requireOriginOnly: false, out string? normalizedOrigin) + && this._allowedOrigins.Contains(normalizedOrigin); + } + + private static bool TryNormalizeOrigin( + string? value, + bool requireOriginOnly, + [NotNullWhen(true)] out string? normalizedOrigin) + { + normalizedOrigin = null; + + if (string.IsNullOrWhiteSpace(value)) + { + return false; + } + + foreach (char character in value) + { + if (char.IsWhiteSpace(character) || char.IsControl(character)) + { + return false; + } + } + + if (!Uri.TryCreate(value, UriKind.Absolute, out Uri? uri) + || !string.Equals(uri.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase) + || !string.IsNullOrEmpty(uri.UserInfo) + || uri.HostNameType == UriHostNameType.Unknown) + { + return false; + } + + if (requireOriginOnly + && (uri.AbsolutePath != "/" || !string.IsNullOrEmpty(uri.Query) || !string.IsNullOrEmpty(uri.Fragment))) + { + return false; + } + + normalizedOrigin = uri.GetLeftPart(UriPartial.Authority); + return true; + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/FoundryToolboxServiceTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/FoundryToolboxServiceTests.cs index a398addd4cd..59b01ed9c45 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/FoundryToolboxServiceTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/FoundryToolboxServiceTests.cs @@ -14,6 +14,24 @@ namespace Microsoft.Agents.AI.Foundry.Hosting.UnitTests; [Collection(FoundryProjectEndpointEnvFixture.Name)] public class FoundryToolboxServiceTests { + [Fact] + public void Constructor_InvalidAllowedOAuthConsentOrigin_Throws() + { + // Arrange + var options = new FoundryToolboxOptions + { + AllowedOAuthConsentOrigins = ["https://auth.example.com/path"], + }; + + // Act + void CreateService() => _ = new FoundryToolboxService( + Options.Create(options), + Mock.Of()); + + // Assert + Assert.Throws(CreateService); + } + [Fact] public async Task GetToolboxToolsAsync_StrictMode_ThrowsForUnknownToolboxAsync() { diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentEmissionTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentEmissionTests.cs index 8ec80b11e87..0d5d4cb673d 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentEmissionTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentEmissionTests.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. +using System; using System.Collections.Generic; using System.Linq; using Azure.AI.AgentServer.Responses; @@ -44,4 +45,46 @@ public void EmitOAuthConsentRequest_EmitsOAuthConsentRequestItem_NotMcpApproval( var doneItem = Assert.IsType(done.Item); Assert.Equal(addedItem.Id, doneItem.Id); } + + [Fact] + public void EmitOAuthConsentRequest_ConfiguredOriginAllowlist_AllowsMatchingOrigin() + { + // Arrange + const string ConsentUrl = "https://auth.example.com/authorize?state=1"; + var stream = CreateTestStream(); + var policy = new OAuthConsentLinkPolicy(["https://auth.example.com"]); + + // Act + List events = + AgentFrameworkResponseHandler.EmitOAuthConsentRequest( + stream, + "outlook_mail", + ConsentUrl, + policy).ToList(); + + // Assert + Assert.Equal(2, events.Count); + } + + [Fact] + public void EmitOAuthConsentRequest_ConfiguredOriginAllowlist_RejectsOtherOrigin() + { + // Arrange + var stream = CreateTestStream(); + var policy = new OAuthConsentLinkPolicy(["https://auth.example.com"]); + + // Act + void Emit() + { + var events = AgentFrameworkResponseHandler.EmitOAuthConsentRequest( + stream, + "outlook_mail", + "https://other.example.com/authorize", + policy).ToList(); + Assert.Empty(events); + } + + // Assert + Assert.Throws(Emit); + } } diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/ToolboxConsentParserTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/ToolboxConsentParserTests.cs index 3d008ef0f16..ce26b188f20 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/ToolboxConsentParserTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/ToolboxConsentParserTests.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. +using System; + namespace Microsoft.Agents.AI.Foundry.Hosting.UnitTests; public class ToolboxConsentParserTests @@ -132,6 +134,82 @@ public void TryParseConsentRequired_ConsentErrorWithoutUrl_ReturnsFalse() Assert.Empty(consents); } + [Fact] + public void TryParseConsentRequired_NullOriginAllowlist_PreservesExistingBehavior() + { + // Arrange + const string Message = + "tools/list failed " + + "{\"errors\":[{\"name\":\"send_email\",\"type\":\"mcp\",\"error\":{\"code\":\"CONSENT_REQUIRED\",\"message\":\"https://external.example/authorize\"}}]}"; + var policy = new OAuthConsentLinkPolicy(null); + + // Act + var parsed = ToolboxConsentParser.TryParseConsentRequired("toolbox", Message, policy, out var consents); + + // Assert + Assert.True(parsed); + Assert.Single(consents); + } + + [Fact] + public void TryParseConsentRequired_EmptyOriginAllowlist_RejectsAllOrigins() + { + // Arrange + const string Message = + "tools/list failed " + + "{\"errors\":[{\"name\":\"send_email\",\"type\":\"mcp\",\"error\":{\"code\":\"CONSENT_REQUIRED\",\"message\":\"https://external.example/authorize\"}}]}"; + var policy = new OAuthConsentLinkPolicy([]); + + // Act + var parsed = ToolboxConsentParser.TryParseConsentRequired("toolbox", Message, policy, out var consents); + + // Assert + Assert.False(parsed); + Assert.Empty(consents); + } + + [Theory] + [InlineData("https://auth.example.com/authorize?state=1", true)] + [InlineData("https://auth.example.com:443/authorize", true)] + [InlineData("https://login.partner.example:8443/consent", true)] + [InlineData("https://other.example.com/authorize", false)] + public void TryParseConsentRequired_ConfiguredOriginAllowlist_GatesConsentUrl( + string consentUrl, + bool expected) + { + // Arrange + string message = + "tools/list failed " + + "{\"errors\":[{\"name\":\"send_email\",\"type\":\"mcp\",\"error\":{\"code\":\"CONSENT_REQUIRED\",\"message\":\"" + + consentUrl + + "\"}}]}"; + var policy = new OAuthConsentLinkPolicy( + [ + "https://auth.example.com", + "https://login.partner.example:8443", + ]); + + // Act + var parsed = ToolboxConsentParser.TryParseConsentRequired("toolbox", message, policy, out var consents); + + // Assert + Assert.Equal(expected, parsed); + Assert.Equal(expected ? 1 : 0, consents.Count); + } + + [Theory] + [InlineData("http://auth.example.com")] + [InlineData("https://auth.example.com/path")] + [InlineData("https://auth.example.com?tenant=1")] + public void OAuthConsentLinkPolicy_InvalidConfiguredOrigin_Throws(string origin) + { + // Act + void CreatePolicy() => _ = new OAuthConsentLinkPolicy([origin]); + + // Assert + Assert.Throws(CreatePolicy); + } + [Fact] public void TryParseConsentRequired_MissingToolName_FallsBackToToolboxName() { diff --git a/python/packages/foundry_hosting/README.md b/python/packages/foundry_hosting/README.md index a7a59803df4..4a7b60e9c04 100644 --- a/python/packages/foundry_hosting/README.md +++ b/python/packages/foundry_hosting/README.md @@ -52,6 +52,25 @@ implementations must use `history_source="agent"` because that protocol does not instance with another host or invoke it directly after constructing the server. An agent returned by a callable belongs to that request. +### OAuth consent origin allowlist + +OAuth consent links keep their existing absolute-HTTPS safety validation. Hosts that know the expected authorization +origins can add an exact origin allowlist: + +```python +server = ResponsesHostServer( + agent, + allowed_oauth_consent_origins=[ + "https://logic-region.consent.azure-apihub.net", + "https://auth.partner.example", + ], +) +``` + +An omitted allowlist preserves existing behavior and does not restrict the HTTPS origin. A provided allowlist activates +the gate, so an empty sequence rejects every consent link. Entries are normalized as origins, so paths and query strings +belong on the emitted consent link, not in the configuration. + To preserve the agent's regular history and service-storage behavior, select the agent as the history source: ```python diff --git a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py index 325bf264cd1..b297e05e40a 100644 --- a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py +++ b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py @@ -350,6 +350,44 @@ def _is_safe_oauth_consent_link(consent_link: object) -> TypeGuard[str]: return _OAUTH_HOST_PATTERN.fullmatch(hostname) is not None +def _normalize_oauth_consent_origin(value: str, *, require_origin_only: bool) -> str: + """Normalize an HTTPS consent URL or configured origin for exact matching.""" + if not _is_safe_oauth_consent_link(value): + raise ValueError("OAuth consent origins must be absolute HTTPS URLs without user information.") + + parsed = urlparse(value) + if require_origin_only and (parsed.path not in ("", "/") or parsed.params or parsed.query or parsed.fragment): + raise ValueError("OAuth consent origin entries must not include a path, query, parameters, or fragment.") + + hostname = parsed.hostname + if hostname is None: # pragma: no cover - _is_safe_oauth_consent_link already proved this. + raise ValueError("OAuth consent origin must include a hostname.") + normalized_host = f"[{hostname.lower()}]" if ":" in hostname else hostname.lower() + port = parsed.port + return f"https://{normalized_host}" if port in (None, 443) else f"https://{normalized_host}:{port}" + + +class _OAuthConsentLinkPolicy: + """Apply existing URL safety checks plus an optional host-owned origin allowlist.""" + + def __init__(self, allowed_origins: Sequence[str] | None = None) -> None: + self._allowed_origins: frozenset[str] | None = ( + None + if allowed_origins is None + else frozenset( + _normalize_oauth_consent_origin(origin, require_origin_only=True) for origin in allowed_origins + ) + ) + + def is_allowed(self, consent_link: object) -> TypeGuard[str]: + """Return whether the link is safe and, when configured, has an allowed origin.""" + if not _is_safe_oauth_consent_link(consent_link): + return False + if self._allowed_origins is None: + return True + return _normalize_oauth_consent_origin(consent_link, require_origin_only=False) in self._allowed_origins + + def consent_url_from_error(exc: BaseException) -> list[ConsentError] | None: """Return the consent URLs when ``exc`` wraps Foundry MCP gateway consent errors. @@ -517,6 +555,7 @@ def __init__( agent_session_store_provider: StoreProvider[SessionStore] | None = None, checkpoint_store_provider: ContextScopedStoreProvider[CheckpointStorage] | None = None, function_approval_store_provider: StoreProvider[FunctionApprovalStore] | None = None, + allowed_oauth_consent_origins: Sequence[str] | None = None, history_source: Literal["agent_server", "agent"] = "agent_server", **kwargs: Any, ) -> None: @@ -534,6 +573,11 @@ def __init__( If not provided, a default `CheckpointStoreProvider` will be used. function_approval_store_provider: Optional provider for function approval storage. If not provided, a default `FunctionApprovalStoreProvider` will be used. + allowed_oauth_consent_origins: Optional exact HTTPS origins allowed for OAuth consent links. + When omitted, hosting retains its existing safe-HTTPS validation without restricting the + destination origin. When provided, every link must match an entry; an empty sequence rejects + every link. Entries must be origins such as `"https://auth.example.com"` and must not include + a path, query, or fragment. history_source: Source of conversation history supplied to the model for regular agents. `"agent_server"` (default) uses the transcript from the configured response store, requires a `RawAgent` whose client declares `STORES_BY_DEFAULT`, rejects load-enabled @@ -585,6 +629,7 @@ def __init__( ) # No caller-owned agent state is mutated until all validation and base-host construction succeed. + self._oauth_consent_link_policy = _OAuthConsentLinkPolicy(allowed_oauth_consent_origins) super().__init__(prefix=prefix, options=options, store=store, **kwargs) self._agent_source = agent @@ -733,7 +778,7 @@ async def _handle_prepared_response( ( consent_error for consent_error in consent_errors_to_emit - if not _is_safe_oauth_consent_link(consent_error.consent_url) + if not self._oauth_consent_link_policy.is_allowed(consent_error.consent_url) ), None, ) @@ -788,7 +833,7 @@ async def _handle_prepared_response( yield response_event_stream.emit_incomplete() return - tracker = _OutputItemTracker(response_event_stream) + tracker = _OutputItemTracker(response_event_stream, self._oauth_consent_link_policy) try: if configuration.workflow: inner = self._handle_inner_workflow( @@ -1327,8 +1372,13 @@ class _OutputItemTracker: approval requests, etc.) are emitted in one shot, closing any still-open streaming item first. """ - def __init__(self, stream: ResponseEventStream) -> None: + def __init__( + self, + stream: ResponseEventStream, + oauth_consent_link_policy: _OAuthConsentLinkPolicy | None = None, + ) -> None: self._stream = stream + self._oauth_consent_link_policy = oauth_consent_link_policy or _OAuthConsentLinkPolicy() self._usage_details: UsageDetails | None = None self._active_type: str | None = None self._active_id: str | None = None @@ -1647,8 +1697,8 @@ async def handle( yield event consent_link = content.consent_link - if not _is_safe_oauth_consent_link(consent_link): - raise ValueError("OAuth consent request content must include a safe HTTPS consent link.") + if not self._oauth_consent_link_policy.is_allowed(consent_link): + raise ValueError("OAuth consent request content must include an allowed safe HTTPS consent link.") server_label = content.additional_properties.get("server_label") if not isinstance(server_label, str) or not server_label: diff --git a/python/packages/foundry_hosting/tests/test_responses.py b/python/packages/foundry_hosting/tests/test_responses.py index f781bfaa2f4..157e02f87a3 100644 --- a/python/packages/foundry_hosting/tests/test_responses.py +++ b/python/packages/foundry_hosting/tests/test_responses.py @@ -80,6 +80,7 @@ ConsentError, _item_to_message, # pyright: ignore[reportPrivateUsage] _json_safe_to_str, # pyright: ignore[reportPrivateUsage] + _OAuthConsentLinkPolicy, # pyright: ignore[reportPrivateUsage] _output_item_to_message, # pyright: ignore[reportPrivateUsage] _OutputItemTracker, # pyright: ignore[reportPrivateUsage] _SignalledIterator, # pyright: ignore[reportPrivateUsage] @@ -5002,6 +5003,58 @@ def test_returns_none_when_message_has_no_json(self) -> None: assert consent_url_from_error(exc) is None +class TestOAuthConsentLinkPolicy: + @pytest.mark.parametrize( + "policy_kwargs", + [{}, {"allowed_origins": None}], + ids=["omitted", "explicit-none"], + ) + def test_omitted_allowlist_preserves_existing_safe_https_behavior(self, policy_kwargs: dict[str, Any]) -> None: + policy = _OAuthConsentLinkPolicy(**policy_kwargs) + + assert policy.is_allowed("https://external.example/authorize") + assert not policy.is_allowed("http://external.example/authorize") + assert not policy.is_allowed("javascript:alert(1)") + + def test_empty_allowlist_rejects_all_origins(self) -> None: + policy = _OAuthConsentLinkPolicy([]) + + assert not policy.is_allowed("https://external.example/authorize") + + @pytest.mark.parametrize( + "consent_link", + [ + "https://auth.example.com/authorize?state=1", + "https://auth.example.com:443/authorize", + "https://login.partner.example:8443/consent", + ], + ) + def test_configured_allowlist_accepts_matching_origins(self, consent_link: str) -> None: + policy = _OAuthConsentLinkPolicy([ + "https://auth.example.com", + "https://login.partner.example:8443", + ]) + + assert policy.is_allowed(consent_link) + + def test_configured_allowlist_rejects_other_safe_https_origins(self) -> None: + policy = _OAuthConsentLinkPolicy(["https://auth.example.com"]) + + assert not policy.is_allowed("https://other.example.com/authorize") + + @pytest.mark.parametrize( + "origin", + [ + "http://auth.example.com", + "https://auth.example.com/path", + "https://auth.example.com?tenant=1", + ], + ) + def test_invalid_allowlist_origin_raises(self, origin: str) -> None: + with pytest.raises(ValueError, match="origin"): + _OAuthConsentLinkPolicy([origin]) + + class TestAgentLifecycle: async def test_factory_agent_is_entered_and_exited_for_each_request(self) -> None: agents: list[MagicMock] = [] @@ -5079,6 +5132,45 @@ async def test_failed_entry_does_not_cache_stack(self) -> None: class TestOAuthConsentSurfacing: + async def test_explicit_none_origin_allowlist_accepts_any_safe_https_consent(self) -> None: + agent = _make_agent( + response=AgentResponse(messages=[Message(role="assistant", contents=[Content.from_text("hi")])]) + ) + agent.__aenter__.side_effect = _make_consent_error("https://external.example/authorize") + server = _make_server(agent, allowed_oauth_consent_origins=None) + + resp = await _post(server, input_text="hello", stream=False) + body = resp.json() + + assert body["status"] == "incomplete" + oauth_items = [item for item in body["output"] if item["type"] == "oauth_consent_request"] + assert [item["consent_link"] for item in oauth_items] == ["https://external.example/authorize"] + + async def test_configured_origin_allowlist_accepts_connect_time_consent(self) -> None: + agent = _make_agent( + response=AgentResponse(messages=[Message(role="assistant", contents=[Content.from_text("hi")])]) + ) + agent.__aenter__.side_effect = _make_consent_error("https://auth.example.com/authorize?state=1") + server = _make_server(agent, allowed_oauth_consent_origins=["https://auth.example.com"]) + + resp = await _post(server, input_text="hello", stream=False) + + assert resp.json()["status"] == "incomplete" + + async def test_configured_origin_allowlist_rejects_connect_time_consent(self) -> None: + agent = _make_agent( + response=AgentResponse(messages=[Message(role="assistant", contents=[Content.from_text("hi")])]) + ) + agent.__aenter__.side_effect = _make_consent_error("https://other.example.com/authorize") + server = _make_server(agent, allowed_oauth_consent_origins=["https://auth.example.com"]) + + resp = await _post(server, input_text="hello", stream=False) + body = resp.json() + + assert body["status"] == "failed" + assert not any(item["type"] == "oauth_consent_request" for item in body["output"]) + agent.run.assert_not_called() + async def test_non_streaming_consent_error_emits_oauth_output_item(self) -> None: agent = _make_agent( response=AgentResponse(messages=[Message(role="assistant", contents=[Content.from_text("hi")])]) @@ -5333,6 +5425,29 @@ async def test_mid_run_consent_rejects_unsafe_links(self, consent_link: str) -> assert body["status"] == "failed" assert not any(item["type"] == "oauth_consent_request" for item in body["output"]) + async def test_mid_run_consent_rejects_origin_outside_configured_allowlist(self) -> None: + agent = _make_agent( + response=AgentResponse( + messages=[ + Message( + role="assistant", + contents=[ + Content.from_oauth_consent_request( + consent_link="https://other.example.com/authorize", + ) + ], + ) + ] + ) + ) + server = _make_server(agent, allowed_oauth_consent_origins=["https://auth.example.com"]) + + resp = await _post(server, input_text="hello", stream=False) + body = resp.json() + + assert body["status"] == "failed" + assert not any(item["type"] == "oauth_consent_request" for item in body["output"]) + async def test_connect_time_consent_rejects_unsafe_links(self) -> None: agent = _make_agent( response=AgentResponse(messages=[Message(role="assistant", contents=[Content.from_text("hi")])]) From 645e4cf521af6e3b50fbe333a4ab2494acf03bf5 Mon Sep 17 00:00:00 2001 From: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> Date: Thu, 24 Sep 2026 11:14:30 +0100 Subject: [PATCH 2/3] Enforce consent link policy once in the .NET response handler Validate every surfaced OAuth consent link as a safe absolute HTTPS URL in .NET, matching Python, and apply the optional origin allowlist in AgentFrameworkResponseHandler before emission on all three consent paths. A rejected link now fails the response with a neutral message instead of being treated as a toolbox enumeration failure or returned to the model as a tool error. Clarify the Python connect-time error for links outside the configured allowlist. --- .../AgentFrameworkResponseHandler.cs | 65 ++++- .../ConsentAwareMcpClientAIFunction.cs | 16 +- .../FoundryToolboxOptions.cs | 17 +- .../FoundryToolboxService.cs | 12 +- .../OAuthConsentLinkPolicy.cs | 116 ++++++++ .../ToolboxConsentParser.cs | 97 ------- .../OAuthConsentEmissionTests.cs | 22 ++ .../OAuthConsentLinkPolicyTests.cs | 95 ++++++ .../OAuthConsentPolicyHandlerTests.cs | 270 ++++++++++++++++++ .../ToolboxConsentParserTests.cs | 78 ----- .../_responses.py | 3 +- .../foundry_hosting/tests/test_responses.py | 8 +- 12 files changed, 592 insertions(+), 207 deletions(-) create mode 100644 dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/OAuthConsentLinkPolicy.cs create mode 100644 dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentLinkPolicyTests.cs create mode 100644 dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentPolicyHandlerTests.cs diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs index 6eed78d848e..77c6a0eb683 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs @@ -37,6 +37,7 @@ public class AgentFrameworkResponseHandler : ResponseHandler { private const string LatestWorkflowCheckpointIdMetadataKey = "_last_checkpoint_id"; private const string UserPartitionName = "user"; + private const string ConsentLinkRejectedMessage = "The OAuth consent request was rejected by the consent link policy."; private readonly IServiceProvider _serviceProvider; private readonly ILogger _logger; @@ -304,6 +305,14 @@ await this._toolboxService .ConfigureAwait(false); if (pendingConsents.Count > 0) { + // A pending consent whose link fails the policy fails the request rather than + // surfacing the link. The toolbox stays pending, so a later request retries it. + if (!this.AreConsentLinksAllowed(pendingConsents, context.ResponseId)) + { + yield return stream.EmitFailed(ResponseErrorCode.ServerError, ConsentLinkRejectedMessage); + yield break; + } + foreach (var consent in pendingConsents) { foreach (var consentEvent in EmitOAuthConsentRequest( @@ -392,6 +401,12 @@ await this._toolboxService // not recorded globally), so it cannot leak onto a request that did not reference the marker. if (markerConsents is { Count: > 0 }) { + if (!this.AreConsentLinksAllowed(markerConsents, context.ResponseId)) + { + yield return stream.EmitFailed(ResponseErrorCode.ServerError, ConsentLinkRejectedMessage); + yield break; + } + foreach (var consent in markerConsents) { foreach (var consentEvent in EmitOAuthConsentRequest( @@ -564,6 +579,15 @@ await PersistWorkflowCheckpointAsync( if (consentInfo is not null) { + // The tool wrapper records every -32006 and cancels the loop, so a rejected link + // fails the turn here instead of reaching the model as a tool error. + if (!this.AreConsentLinksAllowed([consentInfo], context.ResponseId)) + { + turnFailed = true; + yield return stream.EmitFailed(ResponseErrorCode.ServerError, ConsentLinkRejectedMessage); + yield break; + } + // Emit oauth_consent_request output item + incomplete for the consent URL. foreach (var consentEvent in EmitOAuthConsentRequest( stream, @@ -785,18 +809,23 @@ bool CheckNotAllowedStoreUsage() => /// The response event stream to emit on. /// The tool source / server label that requires consent. /// The OAuth consent URL the user must visit. - /// Optional host-owned origin allowlist policy. + /// + /// The consent link policy to enforce. When , + /// is used, so the link must still be a safe absolute HTTPS URL. + /// /// An enumerable of events: output_item.added → output_item.done. + /// The consent link does not satisfy the policy. internal static IEnumerable EmitOAuthConsentRequest( ResponseEventStream stream, string serverLabel, string consentUrl, OAuthConsentLinkPolicy? consentLinkPolicy = null) { - if (consentLinkPolicy?.IsAllowed(consentUrl) == false) + // Callers validate first so they can fail the response cleanly; this is the last line of + // defense and never skips validation, even when no policy is supplied. + if (!(consentLinkPolicy ?? OAuthConsentLinkPolicy.AnySafeOrigin).IsAllowed(consentUrl)) { - throw new InvalidOperationException( - "OAuth consent request did not match the configured allowed origins."); + throw new InvalidOperationException(ConsentLinkRejectedMessage); } var item = new OAuthConsentRequestOutputItem(NewOAuthConsentItemId(), consentUrl, serverLabel); @@ -805,6 +834,34 @@ internal static IEnumerable EmitOAuthConsentRequest( yield return builder.EmitDone(item); } + /// + /// Returns whether every consent link satisfies the toolbox consent link policy. Rejected links are + /// logged by tool and toolbox name only; the URL is not logged because it comes from an external + /// error payload. + /// + private bool AreConsentLinksAllowed(IEnumerable consents, string responseId) + { + var policy = this._toolboxService?.ConsentLinkPolicy ?? OAuthConsentLinkPolicy.AnySafeOrigin; + foreach (var consent in consents) + { + if (!policy.IsAllowed(consent.ConsentUrl)) + { + if (this._logger.IsEnabled(LogLevel.Warning)) + { + this._logger.LogWarning( + "OAuth consent request for tool '{ToolName}' in toolbox '{ToolboxName}' was rejected by the consent link policy for response {ResponseId}.", + consent.ToolName, + consent.ToolboxName, + responseId); + } + + return false; + } + } + + return true; + } + /// /// Generates a wire-format-valid item id for an oauth_consent_request output item. /// The Responses Server SDK requires ids of the shape {prefix}_{50-char-body}; we use the diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ConsentAwareMcpClientAIFunction.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ConsentAwareMcpClientAIFunction.cs index 2283fed6a23..5f3ec0ed9b1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ConsentAwareMcpClientAIFunction.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ConsentAwareMcpClientAIFunction.cs @@ -1,6 +1,5 @@ // Copyright (c) Microsoft. All rights reserved. -using System; using System.Text.Json; using System.Threading; using System.Threading.Tasks; @@ -22,7 +21,7 @@ namespace Microsoft.Agents.AI.Foundry.Hosting; /// and the per-request is cancelled. This causes /// to stop the tool loop (it guards /// exceptions with when (!ct.IsCancellationRequested)) and surfaces an -/// to the handler. The handler then emits the +/// to the handler. The handler then emits the /// mcp_approval_request output item and marks the response as incomplete. /// /// @@ -30,16 +29,11 @@ internal sealed class ConsentAwareMcpClientAIFunction : AIFunction { private readonly McpClientTool _inner; private readonly string _toolboxName; - private readonly OAuthConsentLinkPolicy _consentLinkPolicy; - internal ConsentAwareMcpClientAIFunction( - McpClientTool inner, - string toolboxName, - OAuthConsentLinkPolicy consentLinkPolicy) + internal ConsentAwareMcpClientAIFunction(McpClientTool inner, string toolboxName) { this._inner = inner; this._toolboxName = toolboxName; - this._consentLinkPolicy = consentLinkPolicy; } public override string Name => this._inner.Name; @@ -62,12 +56,6 @@ internal ConsentAwareMcpClientAIFunction( } catch (McpProtocolException ex) when ((int)ex.ErrorCode == -32006) { - if (!this._consentLinkPolicy.IsAllowed(ex.Message)) - { - throw new InvalidOperationException( - "The OAuth consent response did not match the configured allowed origins."); - } - var state = McpConsentContext.Current.Value; if (state is not null) { diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxOptions.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxOptions.cs index 784f6ad6beb..58490923d97 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxOptions.cs @@ -37,13 +37,20 @@ public sealed class FoundryToolboxOptions public bool StrictMode { get; set; } = true; /// - /// Gets the optional exact HTTPS origins allowed for OAuth consent links. + /// Gets or sets the optional exact HTTPS origins allowed for OAuth consent links. /// /// - /// Leave this property to preserve existing behavior without restricting consent-link origins. - /// When a collection is provided, every surfaced consent link must match one of its normalized origins. - /// An empty collection therefore rejects every consent link. Duplicate entries are ignored after normalization. - /// Configure origins such as https://auth.example.com; paths, queries, and fragments are rejected. + /// + /// Every surfaced consent link must be a safe absolute HTTPS URL, whatever this property is set to. + /// Leave this property to accept any such link without restricting its origin. + /// When a collection is provided, every consent link must also match one of its normalized origins, + /// so an empty collection rejects every consent link. Duplicate entries are ignored after normalization. + /// + /// + /// Configure origins such as https://auth.example.com; entries with a path, query, or fragment + /// are rejected when the toolbox service is constructed. A consent link that fails the policy fails + /// the response instead of being surfaced. + /// /// public IList? AllowedOAuthConsentOrigins { get; set; } diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxService.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxService.cs index bc46f44084c..f09cb01d43c 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxService.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxService.cs @@ -60,6 +60,10 @@ public sealed class FoundryToolboxService : IHostedService, IAsyncDisposable private string _agentName = "hosted-agent"; private string _agentVersion = "1.0.0"; + /// + /// Gets the consent link policy built from . + /// The response handler applies it before surfacing any consent link from these toolboxes. + /// internal OAuthConsentLinkPolicy ConsentLinkPolicy { get; } /// @@ -619,11 +623,7 @@ private async Task OpenToolboxAsync( mcpTools = await client.ListToolsAsync(cancellationToken: cancellationToken).ConfigureAwait(false); } catch (McpProtocolException ex) when ( - ToolboxConsentParser.TryParseConsentRequired( - toolboxName, - ex.Message, - this.ConsentLinkPolicy, - out var consents)) + ToolboxConsentParser.TryParseConsentRequired(toolboxName, ex.Message, out var consents)) { // A tool source needs user OAuth consent before it can be enumerated. Dispose the // half-open client and signal the caller, which keeps the container routable and @@ -658,7 +658,7 @@ private async Task OpenToolboxAsync( var wrapped = new List(mcpTools.Count); foreach (var tool in mcpTools) { - wrapped.Add(new ConsentAwareMcpClientAIFunction(tool, toolboxName, this.ConsentLinkPolicy)); + wrapped.Add(new ConsentAwareMcpClientAIFunction(tool, toolboxName)); } _ = version; // reserved for future version-specific routing; currently handled server-side by the proxy. diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/OAuthConsentLinkPolicy.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/OAuthConsentLinkPolicy.cs new file mode 100644 index 00000000000..3eb92d4a877 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/OAuthConsentLinkPolicy.cs @@ -0,0 +1,116 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; + +namespace Microsoft.Agents.AI.Foundry.Hosting; + +/// +/// Validates OAuth consent links before they are surfaced as oauth_consent_request output items. +/// +/// +/// +/// Every link must be an absolute HTTPS URL with a valid host and no user information, whitespace, +/// control characters, or backslashes. This check always runs. +/// +/// +/// When the host configures , the link's +/// normalized origin (scheme, host, and port) must also match one of the configured origins. A +/// configuration skips only this origin check; an empty configuration rejects +/// every link. +/// +/// +internal sealed class OAuthConsentLinkPolicy +{ + private readonly HashSet? _allowedOrigins; + + /// + /// Gets the policy that accepts any safe absolute HTTPS consent link without restricting its origin. + /// + internal static OAuthConsentLinkPolicy AnySafeOrigin { get; } = new(null); + + /// + /// Initializes a new instance of the class. + /// + /// + /// The exact HTTPS origins allowed for consent links, or to allow any safe origin. + /// + /// An entry is not an absolute HTTPS origin. + internal OAuthConsentLinkPolicy(IEnumerable? allowedOrigins) + { + if (allowedOrigins is null) + { + return; + } + + this._allowedOrigins = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (string origin in allowedOrigins) + { + if (!TryNormalizeOrigin(origin, requireOriginOnly: true, out string? normalizedOrigin)) + { + throw new ArgumentException( + $"OAuth consent allowlist entry '{origin}' must be an absolute HTTPS origin without a path, query, or fragment.", + nameof(allowedOrigins)); + } + + this._allowedOrigins.Add(normalizedOrigin); + } + } + + /// + /// Returns whether may be surfaced as an OAuth consent link. + /// + internal bool IsAllowed(string? consentUrl) + { + // URL safety is enforced before the optional origin gate so that an omitted allowlist still + // rejects non-HTTPS schemes such as javascript: or http:. + if (!TryNormalizeOrigin(consentUrl, requireOriginOnly: false, out string? normalizedOrigin)) + { + return false; + } + + return this._allowedOrigins?.Contains(normalizedOrigin) ?? true; + } + + private static bool TryNormalizeOrigin( + string? value, + bool requireOriginOnly, + [NotNullWhen(true)] out string? normalizedOrigin) + { + normalizedOrigin = null; + + if (string.IsNullOrWhiteSpace(value)) + { + return false; + } + + foreach (char character in value) + { + // Backslashes are rejected because Uri normalizes them to forward slashes, which would let + // the validated URL differ from the raw string that clients receive and parse. + if (char.IsWhiteSpace(character) || char.IsControl(character) || character == '\\') + { + return false; + } + } + + if (!Uri.TryCreate(value, UriKind.Absolute, out Uri? uri) + || !string.Equals(uri.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase) + || !string.IsNullOrEmpty(uri.UserInfo) + || uri.HostNameType == UriHostNameType.Unknown + || string.IsNullOrEmpty(uri.Host)) + { + return false; + } + + if (requireOriginOnly + && (uri.AbsolutePath != "/" || !string.IsNullOrEmpty(uri.Query) || !string.IsNullOrEmpty(uri.Fragment))) + { + return false; + } + + normalizedOrigin = uri.GetLeftPart(UriPartial.Authority); + return true; + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolboxConsentParser.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolboxConsentParser.cs index c9e1b845df7..806a613b2be 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolboxConsentParser.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolboxConsentParser.cs @@ -2,7 +2,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.Text.Json; namespace Microsoft.Agents.AI.Foundry.Hosting; @@ -52,17 +51,6 @@ public static bool TryParseConsentRequired( string toolboxName, string? exceptionMessage, out IReadOnlyList consents) - => TryParseConsentRequired( - toolboxName, - exceptionMessage, - OAuthConsentLinkPolicy.AllowAll, - out consents); - - internal static bool TryParseConsentRequired( - string toolboxName, - string? exceptionMessage, - OAuthConsentLinkPolicy consentLinkPolicy, - out IReadOnlyList consents) { consents = []; @@ -114,11 +102,6 @@ internal static bool TryParseConsentRequired( return false; } - if (!consentLinkPolicy.IsAllowed(consentUrl)) - { - return false; - } - string toolName = error.TryGetProperty("name", out var name) && name.ValueKind == JsonValueKind.String ? name.GetString() ?? toolboxName @@ -141,83 +124,3 @@ internal static bool TryParseConsentRequired( return true; } } - -/// -/// Applies an optional host-owned exact-origin allowlist to OAuth consent links. -/// -internal sealed class OAuthConsentLinkPolicy -{ - private readonly HashSet? _allowedOrigins; - - internal static OAuthConsentLinkPolicy AllowAll { get; } = new(null); - - internal OAuthConsentLinkPolicy(IEnumerable? allowedOrigins) - { - if (allowedOrigins is null) - { - return; - } - - this._allowedOrigins = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (string origin in allowedOrigins) - { - if (!TryNormalizeOrigin(origin, requireOriginOnly: true, out string? normalizedOrigin)) - { - throw new ArgumentException( - $"OAuth consent allowlist entry '{origin}' must be an absolute HTTPS origin without a path, query, or fragment.", - nameof(allowedOrigins)); - } - - this._allowedOrigins.Add(normalizedOrigin); - } - } - - internal bool IsAllowed(string? consentUrl) - { - if (this._allowedOrigins is null) - { - return true; - } - - return TryNormalizeOrigin(consentUrl, requireOriginOnly: false, out string? normalizedOrigin) - && this._allowedOrigins.Contains(normalizedOrigin); - } - - private static bool TryNormalizeOrigin( - string? value, - bool requireOriginOnly, - [NotNullWhen(true)] out string? normalizedOrigin) - { - normalizedOrigin = null; - - if (string.IsNullOrWhiteSpace(value)) - { - return false; - } - - foreach (char character in value) - { - if (char.IsWhiteSpace(character) || char.IsControl(character)) - { - return false; - } - } - - if (!Uri.TryCreate(value, UriKind.Absolute, out Uri? uri) - || !string.Equals(uri.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase) - || !string.IsNullOrEmpty(uri.UserInfo) - || uri.HostNameType == UriHostNameType.Unknown) - { - return false; - } - - if (requireOriginOnly - && (uri.AbsolutePath != "/" || !string.IsNullOrEmpty(uri.Query) || !string.IsNullOrEmpty(uri.Fragment))) - { - return false; - } - - normalizedOrigin = uri.GetLeftPart(UriPartial.Authority); - return true; - } -} diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentEmissionTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentEmissionTests.cs index 0d5d4cb673d..37c473c72d4 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentEmissionTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentEmissionTests.cs @@ -87,4 +87,26 @@ void Emit() // Assert Assert.Throws(Emit); } + + [Theory] + [InlineData("http://external.example/authorize")] + [InlineData("javascript:alert(1)")] + public void EmitOAuthConsentRequest_NullPolicy_StillRejectsUnsafeLink(string consentUrl) + { + // Arrange: a null policy falls back to the safe-HTTPS policy instead of skipping validation. + var stream = CreateTestStream(); + + // Act + void Emit() + { + var events = AgentFrameworkResponseHandler.EmitOAuthConsentRequest( + stream, + "outlook_mail", + consentUrl).ToList(); + Assert.Empty(events); + } + + // Assert + Assert.Throws(Emit); + } } diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentLinkPolicyTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentLinkPolicyTests.cs new file mode 100644 index 00000000000..db7a5ad59cc --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentLinkPolicyTests.cs @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; + +namespace Microsoft.Agents.AI.Foundry.Hosting.UnitTests; + +public class OAuthConsentLinkPolicyTests +{ + [Fact] + public void IsAllowed_NullAllowlist_AcceptsAnySafeHttpsOrigin() + { + // Arrange + var policy = new OAuthConsentLinkPolicy(null); + + // Act + var allowed = policy.IsAllowed("https://external.example/authorize?state=1"); + + // Assert + Assert.True(allowed); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("http://external.example/authorize")] + [InlineData("javascript:alert(1)")] + [InlineData("https://user@external.example/authorize")] + [InlineData("https://exter nal.example/authorize")] + [InlineData("https://external.example/authorize\n")] + [InlineData("https:\\\\external.example/authorize")] + [InlineData("/relative/authorize")] + public void IsAllowed_NullAllowlist_RejectsUnsafeLinks(string? consentUrl) + { + // Arrange: an omitted allowlist skips only the origin check, never URL safety. + var policy = new OAuthConsentLinkPolicy(null); + + // Act + var allowed = policy.IsAllowed(consentUrl); + + // Assert + Assert.False(allowed); + } + + [Fact] + public void IsAllowed_EmptyAllowlist_RejectsEveryLink() + { + // Arrange + var policy = new OAuthConsentLinkPolicy([]); + + // Act + var allowed = policy.IsAllowed("https://external.example/authorize"); + + // Assert + Assert.False(allowed); + } + + [Theory] + [InlineData("https://auth.example.com/authorize?state=1", true)] + [InlineData("https://AUTH.example.com/authorize", true)] + [InlineData("https://auth.example.com:443/authorize", true)] + [InlineData("https://login.partner.example:8443/consent", true)] + [InlineData("https://login.partner.example/consent", false)] + [InlineData("https://other.example.com/authorize", false)] + [InlineData("https://auth.example.com.other.example/authorize", false)] + public void IsAllowed_ConfiguredAllowlist_MatchesExactOrigins(string consentUrl, bool expected) + { + // Arrange + var policy = new OAuthConsentLinkPolicy( + [ + "https://auth.example.com", + "https://login.partner.example:8443/", + ]); + + // Act + var allowed = policy.IsAllowed(consentUrl); + + // Assert + Assert.Equal(expected, allowed); + } + + [Theory] + [InlineData("http://auth.example.com")] + [InlineData("https://auth.example.com/path")] + [InlineData("https://auth.example.com?tenant=1")] + [InlineData("https://auth.example.com#fragment")] + [InlineData("auth.example.com")] + public void Constructor_InvalidConfiguredOrigin_Throws(string origin) + { + // Act + void CreatePolicy() => _ = new OAuthConsentLinkPolicy([origin]); + + // Assert + Assert.Throws(CreatePolicy); + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentPolicyHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentPolicyHandlerTests.cs new file mode 100644 index 00000000000..911cfa1385e --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentPolicyHandlerTests.cs @@ -0,0 +1,270 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Azure.AI.AgentServer.Responses; +using Azure.AI.AgentServer.Responses.Models; +using Azure.Core; +using Microsoft.Extensions.AI; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Moq; +using MeaiTextContent = Microsoft.Extensions.AI.TextContent; + +namespace Microsoft.Agents.AI.Foundry.Hosting.UnitTests; + +/// +/// Verifies that applies the toolbox consent link policy on +/// every path that surfaces an oauth_consent_request, and fails the response without exposing a +/// rejected link. +/// +[Collection(FoundryProjectEndpointEnvFixture.Name)] +public class OAuthConsentPolicyHandlerTests +{ + private const string AllowedOrigin = "https://auth.example.com"; + private const string AllowedLink = "https://auth.example.com/authorize?state=1"; + private const string OtherLink = "https://other.example.com/authorize?state=1"; + + [Fact] + public async Task CreateAsync_PendingConsentOutsideAllowlist_FailsWithoutSurfacingLinkAsync() + { + // Arrange + await using var service = await CreatePendingConsentServiceAsync(OtherLink, [AllowedOrigin]); + + // Act + var events = await RunAsync(new TestAgent(), service); + + // Assert + AssertFailedWithoutLink(events, OtherLink); + } + + [Fact] + public async Task CreateAsync_PendingConsentMatchingAllowlist_SurfacesConsentRequestAsync() + { + // Arrange + await using var service = await CreatePendingConsentServiceAsync(AllowedLink, [AllowedOrigin]); + + // Act + var events = await RunAsync(new TestAgent(), service); + + // Assert + AssertConsentSurfaced(events, AllowedLink); + } + + [Fact] + public async Task CreateAsync_PendingUnsafeConsentWithoutAllowlist_FailsWithoutSurfacingLinkAsync() + { + // Arrange: without an allowlist the safe-HTTPS check still applies. + const string UnsafeLink = "http://external.example/authorize"; + await using var service = await CreatePendingConsentServiceAsync(UnsafeLink, allowedOrigins: null); + + // Act + var events = await RunAsync(new TestAgent(), service); + + // Assert + AssertFailedWithoutLink(events, UnsafeLink); + } + + [Fact] + public async Task CreateAsync_MarkerConsentOutsideAllowlist_FailsWithoutSurfacingLinkAsync() + { + // Arrange + await using var service = await CreateStartedServiceAsync( + [AllowedOrigin], + toolboxNames: [], + opener: (name, _, _) => Task.FromResult( + new FoundryToolboxService.ToolboxOpenResult( + Cached: null, + Consents: [new McpConsentInfo(name, $"{name}.tool", OtherLink)]))); + var request = CreateRequest(); + request.Tools.Add(new MCPTool("marker") { ServerUrl = new Uri("foundry-toolbox://marker-a") }); + + // Act + var events = await RunAsync(new TestAgent(), service, request); + + // Assert + AssertFailedWithoutLink(events, OtherLink); + } + + [Fact] + public async Task CreateAsync_PerCallConsentOutsideAllowlist_FailsWithoutSurfacingLinkAsync() + { + // Arrange + await using var service = await CreateStartedServiceAsync([AllowedOrigin], toolboxNames: [], opener: null); + + // Act + var events = await RunAsync(new ConsentRequiringAgent(OtherLink), service); + + // Assert + AssertFailedWithoutLink(events, OtherLink); + } + + [Fact] + public async Task CreateAsync_PerCallConsentWithoutAllowlist_SurfacesConsentRequestAsync() + { + // Arrange + await using var service = await CreateStartedServiceAsync(allowedOrigins: null, toolboxNames: [], opener: null); + + // Act + var events = await RunAsync(new ConsentRequiringAgent(OtherLink), service); + + // Assert + AssertConsentSurfaced(events, OtherLink); + } + + private static void AssertFailedWithoutLink(List events, string rejectedLink) + { + Assert.DoesNotContain(events, e => e is ResponseOutputItemAddedEvent { Item: OAuthConsentRequestOutputItem }); + var failed = Assert.IsType(events[^1]); + Assert.Contains("consent link policy", failed.Response.Error.Message); + Assert.DoesNotContain(rejectedLink, failed.Response.Error.Message); + } + + private static void AssertConsentSurfaced(List events, string consentLink) + { + var added = Assert.Single(events.OfType(), e => e.Item is OAuthConsentRequestOutputItem); + Assert.Equal(consentLink, Assert.IsType(added.Item).ConsentLink); + Assert.IsType(events[^1]); + } + + private static Task CreatePendingConsentServiceAsync(string consentLink, IList? allowedOrigins) + => CreateStartedServiceAsync( + allowedOrigins, + toolboxNames: ["tb"], + opener: (name, _, _) => Task.FromResult( + new FoundryToolboxService.ToolboxOpenResult( + Cached: null, + Consents: [new McpConsentInfo(name, $"{name}.tool", consentLink)]))); + + private static async Task CreateStartedServiceAsync( + IList? allowedOrigins, + IList toolboxNames, + Func>? opener) + { + var options = new FoundryToolboxOptions + { + StrictMode = false, + EndpointOverride = "http://127.0.0.1:1/unused", + AllowedOAuthConsentOrigins = allowedOrigins, + }; + foreach (var name in toolboxNames) + { + options.ToolboxNames.Add(name); + } + + var service = new FoundryToolboxService(Options.Create(options), Mock.Of()) + { + ToolboxOpener = opener, + }; + await service.StartAsync(CancellationToken.None); + return service; + } + + private static CreateResponse CreateRequest() + { + var request = new CreateResponse { Model = "test" }; + request.Input = BinaryData.FromObjectAsJson(new[] + { + new { type = "message", id = "msg_1", status = "completed", role = "user", + content = new[] { new { type = "input_text", text = "Hello" } } } + }); + return request; + } + + private static async Task> RunAsync( + AIAgent agent, + FoundryToolboxService toolboxService, + CreateResponse? request = null) + { + var services = new ServiceCollection(); + services.AddSingleton(new InMemoryAgentSessionStore()); + services.AddSingleton(agent); + services.AddSingleton(new FakeHostedSessionIsolationKeyProvider()); + var handler = new AgentFrameworkResponseHandler( + services.BuildServiceProvider(), + NullLogger.Instance, + toolboxService); + + var context = new Mock("resp_" + new string('0', 46)) { CallBase = true }; + context.Setup(x => x.GetHistoryAsync(It.IsAny())).ReturnsAsync(Array.Empty()); + context.Setup(x => x.GetInputItemsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(Array.Empty()); + + var events = new List(); + await foreach (var evt in handler.CreateAsync(request ?? CreateRequest(), context.Object, CancellationToken.None)) + { + events.Add(evt); + } + + return events; + } + + private sealed class SimpleAgentSession : AgentSession + { + } + + private abstract class AgentBase : AIAgent + { + protected override Task RunCoreAsync( + IEnumerable messages, + AgentSession? session, + AgentRunOptions? options, + CancellationToken cancellationToken = default) => + throw new NotImplementedException(); + + protected override ValueTask CreateSessionCoreAsync(CancellationToken cancellationToken = default) => + new(new SimpleAgentSession()); + + protected override ValueTask SerializeSessionCoreAsync( + AgentSession session, + JsonSerializerOptions? jsonSerializerOptions, + CancellationToken cancellationToken = default) => + new(JsonDocument.Parse("{}").RootElement); + + protected override ValueTask DeserializeSessionCoreAsync( + JsonElement serializedState, + JsonSerializerOptions? jsonSerializerOptions, + CancellationToken cancellationToken = default) => + new(new SimpleAgentSession()); + } + + private sealed class TestAgent : AgentBase + { + protected override async IAsyncEnumerable RunCoreStreamingAsync( + IEnumerable messages, + AgentSession? session, + AgentRunOptions? options, + [EnumeratorCancellation] CancellationToken cancellationToken = default) + { + await Task.Yield(); + yield return new AgentResponseUpdate { MessageId = "resp_msg_1", Contents = [new MeaiTextContent("hi")] }; + } + } + + /// + /// Mirrors on a -32006 response: records the + /// consent on the request state, cancels the tool loop, and surfaces the cancellation. + /// + private sealed class ConsentRequiringAgent(string consentLink) : AgentBase + { + protected override async IAsyncEnumerable RunCoreStreamingAsync( + IEnumerable messages, + AgentSession? session, + AgentRunOptions? options, + [EnumeratorCancellation] CancellationToken cancellationToken = default) + { + var state = McpConsentContext.Current.Value + ?? throw new InvalidOperationException("The handler did not set the consent state."); + state.Pending = new McpConsentInfo("tb", "tb.tool", consentLink); + state.CancellationSource?.Cancel(); + await Task.Yield(); + cancellationToken.ThrowIfCancellationRequested(); + yield break; + } + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/ToolboxConsentParserTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/ToolboxConsentParserTests.cs index ce26b188f20..3d008ef0f16 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/ToolboxConsentParserTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/ToolboxConsentParserTests.cs @@ -1,7 +1,5 @@ // Copyright (c) Microsoft. All rights reserved. -using System; - namespace Microsoft.Agents.AI.Foundry.Hosting.UnitTests; public class ToolboxConsentParserTests @@ -134,82 +132,6 @@ public void TryParseConsentRequired_ConsentErrorWithoutUrl_ReturnsFalse() Assert.Empty(consents); } - [Fact] - public void TryParseConsentRequired_NullOriginAllowlist_PreservesExistingBehavior() - { - // Arrange - const string Message = - "tools/list failed " + - "{\"errors\":[{\"name\":\"send_email\",\"type\":\"mcp\",\"error\":{\"code\":\"CONSENT_REQUIRED\",\"message\":\"https://external.example/authorize\"}}]}"; - var policy = new OAuthConsentLinkPolicy(null); - - // Act - var parsed = ToolboxConsentParser.TryParseConsentRequired("toolbox", Message, policy, out var consents); - - // Assert - Assert.True(parsed); - Assert.Single(consents); - } - - [Fact] - public void TryParseConsentRequired_EmptyOriginAllowlist_RejectsAllOrigins() - { - // Arrange - const string Message = - "tools/list failed " + - "{\"errors\":[{\"name\":\"send_email\",\"type\":\"mcp\",\"error\":{\"code\":\"CONSENT_REQUIRED\",\"message\":\"https://external.example/authorize\"}}]}"; - var policy = new OAuthConsentLinkPolicy([]); - - // Act - var parsed = ToolboxConsentParser.TryParseConsentRequired("toolbox", Message, policy, out var consents); - - // Assert - Assert.False(parsed); - Assert.Empty(consents); - } - - [Theory] - [InlineData("https://auth.example.com/authorize?state=1", true)] - [InlineData("https://auth.example.com:443/authorize", true)] - [InlineData("https://login.partner.example:8443/consent", true)] - [InlineData("https://other.example.com/authorize", false)] - public void TryParseConsentRequired_ConfiguredOriginAllowlist_GatesConsentUrl( - string consentUrl, - bool expected) - { - // Arrange - string message = - "tools/list failed " + - "{\"errors\":[{\"name\":\"send_email\",\"type\":\"mcp\",\"error\":{\"code\":\"CONSENT_REQUIRED\",\"message\":\"" + - consentUrl + - "\"}}]}"; - var policy = new OAuthConsentLinkPolicy( - [ - "https://auth.example.com", - "https://login.partner.example:8443", - ]); - - // Act - var parsed = ToolboxConsentParser.TryParseConsentRequired("toolbox", message, policy, out var consents); - - // Assert - Assert.Equal(expected, parsed); - Assert.Equal(expected ? 1 : 0, consents.Count); - } - - [Theory] - [InlineData("http://auth.example.com")] - [InlineData("https://auth.example.com/path")] - [InlineData("https://auth.example.com?tenant=1")] - public void OAuthConsentLinkPolicy_InvalidConfiguredOrigin_Throws(string origin) - { - // Act - void CreatePolicy() => _ = new OAuthConsentLinkPolicy([origin]); - - // Assert - Assert.Throws(CreatePolicy); - } - [Fact] public void TryParseConsentRequired_MissingToolName_FallsBackToToolboxName() { diff --git a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py index b297e05e40a..04aac5b241d 100644 --- a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py +++ b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py @@ -784,7 +784,8 @@ async def _handle_prepared_response( ) if invalid_consent is not None: validation_error = ValueError( - f"OAuth consent request for tool '{invalid_consent.name}' must include a safe HTTPS consent link." + f"OAuth consent request for tool '{invalid_consent.name}' must include an allowed safe HTTPS " + "consent link." ) logger.error("%s", validation_error) for event in self._emit_failure(response_event_stream, None, validation_error): diff --git a/python/packages/foundry_hosting/tests/test_responses.py b/python/packages/foundry_hosting/tests/test_responses.py index 157e02f87a3..06187869bb6 100644 --- a/python/packages/foundry_hosting/tests/test_responses.py +++ b/python/packages/foundry_hosting/tests/test_responses.py @@ -5157,18 +5157,22 @@ async def test_configured_origin_allowlist_accepts_connect_time_consent(self) -> assert resp.json()["status"] == "incomplete" - async def test_configured_origin_allowlist_rejects_connect_time_consent(self) -> None: + async def test_configured_origin_allowlist_rejects_connect_time_consent( + self, caplog: pytest.LogCaptureFixture + ) -> None: agent = _make_agent( response=AgentResponse(messages=[Message(role="assistant", contents=[Content.from_text("hi")])]) ) agent.__aenter__.side_effect = _make_consent_error("https://other.example.com/authorize") server = _make_server(agent, allowed_oauth_consent_origins=["https://auth.example.com"]) - resp = await _post(server, input_text="hello", stream=False) + with caplog.at_level(logging.ERROR): + resp = await _post(server, input_text="hello", stream=False) body = resp.json() assert body["status"] == "failed" assert not any(item["type"] == "oauth_consent_request" for item in body["output"]) + assert "must include an allowed safe HTTPS consent link" in caplog.text agent.run.assert_not_called() async def test_non_streaming_consent_error_emits_oauth_output_item(self) -> None: From 6b7604b6ac31907cb41d34aaa383d99c0b9380c4 Mon Sep 17 00:00:00 2001 From: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:46:43 +0100 Subject: [PATCH 3/3] Simplify consent link handling after review .NET: move Foundry toolbox resolution out of CreateAsync into ResolveToolboxToolsAsync, and route every consent-required response through a single EmitConsentRequiredResponse helper. Behavior is unchanged. New tests check that a rejected per-call consent link does not save the session and an allowed one does. Python: replace the _OAuthConsentLinkPolicy class with the _normalize_allowed_oauth_consent_origins and _is_allowed_oauth_consent_link functions. --- .../AgentFrameworkResponseHandler.cs | 323 ++++++++++-------- .../OAuthConsentPolicyHandlerTests.cs | 59 +++- .../_responses.py | 49 +-- .../foundry_hosting/tests/test_responses.py | 39 ++- 4 files changed, 277 insertions(+), 193 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs index 77c6a0eb683..c3e91a9d1a1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/AgentFrameworkResponseHandler.cs @@ -271,161 +271,44 @@ public override async IAsyncEnumerable CreateAsync( hostingOptions); chatOptions.Instructions = request.Instructions; - // Inject Foundry Toolbox tools when the toolbox service is available. - // - // Two sources are considered: - // 1. Pre-registered toolboxes (via AddFoundryToolboxes) — always appended. - // 2. Per-request markers embedded in request.Tools (HostedMcpToolboxAITool) - // whose ServerAddress scheme is "foundry-toolbox://". Strict mode rejects - // unknown names; otherwise a lazy MCP client is opened and cached. - // - // Each toolbox's tools are only appended once per request, even if it appears - // in both the pre-registered list and the per-request markers. + // Inject Foundry Toolbox tools when the toolbox service is available. ResolveToolboxToolsAsync + // describes the toolbox sources and returns the tools, the outstanding consents, or an error. if (this._toolboxService is not null) { // Re-apply the call id: the EmitCreated/EmitInProgress yields above reverted the ambient // value, and the toolbox tools/list + consent egress below must carry it per request. HostedCallContext.CallId = platformCallId; - // Retry any pre-registered toolbox that was deferred at startup because it could not be - // enumerated without a per-user context (non-consent failure). The request's egress now - // carries the platform-injected per-user isolation key, so a delegated tool source can - // enumerate as the user — or report that it needs OAuth consent, which is then surfaced - // by ResolvePendingConsentsAsync below. - await this._toolboxService - .RetryDeferredToolboxesAsync(cancellationToken) - .ConfigureAwait(false); - - // Resolve any pre-registered toolbox that was awaiting user OAuth consent at startup - // (CONSENT_REQUIRED at tools/list time). If consent is still outstanding, surface it to - // the caller as an oauth_consent_request and stop: the user completes consent out of band, - // then re-sends the request, at which point enumeration succeeds and the tools appear. - var pendingConsents = await this._toolboxService - .ResolvePendingConsentsAsync(cancellationToken) - .ConfigureAwait(false); - if (pendingConsents.Count > 0) - { - // A pending consent whose link fails the policy fails the request rather than - // surfacing the link. The toolbox stays pending, so a later request retries it. - if (!this.AreConsentLinksAllowed(pendingConsents, context.ResponseId)) - { - yield return stream.EmitFailed(ResponseErrorCode.ServerError, ConsentLinkRejectedMessage); - yield break; - } - - foreach (var consent in pendingConsents) - { - foreach (var consentEvent in EmitOAuthConsentRequest( - stream, - consent.ToolName, - consent.ConsentUrl, - this._toolboxService.ConsentLinkPolicy)) - { - yield return consentEvent; - } - } - - yield return stream.EmitIncomplete(reason: null); - yield break; - } - - List? toolsToAdd = null; - - if (this._toolboxService.Tools.Count > 0) - { - toolsToAdd = [.. this._toolboxService.Tools]; - } - - var markers = InputConverter.ReadMcpToolboxMarkers(request); - var seen = new HashSet(StringComparer.OrdinalIgnoreCase); - string? resolutionError = null; - List? markerConsents = null; - - foreach (var (name, version) in markers) - { - if (!seen.Add(name)) - { - continue; - } - - FoundryToolboxService.ToolboxResolution resolution; - try - { - resolution = await this._toolboxService - .GetToolboxToolsAsync(name, version, cancellationToken) - .ConfigureAwait(false); - } - catch (InvalidOperationException ex) - { - if (this._logger.IsEnabled(LogLevel.Warning)) - { - this._logger.LogWarning( - ex, - "Foundry toolbox '{ToolboxName}' could not be resolved for response {ResponseId}.", - name, - context.ResponseId); - } - - resolutionError = ex.Message; - break; - } - - // The marker hit CONSENT_REQUIRED: collect its consent requirement (request-scoped) - // and keep resolving the other markers so we can surface every outstanding consent at - // once. This toolbox contributes no tools to this turn. - if (resolution.Consents.Count > 0) - { - (markerConsents ??= []).AddRange(resolution.Consents); - continue; - } - - toolsToAdd ??= []; - foreach (var t in resolution.Tools) - { - if (!toolsToAdd.Contains(t)) - { - toolsToAdd.Add(t); - } - } - } + var toolboxResolution = await this.ResolveToolboxToolsAsync( + this._toolboxService, + request, + context.ResponseId, + cancellationToken).ConfigureAwait(false); - if (resolutionError is not null) + if (toolboxResolution.Error is not null) { - yield return stream.EmitFailed(ResponseErrorCode.ServerError, resolutionError); + yield return stream.EmitFailed(ResponseErrorCode.ServerError, toolboxResolution.Error); yield break; } - // A lazy / per-request marker that needs OAuth consent is surfaced as an - // oauth_consent_request and stops this turn, instead of silently running without that - // toolbox. The consent is scoped to this request (it was returned by GetToolboxToolsAsync, - // not recorded globally), so it cannot leak onto a request that did not reference the marker. - if (markerConsents is { Count: > 0 }) + // A toolbox that still needs OAuth consent stops this turn instead of silently running + // without its tools: the user completes consent out of band, then re-sends the request. + if (toolboxResolution.Consents.Count > 0) { - if (!this.AreConsentLinksAllowed(markerConsents, context.ResponseId)) + foreach (var consentEvent in this.EmitConsentRequiredResponse( + stream, + toolboxResolution.Consents, + context.ResponseId)) { - yield return stream.EmitFailed(ResponseErrorCode.ServerError, ConsentLinkRejectedMessage); - yield break; + yield return consentEvent; } - foreach (var consent in markerConsents) - { - foreach (var consentEvent in EmitOAuthConsentRequest( - stream, - consent.ToolName, - consent.ConsentUrl, - this._toolboxService.ConsentLinkPolicy)) - { - yield return consentEvent; - } - } - - yield return stream.EmitIncomplete(reason: null); yield break; } - if (toolsToAdd?.Count > 0) + if (toolboxResolution.Tools.Count > 0) { - chatOptions.Tools = [.. chatOptions.Tools ?? [], .. toolsToAdd]; + chatOptions.Tools = [.. chatOptions.Tools ?? [], .. toolboxResolution.Tools]; } } @@ -579,26 +462,20 @@ await PersistWorkflowCheckpointAsync( if (consentInfo is not null) { + // Emit oauth_consent_request output item + incomplete for the consent URL. // The tool wrapper records every -32006 and cancels the loop, so a rejected link // fails the turn here instead of reaching the model as a tool error. - if (!this.AreConsentLinksAllowed([consentInfo], context.ResponseId)) - { - turnFailed = true; - yield return stream.EmitFailed(ResponseErrorCode.ServerError, ConsentLinkRejectedMessage); - yield break; - } - - // Emit oauth_consent_request output item + incomplete for the consent URL. - foreach (var consentEvent in EmitOAuthConsentRequest( + foreach (var consentEvent in this.EmitConsentRequiredResponse( stream, - consentInfo.ToolName, - consentInfo.ConsentUrl, - this._toolboxService?.ConsentLinkPolicy)) + [consentInfo], + context.ResponseId)) { + // Mark the failure before yielding it, so the finally block below does not keep + // the session even when the caller stops reading at the failed event. + turnFailed |= consentEvent is ResponseFailedEvent; yield return consentEvent; } - yield return stream.EmitIncomplete(reason: null); yield break; } @@ -834,6 +711,144 @@ internal static IEnumerable EmitOAuthConsentRequest( yield return builder.EmitDone(item); } + /// + /// Resolves the Foundry toolbox tools to add to this request, or the reason the turn must stop. + /// + /// + /// The tools to add, the OAuth consents still outstanding, or the error that fails the request. + /// + private async ValueTask ResolveToolboxToolsAsync( + FoundryToolboxService toolboxService, + CreateResponse request, + string responseId, + CancellationToken cancellationToken) + { + // Two sources are considered: + // 1. Pre-registered toolboxes (via AddFoundryToolboxes) — always appended. + // 2. Per-request markers embedded in request.Tools (HostedMcpToolboxAITool) + // whose ServerAddress scheme is "foundry-toolbox://". Strict mode rejects + // unknown names; otherwise a lazy MCP client is opened and cached. + // + // Each toolbox's tools are only appended once per request, even if it appears + // in both the pre-registered list and the per-request markers. + + // Retry any pre-registered toolbox that was deferred at startup because it could not be + // enumerated without a per-user context (non-consent failure). The request's egress now + // carries the platform-injected per-user isolation key, so a delegated tool source can + // enumerate as the user — or report that it needs OAuth consent, which is then surfaced + // by ResolvePendingConsentsAsync below. + await toolboxService + .RetryDeferredToolboxesAsync(cancellationToken) + .ConfigureAwait(false); + + // Resolve any pre-registered toolbox that was awaiting user OAuth consent at startup + // (CONSENT_REQUIRED at tools/list time). If consent is still outstanding, surface it to + // the caller as an oauth_consent_request and stop: the user completes consent out of band, + // then re-sends the request, at which point enumeration succeeds and the tools appear. + // Markers are not resolved in that case, matching a turn that stops before running. + var pendingConsents = await toolboxService + .ResolvePendingConsentsAsync(cancellationToken) + .ConfigureAwait(false); + if (pendingConsents.Count > 0) + { + return new(Tools: [], Consents: pendingConsents, Error: null); + } + + List toolsToAdd = [.. toolboxService.Tools]; + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + List? markerConsents = null; + + foreach (var (name, version) in InputConverter.ReadMcpToolboxMarkers(request)) + { + if (!seen.Add(name)) + { + continue; + } + + FoundryToolboxService.ToolboxResolution resolution; + try + { + resolution = await toolboxService + .GetToolboxToolsAsync(name, version, cancellationToken) + .ConfigureAwait(false); + } + catch (InvalidOperationException ex) + { + if (this._logger.IsEnabled(LogLevel.Warning)) + { + this._logger.LogWarning( + ex, + "Foundry toolbox '{ToolboxName}' could not be resolved for response {ResponseId}.", + name, + responseId); + } + + // A resolution error fails the request even when earlier markers collected consents. + return new(Tools: [], Consents: [], Error: ex.Message); + } + + // The marker hit CONSENT_REQUIRED: collect its consent requirement (request-scoped) + // and keep resolving the other markers so we can surface every outstanding consent at + // once. This toolbox contributes no tools to this turn. + if (resolution.Consents.Count > 0) + { + (markerConsents ??= []).AddRange(resolution.Consents); + continue; + } + + foreach (var t in resolution.Tools) + { + if (!toolsToAdd.Contains(t)) + { + toolsToAdd.Add(t); + } + } + } + + // A lazy / per-request marker that needs OAuth consent is surfaced as an + // oauth_consent_request and stops this turn, instead of silently running without that + // toolbox. The consent is scoped to this request (it was returned by GetToolboxToolsAsync, + // not recorded globally), so it cannot leak onto a request that did not reference the marker. + return markerConsents is { Count: > 0 } + ? new(Tools: [], Consents: markerConsents, Error: null) + : new(Tools: toolsToAdd, Consents: [], Error: null); + } + + /// + /// Emits the terminal events for a turn that stops on OAuth consent: one oauth_consent_request + /// item per consent, then response.incomplete. + /// + /// + /// When any consent link fails the consent link policy, only response.failed is emitted, so + /// no link from that turn is surfaced. A pre-registered toolbox that is awaiting consent stays + /// pending, so a later request retries it. + /// + private IEnumerable EmitConsentRequiredResponse( + ResponseEventStream stream, + IReadOnlyList consents, + string responseId) + { + if (!this.AreConsentLinksAllowed(consents, responseId)) + { + yield return stream.EmitFailed(ResponseErrorCode.ServerError, ConsentLinkRejectedMessage); + yield break; + } + + foreach (var consent in consents) + { + foreach (var consentEvent in EmitOAuthConsentRequest( + stream, + consent.ToolName, + consent.ConsentUrl, + this._toolboxService?.ConsentLinkPolicy)) + { + yield return consentEvent; + } + } + + yield return stream.EmitIncomplete(reason: null); + } + /// /// Returns whether every consent link satisfies the toolbox consent link policy. Rejected links are /// logged by tool and toolbox name only; the URL is not logged because it comes from an external @@ -997,4 +1012,14 @@ private AgentSessionStore ResolveSessionStore(CreateResponse request) return agentName; } + + /// + /// The toolbox outcome for one request. When is set, the request fails. + /// Otherwise, a non-empty stops the turn for OAuth consent, and + /// holds the toolbox tools to add to the run. + /// + private readonly record struct ToolboxRequestResolution( + IReadOnlyList Tools, + IReadOnlyList Consents, + string? Error); } diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentPolicyHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentPolicyHandlerTests.cs index 911cfa1385e..2618f11165c 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentPolicyHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OAuthConsentPolicyHandlerTests.cs @@ -118,6 +118,36 @@ public async Task CreateAsync_PerCallConsentWithoutAllowlist_SurfacesConsentRequ AssertConsentSurfaced(events, OtherLink); } + [Fact] + public async Task CreateAsync_PerCallConsentOutsideAllowlist_DoesNotSaveSessionAsync() + { + // Arrange + await using var service = await CreateStartedServiceAsync([AllowedOrigin], toolboxNames: [], opener: null); + var sessionStore = new CountingSessionStore(); + + // Act + var events = await RunAsync(new ConsentRequiringAgent(OtherLink), service, sessionStore: sessionStore); + + // Assert: a rejected consent link fails the turn, so its session is not kept. + AssertFailedWithoutLink(events, OtherLink); + Assert.Equal(0, sessionStore.SaveAttempts); + } + + [Fact] + public async Task CreateAsync_PerCallConsentMatchingAllowlist_SavesSessionAsync() + { + // Arrange + await using var service = await CreateStartedServiceAsync([AllowedOrigin], toolboxNames: [], opener: null); + var sessionStore = new CountingSessionStore(); + + // Act + var events = await RunAsync(new ConsentRequiringAgent(AllowedLink), service, sessionStore: sessionStore); + + // Assert: a turn waiting on consent is not a failure, so its session is kept for the retry. + AssertConsentSurfaced(events, AllowedLink); + Assert.Equal(1, sessionStore.SaveAttempts); + } + private static void AssertFailedWithoutLink(List events, string rejectedLink) { Assert.DoesNotContain(events, e => e is ResponseOutputItemAddedEvent { Item: OAuthConsentRequestOutputItem }); @@ -180,10 +210,11 @@ private static CreateResponse CreateRequest() private static async Task> RunAsync( AIAgent agent, FoundryToolboxService toolboxService, - CreateResponse? request = null) + CreateResponse? request = null, + AgentSessionStore? sessionStore = null) { var services = new ServiceCollection(); - services.AddSingleton(new InMemoryAgentSessionStore()); + services.AddSingleton(sessionStore ?? new InMemoryAgentSessionStore()); services.AddSingleton(agent); services.AddSingleton(new FakeHostedSessionIsolationKeyProvider()); var handler = new AgentFrameworkResponseHandler( @@ -208,6 +239,30 @@ private sealed class SimpleAgentSession : AgentSession { } + /// Counts session saves and never returns a stored session. + private sealed class CountingSessionStore : AgentSessionStore + { + private int _saveAttempts; + + public int SaveAttempts => this._saveAttempts; + + public override ValueTask SaveSessionAsync( + AIAgent agent, + AgentSessionStoreKey key, + AgentSession session, + CancellationToken cancellationToken = default) + { + Interlocked.Increment(ref this._saveAttempts); + return default; + } + + public override ValueTask GetSessionAsync( + AIAgent agent, + AgentSessionStoreKey key, + CancellationToken cancellationToken = default) => + new((AgentSession?)null); + } + private abstract class AgentBase : AIAgent { protected override Task RunCoreAsync( diff --git a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py index 04aac5b241d..1cc3cf2b4fb 100644 --- a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py +++ b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py @@ -367,25 +367,24 @@ def _normalize_oauth_consent_origin(value: str, *, require_origin_only: bool) -> return f"https://{normalized_host}" if port in (None, 443) else f"https://{normalized_host}:{port}" -class _OAuthConsentLinkPolicy: - """Apply existing URL safety checks plus an optional host-owned origin allowlist.""" - - def __init__(self, allowed_origins: Sequence[str] | None = None) -> None: - self._allowed_origins: frozenset[str] | None = ( - None - if allowed_origins is None - else frozenset( - _normalize_oauth_consent_origin(origin, require_origin_only=True) for origin in allowed_origins - ) - ) +def _normalize_allowed_oauth_consent_origins(allowed_origins: Sequence[str] | None) -> frozenset[str] | None: + """Normalize the host-configured consent origin allowlist, or return ``None`` when none is configured.""" + if allowed_origins is None: + return None + return frozenset(_normalize_oauth_consent_origin(origin, require_origin_only=True) for origin in allowed_origins) - def is_allowed(self, consent_link: object) -> TypeGuard[str]: - """Return whether the link is safe and, when configured, has an allowed origin.""" - if not _is_safe_oauth_consent_link(consent_link): - return False - if self._allowed_origins is None: - return True - return _normalize_oauth_consent_origin(consent_link, require_origin_only=False) in self._allowed_origins + +def _is_allowed_oauth_consent_link(consent_link: object, allowed_origins: frozenset[str] | None) -> TypeGuard[str]: + """Return whether the link is safe and, when an allowlist is configured, has an allowed origin. + + ``allowed_origins`` must come from ``_normalize_allowed_oauth_consent_origins``. ``None`` keeps the + safe-HTTPS check without restricting the origin; an empty set rejects every link. + """ + if not _is_safe_oauth_consent_link(consent_link): + return False + if allowed_origins is None: + return True + return _normalize_oauth_consent_origin(consent_link, require_origin_only=False) in allowed_origins def consent_url_from_error(exc: BaseException) -> list[ConsentError] | None: @@ -629,7 +628,7 @@ def __init__( ) # No caller-owned agent state is mutated until all validation and base-host construction succeed. - self._oauth_consent_link_policy = _OAuthConsentLinkPolicy(allowed_oauth_consent_origins) + self._allowed_oauth_consent_origins = _normalize_allowed_oauth_consent_origins(allowed_oauth_consent_origins) super().__init__(prefix=prefix, options=options, store=store, **kwargs) self._agent_source = agent @@ -778,7 +777,9 @@ async def _handle_prepared_response( ( consent_error for consent_error in consent_errors_to_emit - if not self._oauth_consent_link_policy.is_allowed(consent_error.consent_url) + if not _is_allowed_oauth_consent_link( + consent_error.consent_url, self._allowed_oauth_consent_origins + ) ), None, ) @@ -834,7 +835,7 @@ async def _handle_prepared_response( yield response_event_stream.emit_incomplete() return - tracker = _OutputItemTracker(response_event_stream, self._oauth_consent_link_policy) + tracker = _OutputItemTracker(response_event_stream, self._allowed_oauth_consent_origins) try: if configuration.workflow: inner = self._handle_inner_workflow( @@ -1376,10 +1377,10 @@ class _OutputItemTracker: def __init__( self, stream: ResponseEventStream, - oauth_consent_link_policy: _OAuthConsentLinkPolicy | None = None, + allowed_oauth_consent_origins: frozenset[str] | None = None, ) -> None: self._stream = stream - self._oauth_consent_link_policy = oauth_consent_link_policy or _OAuthConsentLinkPolicy() + self._allowed_oauth_consent_origins = allowed_oauth_consent_origins self._usage_details: UsageDetails | None = None self._active_type: str | None = None self._active_id: str | None = None @@ -1698,7 +1699,7 @@ async def handle( yield event consent_link = content.consent_link - if not self._oauth_consent_link_policy.is_allowed(consent_link): + if not _is_allowed_oauth_consent_link(consent_link, self._allowed_oauth_consent_origins): raise ValueError("OAuth consent request content must include an allowed safe HTTPS consent link.") server_label = content.additional_properties.get("server_label") diff --git a/python/packages/foundry_hosting/tests/test_responses.py b/python/packages/foundry_hosting/tests/test_responses.py index 06187869bb6..b339a4dcfc1 100644 --- a/python/packages/foundry_hosting/tests/test_responses.py +++ b/python/packages/foundry_hosting/tests/test_responses.py @@ -78,9 +78,10 @@ _LATEST_CHECKPOINT_ID_KEY, # pyright: ignore[reportPrivateUsage] CONSENT_ERROR_CODE, ConsentError, + _is_allowed_oauth_consent_link, # pyright: ignore[reportPrivateUsage] _item_to_message, # pyright: ignore[reportPrivateUsage] _json_safe_to_str, # pyright: ignore[reportPrivateUsage] - _OAuthConsentLinkPolicy, # pyright: ignore[reportPrivateUsage] + _normalize_allowed_oauth_consent_origins, # pyright: ignore[reportPrivateUsage] _output_item_to_message, # pyright: ignore[reportPrivateUsage] _OutputItemTracker, # pyright: ignore[reportPrivateUsage] _SignalledIterator, # pyright: ignore[reportPrivateUsage] @@ -5004,22 +5005,18 @@ def test_returns_none_when_message_has_no_json(self) -> None: class TestOAuthConsentLinkPolicy: - @pytest.mark.parametrize( - "policy_kwargs", - [{}, {"allowed_origins": None}], - ids=["omitted", "explicit-none"], - ) - def test_omitted_allowlist_preserves_existing_safe_https_behavior(self, policy_kwargs: dict[str, Any]) -> None: - policy = _OAuthConsentLinkPolicy(**policy_kwargs) + def test_omitted_allowlist_preserves_existing_safe_https_behavior(self) -> None: + allowed_origins = _normalize_allowed_oauth_consent_origins(None) - assert policy.is_allowed("https://external.example/authorize") - assert not policy.is_allowed("http://external.example/authorize") - assert not policy.is_allowed("javascript:alert(1)") + assert allowed_origins is None + assert _is_allowed_oauth_consent_link("https://external.example/authorize", allowed_origins) + assert not _is_allowed_oauth_consent_link("http://external.example/authorize", allowed_origins) + assert not _is_allowed_oauth_consent_link("javascript:alert(1)", allowed_origins) def test_empty_allowlist_rejects_all_origins(self) -> None: - policy = _OAuthConsentLinkPolicy([]) + allowed_origins = _normalize_allowed_oauth_consent_origins([]) - assert not policy.is_allowed("https://external.example/authorize") + assert not _is_allowed_oauth_consent_link("https://external.example/authorize", allowed_origins) @pytest.mark.parametrize( "consent_link", @@ -5030,17 +5027,23 @@ def test_empty_allowlist_rejects_all_origins(self) -> None: ], ) def test_configured_allowlist_accepts_matching_origins(self, consent_link: str) -> None: - policy = _OAuthConsentLinkPolicy([ + allowed_origins = _normalize_allowed_oauth_consent_origins([ "https://auth.example.com", "https://login.partner.example:8443", ]) - assert policy.is_allowed(consent_link) + assert _is_allowed_oauth_consent_link(consent_link, allowed_origins) def test_configured_allowlist_rejects_other_safe_https_origins(self) -> None: - policy = _OAuthConsentLinkPolicy(["https://auth.example.com"]) + allowed_origins = _normalize_allowed_oauth_consent_origins(["https://auth.example.com"]) + + assert not _is_allowed_oauth_consent_link("https://other.example.com/authorize", allowed_origins) + + def test_configured_allowlist_still_rejects_unsafe_links(self) -> None: + allowed_origins = _normalize_allowed_oauth_consent_origins(["https://auth.example.com"]) - assert not policy.is_allowed("https://other.example.com/authorize") + assert not _is_allowed_oauth_consent_link("http://auth.example.com/authorize", allowed_origins) + assert not _is_allowed_oauth_consent_link(None, allowed_origins) @pytest.mark.parametrize( "origin", @@ -5052,7 +5055,7 @@ def test_configured_allowlist_rejects_other_safe_https_origins(self) -> None: ) def test_invalid_allowlist_origin_raises(self, origin: str) -> None: with pytest.raises(ValueError, match="origin"): - _OAuthConsentLinkPolicy([origin]) + _normalize_allowed_oauth_consent_origins([origin]) class TestAgentLifecycle: