diff --git a/src/Components/Server/src/Circuits/RevalidatingServerAuthenticationStateProvider.cs b/src/Components/Server/src/Circuits/RevalidatingServerAuthenticationStateProvider.cs index ee664eaf49eb..d455fb8a1f29 100644 --- a/src/Components/Server/src/Circuits/RevalidatingServerAuthenticationStateProvider.cs +++ b/src/Components/Server/src/Circuits/RevalidatingServerAuthenticationStateProvider.cs @@ -3,6 +3,7 @@ using System.Security.Claims; using Microsoft.AspNetCore.Components.Authorization; +using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Components.Server; @@ -61,7 +62,7 @@ private async Task RevalidationLoop(Task authenticationStat try { var authenticationState = await authenticationStateTask; - if (authenticationState.User.Identity?.IsAuthenticated == true) + if (SecurityHelper.IsAuthenticated(authenticationState.User)) { while (!cancellationToken.IsCancellationRequested) { diff --git a/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj b/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj index 4dad856d9c78..0be7c8bec04e 100644 --- a/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj +++ b/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj @@ -44,6 +44,7 @@ + diff --git a/src/Components/Web.JS/src/Virtualize.ts b/src/Components/Web.JS/src/Virtualize.ts index 4c0ca06d3b91..4b6fe3d7084a 100644 --- a/src/Components/Web.JS/src/Virtualize.ts +++ b/src/Components/Web.JS/src/Virtualize.ts @@ -24,6 +24,12 @@ const SpacerVisibilityReason = { RenderedContentMeasurement: 3, } as const; +const ViewportFillDirection = { + Covered: 0, + Before: 1, + After: 2, +} as const; + const ScrollSource = { None: 0, UserScroll: 1, @@ -644,7 +650,7 @@ function init(dotNetHelper: DotNet.DotNetObject, spacerBefore: HTMLElement, spac } // Measures the target's viewport-relative top and aligns it to containerTop. - function alignToItemAt(localIndex: number): void { + function alignToItemAt(localIndex: number): number | null { function beginAlign(): void { scrollActivity.ignoreNextScroll(); scrollActivity.source = ScrollSource.AlignToItem; @@ -656,10 +662,10 @@ function init(dotNetHelper: DotNet.DotNetObject, spacerBefore: HTMLElement, spac flushPendingStyleMutations(); const delta = measureLocalChildOffset(localIndex); if (Number.isNaN(delta)) { - // Target item isn't in DOM yet. Retry after the next render. + // Target item isn't in the committed window. pendingAlignLocalIndex = localIndex; beginAlign(); - return; + return null; } pendingAlignLocalIndex = null; @@ -671,6 +677,36 @@ function init(dotNetHelper: DotNet.DotNetObject, spacerBefore: HTMLElement, spac pendingJumpToEnd = false; scrollElement.scrollTo({ top: scrollElement.scrollTop + delta, behavior: 'instant' }); } + + return getViewportFillDirection(); + } + + function getViewportBounds(scaleFactor: number): { top: number; bottom: number } { + let viewportTop = 0; + let viewportBottom = document.documentElement.clientHeight; + if (scrollContainer) { + const scrollContainerRect = scrollContainer.getBoundingClientRect(); + viewportTop = scrollContainerRect.top + scrollContainer.clientTop * scaleFactor; + viewportBottom = viewportTop + scrollContainer.clientHeight * scaleFactor; + } + return { top: viewportTop, bottom: viewportBottom }; + } + + function occupiesViewport(spacer: HTMLElement, viewport: { top: number; bottom: number }): boolean { + const spacerRect = spacer.getBoundingClientRect(); + return Math.min(spacerRect.bottom, viewport.bottom) > Math.max(spacerRect.top, viewport.top); + } + + function getViewportFillDirection(): number { + const scaleFactor = getScaleFactor(spacerBefore, spacerAfter); + const viewport = getViewportBounds(scaleFactor); + if (occupiesViewport(spacerBefore, viewport)) { + return ViewportFillDirection.Before; + } + if (occupiesViewport(spacerAfter, viewport)) { + return ViewportFillDirection.After; + } + return ViewportFillDirection.Covered; } observersByDotNetObjectId[id] = { @@ -855,6 +891,9 @@ function init(dotNetHelper: DotNet.DotNetObject, spacerBefore: HTMLElement, spac }); if (intersectingEntries.length === 0) { + if (source === ScrollSource.AlignToItem) { + scrollActivity.clear(); + } return; } @@ -875,7 +914,6 @@ function init(dotNetHelper: DotNet.DotNetObject, spacerBefore: HTMLElement, spac const isBefore = entry.target === spacerBefore; const spacer = isBefore ? spacerBefore : spacerAfter; - // Skip an empty after spacer because it provides no useful measurement. if (!isBefore && spacer.offsetHeight === 0) { return; } @@ -895,6 +933,10 @@ function init(dotNetHelper: DotNet.DotNetObject, spacerBefore: HTMLElement, spac const methodName = isBefore ? 'OnSpacerBeforeVisible' : 'OnSpacerAfterVisible'; dotNetHelper.invokeMethodAsync(methodName, spacerSize, spacerSeparation, containerSize, reason); }); + + if (source === ScrollSource.AlignToItem) { + scrollActivity.clear(); + } } function isValidTableElement(element: HTMLElement | null): boolean { @@ -934,9 +976,9 @@ function restoreAnchor(dotNetHelper: DotNet.DotNetObject): void { entry?.restoreAnchor?.(); } -function alignToItem(dotNetHelper: DotNet.DotNetObject, localIndex: number): void { +function alignToItem(dotNetHelper: DotNet.DotNetObject, localIndex: number): number | null { const { observersByDotNetObjectId, id } = getObserversMapEntry(dotNetHelper); - observersByDotNetObjectId[id]?.alignToItem?.(localIndex); + return observersByDotNetObjectId[id]?.alignToItem?.(localIndex) ?? null; } function beginProgrammaticScroll(dotNetHelper: DotNet.DotNetObject): void { diff --git a/src/Components/Web/src/Forms/ExpressionMemberAccessor.cs b/src/Components/Web/src/Forms/ExpressionMemberAccessor.cs index 8fbe3562a2d8..ff5eab76e673 100644 --- a/src/Components/Web/src/Forms/ExpressionMemberAccessor.cs +++ b/src/Components/Web/src/Forms/ExpressionMemberAccessor.cs @@ -4,6 +4,7 @@ using System.Collections.Concurrent; using System.ComponentModel; using System.ComponentModel.DataAnnotations; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq.Expressions; using System.Reflection; @@ -31,26 +32,37 @@ private static MemberInfo GetMemberInfo(Expression> accesso return _memberInfoCache.GetOrAdd(accessor, static expr => { var lambdaExpression = (LambdaExpression)expr; - var accessorBody = lambdaExpression.Body; - - if (accessorBody is UnaryExpression unaryExpression - && unaryExpression.NodeType == ExpressionType.Convert - && unaryExpression.Type == typeof(object)) - { - accessorBody = unaryExpression.Operand; - } - - if (accessorBody is not MemberExpression memberExpression) + var member = GetMemberInfo(lambdaExpression.Body, out var accessorBody); + if (member is null) { throw new ArgumentException( $"The provided expression contains a {accessorBody.GetType().Name} which is not supported. " + $"Only simple member accessors (fields, properties) of an object are supported."); } - return memberExpression.Member; + return member; }); } + private static MemberInfo? GetMemberInfo(Expression accessorBody, out Expression normalizedAccessorBody) + { + normalizedAccessorBody = accessorBody; + + if (normalizedAccessorBody is UnaryExpression + { + NodeType: ExpressionType.Convert, + Type: var type + } unaryExpression && + type == typeof(object)) + { + normalizedAccessorBody = unaryExpression.Operand; + } + + return normalizedAccessorBody is MemberExpression memberExpression + ? memberExpression.Member + : null; + } + public static string GetDisplayName(MemberInfo member) { ArgumentNullException.ThrowIfNull(member); @@ -84,6 +96,23 @@ public static string GetDisplayName(Expression> accessor) return GetDisplayName(member); } + public static bool TryGetDisplayName( + Expression> accessor, + [NotNullWhen(true)] out string? displayName) + { + ArgumentNullException.ThrowIfNull(accessor); + + var member = GetMemberInfo(accessor.Body, out _); + if (member is null) + { + displayName = null; + return false; + } + + displayName = GetDisplayName(member); + return true; + } + private static void ClearCache() { _memberInfoCache.Clear(); diff --git a/src/Components/Web/src/Forms/InputBase.cs b/src/Components/Web/src/Forms/InputBase.cs index cf1bb261e6fe..bc1e815e62f4 100644 --- a/src/Components/Web/src/Forms/InputBase.cs +++ b/src/Components/Web/src/Forms/InputBase.cs @@ -60,6 +60,18 @@ public abstract class InputBase : ComponentBase, IDisposable /// [Parameter] public string? DisplayName { get; set; } + internal string GetDisplayName() + { + if (DisplayName is not null) + { + return DisplayName; + } + + return ExpressionMemberAccessor.TryGetDisplayName(ValueExpression!, out var displayName) + ? displayName + : FieldIdentifier.FieldName; + } + /// /// Gets the associated . /// This property is uninitialized if the input does not have a parent . diff --git a/src/Components/Web/src/Forms/InputDate.cs b/src/Components/Web/src/Forms/InputDate.cs index 3a7b1ba960ca..f00d425da118 100644 --- a/src/Components/Web/src/Forms/InputDate.cs +++ b/src/Components/Web/src/Forms/InputDate.cs @@ -118,7 +118,7 @@ protected override bool TryParseValueFromString(string? value, [MaybeNullWhen(fa } else { - validationErrorMessage = string.Format(CultureInfo.InvariantCulture, _parsingErrorMessage, DisplayName ?? FieldIdentifier.FieldName); + validationErrorMessage = string.Format(CultureInfo.InvariantCulture, _parsingErrorMessage, GetDisplayName()); return false; } } diff --git a/src/Components/Web/src/Forms/InputExtensions.cs b/src/Components/Web/src/Forms/InputExtensions.cs index 2d88d7772e20..7ca51fd811f1 100644 --- a/src/Components/Web/src/Forms/InputExtensions.cs +++ b/src/Components/Web/src/Forms/InputExtensions.cs @@ -40,7 +40,7 @@ internal static class InputExtensions } result = default; - validationErrorMessage = $"The {input.DisplayName ?? input.FieldIdentifier.FieldName} field is not valid."; + validationErrorMessage = $"The {input.GetDisplayName()} field is not valid."; return false; } catch (InvalidOperationException ex) diff --git a/src/Components/Web/src/Forms/InputNumber.cs b/src/Components/Web/src/Forms/InputNumber.cs index c2e9a1ebef50..b69c3ee3d87d 100644 --- a/src/Components/Web/src/Forms/InputNumber.cs +++ b/src/Components/Web/src/Forms/InputNumber.cs @@ -75,7 +75,7 @@ protected override bool TryParseValueFromString(string? value, [MaybeNullWhen(fa } else { - validationErrorMessage = string.Format(CultureInfo.InvariantCulture, ParsingErrorMessage, DisplayName ?? FieldIdentifier.FieldName); + validationErrorMessage = string.Format(CultureInfo.InvariantCulture, ParsingErrorMessage, GetDisplayName()); return false; } } diff --git a/src/Components/Web/src/Virtualization/ViewportFillDirection.cs b/src/Components/Web/src/Virtualization/ViewportFillDirection.cs new file mode 100644 index 000000000000..e2c948fdf41b --- /dev/null +++ b/src/Components/Web/src/Virtualization/ViewportFillDirection.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Components.Web.Virtualization; + +/// +/// The numeric values must stay in sync with the ViewportFillDirection constant in +/// Virtualize.ts. +/// +internal enum ViewportFillDirection +{ + Covered = 0, + Before = 1, + After = 2, +} diff --git a/src/Components/Web/src/Virtualization/Virtualize.cs b/src/Components/Web/src/Virtualization/Virtualize.cs index c8c1d7935c1e..1dde437f7666 100644 --- a/src/Components/Web/src/Virtualization/Virtualize.cs +++ b/src/Components/Web/src/Virtualization/Virtualize.cs @@ -264,6 +264,7 @@ private async Task ScrollToItemAsyncCore(int itemIndex, CancellationToken cancel var ourCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); _currentScrollCts = ourCts; var token = ourCts.Token; + ViewportFillDirection? fillDirection = null; if (_jsInterop is not null) { @@ -280,7 +281,7 @@ private async Task ScrollToItemAsyncCore(int itemIndex, CancellationToken cancel token.ThrowIfCancellationRequested(); var refetchRequired = MoveWindowToContain(itemIndex); await EnsureRenderCommittedAsync(refetchRequired, token); - await AlignToTargetAsync(itemIndex, token); + fillDirection = await AlignToTargetAsync(itemIndex, token); } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) { @@ -295,6 +296,8 @@ private async Task ScrollToItemAsyncCore(int itemIndex, CancellationToken cancel } ourCts.Dispose(); } + + UpdateWindowFromViewport(fillDirection, _visibleItemCapacity, _unusedItemCapacity); } private bool MoveWindowToContain(int itemIndex) @@ -346,21 +349,30 @@ private async Task EnsureRenderCommittedAsync(bool refetchRequired, Cancellation token.ThrowIfCancellationRequested(); } - private async ValueTask AlignToTargetAsync(int itemIndex, CancellationToken token) + private async ValueTask AlignToTargetAsync(int itemIndex, CancellationToken token) { // Re-clamp in case _itemCount shifted during the fetch. var localIndex = ClampToItemRange(itemIndex) - _itemsBefore; if (localIndex < 0 || localIndex >= _visibleItemCapacity || _lastRenderedItemCount == 0) { // Window doesn't contain the target (e.g., empty provider result) — bail cleanly. - return; + return null; } - // Pixel-exact one-shot scroll: JS reads getBoundingClientRect() and sets scrollTop. - if (_jsInterop is not null) + if (_jsInterop is null) { - await _jsInterop.AlignToItemAsync(localIndex, token); + return null; } + + var initialItemSize = _itemSize; + var fillDirection = await _jsInterop.AlignToItemAsync(localIndex, token); + if (_initialIndex.Phase == InitialIndexPhase.Pending && _itemSize != initialItemSize) + { + StateHasChanged(); + return null; + } + + return fillDirection; } private int ClampToItemRange(int requested) @@ -516,9 +528,15 @@ protected override async Task OnAfterRenderAsync(bool firstRender) } } - if (_jsInterop is not null && _lastRenderedItemCount > 0 && _initialIndex.ShouldRealign(_itemSize)) + if (_jsInterop is not null + && !_loading + && _loadedItemsStartIndex == _itemsBefore + && _lastRenderedItemCount > 0 + && _lastRenderedPlaceholderCount == 0 + && _initialIndex.Phase == InitialIndexPhase.Pending) { - await AlignToTargetAsync(InitialItemIndex, CancellationToken.None); + var fillDirection = await AlignToTargetAsync(InitialItemIndex, CancellationToken.None); + UpdateWindowFromViewport(fillDirection, _visibleItemCapacity, _unusedItemCapacity); } } @@ -626,17 +644,7 @@ private void UpdateItemSizeFromRenderedContent(float spacerSize, float spacerSep return; } - var previousItemSize = _itemSize; CalculateItemDistribution(spacerSize, spacerSeparation, containerSize, out _, out _, out _); - RerenderSpacersIfItemSizeChanged(previousItemSize); - } - - private void RerenderSpacersIfItemSizeChanged(float previousItemSize) - { - if (_itemSize != previousItemSize) - { - StateHasChanged(); - } } private void CancelInFlightScrollForUserInteraction() @@ -673,9 +681,9 @@ void IVirtualizeJsCallbacks.OnBeforeSpacerVisible(float spacerSize, float spacer CancelInFlightScrollForUserInteraction(); break; case SpacerVisibilityReason.ViewportFill: - // A fill callback while our own scroll is in flight, or while the initial target is pinned, - // is a side effect of that scroll — acting on it would move the target. - if (_currentScrollCts is not null || _initialIndex.Phase == InitialIndexPhase.Pending) + // A fill callback while our own scroll is in flight is a side effect of that scroll — + // acting on it would move the target. + if (_currentScrollCts is not null) { return; } @@ -684,6 +692,15 @@ void IVirtualizeJsCallbacks.OnBeforeSpacerVisible(float spacerSize, float spacer CalculateItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsBefore, out var visibleItemCapacity, out var unusedItemCapacity); + if (_initialIndex.Phase == InitialIndexPhase.Pending) + { + UpdateWindowFromViewport( + ViewportFillDirection.Before, + visibleItemCapacity, + unusedItemCapacity); + return; + } + // Slide window up by at least one if spacer is visible but position unchanged. if (_lastRenderedItemCount > 0 && itemsBefore == _itemsBefore && itemsBefore > 0) { @@ -714,11 +731,14 @@ void IVirtualizeJsCallbacks.OnAfterSpacerVisible(float spacerSize, float spacerS // landed, so acting on it would undo the target. The real fill runs once the scroll completes. return; } - var hadNewMeasurements = CalculateItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsAfter, out var visibleItemCapacity, out var unusedItemCapacity); if (_initialIndex.Phase == InitialIndexPhase.Pending) { + UpdateWindowFromViewport( + ViewportFillDirection.After, + visibleItemCapacity, + unusedItemCapacity); return; } @@ -744,6 +764,51 @@ void IVirtualizeJsCallbacks.OnAfterSpacerVisible(float spacerSize, float spacerS UpdateItemDistribution(itemsBefore, visibleItemCapacity, unusedItemCapacity); } + private void UpdateWindowFromViewport( + ViewportFillDirection? fillDirection, + int visibleItemCapacity, + int unusedItemCapacity) + { + if (fillDirection == ViewportFillDirection.Covered) + { + if (_initialIndex.Phase == InitialIndexPhase.Pending && _lastRenderedPlaceholderCount == 0) + { + _initialIndex.Complete(); + } + return; + } + + if (fillDirection is null) + { + return; + } + + var maximumCapacity = Math.Min(GetMaximumItemCapacity(), _itemCount); + var doubledCapacity = (long)Math.Max(1, _visibleItemCapacity) * 2; + var desiredCapacity = (int)Math.Min( + Math.Max((long)visibleItemCapacity, doubledCapacity), + maximumCapacity); + var availableItems = fillDirection == ViewportFillDirection.Before + ? _itemsBefore + : Math.Max(0, _itemCount - _itemsBefore - _visibleItemCapacity); + var addedItems = Math.Min( + Math.Max(0, desiredCapacity - _visibleItemCapacity), + availableItems); + + if (addedItems > 0 || unusedItemCapacity != _unusedItemCapacity) + { + _skipNextDistributionRefresh = false; + UpdateItemDistribution( + fillDirection == ViewportFillDirection.Before ? _itemsBefore - addedItems : _itemsBefore, + _visibleItemCapacity + addedItems, + unusedItemCapacity); + } + else if (_initialIndex.Phase == InitialIndexPhase.Pending && !_loading) + { + _initialIndex.Complete(); + } + } + private float GetEffectiveItemSizeForStaleSpacer() { var effectiveItemSize = GetItemHeight(); @@ -801,6 +866,18 @@ private bool CalculateItemDistribution( // This AppContext data was added as a stopgap for .NET 8 and earlier, since it was added in a patch // where we couldn't add new public API. For backcompat we still support the AppContext setting, but // new applications should use the much more convenient MaxItemCount parameter. + var maxItemCount = GetMaximumItemCapacity(); + + itemsInSpacer = Math.Max(0, (int)Math.Floor(spacerSize / effectiveItemSize) - OverscanCount); + visibleItemCapacity = (int)Math.Ceiling(containerSize / effectiveItemSize) + 2 * OverscanCount; + unusedItemCapacity = Math.Max(0, visibleItemCapacity - maxItemCount); + visibleItemCapacity -= unusedItemCapacity; + + return hadNewMeasurements; + } + + private int GetMaximumItemCapacity() + { var maxItemCount = AppContext.GetData("Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount") switch { int val => Math.Min(val, MaxItemCount), @@ -809,14 +886,7 @@ private bool CalculateItemDistribution( // Count the OverscanCount as used capacity, so we don't end up in a situation where // the user has set a very low MaxItemCount and we end up in an infinite loading loop. - maxItemCount += OverscanCount * 2; - - itemsInSpacer = Math.Max(0, (int)Math.Floor(spacerSize / effectiveItemSize) - OverscanCount); - visibleItemCapacity = (int)Math.Ceiling(containerSize / effectiveItemSize) + 2 * OverscanCount; - unusedItemCapacity = Math.Max(0, visibleItemCapacity - maxItemCount); - visibleItemCapacity -= unusedItemCapacity; - - return hadNewMeasurements; + return (int)Math.Min((long)maxItemCount + (long)OverscanCount * 2, int.MaxValue); } private void UpdateItemDistribution(int itemsBefore, int visibleItemCapacity, int unusedItemCapacity) @@ -1119,10 +1189,10 @@ private enum InitialIndexPhase private sealed class InitialIndexState { - public InitialIndexPhase Phase { get; private set; } - private float _alignItemSize; + public InitialIndexPhase Phase { get; private set; } + public void Complete() => Phase = InitialIndexPhase.Completed; public void BeginPending(float itemSize) @@ -1131,16 +1201,6 @@ public void BeginPending(float itemSize) _alignItemSize = itemSize; } - public bool ShouldRealign(float itemSize) - { - if (Phase != InitialIndexPhase.Pending || itemSize == _alignItemSize) - { - return false; - } - _alignItemSize = itemSize; - return true; - } - public void Abort() { if (Phase == InitialIndexPhase.Pending) diff --git a/src/Components/Web/src/Virtualization/VirtualizeJsInterop.cs b/src/Components/Web/src/Virtualization/VirtualizeJsInterop.cs index 9b644ab30d58..a020df48732d 100644 --- a/src/Components/Web/src/Virtualization/VirtualizeJsInterop.cs +++ b/src/Components/Web/src/Virtualization/VirtualizeJsInterop.cs @@ -62,9 +62,9 @@ public ValueTask RestoreAnchorAsync() return _jsRuntime.InvokeVoidAsync($"{JsFunctionsPrefix}.restoreAnchor", _selfReference); } - public ValueTask AlignToItemAsync(int localIndex, CancellationToken cancellationToken = default) + public ValueTask AlignToItemAsync(int localIndex, CancellationToken cancellationToken = default) { - return _jsRuntime.InvokeVoidAsync($"{JsFunctionsPrefix}.alignToItem", cancellationToken, _selfReference, localIndex); + return _jsRuntime.InvokeAsync($"{JsFunctionsPrefix}.alignToItem", cancellationToken, _selfReference, localIndex); } public ValueTask BeginProgrammaticScrollAsync() diff --git a/src/Components/Web/test/Forms/InputDateTest.cs b/src/Components/Web/test/Forms/InputDateTest.cs index 4192bf1f218b..6f9fffe4223b 100644 --- a/src/Components/Web/test/Forms/InputDateTest.cs +++ b/src/Components/Web/test/Forms/InputDateTest.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.ComponentModel.DataAnnotations; using Microsoft.AspNetCore.Components.RenderTree; using Microsoft.AspNetCore.Components.Test.Helpers; @@ -11,7 +12,7 @@ public class InputDateTest private readonly TestRenderer _testRenderer = new TestRenderer(); [Fact] - public async Task ValidationErrorUsesDisplayAttributeName() + public async Task ValidationErrorUsesExplicitDisplayName() { // Arrange var model = new TestModel(); @@ -36,6 +37,24 @@ public async Task ValidationErrorUsesDisplayAttributeName() Assert.Contains("The Date property field must be a date.", validationMessages); } + [Fact] + public async Task ValidationErrorUsesDisplayAttributeOnModel() + { + var model = new TestModel(); + var rootComponent = new TestInputHostComponent + { + EditContext = new EditContext(model), + ValueExpression = () => model.DateProperty, + }; + var fieldIdentifier = FieldIdentifier.Create(() => model.DateProperty); + var inputComponent = await InputRenderer.RenderAndGetComponent(rootComponent); + + await inputComponent.SetCurrentValueAsStringAsync("invalidDate"); + + var validationMessages = rootComponent.EditContext.GetValidationMessages(fieldIdentifier); + Assert.Contains("The Date from attribute field must be a date.", validationMessages); + } + [Fact] public async Task InputElementIsAssignedSuccessfully() { @@ -99,6 +118,7 @@ private async Task RenderAndGetInputDateComponentIdAsync(TestInputHostCompo private class TestModel { + [Display(Name = "Date from attribute")] public DateTime DateProperty { get; set; } } diff --git a/src/Components/Web/test/Forms/InputNumberTest.cs b/src/Components/Web/test/Forms/InputNumberTest.cs index dc4f76ce4acc..55e1d23ad27f 100644 --- a/src/Components/Web/test/Forms/InputNumberTest.cs +++ b/src/Components/Web/test/Forms/InputNumberTest.cs @@ -1,6 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.ComponentModel; +using System.ComponentModel.DataAnnotations; +using System.Linq.Expressions; using Microsoft.AspNetCore.Components.Forms.Mapping; using Microsoft.AspNetCore.Components.Infrastructure; using Microsoft.AspNetCore.Components.RenderTree; @@ -21,20 +24,20 @@ public InputNumberTest() } [Fact] - public async Task ValidationErrorUsesDisplayAttributeName() + public async Task ValidationErrorUsesExplicitDisplayName() { // Arrange var model = new TestModel(); var rootComponent = new TestInputHostComponent { EditContext = new EditContext(model), - ValueExpression = () => model.SomeNumber, + ValueExpression = () => model.NumberWithDisplayAttribute, AdditionalAttributes = new Dictionary { { "DisplayName", "Some number" } } }; - var fieldIdentifier = FieldIdentifier.Create(() => model.SomeNumber); + var fieldIdentifier = FieldIdentifier.Create(() => model.NumberWithDisplayAttribute); var inputComponent = await InputRenderer.RenderAndGetComponent(rootComponent); // Act @@ -46,6 +49,26 @@ public async Task ValidationErrorUsesDisplayAttributeName() Assert.Contains("The Some number field must be a number.", validationMessages); } + [Fact] + public Task ValidationErrorUsesDisplayAttributeOnModel() + { + var model = new TestModel(); + + return AssertValidationErrorUsesDisplayName( + () => model.NumberWithDisplayAttribute, + "The Display attribute number field must be a number."); + } + + [Fact] + public Task ValidationErrorUsesDisplayNameAttributeName() + { + var model = new TestModel(); + + return AssertValidationErrorUsesDisplayName( + () => model.NumberWithDisplayNameAttribute, + "The DisplayName attribute number field must be a number."); + } + [Fact] public async Task InputElementIsAssignedSuccessfully() { @@ -128,6 +151,24 @@ public async Task ExplicitIdOverridesGenerated() Assert.Equal("custom-number-id", idAttribute.AttributeValue); } + private static async Task AssertValidationErrorUsesDisplayName( + Expression> valueExpression, + string expectedValidationMessage) + { + var fieldIdentifier = FieldIdentifier.Create(valueExpression); + var rootComponent = new TestInputHostComponent + { + EditContext = new EditContext(fieldIdentifier.Model), + ValueExpression = valueExpression, + }; + var inputComponent = await InputRenderer.RenderAndGetComponent(rootComponent); + + await inputComponent.SetCurrentValueAsStringAsync("notANumber"); + + var validationMessages = rootComponent.EditContext.GetValidationMessages(fieldIdentifier); + Assert.Contains(expectedValidationMessage, validationMessages); + } + private async Task RenderAndGetTestInputNumberComponentIdAsync(TestInputHostComponent hostComponent) { var hostComponentId = _testRenderer.AssignRootComponentId(hostComponent); @@ -139,6 +180,12 @@ private async Task RenderAndGetTestInputNumberComponentIdAsync(TestInputHos private class TestModel { public int SomeNumber { get; set; } + + [Display(Name = "Display attribute number")] + public int NumberWithDisplayAttribute { get; set; } + + [DisplayName("DisplayName attribute number")] + public int NumberWithDisplayNameAttribute { get; set; } } private class TestInputNumberComponent : InputNumber diff --git a/src/Components/Web/test/Forms/InputRadioTest.cs b/src/Components/Web/test/Forms/InputRadioTest.cs index 483b80697986..c72284718217 100644 --- a/src/Components/Web/test/Forms/InputRadioTest.cs +++ b/src/Components/Web/test/Forms/InputRadioTest.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.ComponentModel.DataAnnotations; using System.Linq.Expressions; using Microsoft.AspNetCore.Components.Rendering; using Microsoft.AspNetCore.Components.RenderTree; @@ -73,6 +74,24 @@ public async Task InputElementIsAssignedSuccessfully() Assert.All(inputRadioComponents, inputRadio => Assert.NotNull(inputRadio.Element)); } + [Fact] + public async Task ValidationErrorUsesDisplayAttributeOnModel() + { + var model = new TestModel(); + var rootComponent = new TestInputHostComponent + { + EditContext = new EditContext(model), + ValueExpression = () => model.TestEnum, + }; + var fieldIdentifier = FieldIdentifier.Create(() => model.TestEnum); + var inputRadioGroup = await InputRenderer.RenderAndGetComponent(rootComponent); + + await inputRadioGroup.SetCurrentValueAsStringAsync("invalidValue"); + + var validationMessages = rootComponent.EditContext.GetValidationMessages(fieldIdentifier); + Assert.Contains("The Radio choice field is not valid.", validationMessages); + } + private static RenderFragment RadioButtonsWithoutGroup(string name) => (builder) => { foreach (var selectedValue in (TestEnum[])Enum.GetValues(typeof(TestEnum))) @@ -125,9 +144,18 @@ private enum TestEnum private class TestModel { + [Display(Name = "Radio choice")] public TestEnum TestEnum { get; set; } } + private class TestInputRadioGroup : InputRadioGroup + { + public async Task SetCurrentValueAsStringAsync(string value) + { + await InvokeAsync(() => { base.CurrentValueAsString = value; }); + } + } + private class TestInputRadio : InputRadio { public string GroupName => Context.GroupName; diff --git a/src/Components/Web/test/Forms/InputSelectTest.cs b/src/Components/Web/test/Forms/InputSelectTest.cs index a2d6d0112138..7d4dc5374804 100644 --- a/src/Components/Web/test/Forms/InputSelectTest.cs +++ b/src/Components/Web/test/Forms/InputSelectTest.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.ComponentModel.DataAnnotations; using Microsoft.AspNetCore.Components.RenderTree; using Microsoft.AspNetCore.Components.Test.Helpers; @@ -169,7 +170,7 @@ public async Task ParsesCurrentValueWhenUsingNullableInt() } [Fact] - public async Task ValidationErrorUsesDisplayAttributeName() + public async Task ValidationErrorUsesExplicitDisplayName() { // Arrange var model = new TestModel(); @@ -194,6 +195,24 @@ public async Task ValidationErrorUsesDisplayAttributeName() Assert.Contains("The Some number field is not valid.", validationMessages); } + [Fact] + public async Task ValidationErrorUsesDisplayAttributeOnModel() + { + var model = new TestModel(); + var rootComponent = new TestInputHostComponent> + { + EditContext = new EditContext(model), + ValueExpression = () => model.NotNullableInt, + }; + var fieldIdentifier = FieldIdentifier.Create(() => model.NotNullableInt); + var inputSelectComponent = await InputRenderer.RenderAndGetComponent(rootComponent); + + await inputSelectComponent.SetCurrentValueAsStringAsync("invalidNumber"); + + var validationMessages = rootComponent.EditContext.GetValidationMessages(fieldIdentifier); + Assert.Contains("The Number from attribute field is not valid.", validationMessages); + } + [Fact] public async Task InputElementIsAssignedSuccessfully() { @@ -273,6 +292,7 @@ class TestModel public Guid? NullableGuid { get; set; } + [Display(Name = "Number from attribute")] public int NotNullableInt { get; set; } public int? NullableInt { get; set; } diff --git a/src/Components/test/E2ETest/Tests/FormsTest.cs b/src/Components/test/E2ETest/Tests/FormsTest.cs index 21a982e107c1..d8d02890c6d5 100644 --- a/src/Components/test/E2ETest/Tests/FormsTest.cs +++ b/src/Components/test/E2ETest/Tests/FormsTest.cs @@ -581,6 +581,18 @@ public void DisplayNameReadsAttributesCorrectly() Browser.Equal("Product Name", () => localizedLabel.Text); } + [Fact] + public void InputNumberParsingErrorUsesDisplayNameAttribute() + { + var appElement = Browser.MountTestComponent(); + var priceInput = appElement.FindElement(By.Id("price-input")); + var messagesAccessor = CreateValidationMessagesAccessor(appElement); + priceInput.SendKeys(Keys.Control + "a"); + priceInput.SendKeys(Keys.Delete); + priceInput.SendKeys(Keys.Tab); + Browser.Equal(new[] { "The Unit Price field must be a number." }, messagesAccessor); + } + [Fact] public void InputComponentsCauseContainerToRerenderOnChange() { diff --git a/src/Components/test/E2ETest/Tests/VirtualizationTest.cs b/src/Components/test/E2ETest/Tests/VirtualizationTest.cs index 9de44f451d01..b4f1f77ebce8 100644 --- a/src/Components/test/E2ETest/Tests/VirtualizationTest.cs +++ b/src/Components/test/E2ETest/Tests/VirtualizationTest.cs @@ -151,12 +151,13 @@ public void InitialRender_DispatchesSingleSpacerCallback(bool useItemsProvider) var firstBatchCallCount = GetFirstBatchSpacerCallbackCount(); var firstBatchCall = GetFirstBatchSpacerCallback(); var args = (ReadOnlyCollection)firstBatchCall["args"]; + var reason = Convert.ToInt32(args[3], CultureInfo.InvariantCulture); Assert.Equal(1, firstBatchCallCount); Assert.Equal("OnSpacerBeforeVisible", firstBatchCall["methodName"]); Assert.Equal(0, Convert.ToDouble(args[0], CultureInfo.InvariantCulture)); Assert.Equal(400, Convert.ToDouble(args[2], CultureInfo.InvariantCulture)); - Assert.Equal(viewportFillReason, Convert.ToInt32(args[3], CultureInfo.InvariantCulture)); + Assert.Equal(viewportFillReason, reason); } finally { @@ -5503,6 +5504,277 @@ private long GetTopRenderedIndex(IJavaScriptExecutor js) private long GetScrollTop(IJavaScriptExecutor js, IWebElement container) => (long)js.ExecuteScript("return Math.round(arguments[0].scrollTop)", container); + private long GetBottomRenderedIndex(IJavaScriptExecutor js) + { + return (long)js.ExecuteScript(@" + var container = document.getElementById('scroll-container'); + var rect = container.getBoundingClientRect(); + var items = container.querySelectorAll('.item, .loading-placeholder'); + var best = null; + var bestBottom = Number.NEGATIVE_INFINITY; + for (var i = 0; i < items.length; i++) { + var ir = items[i].getBoundingClientRect(); + if (ir.top >= rect.bottom - 1) continue; // below viewport + if (ir.bottom > bestBottom) { bestBottom = ir.bottom; best = items[i]; } + } + if (!best) return -1; + if (best.classList.contains('loading-placeholder')) return -1; + var idx = best.getAttribute('data-index'); + return idx === null ? -1 : parseInt(idx, 10); + "); + } + + // Whether the given edge ('top' or 'bottom') of #scroll-container's viewport is covered by a real + // (non-placeholder) item, with no blank gap between it and the container's edge. + private bool ViewportEdgeCoveredByRealItem(IJavaScriptExecutor js, string edge) + { + return (bool)js.ExecuteScript(@" + var edge = arguments[0]; + var c = document.getElementById('scroll-container'); + var rect = c.getBoundingClientRect(); + var items = c.querySelectorAll('.item'); + var best = edge === 'top' ? Number.POSITIVE_INFINITY : Number.NEGATIVE_INFINITY; + for (var i = 0; i < items.length; i++) { + var r = items[i].getBoundingClientRect(); + if (r.bottom <= rect.top + 1) continue; // above viewport + if (r.top >= rect.bottom - 1) continue; // below viewport + if (edge === 'top') { + if (r.top < best) best = r.top; + } else { + if (r.bottom > best) best = r.bottom; + } + } + return edge === 'top' ? (best <= rect.top + 2) : (best >= rect.bottom - 2); + ", edge); + } + + [Fact] + public void InitialIndex_PendingGrowth_DoesNotExceedMaxItemCount() + { + const int maxItemCount = 20; + const int overscanCount = 3; + const int maximumRenderedItemCount = maxItemCount + 2 * overscanCount; + + Browser.MountTestComponent(); + Browser.SetWindowSize(1024, 2900); + var container = Browser.Exists(By.Id("scroll-container")); + var js = (IJavaScriptExecutor)Browser; + + js.ExecuteScript("document.getElementById('scroll-container').style.height = '2500px';"); + Browser.Exists(By.Id("set-low-max-item-count")).Click(); + Browser.Contains("MaxItemCount 20, overscan 3", () => Browser.Exists(By.Id("status")).Text); + Browser.Exists(By.Id("toggle-provider")).Click(); + Browser.Exists(By.Id("unload-list")).Click(); + Browser.Exists(By.Id("list-not-loaded")); + SetManualInitialIndex(100); + Browser.Exists(By.Id("reload-with-initial-index")).Click(); + + WaitForRenderToSettle(container, js); + var renderedItemCount = GetElementCount(container, ".item"); + + Assert.True(renderedItemCount <= maximumRenderedItemCount, + $"Expected at most {maximumRenderedItemCount} rendered items " + + $"(MaxItemCount={maxItemCount} + 2*OverscanCount={2 * overscanCount}), but found {renderedItemCount}."); + } + + [Fact] + public void InitialIndex_PendingGrowth_TallItemsBeforeTarget_KeepsTargetAligned() + { + const int initialItemIndex = 100; + + Browser.MountTestComponent(); + Browser.SetWindowSize(1024, 2400); + var container = Browser.Exists(By.Id("scroll-container")); + var js = (IJavaScriptExecutor)Browser; + + js.ExecuteScript("document.getElementById('scroll-container').style.height = '2000px';"); + Browser.Exists(By.Id("set-average-height-trap")).Click(); + Browser.Contains("Tall rows before index 100", () => Browser.Exists(By.Id("status")).Text); + Browser.Exists(By.Id("toggle-provider")).Click(); + Browser.Exists(By.Id("unload-list")).Click(); + Browser.Exists(By.Id("list-not-loaded")); + SetManualInitialIndex(initialItemIndex); + Browser.Exists(By.Id("reload-with-initial-index")).Click(); + + WaitForRenderToSettle(container, js); + + Assert.True(ViewportEdgeCoveredByRealItem(js, "bottom"), + $"Viewport bottom should be covered by a real item, but a gap was found " + + $"(top={GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + + Browser.Exists(By.CssSelector($".item[data-index='{initialItemIndex}']")); + var targetTopOffset = Convert.ToDouble(js.ExecuteScript($@" + var container = document.getElementById('scroll-container'); + var target = container.querySelector('.item[data-index=""{initialItemIndex}""]'); + return target.getBoundingClientRect().top - container.getBoundingClientRect().top; + "), CultureInfo.InvariantCulture); + + Assert.True(Math.Abs(targetTopOffset) <= 2, + $"Item {initialItemIndex} should remain aligned with the viewport top, " + + $"but its offset was {targetTopOffset}px (scrollTop={GetScrollTop(js, container)})."); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void InitialIndex_TallContainer_FillsViewportWithoutUserScroll(bool useProvider) + { + Browser.MountTestComponent(); + Browser.SetWindowSize(1024, 2400); + var container = Browser.Exists(By.Id("scroll-container")); + Browser.True(() => GetElementCount(container, ".item") > 0); + var js = (IJavaScriptExecutor)Browser; + + js.ExecuteScript("document.getElementById('scroll-container').style.height = '2000px';"); + Browser.Exists(By.Id("toggle-height")).Click(); + Browser.Contains("Switched to variable heights", () => Browser.Exists(By.Id("status")).Text); + if (useProvider) + { + Browser.Exists(By.Id("toggle-provider")).Click(); + Browser.Exists(By.Id("toggle-delay")).Click(); + } + Browser.Exists(By.Id("unload-list")).Click(); + Browser.Exists(By.Id("list-not-loaded")); + SetManualInitialIndex(10); + Browser.Exists(By.Id("reload-with-initial-index")).Click(); + + Browser.True(() => GetTopRenderedIndex(js) == 10, + $"Item 10 should remain aligned with the viewport top once the last item has loaded, " + + $"but top rendered index was {GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)}."); + Browser.True(() => ViewportEdgeCoveredByRealItem(js, "bottom"), + $"Viewport bottom should be covered by a real item, but a gap was found " + + $"(top={GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void QuickGrid_InitialIndex_TallContainer_FillsViewportWithoutUserScroll(bool useProvider) + { + Browser.MountTestComponent(); + Browser.SetWindowSize(1024, 2400); + var container = Browser.Exists(By.Id("scroll-container")); + Browser.True(() => GetElementCount(container, ".item") > 0); + var js = (IJavaScriptExecutor)Browser; + + js.ExecuteScript("document.getElementById('scroll-container').style.height = '2000px';"); + Browser.Exists(By.Id("toggle-height")).Click(); + Browser.Contains("Switched to variable heights", () => Browser.Exists(By.Id("status")).Text); + if (useProvider) + { + Browser.Exists(By.Id("toggle-provider")).Click(); + Browser.Exists(By.Id("toggle-delay")).Click(); + } + Browser.Exists(By.Id("unload-list")).Click(); + Browser.Exists(By.Id("list-not-loaded")); + SetManualInitialIndex(10); + Browser.Exists(By.Id("reload-with-initial-index")).Click(); + + Browser.True(() => GetTopRenderedIndex(js) == 10, + $"Item 10 should remain aligned with the viewport top once the last item has loaded, " + + $"but top rendered index was {GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)}."); + Browser.True(() => ViewportEdgeCoveredByRealItem(js, "bottom"), + $"Viewport bottom should be covered by a real item, but a gap was found " + + $"(top={GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void InitialIndex_TallContainer_NearEnd_FillsViewportWithoutUserScroll(bool useProvider) + { + Browser.MountTestComponent(); + Browser.SetWindowSize(1024, 2400); + var container = Browser.Exists(By.Id("scroll-container")); + Browser.True(() => GetElementCount(container, ".item") > 0); + var js = (IJavaScriptExecutor)Browser; + + js.ExecuteScript("document.getElementById('scroll-container').style.height = '2000px';"); + Browser.Exists(By.Id("toggle-height")).Click(); + Browser.Contains("Switched to variable heights", () => Browser.Exists(By.Id("status")).Text); + if (useProvider) + { + Browser.Exists(By.Id("toggle-provider")).Click(); + Browser.Exists(By.Id("toggle-delay")).Click(); + } + Browser.Exists(By.Id("unload-list")).Click(); + Browser.Exists(By.Id("list-not-loaded")); + SetManualInitialIndex(950); + Browser.Exists(By.Id("reload-with-initial-index")).Click(); + + Browser.True(() => GetTopRenderedIndex(js) == 950, + $"Item 950 should remain aligned with the viewport top once the last item has loaded, " + + $"but top rendered index was {GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)}."); + Browser.True(() => ViewportEdgeCoveredByRealItem(js, "top"), + $"Viewport top should be covered by a real item, but a gap was found " + + $"(bottom={GetBottomRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + Browser.True(() => ViewportEdgeCoveredByRealItem(js, "bottom"), + $"Viewport bottom should be covered by a real item, but a gap was found " + + $"(top={GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + } + + [Fact] + public void InitialIndex_TallContainer_ClampedNearEnd_FillsViewportWithoutUserScroll() + { + Browser.MountTestComponent(); + Browser.SetWindowSize(1024, 5400); + var container = Browser.Exists(By.Id("scroll-container")); + var js = (IJavaScriptExecutor)Browser; + + js.ExecuteScript("document.getElementById('scroll-container').style.height = '5000px';"); + Browser.Exists(By.Id("set-double-size")).Click(); + Browser.Contains("Item size 100", () => Browser.Exists(By.Id("status")).Text); + Browser.Exists(By.Id("toggle-provider")).Click(); + Browser.Exists(By.Id("unload-list")).Click(); + Browser.Exists(By.Id("list-not-loaded")); + SetManualInitialIndex(990); + Browser.Exists(By.Id("reload-with-initial-index")).Click(); + + WaitForRenderToSettle(container, js); + + Assert.True(ViewportEdgeCoveredByRealItem(js, "top"), + $"Viewport top should be covered by a real item, but a gap was found " + + $"(bottom={GetBottomRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + Assert.True(ViewportEdgeCoveredByRealItem(js, "bottom"), + $"Viewport bottom should be covered by a real item, but a gap was found " + + $"(top={GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void QuickGrid_InitialIndex_TallContainer_NearEnd_FillsViewportWithoutUserScroll(bool useProvider) + { + Browser.MountTestComponent(); + Browser.SetWindowSize(1024, 2400); + var container = Browser.Exists(By.Id("scroll-container")); + Browser.True(() => GetElementCount(container, ".item") > 0); + var js = (IJavaScriptExecutor)Browser; + + js.ExecuteScript("document.getElementById('scroll-container').style.height = '2000px';"); + Browser.Exists(By.Id("toggle-height")).Click(); + Browser.Contains("Switched to variable heights", () => Browser.Exists(By.Id("status")).Text); + if (useProvider) + { + Browser.Exists(By.Id("toggle-provider")).Click(); + Browser.Exists(By.Id("toggle-delay")).Click(); + } + Browser.Exists(By.Id("unload-list")).Click(); + Browser.Exists(By.Id("list-not-loaded")); + SetManualInitialIndex(950); + Browser.Exists(By.Id("reload-with-initial-index")).Click(); + + Browser.True(() => GetTopRenderedIndex(js) == 950, + $"Item 950 should remain aligned with the viewport top once the last item has loaded, " + + $"but top rendered index was {GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)}."); + Browser.True(() => ViewportEdgeCoveredByRealItem(js, "top"), + $"Viewport top should be covered by a real item, but a gap was found " + + $"(bottom={GetBottomRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + Browser.True(() => ViewportEdgeCoveredByRealItem(js, "bottom"), + $"Viewport bottom should be covered by a real item, but a gap was found " + + $"(top={GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + } + [Theory] [InlineData(false)] [InlineData(true)] @@ -5556,6 +5828,79 @@ public void ScrollToItem_VariableHeight_LandsAtTop(bool useProvider) $"Variable-height: top rendered item should be 200 but was {GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)}"); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public void ScrollToItem_TallItemsBeforeTarget_FillsViewportAndKeepsTargetAligned(bool useProvider) + { + const int targetIndex = 100; + + MountAnchorModeForScrollToItem(useProvider, delay: useProvider); + Browser.SetWindowSize(1024, 2400); + var container = Browser.Exists(By.Id("scroll-container")); + var js = (IJavaScriptExecutor)Browser; + + js.ExecuteScript("document.getElementById('scroll-container').style.height = '2000px';"); + Browser.Exists(By.Id("set-average-height-trap")).Click(); + Browser.Contains("Tall rows before index 100", () => Browser.Exists(By.Id("status")).Text); + + SetScrollTargetIndex(targetIndex); + Browser.Exists(By.Id("scroll-to-item")).Click(); + WaitForScrollStatus($"Completed: {targetIndex}"); + WaitForRenderToSettle(container, js); + + Assert.True(ViewportEdgeCoveredByRealItem(js, "bottom"), + $"Viewport bottom should be covered by a real item, but a gap was found " + + $"(top={GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + + var targetTopOffset = Convert.ToDouble(js.ExecuteScript($@" + var container = document.getElementById('scroll-container'); + var target = container.querySelector('.item[data-index=""{targetIndex}""]'); + return target.getBoundingClientRect().top - container.getBoundingClientRect().top; + "), CultureInfo.InvariantCulture); + + Assert.True(Math.Abs(targetTopOffset) <= 2, + $"Item {targetIndex} should remain aligned with the viewport top, " + + $"but its offset was {targetTopOffset}px (scrollTop={GetScrollTop(js, container)})."); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void ScrollToItem_TallContainer_ClampedNearEnd_FillsViewport(bool useProvider) + { + const int targetIndex = 990; + + MountAnchorModeForScrollToItem(useProvider, delay: useProvider); + Browser.SetWindowSize(1024, 5400); + var container = Browser.Exists(By.Id("scroll-container")); + var js = (IJavaScriptExecutor)Browser; + + js.ExecuteScript("document.getElementById('scroll-container').style.height = '5000px';"); + Browser.Exists(By.Id("set-double-size")).Click(); + Browser.Contains("Item size 100", () => Browser.Exists(By.Id("status")).Text); + + SetScrollTargetIndex(targetIndex); + Browser.Exists(By.Id("scroll-to-item")).Click(); + WaitForScrollStatus($"Completed: {targetIndex}"); + WaitForRenderToSettle(container, js); + + Assert.True(ViewportEdgeCoveredByRealItem(js, "top"), + $"Viewport top should be covered by a real item, but a gap was found " + + $"(bottom={GetBottomRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + Assert.True(ViewportEdgeCoveredByRealItem(js, "bottom"), + $"Viewport bottom should be covered by a real item, but a gap was found " + + $"(top={GetTopRenderedIndex(js)}, scrollTop={GetScrollTop(js, container)})."); + Assert.True((bool)js.ExecuteScript($@" + var container = document.getElementById('scroll-container'); + var viewport = container.getBoundingClientRect(); + var target = container.querySelector('.item[data-index=""{targetIndex}""]'); + if (!target) return false; + var targetRect = target.getBoundingClientRect(); + return targetRect.bottom > viewport.top && targetRect.top < viewport.bottom; + "), $"Item {targetIndex} should remain visible after the viewport fills."); + } + [Theory] [InlineData(false)] [InlineData(true)] diff --git a/src/Components/test/testassets/BasicTestApp/FormsTest/DisplayNameComponent.razor b/src/Components/test/testassets/BasicTestApp/FormsTest/DisplayNameComponent.razor index fda324c7c8a9..4db681409948 100644 --- a/src/Components/test/testassets/BasicTestApp/FormsTest/DisplayNameComponent.razor +++ b/src/Components/test/testassets/BasicTestApp/FormsTest/DisplayNameComponent.razor @@ -10,6 +10,11 @@

+ + + + + @code { private Product _product = new Product(); diff --git a/src/Components/test/testassets/BasicTestApp/VirtualizationAnchorMode.razor b/src/Components/test/testassets/BasicTestApp/VirtualizationAnchorMode.razor index 5e92148fad87..2cf5364d4716 100644 --- a/src/Components/test/testassets/BasicTestApp/VirtualizationAnchorMode.razor +++ b/src/Components/test/testassets/BasicTestApp/VirtualizationAnchorMode.razor @@ -19,7 +19,7 @@ { @if (useItemsProvider) { - +
+
Expand Item 3 to 400px + + + @@ -113,9 +116,12 @@ private string statusMessage = "Ready"; private VirtualizeAnchorMode anchorMode = VirtualizeAnchorMode.Start; private bool useVariableHeight = false; + private bool useAverageHeightTrap; private bool useItemsProvider = false; private bool omitItemComparer = false; + private int itemSize = 50; private int overscanCount = 15; + private int maxItemCount = 100; private bool useProviderDelay = false; private bool useProviderGate = false; private TaskCompletionSource providerGate = new(TaskCreationOptions.RunContinuationsAsynchronously); @@ -158,11 +164,17 @@ private int GetHeight(int index) { - return useVariableHeight ? 10 + (Math.Abs(index * 7 + 13) % 191) : 50; + if (useAverageHeightTrap) + { + return index < 100 ? 200 : 10; + } + + return useVariableHeight ? 10 + (Math.Abs(index * 7 + 13) % 191) : itemSize; } private void ToggleVariableHeight() { + useAverageHeightTrap = false; useVariableHeight = !useVariableHeight; foreach (var item in items) { @@ -301,6 +313,40 @@ if (useItemsProvider && virtualizeRef != null) { await virtualizeRef.RefreshDataAsync(); } } + private void SetLowMaxItemCount() + { + itemSize = 50; + maxItemCount = 20; + overscanCount = 3; + statusMessage = "MaxItemCount 20, overscan 3"; + } + + private void SetDoubleSize() + { + useAverageHeightTrap = false; + useVariableHeight = false; + itemSize = 100; + foreach (var item in items) + { + item.Height = itemSize; + } + statusMessage = "Item size 100"; + } + + private void SetAverageHeightTrap() + { + itemSize = 50; + maxItemCount = 300; + overscanCount = 3; + useVariableHeight = false; + useAverageHeightTrap = true; + foreach (var item in items) + { + item.Height = GetHeight(item.Index); + } + statusMessage = "Tall rows before index 100"; + } + private void ExpandVisibleItem() { var target = items.FirstOrDefault(i => i.Index == 3); diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/RemoteAuthenticationCsrfTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/RemoteAuthenticationCsrfTests.cs new file mode 100644 index 000000000000..5899c75d7b37 --- /dev/null +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/RemoteAuthenticationCsrfTests.cs @@ -0,0 +1,167 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable enable + +using System.Net; +using System.Net.Http; +using System.Text.Encodings.Web; +using Microsoft.AspNetCore.Antiforgery; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Tests; + +// A remote provider's callback (e.g. OpenID Connect response_mode=form_post) is a cross-site form POST by +// protocol design. When the callback path also matches a routed endpoint that requires antiforgery +// validation, the auto-injected CSRF middleware records an invalid verdict for it, and the handler used to +// fail while reading its own callback body - before any of its events could run. +public class RemoteAuthenticationCsrfTests +{ + [Fact] + public async Task RemoteCallback_CrossSiteFormPost_CanReadCallbackForm() + { + using var app = await CreateAppWithTokenAntiforgery(); + + var response = await app.GetTestClient().SendAsync(CreateCallbackRequest()); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("handled:2", await response.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task RemoteCallback_WhenHandlerSkipsRequest_WithTokenAntiforgery_ProtectsDownstreamEndpoint() + { + using var app = await CreateAppWithTokenAntiforgery(skipRequest: true); + + var response = await app.GetTestClient().SendAsync(CreateCallbackRequest()); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Equal("protected", await response.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task RemoteCallback_WhenHandlerSkipsRequest_RestoresAutoCsrfVerdict() + { + using var app = await CreateAppWithAutoCsrfOnly(skipRequest: true); + + var response = await app.GetTestClient().SendAsync(CreateCallbackRequest()); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Equal("protected", await response.Content.ReadAsStringAsync()); + } + + private static HttpRequestMessage CreateCallbackRequest() + { + var request = new HttpRequestMessage(HttpMethod.Post, "/signin-oidc") + { + Content = new FormUrlEncodedContent(new Dictionary + { + ["state"] = "fakestate", + ["code"] = "fakecode", + }) + }; + request.Headers.Add("Sec-Fetch-Site", "cross-site"); + return request; + } + + private static Task CreateAppWithTokenAntiforgery(bool skipRequest = false) + => CreateApp(skipRequest, useTokenAntiforgery: true); + + private static Task CreateAppWithAutoCsrfOnly(bool skipRequest = false) + => CreateApp(skipRequest, useTokenAntiforgery: false); + + private static async Task CreateApp(bool skipRequest, bool useTokenAntiforgery) + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + builder.Services.AddAuthentication("signin") + .AddScheme("signin", _ => { }) + .AddScheme("remote", o => + { + o.CallbackPath = "/signin-oidc"; + o.SignInScheme = "signin"; + o.SkipRequest = skipRequest; + }); + builder.Services.AddAuthorization(); + if (useTokenAntiforgery) + { + builder.Services.AddAntiforgery(); + } + + var app = builder.Build(); + app.UseAuthentication(); + app.UseAuthorization(); + if (useTokenAntiforgery) + { + app.UseAntiforgery(); + } + + // Stands in for a catch-all server-rendered page: it makes routing match the remote callback path, + // which is what causes the CSRF middleware to record a verdict for the callback request. + app.MapPost("/{**slug}", EnforceCsrf).WithMetadata(new RequiresValidationMetadata()); + + await app.StartAsync(); + return app; + } + + private static string EnforceCsrf(HttpContext context) + { + var feature = context.Features.Get(); + if (feature is null) + { + return "passthrough"; + } + + if (!feature.IsValid) + { + context.Response.StatusCode = StatusCodes.Status400BadRequest; + return "protected"; + } + + return "allowed"; + } + + private sealed class RequiresValidationMetadata : IAntiforgeryMetadata + { + public bool RequiresValidation => true; + } + + private sealed class NoOpHandler(IOptionsMonitor options, ILoggerFactory logger, UrlEncoder encoder) + : AuthenticationHandler(options, logger, encoder) + { + protected override Task HandleAuthenticateAsync() + => Task.FromResult(AuthenticateResult.NoResult()); + } + + private sealed class FakeRemoteOptions : RemoteAuthenticationOptions + { + public bool SkipRequest { get; set; } + } + + private sealed class FakeRemoteHandler(IOptionsMonitor options, ILoggerFactory logger, UrlEncoder encoder) + : RemoteAuthenticationHandler(options, logger, encoder) + { + protected override async Task HandleRemoteAuthenticateAsync() + { + // Mirrors OpenIdConnectHandler.HandleRemoteAuthenticateAsync: the form_post callback body is read + // before the handler raises any of its events. + var form = await Request.ReadFormAsync(Context.RequestAborted); + + if (Options.SkipRequest) + { + return HandleRequestResult.SkipHandler(); + } + + Response.StatusCode = StatusCodes.Status200OK; + await Response.WriteAsync($"handled:{form.Count}"); + return HandleRequestResult.Handle(); + } + } +} diff --git a/src/Identity/Core/src/SignInManager.cs b/src/Identity/Core/src/SignInManager.cs index 285e8f87319b..03b0dc49b3d5 100644 --- a/src/Identity/Core/src/SignInManager.cs +++ b/src/Identity/Core/src/SignInManager.cs @@ -665,8 +665,22 @@ public virtual async Task PasskeySignInAsync([StringSyntax(StringS private async Task PasskeySignInCoreAsync(string credentialJson) { + ThrowIfNoPasskeyHandler(); ArgumentException.ThrowIfNullOrEmpty(credentialJson); + var passkeyInfo = await RetrievePasskeyAuthenticationInfoAsync(); + if (passkeyInfo is null) + { + return SignInResult.Failed; + } + + if (!string.Equals(PasskeyOperations.Assertion, passkeyInfo.Operation, StringComparison.Ordinal)) + { + throw new InvalidOperationException( + $"Expected passkey operation '{PasskeyOperations.Assertion}', but got '{passkeyInfo.Operation}'. " + + $"This may indicate that you have not previously called '{nameof(SignInManager<>)}.{nameof(MakePasskeyRequestOptionsAsync)}()'."); + } + var assertionResult = await PerformPasskeyAssertionAsync(credentialJson); if (!assertionResult.Succeeded) { diff --git a/src/Identity/UI/src/Areas/Identity/Pages/V4/Account/ExternalLogin.cshtml.cs b/src/Identity/UI/src/Areas/Identity/Pages/V4/Account/ExternalLogin.cshtml.cs index 30eac04d72c8..a92c3b7964f6 100644 --- a/src/Identity/UI/src/Areas/Identity/Pages/V4/Account/ExternalLogin.cshtml.cs +++ b/src/Identity/UI/src/Areas/Identity/Pages/V4/Account/ExternalLogin.cshtml.cs @@ -207,8 +207,8 @@ public override async Task OnPostConfirmationAsync(string? return await _emailSender.SendConfirmationLinkAsync(user, Input.Email, HtmlEncoder.Default.Encode(callbackUrl)); - // If account confirmation is required, we need to show the link if we don't have a real email sender - if (_userManager.Options.SignIn.RequireConfirmedAccount) + // If confirmation is required, we need to show the link if we don't have a real email sender + if (!await _signInManager.CanSignInAsync(user)) { return RedirectToPage("./RegisterConfirmation", new { Email = Input.Email }); } diff --git a/src/Identity/UI/src/Areas/Identity/Pages/V4/Account/Register.cshtml.cs b/src/Identity/UI/src/Areas/Identity/Pages/V4/Account/Register.cshtml.cs index c8390ae7c62f..52660d8657a0 100644 --- a/src/Identity/UI/src/Areas/Identity/Pages/V4/Account/Register.cshtml.cs +++ b/src/Identity/UI/src/Areas/Identity/Pages/V4/Account/Register.cshtml.cs @@ -147,7 +147,7 @@ public override async Task OnPostAsync(string? returnUrl = null) await _emailSender.SendConfirmationLinkAsync(user, Input.Email, HtmlEncoder.Default.Encode(callbackUrl)); - if (_userManager.Options.SignIn.RequireConfirmedAccount) + if (!await _signInManager.CanSignInAsync(user)) { return RedirectToPage("RegisterConfirmation", new { email = Input.Email, returnUrl = returnUrl }); } diff --git a/src/Identity/UI/src/Areas/Identity/Pages/V5/Account/ExternalLogin.cshtml.cs b/src/Identity/UI/src/Areas/Identity/Pages/V5/Account/ExternalLogin.cshtml.cs index 0eccb938ec26..4deb95928f69 100644 --- a/src/Identity/UI/src/Areas/Identity/Pages/V5/Account/ExternalLogin.cshtml.cs +++ b/src/Identity/UI/src/Areas/Identity/Pages/V5/Account/ExternalLogin.cshtml.cs @@ -207,8 +207,8 @@ public override async Task OnPostConfirmationAsync(string? return await _emailSender.SendConfirmationLinkAsync(user, Input.Email, HtmlEncoder.Default.Encode(callbackUrl)); - // If account confirmation is required, we need to show the link if we don't have a real email sender - if (_userManager.Options.SignIn.RequireConfirmedAccount) + // If confirmation is required, we need to show the link if we don't have a real email sender + if (!await _signInManager.CanSignInAsync(user)) { return RedirectToPage("./RegisterConfirmation", new { Email = Input.Email }); } diff --git a/src/Identity/UI/src/Areas/Identity/Pages/V5/Account/Register.cshtml.cs b/src/Identity/UI/src/Areas/Identity/Pages/V5/Account/Register.cshtml.cs index b55c9eba5167..0a85709266b2 100644 --- a/src/Identity/UI/src/Areas/Identity/Pages/V5/Account/Register.cshtml.cs +++ b/src/Identity/UI/src/Areas/Identity/Pages/V5/Account/Register.cshtml.cs @@ -147,7 +147,7 @@ public override async Task OnPostAsync(string? returnUrl = null) await _emailSender.SendConfirmationLinkAsync(user, Input.Email, HtmlEncoder.Default.Encode(callbackUrl)); - if (_userManager.Options.SignIn.RequireConfirmedAccount) + if (!await _signInManager.CanSignInAsync(user)) { return RedirectToPage("RegisterConfirmation", new { email = Input.Email, returnUrl = returnUrl }); } diff --git a/src/Identity/samples/IdentitySample.DefaultUI/Areas/Identity/Pages/Account/Register.cshtml.cs b/src/Identity/samples/IdentitySample.DefaultUI/Areas/Identity/Pages/Account/Register.cshtml.cs index 4a9806e417ef..059ea8e4f8fa 100644 --- a/src/Identity/samples/IdentitySample.DefaultUI/Areas/Identity/Pages/Account/Register.cshtml.cs +++ b/src/Identity/samples/IdentitySample.DefaultUI/Areas/Identity/Pages/Account/Register.cshtml.cs @@ -101,7 +101,7 @@ public async Task OnPostAsync(string returnUrl = null) await _emailSender.SendConfirmationLinkAsync(user, Input.Email, HtmlEncoder.Default.Encode(callbackUrl)); - if (_userManager.Options.SignIn.RequireConfirmedAccount) + if (!await _signInManager.CanSignInAsync(user)) { return RedirectToPage("RegisterConfirmation", new { email = Input.Email }); } diff --git a/src/Identity/test/Identity.FunctionalTests/LoginTests.cs b/src/Identity/test/Identity.FunctionalTests/LoginTests.cs index e7caa23b72aa..05caeb216d0e 100644 --- a/src/Identity/test/Identity.FunctionalTests/LoginTests.cs +++ b/src/Identity/test/Identity.FunctionalTests/LoginTests.cs @@ -167,7 +167,7 @@ void ConfigureTestServices(IServiceCollection services) => services var userName = $"{Guid.NewGuid()}@example.com"; var password = $"[PLACEHOLDER]-1a"; - var loggedIn = await UserStories.RegisterNewUserAsync(client, userName, password); + await UserStories.RegisterNewUserAsyncWithConfirmation(client, userName, password, hasRealEmailSender: true); // Act & Assert // Use a new client to simulate a new browser session. @@ -192,7 +192,7 @@ void ConfigureTestServices(IServiceCollection services) => services var userName = $"{Guid.NewGuid()}@example.com"; var password = $"[PLACEHOLDER]-1a"; - var loggedIn = await UserStories.RegisterNewUserAsync(client, userName, password); + await UserStories.RegisterNewUserAsyncWithConfirmation(client, userName, password, hasRealEmailSender: true); // Act & Assert // Use a new client to simulate a new browser session. @@ -216,7 +216,7 @@ void ConfigureTestServices(IServiceCollection services) => services var userName = $"{Guid.NewGuid()}@example.com"; var password = $"[PLACEHOLDER]-1a"; - var loggedIn = await UserStories.RegisterNewUserAsync(client, userName, password); + await UserStories.RegisterNewUserAsyncWithConfirmation(client, userName, password, hasRealEmailSender: true); // Act & Assert // Use a new client to simulate a new browser session. @@ -243,7 +243,7 @@ void ConfigureTestServices(IServiceCollection services) => services var userName = $"{Guid.NewGuid()}@example.com"; var password = $"[PLACEHOLDER]-1a"; - var loggedIn = await UserStories.RegisterNewUserAsync(client, userName, password); + await UserStories.RegisterNewUserAsyncWithConfirmation(client, userName, password, hasRealEmailSender: true); // Act & Assert // Use a new client to simulate a new browser session. @@ -271,7 +271,7 @@ void ConfigureTestServices(IServiceCollection services) => services var userName = $"{Guid.NewGuid()}@example.com"; var password = $"[PLACEHOLDER]-1a"; - var loggedIn = await UserStories.RegisterNewUserAsync(client, userName, password); + await UserStories.RegisterNewUserAsyncWithConfirmation(client, userName, password, hasRealEmailSender: true); // Act & Assert // Use a new client to simulate a new browser session. @@ -406,6 +406,6 @@ void ConfigureTestServices(IServiceCollection services) => await UserStories.ConfirmEmailAsync(registrationEmail, client); // Act & Assert - await UserStories.LoginFailsWithWrongPasswordAsync(newClient, userName, wrongPassword); + await UserStories.LoginFailsAsync(newClient, userName, wrongPassword); } } diff --git a/src/Identity/test/Identity.FunctionalTests/ManagementTests.cs b/src/Identity/test/Identity.FunctionalTests/ManagementTests.cs index a36543132ced..b7e44ff556bd 100644 --- a/src/Identity/test/Identity.FunctionalTests/ManagementTests.cs +++ b/src/Identity/test/Identity.FunctionalTests/ManagementTests.cs @@ -104,7 +104,7 @@ void ConfigureTestServices(IServiceCollection services) => // Verify can login with new email, fails with old await UserStories.LoginExistingUserAsync(newClient, newEmail, password); - await UserStories.LoginFailsWithWrongPasswordAsync(failedClient, userName, password); + await UserStories.LoginFailsAsync(failedClient, userName, password); } diff --git a/src/Identity/test/Identity.FunctionalTests/Pages/Account/Login.cs b/src/Identity/test/Identity.FunctionalTests/Pages/Account/Login.cs index 909a1edebc74..6a5234e79e16 100644 --- a/src/Identity/test/Identity.FunctionalTests/Pages/Account/Login.cs +++ b/src/Identity/test/Identity.FunctionalTests/Pages/Account/Login.cs @@ -73,7 +73,7 @@ public async Task LoginValidUserAsync(string userName, string password) Context.WithAuthenticatedUser().WithPasswordLogin()); } - public async Task LoginWrongPasswordAsync(string userName, string password) + public async Task LoginFailsAsync(string userName, string password) { var failedLogin = await SendLoginForm(userName, password); diff --git a/src/Identity/test/Identity.FunctionalTests/RegistrationTests.cs b/src/Identity/test/Identity.FunctionalTests/RegistrationTests.cs index 6698032b3b00..5dc632e4c9b6 100644 --- a/src/Identity/test/Identity.FunctionalTests/RegistrationTests.cs +++ b/src/Identity/test/Identity.FunctionalTests/RegistrationTests.cs @@ -55,7 +55,26 @@ public async Task CanRegisterAUserWithRequiredConfirmation() var register = await UserStories.RegisterNewUserAsyncWithConfirmation(client, userName, password); // Since we aren't confirmed yet, login should fail until we confirm - await UserStories.LoginFailsWithWrongPasswordAsync(client, userName, password); + await UserStories.LoginFailsAsync(client, userName, password); + await register.ClickConfirmLinkAsync(); + await UserStories.LoginExistingUserAsync(client, userName, password); + } + + [Fact] + public async Task CanRegisterAUserWithRequiredEmailConfirmation() + { + void ConfigureTestServices(IServiceCollection services) { services.SetupEmailRequired(); }; + + var server = ServerFactory + .WithWebHostBuilder(whb => whb.ConfigureServices(ConfigureTestServices)); + var client = server.CreateClient(); + + var userName = $"{Guid.NewGuid()}@example.com"; + var password = $"[PLACEHOLDER]-1a"; + + var register = await UserStories.RegisterNewUserAsyncWithConfirmation(client, userName, password); + + await UserStories.LoginFailsAsync(client, userName, password); await register.ClickConfirmLinkAsync(); await UserStories.LoginExistingUserAsync(client, userName, password); } @@ -88,7 +107,7 @@ void ConfigureTestServices(IServiceCollection services) var register = await UserStories.RegisterNewUserAsyncWithConfirmation(client, userName, password, hasRealEmailSender: true); // Since we aren't confirmed yet, login should fail until we confirm - await UserStories.LoginFailsWithWrongPasswordAsync(client, userName, password); + await UserStories.LoginFailsAsync(client, userName, password); } [Fact] @@ -151,6 +170,25 @@ void ConfigureTestServices(IServiceCollection services) await UserStories.RegisterNewUserWithSocialLoginWithConfirmationAsync(client, userName, email); } + [Fact] + public async Task CanRegisterWithASocialLoginProviderFromLoginWithRequiredEmailConfirmation() + { + void ConfigureTestServices(IServiceCollection services) => + services + .SetupEmailRequired() + .SetupTestThirdPartyLogin(); + + var client = ServerFactory + .WithWebHostBuilder(whb => whb.ConfigureServices(ConfigureTestServices)) + .CreateClient(); + + var guid = Guid.NewGuid(); + var userName = $"{guid}"; + var email = $"{guid}@example.com"; + + await UserStories.RegisterNewUserWithSocialLoginWithConfirmationAsync(client, userName, email); + } + [Fact] public async Task CanRegisterWithASocialLoginProviderFromLoginWithConfirmationAndRealEmailSender() { diff --git a/src/Identity/test/Identity.FunctionalTests/UserStories.cs b/src/Identity/test/Identity.FunctionalTests/UserStories.cs index ce85a9704138..8b6ee9d36760 100644 --- a/src/Identity/test/Identity.FunctionalTests/UserStories.cs +++ b/src/Identity/test/Identity.FunctionalTests/UserStories.cs @@ -44,13 +44,13 @@ internal static async Task LoginExistingUserAsync(HttpClient client, stri return await login.LoginValidUserAsync(userName, password); } - internal static async Task LoginFailsWithWrongPasswordAsync(HttpClient client, string userName, string password) + internal static async Task LoginFailsAsync(HttpClient client, string userName, string password) { var index = await Index.CreateAsync(client); var login = await index.ClickLoginLinkAsync(); - await login.LoginWrongPasswordAsync(userName, password); + await login.LoginFailsAsync(userName, password); } internal static async Task LockoutExistingUserAsync(HttpClient client, string userName, string password) diff --git a/src/Identity/test/Identity.Test/SignInManagerTest.cs b/src/Identity/test/Identity.Test/SignInManagerTest.cs index 2dcba35b6fd0..f6d9c3159c5f 100644 --- a/src/Identity/test/Identity.Test/SignInManagerTest.cs +++ b/src/Identity/test/Identity.Test/SignInManagerTest.cs @@ -648,6 +648,36 @@ public async Task PasskeySignInReturnsLockedOutWhenLockedOut() auth.Verify(); } + [Fact] + public async Task PasskeySignInReturnsFailedWhenSessionChallengeHasExpired() + { + // Setup + var user = new PocoUser { UserName = "Foo" }; + var passkeyHandler = new Mock>(); + var manager = SetupUserManager(user, passkeyHandler: passkeyHandler.Object); + var context = new DefaultHttpContext(); + var auth = MockAuth(context); + + // Do NOT call SetupPasskeyAuth — simulates expired/missing session + auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.TwoFactorUserIdScheme)) + .ReturnsAsync(AuthenticateResult.Fail("Session expired.")) + .Verifiable(); + auth.Setup(a => a.SignOutAsync(context, IdentityConstants.TwoFactorUserIdScheme, It.IsAny())) + .Returns(Task.CompletedTask) + .Verifiable(); + + var helper = SetupSignInManager(manager.Object, context); + + // Act + var signInResult = await helper.PasskeySignInAsync(credentialJson: ""); + + // Assert + Assert.False(signInResult.Succeeded); + Assert.Same(SignInResult.Failed, signInResult); + passkeyHandler.Verify(h => h.PerformAssertionAsync(It.IsAny()), Times.Never); + auth.Verify(); + } + private static void SetupPasskeyAuth(HttpContext context, Mock auth) { // Calling AuthenticateAsync will return a failure result diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs index f6f5bed77f60..6d7ae22449b4 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs @@ -164,7 +164,14 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed context.Features.Set(exceptionHandlerFeature); context.Features.Set(exceptionHandlerFeature); - context.Response.StatusCode = _options.StatusCodeSelector?.Invoke(edi.SourceException) ?? DefaultStatusCode; + var is404FromBadHttpRequestException = edi.SourceException is + BadHttpRequestException { StatusCode: StatusCodes.Status404NotFound }; + context.Response.StatusCode = _options.StatusCodeSelector?.Invoke(edi.SourceException) + ?? (edi.SourceException switch + { + BadHttpRequestException badHttpRequestException => badHttpRequestException.StatusCode, + _ => DefaultStatusCode, + }); context.Response.OnStarting(_clearCacheHeadersDelegate, context.Response); string? handlerTag = null; @@ -215,7 +222,7 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed } } - if (result != ExceptionHandledType.Unhandled || _options.StatusCodeSelector != null || context.Response.StatusCode != StatusCodes.Status404NotFound || _options.AllowStatusCode404Response) + if (result != ExceptionHandledType.Unhandled || _options.StatusCodeSelector is not null || is404FromBadHttpRequestException || context.Response.StatusCode != StatusCodes.Status404NotFound || _options.AllowStatusCode404Response) { var suppressDiagnostics = false; diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerOptions.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerOptions.cs index a38352d93715..52261ff2b407 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerOptions.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerOptions.cs @@ -45,7 +45,8 @@ public class ExceptionHandlerOptions /// Gets or sets a delegate used to map an exception to an HTTP status code. /// /// - /// If is null, the default exception status code 500 is used. + /// If is null, the default exception status code is 500. If the exception + /// is a , its is used instead. /// public Func? StatusCodeSelector { get; set; } diff --git a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs index 0fb82da9b733..1c834bb97eb9 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs @@ -113,6 +113,71 @@ public async Task Invoke_ExceptionThrownResultsInClearedRouteValuesAndEndpoint() Assert.Collection(sink.Writes, w => Assert.Equal("UnhandledException", w.EventId.Name)); } + [Theory] + [InlineData(StatusCodes.Status400BadRequest)] + [InlineData(StatusCodes.Status404NotFound)] // Does not require AllowStatusCode404Response. + [InlineData(StatusCodes.Status418ImATeapot)] + public async Task Invoke_BadHttpRequestException_PreservesStatusCode(int statusCode) + { + var httpContext = CreateHttpContext(); + var optionsAccessor = CreateOptionsAccessor(); + var middleware = CreateMiddleware(_ => throw new BadHttpRequestException("Bad request.", statusCode), optionsAccessor); + + await middleware.Invoke(httpContext); + + Assert.Equal(statusCode, httpContext.Response.StatusCode); + } + + [Fact] + public async Task Invoke_BadHttpRequestException_StatusCodeSelectorTakesPrecedence() + { + var httpContext = CreateHttpContext(); + var optionsAccessor = CreateOptionsAccessor(statusCodeSelector: _ => StatusCodes.Status409Conflict); + var middleware = CreateMiddleware( + _ => throw new BadHttpRequestException("Bad request.", StatusCodes.Status418ImATeapot), + optionsAccessor); + + await middleware.Invoke(httpContext); + + Assert.Equal(StatusCodes.Status409Conflict, httpContext.Response.StatusCode); + } + + [Fact] + public async Task Invoke_BadHttpRequestException_ExceptionHandlerDelegateCanOverrideStatusCode() + { + var httpContext = CreateHttpContext(); + var optionsAccessor = CreateOptionsAccessor(exceptionHandler: context => + { + context.Response.StatusCode = StatusCodes.Status422UnprocessableEntity; + return Task.CompletedTask; + }); + var middleware = CreateMiddleware( + _ => throw new BadHttpRequestException("Bad request.", StatusCodes.Status418ImATeapot), + optionsAccessor); + + await middleware.Invoke(httpContext); + + Assert.Equal(StatusCodes.Status422UnprocessableEntity, httpContext.Response.StatusCode); + } + + [Fact] + public async Task Invoke_BadHttpRequestException_Non404StatusDoesNotBypass404ResponseGuard() + { + var httpContext = CreateHttpContext(); + var optionsAccessor = CreateOptionsAccessor(exceptionHandler: context => + { + context.Response.StatusCode = StatusCodes.Status404NotFound; + return Task.CompletedTask; + }); + var middleware = CreateMiddleware( + _ => throw new BadHttpRequestException("Bad request.", StatusCodes.Status400BadRequest), + optionsAccessor); + + var exception = await Assert.ThrowsAsync(() => middleware.Invoke(httpContext)); + + Assert.IsType(exception.InnerException); + } + [Theory] [InlineData(ExceptionHandledType.ExceptionHandlerDelegate, false)] [InlineData(ExceptionHandledType.ProblemDetailsService, true)] @@ -664,13 +729,15 @@ private HttpContext CreateHttpContext() private IOptions CreateOptionsAccessor( RequestDelegate exceptionHandler = null, string exceptionHandlingPath = null, - Func suppressDiagnosticsCallback = null) + Func suppressDiagnosticsCallback = null, + Func statusCodeSelector = null) { exceptionHandler ??= c => Task.CompletedTask; var options = new ExceptionHandlerOptions() { ExceptionHandler = exceptionHandler, ExceptionHandlingPath = exceptionHandlingPath, + StatusCodeSelector = statusCodeSelector, }; if (suppressDiagnosticsCallback != null) { diff --git a/src/Middleware/OutputCaching/src/Microsoft.AspNetCore.OutputCaching.csproj b/src/Middleware/OutputCaching/src/Microsoft.AspNetCore.OutputCaching.csproj index eb59546ea510..8454266f78d4 100644 --- a/src/Middleware/OutputCaching/src/Microsoft.AspNetCore.OutputCaching.csproj +++ b/src/Middleware/OutputCaching/src/Microsoft.AspNetCore.OutputCaching.csproj @@ -22,4 +22,8 @@ + + + + diff --git a/src/Middleware/OutputCaching/src/Policies/DefaultPolicy.cs b/src/Middleware/OutputCaching/src/Policies/DefaultPolicy.cs index 1f633dbaff83..ecfef9a7052e 100644 --- a/src/Middleware/OutputCaching/src/Policies/DefaultPolicy.cs +++ b/src/Middleware/OutputCaching/src/Policies/DefaultPolicy.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Internal; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.OutputCaching; @@ -50,7 +51,7 @@ ValueTask IOutputCachePolicy.ServeResponseAsync(OutputCacheContext context, Canc return ValueTask.CompletedTask; } - if (context.HttpContext.User?.Identity?.IsAuthenticated == true) + if (SecurityHelper.IsAuthenticated(context.HttpContext.User)) { context.AllowCacheStorage = false; return ValueTask.CompletedTask; @@ -79,7 +80,7 @@ private static bool AttemptOutputCaching(OutputCacheContext context) } // Verify existence of authorization headers - if (!StringValues.IsNullOrEmpty(request.Headers.Authorization) || request.HttpContext.User?.Identity?.IsAuthenticated == true) + if (!StringValues.IsNullOrEmpty(request.Headers.Authorization) || SecurityHelper.IsAuthenticated(request.HttpContext.User)) { return false; } diff --git a/src/Middleware/OutputCaching/test/OutputCachePolicyProviderTests.cs b/src/Middleware/OutputCaching/test/OutputCachePolicyProviderTests.cs index a91138df1422..323ba2035fd7 100644 --- a/src/Middleware/OutputCaching/test/OutputCachePolicyProviderTests.cs +++ b/src/Middleware/OutputCaching/test/OutputCachePolicyProviderTests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Security.Claims; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging.Testing; using Microsoft.Net.Http.Headers; @@ -89,6 +90,65 @@ public async Task AttemptResponseCaching_AuthorizationHeaders_NotAllowed() Assert.False(context.AllowCacheLookup); } + [Fact] + public async Task AttemptOutputCaching_AuthenticatedUser_NotAllowed() + { + var sink = new TestSink(); + var context = TestUtils.CreateTestContext(testSink: sink); + context.HttpContext.Request.Method = HttpMethods.Get; + context.HttpContext.User = new ClaimsPrincipal(new ClaimsIdentity(authenticationType: "custom")); + + var policy = new OutputCachePolicyBuilder().Build(); + + await policy.CacheRequestAsync(context, default); + + Assert.False(context.AllowCacheStorage); + Assert.False(context.AllowCacheLookup); + } + + [Fact] + public async Task AttemptOutputCaching_AuthenticatedByNonPrimaryIdentity_NotAllowed() + { + var sink = new TestSink(); + var context = TestUtils.CreateTestContext(testSink: sink); + context.HttpContext.Request.Method = HttpMethods.Get; + // The primary identity is unauthenticated, but a later identity is authenticated. Authorization + // treats this principal as authenticated, so output caching must classify it the same way. + context.HttpContext.User = new ClaimsPrincipal(new[] + { + new ClaimsIdentity(), + new ClaimsIdentity(authenticationType: "custom"), + }); + + Assert.False(context.HttpContext.User.Identity?.IsAuthenticated); + + var policy = new OutputCachePolicyBuilder().Build(); + + await policy.CacheRequestAsync(context, default); + + Assert.False(context.AllowCacheStorage); + Assert.False(context.AllowCacheLookup); + } + + [Fact] + public async Task IsResponseCacheable_AuthenticatedByNonPrimaryIdentity_NotAllowed() + { + var sink = new TestSink(); + var context = TestUtils.CreateTestContext(testSink: sink); + context.HttpContext.User = new ClaimsPrincipal(new[] + { + new ClaimsIdentity(), + new ClaimsIdentity(authenticationType: "custom"), + }); + + Assert.False(context.HttpContext.User.Identity?.IsAuthenticated); + + var policy = new OutputCachePolicyBuilder().Build(); + await policy.ServeResponseAsync(context, default); + + Assert.False(context.AllowCacheStorage); + } + [Fact] public async Task AllowCacheStorage_NoStore_IsAllowed() { diff --git a/src/OpenApi/sample/Endpoints/MapUnionsEndpoints.cs b/src/OpenApi/sample/Endpoints/MapUnionsEndpoints.cs index a7d473c56e52..d9f616047af0 100644 --- a/src/OpenApi/sample/Endpoints/MapUnionsEndpoints.cs +++ b/src/OpenApi/sample/Endpoints/MapUnionsEndpoints.cs @@ -44,6 +44,9 @@ public static IEndpointRouteBuilder MapUnionsEndpoints(this IEndpointRouteBuilde // used directly elsewhere registers as a separate component schema. unions.MapGet("/kitten-standalone", () => new Kitten("Whiskers", 9)); + // Container record whose property is a *nullable* union with a polymorphic case type. + unions.MapGet("/beasts", () => new BeastEnvelope(new OneOrManyBeast(new Dachshund("Chili")))); + return endpointRouteBuilder; } } diff --git a/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs b/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs index f25fe33666ed..ca4d3abdbf87 100644 --- a/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs +++ b/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs @@ -476,7 +476,17 @@ internal static void ApplySchemaReferenceId(this JsonNode schema, JsonSchemaExpo { schema[OpenApiConstants.SchemaId] = schemaReferenceId; } - if (context.TypeInfo.Kind == JsonTypeInfoKind.Union) + + // C# union types are value types, so in case of Nullable JsonTypeInfoKind is None. + // We need to unpack it to properly detect the union: see https://github.com/dotnet/aspnetcore/issues/68653. + var unionTypeInfo = context.TypeInfo; + if (Nullable.GetUnderlyingType(unionTypeInfo.Type) is { } underlyingType + && unionTypeInfo.Options.TryGetTypeInfo(underlyingType, out var underlyingTypeInfo)) + { + unionTypeInfo = underlyingTypeInfo; + } + + if (unionTypeInfo.Kind == JsonTypeInfoKind.Union) { schema[OpenApiConstants.SchemaIsUnion] = true; } diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_0/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_0/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt index 7aac8ea450ee..68a83791f6e7 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_0/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_0/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt @@ -202,10 +202,105 @@ } } } + }, + "/unions/beasts": { + "get": { + "tags": [ + "Sample" + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/BeastEnvelope" + } + } + } + } + } + } } }, "components": { "schemas": { + "Beast": { + "required": [ + "type" + ], + "type": "object", + "anyOf": [ + { + "$ref": "#/components/schemas/BeastLion" + }, + { + "$ref": "#/components/schemas/BeastDachshund" + } + ], + "discriminator": { + "propertyName": "type", + "mapping": { + "lion": "#/components/schemas/BeastLion", + "dachshund": "#/components/schemas/BeastDachshund" + } + } + }, + "BeastDachshund": { + "required": [ + "name" + ], + "type": "object", + "properties": { + "type": { + "enum": [ + "dachshund" + ], + "type": "string" + }, + "name": { + "type": "string" + } + } + }, + "BeastEnvelope": { + "required": [ + "beasts" + ], + "type": "object", + "properties": { + "beasts": { + "oneOf": [ + { + "enum": [ + null + ], + "nullable": true + }, + { + "$ref": "#/components/schemas/OneOrManyBeast" + } + ] + } + } + }, + "BeastLion": { + "required": [ + "name" + ], + "type": "object", + "properties": { + "type": { + "enum": [ + "lion" + ], + "type": "string" + }, + "name": { + "type": "string" + } + } + }, "Clinic": { "required": [ "address", @@ -237,6 +332,19 @@ } } }, + "OneOrManyBeast": { + "anyOf": [ + { + "$ref": "#/components/schemas/Beast" + }, + { + "type": "array", + "items": { + "$ref": "#/components/schemas/Beast" + } + } + ] + }, "Puppy": { "required": [ "name", diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_1/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_1/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt index 844a9ccdd0a1..02426ed79a00 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_1/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_1/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt @@ -202,10 +202,102 @@ } } } + }, + "/unions/beasts": { + "get": { + "tags": [ + "Sample" + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/BeastEnvelope" + } + } + } + } + } + } } }, "components": { "schemas": { + "Beast": { + "required": [ + "type" + ], + "type": "object", + "anyOf": [ + { + "$ref": "#/components/schemas/BeastLion" + }, + { + "$ref": "#/components/schemas/BeastDachshund" + } + ], + "discriminator": { + "propertyName": "type", + "mapping": { + "lion": "#/components/schemas/BeastLion", + "dachshund": "#/components/schemas/BeastDachshund" + } + } + }, + "BeastDachshund": { + "required": [ + "name" + ], + "type": "object", + "properties": { + "type": { + "enum": [ + "dachshund" + ], + "type": "string" + }, + "name": { + "type": "string" + } + } + }, + "BeastEnvelope": { + "required": [ + "beasts" + ], + "type": "object", + "properties": { + "beasts": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/OneOrManyBeast" + } + ] + } + } + }, + "BeastLion": { + "required": [ + "name" + ], + "type": "object", + "properties": { + "type": { + "enum": [ + "lion" + ], + "type": "string" + }, + "name": { + "type": "string" + } + } + }, "Clinic": { "required": [ "address", @@ -237,6 +329,19 @@ } } }, + "OneOrManyBeast": { + "anyOf": [ + { + "$ref": "#/components/schemas/Beast" + }, + { + "type": "array", + "items": { + "$ref": "#/components/schemas/Beast" + } + } + ] + }, "Puppy": { "required": [ "name", diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_2/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_2/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt index 4feefa691b8b..de98ef7c6c4e 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_2/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_2/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=unions.verified.txt @@ -202,10 +202,102 @@ } } } + }, + "/unions/beasts": { + "get": { + "tags": [ + "Sample" + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/BeastEnvelope" + } + } + } + } + } + } } }, "components": { "schemas": { + "Beast": { + "required": [ + "type" + ], + "type": "object", + "anyOf": [ + { + "$ref": "#/components/schemas/BeastLion" + }, + { + "$ref": "#/components/schemas/BeastDachshund" + } + ], + "discriminator": { + "propertyName": "type", + "mapping": { + "lion": "#/components/schemas/BeastLion", + "dachshund": "#/components/schemas/BeastDachshund" + } + } + }, + "BeastDachshund": { + "required": [ + "name" + ], + "type": "object", + "properties": { + "type": { + "enum": [ + "dachshund" + ], + "type": "string" + }, + "name": { + "type": "string" + } + } + }, + "BeastEnvelope": { + "required": [ + "beasts" + ], + "type": "object", + "properties": { + "beasts": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/OneOrManyBeast" + } + ] + } + } + }, + "BeastLion": { + "required": [ + "name" + ], + "type": "object", + "properties": { + "type": { + "enum": [ + "lion" + ], + "type": "string" + }, + "name": { + "type": "string" + } + } + }, "Clinic": { "required": [ "address", @@ -237,6 +329,19 @@ } } }, + "OneOrManyBeast": { + "anyOf": [ + { + "$ref": "#/components/schemas/Beast" + }, + { + "type": "array", + "items": { + "$ref": "#/components/schemas/Beast" + } + } + ] + }, "Puppy": { "required": [ "name", diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentLocalizationTests.VerifyOpenApiDocumentIsInvariant.verified.txt b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentLocalizationTests.VerifyOpenApiDocumentIsInvariant.verified.txt index 5ba2af19482a..f9e9eada5d50 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentLocalizationTests.VerifyOpenApiDocumentIsInvariant.verified.txt +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentLocalizationTests.VerifyOpenApiDocumentIsInvariant.verified.txt @@ -2297,6 +2297,25 @@ } } }, + "/unions/beasts": { + "get": { + "tags": [ + "Sample" + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/BeastEnvelope" + } + } + } + } + } + } + }, "/obsolete/deprecated": { "post": { "tags": [ @@ -2783,6 +2802,79 @@ } } }, + "Beast": { + "required": [ + "type" + ], + "type": "object", + "anyOf": [ + { + "$ref": "#/components/schemas/BeastLion" + }, + { + "$ref": "#/components/schemas/BeastDachshund" + } + ], + "discriminator": { + "propertyName": "type", + "mapping": { + "lion": "#/components/schemas/BeastLion", + "dachshund": "#/components/schemas/BeastDachshund" + } + } + }, + "BeastDachshund": { + "required": [ + "name" + ], + "type": "object", + "properties": { + "type": { + "enum": [ + "dachshund" + ], + "type": "string" + }, + "name": { + "type": "string" + } + } + }, + "BeastEnvelope": { + "required": [ + "beasts" + ], + "type": "object", + "properties": { + "beasts": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/OneOrManyBeast" + } + ] + } + } + }, + "BeastLion": { + "required": [ + "name" + ], + "type": "object", + "properties": { + "type": { + "enum": [ + "lion" + ], + "type": "string" + }, + "name": { + "type": "string" + } + } + }, "BoardItem": { "required": [ "name" @@ -3386,6 +3478,19 @@ }, "deprecated": true }, + "OneOrManyBeast": { + "anyOf": [ + { + "$ref": "#/components/schemas/Beast" + }, + { + "type": "array", + "items": { + "$ref": "#/components/schemas/Beast" + } + } + ] + }, "ParentObject": { "type": "object", "properties": { diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.UnionSchemas.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.UnionSchemas.cs index 330278fca505..6976c505dbeb 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.UnionSchemas.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.UnionSchemas.cs @@ -146,4 +146,98 @@ await VerifyOpenApiDocument(builder, document => Assert.Equal(2, unionComponent.AnyOf.Count); }); } + + [Fact] + public async Task GetOpenApiResponse_UnionWithPolymorphicCase_ReturnedDirectly_BranchesRefPolymorphicBase() + { + var builder = CreateBuilder(); + + builder.MapGet("/api/beasts", () => new OneOrManyBeast(new Dachshund("Chili"))); + + await VerifyOpenApiDocument(builder, document => + { + Assert.True(document.Components.Schemas.TryGetValue(nameof(OneOrManyBeast), out var unionComponent)); + Assert.NotNull(unionComponent.AnyOf); + Assert.Equal(2, unionComponent.AnyOf.Count); + + var objectBranch = Assert.IsType(unionComponent.AnyOf[0]); + Assert.Equal(nameof(Beast), objectBranch.Reference.Id); + + var arrayItems = Assert.IsType(unionComponent.AnyOf[1].Items); + Assert.Equal(nameof(Beast), arrayItems.Reference.Id); + + Assert.DoesNotContain(nameof(OneOrManyBeast) + nameof(Beast), document.Components.Schemas.Keys); + }); + } + + [Fact] + public async Task GetOpenApiResponse_NestedUnionWithPolymorphicCase_DoesNotDuplicatePolymorphicComponent() + { + var builder = CreateBuilder(); + + builder.MapGet("/api/beasts", () => new BeastEnvelope(new OneOrManyBeast(new Dachshund("Chili")))); + + await VerifyOpenApiDocument(builder, document => + { + Assert.True(document.Components.Schemas.TryGetValue(nameof(OneOrManyBeast), out var unionComponent)); + Assert.NotNull(unionComponent.AnyOf); + Assert.Equal(2, unionComponent.AnyOf.Count); + + // Both branches must reference the polymorphic base component `Beast`; the object + // case must not be lifted into a duplicate `OneOrManyBeastBeast` component. + var objectBranch = Assert.IsType(unionComponent.AnyOf[0]); + Assert.Equal(nameof(Beast), objectBranch.Reference.Id); + + var arrayItems = Assert.IsType(unionComponent.AnyOf[1].Items); + Assert.Equal(nameof(Beast), arrayItems.Reference.Id); + + Assert.DoesNotContain(nameof(OneOrManyBeast) + nameof(Beast), document.Components.Schemas.Keys); + }); + } + + [Fact] + public async Task GetOpenApiResponse_NestedNonNullableUnionWithPolymorphicCase_DoesNotDuplicatePolymorphicComponent() + { + var builder = CreateBuilder(); + + builder.MapGet("/api/beasts", () => new BeastContainer(new OneOrManyBeast(new Dachshund("Chili")))); + + await VerifyOpenApiDocument(builder, document => + { + Assert.True(document.Components.Schemas.TryGetValue(nameof(OneOrManyBeast), out var unionComponent)); + Assert.NotNull(unionComponent.AnyOf); + Assert.Equal(2, unionComponent.AnyOf.Count); + + var objectBranch = Assert.IsType(unionComponent.AnyOf[0]); + Assert.Equal(nameof(Beast), objectBranch.Reference.Id); + + var arrayItems = Assert.IsType(unionComponent.AnyOf[1].Items); + Assert.Equal(nameof(Beast), arrayItems.Reference.Id); + + Assert.DoesNotContain(nameof(OneOrManyBeast) + nameof(Beast), document.Components.Schemas.Keys); + }); + } + + [Fact] + public async Task GetOpenApiResponse_TopLevelNullableUnionWithPolymorphicCase_DoesNotDuplicatePolymorphicComponent() + { + var builder = CreateBuilder(); + + builder.MapGet("/api/beasts", OneOrManyBeast? () => new OneOrManyBeast(new Dachshund("Chili"))); + + await VerifyOpenApiDocument(builder, document => + { + Assert.True(document.Components.Schemas.TryGetValue(nameof(OneOrManyBeast), out var unionComponent)); + Assert.NotNull(unionComponent.AnyOf); + Assert.Equal(2, unionComponent.AnyOf.Count); + + var objectBranch = Assert.IsType(unionComponent.AnyOf[0]); + Assert.Equal(nameof(Beast), objectBranch.Reference.Id); + + var arrayItems = Assert.IsType(unionComponent.AnyOf[1].Items); + Assert.Equal(nameof(Beast), arrayItems.Reference.Id); + + Assert.DoesNotContain(nameof(OneOrManyBeast) + nameof(Beast), document.Components.Schemas.Keys); + }); + } } diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Shared/SharedTypes.Unions.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Shared/SharedTypes.Unions.cs index 0cb7de1a45be..830d77b10b05 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Shared/SharedTypes.Unions.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Shared/SharedTypes.Unions.cs @@ -6,6 +6,8 @@ // primitive-paired and object-cased unions, plus a container record that references a union // to validate component schema reuse. +using System.Text.Json.Serialization; + internal record Kitten(string Name, int Lives); internal record Puppy(string Name, string Breed); @@ -15,3 +17,19 @@ internal record Puppy(string Name, string Breed); internal union UnionIntString(int, string); internal record Clinic(string Address, UnionPet Patient); + +// Union with a polymorphic case type plus a collection of the same polymorphic type. +[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")] +[JsonDerivedType(typeof(Lion), "lion")] +[JsonDerivedType(typeof(Dachshund), "dachshund")] +internal abstract record Beast(string Name); + +internal sealed record Lion(string Name) : Beast(Name); + +internal sealed record Dachshund(string Name) : Beast(Name); + +internal union OneOrManyBeast(Beast, IReadOnlyList); + +internal sealed record BeastEnvelope(OneOrManyBeast? Beasts); + +internal sealed record BeastContainer(OneOrManyBeast Beasts); diff --git a/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Account/Pages/ExternalLogin.razor b/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Account/Pages/ExternalLogin.razor index a8a386f32a4a..634ff782e15b 100644 --- a/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Account/Pages/ExternalLogin.razor +++ b/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Account/Pages/ExternalLogin.razor @@ -177,8 +177,8 @@ new Dictionary { ["userId"] = userId, ["code"] = code }); await EmailSender.SendConfirmationLinkAsync(user, Input.Email, HtmlEncoder.Default.Encode(callbackUrl)); - // If account confirmation is required, we need to show the link if we don't have a real email sender - if (UserManager.Options.SignIn.RequireConfirmedAccount) + // If confirmation is required, we need to show the link if we don't have a real email sender + if (!await SignInManager.CanSignInAsync(user)) { RedirectManager.RedirectTo("Account/RegisterConfirmation", new() { ["email"] = Input.Email }); } diff --git a/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Account/Pages/Register.razor b/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Account/Pages/Register.razor index fe9296f4a474..ac65ddf499d2 100644 --- a/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Account/Pages/Register.razor +++ b/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Account/Pages/Register.razor @@ -102,7 +102,7 @@ await EmailSender.SendConfirmationLinkAsync(user, Input.Email, HtmlEncoder.Default.Encode(callbackUrl)); - if (UserManager.Options.SignIn.RequireConfirmedAccount) + if (!await SignInManager.CanSignInAsync(user)) { RedirectManager.RedirectTo( "Account/RegisterConfirmation", diff --git a/src/ProjectTemplates/test/Templates.Blazor.Tests/BlazorWebTemplateTest.cs b/src/ProjectTemplates/test/Templates.Blazor.Tests/BlazorWebTemplateTest.cs index f6789caa76ff..1c9579d5c2ad 100644 --- a/src/ProjectTemplates/test/Templates.Blazor.Tests/BlazorWebTemplateTest.cs +++ b/src/ProjectTemplates/test/Templates.Blazor.Tests/BlazorWebTemplateTest.cs @@ -64,6 +64,34 @@ public async Task BlazorWebTemplate_CanUsePasskeys(BrowserKind browserKind) await TestProjectCoreAsync(project, browserKind, pagesToExclude, authenticationFeatures); } + [Theory] + [InlineData(BrowserKind.Chromium)] + public async Task BlazorWebTemplate_CanRequireConfirmedEmail(BrowserKind browserKind) + { + var project = await CreateBuildPublishAsync( + args: ["-int", "None", "-au", "Individual"], + onlyCreate: true); + + var programPath = Path.Combine(project.TemplateOutputDir, "Program.cs"); + var program = await File.ReadAllTextAsync(programPath); + const string requireConfirmedAccount = "options.SignIn.RequireConfirmedAccount = true;"; + Assert.Contains(requireConfirmedAccount, program); + program = program.Replace( + requireConfirmedAccount, + "options.SignIn.RequireConfirmedEmail = true;", + StringComparison.Ordinal); + await File.WriteAllTextAsync(programPath, program); + + await project.RunDotNetPublishAsync(noRestore: false); + await project.RunDotNetBuildAsync(); + + await TestProjectCoreAsync( + project, + browserKind, + BlazorTemplatePages.Counter, + AuthenticationFeatures.RegisterAndLogIn); + } + private async Task TestProjectCoreAsync(Project project, BrowserKind browserKind, BlazorTemplatePages pagesToExclude, AuthenticationFeatures authenticationFeatures) { var appName = project.ProjectName; diff --git a/src/Security/Authentication/Core/src/Microsoft.AspNetCore.Authentication.csproj b/src/Security/Authentication/Core/src/Microsoft.AspNetCore.Authentication.csproj index 0a9f4a34f0dd..65c808aeb083 100644 --- a/src/Security/Authentication/Core/src/Microsoft.AspNetCore.Authentication.csproj +++ b/src/Security/Authentication/Core/src/Microsoft.AspNetCore.Authentication.csproj @@ -12,6 +12,7 @@ + diff --git a/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs b/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs index 203f17740063..0119ad1493b1 100644 --- a/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs +++ b/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Security.Cryptography; using System.Text.Encodings.Web; +using Microsoft.AspNetCore.Antiforgery; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -79,6 +80,11 @@ public virtual async Task HandleRequestAsync() return false; } + return await RemoteAuthenticationAntiforgery.HandleWithoutAntiforgeryVerdictAsync(Context, HandleRequestCoreAsync); + } + + private async Task HandleRequestCoreAsync() + { AuthenticationTicket? ticket = null; Exception? exception = null; AuthenticationProperties? properties = null; diff --git a/src/Security/Authentication/OpenIdConnect/src/Microsoft.AspNetCore.Authentication.OpenIdConnect.csproj b/src/Security/Authentication/OpenIdConnect/src/Microsoft.AspNetCore.Authentication.OpenIdConnect.csproj index 08d735332136..8de8159a9a01 100644 --- a/src/Security/Authentication/OpenIdConnect/src/Microsoft.AspNetCore.Authentication.OpenIdConnect.csproj +++ b/src/Security/Authentication/OpenIdConnect/src/Microsoft.AspNetCore.Authentication.OpenIdConnect.csproj @@ -19,6 +19,7 @@ + diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index 525f18a1050e..6e64f3d3aebb 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -11,6 +11,7 @@ using System.Text; using System.Text.Encodings.Web; using System.Text.Json; +using Microsoft.AspNetCore.Antiforgery; using Microsoft.AspNetCore.Authentication.OAuth; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.WebUtilities; @@ -88,13 +89,16 @@ public OpenIdConnectHandler(IOptionsMonitor options, ILogg /// public override Task HandleRequestAsync() { + // Both paths below are, like the sign-in callback, cross-site requests owned by this handler, and + // HandleRemoteSignOutAsync reads a form_post body, so they need the same antiforgery verdict handling + // that RemoteAuthenticationHandler applies to CallbackPath. if (Options.RemoteSignOutPath.HasValue && Options.RemoteSignOutPath == Request.Path) { - return HandleRemoteSignOutAsync(); + return RemoteAuthenticationAntiforgery.HandleWithoutAntiforgeryVerdictAsync(Context, HandleRemoteSignOutAsync); } else if (Options.SignedOutCallbackPath.HasValue && Options.SignedOutCallbackPath == Request.Path) { - return HandleSignOutCallbackAsync(); + return RemoteAuthenticationAntiforgery.HandleWithoutAntiforgeryVerdictAsync(Context, HandleSignOutCallbackAsync); } return base.HandleRequestAsync(); diff --git a/src/Security/Authorization/Core/src/DenyAnonymousAuthorizationRequirement.cs b/src/Security/Authorization/Core/src/DenyAnonymousAuthorizationRequirement.cs index ed9594562d08..2cb0bf019435 100644 --- a/src/Security/Authorization/Core/src/DenyAnonymousAuthorizationRequirement.cs +++ b/src/Security/Authorization/Core/src/DenyAnonymousAuthorizationRequirement.cs @@ -1,8 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Linq; using System.Threading.Tasks; +using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Authorization.Infrastructure; @@ -21,11 +21,7 @@ public class DenyAnonymousAuthorizationRequirement : AuthorizationHandlerThe requirement to evaluate. protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, DenyAnonymousAuthorizationRequirement requirement) { - var user = context.User; - var userIsAnonymous = - user?.Identity == null || - !user.Identities.Any(i => i.IsAuthenticated); - if (!userIsAnonymous) + if (SecurityHelper.IsAuthenticated(context.User)) { context.Succeed(requirement); } diff --git a/src/Security/Authorization/Core/src/Microsoft.AspNetCore.Authorization.csproj b/src/Security/Authorization/Core/src/Microsoft.AspNetCore.Authorization.csproj index de4ce273fc21..9b2d272e7bf4 100644 --- a/src/Security/Authorization/Core/src/Microsoft.AspNetCore.Authorization.csproj +++ b/src/Security/Authorization/Core/src/Microsoft.AspNetCore.Authorization.csproj @@ -28,6 +28,7 @@ Microsoft.AspNetCore.Authorization.AuthorizeAttribute + diff --git a/src/Shared/RemoteAuthenticationAntiforgery.cs b/src/Shared/RemoteAuthenticationAntiforgery.cs new file mode 100644 index 000000000000..be291fa09d65 --- /dev/null +++ b/src/Shared/RemoteAuthenticationAntiforgery.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Antiforgery; + +// Shared between Microsoft.AspNetCore.Authentication (RemoteAuthenticationHandler) and the remote handlers +// that own additional callback paths of their own (e.g. Microsoft.AspNetCore.Authentication.OpenIdConnect). +// +// A remote provider's callback is a cross-site request by protocol design: OpenID Connect +// response_mode=form_post and WS-Federation both deliver the response as a top-level form POST from the +// identity provider's origin. Cross-origin CSRF protection therefore records an invalid +// IAntiforgeryValidationFeature verdict for it, and the handler then fails on its very first action - reading +// the callback body - before any of its events can run, so an application has no way to opt out. +// +// These callbacks carry their own forgery protection: the state parameter round-trips a protected +// AuthenticationProperties payload whose correlation id must match the correlation cookie, which the handler +// validates. The verdict is therefore suppressed while the handler owns the request, and restored when the +// handler declines it so the rest of the pipeline still sees the original verdict. +internal static class RemoteAuthenticationAntiforgery +{ + public static async Task HandleWithoutAntiforgeryVerdictAsync(HttpContext context, Func> handler) + { + var suppressedVerdict = context.Features.Get(); + if (suppressedVerdict is { IsValid: false }) + { + context.Features.Set(null); + } + + var handled = false; + try + { + handled = await handler(); + return handled; + } + finally + { + if (!handled && suppressedVerdict is { IsValid: false }) + { + context.Features.Set(suppressedVerdict); + } + } + } +} diff --git a/src/Shared/SecurityHelper/SecurityHelper.cs b/src/Shared/SecurityHelper/SecurityHelper.cs index 7871714db7af..7d31f4a99371 100644 --- a/src/Shared/SecurityHelper/SecurityHelper.cs +++ b/src/Shared/SecurityHelper/SecurityHelper.cs @@ -43,4 +43,15 @@ public static ClaimsPrincipal MergeUserPrincipal(ClaimsPrincipal? existingPrinci } return newPrincipal; } + + /// + /// Determines whether the is authenticated. + /// Uses the same aggregate semantics as authorization's + /// DenyAnonymousAuthorizationRequirement: the principal is authenticated when it has an + /// identity and at least one of its identities is authenticated (not only the primary identity). + /// + /// The to inspect. + /// if the principal is authenticated; otherwise . + public static bool IsAuthenticated(ClaimsPrincipal? user) + => user?.Identity is not null && user.Identities.Any(static identity => identity.IsAuthenticated); } diff --git a/src/Shared/test/Shared.Tests/SecurityHelperTests.cs b/src/Shared/test/Shared.Tests/SecurityHelperTests.cs index 47a7f6cdaafb..95fea2ad2c9c 100644 --- a/src/Shared/test/Shared.Tests/SecurityHelperTests.cs +++ b/src/Shared/test/Shared.Tests/SecurityHelperTests.cs @@ -89,4 +89,42 @@ public void AddingPreservesNewIdentitiesAndDropsEmpty() Assert.Equal(identityNoAuthTypeWithClaim, user.Identities.Skip(2).First()); Assert.Equal(identityEmptyWithAuthType, user.Identities.Skip(3).First()); } + + [Fact] + public void IsAuthenticated_NullUser_ReturnsFalse() + { + Assert.False(SecurityHelper.IsAuthenticated(null)); + } + + [Fact] + public void IsAuthenticated_NoIdentities_ReturnsFalse() + { + Assert.False(SecurityHelper.IsAuthenticated(new ClaimsPrincipal())); + } + + [Fact] + public void IsAuthenticated_UnauthenticatedIdentity_ReturnsFalse() + { + Assert.False(SecurityHelper.IsAuthenticated(new ClaimsPrincipal(new ClaimsIdentity()))); + } + + [Fact] + public void IsAuthenticated_AuthenticatedIdentity_ReturnsTrue() + { + Assert.True(SecurityHelper.IsAuthenticated(new ClaimsPrincipal(new ClaimsIdentity(authenticationType: "custom")))); + } + + [Fact] + public void IsAuthenticated_AuthenticatedByNonPrimaryIdentity_ReturnsTrue() + { + // The primary identity is unauthenticated, but a later identity is authenticated. + var user = new ClaimsPrincipal(new[] + { + new ClaimsIdentity(), + new ClaimsIdentity(authenticationType: "custom"), + }); + + Assert.False(user.Identity.IsAuthenticated); + Assert.True(SecurityHelper.IsAuthenticated(user)); + } } diff --git a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs index f049147911f7..279fd103e2e4 100644 --- a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs +++ b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs @@ -370,6 +370,7 @@ private async Task Invoke(HubMethodDescriptor descriptor, HubConnectionCon var scope = _serviceScopeFactory.CreateAsyncScope(); IHubActivator? hubActivator = null; THub? hub = null; + long? streamOwner = null; try { hubActivator = scope.ServiceProvider.GetRequiredService>(); @@ -409,7 +410,7 @@ await SendInvocationError(hubMethodInvocationMessage.InvocationId, connection, CancellationTokenSource? cts = null; if (descriptor.HasSyntheticArguments) { - ReplaceArguments(descriptor, hubMethodInvocationMessage, isStreamCall, connection, scope, ref arguments, out cts); + ReplaceArguments(descriptor, hubMethodInvocationMessage, isStreamCall, connection, scope, ref arguments, ref streamOwner, out cts); } if (isStreamCall || isStreamResponse) @@ -424,7 +425,7 @@ await SendInvocationError(hubMethodInvocationMessage.InvocationId, connection, if (isStreamResponse) { _ = StreamAsync(hubMethodInvocationMessage.InvocationId!, connection, hubCallerContext, - arguments, scope, hubActivator, hub, cts, hubMethodInvocationMessage, descriptor); + arguments, scope, hubActivator, hub, cts, hubMethodInvocationMessage, descriptor, streamOwner); } else { @@ -439,7 +440,8 @@ static async Task ExecuteInvocation(DefaultHubDispatcher dispatcher, HubCallerContext hubCallerContext, HubMethodInvocationMessage hubMethodInvocationMessage, bool isStreamCall, - CancellationTokenSource? cts) + CancellationTokenSource? cts, + long? streamOwner) { var logger = dispatcher._logger; var enableDetailedErrors = dispatcher._enableDetailedErrors; @@ -509,7 +511,7 @@ await SendInvocationError(hubMethodInvocationMessage.InvocationId, connection, // And normal invocations handle cleanup below in the finally if (isStreamCall) { - await CleanupInvocation(connection, hubMethodInvocationMessage, hubActivator, hub, scope); + await CleanupInvocation(connection, hubMethodInvocationMessage, streamOwner, hubActivator, hub, scope); } } @@ -521,7 +523,7 @@ await SendInvocationError(hubMethodInvocationMessage.InvocationId, connection, } } - invocation = ExecuteInvocation(this, methodExecutor, hub, arguments, scope, hubActivator, connection, hubCallerContext, hubMethodInvocationMessage, isStreamCall, cts); + invocation = ExecuteInvocation(this, methodExecutor, hub, arguments, scope, hubActivator, connection, hubCallerContext, hubMethodInvocationMessage, isStreamCall, cts, streamOwner); } if (isStreamCall || isStreamResponse) @@ -557,21 +559,21 @@ await SendInvocationError(hubMethodInvocationMessage.InvocationId, connection, { wasSemaphoreReleased = !hubCallerClients.TrySetSemaphoreReleased(); } - await CleanupInvocation(connection, hubMethodInvocationMessage, hubActivator, hub, scope); + await CleanupInvocation(connection, hubMethodInvocationMessage, streamOwner, hubActivator, hub, scope); } } return !wasSemaphoreReleased; } - private static ValueTask CleanupInvocation(HubConnectionContext connection, HubMethodInvocationMessage hubMessage, IHubActivator? hubActivator, + private static ValueTask CleanupInvocation(HubConnectionContext connection, HubMethodInvocationMessage hubMessage, long? streamOwner, IHubActivator? hubActivator, THub? hub, AsyncServiceScope scope) { - if (hubMessage.StreamIds != null) + if (streamOwner is not null) { - foreach (var stream in hubMessage.StreamIds) + foreach (var streamId in hubMessage.StreamIds!) { - connection.StreamTracker.TryComplete(CompletionMessage.Empty(stream)); + connection.StreamTracker.TryComplete(streamId, streamOwner.Value); } } @@ -584,7 +586,8 @@ private static ValueTask CleanupInvocation(HubConnectionContext connection, HubM } private async Task StreamAsync(string invocationId, HubConnectionContext connection, HubCallerContext hubCallerContext, object?[] arguments, AsyncServiceScope scope, - IHubActivator hubActivator, THub hub, CancellationTokenSource? streamCts, HubMethodInvocationMessage hubMethodInvocationMessage, HubMethodDescriptor descriptor) + IHubActivator hubActivator, THub hub, CancellationTokenSource? streamCts, HubMethodInvocationMessage hubMethodInvocationMessage, + HubMethodDescriptor descriptor, long? streamOwner) { string? error = null; @@ -671,7 +674,7 @@ private async Task StreamAsync(string invocationId, HubConnectionContext connect Activity.Current = previousActivity; } - await CleanupInvocation(connection, hubMethodInvocationMessage, hubActivator, hub, scope); + await CleanupInvocation(connection, hubMethodInvocationMessage, streamOwner, hubActivator, hub, scope); // Only remove/dispose the CTS if we successfully registered it, otherwise we'd evict // another invocation's CTS on ID collision. @@ -817,7 +820,8 @@ await connection.WriteAsync(CompletionMessage.WithError(hubMethodInvocationMessa } private void ReplaceArguments(HubMethodDescriptor descriptor, HubMethodInvocationMessage hubMethodInvocationMessage, bool isStreamCall, - HubConnectionContext connection, AsyncServiceScope scope, ref object?[] arguments, out CancellationTokenSource? cts) + HubConnectionContext connection, AsyncServiceScope scope, ref object?[] arguments, + ref long? streamOwner, out CancellationTokenSource? cts) { cts = null; // In order to add the synthetic arguments we need a new array because the invocation array is too small (it doesn't know about synthetic arguments) @@ -825,39 +829,52 @@ private void ReplaceArguments(HubMethodDescriptor descriptor, HubMethodInvocatio var streamPointer = 0; var hubInvocationArgumentPointer = 0; - for (var parameterPointer = 0; parameterPointer < arguments.Length; parameterPointer++) + var argumentsReplaced = false; + try { - // populate the synthetic arguments first - if (descriptor.IsServiceArgument(parameterPointer)) - { - arguments[parameterPointer] = descriptor.GetService(scope.ServiceProvider, parameterPointer, descriptor.OriginalParameterTypes[parameterPointer]); - } - else if (descriptor.OriginalParameterTypes[parameterPointer] == typeof(CancellationToken)) + for (var parameterPointer = 0; parameterPointer < arguments.Length; parameterPointer++) { - cts = CancellationTokenSource.CreateLinkedTokenSource(connection.ConnectionAborted); - arguments[parameterPointer] = cts.Token; - } - else if (isStreamCall && ReflectionHelper.IsStreamingType(descriptor.OriginalParameterTypes[parameterPointer], mustBeDirectType: true)) - { - Log.StartingParameterStream(_logger, hubMethodInvocationMessage.StreamIds![streamPointer]); - var itemType = descriptor.StreamingParameters![streamPointer]; - arguments[parameterPointer] = connection.StreamTracker.AddStream(hubMethodInvocationMessage.StreamIds[streamPointer], - itemType, descriptor.OriginalParameterTypes[parameterPointer]); + // populate the synthetic arguments first + if (descriptor.IsServiceArgument(parameterPointer)) + { + arguments[parameterPointer] = descriptor.GetService(scope.ServiceProvider, parameterPointer, descriptor.OriginalParameterTypes[parameterPointer]); + } + else if (descriptor.OriginalParameterTypes[parameterPointer] == typeof(CancellationToken)) + { + cts = CancellationTokenSource.CreateLinkedTokenSource(connection.ConnectionAborted); + arguments[parameterPointer] = cts.Token; + } + else if (isStreamCall && ReflectionHelper.IsStreamingType(descriptor.OriginalParameterTypes[parameterPointer], mustBeDirectType: true)) + { + Log.StartingParameterStream(_logger, hubMethodInvocationMessage.StreamIds![streamPointer]); + var itemType = descriptor.StreamingParameters![streamPointer]; + arguments[parameterPointer] = connection.StreamTracker.AddStream(hubMethodInvocationMessage.StreamIds[streamPointer], + itemType, descriptor.OriginalParameterTypes[parameterPointer], streamOwner ??= connection.StreamTracker.GetNextStreamOwner()); - streamPointer++; - } - else if (hubMethodInvocationMessage.Arguments?.Length > hubInvocationArgumentPointer && - (hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer] == null || - descriptor.OriginalParameterTypes[parameterPointer].IsAssignableFrom(hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer]?.GetType()))) - { - // The types match so it isn't a synthetic argument, just copy it into the arguments array - arguments[parameterPointer] = hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer]; - hubInvocationArgumentPointer++; + streamPointer++; + } + else if (hubMethodInvocationMessage.Arguments?.Length > hubInvocationArgumentPointer && + (hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer] == null || + descriptor.OriginalParameterTypes[parameterPointer].IsAssignableFrom(hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer]?.GetType()))) + { + // The types match so it isn't a synthetic argument, just copy it into the arguments array + arguments[parameterPointer] = hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer]; + hubInvocationArgumentPointer++; + } + else + { + // This should never happen + Debug.Assert(false, $"Failed to bind argument of type '{descriptor.OriginalParameterTypes[parameterPointer].Name}' for hub method '{descriptor.MethodExecutor.MethodInfo.Name}'."); + } } - else + + argumentsReplaced = true; + } + finally + { + if (!argumentsReplaced) { - // This should never happen - Debug.Assert(false, $"Failed to bind argument of type '{descriptor.OriginalParameterTypes[parameterPointer].Name}' for hub method '{descriptor.MethodExecutor.MethodInfo.Name}'."); + cts?.Dispose(); } } } diff --git a/src/SignalR/server/Core/src/StreamTracker.cs b/src/SignalR/server/Core/src/StreamTracker.cs index 81fea34566de..7666cd420b06 100644 --- a/src/SignalR/server/Core/src/StreamTracker.cs +++ b/src/SignalR/server/Core/src/StreamTracker.cs @@ -16,13 +16,19 @@ internal sealed class StreamTracker { private static readonly MethodInfo _buildConverterMethod = typeof(StreamTracker).GetMethods(BindingFlags.NonPublic | BindingFlags.Static).Single(m => m.Name.Equals(nameof(BuildStream))); private readonly object[] _streamConverterArgs; - private readonly ConcurrentDictionary _lookup = new ConcurrentDictionary(); + private readonly ConcurrentDictionary _lookup = new(); + private long _nextStreamOwner; public StreamTracker(int streamBufferCapacity) { _streamConverterArgs = new object[] { streamBufferCapacity }; } + public long GetNextStreamOwner() + { + return Interlocked.Increment(ref _nextStreamOwner); + } + /// /// Creates a new stream and returns the ChannelReader for it as an object. /// @@ -30,18 +36,22 @@ public StreamTracker(int streamBufferCapacity) Justification = "BuildStream doesn't have trimming annotations.")] [UnconditionalSuppressMessage("AOT", "IL3050:RequiresDynamicCode", Justification = "HubMethodDescriptor checks for ValueType streaming item types when PublishAot=true. Developers will get an exception in this situation before publishing.")] - public object AddStream(string streamId, Type itemType, Type targetType) + public object AddStream(string streamId, Type itemType, Type targetType, long streamOwner) { Debug.Assert(RuntimeFeature.IsDynamicCodeSupported || !itemType.IsValueType, "HubMethodDescriptor ensures itemType is not a ValueType when PublishAot=true."); var newConverter = (IStreamConverter)_buildConverterMethod.MakeGenericMethod(itemType).Invoke(null, _streamConverterArgs)!; - _lookup[streamId] = newConverter; + if (!_lookup.TryAdd(streamId, (streamOwner, newConverter))) + { + throw new HubException($"Stream ID '{streamId}' is already in use."); + } + return newConverter.GetReaderAsObject(targetType); } - private bool TryGetConverter(string streamId, [NotNullWhen(true)] out IStreamConverter? converter) + private bool TryGetRegistration(string streamId, out (long Owner, IStreamConverter Converter) registration) { - if (_lookup.TryGetValue(streamId, out converter)) + if (_lookup.TryGetValue(streamId, out registration)) { return true; } @@ -51,9 +61,9 @@ private bool TryGetConverter(string streamId, [NotNullWhen(true)] out IStreamCon public bool TryProcessItem(StreamItemMessage message, [NotNullWhen(true)] out Task? task) { - if (TryGetConverter(message.InvocationId!, out var converter)) + if (TryGetRegistration(message.InvocationId!, out var registration)) { - task = converter.WriteToStream(message.Item); + task = registration.Converter.WriteToStream(message.Item); return true; } @@ -63,9 +73,9 @@ public bool TryProcessItem(StreamItemMessage message, [NotNullWhen(true)] out Ta public Type GetStreamItemType(string streamId) { - if (TryGetConverter(streamId, out var converter)) + if (TryGetRegistration(streamId, out var registration)) { - return converter.GetItemType(); + return registration.Converter.GetItemType(); } throw new KeyNotFoundException($"No stream with id '{streamId}' could be found."); @@ -73,12 +83,27 @@ public Type GetStreamItemType(string streamId) public bool TryComplete(CompletionMessage message) { - _lookup.TryRemove(message.InvocationId!, out var converter); - if (converter == null) + if (!_lookup.TryRemove(message.InvocationId!, out var registration)) + { + return false; + } + registration.Converter.TryComplete(message.HasResult || message.Error == null ? null : new HubException(message.Error)); + return true; + } + + public bool TryComplete(string streamId, long streamOwner) + { + if (!_lookup.TryGetValue(streamId, out var registration) || registration.Owner != streamOwner) { return false; } - converter.TryComplete(message.HasResult || message.Error == null ? null : new HubException(message.Error)); + + if (!_lookup.TryRemove(KeyValuePair.Create(streamId, registration))) + { + return false; + } + + registration.Converter.TryComplete(null); return true; } @@ -86,7 +111,7 @@ public void CompleteAll(Exception ex) { foreach (var converter in _lookup) { - converter.Value.TryComplete(ex); + converter.Value.Converter.TryComplete(ex); } } diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs index afb56aa755a0..03328940668d 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs @@ -223,6 +223,11 @@ public async Task StreamingConcat(ChannelReader source) return sb.ToString(); } + public async Task StreamingConcatTwoStreams(ChannelReader first, ChannelReader second) + { + return await StreamingConcat(first) + await StreamingConcat(second); + } + public async Task StreamDontRead(ChannelReader source) { while (await source.WaitToReadAsync()) diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs index 051ae6409ac0..6fe265df7af5 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs @@ -3696,6 +3696,88 @@ public async Task UploadStringsToConcat() } } + [Fact] + public async Task UploadStreamWithDuplicateIdsFailsAndConnectionContinues() + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services => + { + services.AddSignalR(options => + { + options.EnableDetailedErrors = true; + options.StreamBufferCapacity = 1; + }); + }); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + await client.BeginUploadStreamAsync("duplicate", nameof(MethodHub.StreamingConcatTwoStreams), new[] { "id", "id" }, Array.Empty()).DefaultTimeout(); + await client.SendHubMessageAsync(new StreamItemMessage("id", "first")).DefaultTimeout(); + await client.SendHubMessageAsync(new StreamItemMessage("id", "second")).DefaultTimeout(); + await client.SendInvocationAsync(nameof(MethodHub.Echo), "test").DefaultTimeout(); + + var duplicateCompletion = Assert.IsType(await client.ReadAsync().DefaultTimeout()); + Assert.Equal("An unexpected error occurred invoking 'StreamingConcatTwoStreams' on the server. HubException: Stream ID 'id' is already in use.", duplicateCompletion.Error); + + var echoCompletion = Assert.IsType(await client.ReadAsync().DefaultTimeout()); + Assert.Equal("test", echoCompletion.Result); + } + } + + [Fact] + public async Task ActiveUploadStreamCannotBeReplacedAndIdCanBeReusedAfterCompletion() + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services => + { + services.AddSignalR(options => options.EnableDetailedErrors = true); + }); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + await client.BeginUploadStreamAsync("original", nameof(MethodHub.StreamingConcat), new[] { "id" }, Array.Empty()).DefaultTimeout(); + await client.BeginUploadStreamAsync("duplicate", nameof(MethodHub.StreamingConcat), new[] { "id" }, Array.Empty()).DefaultTimeout(); + + var duplicateCompletion = Assert.IsType(await client.ReadAsync().DefaultTimeout()); + Assert.Equal("An unexpected error occurred invoking 'StreamingConcat' on the server. HubException: Stream ID 'id' is already in use.", duplicateCompletion.Error); + + await client.SendHubMessageAsync(new StreamItemMessage("id", "original")).DefaultTimeout(); + await client.SendHubMessageAsync(CompletionMessage.Empty("id")).DefaultTimeout(); + + var originalCompletion = Assert.IsType(await client.ReadAsync().DefaultTimeout()); + Assert.Equal("original", originalCompletion.Result); + + await client.BeginUploadStreamAsync("reused", nameof(MethodHub.StreamingConcat), new[] { "id" }, Array.Empty()).DefaultTimeout(); + await client.SendHubMessageAsync(new StreamItemMessage("id", "reused")).DefaultTimeout(); + await client.SendHubMessageAsync(CompletionMessage.Empty("id")).DefaultTimeout(); + + var reusedCompletion = Assert.IsType(await client.ReadAsync().DefaultTimeout()); + Assert.Equal("reused", reusedCompletion.Result); + } + } + + [Fact] + public async Task UploadMultipleStreamsWithUniqueIds() + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + await client.BeginUploadStreamAsync("invocation", nameof(MethodHub.StreamingConcatTwoStreams), new[] { "first", "second" }, Array.Empty()).DefaultTimeout(); + await client.SendHubMessageAsync(new StreamItemMessage("first", "hello ")).DefaultTimeout(); + await client.SendHubMessageAsync(CompletionMessage.Empty("first")).DefaultTimeout(); + await client.SendHubMessageAsync(new StreamItemMessage("second", "world")).DefaultTimeout(); + await client.SendHubMessageAsync(CompletionMessage.Empty("second")).DefaultTimeout(); + + var completion = Assert.IsType(await client.ReadAsync().DefaultTimeout()); + Assert.Equal("hello world", completion.Result); + } + } + [Fact] public async Task UploadStreamedObjects() {