Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<LegacyFunctionInvocationHandler>.Instance);
_currentHandler = new FunctionInvocationAICompletionServiceHandler(
evaluator,
httpContextAccessor,
userAccessor,
serviceProvider,
NullLogger<FunctionInvocationAICompletionServiceHandler>.Instance);
_legacyContext = CreateContext(entries);
Expand Down Expand Up @@ -123,20 +123,30 @@ private static CompletionServiceConfigureContext CreateContext(IReadOnlyList<Too
return new CompletionServiceConfigureContext(new ChatOptions(), completionContext, true);
}

private static StaticUserAccessor CreateUserAccessor()
{
return new StaticUserAccessor
{
User = new ClaimsPrincipal(new ClaimsIdentity(
[new Claim(ClaimTypes.Name, "benchmark")],
"Benchmark")),
};
}

private static async Task VerifyEquivalenceAsync(IReadOnlyList<ToolRegistryEntry> 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<LegacyFunctionInvocationHandler>.Instance);
var current = new FunctionInvocationAICompletionServiceHandler(
currentEvaluator,
httpContextAccessor,
userAccessor,
serviceProvider,
NullLogger<FunctionInvocationAICompletionServiceHandler>.Instance);
var legacyContext = CreateContext(entries);
Expand All @@ -162,6 +172,11 @@ public Task<bool> IsAuthorizedAsync(ClaimsPrincipal user, string toolName)
}
}

private sealed class StaticUserAccessor : IUserAccessor
{
public ClaimsPrincipal User { get; set; }
}

private sealed class RecordingToolAccessEvaluator : IAIToolAccessEvaluator
{
private static readonly Task<bool> _allowed = Task.FromResult(true);
Expand Down Expand Up @@ -209,18 +224,18 @@ protected override ValueTask<object> InvokeCoreAsync(
private sealed class LegacyFunctionInvocationHandler
{
private readonly IAIToolAccessEvaluator _toolAccessEvaluator;
private readonly IHttpContextAccessor _httpContextAccessor;
private readonly IUserAccessor _userAccessor;
private readonly IServiceProvider _serviceProvider;
private readonly ILogger<LegacyFunctionInvocationHandler> _logger;

public LegacyFunctionInvocationHandler(
IAIToolAccessEvaluator toolAccessEvaluator,
IHttpContextAccessor httpContextAccessor,
IUserAccessor userAccessor,
IServiceProvider serviceProvider,
ILogger<LegacyFunctionInvocationHandler> logger)
{
_toolAccessEvaluator = toolAccessEvaluator;
_httpContextAccessor = httpContextAccessor;
_userAccessor = userAccessor;
_serviceProvider = serviceProvider;
_logger = logger;
}
Expand All @@ -242,14 +257,14 @@ entriesObj is not IReadOnlyList<ToolRegistryEntry> scopedEntries ||

context.ChatOptions.Tools ??= [];

var user = _httpContextAccessor.HttpContext?.User;
var user = _userAccessor.User;
var addedNames = new HashSet<string>(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))
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using System.Security.Claims;

namespace CrestApps.Core.Security;

/// <summary>
/// Provides access to the <see cref="ClaimsPrincipal"/> that owns the current operation.
/// </summary>
/// <remarks>
/// Services that make security decisions must resolve the caller through this abstraction rather than through
/// <c>IHttpContextAccessor</c>. SignalR dispatches hub methods outside the request pipeline, so
/// <c>IHttpContextAccessor.HttpContext</c> is unreliable during a hub invocation and is frequently <see langword="null"/>.
/// 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.
/// </remarks>
public interface IUserAccessor
{
/// <summary>
/// Gets or sets the principal that owns the current operation, or <see langword="null"/> when the operation did
/// not originate from a caller. A <see langword="null"/> principal indicates a trusted server-side invocation,
/// such as a background task, rather than an unauthenticated caller. An unauthenticated caller is represented
/// by a <see cref="ClaimsPrincipal"/> whose identity is not authenticated.
/// </summary>
ClaimsPrincipal User { get; set; }
}
1 change: 1 addition & 0 deletions src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
34 changes: 34 additions & 0 deletions src/CrestApps.Core.Docs/docs/core/tools.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>`, 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.):
Expand Down
44 changes: 36 additions & 8 deletions src/Primitives/CrestApps.Core.AI.Chat/Hubs/AIChatHubCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -72,6 +73,33 @@ protected virtual Task ExecuteInScopeAsync(Func<IServiceProvider, Task> action)
return action(_services);
}

/// <summary>
/// Executes an action within a service scope, assigning the caller's principal to
/// <see cref="IUserAccessor"/> so that services can authorize the invocation.
/// </summary>
/// <param name="action">The action.</param>
private async Task RunInScopeAsync(Func<IServiceProvider, Task> 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<IUserAccessor>();

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);
});
}

/// <summary>
/// Gets the chat context type for this hub. Override when using a different
/// chat context type (e.g., <see cref="ChatContextType.ChatInteraction"/>).
Expand Down Expand Up @@ -467,7 +495,7 @@ protected virtual Task<DefaultAIDeploymentSettings> GetDeploymentSettingsAsync(I
public virtual ChannelReader<CompletionPartialMessage> SendMessage(string profileId, string prompt, string sessionId, string sessionProfileId, CancellationToken cancellationToken)
{
var channel = Channel.CreateUnbounded<CompletionPartialMessage>();
_ = 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;
}
Expand All @@ -486,7 +514,7 @@ public virtual async Task LoadSession(string sessionId)
return;
}

await ExecuteInScopeAsync(async services =>
await RunInScopeAsync(async services =>
{
var sessionManager = services.GetRequiredService<IAIChatSessionManager>();
var profileManager = services.GetRequiredService<IAIProfileManager>();
Expand Down Expand Up @@ -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<IAIChatSessionManager>();
var profileManager = services.GetRequiredService<IAIProfileManager>();
Expand Down Expand Up @@ -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<IAIChatSessionManager>();
var profileManager = services.GetRequiredService<IAIProfileManager>();
Expand Down Expand Up @@ -675,7 +703,7 @@ public virtual async Task HandleNotificationAction(string sessionId, string noti
return;
}

await ExecuteInScopeAsync(async services =>
await RunInScopeAsync(async services =>
{
try
{
Expand Down Expand Up @@ -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<IAIProfileManager>();
var deploymentManager = services.GetRequiredService<IAIDeploymentManager>();
Expand Down Expand Up @@ -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<IAIProfileManager>();
var deploymentManager = services.GetRequiredService<IAIDeploymentManager>();
Expand Down Expand Up @@ -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<IAIProfileManager>();
var deploymentManager = services.GetRequiredService<IAIDeploymentManager>();
Expand Down
Loading
Loading