diff --git a/benchmarks/CrestApps.Core.Benchmarks/FunctionInvocationToolOrderingBenchmarks.cs b/benchmarks/CrestApps.Core.Benchmarks/FunctionInvocationToolOrderingBenchmarks.cs index 6b569385..a66c7950 100644 --- a/benchmarks/CrestApps.Core.Benchmarks/FunctionInvocationToolOrderingBenchmarks.cs +++ b/benchmarks/CrestApps.Core.Benchmarks/FunctionInvocationToolOrderingBenchmarks.cs @@ -5,7 +5,7 @@ using CrestApps.Core.AI.Handlers; using CrestApps.Core.AI.Models; using CrestApps.Core.AI.Tooling; -using Microsoft.AspNetCore.Http; +using CrestApps.Core.Security; using Microsoft.Extensions.AI; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -42,17 +42,17 @@ public async Task Setup() await VerifyEquivalenceAsync(entries); var evaluator = new AllowAllToolAccessEvaluator(); - var httpContextAccessor = new HttpContextAccessor(); + var userAccessor = CreateUserAccessor(); var serviceProvider = new EmptyServiceProvider(); _legacyHandler = new LegacyFunctionInvocationHandler( evaluator, - httpContextAccessor, + userAccessor, serviceProvider, NullLogger.Instance); _currentHandler = new FunctionInvocationAICompletionServiceHandler( evaluator, - httpContextAccessor, + userAccessor, serviceProvider, NullLogger.Instance); _legacyContext = CreateContext(entries); @@ -123,20 +123,30 @@ private static CompletionServiceConfigureContext CreateContext(IReadOnlyList entries) { var legacyEvaluator = new RecordingToolAccessEvaluator(); var currentEvaluator = new RecordingToolAccessEvaluator(); - var httpContextAccessor = new HttpContextAccessor(); + var userAccessor = CreateUserAccessor(); var serviceProvider = new EmptyServiceProvider(); var legacy = new LegacyFunctionInvocationHandler( legacyEvaluator, - httpContextAccessor, + userAccessor, serviceProvider, NullLogger.Instance); var current = new FunctionInvocationAICompletionServiceHandler( currentEvaluator, - httpContextAccessor, + userAccessor, serviceProvider, NullLogger.Instance); var legacyContext = CreateContext(entries); @@ -162,6 +172,11 @@ public Task IsAuthorizedAsync(ClaimsPrincipal user, string toolName) } } + private sealed class StaticUserAccessor : IUserAccessor + { + public ClaimsPrincipal User { get; set; } + } + private sealed class RecordingToolAccessEvaluator : IAIToolAccessEvaluator { private static readonly Task _allowed = Task.FromResult(true); @@ -209,18 +224,18 @@ protected override ValueTask InvokeCoreAsync( private sealed class LegacyFunctionInvocationHandler { private readonly IAIToolAccessEvaluator _toolAccessEvaluator; - private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IUserAccessor _userAccessor; private readonly IServiceProvider _serviceProvider; private readonly ILogger _logger; public LegacyFunctionInvocationHandler( IAIToolAccessEvaluator toolAccessEvaluator, - IHttpContextAccessor httpContextAccessor, + IUserAccessor userAccessor, IServiceProvider serviceProvider, ILogger logger) { _toolAccessEvaluator = toolAccessEvaluator; - _httpContextAccessor = httpContextAccessor; + _userAccessor = userAccessor; _serviceProvider = serviceProvider; _logger = logger; } @@ -242,14 +257,14 @@ entriesObj is not IReadOnlyList scopedEntries || context.ChatOptions.Tools ??= []; - var user = _httpContextAccessor.HttpContext?.User; + var user = _userAccessor.User; var addedNames = new HashSet(StringComparer.OrdinalIgnoreCase); var orderedEntries = scopedEntries .OrderBy(entry => entry.Source == ToolRegistryEntrySource.McpServer ? 1 : 0); foreach (var entry in orderedEntries) { - if (!await _toolAccessEvaluator.IsAuthorizedAsync(user, entry.Name)) + if (user is not null && !await _toolAccessEvaluator.IsAuthorizedAsync(user, entry.Name)) { if (_logger.IsEnabled(LogLevel.Debug)) { diff --git a/src/Abstractions/CrestApps.Core.Abstractions/Security/IUserAccessor.cs b/src/Abstractions/CrestApps.Core.Abstractions/Security/IUserAccessor.cs new file mode 100644 index 00000000..61cd7c17 --- /dev/null +++ b/src/Abstractions/CrestApps.Core.Abstractions/Security/IUserAccessor.cs @@ -0,0 +1,24 @@ +using System.Security.Claims; + +namespace CrestApps.Core.Security; + +/// +/// Provides access to the that owns the current operation. +/// +/// +/// Services that make security decisions must resolve the caller through this abstraction rather than through +/// IHttpContextAccessor. SignalR dispatches hub methods outside the request pipeline, so +/// IHttpContextAccessor.HttpContext is unreliable during a hub invocation and is frequently . +/// The default implementation returns the principal a hub assigned for the current invocation and falls back to the +/// HTTP request principal when the operation did not originate from a hub. +/// +public interface IUserAccessor +{ + /// + /// Gets or sets the principal that owns the current operation, or when the operation did + /// not originate from a caller. A principal indicates a trusted server-side invocation, + /// such as a background task, rather than an unauthenticated caller. An unauthenticated caller is represented + /// by a whose identity is not authenticated. + /// + ClaimsPrincipal User { get; set; } +} diff --git a/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md b/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md index e03a1132..1c38f6fb 100644 --- a/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md +++ b/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md @@ -120,3 +120,4 @@ description: Initial standalone release notes for the CrestApps.Core repository. - makes the Copilot CLI acquisition work behind corporate proxies and artifact mirrors, and downloads it only once per machine: `CrestApps.Core.AI.Copilot` now resolves the effective npm registry from `NPM_CONFIG_REGISTRY` or `npm config get registry` before the `GitHub.Copilot.SDK` targets download the CLI tarball (the SDK hardcodes `https://registry.npmjs.org`, and MSBuild's `DownloadFile` task cannot read npm configuration), and redirects the SDK's per-project, per-configuration cache to a shared cache under the NuGet global packages folder so a multi-project solution, a fresh worktree, or a CI agent no longer re-downloads the same large tarball for every project; both behaviors are opt-out through `CopilotResolveNpmRegistry` and `CopilotUseSharedCliCache`, the cache location is configurable through `CopilotCliCacheDir` (point it at a pre-seeded directory to build offline), and an explicitly set `CopilotNpmRegistryUrl`, `CopilotCliBinaryPath`, or `CopilotSkipCliDownload` always takes precedence - lets post-session processing invoke parameterized AI tool instances through the new `AIProfilePostSessionSettings.ToolInstanceNames` and `PostSessionTask.ToolInstanceNames`, merged and forwarded to the tool registry alongside the equivalent `ToolNames` so configuring only tool instances is enough to enable the tool-driven post-session path, and surfaces the per-task selection on the **Capabilities** tab of each post-session task in the AI profile create and edit screens of both the MVC and Blazor sample hosts - reports AI tools that were excluded from a completion because the current user is not authorized for them with a single `Warning` log entry per request instead of a `Debug`-only entry, so an answer that silently lost its tools is now traceable in the default logs, and corrects the documented `IAIToolAccessEvaluator` contract to match the implemented `IsAuthorizedAsync(ClaimsPrincipal user, string toolName)` signature +- resolves the caller used for AI tool authorization through the new `IUserAccessor` abstraction instead of `IHttpContextAccessor`, because `HttpContext` is frequently unavailable inside SignalR hub invocations on long-lived transports, backplane-delivered messages, and hosted SignalR services; the built-in AI chat and chat interaction hubs now assign `Context.User` to that accessor for every invocation, and tool authorization is skipped only when there is genuinely no caller (background tasks, workflows, recipes) while unauthenticated callers are still evaluated so host-defined anonymous permissions continue to apply diff --git a/src/CrestApps.Core.Docs/docs/core/tools.md b/src/CrestApps.Core.Docs/docs/core/tools.md index be55f102..a5f58d19 100644 --- a/src/CrestApps.Core.Docs/docs/core/tools.md +++ b/src/CrestApps.Core.Docs/docs/core/tools.md @@ -146,6 +146,40 @@ The default implementation permits every tool. Hosts that enforce permissions, s Tools the user is not authorized for are excluded from the request instead of failing it, so the model simply answers without that capability. Because a missing tool permission usually looks like an incomplete answer, every excluded tool is reported once per request with a `Warning` log entry that lists the denied tool names. +### `IUserAccessor` + +The principal passed to the evaluator comes from `IUserAccessor`, not from `IHttpContextAccessor`: + +```csharp +public interface IUserAccessor +{ + ClaimsPrincipal User { get; set; } +} +``` + +It follows the same shape as `IHttpContextAccessor`. The default implementation resolves the caller in two steps: + +1. If a principal was assigned on the current asynchronous flow, that principal wins. +2. Otherwise it falls back to `HttpContext.User` for ordinary HTTP requests. + +This indirection exists because `IHttpContextAccessor.HttpContext` is unreliable inside SignalR hub invocations. Long-lived transports such as WebSockets, backplane-delivered invocations, and hosted SignalR services all run hub methods outside the request that opened the connection, so the accessor is frequently `null` there. The built-in hubs therefore assign `Context.User` at the start of every invocation: + +```csharp +userAccessor.User = Context.User; + +await DoWorkAsync(); +``` + +The principal is tracked with an `AsyncLocal`, exactly as `HttpContextAccessor` tracks the current request, so the assignment is confined to the invocation that made it. Concurrent connections never observe one another's caller, and the value does not leak back to the caller of the method that assigned it. + +:::info Null means "no caller" +`User` returns `null` only when there is no caller at all, such as a background task, a workflow, or a recipe running server-side. Authorization is skipped in that case and every tool stays available, because trusted server-side code is not a security boundary. + +An unauthenticated caller is different: hubs and HTTP requests always provide a non-`null` `ClaimsPrincipal` with an unauthenticated identity, so the evaluator still runs and can grant or deny tools based on whatever the host allows anonymous users to do. +::: + +Custom hosts that invoke completions outside of an HTTP request or a hub should assign the caller themselves so tool authorization sees the right principal. + ## Custom Tool Registry Provider Supply tools from an external source (database, API, etc.): diff --git a/src/Primitives/CrestApps.Core.AI.Chat/Hubs/AIChatHubCore.cs b/src/Primitives/CrestApps.Core.AI.Chat/Hubs/AIChatHubCore.cs index 94c38ba9..fc103f43 100644 --- a/src/Primitives/CrestApps.Core.AI.Chat/Hubs/AIChatHubCore.cs +++ b/src/Primitives/CrestApps.Core.AI.Chat/Hubs/AIChatHubCore.cs @@ -11,6 +11,7 @@ using CrestApps.Core.AI.Profiles; using CrestApps.Core.AI.ResponseHandling; using CrestApps.Core.AI.Security; +using CrestApps.Core.Security; using CrestApps.Core.AI.Services; using CrestApps.Core.Extensions; using CrestApps.Core.Services; @@ -72,6 +73,33 @@ protected virtual Task ExecuteInScopeAsync(Func action) return action(_services); } + /// + /// Executes an action within a service scope, assigning the caller's principal to + /// so that services can authorize the invocation. + /// + /// The action. + private async Task RunInScopeAsync(Func action) + { + // Capture the principal eagerly. The hub caller context is not guaranteed to remain + // available once the invocation returns, which matters for the streaming path that does + // not await the scope. + var user = Context?.User; + + await ExecuteInScopeAsync(async services => + { + var userAccessor = services.GetService(); + + if (userAccessor is not null) + { + // The accessor tracks the principal with an AsyncLocal, so the assignment is + // scoped to this invocation and never leaks to other connections. + userAccessor.User = user; + } + + await action(services); + }); + } + /// /// Gets the chat context type for this hub. Override when using a different /// chat context type (e.g., ). @@ -467,7 +495,7 @@ protected virtual Task GetDeploymentSettingsAsync(I public virtual ChannelReader SendMessage(string profileId, string prompt, string sessionId, string sessionProfileId, CancellationToken cancellationToken) { var channel = Channel.CreateUnbounded(); - _ = ExecuteInScopeAsync(services => HandleSendMessageAsync(channel.Writer, services, profileId, prompt, sessionId, sessionProfileId, cancellationToken)); + _ = RunInScopeAsync(services => HandleSendMessageAsync(channel.Writer, services, profileId, prompt, sessionId, sessionProfileId, cancellationToken)); return channel.Reader; } @@ -486,7 +514,7 @@ public virtual async Task LoadSession(string sessionId) return; } - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { var sessionManager = services.GetRequiredService(); var profileManager = services.GetRequiredService(); @@ -534,7 +562,7 @@ public virtual async Task StartSession(string profileId, string initialResponseH return; } - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { var sessionManager = services.GetRequiredService(); var profileManager = services.GetRequiredService(); @@ -596,7 +624,7 @@ public virtual async Task RateMessage(string sessionId, string messageId, bool i return; } - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { var sessionManager = services.GetRequiredService(); var profileManager = services.GetRequiredService(); @@ -675,7 +703,7 @@ public virtual async Task HandleNotificationAction(string sessionId, string noti return; } - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { try { @@ -748,7 +776,7 @@ public virtual async Task StartConversation(string profileId, string sessionId, var cancellationToken = Context.ConnectionAborted; try { - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { var profileManager = services.GetRequiredService(); var deploymentManager = services.GetRequiredService(); @@ -864,7 +892,7 @@ public virtual async Task SendAudioStream(string profileId, string sessionId, IA var cancellationToken = Context.ConnectionAborted; try { - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { var profileManager = services.GetRequiredService(); var deploymentManager = services.GetRequiredService(); @@ -954,7 +982,7 @@ public virtual async Task SynthesizeSpeech(string profileId, string sessionId, s var cancellationToken = Context.ConnectionAborted; try { - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { var profileManager = services.GetRequiredService(); var deploymentManager = services.GetRequiredService(); diff --git a/src/Primitives/CrestApps.Core.AI.Chat/Hubs/ChatInteractionHubBase.cs b/src/Primitives/CrestApps.Core.AI.Chat/Hubs/ChatInteractionHubBase.cs index fb20a91a..b6ce79ac 100644 --- a/src/Primitives/CrestApps.Core.AI.Chat/Hubs/ChatInteractionHubBase.cs +++ b/src/Primitives/CrestApps.Core.AI.Chat/Hubs/ChatInteractionHubBase.cs @@ -8,6 +8,8 @@ using CrestApps.Core.AI.Orchestration; using CrestApps.Core.AI.Profiles; using CrestApps.Core.AI.ResponseHandling; +using CrestApps.Core.AI.Security; +using CrestApps.Core.Security; using CrestApps.Core.AI.Services; using CrestApps.Core.AI.Tooling; using CrestApps.Core.Services; @@ -67,6 +69,33 @@ protected virtual Task ExecuteInScopeAsync(Func action) return action(_services); } + /// + /// Executes an action within a service scope, assigning the caller's principal to + /// so that services can authorize the invocation. + /// + /// The action. + private async Task RunInScopeAsync(Func action) + { + // Capture the principal eagerly. The hub caller context is not guaranteed to remain + // available once the invocation returns, which matters for the streaming path that does + // not await the scope. + var user = Context?.User; + + await ExecuteInScopeAsync(async services => + { + var userAccessor = services.GetService(); + + if (userAccessor is not null) + { + // The accessor tracks the principal with an AsyncLocal, so the assignment is + // scoped to this invocation and never leaks to other connections. + userAccessor.User = user; + } + + await action(services); + }); + } + /// /// Gets the chat context type for this hub. /// @@ -442,7 +471,7 @@ public virtual Task StopConversation() public virtual ChannelReader SendMessage(string itemId, string prompt, CancellationToken cancellationToken) { var channel = Channel.CreateUnbounded(); - _ = ExecuteInScopeAsync(services => HandlePromptAsync(channel.Writer, services, itemId, prompt, cancellationToken)); + _ = RunInScopeAsync(services => HandlePromptAsync(channel.Writer, services, itemId, prompt, cancellationToken)); return channel.Reader; } @@ -461,7 +490,7 @@ public virtual async Task LoadInteraction(string itemId) return; } - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { var interactionManager = services.GetRequiredService>(); var promptStore = services.GetRequiredService(); @@ -501,7 +530,7 @@ public virtual async Task SaveSettings(string itemId, JsonElement settings) return; } - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { var interactionManager = services.GetRequiredService>(); var settingsHandlers = services.GetRequiredService>(); @@ -574,7 +603,7 @@ public virtual async Task ClearHistory(string itemId) return; } - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { var interactionManager = services.GetRequiredService>(); var promptStore = services.GetRequiredService(); @@ -624,7 +653,7 @@ public virtual async Task HandleNotificationAction(string sessionId, string noti return; } - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { try { @@ -683,7 +712,7 @@ public virtual async Task StartConversation(string itemId, IAsyncEnumerable + await RunInScopeAsync(async services => { var interactionManager = services.GetRequiredService>(); var deploymentManager = services.GetRequiredService(); @@ -792,7 +821,7 @@ public virtual async Task SendAudioStream(string itemId, IAsyncEnumerable + await RunInScopeAsync(async services => { var interactionManager = services.GetRequiredService>(); var deploymentManager = services.GetRequiredService(); @@ -872,7 +901,7 @@ public virtual async Task SynthesizeSpeech(string itemId, string text, string vo var cancellationToken = Context.ConnectionAborted; try { - await ExecuteInScopeAsync(async services => + await RunInScopeAsync(async services => { var interactionManager = services.GetRequiredService>(); var deploymentManager = services.GetRequiredService(); diff --git a/src/Primitives/CrestApps.Core.AI/Handlers/FunctionInvocationAICompletionServiceHandler.cs b/src/Primitives/CrestApps.Core.AI/Handlers/FunctionInvocationAICompletionServiceHandler.cs index 45c7e395..74162d1b 100644 --- a/src/Primitives/CrestApps.Core.AI/Handlers/FunctionInvocationAICompletionServiceHandler.cs +++ b/src/Primitives/CrestApps.Core.AI/Handlers/FunctionInvocationAICompletionServiceHandler.cs @@ -1,7 +1,7 @@ using CrestApps.Core.AI.Completions; using CrestApps.Core.AI.Models; +using CrestApps.Core.Security; using CrestApps.Core.AI.Tooling; -using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; namespace CrestApps.Core.AI.Handlers; @@ -20,7 +20,7 @@ public sealed class FunctionInvocationAICompletionServiceHandler : IAICompletion public const string ScopedEntriesKey = "_scopedToolEntries"; private readonly IAIToolAccessEvaluator _toolAccessEvaluator; - private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IUserAccessor _userAccessor; private readonly IServiceProvider _serviceProvider; private readonly ILogger _logger; @@ -28,17 +28,17 @@ public sealed class FunctionInvocationAICompletionServiceHandler : IAICompletion /// Initializes a new instance of the class. /// /// The tool access evaluator. - /// The http context accessor. + /// The accessor that resolves the principal owning the current request. /// The service provider. /// The logger. public FunctionInvocationAICompletionServiceHandler( IAIToolAccessEvaluator toolAccessEvaluator, - IHttpContextAccessor httpContextAccessor, + IUserAccessor userAccessor, IServiceProvider serviceProvider, ILogger logger) { _toolAccessEvaluator = toolAccessEvaluator; - _httpContextAccessor = httpContextAccessor; + _userAccessor = userAccessor; _serviceProvider = serviceProvider; _logger = logger; } @@ -63,7 +63,7 @@ entriesObj is not IReadOnlyList scopedEntries || context.ChatOptions.Tools ??= []; - var user = _httpContextAccessor.HttpContext?.User; + var user = _userAccessor.User; var addedNames = new HashSet(StringComparer.OrdinalIgnoreCase); List deniedToolNames = null; @@ -93,7 +93,10 @@ entriesObj is not IReadOnlyList scopedEntries || // like Claude/Copilot manage their own tool selection), MCP tools surfaced here should // still be subject to the same per-user access policy as Local tools to prevent // unauthorized invocation via prompt injection. - if (!await _toolAccessEvaluator.IsAuthorizedAsync(user, entry.Name)) + // A null principal means there is no caller at all, such as a background task, so the + // request is treated as a trusted server-side invocation. An unauthenticated caller is + // represented by a non-null principal and is still evaluated. + if (user is not null && !await _toolAccessEvaluator.IsAuthorizedAsync(user, entry.Name)) { (deniedToolNames ??= []).Add(entry.Name); diff --git a/src/Primitives/CrestApps.Core/ServiceCollectionExtensions.cs b/src/Primitives/CrestApps.Core/ServiceCollectionExtensions.cs index 89bb0ae9..64d0c969 100644 --- a/src/Primitives/CrestApps.Core/ServiceCollectionExtensions.cs +++ b/src/Primitives/CrestApps.Core/ServiceCollectionExtensions.cs @@ -1,6 +1,8 @@ using CrestApps.Core.Builders; using CrestApps.Core.Filters; +using CrestApps.Core.Security; using CrestApps.Core.Services; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.DependencyInjection; @@ -58,6 +60,12 @@ public static IServiceCollection AddCoreServices(this IServiceCollection service services.TryAddScoped(); + // The user accessor falls back to the HTTP request principal, so the HTTP context accessor + // must be available even for hosts that never registered it explicitly. + + services.TryAddSingleton(); + services.TryAddSingleton(); + return services; } diff --git a/src/Primitives/CrestApps.Core/Services/UserAccessor.cs b/src/Primitives/CrestApps.Core/Services/UserAccessor.cs new file mode 100644 index 00000000..28785432 --- /dev/null +++ b/src/Primitives/CrestApps.Core/Services/UserAccessor.cs @@ -0,0 +1,57 @@ +using System.Security.Claims; +using CrestApps.Core.Security; +using Microsoft.AspNetCore.Http; + +namespace CrestApps.Core.Services; + +/// +/// Default implementation. The assigned principal is tracked with an +/// , following the same pattern as , so it flows +/// across asynchronous continuations without depending on the service scope. When no principal has been +/// assigned, the accessor falls back to the principal of the current HTTP request. +/// +public sealed class UserAccessor : IUserAccessor +{ + private static readonly AsyncLocal _current = new(); + + private readonly IHttpContextAccessor _httpContextAccessor; + + /// + /// Initializes a new instance of the class. + /// + /// The HTTP context accessor used when no principal has been assigned. + public UserAccessor(IHttpContextAccessor httpContextAccessor) + { + _httpContextAccessor = httpContextAccessor; + } + + /// + public ClaimsPrincipal? User + { + get => _current.Value?.User ?? _httpContextAccessor.HttpContext?.User; + set + { + var holder = _current.Value; + + if (holder is not null) + { + // Detach the principal from the holder so that asynchronous flows which already + // captured it stop observing the previous value. + holder.User = null; + } + + if (value is not null) + { + _current.Value = new UserHolder + { + User = value, + }; + } + } + } + + private sealed class UserHolder + { + public ClaimsPrincipal? User { get; set; } + } +} diff --git a/tests/CrestApps.Core.Tests/Core/Orchestration/FunctionInvocationAICompletionServiceHandlerTests.cs b/tests/CrestApps.Core.Tests/Core/Orchestration/FunctionInvocationAICompletionServiceHandlerTests.cs index c93c49d8..74f62a7c 100644 --- a/tests/CrestApps.Core.Tests/Core/Orchestration/FunctionInvocationAICompletionServiceHandlerTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Orchestration/FunctionInvocationAICompletionServiceHandlerTests.cs @@ -1,8 +1,8 @@ using System.Security.Claims; using CrestApps.Core.AI.Handlers; using CrestApps.Core.AI.Models; +using CrestApps.Core.Security; using CrestApps.Core.AI.Tooling; -using Microsoft.AspNetCore.Http; using Microsoft.Extensions.AI; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -41,7 +41,7 @@ public async Task ConfigureAsync_StablyPrioritizesNonMcpEntriesAndPreservesDupli var context = new CompletionServiceConfigureContext(new ChatOptions(), completionContext, true); var handler = new FunctionInvocationAICompletionServiceHandler( evaluator, - new HttpContextAccessor(), + new TestUserAccessor(), new EmptyServiceProvider(), NullLogger.Instance); @@ -77,7 +77,7 @@ public async Task ConfigureAsync_SnapshotsEntriesBeforeInvokingFactories() var context = new CompletionServiceConfigureContext(new ChatOptions(), completionContext, true); var handler = new FunctionInvocationAICompletionServiceHandler( evaluator, - new HttpContextAccessor(), + new TestUserAccessor(), new EmptyServiceProvider(), NullLogger.Instance); @@ -108,7 +108,7 @@ public async Task ConfigureAsync_WhenToolsAreDenied_ExcludesThemAndLogsASingleWa var logger = new CapturingLogger(); var handler = new FunctionInvocationAICompletionServiceHandler( evaluator, - new HttpContextAccessor(), + new TestUserAccessor(), new EmptyServiceProvider(), logger); @@ -121,6 +121,65 @@ public async Task ConfigureAsync_WhenToolsAreDenied_ExcludesThemAndLogsASingleWa Assert.Contains("denied-second", warning.Message); } + [Fact] + public async Task ConfigureAsync_WhenThereIsNoCaller_SkipsAuthorizationAndKeepsEveryTool() + { + // Arrange + var evaluator = new RecordingToolAccessEvaluator + { + DeniedToolNames = { "denied" }, + }; + var deniedTool = new TestAIFunction("denied-tool"); + IReadOnlyList entries = + [ + CreateEntry("denied", "denied", ToolRegistryEntrySource.Local, deniedTool), + ]; + var completionContext = new AICompletionContext(); + completionContext.AdditionalProperties[FunctionInvocationAICompletionServiceHandler.ScopedEntriesKey] = entries; + var context = new CompletionServiceConfigureContext(new ChatOptions(), completionContext, true); + var handler = new FunctionInvocationAICompletionServiceHandler( + evaluator, + new TestUserAccessor(user: null), + new EmptyServiceProvider(), + NullLogger.Instance); + + // Act + await handler.ConfigureAsync(context, TestContext.Current.CancellationToken); + + // Assert + Assert.Empty(evaluator.ToolNames); + Assert.Equal([deniedTool], context.ChatOptions.Tools); + } + + [Fact] + public async Task ConfigureAsync_WhenCallerIsAnonymous_StillEvaluatesAuthorization() + { + // Arrange + var evaluator = new RecordingToolAccessEvaluator + { + DeniedToolNames = { "denied" }, + }; + IReadOnlyList entries = + [ + CreateEntry("denied", "denied", ToolRegistryEntrySource.Local, new TestAIFunction("denied-tool")), + ]; + var completionContext = new AICompletionContext(); + completionContext.AdditionalProperties[FunctionInvocationAICompletionServiceHandler.ScopedEntriesKey] = entries; + var context = new CompletionServiceConfigureContext(new ChatOptions(), completionContext, true); + var handler = new FunctionInvocationAICompletionServiceHandler( + evaluator, + new TestUserAccessor(new ClaimsPrincipal(new ClaimsIdentity())), + new EmptyServiceProvider(), + NullLogger.Instance); + + // Act + await handler.ConfigureAsync(context, TestContext.Current.CancellationToken); + + // Assert + Assert.Equal(["denied"], evaluator.ToolNames); + Assert.Empty(context.ChatOptions.Tools); + } + private static ToolRegistryEntry CreateEntry( string id, string name, @@ -171,6 +230,21 @@ public void Log( } } + private sealed class TestUserAccessor : IUserAccessor + { + public TestUserAccessor() + : this(new ClaimsPrincipal(new ClaimsIdentity([new Claim(ClaimTypes.Name, "test-user")], "Test"))) + { + } + + public TestUserAccessor(ClaimsPrincipal user) + { + User = user; + } + + public ClaimsPrincipal User { get; set; } + } + private sealed class EmptyServiceProvider : IServiceProvider { public object GetService(Type serviceType) diff --git a/tests/CrestApps.Core.Tests/Core/Services/UserAccessorTests.cs b/tests/CrestApps.Core.Tests/Core/Services/UserAccessorTests.cs new file mode 100644 index 00000000..4de00a68 --- /dev/null +++ b/tests/CrestApps.Core.Tests/Core/Services/UserAccessorTests.cs @@ -0,0 +1,146 @@ +using System.Security.Claims; +using CrestApps.Core.Services; +using Microsoft.AspNetCore.Http; + +namespace CrestApps.Core.Tests.Core.Services; + +public sealed class UserAccessorTests +{ + [Fact] + public void User_WhenNothingWasAssigned_FallsBackToTheHttpRequestPrincipal() + { + // Arrange + var requestUser = CreatePrincipal("request-user"); + var accessor = new UserAccessor(CreateHttpContextAccessor(requestUser)); + + // Act + var user = accessor.User; + + // Assert + Assert.Same(requestUser, user); + } + + [Fact] + public void User_WhenThereIsNoHttpContext_ReturnsNull() + { + // Arrange + var accessor = new UserAccessor(new HttpContextAccessor()); + + // Act + var user = accessor.User; + + // Assert + Assert.Null(accessor.User); + } + + [Fact] + public void User_WhenAssigned_OverridesTheHttpRequestPrincipal() + { + // Arrange + var hubUser = CreatePrincipal("hub-user"); + var accessor = new UserAccessor(CreateHttpContextAccessor(CreatePrincipal("request-user"))) + { + User = hubUser, + }; + + // Act + var user = accessor.User; + + // Assert + Assert.Same(hubUser, user); + } + + [Fact] + public void User_WhenAssignedNull_FallsBackToTheHttpRequestPrincipal() + { + // Arrange + var requestUser = CreatePrincipal("request-user"); + var accessor = new UserAccessor(CreateHttpContextAccessor(requestUser)) + { + User = CreatePrincipal("hub-user"), + }; + + // Act + accessor.User = null; + + // Assert + Assert.Same(requestUser, accessor.User); + } + + [Fact] + public async Task User_WhenAssigned_FlowsAcrossAsynchronousContinuations() + { + // Arrange + var hubUser = CreatePrincipal("hub-user"); + var accessor = new UserAccessor(new HttpContextAccessor()) + { + User = hubUser, + }; + + // Act + await Task.Yield(); + + var observedUser = await Task.Run(() => accessor.User, TestContext.Current.CancellationToken); + + // Assert + Assert.Same(hubUser, observedUser); + } + + [Fact] + public async Task User_WhenAssignedInsideAnAsynchronousFlow_DoesNotLeakToTheCaller() + { + // Arrange + var accessor = new UserAccessor(new HttpContextAccessor()); + + // Act + var observedUser = await AssignAndObserveAsync(accessor, CreatePrincipal("hub-user")); + + // Assert + Assert.NotNull(observedUser); + Assert.Null(accessor.User); + } + + [Fact] + public async Task User_IsIsolatedBetweenConcurrentFlows() + { + // Arrange + var accessor = new UserAccessor(new HttpContextAccessor()); + var firstUser = CreatePrincipal("first-user"); + var secondUser = CreatePrincipal("second-user"); + + // Act + var results = await Task.WhenAll( + AssignAndObserveAsync(accessor, firstUser), + AssignAndObserveAsync(accessor, secondUser)); + + // Assert + Assert.Same(firstUser, results[0]); + Assert.Same(secondUser, results[1]); + Assert.Null(accessor.User); + } + + private static async Task AssignAndObserveAsync(UserAccessor accessor, ClaimsPrincipal user) + { + accessor.User = user; + + await Task.Delay(10, TestContext.Current.CancellationToken); + + return accessor.User; + } + + private static HttpContextAccessor CreateHttpContextAccessor(ClaimsPrincipal user) + { + return new HttpContextAccessor + { + HttpContext = new DefaultHttpContext + { + User = user, + }, + }; + } + + private static ClaimsPrincipal CreatePrincipal(string name) + { + return new ClaimsPrincipal(new ClaimsIdentity([new Claim(ClaimTypes.Name, name)], "Test")); + } +}