diff --git a/src/Components/Server/src/Builder/ComponentEndpointRouteBuilderExtensions.cs b/src/Components/Server/src/Builder/ComponentEndpointRouteBuilderExtensions.cs index 26cca2322d0f..fa3379264e6a 100644 --- a/src/Components/Server/src/Builder/ComponentEndpointRouteBuilderExtensions.cs +++ b/src/Components/Server/src/Builder/ComponentEndpointRouteBuilderExtensions.cs @@ -73,7 +73,11 @@ public static ComponentEndpointConventionBuilder MapBlazorHub( ArgumentNullException.ThrowIfNull(path); ArgumentNullException.ThrowIfNull(configureOptions); - var hubEndpoint = endpoints.MapHub(path, configureOptions); + var hubEndpoint = endpoints.MapHub(path, options => + { + options.EnableAuthenticationRefresh = true; + configureOptions(options); + }); var disconnectEndpoint = endpoints.Map( (path.EndsWith('/') ? path : path + "/") + "disconnect/", diff --git a/src/Components/Server/src/ComponentHub.cs b/src/Components/Server/src/ComponentHub.cs index 53afb267b2a6..23138682b395 100644 --- a/src/Components/Server/src/ComponentHub.cs +++ b/src/Components/Server/src/ComponentHub.cs @@ -5,6 +5,7 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Components.Server.Circuits; +using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; @@ -72,6 +73,15 @@ public ComponentHub( /// public static PathString DefaultPath { get; } = "/_blazor"; + public override Task OnConnectedAsync() + { + // ComponentHub owns authentication state at the circuit layer and does not use SignalR + // groups or user routing, so it can accept an identity change without rekeying those. + Context.Features.Get()?.OnUserRefreshing = static _ => true; + + return Task.CompletedTask; + } + public override Task OnDisconnectedAsync(Exception exception) { // If the CircuitHost is gone now this isn't an error. This could happen if the disconnect diff --git a/src/Components/Server/test/Circuits/ComponentHubTest.cs b/src/Components/Server/test/Circuits/ComponentHubTest.cs index 69bcfe69c827..9d2108195e4a 100644 --- a/src/Components/Server/test/Circuits/ComponentHubTest.cs +++ b/src/Components/Server/test/Circuits/ComponentHubTest.cs @@ -7,6 +7,7 @@ using System.Text.RegularExpressions; using Microsoft.AspNetCore.Components.Authorization; using Microsoft.AspNetCore.Components.Server.Circuits; +using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Connections.Features; @@ -335,6 +336,21 @@ public async Task ResumeCircuitFailsWithUnresolvedCircuitHandlerDependency_Notif mockClientProxy.Verify(m => m.SendCoreAsync("JS.Error", new[] { errorMessage }, It.IsAny()), Times.Once()); } + [Fact] + public async Task OnConnectedAsyncReplacesSignalRUserRefreshPolicy() + { + var userRefreshFeature = new Mock(); + userRefreshFeature.SetupAllProperties(); + userRefreshFeature.Object.OnUserRefreshing = static _ => false; + var (_, hub) = InitializeComponentHub(userRefreshFeature: userRefreshFeature.Object); + + await hub.OnConnectedAsync(); + + var callback = userRefreshFeature.Object.OnUserRefreshing; + Assert.NotNull(callback); + Assert.True(callback(new ClaimsPrincipal())); + } + [Fact] public async Task OnAuthenticationRefreshedAsyncUpdatesCircuitUser() { @@ -372,7 +388,8 @@ private static (Mock, ComponentHub) InitializeComponentHub( ICircuitHandleRegistry handleRegistry = null, ICircuitPersistenceProvider provider = null, ICircuitFactory circuitFactory = null, - ClaimsPrincipal user = null) + ClaimsPrincipal user = null, + IConnectionUserRefreshFeature userRefreshFeature = null) { deserializer ??= new TestServerComponentDeserializer(); var ephemeralDataProtectionProvider = new EphemeralDataProtectionProvider(); @@ -416,6 +433,10 @@ private static (Mock, ComponentHub) InitializeComponentHub( var httpContextFeature = new Mock(); httpContextFeature.Setup(x => x.HttpContext).Returns(() => new DefaultHttpContext()); feature.Set(httpContextFeature.Object); + if (userRefreshFeature is not null) + { + feature.Set(userRefreshFeature); + } mockContext.Setup(x => x.Features).Returns(feature); mockContext.Setup(x => x.ConnectionId).Returns("123"); mockContext.Setup(x => x.User).Returns(user ?? new ClaimsPrincipal()); diff --git a/src/Components/Server/test/ComponentEndpointRouteBuilderExtensionsTest.cs b/src/Components/Server/test/ComponentEndpointRouteBuilderExtensionsTest.cs index 5d28dae0be1a..6997e2913454 100644 --- a/src/Components/Server/test/ComponentEndpointRouteBuilderExtensionsTest.cs +++ b/src/Components/Server/test/ComponentEndpointRouteBuilderExtensionsTest.cs @@ -21,17 +21,47 @@ public void MapBlazorHub_WiresUp_UnderlyingHub() // Arrange var applicationBuilder = CreateAppBuilder(); var called = false; + var authenticationRefreshEnabled = false; // Act var app = applicationBuilder .UseRouting() .UseEndpoints(endpoints => { - endpoints.MapBlazorHub(dispatchOptions => called = true); + endpoints.MapBlazorHub(dispatchOptions => + { + called = true; + authenticationRefreshEnabled = dispatchOptions.EnableAuthenticationRefresh; + }); }).Build(); // Assert Assert.True(called); + Assert.True(authenticationRefreshEnabled); + } + + [Fact] + public void MapBlazorHub_AllowsAuthenticationRefreshToBeDisabled() + { + var applicationBuilder = CreateAppBuilder(); + var authenticationRefreshEnabledBeforeConfiguration = false; + var endpointDisplayNames = new List(); + + var app = applicationBuilder + .UseRouting() + .UseEndpoints(endpoints => + { + endpoints.MapBlazorHub(dispatchOptions => + { + authenticationRefreshEnabledBeforeConfiguration = dispatchOptions.EnableAuthenticationRefresh; + dispatchOptions.EnableAuthenticationRefresh = false; + }).Finally(builder => endpointDisplayNames.Add(builder.DisplayName)); + }).Build(); + + app.Invoke(new DefaultHttpContext()); + + Assert.True(authenticationRefreshEnabledBeforeConfiguration); + Assert.DoesNotContain("/_blazor/refresh", endpointDisplayNames); } [Fact] @@ -85,9 +115,10 @@ public void MapBlazorHub_AppliesFinalConventionToEachBuilder() Assert.True(called); // Final conventions are applied to each of the builders // in the Blazor component hub - Assert.Equal(4, buildersAffected.Count); + Assert.Equal(5, buildersAffected.Count); Assert.Contains("/_blazor/negotiate", buildersAffected); Assert.Contains("/_blazor", buildersAffected); + Assert.Contains("/_blazor/refresh", buildersAffected); Assert.Contains("Blazor disconnect", buildersAffected); Assert.Contains("Blazor initializers", buildersAffected); } diff --git a/src/Components/Web.JS/src/Platform/Circuits/CircuitManager.ts b/src/Components/Web.JS/src/Platform/Circuits/CircuitManager.ts index 6188c4ec5ab0..5b7f432e326e 100644 --- a/src/Components/Web.JS/src/Platform/Circuits/CircuitManager.ts +++ b/src/Components/Web.JS/src/Platform/Circuits/CircuitManager.ts @@ -139,7 +139,8 @@ export class CircuitManager implements DotNet.DotNetCallDispatcher { const connectionBuilder = new HubConnectionBuilder() .withUrl('_blazor') - .withHubProtocol(hubProtocol); + .withHubProtocol(hubProtocol) + .withAuthenticationRefresh(); this._options.configureSignalR(connectionBuilder); diff --git a/src/Components/Web.JS/test/Platform/Circuits/CircuitManagerAuthenticationRefresh.test.ts b/src/Components/Web.JS/test/Platform/Circuits/CircuitManagerAuthenticationRefresh.test.ts new file mode 100644 index 000000000000..54a3dbea21e4 --- /dev/null +++ b/src/Components/Web.JS/test/Platform/Circuits/CircuitManagerAuthenticationRefresh.test.ts @@ -0,0 +1,50 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +import { afterEach, describe, expect, jest, test } from '@jest/globals'; +import { HubConnection, HubConnectionBuilder } from '@microsoft/signalr'; +import { CircuitManager } from '../../../src/Platform/Circuits/CircuitManager'; +import { resolveOptions } from '../../../src/Platform/Circuits/CircuitStartOptions'; +import { JSEventRegistry } from '../../../src/Services/JSEventRegistry'; + +interface InternalCircuitManager { + startConnection(): Promise; +} + +describe('CircuitManager authentication refresh', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('enables authentication refresh before applying user configuration', async () => { + const configuredOptions: unknown[] = []; + jest.spyOn(HubConnectionBuilder.prototype, 'withAuthenticationRefresh') + .mockImplementation(function (this: HubConnectionBuilder, options = {}) { + configuredOptions.push(options); + return this; + }); + + const connection = { + on: jest.fn(), + onclose: jest.fn(), + start: () => Promise.resolve(), + } as unknown as HubConnection; + jest.spyOn(HubConnectionBuilder.prototype, 'build').mockReturnValue(connection); + + const options = resolveOptions({ + configureSignalR: builder => { + builder.withAuthenticationRefresh({ enableAutoRefresh: false }); + }, + }); + const circuit = new CircuitManager( + {} as never, + '', + options, + { log: () => { /* no-op */ } } as never, + new JSEventRegistry()); + + await (circuit as unknown as InternalCircuitManager).startConnection(); + + expect(configuredOptions).toEqual([{}, { enableAutoRefresh: false }]); + }); +}); diff --git a/src/Components/test/E2ETest/Infrastructure/WebDriverExtensions/BasicTestAppAuthenticationWebDriverExtensions.cs b/src/Components/test/E2ETest/Infrastructure/WebDriverExtensions/BasicTestAppAuthenticationWebDriverExtensions.cs index 99447204fd19..b1b078d38b55 100644 --- a/src/Components/test/E2ETest/Infrastructure/WebDriverExtensions/BasicTestAppAuthenticationWebDriverExtensions.cs +++ b/src/Components/test/E2ETest/Infrastructure/WebDriverExtensions/BasicTestAppAuthenticationWebDriverExtensions.cs @@ -8,13 +8,19 @@ namespace Microsoft.AspNetCore.Components.E2ETest; internal static class BasicTestAppAuthenticationWebDriverExtensions { - public static void SignInAs(this IWebDriver browser, Uri baseUri, string usernameOrNull, string rolesOrNull, bool useSeparateTab = false) + public static void SignInAs( + this IWebDriver browser, + Uri baseUri, + string usernameOrNull, + string rolesOrNull, + bool useSeparateTab = false, + bool includeNameIdentifier = false) { var basePath = baseUri.LocalPath.EndsWith('/') ? baseUri.LocalPath : baseUri.LocalPath + "/"; var authenticationPageUrl = $"{basePath}Authentication"; var baseRelativeUri = usernameOrNull == null ? $"{authenticationPageUrl}?signout=true" - : $"{authenticationPageUrl}?username={usernameOrNull}&roles={rolesOrNull}"; + : $"{authenticationPageUrl}?username={usernameOrNull}&roles={rolesOrNull}&nameIdentifier={includeNameIdentifier.ToString().ToLowerInvariant()}"; if (useSeparateTab) { diff --git a/src/Components/test/E2ETest/ServerExecutionTests/ServerAuthTest.cs b/src/Components/test/E2ETest/ServerExecutionTests/ServerAuthTest.cs index 9bbfdf7174c2..5b9eff853310 100644 --- a/src/Components/test/E2ETest/ServerExecutionTests/ServerAuthTest.cs +++ b/src/Components/test/E2ETest/ServerExecutionTests/ServerAuthTest.cs @@ -52,33 +52,51 @@ void AssertState(string username) [Fact] public void UpdatesAuthenticationStateWhenAuthenticationRefreshed() { - SignInAs("Someone", "IrrelevantRole"); + SignInAs("user-a", "TestRole", includeNameIdentifier: true); var appElement = MountAndNavigateToAuthTest(AuthorizeViewCases, "?captureAuthenticationRefresh"); - Browser.Equal("You're not authorized, Someone", () => - appElement.FindElement(By.CssSelector("#authorize-role .not-authorized")).Text); + Browser.Equal("Welcome, user-a!", () => + appElement.FindElement(By.CssSelector("#authorize-role .authorized")).Text); var javascript = (IJavaScriptExecutor)Browser; var connectionId = Assert.IsType( javascript.ExecuteScript("return authenticationRefreshConnection.connectionId;")); - SignInAs("Someone", "TestRole", useSeparateTab: true); - var refreshError = javascript.ExecuteAsyncScript(""" - const callback = arguments[arguments.length - 1]; - authenticationRefreshConnection.refreshAuthentication().then( - () => callback(), - error => callback(String(error))); - """); - - Assert.Null(refreshError); - Browser.Equal("Welcome, Someone!", () => + SignInAs("user-b", "TestRole", useSeparateTab: true, includeNameIdentifier: true); + Assert.Null(RefreshAuthentication()); + Browser.Equal("Welcome, user-b!", () => appElement.FindElement(By.CssSelector("#authorize-role .authorized")).Text); Assert.Equal( connectionId, Assert.IsType(javascript.ExecuteScript("return authenticationRefreshConnection.connectionId;"))); + + SignInAs(null, null, useSeparateTab: true); + Assert.Null(RefreshAuthentication()); + Browser.Equal("You're not authorized, anonymous", () => + appElement.FindElement(By.CssSelector("#authorize-role .not-authorized")).Text); + Assert.Equal( + connectionId, + Assert.IsType(javascript.ExecuteScript("return authenticationRefreshConnection.connectionId;"))); + + object RefreshAuthentication() => + javascript.ExecuteAsyncScript(""" + const callback = arguments[arguments.length - 1]; + authenticationRefreshConnection.refreshAuthentication().then( + () => callback(), + error => callback(String(error))); + """); } - private void SignInAs(string usernName, string roles, bool useSeparateTab = false) => - Browser.SignInAs(new Uri(_serverFixture.RootUri, "/subdir"), usernName, roles, useSeparateTab); + private void SignInAs( + string userName, + string roles, + bool useSeparateTab = false, + bool includeNameIdentifier = false) => + Browser.SignInAs( + new Uri(_serverFixture.RootUri, "/subdir"), + userName, + roles, + useSeparateTab, + includeNameIdentifier); private void PerformReconnection() { diff --git a/src/Components/test/testassets/Components.TestServer/AuthenticationStartup.cs b/src/Components/test/testassets/Components.TestServer/AuthenticationStartup.cs index 1b110815a4d4..f92842b82d1b 100644 --- a/src/Components/test/testassets/Components.TestServer/AuthenticationStartup.cs +++ b/src/Components/test/testassets/Components.TestServer/AuthenticationStartup.cs @@ -61,7 +61,7 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) { endpoints.MapControllers(); endpoints.MapRazorPages(); - endpoints.MapBlazorHub(options => options.EnableAuthenticationRefresh = true) + endpoints.MapBlazorHub() .AddEndpointFilter(async (context, next) => { if (context.HttpContext.WebSockets.IsWebSocketRequest) diff --git a/src/Components/test/testassets/Components.TestServer/Pages/Authentication.cshtml b/src/Components/test/testassets/Components.TestServer/Pages/Authentication.cshtml index 5609fe8b97cc..9b56a58df87b 100644 --- a/src/Components/test/testassets/Components.TestServer/Pages/Authentication.cshtml +++ b/src/Components/test/testassets/Components.TestServer/Pages/Authentication.cshtml @@ -68,6 +68,10 @@ new Claim(ClaimTypes.Name, username), new Claim("test-claim", "Test claim value"), }; + if (Request.Query["nameIdentifier"] == "true") + { + claims.Add(new Claim(ClaimTypes.NameIdentifier, username)); + } var roles = Request.Query["roles"]; if (!string.IsNullOrEmpty(roles)) diff --git a/src/Servers/Connections.Abstractions/src/Features/IConnectionUserRefreshFeature.cs b/src/Servers/Connections.Abstractions/src/Features/IConnectionUserRefreshFeature.cs index 1a72524d972b..31cc0debca3d 100644 --- a/src/Servers/Connections.Abstractions/src/Features/IConnectionUserRefreshFeature.cs +++ b/src/Servers/Connections.Abstractions/src/Features/IConnectionUserRefreshFeature.cs @@ -7,11 +7,23 @@ namespace Microsoft.AspNetCore.Connections.Features; /// -/// A feature that allows callbacks to be notified when the user associated with the connection is refreshed, -/// for example, via an authentication refresh. +/// A feature that allows components to validate and observe changes to the user associated with a connection, +/// for example, during an authentication refresh. /// public interface IConnectionUserRefreshFeature { + /// + /// Gets or sets the callback invoked before the is refreshed. + /// + /// + /// The callback is invoked synchronously while the user update is locked and must return + /// for the update to proceed. It should complete quickly and must not block or reenter the user update. + /// An exception thrown by the callback rejects the update and is not propagated to the caller. + /// Setting this property replaces any previously configured callback. When set to , + /// the feature implementation's default validation policy applies. + /// + Func? OnUserRefreshing { get; set; } + /// /// Registers a callback to be invoked after the has been refreshed. /// diff --git a/src/Servers/Connections.Abstractions/src/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/src/Servers/Connections.Abstractions/src/PublicAPI/net10.0/PublicAPI.Unshipped.txt index 737ba6d817df..fb801a3af5c6 100644 --- a/src/Servers/Connections.Abstractions/src/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/src/Servers/Connections.Abstractions/src/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -4,4 +4,6 @@ Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature.InitialT Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature.RefreshAuthenticationAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshed(System.Action! callback, object? state) -> System.IDisposable! +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshing.get -> System.Func? +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshing.set -> void Microsoft.AspNetCore.Connections.Features.ITlsHandshakeFeature.Exception.get -> System.Exception? diff --git a/src/Servers/Connections.Abstractions/src/PublicAPI/net11.0/PublicAPI.Unshipped.txt b/src/Servers/Connections.Abstractions/src/PublicAPI/net11.0/PublicAPI.Unshipped.txt index 737ba6d817df..fb801a3af5c6 100644 --- a/src/Servers/Connections.Abstractions/src/PublicAPI/net11.0/PublicAPI.Unshipped.txt +++ b/src/Servers/Connections.Abstractions/src/PublicAPI/net11.0/PublicAPI.Unshipped.txt @@ -4,4 +4,6 @@ Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature.InitialT Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature.RefreshAuthenticationAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshed(System.Action! callback, object? state) -> System.IDisposable! +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshing.get -> System.Func? +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshing.set -> void Microsoft.AspNetCore.Connections.Features.ITlsHandshakeFeature.Exception.get -> System.Exception? diff --git a/src/Servers/Connections.Abstractions/src/PublicAPI/net462/PublicAPI.Unshipped.txt b/src/Servers/Connections.Abstractions/src/PublicAPI/net462/PublicAPI.Unshipped.txt index 5e1a79504064..7ca4e2a350c5 100644 --- a/src/Servers/Connections.Abstractions/src/PublicAPI/net462/PublicAPI.Unshipped.txt +++ b/src/Servers/Connections.Abstractions/src/PublicAPI/net462/PublicAPI.Unshipped.txt @@ -3,4 +3,6 @@ Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature.InitialTokenLifetime.get -> System.TimeSpan? Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature.RefreshAuthenticationAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature -Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshed(System.Action! callback, object? state) -> System.IDisposable! \ No newline at end of file +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshed(System.Action! callback, object? state) -> System.IDisposable! +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshing.get -> System.Func? +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshing.set -> void \ No newline at end of file diff --git a/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index 5e1a79504064..7ca4e2a350c5 100644 --- a/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -3,4 +3,6 @@ Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature.InitialTokenLifetime.get -> System.TimeSpan? Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature.RefreshAuthenticationAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature -Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshed(System.Action! callback, object? state) -> System.IDisposable! \ No newline at end of file +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshed(System.Action! callback, object? state) -> System.IDisposable! +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshing.get -> System.Func? +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshing.set -> void \ No newline at end of file diff --git a/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.1/PublicAPI.Unshipped.txt b/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.1/PublicAPI.Unshipped.txt index 5e1a79504064..7ca4e2a350c5 100644 --- a/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.1/PublicAPI.Unshipped.txt +++ b/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.1/PublicAPI.Unshipped.txt @@ -3,4 +3,6 @@ Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature.InitialTokenLifetime.get -> System.TimeSpan? Microsoft.AspNetCore.Connections.Features.IAuthenticationRefreshFeature.RefreshAuthenticationAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature -Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshed(System.Action! callback, object? state) -> System.IDisposable! \ No newline at end of file +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshed(System.Action! callback, object? state) -> System.IDisposable! +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshing.get -> System.Func? +Microsoft.AspNetCore.Connections.Features.IConnectionUserRefreshFeature.OnUserRefreshing.set -> void \ No newline at end of file diff --git a/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs b/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs index c2d061c6340f..47428bc8cb69 100644 --- a/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs +++ b/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs @@ -70,6 +70,7 @@ public partial class HubConnection : IAsyncDisposable private static readonly MethodInfo _sendStreamItemsMethod = typeof(HubConnection).GetMethods(BindingFlags.NonPublic | BindingFlags.Instance).Single(m => m.Name.Equals(nameof(SendStreamItems))); private static readonly MethodInfo _sendIAsyncStreamItemsMethod = typeof(HubConnection).GetMethods(BindingFlags.NonPublic | BindingFlags.Instance).Single(m => m.Name.Equals(nameof(SendIAsyncEnumerableStreamItems))); + private static readonly TimeSpan _maximumAuthenticationRefreshDelay = TimeSpan.FromMilliseconds(int.MaxValue); // Persistent across all connections private readonly ILoggerFactory _loggerFactory; @@ -90,7 +91,10 @@ public partial class HubConnection : IAsyncDisposable private bool _disposed; // Authentication refresh fields + private readonly object _authRefreshTimerLock = new object(); private Timer? _authRefreshTimer; + private AuthenticationRefreshTimerState? _authRefreshTimerState; + private ConnectionState? _authRefreshConnectionState; // The delay the authentication-refresh timer was last armed with. Exposed for tests to assert scheduling. private TimeSpan _lastAuthenticationRefreshDelay; private readonly AuthenticationRefreshOptions _authenticationRefreshOptions; @@ -553,6 +557,7 @@ private async Task StartAsyncCore(CancellationToken cancellationToken) { await SendHubMessage(startingConnectionState, PingMessage.Instance, cancellationToken).ConfigureAwait(false); } + EnableAuthenticationRefreshTimer(startingConnectionState); startingConnectionState.ReceiveTask = ReceiveLoop(startingConnectionState); // Schedule automatic authentication refresh if enabled and the server reported a token lifetime. @@ -561,7 +566,7 @@ private async Task StartAsyncCore(CancellationToken cancellationToken) var authenticationRefreshFeature = connection.Features.Get(); if (authenticationRefreshFeature?.InitialTokenLifetime is { } initialTokenLifetime && initialTokenLifetime > TimeSpan.Zero) { - ScheduleAuthenticationRefresh(initialTokenLifetime); + ScheduleAuthenticationRefresh(initialTokenLifetime, startingConnectionState); } } @@ -577,12 +582,19 @@ private async Task StartAsyncCore(CancellationToken cancellationToken) /// The new token lifetime from the server, or null. public async Task RefreshAuthenticationAsync(CancellationToken cancellationToken = default) { + CheckDisposed(); + var connectionState = _state.CurrentConnectionStateUnsynchronized; if (connectionState == null) { throw new InvalidOperationException("Cannot refresh authentication when the connection is not active."); } + return await RefreshAuthenticationAsyncCore(connectionState, cancellationToken).ConfigureAwait(false); + } + + private async Task RefreshAuthenticationAsyncCore(ConnectionState connectionState, CancellationToken cancellationToken) + { var connection = connectionState.Connection; var authenticationRefreshFeature = connection.Features.Get(); if (authenticationRefreshFeature is null) @@ -617,7 +629,7 @@ private async Task StartAsyncCore(CancellationToken cancellationToken) { if (newTtl is { } tokenLifetime && tokenLifetime > TimeSpan.Zero) { - ScheduleAuthenticationRefresh(tokenLifetime); + ScheduleAuthenticationRefresh(tokenLifetime, connectionState); } } @@ -643,11 +655,20 @@ private async Task StartAsyncCore(CancellationToken cancellationToken) /// For short-lived tokens (TTL < 2x RefreshBeforeExpiration), refreshes at half the TTL. /// internal void ScheduleAuthenticationRefresh(TimeSpan tokenLifetime) + { + if (_state.CurrentConnectionStateUnsynchronized is { } connectionState) + { + ScheduleAuthenticationRefresh(tokenLifetime, connectionState); + } + } + + private void ScheduleAuthenticationRefresh(TimeSpan tokenLifetime, ConnectionState connectionState) { var refreshBefore = _authenticationRefreshOptions.RefreshBeforeExpiration; TimeSpan refreshIn; - if (tokenLifetime <= new TimeSpan(refreshBefore.Ticks * 2)) + var timeUntilRefreshWindow = tokenLifetime - refreshBefore; + if (timeUntilRefreshWindow <= refreshBefore) { // Short-lived token: refresh at half the TTL to avoid spamming refreshIn = new TimeSpan(tokenLifetime.Ticks / 2); @@ -657,39 +678,69 @@ internal void ScheduleAuthenticationRefresh(TimeSpan tokenLifetime) refreshIn = tokenLifetime - refreshBefore; } - ScheduleAuthenticationRefreshAt(refreshIn); + ScheduleAuthenticationRefreshAt(refreshIn, connectionState); } /// /// Arms the one-shot authentication-refresh timer to fire after , replacing any - /// existing timer. A minimum interval is enforced to avoid spamming the server. + /// existing timer. /// internal void ScheduleAuthenticationRefreshAt(TimeSpan refreshIn) { - // Cancel any existing timer - _authRefreshTimer?.Dispose(); + if (_state.CurrentConnectionStateUnsynchronized is { } connectionState) + { + ScheduleAuthenticationRefreshAt(refreshIn, connectionState); + } + } - // Enforce a minimum interval to prevent spamming the server - var minimumRefreshInterval = TimeSpan.FromSeconds(30); - if (refreshIn < minimumRefreshInterval) + private void ScheduleAuthenticationRefreshAt(TimeSpan refreshIn, ConnectionState connectionState) + { + if (refreshIn > _maximumAuthenticationRefreshDelay) { - refreshIn = minimumRefreshInterval; + refreshIn = _maximumAuthenticationRefreshDelay; } - _authRefreshTimer = new Timer( - static state => _ = ((HubConnection)state!).OnAuthenticationRefreshTimerFired(), - this, - refreshIn, - Timeout.InfiniteTimeSpan); // One-shot timer - _lastAuthenticationRefreshDelay = refreshIn; + Timer? previousTimer; + lock (_authRefreshTimerLock) + { + if (!ReferenceEquals(_authRefreshConnectionState, connectionState)) + { + return; + } + + var timerState = new AuthenticationRefreshTimerState(this, connectionState); + var timer = new Timer( + static state => + { + var timerState = (AuthenticationRefreshTimerState)state!; + _ = timerState.HubConnection.OnAuthenticationRefreshTimerFired(timerState); + }, + timerState, + refreshIn, + Timeout.InfiniteTimeSpan); // One-shot timer + + previousTimer = _authRefreshTimer; + _authRefreshTimer = timer; + _authRefreshTimerState = timerState; + _lastAuthenticationRefreshDelay = refreshIn; + } + + previousTimer?.Dispose(); } - private async Task OnAuthenticationRefreshTimerFired() + private async Task OnAuthenticationRefreshTimerFired(AuthenticationRefreshTimerState timerState) { + if (!TryClaimAuthenticationRefreshTimer(timerState, out var timer)) + { + return; + } + + timer.Dispose(); + try { Log.AuthenticationRefreshStarting(_logger); - var newTtl = await RefreshAuthenticationAsync().ConfigureAwait(false); + var newTtl = await RefreshAuthenticationAsyncCore(timerState.ConnectionState, cancellationToken: default).ConfigureAwait(false); Log.AuthenticationRefreshCompleted(_logger, newTtl); } catch (Exception ex) @@ -698,6 +749,59 @@ private async Task OnAuthenticationRefreshTimerFired() } } + private bool TryClaimAuthenticationRefreshTimer(AuthenticationRefreshTimerState timerState, [NotNullWhen(true)] out Timer? timer) + { + lock (_authRefreshTimerLock) + { + if (!ReferenceEquals(_authRefreshConnectionState, timerState.ConnectionState) + || !ReferenceEquals(_authRefreshTimerState, timerState)) + { + timer = null; + return false; + } + + timer = _authRefreshTimer; + _authRefreshTimer = null; + _authRefreshTimerState = null; + return timer is not null; + } + } + + private void EnableAuthenticationRefreshTimer(ConnectionState connectionState) + { + Timer? timer; + lock (_authRefreshTimerLock) + { + timer = _authRefreshTimer; + _authRefreshTimer = null; + _authRefreshTimerState = null; + _authRefreshConnectionState = connectionState; + } + + timer?.Dispose(); + } + + private void DisableAuthenticationRefreshTimer() + { + Timer? timer; + lock (_authRefreshTimerLock) + { + _authRefreshConnectionState = null; + _authRefreshTimerState = null; + timer = _authRefreshTimer; + _authRefreshTimer = null; + } + + timer?.Dispose(); + } + + private sealed class AuthenticationRefreshTimerState(HubConnection hubConnection, ConnectionState connectionState) + { + public HubConnection HubConnection { get; } = hubConnection; + + public ConnectionState ConnectionState { get; } = connectionState; + } + private static ValueTask CloseAsync(ConnectionContext connection) { return connection.DisposeAsync(); @@ -708,9 +812,7 @@ private static ValueTask CloseAsync(ConnectionContext connection) // if we're disposing. private async Task StopAsyncCore(bool disposing) { - // Dispose the authentication refresh timer - _authRefreshTimer?.Dispose(); - _authRefreshTimer = null; + DisableAuthenticationRefreshTimer(); // StartAsync acquires the connection lock for the duration of the handshake. // ReconnectAsync also acquires the connection lock for reconnect attempts and handshakes. @@ -748,6 +850,7 @@ private async Task StopAsyncCore(bool disposing) } CheckDisposed(); + DisableAuthenticationRefreshTimer(); connectionState = _state.CurrentConnectionStateUnsynchronized; // Set the stopping flag so that any invocations after this get a useful error message instead of @@ -1906,6 +2009,8 @@ internal void OnServerTimeout() private async Task HandleConnectionClose(ConnectionState connectionState) { + DisableAuthenticationRefreshTimer(); + // Clear the connectionState field await _state.WaitConnectionLockAsync(token: default).ConfigureAwait(false); try diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.AuthenticationRefresh.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.AuthenticationRefresh.cs index 86f665c39b8d..3ee4f171ca05 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.AuthenticationRefresh.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.AuthenticationRefresh.cs @@ -75,6 +75,26 @@ public async Task RefreshAuthenticationAsyncThrowsWhenConnectionNotActive() } } + [Fact] + public async Task RefreshAuthenticationAsyncThrowsObjectDisposedExceptionWhenDisposed() + { + using (StartVerifiableLog()) + { + var feature = new FakeAuthenticationRefreshFeature { NextTtl = TimeSpan.FromSeconds(3600) }; + var connection = new TestConnection(); + connection.Features.Set(feature); + var hubConnection = BuildHubConnection(connection); + await hubConnection.StartAsync().DefaultTimeout(); + + await hubConnection.DisposeAsync().DefaultTimeout(); + await connection.DisposeAsync().DefaultTimeout(); + + await Assert.ThrowsAsync( + () => hubConnection.RefreshAuthenticationAsync()).DefaultTimeout(); + Assert.Equal(0, feature.RefreshCallCount); + } + } + [Fact] public async Task RefreshAuthenticationAsyncThrowsWhenFeatureMissing() { @@ -306,6 +326,55 @@ public async Task StartSchedulesTimerWhenInitialTtlProvided() } } + [Fact] + public async Task StartClampsLongLivedTokenRefreshToMaximumTimerDelay() + { + using (StartVerifiableLog()) + { + var feature = new FakeAuthenticationRefreshFeature { InitialTokenLifetime = TimeSpan.FromSeconds(int.MaxValue) }; + var connection = new TestConnection(); + connection.Features.Set(feature); + + var hubConnection = BuildHubConnection(connection); + try + { + await hubConnection.StartAsync().DefaultTimeout(); + + Assert.NotNull(GetAuthenticationRefreshTimer(hubConnection)); + Assert.Equal(TimeSpan.FromMilliseconds(int.MaxValue), GetLastAuthenticationRefreshDelay(hubConnection)); + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + await connection.DisposeAsync().DefaultTimeout(); + } + } + } + + [Fact] + public async Task StartSchedulesShortLivedTokenRefreshAtHalfLifetime() + { + using (StartVerifiableLog()) + { + var feature = new FakeAuthenticationRefreshFeature { InitialTokenLifetime = TimeSpan.FromSeconds(10) }; + var connection = new TestConnection(); + connection.Features.Set(feature); + + var hubConnection = BuildHubConnection(connection); + try + { + await hubConnection.StartAsync().DefaultTimeout(); + + Assert.Equal(TimeSpan.FromSeconds(5), GetLastAuthenticationRefreshDelay(hubConnection)); + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + await connection.DisposeAsync().DefaultTimeout(); + } + } + } + [Fact] public async Task StartDoesNotScheduleTimerWhenEnableAutoRefreshFalse() { @@ -529,6 +598,126 @@ public async Task RefreshReschedulesWhenServerReturnsTtl() } } + [Fact] + public async Task StartSchedulesHalfTokenLifetimeWhenRefreshBeforeExpirationIsVeryLarge() + { + using (StartVerifiableLog()) + { + var feature = new FakeAuthenticationRefreshFeature { InitialTokenLifetime = TimeSpan.FromHours(1) }; + var connection = new TestConnection(); + connection.Features.Set(feature); + + var hubConnection = BuildHubConnection(connection, builder => + { + builder.WithAuthenticationRefresh(o => o.RefreshBeforeExpiration = TimeSpan.MaxValue); + }); + try + { + await hubConnection.StartAsync().DefaultTimeout(); + + Assert.Equal(TimeSpan.FromMinutes(30), GetLastAuthenticationRefreshDelay(hubConnection)); + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + await connection.DisposeAsync().DefaultTimeout(); + } + } + } + + [Fact] + public async Task ManualRefreshCompletionAfterConnectionCloseDoesNotRearmTimer() + { + using (StartVerifiableLog()) + { + var feature = new BlockingAuthenticationRefreshFeature + { + InitialTokenLifetime = TimeSpan.FromHours(1), + }; + var connection = new TestConnection(); + connection.Features.Set(feature); + var closed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var hubConnection = BuildHubConnection(connection); + hubConnection.Closed += _ => + { + closed.TrySetResult(); + return Task.CompletedTask; + }; + + try + { + await hubConnection.StartAsync().DefaultTimeout(); + Assert.NotNull(GetAuthenticationRefreshTimer(hubConnection)); + + var refreshTask = hubConnection.RefreshAuthenticationAsync(); + await feature.RefreshStarted.Task.DefaultTimeout(); + + connection.CompleteFromTransport(); + await closed.Task.DefaultTimeout(); + Assert.Null(GetAuthenticationRefreshTimer(hubConnection)); + + feature.RefreshCompletion.SetResult(TimeSpan.FromHours(2)); + await refreshTask.DefaultTimeout(); + + Assert.Null(GetAuthenticationRefreshTimer(hubConnection)); + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + await connection.DisposeAsync().DefaultTimeout(); + } + } + } + + [Fact] + public async Task TimerRefreshCompletionAfterStopDoesNotRearmTimer() + { + using (StartVerifiableLog()) + { + var feature = new BlockingAuthenticationRefreshFeature + { + InitialTokenLifetime = TimeSpan.FromHours(1), + }; + var connection = new TestConnection(); + connection.Features.Set(feature); + var refreshCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var hubConnection = BuildHubConnection(connection, builder => + { + builder.WithAuthenticationRefresh(o => + { + o.OnAuthenticationRefreshed = _ => + { + refreshCompleted.TrySetResult(); + return Task.CompletedTask; + }; + }); + }); + try + { + await hubConnection.StartAsync().DefaultTimeout(); + var timer = GetAuthenticationRefreshTimer(hubConnection); + Assert.NotNull(timer); + Assert.True(timer.Change(TimeSpan.Zero, Timeout.InfiniteTimeSpan)); + await feature.RefreshStarted.Task.DefaultTimeout(); + + await hubConnection.StopAsync().DefaultTimeout(); + Assert.Null(GetAuthenticationRefreshTimer(hubConnection)); + + feature.RefreshCompletion.SetResult(TimeSpan.FromHours(2)); + await refreshCompleted.Task.DefaultTimeout(); + + Assert.Null(GetAuthenticationRefreshTimer(hubConnection)); + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + await connection.DisposeAsync().DefaultTimeout(); + } + } + } + [Fact] public async Task DisposeDisposesAuthenticationRefreshTimer() { @@ -598,5 +787,20 @@ private sealed class FakeAuthenticationRefreshFeature : IAuthenticationRefreshFe return Task.FromResult(NextTtl); } } + + private sealed class BlockingAuthenticationRefreshFeature : IAuthenticationRefreshFeature + { + public TimeSpan? InitialTokenLifetime { get; set; } + + public TaskCompletionSource RefreshStarted { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public TaskCompletionSource RefreshCompletion { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public Task RefreshAuthenticationAsync(CancellationToken cancellationToken = default) + { + RefreshStarted.TrySetResult(); + return RefreshCompletion.Task; + } + } } } diff --git a/src/SignalR/common/Http.Connections/src/HttpConnectionDispatcherOptions.cs b/src/SignalR/common/Http.Connections/src/HttpConnectionDispatcherOptions.cs index 1dd2c941f6a4..ecdabe3eb960 100644 --- a/src/SignalR/common/Http.Connections/src/HttpConnectionDispatcherOptions.cs +++ b/src/SignalR/common/Http.Connections/src/HttpConnectionDispatcherOptions.cs @@ -142,7 +142,7 @@ public TimeSpan TransportSendTimeout /// principal, or false to reject the refresh. When rejected, the endpoint responds with /// HTTP 403 and the connection's current user remains in place. /// - public Func>? OnAuthenticationRefresh { get; set; } + public Func>? OnAuthenticationRefresh { get; set; } internal bool TransportSendTimeoutEnabled => TransportSendTimeout != Timeout.InfiniteTimeSpan; diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs index 892c01f3aeed..8d6ecc10a846 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs @@ -35,6 +35,10 @@ internal sealed partial class HttpConnectionContext : ConnectionContext, IStatefulReconnectFeature #pragma warning restore CA2252 // This API requires opting into preview features { + // Prefer the standard identity-claim precedence used by DefaultClaimUidExtractor before + // falling back to exact principal content. + private static readonly string[] _userIdentityClaimTypes = ["sub", ClaimTypes.NameIdentifier, ClaimTypes.Upn]; + private readonly HttpConnectionDispatcherOptions _options; private readonly object _stateLock = new object(); @@ -55,6 +59,12 @@ internal sealed partial class HttpConnectionContext : ConnectionContext, // Guards User swaps in UpdateUser so concurrent /refresh requests (or long-polling polls racing // explicit /refresh requests) can't roll the connection back to an older identity. private readonly object _userLock = new object(); + // Refresh validation is synchronous, so temporarily expose the candidate request context to callbacks + // without publishing it as the connection's accepted HttpContext. + [ThreadStatic] + private static HttpConnectionContext? t_userRefreshHttpContextConnection; + [ThreadStatic] + private static HttpContext? t_userRefreshHttpContext; // True only for the WindowsIdentity user clone created by the first long-polling request. private bool _ownsUserIdentities; private readonly object _userRefreshCallbackLock = new object(); @@ -90,6 +100,7 @@ public HttpConnectionContext(string connectionId, string connectionToken, ILogge _logger = logger ?? NullLogger.Instance; MetricsContext = metricsContext; + OnUserRefreshing = IsUserRefreshAcceptedByDefault; // PERF: This type could just implement IFeatureCollection Features = new FeatureCollection(); @@ -212,7 +223,21 @@ public IDuplexPipe Application public TransferFormat ActiveFormat { get; set; } - public HttpContext? HttpContext { get; set; } + private HttpContext? _httpContext; + + public HttpContext? HttpContext + { + get + { + if (ReferenceEquals(t_userRefreshHttpContextConnection, this)) + { + return t_userRefreshHttpContext; + } + + return _httpContext; + } + set => _httpContext = value; + } public override CancellationToken ConnectionClosed { get; set; } @@ -253,6 +278,24 @@ public void TickHeartbeat() } } + public Func? OnUserRefreshing + { + get + { + lock (_userLock) + { + return field; + } + } + set + { + lock (_userLock) + { + field = value ?? IsUserRefreshAcceptedByDefault; + } + } + } + public IDisposable OnUserRefreshed(Action callback, object? state) { ArgumentNullException.ThrowIfNull(callback); @@ -280,13 +323,17 @@ private void RemoveUserRefreshedCallback(UserRefreshedCallbackRegistration regis /// /// The refreshed principal to apply to the connection. /// The expiration of the refreshed authentication. + /// The request context to expose while validating the refreshed principal. /// /// The update is skipped if is older than the currently /// applied . This makes the staleness check and the swap atomic /// so a caller racing a concurrent refresh that already applied a newer token can't roll the connection /// back to an older identity. /// - internal void UpdateUser(ClaimsPrincipal user, DateTimeOffset authenticationExpiration) + internal UserUpdateResult UpdateUser( + ClaimsPrincipal user, + DateTimeOffset authenticationExpiration, + HttpContext? userRefreshHttpContext = null) { ClaimsPrincipal? previouslyOwnedUser = null; @@ -299,7 +346,12 @@ internal void UpdateUser(ClaimsPrincipal user, DateTimeOffset authenticationExpi && AuthenticationExpiration != DateTimeOffset.MaxValue && authenticationExpiration < AuthenticationExpiration) { - return; + return UserUpdateResult.Stale; + } + + if (!IsUserRefreshAccepted(user, userRefreshHttpContext)) + { + return UserUpdateResult.Rejected; } if (_ownsUserIdentities && !ReferenceEquals(User, user)) @@ -348,6 +400,148 @@ internal void UpdateUser(ClaimsPrincipal user, DateTimeOffset authenticationExpi { DisposeOwnedIdentities(previouslyOwnedUser); } + + return UserUpdateResult.Updated; + } + + internal bool IsUserRefreshAccepted(ClaimsPrincipal user, HttpContext? userRefreshHttpContext = null) + { + lock (_userLock) + { + var previousConnection = t_userRefreshHttpContextConnection; + var previousHttpContext = t_userRefreshHttpContext; + if (userRefreshHttpContext is not null) + { + t_userRefreshHttpContextConnection = this; + t_userRefreshHttpContext = userRefreshHttpContext; + } + + try + { + return OnUserRefreshing!(user); + } + catch (Exception ex) + { + Log.UserRefreshingCallbackFailed(_logger, ex); + return false; + } + finally + { + t_userRefreshHttpContextConnection = previousConnection; + t_userRefreshHttpContext = previousHttpContext; + } + } + } + + internal bool IsUserAssociatedWithConnection(ClaimsPrincipal user) + { + lock (_userLock) + { + return IsUserRefreshAcceptedByDefault(user); + } + } + + private bool IsUserRefreshAcceptedByDefault(ClaimsPrincipal user) + { + var currentUser = User; + if (currentUser is null || ReferenceEquals(currentUser, user)) + { + return true; + } + + var currentIdentityKey = GetUserIdentityKey(currentUser); + var newIdentityKey = GetUserIdentityKey(user); + if (currentIdentityKey is not null || newIdentityKey is not null) + { + return currentIdentityKey == newIdentityKey; + } + + if (!HasAuthenticatedIdentity(currentUser) && !HasAuthenticatedIdentity(user)) + { + return true; + } + + // Without a stable identity key, require the authenticated principal content to remain unchanged. + return ClaimsPrincipalContentEquals(currentUser, user); + } + + internal static (string Type, string Value, string Issuer)? GetUserIdentityKey(ClaimsPrincipal user) + { + foreach (var claimType in _userIdentityClaimTypes) + { + var claim = user.FindFirst(claimType); + if (claim is not null && !string.IsNullOrEmpty(claim.Value)) + { + return (claim.Type, claim.Value, claim.Issuer); + } + } + + return null; + } + + internal static bool ClaimsPrincipalContentEquals(ClaimsPrincipal current, ClaimsPrincipal incoming) + { + return SequenceEqual(current.Identities, incoming.Identities, ClaimsIdentityContentEquals); + } + + private static bool ClaimsIdentityContentEquals(ClaimsIdentity current, ClaimsIdentity incoming) + { + if (!string.Equals(current.AuthenticationType, incoming.AuthenticationType, StringComparison.Ordinal) + || !string.Equals(current.NameClaimType, incoming.NameClaimType, StringComparison.Ordinal) + || !string.Equals(current.RoleClaimType, incoming.RoleClaimType, StringComparison.Ordinal) + || !string.Equals(current.Label, incoming.Label, StringComparison.Ordinal)) + { + return false; + } + + return SequenceEqual(current.Claims, incoming.Claims, ClaimContentEquals); + } + + private static bool ClaimContentEquals(Claim current, Claim incoming) + { + return string.Equals(current.Type, incoming.Type, StringComparison.Ordinal) + && string.Equals(current.Value, incoming.Value, StringComparison.Ordinal) + && string.Equals(current.ValueType, incoming.ValueType, StringComparison.Ordinal) + && string.Equals(current.Issuer, incoming.Issuer, StringComparison.Ordinal) + && string.Equals(current.OriginalIssuer, incoming.OriginalIssuer, StringComparison.Ordinal); + } + + private static bool SequenceEqual(IEnumerable current, IEnumerable incoming, Func equals) + { + using var currentEnumerator = current.GetEnumerator(); + using var incomingEnumerator = incoming.GetEnumerator(); + + while (true) + { + var currentHasValue = currentEnumerator.MoveNext(); + if (currentHasValue != incomingEnumerator.MoveNext()) + { + return false; + } + + if (!currentHasValue) + { + return true; + } + + if (!equals(currentEnumerator.Current, incomingEnumerator.Current)) + { + return false; + } + } + } + + private static bool HasAuthenticatedIdentity(ClaimsPrincipal user) + { + foreach (var identity in user.Identities) + { + if (identity.IsAuthenticated) + { + return true; + } + } + + return false; } internal void MarkUserOwned() @@ -910,6 +1104,13 @@ internal enum SetTransportState CannotChange, } + internal enum UserUpdateResult + { + Updated, + Stale, + Rejected, + } + private static partial class Log { [LoggerMessage(1, LogLevel.Trace, "Disposing connection {TransportConnectionId}.", EventName = "DisposingConnection")] @@ -941,5 +1142,8 @@ private static partial class Log [LoggerMessage(10, LogLevel.Error, "An IConnectionUserRefreshFeature.OnUserRefreshed callback threw an exception.", EventName = "UserRefreshedCallbackFailed")] public static partial void UserRefreshedCallbackFailed(ILogger logger, Exception exception); + + [LoggerMessage(11, LogLevel.Error, "The IConnectionUserRefreshFeature.OnUserRefreshing callback threw an exception. The user update was rejected.", EventName = "UserRefreshingCallbackFailed")] + public static partial void UserRefreshingCallbackFailed(ILogger logger, Exception exception); } } diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs index f7141372a7c8..50df2b602978 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs @@ -42,12 +42,6 @@ internal sealed partial class HttpConnectionDispatcher TransferFormats = new List { nameof(TransferFormat.Text), nameof(TransferFormat.Binary) } }; - // SignalR's default IUserIdProvider keys user identity off NameIdentifier, but an app can - // override it to use a different claim. This transport-layer dispatcher can't see IUserIdProvider, - // so mirror the standard identity-claim precedence antiforgery uses (DefaultClaimUidExtractor): - // sub, then NameIdentifier, then Upn. The first claim present on the principal identifies the user. - private static readonly string[] _userIdentityClaimTypes = ["sub", ClaimTypes.NameIdentifier, ClaimTypes.Upn]; - private readonly HttpConnectionManager _manager; private readonly ILoggerFactory _loggerFactory; private readonly HttpConnectionsMetrics _metrics; @@ -180,27 +174,29 @@ private async Task ProcessRefresh(HttpContext context, HttpConnectionDispatcherO return; } - // Use the AuthenticateResult the authorization middleware already produced for this request. - // The /refresh endpoint is stamped with the hub's authorization metadata, so the middleware - // authenticates it against the endpoint's declared schemes and exposes the result via - // IAuthenticateResultFeature — the same source negotiate and the connect/poll paths read. This keeps - // /refresh consistent with those paths and avoids re-authenticating against the app's default scheme, - // which throws or picks the wrong credential in multi-scheme apps. + // Use the principal and AuthenticateResult produced by authentication middleware. Endpoints with + // authorization metadata cannot reach this dispatcher unless authorization middleware has run. + // On endpoints that allow anonymous access, a missing result means no identity was established, so + // treat context.User as the candidate and let the connection's user refresh policy decide whether + // the transition is valid. var authResult = context.Features.Get()?.AuthenticateResult; - if (authResult is null || !authResult.Succeeded) + var newPrincipal = authResult?.Principal ?? context.User; + if (HasWindowsIdentity(connection.User) || HasWindowsIdentity(newPrincipal)) { - await WriteRefreshErrorAsync(context, StatusCodes.Status401Unauthorized, "invalid_token"); + await WriteRefreshErrorAsync(context, StatusCodes.Status400BadRequest, "windows_identity_not_supported"); return; } - var newPrincipal = authResult.Principal ?? context.User; - if (HasWindowsIdentity(connection.User) || HasWindowsIdentity(newPrincipal)) + if (!connection.IsUserRefreshAccepted(newPrincipal, context)) { - await WriteRefreshErrorAsync(context, StatusCodes.Status400BadRequest, "windows_identity_not_supported"); + LogUserChanged(connection.User, newPrincipal); + await WriteRefreshErrorAsync(context, StatusCodes.Status403Forbidden, "user_changed"); return; } - var newExpiration = GetAuthenticationExpiration(authResult, context.User); + var newExpiration = authResult is null + ? DateTimeOffset.MaxValue + : GetAuthenticationExpiration(authResult, context.User); if (options.OnAuthenticationRefresh is { } callback) { @@ -212,7 +208,14 @@ private async Task ProcessRefresh(HttpContext context, HttpConnectionDispatcherO } } - connection.UpdateUser(newPrincipal, newExpiration); + if (connection.UpdateUser(newPrincipal, newExpiration, context) == HttpConnectionContext.UserUpdateResult.Rejected) + { + // Revalidate atomically with the swap because the connection user or validation callback + // may have changed while the application callback was running. + LogUserChanged(connection.User, newPrincipal); + await WriteRefreshErrorAsync(context, StatusCodes.Status403Forbidden, "user_changed"); + return; + } // Compute TTL for the response int? tokenLifetimeSeconds = null; @@ -274,9 +277,9 @@ private static async Task WriteRefreshErrorAsync(HttpContext context, int status // Runs the application-provided OnAuthenticationRefresh callback for a re-authenticated principal. Shared by the // /refresh endpoint and the Long Polling poll path so both apply the same accept/reject policy. - private async ValueTask InvokeAuthenticationRefreshCallbackAsync( + private async Task InvokeAuthenticationRefreshCallbackAsync( HttpConnectionContext connection, HttpContext context, - Func> callback, ClaimsPrincipal newPrincipal, DateTimeOffset newExpiration) + Func> callback, ClaimsPrincipal newPrincipal, DateTimeOffset newExpiration) { var refreshContext = new AuthenticationRefreshContext { @@ -296,58 +299,6 @@ private async ValueTask InvokeAuthenticationRefreshCallbackAsync( return true; } - private static bool ClaimsPrincipalContentEquals(ClaimsPrincipal current, ClaimsPrincipal incoming) - { - return SequenceEqual(current.Identities, incoming.Identities, ClaimsIdentityContentEquals); - } - - private static bool ClaimsIdentityContentEquals(ClaimsIdentity current, ClaimsIdentity incoming) - { - if (!string.Equals(current.AuthenticationType, incoming.AuthenticationType, StringComparison.Ordinal) - || !string.Equals(current.NameClaimType, incoming.NameClaimType, StringComparison.Ordinal) - || !string.Equals(current.RoleClaimType, incoming.RoleClaimType, StringComparison.Ordinal) - || !string.Equals(current.Label, incoming.Label, StringComparison.Ordinal)) - { - return false; - } - - return SequenceEqual(current.Claims, incoming.Claims, ClaimContentEquals); - } - - private static bool ClaimContentEquals(Claim current, Claim incoming) - { - return string.Equals(current.Type, incoming.Type, StringComparison.Ordinal) - && string.Equals(current.Value, incoming.Value, StringComparison.Ordinal) - && string.Equals(current.ValueType, incoming.ValueType, StringComparison.Ordinal) - && string.Equals(current.Issuer, incoming.Issuer, StringComparison.Ordinal) - && string.Equals(current.OriginalIssuer, incoming.OriginalIssuer, StringComparison.Ordinal); - } - - private static bool SequenceEqual(IEnumerable current, IEnumerable incoming, Func equals) - { - using var currentEnumerator = current.GetEnumerator(); - using var incomingEnumerator = incoming.GetEnumerator(); - - while (true) - { - var currentHasValue = currentEnumerator.MoveNext(); - if (currentHasValue != incomingEnumerator.MoveNext()) - { - return false; - } - - if (!currentHasValue) - { - return true; - } - - if (!equals(currentEnumerator.Current, incomingEnumerator.Current)) - { - return false; - } - } - } - private async Task ExecuteAsync(HttpContext context, ConnectionDelegate connectionDelegate, HttpConnectionDispatcherOptions options, ConnectionLogScope logScope) { // set a tag to allow Application Performance Management tools to differentiate long running requests for reporting purposes @@ -712,7 +663,7 @@ private async Task ProcessSend(HttpContext context) return; } - if (await RejectIfUserChangedAsync(connection, context)) + if (await RejectIfConnectionUserChangedAsync(connection, context)) { return; } @@ -810,7 +761,7 @@ private async Task ProcessDeleteAsync(HttpContext context) return; } - if (await RejectIfUserChangedAsync(connection, context)) + if (await RejectIfConnectionUserChangedAsync(connection, context)) { return; } @@ -859,10 +810,13 @@ private async Task EnsureConnectionStateAsync(HttpConnectionContext connec // Only connections that reuse their state across requests (Stateful Reconnect or Long // Polling) re-enter here with an already-populated User; other transports handle a single // request per connection. This must run before any connection state is mutated below so a - // rejected request leaves the existing connection fully intact. When authentication refresh - // is enabled the application owns principal-change policy via OnAuthenticationRefresh, so the - // hardening reject only applies when that feature is not enabled. - if (!options.EnableAuthenticationRefresh && connection.ClientReconnectExpected() && await RejectIfUserChangedAsync(connection, context)) + // rejected request leaves the existing connection fully intact. A registered user-refresh + // validator can use an application-specific identity mapping; otherwise the connection applies + // its secure standard-identity or unchanged-principal fallback. + if (connection.ClientReconnectExpected() + && await (options.EnableAuthenticationRefresh + ? RejectIfUserChangedAsync(connection, context) + : RejectIfConnectionUserChangedAsync(connection, context))) { return false; } @@ -928,7 +882,7 @@ private async Task EnsureConnectionStateAsync(HttpConnectionContext connec // A refreshed token presents either different claims or a different expiration; a request that // carries the same token (same claims and expiration) is not treated as a refresh so we don't // run the callback or fire UserRefreshed on every poll/reconnect. - var principalChanged = !ClaimsPrincipalContentEquals(currentUser, newPrincipal) + var principalChanged = !HttpConnectionContext.ClaimsPrincipalContentEquals(currentUser, newPrincipal) || newExpiration != connection.AuthenticationExpiration; // A request that arrives carrying an older token than the one already applied (e.g. a poll that @@ -995,7 +949,13 @@ private async Task EnsureConnectionStateAsync(HttpConnectionContext connec // AuthenticationExpiration, so no separate swap or UpdateExpiration call is needed on this path. // If a concurrent /refresh applied a newer token between the stale check and here, UpdateUser skips // the rollback atomically with the swap. - connection.UpdateUser(newPrincipal, newExpiration); + if (connection.UpdateUser(newPrincipal, newExpiration, context) == HttpConnectionContext.UserUpdateResult.Rejected) + { + // Revalidate atomically with the swap because the connection user or validation callback + // may have changed while the application callback was running. + await WriteUserChangedResponseAsync(connection.User, newPrincipal, context); + return false; + } } else { @@ -1167,47 +1127,45 @@ private static void CloneHttpContext(HttpContext context, HttpConnectionContext connection.HttpContext = newHttpContext; } - // The connection is resolved purely by its connection token, so a different authenticated and - // endpoint-authorized user who obtained that token could otherwise act on a connection bound to - // another user. Reject the request (403) when the incoming user's identity claim differs from - // the one the connection is bound to, leaving the connection intact for the original user. - private async Task RejectIfUserChangedAsync(HttpConnectionContext connection, HttpContext context) + // Send and delete requests do not refresh the connection user, so always enforce the connection's + // standard identity association rather than the replaceable user-refresh policy. + private async Task RejectIfConnectionUserChangedAsync(HttpConnectionContext connection, HttpContext context) { - if (connection.User is null) + if (connection.IsUserAssociatedWithConnection(context.User)) { return false; } - var originalIdentity = GetUserIdentityKey(connection.User); - var newIdentity = GetUserIdentityKey(context.User); - if (originalIdentity == newIdentity) + await WriteUserChangedResponseAsync(connection.User, context.User, context); + return true; + } + + // Long Polling polls and stateful reconnects can refresh the connection user, so apply the + // replaceable user-refresh policy before mutating any connection state. + private async Task RejectIfUserChangedAsync(HttpConnectionContext connection, HttpContext context) + { + if (connection.IsUserRefreshAccepted(context.User, context)) { return false; } - Log.UserNameChangedRejected(_logger, originalIdentity?.Value, newIdentity?.Value); + await WriteUserChangedResponseAsync(connection.User, context.User, context); + return true; + } + + private async Task WriteUserChangedResponseAsync(ClaimsPrincipal? originalUser, ClaimsPrincipal newUser, HttpContext context) + { + LogUserChanged(originalUser, newUser); context.Response.ContentType = "text/plain"; context.Response.StatusCode = StatusCodes.Status403Forbidden; await context.Response.WriteAsync("The user associated with this connection changed."); - return true; } - // Returns a comparable identity key (claim type, value, issuer) for the first standard identity - // claim present on the principal, or null when none is present. Two principals with the same key - // (e.g. a refreshed token for the same user) are treated as the same user; a null key on both - // sides also compares equal, preserving the prior behavior for principals without these claims. - private static (string Type, string Value, string Issuer)? GetUserIdentityKey(ClaimsPrincipal user) + private void LogUserChanged(ClaimsPrincipal? originalUser, ClaimsPrincipal newUser) { - foreach (var claimType in _userIdentityClaimTypes) - { - var claim = user.FindFirst(claimType); - if (claim is not null && !string.IsNullOrEmpty(claim.Value)) - { - return (claim.Type, claim.Value, claim.Issuer); - } - } - - return null; + var originalIdentity = originalUser is null ? null : HttpConnectionContext.GetUserIdentityKey(originalUser); + var newIdentity = HttpConnectionContext.GetUserIdentityKey(newUser); + Log.UserNameChangedRejected(_logger, originalIdentity?.Value, newIdentity?.Value); } private async Task GetConnectionAsync(HttpContext context) diff --git a/src/SignalR/common/Http.Connections/src/PublicAPI.Unshipped.txt b/src/SignalR/common/Http.Connections/src/PublicAPI.Unshipped.txt index d26884e49864..856734e8e4a4 100644 --- a/src/SignalR/common/Http.Connections/src/PublicAPI.Unshipped.txt +++ b/src/SignalR/common/Http.Connections/src/PublicAPI.Unshipped.txt @@ -15,5 +15,5 @@ Microsoft.AspNetCore.Http.Connections.AuthenticationRefreshContext.NewUser.get - Microsoft.AspNetCore.Http.Connections.AuthenticationRefreshContext.NewUser.init -> void Microsoft.AspNetCore.Http.Connections.AuthenticationRefreshContext.PreviousUser.get -> System.Security.Claims.ClaimsPrincipal! Microsoft.AspNetCore.Http.Connections.AuthenticationRefreshContext.PreviousUser.init -> void -Microsoft.AspNetCore.Http.Connections.HttpConnectionDispatcherOptions.OnAuthenticationRefresh.get -> System.Func>? +Microsoft.AspNetCore.Http.Connections.HttpConnectionDispatcherOptions.OnAuthenticationRefresh.get -> System.Func!>? Microsoft.AspNetCore.Http.Connections.HttpConnectionDispatcherOptions.OnAuthenticationRefresh.set -> void diff --git a/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.AuthenticationRefresh.cs b/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.AuthenticationRefresh.cs index 6e2a1a7deae6..3ec29129222f 100644 --- a/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.AuthenticationRefresh.cs +++ b/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.AuthenticationRefresh.cs @@ -107,7 +107,7 @@ public async Task RefreshReturnsNotFoundWhenConnectionDoesNotExist() } [Fact] - public async Task RefreshReturnsUnauthorizedWhenAuthenticationFails() + public async Task RefreshUpdatesConnectionUserAndReturnsTokenLifetime() { using (StartVerifiableLog()) { @@ -116,6 +116,15 @@ public async Task RefreshReturnsUnauthorizedWhenAuthenticationFails() var options = new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true }; var connection = manager.CreateConnection(options, negotiateVersion: 1); + var originalUser = CreateAuthenticatedUserWithStableIdentity("old"); + connection.User = originalUser; + connection.AuthenticationExpiration = DateTimeOffset.UtcNow.AddMinutes(1); + + var newUser = CreateAuthenticatedUserWithStableIdentity("new"); + var newExpires = DateTimeOffset.UtcNow.AddMinutes(30); + var authProps = new AuthenticationProperties { ExpiresUtc = newExpires }; + var ticket = new AuthenticationTicket(newUser, authProps, "Test"); + var context = new DefaultHttpContext(); context.Request.Path = "/foo/refresh"; context.Request.Method = "POST"; @@ -125,19 +134,72 @@ public async Task RefreshReturnsUnauthorizedWhenAuthenticationFails() ["id"] = connection.ConnectionToken, }); - // The authorization middleware produced a failed authentication result for this request. - context.Features.Set(new TestAuthenticateResultFeature(AuthenticateResult.Fail("Bad token"))); + context.Features.Set(new TestAuthenticateResultFeature(AuthenticateResult.Success(ticket))); await dispatcher.ExecuteRefreshAsync(context, options); - Assert.Equal(StatusCodes.Status401Unauthorized, context.Response.StatusCode); + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + Assert.Equal("application/json", context.Response.ContentType); + var json = ReadJson(context.Response.Body); - AssertRefreshError(json, "invalid_token"); + var ttl = json.Value("tokenLifetimeSeconds"); + Assert.NotNull(ttl); + Assert.InRange(ttl.Value, 1, 1801); + + Assert.Same(newUser, connection.User); + // ExpiresUtc round-trips through AuthenticationProperties' whole-second ("r") format, so the + // stored expiration is truncated to the second. Allow headroom so the sub-second truncation + // can't push this over the tolerance. + Assert.Equal(newExpires, connection.AuthenticationExpiration, TimeSpan.FromSeconds(2)); } } [Fact] - public async Task RefreshUpdatesConnectionUserAndReturnsTokenLifetime() + public async Task RefreshRejectsDifferentStandardIdentityWhenNoCallbackIsConfigured() + { + using (StartVerifiableLog(write => write.EventId.Name == "UserNameChangedRejected")) + { + var manager = CreateConnectionManager(LoggerFactory); + var dispatcher = CreateDispatcher(manager, LoggerFactory); + var callbackInvoked = false; + var options = new HttpConnectionDispatcherOptions + { + EnableAuthenticationRefresh = true, + OnAuthenticationRefresh = _ => + { + callbackInvoked = true; + return Task.FromResult(true); + }, + }; + var connection = manager.CreateConnection(options, negotiateVersion: 1); + var originalUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-a") }, "Test")); + connection.User = originalUser; + + var newUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-b") }, "Test")); + var ticket = new AuthenticationTicket(newUser, new AuthenticationProperties(), "Test"); + var context = new DefaultHttpContext(); + context.Request.Path = "/foo/refresh"; + context.Request.Method = "POST"; + context.Response.Body = new MemoryStream(); + context.Request.Query = new QueryCollection(new Dictionary + { + ["id"] = connection.ConnectionToken, + }); + context.Features.Set(new TestAuthenticateResultFeature(AuthenticateResult.Success(ticket))); + + await dispatcher.ExecuteRefreshAsync(context, options); + + Assert.Equal(StatusCodes.Status403Forbidden, context.Response.StatusCode); + AssertRefreshError(ReadJson(context.Response.Body), "user_changed"); + Assert.False(callbackInvoked); + Assert.Same(originalUser, connection.User); + } + } + + [Fact] + public async Task RefreshCallbackCanAllowStandardIdentityChange() { using (StartVerifiableLog()) { @@ -145,16 +207,25 @@ public async Task RefreshUpdatesConnectionUserAndReturnsTokenLifetime() var dispatcher = CreateDispatcher(manager, LoggerFactory); var options = new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true }; var connection = manager.CreateConnection(options, negotiateVersion: 1); + connection.User = new ClaimsPrincipal(new ClaimsIdentity( + new[] + { + new Claim(ClaimTypes.NameIdentifier, "user-a"), + new Claim("stable-id", "stable-user"), + }, "Test")); - var originalUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "old") }, "Test")); - connection.User = originalUser; - connection.AuthenticationExpiration = DateTimeOffset.UtcNow.AddMinutes(1); - - var newUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "new") }, "Test")); - var newExpires = DateTimeOffset.UtcNow.AddMinutes(30); - var authProps = new AuthenticationProperties { ExpiresUtc = newExpires }; - var ticket = new AuthenticationTicket(newUser, authProps, "Test"); + var feature = connection.Features.Get(); + Assert.NotNull(feature); + feature.OnUserRefreshing = + static user => user.FindFirst("stable-id")?.Value == "stable-user"; + var newUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] + { + new Claim(ClaimTypes.NameIdentifier, "user-b"), + new Claim("stable-id", "stable-user"), + }, "Test")); + var ticket = new AuthenticationTicket(newUser, new AuthenticationProperties(), "Test"); var context = new DefaultHttpContext(); context.Request.Path = "/foo/refresh"; context.Request.Method = "POST"; @@ -163,24 +234,12 @@ public async Task RefreshUpdatesConnectionUserAndReturnsTokenLifetime() { ["id"] = connection.ConnectionToken, }); - context.Features.Set(new TestAuthenticateResultFeature(AuthenticateResult.Success(ticket))); await dispatcher.ExecuteRefreshAsync(context, options); Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); - Assert.Equal("application/json", context.Response.ContentType); - - var json = ReadJson(context.Response.Body); - var ttl = json.Value("tokenLifetimeSeconds"); - Assert.NotNull(ttl); - Assert.InRange(ttl.Value, 1, 1801); - Assert.Same(newUser, connection.User); - // ExpiresUtc round-trips through AuthenticationProperties' whole-second ("r") format, so the - // stored expiration is truncated to the second. Allow headroom so the sub-second truncation - // can't push this over the tolerance. - Assert.Equal(newExpires, connection.AuthenticationExpiration, TimeSpan.FromSeconds(2)); } } @@ -194,10 +253,10 @@ public async Task RefreshClampsTokenLifetimeToMaxInt() var options = new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true }; var connection = manager.CreateConnection(options, negotiateVersion: 1); - connection.User = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "old") }, "Test")); + connection.User = CreateAuthenticatedUserWithStableIdentity("old"); connection.AuthenticationExpiration = DateTimeOffset.UtcNow.AddMinutes(1); - var newUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "new") }, "Test")); + var newUser = CreateAuthenticatedUserWithStableIdentity("new"); var authProps = new AuthenticationProperties { ExpiresUtc = DateTimeOffset.UtcNow.AddYears(100) }; var ticket = new AuthenticationTicket(newUser, authProps, "Test"); @@ -304,10 +363,10 @@ public async Task RefreshUsesAuthenticateResultFeatureWithoutReauthenticating() var options = new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true }; var connection = manager.CreateConnection(options, negotiateVersion: 1); - connection.User = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "old") }, "Test")); + connection.User = CreateAuthenticatedUserWithStableIdentity("old"); connection.AuthenticationExpiration = DateTimeOffset.UtcNow.AddMinutes(1); - var newUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "new") }, "Test")); + var newUser = CreateAuthenticatedUserWithStableIdentity("new"); var newExpires = DateTimeOffset.UtcNow.AddMinutes(30); var services = new ServiceCollection(); @@ -355,11 +414,11 @@ public async Task RefreshDoesNotRollBackNewerPrincipalWhenOlderRefreshCompletesL var dispatcher = CreateDispatcher(manager, LoggerFactory); var connection = manager.CreateConnection(new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true }, negotiateVersion: 1); - var originalUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "original") }, "Test")); + var originalUser = CreateAuthenticatedUserWithStableIdentity("original"); connection.User = originalUser; connection.AuthenticationExpiration = DateTimeOffset.UtcNow.AddMinutes(1); - var newerUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "newer") }, "Test")); + var newerUser = CreateAuthenticatedUserWithStableIdentity("newer"); var newerExpires = DateTimeOffset.UtcNow.AddMinutes(30); var options = new HttpConnectionDispatcherOptions @@ -368,11 +427,11 @@ public async Task RefreshDoesNotRollBackNewerPrincipalWhenOlderRefreshCompletesL OnAuthenticationRefresh = _ => { connection.UpdateUser(newerUser, newerExpires); - return ValueTask.FromResult(true); + return Task.FromResult(true); }, }; - var olderUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "older") }, "Test")); + var olderUser = CreateAuthenticatedUserWithStableIdentity("older"); var olderExpires = DateTimeOffset.UtcNow.AddMinutes(5); var ticket = new AuthenticationTicket(olderUser, new AuthenticationProperties { ExpiresUtc = olderExpires }, "Test"); @@ -395,7 +454,7 @@ public async Task RefreshDoesNotRollBackNewerPrincipalWhenOlderRefreshCompletesL } [Fact] - public async Task RefreshReturnsUnauthorizedWhenAuthenticateResultFeatureMissing() + public async Task RefreshRejectsAnonymousPrincipalByDefaultWhenAuthenticateResultFeatureMissing() { using (StartVerifiableLog()) { @@ -403,6 +462,9 @@ public async Task RefreshReturnsUnauthorizedWhenAuthenticateResultFeatureMissing var dispatcher = CreateDispatcher(manager, LoggerFactory); var options = new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true }; var connection = manager.CreateConnection(options, negotiateVersion: 1); + var originalUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-a") }, "Test")); + connection.User = originalUser; var context = new DefaultHttpContext(); context.Request.Path = "/foo/refresh"; @@ -413,14 +475,146 @@ public async Task RefreshReturnsUnauthorizedWhenAuthenticateResultFeatureMissing ["id"] = connection.ConnectionToken, }); - // No IAuthenticateResultFeature is present (no authorization middleware ran for the endpoint). - // /refresh relies on the middleware-produced result like the other endpoints and does not - // re-authenticate, so it cannot refresh and returns 401. await dispatcher.ExecuteRefreshAsync(context, options); - Assert.Equal(StatusCodes.Status401Unauthorized, context.Response.StatusCode); + Assert.Equal(StatusCodes.Status403Forbidden, context.Response.StatusCode); var json = ReadJson(context.Response.Body); - AssertRefreshError(json, "invalid_token"); + AssertRefreshError(json, "user_changed"); + Assert.Same(originalUser, connection.User); + } + } + + [Fact] + public async Task RefreshRejectsAuthenticatedPrincipalWithoutStableIdentityByDefault() + { + using (StartVerifiableLog()) + { + var manager = CreateConnectionManager(LoggerFactory); + var dispatcher = CreateDispatcher(manager, LoggerFactory); + var options = new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true }; + var connection = manager.CreateConnection(options, negotiateVersion: 1); + var originalUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim("employeeId", "1") }, "Test")); + connection.User = originalUser; + + var refreshedUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim("employeeId", "2") }, "Test")); + var ticket = new AuthenticationTicket(refreshedUser, "Test"); + var context = new DefaultHttpContext(); + context.Request.Path = "/foo/refresh"; + context.Request.Method = "POST"; + context.Response.Body = new MemoryStream(); + context.Request.Query = new QueryCollection(new Dictionary + { + ["id"] = connection.ConnectionToken, + }); + context.Features.Set( + new TestAuthenticateResultFeature(AuthenticateResult.Success(ticket))); + + await dispatcher.ExecuteRefreshAsync(context, options); + + Assert.Equal(StatusCodes.Status403Forbidden, context.Response.StatusCode); + var json = ReadJson(context.Response.Body); + AssertRefreshError(json, "user_changed"); + Assert.Same(originalUser, connection.User); + } + } + + [Fact] + public async Task RefreshAllowsEquivalentAuthenticatedPrincipalWithoutStableIdentityByDefault() + { + using (StartVerifiableLog()) + { + var manager = CreateConnectionManager(LoggerFactory); + var dispatcher = CreateDispatcher(manager, LoggerFactory); + var options = new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true }; + var connection = manager.CreateConnection(options, negotiateVersion: 1); + connection.User = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim("employeeId", "1") }, "Test")); + + var refreshedUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim("employeeId", "1") }, "Test")); + var ticket = new AuthenticationTicket(refreshedUser, "Test"); + var context = new DefaultHttpContext(); + context.Request.Path = "/foo/refresh"; + context.Request.Method = "POST"; + context.Response.Body = new MemoryStream(); + context.Request.Query = new QueryCollection(new Dictionary + { + ["id"] = connection.ConnectionToken, + }); + context.Features.Set( + new TestAuthenticateResultFeature(AuthenticateResult.Success(ticket))); + + await dispatcher.ExecuteRefreshAsync(context, options); + + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + Assert.Same(refreshedUser, connection.User); + } + } + + [Fact] + public async Task RefreshAllowsDistinctAnonymousPrincipalsByDefault() + { + using (StartVerifiableLog()) + { + var manager = CreateConnectionManager(LoggerFactory); + var dispatcher = CreateDispatcher(manager, LoggerFactory); + var options = new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true }; + var connection = manager.CreateConnection(options, negotiateVersion: 1); + connection.User = new ClaimsPrincipal(new ClaimsIdentity()); + + var context = new DefaultHttpContext(); + context.Request.Path = "/foo/refresh"; + context.Request.Method = "POST"; + context.Response.Body = new MemoryStream(); + context.Request.Query = new QueryCollection(new Dictionary + { + ["id"] = connection.ConnectionToken, + }); + + await dispatcher.ExecuteRefreshAsync(context, options); + + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + Assert.Same(context.User, connection.User); + Assert.False(connection.User.Identity?.IsAuthenticated ?? false); + } + } + + [Fact] + public async Task RefreshAllowsAnonymousPrincipalWhenAuthenticationProducesNoResultAndPolicyAccepts() + { + using (StartVerifiableLog()) + { + var manager = CreateConnectionManager(LoggerFactory); + var dispatcher = CreateDispatcher(manager, LoggerFactory); + var options = new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true }; + var connection = manager.CreateConnection(options, negotiateVersion: 1); + connection.User = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-a") }, "Test")); + connection.AuthenticationExpiration = DateTimeOffset.UtcNow.AddMinutes(5); + + var userRefreshFeature = connection.Features.Get(); + Assert.NotNull(userRefreshFeature); + userRefreshFeature.OnUserRefreshing = static _ => true; + + var context = new DefaultHttpContext(); + context.Request.Path = "/foo/refresh"; + context.Request.Method = "POST"; + context.Response.Body = new MemoryStream(); + context.Request.Query = new QueryCollection(new Dictionary + { + ["id"] = connection.ConnectionToken, + }); + context.Features.Set( + new TestAuthenticateResultFeature(AuthenticateResult.NoResult())); + + await dispatcher.ExecuteRefreshAsync(context, options); + + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + Assert.Same(context.User, connection.User); + Assert.False(connection.User.Identity?.IsAuthenticated ?? false); + Assert.Equal(DateTimeOffset.MaxValue, connection.AuthenticationExpiration); } } @@ -645,10 +839,18 @@ public void UpdateUserInvokesUserRefreshedCallbackWithNewPrincipal() var manager = CreateConnectionManager(LoggerFactory); var connection = manager.CreateConnection(new HttpConnectionDispatcherOptions(), negotiateVersion: 1); - var originalUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("n", "old") }, "Test")); + var originalUser = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim(ClaimTypes.NameIdentifier, "user"), + new Claim("n", "old"), + ], "Test")); connection.User = originalUser; - var newUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("n", "new") }, "Test")); + var newUser = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim(ClaimTypes.NameIdentifier, "user"), + new Claim("n", "new"), + ], "Test")); ClaimsPrincipal capturedCurrent = null; var feature = connection.Features.Get(); Assert.NotNull(feature); @@ -657,12 +859,143 @@ public void UpdateUserInvokesUserRefreshedCallbackWithNewPrincipal() capturedCurrent = current; }, state: null); - connection.UpdateUser(newUser, DateTimeOffset.UtcNow.AddMinutes(15)); + var expiration = DateTimeOffset.UtcNow.AddMinutes(15); + connection.UpdateUser(newUser, expiration); Assert.Same(newUser, capturedCurrent); } } + [Fact] + public void UpdateUserInvokesUserRefreshingCallbackBeforePrincipalSwap() + { + using (StartVerifiableLog()) + { + var manager = CreateConnectionManager(LoggerFactory); + var connection = manager.CreateConnection(new HttpConnectionDispatcherOptions(), negotiateVersion: 1); + var originalUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-a") }, "Test")); + connection.User = originalUser; + + var newUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-b") }, "Test")); + ClaimsPrincipal observedCurrentUser = null; + ClaimsPrincipal observedCandidateUser = null; + var feature = connection.Features.Get(); + Assert.NotNull(feature); + feature.OnUserRefreshing = candidate => + { + observedCurrentUser = connection.User; + observedCandidateUser = candidate; + return true; + }; + + var result = connection.UpdateUser(newUser, DateTimeOffset.UtcNow.AddMinutes(15)); + + Assert.Equal(HttpConnectionContext.UserUpdateResult.Updated, result); + Assert.Same(originalUser, observedCurrentUser); + Assert.Same(newUser, observedCandidateUser); + Assert.Same(newUser, connection.User); + } + } + + [Fact] + public void UpdateUserRejectsDifferentStandardIdentityWhenNoCallbackIsConfigured() + { + using (StartVerifiableLog()) + { + var manager = CreateConnectionManager(LoggerFactory); + var connection = manager.CreateConnection(new HttpConnectionDispatcherOptions(), negotiateVersion: 1); + var originalUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-a") }, "Test")); + connection.User = originalUser; + var originalExpiration = connection.AuthenticationExpiration; + + var newUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-b") }, "Test")); + var result = connection.UpdateUser(newUser, DateTimeOffset.UtcNow.AddMinutes(15)); + + Assert.Equal(HttpConnectionContext.UserUpdateResult.Rejected, result); + Assert.Same(originalUser, connection.User); + Assert.Equal(originalExpiration, connection.AuthenticationExpiration); + } + } + + [Fact] + public void SettingUserRefreshingToNullRestoresDefaultIdentityValidation() + { + using (StartVerifiableLog()) + { + var manager = CreateConnectionManager(LoggerFactory); + var connection = manager.CreateConnection(new HttpConnectionDispatcherOptions(), negotiateVersion: 1); + var originalUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-a") }, "Test")); + connection.User = originalUser; + + var feature = connection.Features.Get(); + Assert.NotNull(feature); + feature.OnUserRefreshing = static _ => true; + feature.OnUserRefreshing = null; + + var newUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-b") }, "Test")); + var result = connection.UpdateUser(newUser, DateTimeOffset.UtcNow.AddMinutes(15)); + + Assert.Equal(HttpConnectionContext.UserUpdateResult.Rejected, result); + Assert.Same(originalUser, connection.User); + } + } + + [Fact] + public void SettingUserRefreshingReplacesPreviouslyConfiguredCallback() + { + using (StartVerifiableLog()) + { + var manager = CreateConnectionManager(LoggerFactory); + var connection = manager.CreateConnection(new HttpConnectionDispatcherOptions(), negotiateVersion: 1); + var originalUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-a") }, "Test")); + connection.User = originalUser; + + var feature = connection.Features.Get(); + Assert.NotNull(feature); + feature.OnUserRefreshing = static _ => false; + feature.OnUserRefreshing = static _ => true; + + var newUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-b") }, "Test")); + var result = connection.UpdateUser(newUser, DateTimeOffset.UtcNow.AddMinutes(15)); + + Assert.Equal(HttpConnectionContext.UserUpdateResult.Updated, result); + Assert.Same(newUser, connection.User); + } + } + + [Fact] + public void UpdateUserRejectsExceptionFromUserRefreshingCallback() + { + using (StartVerifiableLog(write => write.EventId.Name == "UserRefreshingCallbackFailed")) + { + var manager = CreateConnectionManager(LoggerFactory); + var connection = manager.CreateConnection(new HttpConnectionDispatcherOptions(), negotiateVersion: 1); + var originalUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-a") }, "Test")); + connection.User = originalUser; + + var feature = connection.Features.Get(); + Assert.NotNull(feature); + feature.OnUserRefreshing = + static _ => throw new InvalidOperationException("boom"); + + var newUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-a") }, "Test")); + var result = connection.UpdateUser(newUser, DateTimeOffset.UtcNow.AddMinutes(15)); + + Assert.Equal(HttpConnectionContext.UserUpdateResult.Rejected, result); + Assert.Same(originalUser, connection.User); + } + } + [Fact] public void DisposedUserRefreshedRegistrationIsNotInvoked() { @@ -722,15 +1055,15 @@ public async Task RefreshInvokesOnAuthenticationRefreshCallbackAndAcceptsWhenTru OnAuthenticationRefresh = ctx => { captured = ctx; - return ValueTask.FromResult(true); + return Task.FromResult(true); }, }; var connection = manager.CreateConnection(options, negotiateVersion: 1); - var originalUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "old") }, "Test")); + var originalUser = CreateAuthenticatedUserWithStableIdentity("old"); connection.User = originalUser; - var newUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "new") }, "Test")); + var newUser = CreateAuthenticatedUserWithStableIdentity("new"); var newExpires = DateTimeOffset.UtcNow.AddMinutes(30); var ticket = new AuthenticationTicket(newUser, new AuthenticationProperties { ExpiresUtc = newExpires }, "Test"); @@ -772,17 +1105,17 @@ public async Task RefreshReturnsForbiddenWhenOnAuthenticationRefreshReturnsFalse EnableAuthenticationRefresh = true, OnAuthenticationRefresh = ctx => { - return ValueTask.FromResult(false); + return Task.FromResult(false); }, }; var connection = manager.CreateConnection(options, negotiateVersion: 1); - var originalUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "old") }, "Test")); + var originalUser = CreateAuthenticatedUserWithStableIdentity("old"); connection.User = originalUser; var originalExpiration = DateTimeOffset.UtcNow.AddMinutes(2); connection.AuthenticationExpiration = originalExpiration; - var newUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "new") }, "Test")); + var newUser = CreateAuthenticatedUserWithStableIdentity("new"); var ticket = new AuthenticationTicket(newUser, new AuthenticationProperties { ExpiresUtc = DateTimeOffset.UtcNow.AddMinutes(30) }, "Test"); var context = new DefaultHttpContext(); @@ -819,12 +1152,12 @@ public async Task RefreshDenyReturnsGenericError() var options = new HttpConnectionDispatcherOptions { EnableAuthenticationRefresh = true, - OnAuthenticationRefresh = _ => ValueTask.FromResult(false), + OnAuthenticationRefresh = _ => Task.FromResult(false), }; var connection = manager.CreateConnection(options, negotiateVersion: 1); - connection.User = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "old") }, "Test")); + connection.User = CreateAuthenticatedUserWithStableIdentity("old"); - var newUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "new") }, "Test")); + var newUser = CreateAuthenticatedUserWithStableIdentity("new"); var ticket = new AuthenticationTicket(newUser, new AuthenticationProperties(), "Test"); var context = new DefaultHttpContext(); @@ -860,10 +1193,10 @@ public async Task RefreshOnAuthenticationRefreshCallbackExceptionPropagates() OnAuthenticationRefresh = _ => throw new InvalidOperationException("boom"), }; var connection = manager.CreateConnection(options, negotiateVersion: 1); - var originalUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "old") }, "Test")); + var originalUser = CreateAuthenticatedUserWithStableIdentity("old"); connection.User = originalUser; - var newUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "new") }, "Test")); + var newUser = CreateAuthenticatedUserWithStableIdentity("new"); var ticket = new AuthenticationTicket(newUser, new AuthenticationProperties(), "Test"); var context = new DefaultHttpContext(); @@ -933,22 +1266,30 @@ public async Task LongPollingRefreshedPrincipalInvokesOnAuthenticationRefreshAnd OnAuthenticationRefresh = ctx => { captured = ctx; - return ValueTask.FromResult(true); + return Task.FromResult(true); }, }; var app = BuildTestConnectionHandlerApp(out var sp); // First poll establishes the original principal on the connection. - var userA = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim(ClaimTypes.NameIdentifier, "userA") }, "Test")); + var userA = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, "userA"), + new Claim(ClaimTypes.Name, "old"), + }, "Test")); var expA = DateTimeOffset.UtcNow.AddMinutes(5); var context1 = BuildAuthPollContext(connection, sp, userA, expA); await dispatcher.ExecuteAsync(context1, options, app).DefaultTimeout(); Assert.Equal(StatusCodes.Status200OK, context1.Response.StatusCode); Assert.Same(userA, connection.User); - // Second poll carries a refreshed token (different subject and later expiration). - var userB = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim(ClaimTypes.NameIdentifier, "userB") }, "Test")); + // Second poll carries a refreshed token for the same user with changed claims and a later expiration. + var userB = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, "userA"), + new Claim(ClaimTypes.Name, "new"), + }, "Test")); var expB = DateTimeOffset.UtcNow.AddMinutes(30); var context2 = BuildAuthPollContext(connection, sp, userB, expB); var pollTask = dispatcher.ExecuteAsync(context2, options, app); @@ -967,6 +1308,46 @@ public async Task LongPollingRefreshedPrincipalInvokesOnAuthenticationRefreshAnd } } + [Fact] + public async Task LongPollingDifferentStandardIdentityIsRejectedBeforeAuthenticationRefresh() + { + using (StartVerifiableLog(write => write.EventId.Name == "UserNameChangedRejected")) + { + var manager = CreateConnectionManager(LoggerFactory); + var connection = manager.CreateConnection(); + connection.TransportType = HttpTransportType.LongPolling; + var dispatcher = CreateDispatcher(manager, LoggerFactory); + var callbackInvoked = false; + var options = new HttpConnectionDispatcherOptions + { + EnableAuthenticationRefresh = true, + OnAuthenticationRefresh = _ => + { + callbackInvoked = true; + return Task.FromResult(true); + }, + }; + var app = BuildTestConnectionHandlerApp(out var sp); + + var originalUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-a") }, "Test")); + var context1 = BuildAuthPollContext(connection, sp, originalUser, DateTimeOffset.UtcNow.AddMinutes(5)); + await dispatcher.ExecuteAsync(context1, options, app).DefaultTimeout(); + Assert.Equal(StatusCodes.Status200OK, context1.Response.StatusCode); + + var differentUser = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-b") }, "Test")); + var context2 = BuildAuthPollContext(connection, sp, differentUser, DateTimeOffset.UtcNow.AddMinutes(30)); + await dispatcher.ExecuteAsync(context2, options, app).DefaultTimeout(); + + Assert.Equal(StatusCodes.Status403Forbidden, context2.Response.StatusCode); + Assert.False(callbackInvoked); + Assert.Same(originalUser, connection.User); + Assert.Null(connection.DisposeAndRemoveTask); + Assert.True(manager.TryGetConnection(connection.ConnectionToken, out _)); + } + } + [Fact] public async Task LongPollingRefreshedPrincipalRejectedByOnAuthenticationRefreshTearsDownConnection() { @@ -982,13 +1363,17 @@ public async Task LongPollingRefreshedPrincipalRejectedByOnAuthenticationRefresh EnableAuthenticationRefresh = true, OnAuthenticationRefresh = ctx => { - return ValueTask.FromResult(false); + return Task.FromResult(false); }, }; var app = BuildTestConnectionHandlerApp(out var sp); - var userA = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim(ClaimTypes.NameIdentifier, "userA") }, "Test")); + var userA = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, "userA"), + new Claim(ClaimTypes.Name, "old"), + }, "Test")); var expA = DateTimeOffset.UtcNow.AddMinutes(5); var context1 = BuildAuthPollContext(connection, sp, userA, expA); await dispatcher.ExecuteAsync(context1, options, app).DefaultTimeout(); @@ -996,7 +1381,11 @@ public async Task LongPollingRefreshedPrincipalRejectedByOnAuthenticationRefresh Assert.Same(userA, connection.User); // Second poll carries a refreshed token the application rejects. - var userB = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim(ClaimTypes.NameIdentifier, "userB") }, "Test")); + var userB = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, "userA"), + new Claim(ClaimTypes.Name, "new"), + }, "Test")); var expB = DateTimeOffset.UtcNow.AddMinutes(30); var context2 = BuildAuthPollContext(connection, sp, userB, expB); await dispatcher.ExecuteAsync(context2, options, app).DefaultTimeout(); @@ -1032,7 +1421,7 @@ public async Task LongPollingPollCarryingSameTokenDoesNotInvokeOnAuthenticationR OnAuthenticationRefresh = _ => { refreshCount++; - return ValueTask.FromResult(true); + return Task.FromResult(true); }, }; @@ -1080,7 +1469,7 @@ public async Task LongPollingPollWithChangedClaimsAndSameExpirationInvokesOnAuth { refreshCount++; captured = ctx; - return ValueTask.FromResult(true); + return Task.FromResult(true); }, }; @@ -1118,13 +1507,14 @@ public async Task LongPollingPollWithChangedClaimsAndSameExpirationInvokesOnAuth } [Fact] - public async Task LongPollingChangedUserRejectedWhenAuthenticationRefreshDisabled() + public async Task LongPollingChangedUserRejectedWhenAuthenticationRefreshDisabledEvenWhenRefreshPolicyAllows() { using (StartVerifiableLog()) { var manager = CreateConnectionManager(LoggerFactory); var connection = manager.CreateConnection(); connection.TransportType = HttpTransportType.LongPolling; + connection.Features.Get().OnUserRefreshing = _ => true; var dispatcher = CreateDispatcher(manager, LoggerFactory); var refreshCount = 0; @@ -1134,7 +1524,7 @@ public async Task LongPollingChangedUserRejectedWhenAuthenticationRefreshDisable OnAuthenticationRefresh = _ => { refreshCount++; - return ValueTask.FromResult(true); + return Task.FromResult(true); }, }; @@ -1178,14 +1568,18 @@ public async Task LongPollingStalePollDoesNotRollBackPrincipalOrExpiration() OnAuthenticationRefresh = _ => { refreshCount++; - return ValueTask.FromResult(true); + return Task.FromResult(true); }, }; var app = BuildTestConnectionHandlerApp(out var sp); // First poll establishes the principal with a later expiration (mimics a token applied by /refresh). - var userA = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim(ClaimTypes.NameIdentifier, "userA") }, "Test")); + var userA = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, "userA"), + new Claim(ClaimTypes.Name, "newer"), + }, "Test")); var laterExpiration = DateTimeOffset.UtcNow.AddMinutes(30); var context1 = BuildAuthPollContext(connection, sp, userA, laterExpiration); await dispatcher.ExecuteAsync(context1, options, app).DefaultTimeout(); @@ -1195,7 +1589,11 @@ public async Task LongPollingStalePollDoesNotRollBackPrincipalOrExpiration() // A delayed poll arrives carrying an OLDER token (earlier expiration) than the one already applied // (e.g. it lost the race against an explicit /refresh). It must not roll the connection back. - var userB = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim(ClaimTypes.NameIdentifier, "userB") }, "Test")); + var userB = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, "userA"), + new Claim(ClaimTypes.Name, "older"), + }, "Test")); var earlierExpiration = DateTimeOffset.UtcNow.AddMinutes(5); var context2 = BuildAuthPollContext(connection, sp, userB, earlierExpiration); var pollTask = dispatcher.ExecuteAsync(context2, options, app); @@ -1238,16 +1636,24 @@ public async Task LongPollingRejectionDoesNotTearDownConnectionWhenTokenSupersed // A delayed poll carrying an older token runs the callback, but while the (slow) callback runs an // explicit /refresh applies a NEWER token. The callback then rejects the older token. The // connection must not be torn down for a token it has already moved past. - var newerUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim(ClaimTypes.NameIdentifier, "userC") }, "Test")); + var newerUser = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, "userA"), + new Claim(ClaimTypes.Name, "newest"), + }, "Test")); var newerExpiration = DateTimeOffset.UtcNow.AddMinutes(30); options.OnAuthenticationRefresh = ctx => { // Simulate a concurrent /refresh landing a newer token mid-callback. connection.UpdateUser(newerUser, newerExpiration); - return ValueTask.FromResult(false); + return Task.FromResult(false); }; - var userB = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim(ClaimTypes.NameIdentifier, "userB") }, "Test")); + var userB = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, "userA"), + new Claim(ClaimTypes.Name, "newer"), + }, "Test")); var context2 = BuildAuthPollContext(connection, sp, userB, DateTimeOffset.UtcNow.AddMinutes(10)); var pollTask = dispatcher.ExecuteAsync(context2, options, app); await connection.Transport.Output.WriteAsync(Encoding.UTF8.GetBytes("Unblock")).AsTask().DefaultTimeout(); @@ -1281,7 +1687,7 @@ public async Task StatefulReconnectRefreshedPrincipalInvokesOnAuthenticationRefr options.OnAuthenticationRefresh = ctx => { captured = ctx; - return ValueTask.FromResult(true); + return Task.FromResult(true); }; var services = new ServiceCollection(); @@ -1342,7 +1748,7 @@ public async Task StatefulReconnectCarryingSameTokenDoesNotInvokeOnAuthenticatio options.OnAuthenticationRefresh = _ => { refreshCount++; - return ValueTask.FromResult(true); + return Task.FromResult(true); }; var services = new ServiceCollection(); @@ -1382,7 +1788,11 @@ public void UpdateUserSkipsOlderToken() var manager = CreateConnectionManager(LoggerFactory); var connection = manager.CreateConnection(new HttpConnectionDispatcherOptions(), negotiateVersion: 1); - var userA = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "A") }, "Test")); + var userA = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim(ClaimTypes.NameIdentifier, "user"), + new Claim("name", "A"), + ], "Test")); var laterExpiration = DateTimeOffset.UtcNow.AddMinutes(30); connection.UpdateUser(userA, laterExpiration); @@ -1391,7 +1801,11 @@ public void UpdateUserSkipsOlderToken() feature.OnUserRefreshed((_, state) => notified++, state: null); // An older token must be skipped (no swap, no notification). - var userB = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "B") }, "Test")); + var userB = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim(ClaimTypes.NameIdentifier, "user"), + new Claim("name", "B"), + ], "Test")); connection.UpdateUser(userB, DateTimeOffset.UtcNow.AddMinutes(5)); Assert.Same(userA, connection.User); @@ -1399,7 +1813,11 @@ public void UpdateUserSkipsOlderToken() Assert.Equal(0, notified); // A newer token is still applied. - var userC = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "C") }, "Test")); + var userC = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim(ClaimTypes.NameIdentifier, "user"), + new Claim("name", "C"), + ], "Test")); var newerExpiration = DateTimeOffset.UtcNow.AddMinutes(60); connection.UpdateUser(userC, newerExpiration); @@ -1487,6 +1905,15 @@ private static void SetAuthenticateResultFeature(HttpContext context, DateTimeOf context.Features.Set(new TestAuthenticateResultFeature(AuthenticateResult.Success(ticket))); } + private static ClaimsPrincipal CreateAuthenticatedUserWithStableIdentity(string name) + { + return new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim(ClaimTypes.NameIdentifier, "test-user"), + new Claim("name", name), + ], "Test")); + } + private sealed class TestAuthenticateResultFeature : IAuthenticateResultFeature { public TestAuthenticateResultFeature(AuthenticateResult result) => AuthenticateResult = result; diff --git a/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs b/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs index 7fe0f72ec70b..d21cbf34a198 100644 --- a/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs +++ b/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs @@ -2606,7 +2606,7 @@ public async Task LongPollingWithDifferentUserNameRejectsPoll() } [Fact] - public async Task SendWithDifferentUserNameRejectsRequestAndKeepsConnection() + public async Task SendWithDifferentUserNameRejectsRequestDespitePermissiveUserRefreshPolicy() { using (StartVerifiableLog()) { @@ -2614,6 +2614,9 @@ public async Task SendWithDifferentUserNameRejectsRequestAndKeepsConnection() var connection = manager.CreateConnection(); connection.TransportType = HttpTransportType.LongPolling; connection.User = MakeUser("user1"); + var userRefreshFeature = connection.Features.Get(); + Assert.NotNull(userRefreshFeature); + userRefreshFeature.OnUserRefreshing = static _ => true; var dispatcher = CreateDispatcher(manager, LoggerFactory); @@ -2641,7 +2644,7 @@ public async Task SendWithDifferentUserNameRejectsRequestAndKeepsConnection() } [Fact] - public async Task DeleteWithDifferentUserNameRejectsRequestAndKeepsConnection() + public async Task DeleteWithDifferentUserNameRejectsRequestDespitePermissiveUserRefreshPolicy() { using (StartVerifiableLog()) { @@ -2649,6 +2652,9 @@ public async Task DeleteWithDifferentUserNameRejectsRequestAndKeepsConnection() var connection = manager.CreateConnection(); connection.TransportType = HttpTransportType.LongPolling; connection.User = MakeUser("user1"); + var userRefreshFeature = connection.Features.Get(); + Assert.NotNull(userRefreshFeature); + userRefreshFeature.OnUserRefreshing = static _ => true; var dispatcher = CreateDispatcher(manager, LoggerFactory); @@ -2714,7 +2720,7 @@ public async Task LongPollingWithDifferentUpnClaimRejectsPoll() } [Fact] - public async Task LongPollingWithoutStandardIdentityClaimDoesNotReject() + public async Task LongPollingAuthenticatedUserWithoutStableIdentityRejectsNewPrincipal() { using (StartVerifiableLog()) { @@ -2739,11 +2745,82 @@ public async Task LongPollingWithoutStandardIdentityClaimDoesNotReject() await dispatcher.ExecuteAsync(context, options, app).DefaultTimeout(); Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); - // Neither principal carries a standard identity claim (sub/NameIdentifier/Upn), so the - // dispatcher can't tell them apart and does not reject; behavior matches the pre-hardening path. var reconnectContext = MakeRequest("/foo", connection, services); reconnectContext.User = MakeUserWithClaim("employeeId", "2"); + await dispatcher.ExecuteAsync(reconnectContext, options, app).DefaultTimeout(); + + Assert.Equal(StatusCodes.Status403Forbidden, reconnectContext.Response.StatusCode); + Assert.Equal("1", connection.User.FindFirst("employeeId")?.Value); + } + } + + [Fact] + public async Task LongPollingAuthenticatedUserWithoutStableIdentityAllowsEquivalentPrincipal() + { + using (StartVerifiableLog()) + { + var manager = CreateConnectionManager(LoggerFactory); + var connection = manager.CreateConnection(); + connection.TransportType = HttpTransportType.LongPolling; + var dispatcher = CreateDispatcher(manager, LoggerFactory); + + var services = new ServiceCollection(); + services.AddOptions(); + services.AddSingleton(); + services.AddLogging(); + + var builder = new ConnectionBuilder(services.BuildServiceProvider()); + builder.UseConnectionHandler(); + var app = builder.Build(); + var options = new HttpConnectionDispatcherOptions(); + + var context = MakeRequest("/foo", connection, services); + context.User = MakeUserWithClaim("employeeId", "1"); + + await dispatcher.ExecuteAsync(context, options, app).DefaultTimeout(); + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + + var reconnectContext = MakeRequest("/foo", connection, services); + reconnectContext.User = MakeUserWithClaim("employeeId", "1"); + + var pollTask = dispatcher.ExecuteAsync(reconnectContext, options, app); + await connection.Transport.Output.WriteAsync(Encoding.UTF8.GetBytes("Unblock")).AsTask().DefaultTimeout(); + await pollTask.DefaultTimeout(); + + Assert.NotEqual(StatusCodes.Status403Forbidden, reconnectContext.Response.StatusCode); + } + } + + [Fact] + public async Task LongPollingAnonymousUserWithoutStableIdentityAllowsNewPrincipal() + { + using (StartVerifiableLog()) + { + var manager = CreateConnectionManager(LoggerFactory); + var connection = manager.CreateConnection(); + connection.TransportType = HttpTransportType.LongPolling; + var dispatcher = CreateDispatcher(manager, LoggerFactory); + + var services = new ServiceCollection(); + services.AddOptions(); + services.AddSingleton(); + services.AddLogging(); + + var builder = new ConnectionBuilder(services.BuildServiceProvider()); + builder.UseConnectionHandler(); + var app = builder.Build(); + var options = new HttpConnectionDispatcherOptions(); + + var context = MakeRequest("/foo", connection, services); + context.User = new ClaimsPrincipal(new ClaimsIdentity()); + + await dispatcher.ExecuteAsync(context, options, app).DefaultTimeout(); + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + + var reconnectContext = MakeRequest("/foo", connection, services); + reconnectContext.User = new ClaimsPrincipal(new ClaimsIdentity()); + var pollTask = dispatcher.ExecuteAsync(reconnectContext, options, app); await connection.Transport.Output.WriteAsync(Encoding.UTF8.GetBytes("Unblock")).AsTask().DefaultTimeout(); await pollTask.DefaultTimeout(); diff --git a/src/SignalR/common/Http.Connections/test/MapConnectionHandlerTests.cs b/src/SignalR/common/Http.Connections/test/MapConnectionHandlerTests.cs index 9a3cbfb90ef5..4b877b456e06 100644 --- a/src/SignalR/common/Http.Connections/test/MapConnectionHandlerTests.cs +++ b/src/SignalR/common/Http.Connections/test/MapConnectionHandlerTests.cs @@ -353,7 +353,7 @@ void ConfigureRoutes(IEndpointRouteBuilder endpoints) } [Fact] - public void MapConnectionHandlerEndPointRoutingAppliesAuthenticationRefreshMetadataWhenEnabled() + public void MapConnectionHandlerEndPointRoutingAppliesAuthenticationRefreshAndAuthorizationMetadataWhenEnabled() { void ConfigureRoutes(IEndpointRouteBuilder endpoints) { @@ -383,6 +383,7 @@ void ConfigureRoutes(IEndpointRouteBuilder endpoints) var optionsMetaData = endpoint.Metadata.GetMetadata(); Assert.NotNull(optionsMetaData); Assert.True(optionsMetaData.EnableAuthenticationRefresh); + Assert.Single(endpoint.Metadata.GetOrderedMetadata()); }, endpoint => { diff --git a/src/SignalR/server/Core/src/Hub.cs b/src/SignalR/server/Core/src/Hub.cs index 6e80e75e249e..ed31aed2ec2e 100644 --- a/src/SignalR/server/Core/src/Hub.cs +++ b/src/SignalR/server/Core/src/Hub.cs @@ -98,6 +98,9 @@ public virtual Task OnDisconnectedAsync(Exception? exception) /// The previous principal is intentionally not exposed because its underlying resources /// (for example a 's SafeHandle) /// may already be disposed by the time this method runs. + /// Authentication refresh does not change or rekey + /// SignalR user routing. Clients.User(...) remains associated with the identifier established + /// when the connection started. Reconnect the client to change its routing identifier. /// public virtual Task OnAuthenticationRefreshedAsync() { diff --git a/src/SignalR/server/Core/src/HubCallerContext.cs b/src/SignalR/server/Core/src/HubCallerContext.cs index 51464a4d0efe..d31e4e24ef10 100644 --- a/src/SignalR/server/Core/src/HubCallerContext.cs +++ b/src/SignalR/server/Core/src/HubCallerContext.cs @@ -19,6 +19,10 @@ public abstract class HubCallerContext /// /// Gets the user identifier. /// + /// + /// The identifier is established when the connection starts and is not changed by authentication + /// refresh. Reconnect the client to change its SignalR user-routing identifier. + /// public abstract string? UserIdentifier { get; } /// diff --git a/src/SignalR/server/Core/src/HubConnectionContext.cs b/src/SignalR/server/Core/src/HubConnectionContext.cs index 3c585e2b21d1..41082a3509f6 100644 --- a/src/SignalR/server/Core/src/HubConnectionContext.cs +++ b/src/SignalR/server/Core/src/HubConnectionContext.cs @@ -53,7 +53,6 @@ public partial class HubConnectionContext private bool _useStatefulReconnect; private DefaultHubCallerContext? _hubCallerContext; private string? _userIdentifier; - // IUserIdProvider.GetUserId receives the connection, not the candidate principal. During refresh this // lets that synchronous call see the pending principal before publishing the refreshed hub state, so we // can reject identifier changes without briefly exposing the new user to concurrent hub code. @@ -223,6 +222,11 @@ internal void ApplyUserState(ClaimsPrincipal user, string? userIdentifier) PublishHubCallerContext(new DefaultHubCallerContext(this, user)); } + internal void ApplyUser(ClaimsPrincipal user) + { + PublishHubCallerContext(new DefaultHubCallerContext(this, user)); + } + private sealed class UserIdProviderUserState { public UserIdProviderUserState(HubConnectionContext connection, ClaimsPrincipal user) diff --git a/src/SignalR/server/Core/src/HubConnectionHandler.cs b/src/SignalR/server/Core/src/HubConnectionHandler.cs index ae492ed4b1fa..1890d48a6c40 100644 --- a/src/SignalR/server/Core/src/HubConnectionHandler.cs +++ b/src/SignalR/server/Core/src/HubConnectionHandler.cs @@ -21,6 +21,8 @@ namespace Microsoft.AspNetCore.SignalR; /// public class HubConnectionHandler<[DynamicallyAccessedMembers(Hub.DynamicallyAccessedMembers)] THub> : ConnectionHandler where THub : Hub { + private static readonly string[] _userIdentityClaimTypes = ["sub", ClaimTypes.NameIdentifier, ClaimTypes.Upn]; + private readonly HubLifetimeManager _lifetimeManager; private readonly ILoggerFactory _loggerFactory; private readonly ILogger> _logger; @@ -147,19 +149,24 @@ public override async Task OnConnectedAsync(ConnectionContext connection) // -- the connectionContext has been set up -- var userRefreshFeature = connection.Features.Get(); + Func? userRefreshingCallback = null; IDisposable? userRefreshedRegistration = null; if (userRefreshFeature is not null) { - // Serializes authentication-refresh handling for this connection so concurrent refreshes don't race the re-key. + // Serializes authentication-refresh handling so concurrent refreshes publish hub user state in order. var authenticationRefreshLock = new SemaphoreSlim(1, 1); + var userRefreshState = new UserRefreshState(this, connectionContext, authenticationRefreshLock); + userRefreshingCallback = user => + userRefreshState.Handler.OnUserRefreshing(userRefreshState.Connection, user); + userRefreshFeature.OnUserRefreshing = userRefreshingCallback; userRefreshedRegistration = userRefreshFeature.OnUserRefreshed(static (user, state) => { - var userRefreshedState = (UserRefreshedState)state!; - userRefreshedState.Handler.OnUserRefreshed( - userRefreshedState.Connection, + var userRefreshState = (UserRefreshState)state!; + userRefreshState.Handler.OnUserRefreshed( + userRefreshState.Connection, user, - userRefreshedState.AuthenticationRefreshLock); - }, new UserRefreshedState(this, connectionContext, authenticationRefreshLock)); + userRefreshState.AuthenticationRefreshLock); + }, userRefreshState); } try @@ -169,6 +176,12 @@ public override async Task OnConnectedAsync(ConnectionContext connection) } finally { + if (userRefreshFeature is not null + && ReferenceEquals(userRefreshFeature.OnUserRefreshing, userRefreshingCallback)) + { + userRefreshFeature.OnUserRefreshing = null; + } + userRefreshedRegistration?.Dispose(); connectionContext.Cleanup(); @@ -178,6 +191,124 @@ public override async Task OnConnectedAsync(ConnectionContext connection) } } + private bool OnUserRefreshing(HubConnectionContext connection, ClaimsPrincipal user) + { + try + { + var newUserId = connection.GetUserIdentifier(user, _userIdProvider); + if (string.Equals(newUserId, connection.UserIdentifier, StringComparison.Ordinal)) + { + // A non-empty IUserIdProvider result is the application's authoritative identity mapping. + // If neither principal maps to a SignalR user, retain the transport's standard-identity + // and unchanged-principal fallbacks instead of treating all unmapped users as the same user. + if (!string.IsNullOrEmpty(newUserId) + || IsSameUserByDefault(connection.User, user)) + { + return true; + } + } + + Log.UserIdentifierChangeRejected(_logger, connection.UserIdentifier, newUserId); + return false; + } + catch (Exception ex) + { + Log.ErrorValidatingAuthenticationRefresh(_logger, ex); + return false; + } + } + + private static bool IsSameUserByDefault(ClaimsPrincipal currentUser, ClaimsPrincipal newUser) + { + if (ReferenceEquals(currentUser, newUser)) + { + return true; + } + + var currentIdentityKey = GetUserIdentityKey(currentUser); + var newIdentityKey = GetUserIdentityKey(newUser); + if (currentIdentityKey is not null || newIdentityKey is not null) + { + return currentIdentityKey == newIdentityKey; + } + + if (!HasAuthenticatedIdentity(currentUser) && !HasAuthenticatedIdentity(newUser)) + { + return true; + } + + return ClaimsPrincipalContentEquals(currentUser, newUser); + } + + private static (string Type, string Value, string Issuer)? GetUserIdentityKey(ClaimsPrincipal user) + { + foreach (var claimType in _userIdentityClaimTypes) + { + var claim = user.FindFirst(claimType); + if (claim is not null && !string.IsNullOrEmpty(claim.Value)) + { + return (claim.Type, claim.Value, claim.Issuer); + } + } + + return null; + } + + private static bool ClaimsPrincipalContentEquals(ClaimsPrincipal current, ClaimsPrincipal incoming) + { + return SequenceEqual(current.Identities, incoming.Identities, ClaimsIdentityContentEquals); + } + + private static bool ClaimsIdentityContentEquals(ClaimsIdentity current, ClaimsIdentity incoming) + { + if (!string.Equals(current.AuthenticationType, incoming.AuthenticationType, StringComparison.Ordinal) + || !string.Equals(current.NameClaimType, incoming.NameClaimType, StringComparison.Ordinal) + || !string.Equals(current.RoleClaimType, incoming.RoleClaimType, StringComparison.Ordinal) + || !string.Equals(current.Label, incoming.Label, StringComparison.Ordinal)) + { + return false; + } + + return SequenceEqual(current.Claims, incoming.Claims, ClaimContentEquals); + } + + private static bool ClaimContentEquals(Claim current, Claim incoming) + { + return string.Equals(current.Type, incoming.Type, StringComparison.Ordinal) + && string.Equals(current.Value, incoming.Value, StringComparison.Ordinal) + && string.Equals(current.ValueType, incoming.ValueType, StringComparison.Ordinal) + && string.Equals(current.Issuer, incoming.Issuer, StringComparison.Ordinal) + && string.Equals(current.OriginalIssuer, incoming.OriginalIssuer, StringComparison.Ordinal); + } + + private static bool SequenceEqual(IEnumerable current, IEnumerable incoming, Func equals) + { + using var currentEnumerator = current.GetEnumerator(); + using var incomingEnumerator = incoming.GetEnumerator(); + + while (true) + { + var currentHasValue = currentEnumerator.MoveNext(); + if (currentHasValue != incomingEnumerator.MoveNext()) + { + return false; + } + + if (!currentHasValue) + { + return true; + } + + if (!equals(currentEnumerator.Current, incomingEnumerator.Current)) + { + return false; + } + } + } + + private static bool HasAuthenticatedIdentity(ClaimsPrincipal user) + => user.Identities.Any(static identity => identity.IsAuthenticated); + private void OnUserRefreshed(HubConnectionContext connection, ClaimsPrincipal user, SemaphoreSlim authenticationRefreshLock) { // Fire and forget; HandleUserRefreshedAsync serializes work per connection through authenticationRefreshLock. @@ -189,17 +320,32 @@ private async Task HandleUserRefreshedAsync(HubConnectionContext connection, Cla await authenticationRefreshLock.WaitAsync(); try { - // Recompute inside the lock so a concurrent refresh observes the latest principal and identifier. - var newUserId = connection.GetUserIdentifier(user, _userIdProvider); - if (!string.Equals(newUserId, connection.UserIdentifier, StringComparison.Ordinal)) + // The connection user can advance again before this asynchronous callback acquires the lock. + // Only publish a callback that still represents the current lower-layer connection state. + var connectionUserFeature = connection.Features.Get(); + if (connectionUserFeature is not null && !ReferenceEquals(connectionUserFeature.User, user)) { - var previousUserId = connection.UserIdentifier; - Log.UserIdentifierChangedOnRefresh(_logger, previousUserId, newUserId); - connection.Abort(); return; } - connection.ApplyUserState(user, newUserId); + try + { + // Compute the refreshed mapping for diagnostics, but keep the connection's routing identifier + // fixed because lifetime managers have no contract for rekeying an existing connection. + var newUserId = connection.GetUserIdentifier(user, _userIdProvider); + if (!string.Equals(newUserId, connection.UserIdentifier, StringComparison.Ordinal)) + { + Log.UserIdentifierChangedOnRefresh(_logger, connection.UserIdentifier, newUserId); + } + } + catch (Exception ex) + { + // The principal has already been accepted and published by the transport. A diagnostic + // IUserIdProvider failure must not prevent the hub layer from publishing the same principal. + Log.ErrorResolvingRefreshedUserIdentifier(_logger, ex); + } + + connection.ApplyUser(user); } catch (Exception ex) { @@ -217,7 +363,7 @@ private async Task HandleUserRefreshedAsync(HubConnectionContext connection, Cla _ = _dispatcher.OnAuthenticationRefreshedAsync(connection); } - private sealed class UserRefreshedState( + private sealed class UserRefreshState( HubConnectionHandler handler, HubConnectionContext connection, SemaphoreSlim authenticationRefreshLock) diff --git a/src/SignalR/server/Core/src/HubConnectionHandlerLog.cs b/src/SignalR/server/Core/src/HubConnectionHandlerLog.cs index 388c1dc045df..59432dd4b82a 100644 --- a/src/SignalR/server/Core/src/HubConnectionHandlerLog.cs +++ b/src/SignalR/server/Core/src/HubConnectionHandlerLog.cs @@ -25,9 +25,18 @@ internal static partial class HubConnectionHandlerLog [LoggerMessage(6, LogLevel.Debug, "OnConnectedAsync ending.", EventName = "ConnectedEnding")] public static partial void ConnectedEnding(ILogger logger); - [LoggerMessage(7, LogLevel.Warning, "Authentication refresh produced a different user identifier (old: '{PreviousUserIdentifier}', new: '{NewUserIdentifier}'). Changing a connection's user identifier during refresh is not supported, so the connection is aborted.", EventName = "UserIdentifierChangedOnRefresh")] + [LoggerMessage(7, LogLevel.Warning, "The refreshed principal maps to a different user identifier (current: '{PreviousUserIdentifier}', refreshed: '{NewUserIdentifier}'). The configured refresh policy accepted the principal, but SignalR retained the current identifier because user routing cannot be rekeyed.", EventName = "UserIdentifierChangedOnRefresh")] public static partial void UserIdentifierChangedOnRefresh(ILogger logger, string? previousUserIdentifier, string? newUserIdentifier); [LoggerMessage(8, LogLevel.Error, "Error when applying refreshed authentication state.", EventName = "ErrorApplyingAuthenticationRefresh")] public static partial void ErrorApplyingAuthenticationRefresh(ILogger logger, Exception exception); + + [LoggerMessage(9, LogLevel.Warning, "Authentication refresh was rejected because it produced a different user identifier (old: '{PreviousUserIdentifier}', new: '{NewUserIdentifier}'). Changing a connection's user identifier during refresh is not supported.", EventName = "UserIdentifierChangeRejected")] + public static partial void UserIdentifierChangeRejected(ILogger logger, string? previousUserIdentifier, string? newUserIdentifier); + + [LoggerMessage(10, LogLevel.Error, "Error when validating refreshed authentication state. The refresh was rejected.", EventName = "ErrorValidatingAuthenticationRefresh")] + public static partial void ErrorValidatingAuthenticationRefresh(ILogger logger, Exception exception); + + [LoggerMessage(11, LogLevel.Warning, "Error when resolving the refreshed principal's user identifier for diagnostics. SignalR retained the connection's current user identifier.", EventName = "ErrorResolvingRefreshedUserIdentifier")] + public static partial void ErrorResolvingRefreshedUserIdentifier(ILogger logger, Exception exception); } diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.AuthenticationRefresh.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.AuthenticationRefresh.cs index 5605b8b02015..4346f67dec85 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.AuthenticationRefresh.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.AuthenticationRefresh.cs @@ -197,6 +197,179 @@ public async Task RefreshWithSameUserIdentifierDoesNotAbortAndDispatches() } } + [Fact] + public async Task UserRefreshingAllowsStandardIdentityClaimChangeWhenUserIdProviderIsStable() + { + using (StartVerifiableLog()) + { + var hubObserver = new AuthenticationRefreshObserver(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services => + { + services.AddSingleton(hubObserver); + services.AddSingleton(); + }, LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient(userIdentifier: "old-name-identifier")) + { + client.Connection.User!.AddIdentity(new ClaimsIdentity([new Claim("stable-id", "stable-user")])); + var feature = new TestConnectionUserRefreshFeature(); + client.Connection.Features.Set(feature); + + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + + var refreshedUser = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim(ClaimTypes.NameIdentifier, "new-name-identifier"), + new Claim("stable-id", "stable-user"), + ], "Test")); + + Assert.True(feature.Validate(refreshedUser)); + + client.Connection.User = refreshedUser; + feature.Raise(refreshedUser); + await hubObserver.RefreshedTask.DefaultTimeout(); + + var identifiers = await client.InvokeAsync(nameof(AuthenticationRefreshHub.GetUserNameIdentifierAndUserIdentifier)).DefaultTimeout(); + Assert.Null(identifiers.Error); + Assert.Equal("new-name-identifier:stable-user", identifiers.Result); + + client.Dispose(); + await connectionHandlerTask.DefaultTimeout(); + } + } + } + + [Fact] + public async Task UserRefreshingRejectsUserIdentifierChangeBeforePublication() + { + using (StartVerifiableLog(write => write.EventId.Name == "UserIdentifierChangeRejected")) + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(_ => { }, LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient(userIdentifier: "user-1")) + { + var feature = new TestConnectionUserRefreshFeature(); + client.Connection.Features.Set(feature); + + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + var originalUser = client.Connection.User; + var refreshedUser = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim(ClaimTypes.NameIdentifier, "user-2"), + ], "Test")); + + Assert.False(feature.Validate(refreshedUser)); + Assert.Same(originalUser, client.Connection.User); + + client.Dispose(); + await connectionHandlerTask.DefaultTimeout(); + } + } + } + + [Fact] + public async Task UserRefreshingUsesStandardIdentityWhenUserIdProviderReturnsNull() + { + using (StartVerifiableLog(write => write.EventId.Name == "UserIdentifierChangeRejected")) + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(_ => { }, LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + client.Connection.User = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim("sub", "user-a"), + ], "Test")); + var feature = new TestConnectionUserRefreshFeature(); + client.Connection.Features.Set(feature); + + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + + var sameUser = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim("sub", "user-a"), + new Claim(ClaimTypes.Role, "admin"), + ], "Test")); + var differentUser = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim("sub", "user-b"), + ], "Test")); + + Assert.True(feature.Validate(sameUser)); + Assert.False(feature.Validate(differentUser)); + + client.Dispose(); + await connectionHandlerTask.DefaultTimeout(); + } + } + } + + [Fact] + public async Task UserRefreshingRejectsAuthenticatedPrincipalWithoutStableIdentity() + { + using (StartVerifiableLog(write => write.EventId.Name == "UserIdentifierChangeRejected")) + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(_ => { }, LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + client.Connection.User = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim("employeeId", "1"), + ], "Test")); + var feature = new TestConnectionUserRefreshFeature(); + client.Connection.Features.Set(feature); + + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + + Assert.True(feature.Validate(client.Connection.User)); + + var equivalentUser = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim("employeeId", "1"), + ], "Test")); + Assert.True(feature.Validate(equivalentUser)); + + var differentUser = new ClaimsPrincipal(new ClaimsIdentity( + [ + new Claim("employeeId", "2"), + ], "Test")); + Assert.False(feature.Validate(differentUser)); + + client.Dispose(); + await connectionHandlerTask.DefaultTimeout(); + } + } + } + + [Fact] + public async Task UserRefreshingAllowsDistinctAnonymousPrincipalsWithoutStableIdentity() + { + using (StartVerifiableLog()) + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(_ => { }, LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + client.Connection.User = new ClaimsPrincipal(new ClaimsIdentity()); + var feature = new TestConnectionUserRefreshFeature(); + client.Connection.Features.Set(feature); + + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + + Assert.True(feature.Validate(new ClaimsPrincipal(new ClaimsIdentity()))); + + client.Dispose(); + await connectionHandlerTask.DefaultTimeout(); + } + } + } + [Fact] public async Task ExceptionFromOnAuthenticationRefreshedAsyncIsLoggedAndConnectionSurvives() { @@ -278,6 +451,51 @@ public async Task MultipleRefreshesEachDispatchInOrder() } } + [Fact] + public async Task OlderAuthenticationRefreshNotificationDoesNotOverwriteNewerHubUser() + { + using (StartVerifiableLog()) + { + var hubObserver = new AuthenticationRefreshObserver(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider( + services => services.AddSingleton(hubObserver), LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient(userIdentifier: "alice")) + { + var feature = new TestConnectionUserRefreshFeature(); + client.Connection.Features.Set(feature); + + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + + var olderUser = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, "alice"), + new Claim(ClaimTypes.Name, "older"), + }, "Test")); + var newerUser = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(ClaimTypes.NameIdentifier, "alice"), + new Claim(ClaimTypes.Name, "newer"), + }, "Test")); + client.Connection.User = newerUser; + feature.Raise(newerUser); + await hubObserver.RefreshedTask.DefaultTimeout(); + + // The lower-layer connection has already advanced to newerUser. A delayed callback for + // olderUser must not overwrite the HubCallerContext with a principal that is no longer current. + feature.Raise(olderUser); + + var seenUser = await client.InvokeAsync(nameof(AuthenticationRefreshHub.GetUserName)).DefaultTimeout(); + Assert.Null(seenUser.Error); + Assert.Equal("newer", seenUser.Result); + + client.Dispose(); + await connectionHandlerTask.DefaultTimeout(); + } + } + } + [Fact] public async Task ConnectionWithoutUserRefreshFeatureStillWorks() { @@ -303,12 +521,14 @@ public async Task ConnectionWithoutUserRefreshFeatureStillWorks() } [Fact] - public async Task UserIdentifierChangeOnRefreshAbortsConnection() + public async Task ReplacementPolicyCanAllowPrincipalChangeWithoutChangingUserIdentifier() { using (StartVerifiableLog(write => write.EventId.Name == "UserIdentifierChangedOnRefresh")) { - var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(_ => { }, LoggerFactory); - var connectionHandler = serviceProvider.GetService>(); + var hubObserver = new AuthenticationRefreshObserver(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider( + services => services.AddSingleton(hubObserver), LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); using (var client = new TestClient(userIdentifier: "user-1")) { @@ -316,28 +536,42 @@ public async Task UserIdentifierChangeOnRefreshAbortsConnection() client.Connection.Features.Set(feature); var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); - var refreshedUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim(ClaimTypes.NameIdentifier, "user-2"), }, "Test")); + + Assert.False(feature.Validate(refreshedUser)); + feature.OnUserRefreshing = static _ => true; + Assert.True(feature.Validate(refreshedUser)); + client.Connection.User = refreshedUser; feature.Raise(refreshedUser); + await hubObserver.RefreshedTask.DefaultTimeout(); + + var identifiers = await client.InvokeAsync(nameof(AuthenticationRefreshHub.GetUserNameIdentifierAndUserIdentifier)).DefaultTimeout(); + Assert.Null(identifiers.Error); + Assert.Equal("user-2:user-1", identifiers.Result); + + client.Dispose(); await connectionHandlerTask.DefaultTimeout(); } } } [Fact] - public async Task UserIdProviderExceptionOnRefreshIsLoggedAndAbortsConnection() + public async Task UserIdProviderExceptionAfterReplacementPolicyDoesNotAbortConnection() { - using (StartVerifiableLog(write => write.EventId.Name == "ErrorApplyingAuthenticationRefresh")) + using (StartVerifiableLog(write => write.EventId.Name == "ErrorResolvingRefreshedUserIdentifier")) { - var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider( - services => services.AddSingleton(), - LoggerFactory); - var connectionHandler = serviceProvider.GetService>(); + var hubObserver = new AuthenticationRefreshObserver(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services => + { + services.AddSingleton(hubObserver); + services.AddSingleton(); + }, LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); using (var client = new TestClient(userIdentifier: "stable-user")) { @@ -348,12 +582,22 @@ public async Task UserIdProviderExceptionOnRefreshIsLoggedAndAbortsConnection() var refreshedUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { - new Claim(ClaimTypes.NameIdentifier, "stable-user"), + new Claim(ClaimTypes.NameIdentifier, "refreshed-user"), + new Claim(ClaimTypes.Name, "refreshed-user"), new Claim("throw", "true"), }, "Test")); + feature.OnUserRefreshing = static _ => true; + Assert.True(feature.Validate(refreshedUser)); client.Connection.User = refreshedUser; feature.Raise(refreshedUser); + Assert.Equal("refreshed-user", await hubObserver.RefreshedTask.DefaultTimeout()); + + var identifiers = await client.InvokeAsync(nameof(AuthenticationRefreshHub.GetUserNameIdentifierAndUserIdentifier)).DefaultTimeout(); + Assert.Null(identifiers.Error); + Assert.Equal("refreshed-user:stable-user", identifiers.Result); + + client.Dispose(); await connectionHandlerTask.DefaultTimeout(); } } @@ -472,12 +716,19 @@ private sealed class TestConnectionUserRefreshFeature : IConnectionUserRefreshFe private Action? _callback; private object? _state; + public Func? OnUserRefreshing { get; set; } + public IDisposable OnUserRefreshed(Action callback, object? state) { _callback = callback; _state = state; - return new CallbackRegistration(this); + return new UserRefreshedRegistration(this); + } + + public bool Validate(ClaimsPrincipal user) + { + return OnUserRefreshing?.Invoke(user) ?? true; } public void Raise(ClaimsPrincipal current) @@ -485,7 +736,7 @@ public void Raise(ClaimsPrincipal current) _callback?.Invoke(current, _state); } - private sealed class CallbackRegistration(TestConnectionUserRefreshFeature feature) : IDisposable + private sealed class UserRefreshedRegistration(TestConnectionUserRefreshFeature feature) : IDisposable { public void Dispose() { @@ -604,4 +855,12 @@ private sealed class ThrowingRefreshUserIdProvider : IUserIdProvider return connection.User.FindFirst(ClaimTypes.NameIdentifier)?.Value; } } + + private sealed class StableClaimUserIdProvider : IUserIdProvider + { + public string? GetUserId(HubConnectionContext connection) + { + return connection.User.FindFirst("stable-id")?.Value; + } + } }