diff --git a/CHANGELOG.md b/CHANGELOG.md index ca08f7a1..228058ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,20 @@ All notable changes to the MMCA.Common packages are documented here. The format [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); versions follow [Semantic Versioning](https://semver.org/) and are derived from git tags by MinVer (see [the published versioning policy](https://ivanball.github.io/docs/guides/common-VERSIONING.html)). +## [Unreleased] + +### Added + +- `SpanishAccentTestsBase` (`MMCA.Common.Testing.Architecture`, flat `Bases` namespace): a fitness test that scans every `*.es.resx` under the subclass's `ResourceRoot` (bin, obj, node_modules and .git skipped) and fails on a string value containing a known unaccented spelling ("codigo", "sesion", "aplicacion" and the rest of `UnaccentedWords`), matched as a whole word, listing each hit as `file:key: words`. Override `UnaccentedWords` to extend the list (`[.. base.UnaccentedWords, "x"]`), `AllowedEntries` to accept an intended spelling by its `relative/path.es.resx:Key`, and `MinimumResourceFileCount` (default 1) so a wrong root fails instead of passing on zero files. MMCA.Common runs it over its own `Source/`. + +### Fixed + +- The server-side `"APIClient"` handler from `AddCommonServerTokenStorage()` (renamed `BrowserOriginHandler`, internal) also forwards the visitor's `User-Agent`, replacing the host's empty one, so a sign-in or registration made on the Blazor Server path records the browser's device on the signed-in devices page instead of "Unrecognized device", matching the cookie-session refresh. A blank user-agent is not forwarded. +- The per-IP registration rate limit trips over the hybrid cache. `LoginProtectionService.CheckRegistrationRateLimitAsync` reads the counter with `ICacheService.GetFromSharedStoreAsync`, since `IncrementAsync` writes the shared store only; it read through `GetAsync`, which `HybridCacheService` answered from a 30-second in-process copy, so the count it saw stayed at its first value and a burst from one IP was never refused. The fail-open behavior on a cache outage is unchanged. The lockout check is unchanged: its flag is written with `SetAsync`, which updates both tiers. +- `DataGridListPageBase` shows "Loading cancelled." only when the cancelled load is still the current one and the page is not disposed. A load superseded by a newer one (a re-sort, a filter change, a search) and a load cut off by leaving the page used to toast it too; a user's Cancel still toasts once. +- The gateway downstream health checks from `AddGatewayDownstreamHealthChecks` stop flapping on an HTTP/1.1 head. Each check is one instance per downstream for the life of the provider (a keyed singleton the registration factory resolves), so the HTTP version latched on the first poll survives to the next; the health-check service rebuilt the check on every poll, so every poll renegotiated. The `gateway-downstream-*` probe clients also drop the resilience handler that `AddServiceDefaults()` puts on every client (`RemoveAllResilienceHandlers`): its retry backoff spent the two-second probe budget on the refused HTTP/2 attempt, so the HTTP/1.1 fallback never ran and a healthy downstream read Unhealthy. +- Spanish role-administration strings (`RoleAdminListResources.es.resx`, `RoleAdminEditResources.es.resx`) and the OAuth unexpected-completion message (`SharedResource.es.resx`) carry their accents; eight values used the unaccented spellings of "codigo", "mas", "aqui", "numero", "estan", "aplicacion", "aun" and "sesion". + ## [1.225.0] - 2026-10-03 ### Fixed diff --git a/FACTS.md b/FACTS.md index 26a34de1..ecf783bf 100644 --- a/FACTS.md +++ b/FACTS.md @@ -48,10 +48,10 @@ The ADRs live in the Website repo (`docs-src/adr/`), published at it owns the range/count and the one-line summaries. Do not restate the `(001-NNN)` range elsewhere. ## Architecture fitness functions -- **141 test methods across 55 abstract `*TestsBase` classes**, shipped once in the +- **142 test methods across 56 abstract `*TestsBase` classes**, shipped once in the `MMCA.Common.Testing.Architecture` package (ADR-015) and re-run as thin subclasses across all consuming repos (Common, ADC, Store). -- MMCA.Common's own build executes **339** of them (the methods of the bases its arch-tests +- MMCA.Common's own build executes **340** of them (the methods of the bases its arch-tests subclass, plus its Common-only direct tests, e.g. `FrameworkSanityTests`/`SpecificationFitnessTests`). ## Governance rubric diff --git a/Source/Core/MMCA.Common.Infrastructure/Auth/LoginProtectionService.cs b/Source/Core/MMCA.Common.Infrastructure/Auth/LoginProtectionService.cs index 2a01f082..bb7aeeb9 100644 --- a/Source/Core/MMCA.Common.Infrastructure/Auth/LoginProtectionService.cs +++ b/Source/Core/MMCA.Common.Infrastructure/Auth/LoginProtectionService.cs @@ -132,7 +132,10 @@ public async Task CheckRegistrationRateLimitAsync(string? ipAddress, Can long registrationCount; try { - registrationCount = await cacheService.GetAsync(key, cancellationToken).ConfigureAwait(false) ?? 0; + // Read the counter from the shared store, never from a process-local copy: IncrementAsync + // writes the shared store only, so a hybrid cache's in-process entry would pin the first + // count it saw and the limit would never trip while that copy lived. + registrationCount = await cacheService.GetFromSharedStoreAsync(key, cancellationToken).ConfigureAwait(false) ?? 0; } catch (Exception ex) when (IsCacheOutage(ex, cancellationToken)) { diff --git a/Source/Hosting/MMCA.Common.Aspire/Gateway/DownstreamServiceHealthCheck.cs b/Source/Hosting/MMCA.Common.Aspire/Gateway/DownstreamServiceHealthCheck.cs index 13970f72..20225a28 100644 --- a/Source/Hosting/MMCA.Common.Aspire/Gateway/DownstreamServiceHealthCheck.cs +++ b/Source/Hosting/MMCA.Common.Aspire/Gateway/DownstreamServiceHealthCheck.cs @@ -22,8 +22,10 @@ namespace MMCA.Common.Aspire.Gateway; /// /// Under the first probe asks for HTTP/2 and, if the /// downstream refuses the protocol, retries once as HTTP/1.1 within the same check, so one poll -/// still yields one verdict. The version that answered is latched for the life of this instance, -/// and the health-check service holds one instance per downstream, so the latch is effectively per +/// still yields one verdict. The version that answered is latched for the life of this instance. +/// The health-check service rebuilds a check from its registration factory on every poll, so +/// AddGatewayDownstreamHealthChecks registers each instance as a keyed singleton and the +/// factory resolves that one: there is one instance per downstream, and the latch is per /// downstream for the life of the process. That is safe because a service cannot change the /// protocol of its cleartext endpoint without a redeploy, and a redeploy of the topology restarts /// this gateway too: a stale latch cannot outlive the endpoint that justified it. diff --git a/Source/Hosting/MMCA.Common.Aspire/Gateway/GatewayHealthCheckExtensions.cs b/Source/Hosting/MMCA.Common.Aspire/Gateway/GatewayHealthCheckExtensions.cs index 099f6616..b936b6fe 100644 --- a/Source/Hosting/MMCA.Common.Aspire/Gateway/GatewayHealthCheckExtensions.cs +++ b/Source/Hosting/MMCA.Common.Aspire/Gateway/GatewayHealthCheckExtensions.cs @@ -1,6 +1,7 @@ using System.Diagnostics.CodeAnalysis; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Http.Resilience; namespace MMCA.Common.Aspire.Gateway; @@ -198,15 +199,44 @@ private static IServiceCollection Register( // request, because under DownstreamProbeVersion.Auto the check discovers which // version this downstream speaks and may send both on one poll. See // GatewayDownstreamHealthCheckOptions.ProbeVersion. + }) + + // No resilience handler on the probe: AddServiceDefaults() puts the standard Polly + // pipeline on every factory client, and its one retry (about two seconds of backoff) + // lands inside the two-second probe budget. A refused HTTP/2 attempt must reach the + // check at once so it can fall back to HTTP/1.1, and a probe is re-sent on the next + // poll anyway, so a retry here only turns a healthy downstream into a timed-out one. + // This is what RemoveAllResilienceHandlers() does, written against the stable + // ConfigureAdditionalHttpMessageHandlers because that helper is still EXTEXP0001 + // (experimental) in Microsoft.Extensions.Http.Resilience. + .ConfigureAdditionalHttpMessageHandlers(static (handlers, _) => + { + for (var i = handlers.Count - 1; i >= 0; i--) + { + if (handlers[i] is ResilienceHandler) + { + handlers.RemoveAt(i); + } + } }); - healthChecks.Add(new HealthCheckRegistration( - CheckName(name), - sp => new DownstreamServiceHealthCheck( + // One check instance per downstream for the life of the provider. The health-check + // service builds the check from its registration factory on EVERY poll, so a factory + // that news one up would drop the latched HTTP version between polls and renegotiate + // each time. The singleton is keyed by the check name; the latch inside it is already + // safe under overlapping polls (Interlocked, first writer wins). + var checkName = CheckName(name); + services.AddKeyedSingleton( + checkName, + (sp, _) => new DownstreamServiceHealthCheck( sp.GetRequiredService(), name, clientName, - probeVersion), + probeVersion)); + + healthChecks.Add(new HealthCheckRegistration( + checkName, + sp => sp.GetRequiredKeyedService(checkName), failureStatus: HealthStatus.Unhealthy, tags: [HealthCheckTags.Ready], timeout: ProbeTimeout)); diff --git a/Source/Hosting/MMCA.Common.Testing.Architecture/Bases/Governance/SpanishAccentTestsBase.cs b/Source/Hosting/MMCA.Common.Testing.Architecture/Bases/Governance/SpanishAccentTestsBase.cs new file mode 100644 index 00000000..4e1e7eae --- /dev/null +++ b/Source/Hosting/MMCA.Common.Testing.Architecture/Bases/Governance/SpanishAccentTestsBase.cs @@ -0,0 +1,134 @@ +using System.Text.RegularExpressions; + +namespace MMCA.Common.Testing.Architecture; + +/// +/// Spanish-localization fitness function: no Spanish resource string ships a common word with its +/// required accent or n-tilde missing (codigo for the word with the accented o, sesion, +/// contrasena for the word with the n-tilde, and so on). Those strings compile, render and pass +/// every functional test, so without this gate they are found by a Spanish-speaking user. +/// Authored once here and re-run as a thin subclass in each repo, which supplies the +/// to scan and, when it has deliberate exceptions, its +/// . +/// +/// Every *.es.resx under the root is read, and only the text of each string +/// <data><value> is checked (keys and comments are not). A word matches only as a +/// whole word, case-insensitively, so the correctly accented form never matches: an accented letter +/// is a different letter, and a longer word (a plural, a derived form) is a different word. +/// +/// +/// The list holds words whose unaccented spelling is almost always the mistake in UI text. Words that +/// are equally correct with and without the accent depending on meaning (esta as "this" +/// against the verb form, solo, which no longer takes an accent) are deliberately left out, and +/// plurals that drop the accent by rule (sesiones, aplicaciones) never match. +/// +/// +public abstract class SpanishAccentTestsBase +{ + /// + /// The unaccented spellings checked by default. Each stands for a word that requires an accent or + /// an n-tilde in the sense UI text uses it. + /// + private static readonly string[] DefaultUnaccentedWords = + [ + "sesion", "codigo", "codigos", "contrasena", "contrasenas", "numero", "numeros", "maximo", + "minimo", "titulo", "direccion", "informacion", "posicion", "clasificacion", "electronico", + "electronica", "pagina", "paginas", "aqui", "mas", "todavia", "aun", "estan", "publico", + "ningun", "podra", "cerrara", "volvera", "encontro", "visito", "tenia", "aplicacion", + "limite", "puntuacion", "perdio", "confirmo", + ]; + + /// + /// The folder to scan recursively for *.es.resx files, normally the repo's Source + /// folder under ArchitectureMapBase.FindRepoRoot("<Repo>.slnx"). + /// + protected abstract string ResourceRoot { get; } + + /// + /// The unaccented spellings to reject. Defaults to the framework list; a subclass extends it with + /// [.. base.UnaccentedWords, "extra"] for domain vocabulary of its own. + /// + protected virtual IReadOnlyCollection UnaccentedWords => DefaultUnaccentedWords; + + /// + /// Intentional exceptions, each written exactly as the failure lists it: + /// {path relative to the resource root, forward slashes}:{resource key}. Keep it short and + /// say why next to each entry; an exception is a decision, not a way to make the gate pass. + /// + protected virtual IReadOnlyCollection AllowedEntries => []; + + /// + /// The fewest *.es.resx files the scan must find. A wrong root that finds none would + /// otherwise pass with nothing checked. + /// + protected virtual int MinimumResourceFileCount => 1; + + [Fact] + public void Spanish_resources_keep_their_accents() + { + ResourceRoot.Should().NotBeNullOrWhiteSpace(); + Directory.Exists(ResourceRoot).Should().BeTrue($"the resource root '{ResourceRoot}' must exist"); + + var files = Directory + .EnumerateFiles(ResourceRoot, "*.es.resx", SearchOption.AllDirectories) + .Where(path => !IsBuildOutput(Path.GetRelativePath(ResourceRoot, path))) + .Order(StringComparer.Ordinal) + .ToList(); + + files.Count.Should().BeGreaterThanOrEqualTo( + MinimumResourceFileCount, + $"the scan must find Spanish resources under '{ResourceRoot}', or it checks nothing"); + + var pattern = @"\b(?:" + string.Join('|', UnaccentedWords.Select(Regex.Escape)) + @")\b"; + var unaccented = new Regex( + pattern, + RegexOptions.IgnoreCase | RegexOptions.CultureInvariant, + TimeSpan.FromSeconds(1)); + + var allowed = AllowedEntries.ToHashSet(StringComparer.Ordinal); + var offenders = new List(); + + foreach (var file in files) + { + var relative = Path.GetRelativePath(ResourceRoot, file).Replace('\\', '/'); + + foreach (var data in XDocument.Load(file).Root?.Elements("data") ?? []) + { + // Non-string entries (file references, serialized objects) carry no UI text. + if (data.Attribute("type") is not null || data.Attribute("mimetype") is not null) + { + continue; + } + + var key = (string?)data.Attribute("name") ?? string.Empty; + var value = (string?)data.Element("value") ?? string.Empty; + var entry = relative + ":" + key; + if (allowed.Contains(entry)) + { + continue; + } + + var words = unaccented.Matches(value) + .Select(static match => match.Value) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToList(); + if (words.Count > 0) + { + offenders.Add($" - {entry}: {string.Join(", ", words)}"); + } + } + } + + ArchitectureAssert.NoViolations( + offenders, + "Spanish resource strings must carry their accents and n-tildes; fix the value, or add " + + "'file:key' to AllowedEntries when the unaccented spelling is intended"); + } + + /// True when the path runs through build output or a tool-owned tree. + private static bool IsBuildOutput(string relativePath) => + relativePath + .Replace('\\', '/') + .Split('/') + .Any(static segment => segment is "bin" or "obj" or "node_modules" or ".git"); +} diff --git a/Source/Hosting/MMCA.Common.Testing.Architecture/PublicAPI.Unshipped.txt b/Source/Hosting/MMCA.Common.Testing.Architecture/PublicAPI.Unshipped.txt index ab058de6..ee6fefd1 100644 --- a/Source/Hosting/MMCA.Common.Testing.Architecture/PublicAPI.Unshipped.txt +++ b/Source/Hosting/MMCA.Common.Testing.Architecture/PublicAPI.Unshipped.txt @@ -1 +1,8 @@ #nullable enable +MMCA.Common.Testing.Architecture.SpanishAccentTestsBase +MMCA.Common.Testing.Architecture.SpanishAccentTestsBase.Spanish_resources_keep_their_accents() -> void +MMCA.Common.Testing.Architecture.SpanishAccentTestsBase.SpanishAccentTestsBase() -> void +abstract MMCA.Common.Testing.Architecture.SpanishAccentTestsBase.ResourceRoot.get -> string! +virtual MMCA.Common.Testing.Architecture.SpanishAccentTestsBase.AllowedEntries.get -> System.Collections.Generic.IReadOnlyCollection! +virtual MMCA.Common.Testing.Architecture.SpanishAccentTestsBase.MinimumResourceFileCount.get -> int +virtual MMCA.Common.Testing.Architecture.SpanishAccentTestsBase.UnaccentedWords.get -> System.Collections.Generic.IReadOnlyCollection! diff --git a/Source/Presentation/MMCA.Common.UI.Web/DependencyInjection.cs b/Source/Presentation/MMCA.Common.UI.Web/DependencyInjection.cs index 9a6ae0d2..42c90e0e 100644 --- a/Source/Presentation/MMCA.Common.UI.Web/DependencyInjection.cs +++ b/Source/Presentation/MMCA.Common.UI.Web/DependencyInjection.cs @@ -29,12 +29,13 @@ public static class DependencyInjection /// cookie plumbing from MMCA.Common.API (AddServerAuthSessionCookie / /// UseCookieSessionRefresh) and a registered ITokenRefresher. /// - /// Also forwards the visitor's address on this host's server-side "APIClient" calls: - /// each request carries X-Forwarded-For set to the remote IP of the HTTP request behind - /// the render (the page request during prerender, the circuit's connection afterwards), the - /// same value the cookie-session refresh already forwards, so per-client limits such as the - /// registration rate limit key on the visitor instead of on this host. Nothing is sent when no - /// request is in scope. + /// Also forwards the visitor's origin on this host's server-side "APIClient" calls: + /// each request carries X-Forwarded-For set to the remote IP, and User-Agent set + /// to the browser's user-agent, of the HTTP request behind the render (the page request during + /// prerender, the circuit's connection afterwards), the same values the cookie-session refresh + /// already forwards, so per-client limits such as the registration rate limit key on the + /// visitor instead of on this host and a sign-in records the visitor's device. Nothing is sent + /// when no request is in scope. /// /// public IServiceCollection AddCommonServerTokenStorage() @@ -43,8 +44,8 @@ public IServiceCollection AddCommonServerTokenStorage() // The UI services' named client (AddUIShared); a second AddHttpClient call with the same // name appends to that client's pipeline, whichever of the two registrations runs first. - services.AddTransient(); - services.AddHttpClient("APIClient").AddHttpMessageHandler(); + services.AddTransient(); + services.AddHttpClient("APIClient").AddHttpMessageHandler(); return services.AddScoped(); } diff --git a/Source/Presentation/MMCA.Common.UI.Web/Services/BrowserForwardedForHandler.cs b/Source/Presentation/MMCA.Common.UI.Web/Services/BrowserForwardedForHandler.cs deleted file mode 100644 index 8cc3e8ea..00000000 --- a/Source/Presentation/MMCA.Common.UI.Web/Services/BrowserForwardedForHandler.cs +++ /dev/null @@ -1,50 +0,0 @@ -using Microsoft.AspNetCore.Http; - -namespace MMCA.Common.UI.Web.Services; - -/// -/// Stamps X-Forwarded-For with the browser's address on the server-side "APIClient" -/// calls a Blazor Server host makes for a visitor (the SSR prerender and the interactive circuit), so -/// the API keys per-client policy (the registration rate limit, the session's recorded IP) on the -/// visitor rather than on this host's own address, which every visitor shares. -/// -/// -/// -/// Same source and semantics as the cookie-session refresh. The address is the -/// of the HTTP request that carries this work: the page -/// request during prerender, the connection the circuit was established on afterwards. That value -/// has already been through this host's forwarded-headers middleware, so it is only as far back as -/// this host trusts its own proxies; a client-supplied X-Forwarded-For is never copied -/// verbatim. The header is single-valued and replaces any value already on the request. -/// -/// -/// Server only. Registered by AddCommonServerTokenStorage(), which only a Blazor -/// Server host calls; the WebAssembly client's calls reach the API through the same-origin proxy, -/// which stamps the header itself. When no request is in scope (a background call with no visitor -/// behind it) nothing is sent. -/// -/// -/// Reads the request in scope; an async-local, so it crosses the -/// handler's own DI scope. -internal sealed class BrowserForwardedForHandler(IHttpContextAccessor httpContextAccessor) : DelegatingHandler -{ - /// The header the API's forwarded-headers configuration reads. - internal const string HeaderName = "X-Forwarded-For"; - - /// - protected override Task SendAsync( - HttpRequestMessage request, - CancellationToken cancellationToken) - { - ArgumentNullException.ThrowIfNull(request); - - var remoteIpAddress = httpContextAccessor.HttpContext?.Connection.RemoteIpAddress?.ToString(); - if (remoteIpAddress is not null) - { - request.Headers.Remove(HeaderName); - request.Headers.TryAddWithoutValidation(HeaderName, remoteIpAddress); - } - - return base.SendAsync(request, cancellationToken); - } -} diff --git a/Source/Presentation/MMCA.Common.UI.Web/Services/BrowserOriginHandler.cs b/Source/Presentation/MMCA.Common.UI.Web/Services/BrowserOriginHandler.cs new file mode 100644 index 00000000..d0ffbaac --- /dev/null +++ b/Source/Presentation/MMCA.Common.UI.Web/Services/BrowserOriginHandler.cs @@ -0,0 +1,69 @@ +using Microsoft.AspNetCore.Http; + +namespace MMCA.Common.UI.Web.Services; + +/// +/// Stamps the browser's origin (X-Forwarded-For with its address, User-Agent with its +/// user-agent) on the server-side "APIClient" calls a Blazor Server host makes for a visitor +/// (the SSR prerender and the interactive circuit), so the API keys per-client policy (the +/// registration rate limit, the session's recorded IP and device) on the visitor rather than on this +/// host, whose own address every visitor shares and whose HTTP client sends no user-agent at all. +/// +/// +/// +/// Same source and semantics as the cookie-session refresh. Both values come from the HTTP +/// request that carries this work: the page request during prerender, the connection the circuit was +/// established on afterwards. The address is its , which +/// has already been through this host's forwarded-headers middleware, so it is only as far back as +/// this host trusts its own proxies; a client-supplied X-Forwarded-For is never copied +/// verbatim. The user-agent is informational only (it names the device on the signed-in devices +/// page), so it is forwarded as the browser sent it. Each header is single-valued and replaces any +/// value already on the request; a blank user-agent is not forwarded. +/// +/// +/// Server only. Registered by AddCommonServerTokenStorage(), which only a Blazor +/// Server host calls; the WebAssembly client's calls reach the API through the same-origin proxy, +/// which forwards the browser's own headers. When no request is in scope (a background call with no +/// visitor behind it) nothing is sent. +/// +/// +/// Reads the request in scope; an async-local, so it crosses the +/// handler's own DI scope. +internal sealed class BrowserOriginHandler(IHttpContextAccessor httpContextAccessor) : DelegatingHandler +{ + /// The header the API's forwarded-headers configuration reads. + internal const string ForwardedForHeaderName = "X-Forwarded-For"; + + /// The header the API records as the session's device. + internal const string UserAgentHeaderName = "User-Agent"; + + /// + protected override Task SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(request); + + var httpContext = httpContextAccessor.HttpContext; + if (httpContext is not null) + { + Replace(request, ForwardedForHeaderName, httpContext.Connection.RemoteIpAddress?.ToString()); + Replace(request, UserAgentHeaderName, httpContext.Request.Headers.UserAgent.ToString()); + } + + return base.SendAsync(request, cancellationToken); + } + + private static void Replace(HttpRequestMessage request, string headerName, string? value) + { + if (string.IsNullOrWhiteSpace(value)) + { + return; + } + + // TryAddWithoutValidation: a real browser user-agent does not always parse as a strict + // product token list, and neither value needs parsing on this side. + request.Headers.Remove(headerName); + request.Headers.TryAddWithoutValidation(headerName, value); + } +} diff --git a/Source/Presentation/MMCA.Common.UI/Pages/Administration/RoleAdminEditResources.es.resx b/Source/Presentation/MMCA.Common.UI/Pages/Administration/RoleAdminEditResources.es.resx index ef820d72..34ecdef3 100644 --- a/Source/Presentation/MMCA.Common.UI/Pages/Administration/RoleAdminEditResources.es.resx +++ b/Source/Presentation/MMCA.Common.UI/Pages/Administration/RoleAdminEditResources.es.resx @@ -16,7 +16,7 @@ Permisos de {0} - Marque los permisos que este rol debe recibir mediante filas almacenadas. Los permisos concedidos en el codigo ya estan marcados y no se pueden cambiar aqui. + Marque los permisos que este rol debe recibir mediante filas almacenadas. Los permisos concedidos en el código ya están marcados y no se pueden cambiar aquí. Cargando permisos @@ -34,16 +34,16 @@ Guardar los permisos almacenados de {0} - Esta aplicacion aun no declara permisos, por lo que no hay nada que conceder. + Esta aplicación aún no declara permisos, por lo que no hay nada que conceder. General - Concedido en el codigo + Concedido en el código - Solo se puede conceder en el codigo + Solo se puede conceder en el código No se guardaron los permisos diff --git a/Source/Presentation/MMCA.Common.UI/Pages/Administration/RoleAdminListResources.es.resx b/Source/Presentation/MMCA.Common.UI/Pages/Administration/RoleAdminListResources.es.resx index cdbe71dd..66d68b39 100644 --- a/Source/Presentation/MMCA.Common.UI/Pages/Administration/RoleAdminListResources.es.resx +++ b/Source/Presentation/MMCA.Common.UI/Pages/Administration/RoleAdminListResources.es.resx @@ -16,7 +16,7 @@ Roles - Cada rol concede los permisos declarados en el codigo mas los permisos almacenados aqui. Solo los almacenados se pueden editar. + Cada rol concede los permisos declarados en el código más los permisos almacenados aquí. Solo los almacenados se pueden editar. Cargando roles @@ -28,13 +28,13 @@ No se encontraron roles. - Roles y su numero de permisos + Roles y su número de permisos Rol - Concedidos en el codigo + Concedidos en el código Almacenados diff --git a/Source/Presentation/MMCA.Common.UI/Pages/Common/DataGridListPageBase.cs b/Source/Presentation/MMCA.Common.UI/Pages/Common/DataGridListPageBase.cs index f4195b89..79c06437 100644 --- a/Source/Presentation/MMCA.Common.UI/Pages/Common/DataGridListPageBase.cs +++ b/Source/Presentation/MMCA.Common.UI/Pages/Common/DataGridListPageBase.cs @@ -733,9 +733,13 @@ private async Task RunFetchAsync( } catch (OperationCanceledException) { - // Covers user/disposal cancellation and the pre-render timeout. During pre-render the - // toast is a no-op (separate render: no JS toast host), so no special-casing is needed. - if (showCancelSnackbar) + // Covers user/disposal cancellation, a superseded load and the pre-render timeout. Only a + // cancel of the CURRENT load on a live component toasts: a superseded load lost its source + // to the newer one (ResetCancellationTokenAsync swaps before it cancels), and a disposed + // component has nobody to tell, so both end silently. A user cancel (CancelLoading) + // cancels the current source without replacing it, so it still toasts. During pre-render + // the toast is a no-op (separate render: no JS toast host), so no special-casing is needed. + if (showCancelSnackbar && ReferenceEquals(loadSource, _cts) && !_disposed) { Toast.Info(Localizer["Grid.Snackbar.LoadCancelled"]); } diff --git a/Source/Presentation/MMCA.Common.UI/Resources/SharedResource.es.resx b/Source/Presentation/MMCA.Common.UI/Resources/SharedResource.es.resx index 71df50af..9a7b2f25 100644 --- a/Source/Presentation/MMCA.Common.UI/Resources/SharedResource.es.resx +++ b/Source/Presentation/MMCA.Common.UI/Resources/SharedResource.es.resx @@ -224,7 +224,7 @@ Error de autenticación: falta el código de intercambio. - Este enlace de inicio de sesion no procede de un inicio de sesion comenzado en este dispositivo. Vuelve a iniciar sesion. + Este enlace de inicio de sesión no procede de un inicio de sesión comenzado en este dispositivo. Vuelve a iniciar sesión. elemento diff --git a/Source/Presentation/MMCA.Common.UI/Services/Notifications/INotificationScopeProvider.cs b/Source/Presentation/MMCA.Common.UI/Services/Notifications/INotificationScopeProvider.cs index 0fec43f9..1beca8e5 100644 --- a/Source/Presentation/MMCA.Common.UI/Services/Notifications/INotificationScopeProvider.cs +++ b/Source/Presentation/MMCA.Common.UI/Services/Notifications/INotificationScopeProvider.cs @@ -23,9 +23,11 @@ public interface INotificationScopeProvider /// /// Gets a human-readable name for the scope currently in force (the conference event's title, the - /// tenant's name), or null when there is nothing to show. The send page uses it to caption who a - /// notification will actually reach, so an operator can see the auto-applied target rather than - /// infer it. + /// tenant's name), or null when there is nothing to show. The send page uses it to caption which + /// scope the notification will be tagged with, so an operator can see the auto-applied scope + /// rather than infer it. The scope only decides which inbox view (for example which event's) + /// lists the notification; it does not narrow delivery, which goes to every recipient the + /// application's recipient provider returns, so the caption must not be read as the audience. /// /// It is a default interface method returning null: an application that has no display name, and /// every existing implementation, keeps compiling untouched. The same never-throw, fail-closed diff --git a/Tests/Architecture/MMCA.Common.Architecture.Tests/Governance/SpanishAccentTests.cs b/Tests/Architecture/MMCA.Common.Architecture.Tests/Governance/SpanishAccentTests.cs new file mode 100644 index 00000000..9014c8e9 --- /dev/null +++ b/Tests/Architecture/MMCA.Common.Architecture.Tests/Governance/SpanishAccentTests.cs @@ -0,0 +1,15 @@ +using MMCA.Common.Testing.Architecture; + +namespace MMCA.Common.Architecture.Tests.Governance; + +/// +/// Spanish-localization rule, driven by the shared : no +/// *.es.resx under this repo's Source/ tree ships a common word with its accent or +/// n-tilde missing. The framework's own UI strings are the ones every consumer inherits, so they are +/// held to the same rule the consumers' Spanish resources are. +/// +public sealed class SpanishAccentTests : SpanishAccentTestsBase +{ + protected override string ResourceRoot { get; } = + Path.Combine(ArchitectureMapBase.FindRepoRoot("MMCA.Common.slnx"), "Source"); +} diff --git a/Tests/Core/MMCA.Common.Infrastructure.Tests/Auth/LoginProtectionServiceHybridCacheTests.cs b/Tests/Core/MMCA.Common.Infrastructure.Tests/Auth/LoginProtectionServiceHybridCacheTests.cs new file mode 100644 index 00000000..07756169 --- /dev/null +++ b/Tests/Core/MMCA.Common.Infrastructure.Tests/Auth/LoginProtectionServiceHybridCacheTests.cs @@ -0,0 +1,128 @@ +using System.Collections.Concurrent; +using System.Globalization; +using AwesomeAssertions; +using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using MMCA.Common.Application.Interfaces; +using MMCA.Common.Infrastructure.Auth; +using MMCA.Common.Shared.Abstractions; + +namespace MMCA.Common.Infrastructure.Tests.Auth; + +/// +/// Runs over the REAL two-level cache that +/// AddCommonHybridCache registers (an in-process L1 in front of a shared +/// L2), with the default +/// . The unit tests in +/// use a single-level fake, so they cannot see a counter whose read and write legs land on +/// different tiers: the write goes to the shared store while the read is answered from a local copy. +/// +public sealed class LoginProtectionServiceHybridCacheTests +{ + private const string TestIp = "203.0.113.7"; + private const string TestEmail = "user@example.com"; + + [Fact] + public async Task RegistrationBurst_FromOneIp_OverTheHybridCache_RefusesTheRegistrationPastTheHourlyLimit() + { + await using var provider = BuildProvider(); + var sut = CreateSut(provider); + var settings = new LoginProtectionSettings(); + var cancellationToken = TestContext.Current.CancellationToken; + + // The registration flow: check the limit, then record the registration. The first + // MaxRegistrationsPerIpPerHour registrations are allowed. + for (var registration = 1; registration <= settings.MaxRegistrationsPerIpPerHour; registration++) + { + Result allowed = await sut.CheckRegistrationRateLimitAsync(TestIp, cancellationToken); + allowed.IsSuccess.Should().BeTrue(string.Create(CultureInfo.InvariantCulture, $"registration {registration} is within the limit of {settings.MaxRegistrationsPerIpPerHour}")); + await sut.IncrementRegistrationCountAsync(TestIp, cancellationToken); + } + + Result refused = await sut.CheckRegistrationRateLimitAsync(TestIp, cancellationToken); + + refused.IsFailure.Should().BeTrue( + string.Create(CultureInfo.InvariantCulture, $"{settings.MaxRegistrationsPerIpPerHour} registrations from one IP were already recorded inside the window, so registration {settings.MaxRegistrationsPerIpPerHour + 1} must be refused; a check answered from a stale in-process copy of the counter never sees the shared count grow")); + refused.Errors.Should().ContainSingle(e => e.Code == "Auth.RegistrationRateLimitExceeded"); + } + + [Fact] + public async Task FailedLoginBurst_OverTheHybridCache_LocksTheAccountAfterMaxFailedAttempts() + { + await using var provider = BuildProvider(); + var sut = CreateSut(provider); + var settings = new LoginProtectionSettings(); + var cancellationToken = TestContext.Current.CancellationToken; + + // The sign-in flow: check the lockout, then record the failed attempt. The first + // MaxFailedAttempts attempts are let through to the password check. + for (var attempt = 1; attempt <= settings.MaxFailedAttempts; attempt++) + { + Result allowed = await sut.CheckLockoutAsync(TestEmail, cancellationToken); + allowed.IsSuccess.Should().BeTrue(string.Create(CultureInfo.InvariantCulture, $"attempt {attempt} is within the {settings.MaxFailedAttempts} permitted failures")); + await sut.IncrementFailedAttemptsAsync(TestEmail, cancellationToken); + } + + Result locked = await sut.CheckLockoutAsync(TestEmail, cancellationToken); + + locked.IsFailure.Should().BeTrue( + string.Create(CultureInfo.InvariantCulture, $"{settings.MaxFailedAttempts} consecutive failures were recorded, so the next attempt must be locked out")); + locked.Errors.Should().ContainSingle(e => e.Code == "Auth.TooManyAttempts"); + } + + /// + /// The production two-level registration: stands in for Redis as the + /// shared L2, and AddCommonHybridCache supplies the L1, the entry policy and the + /// . + /// + /// Not AddDistributedMemoryCache: + /// recognizes MemoryDistributedCache and runs with NO L2 at all, so a counter that bypasses + /// L1 would never be stored and every increment would read zero. + /// + /// + /// The provider owning the cache. + private static ServiceProvider BuildProvider() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddSingleton(new SharedStore()); + services.AddCommonHybridCache(); + return services.BuildServiceProvider(); + } + + private static LoginProtectionService CreateSut(ServiceProvider provider) => + new(provider.GetRequiredService(), Options.Create(new LoginProtectionSettings())); + + /// In-memory playing the shared store (Redis) every replica reads. + private sealed class SharedStore : IDistributedCache + { + private readonly ConcurrentDictionary _store = new(StringComparer.Ordinal); + + public byte[]? Get(string key) => _store.TryGetValue(key, out var bytes) ? bytes : null; + + public Task GetAsync(string key, CancellationToken token = default) => Task.FromResult(Get(key)); + + public void Refresh(string key) + { + } + + public Task RefreshAsync(string key, CancellationToken token = default) => Task.CompletedTask; + + public void Remove(string key) => _store.TryRemove(key, out _); + + public Task RemoveAsync(string key, CancellationToken token = default) + { + Remove(key); + return Task.CompletedTask; + } + + public void Set(string key, byte[] value, DistributedCacheEntryOptions options) => _store[key] = value; + + public Task SetAsync(string key, byte[] value, DistributedCacheEntryOptions options, CancellationToken token = default) + { + Set(key, value, options); + return Task.CompletedTask; + } + } +} diff --git a/Tests/Hosting/MMCA.Common.Aspire.Tests/Gateway/GatewayDownstreamHealthCheckPollingTests.cs b/Tests/Hosting/MMCA.Common.Aspire.Tests/Gateway/GatewayDownstreamHealthCheckPollingTests.cs new file mode 100644 index 00000000..59b04f1e --- /dev/null +++ b/Tests/Hosting/MMCA.Common.Aspire.Tests/Gateway/GatewayDownstreamHealthCheckPollingTests.cs @@ -0,0 +1,155 @@ +using System.Net; +using AwesomeAssertions; +using AwesomeAssertions.Execution; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Hosting; +using MMCA.Common.Aspire.Gateway; + +namespace MMCA.Common.Aspire.Tests.Gateway; + +/// +/// Drives the downstream checks the way a gateway's readiness endpoint does: through the REAL +/// AddGatewayDownstreamHealthChecks registration and , one +/// CheckHealthAsync per poll. The unit tests in +/// call one hand-built check instance twice, so they cannot see what the health-check service does +/// between polls (build the check again from its registration factory), nor what the HttpClient +/// pipeline AddServiceDefaults() wraps around the probe client. +/// +/// The downstream here is a cleartext Http1AndHttp2 head without ALPN: it refuses HTTP/2 with a +/// protocol error and answers HTTP/1.1. The negotiation must happen once per process, and a refused +/// attempt must reach the check at once so it can fall back inside the two-second probe budget. +/// +/// +public sealed class GatewayDownstreamHealthCheckPollingTests +{ + private const string ServiceName = "catalog"; + + private static readonly ProbeAttempt Http2Attempt = + new(HttpVersion.Version20, HttpVersionPolicy.RequestVersionExact); + + private static readonly ProbeAttempt Http11Attempt = + new(HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrLower); + + [Fact] + public async Task ReadinessPolls_ThroughTheRegisteredCheck_ReuseTheVersionLatchedOnTheFirstPoll() + { + var recorder = new AttemptRecorder(); + var services = new ServiceCollection(); + services.AddLogging(); + services.AddGatewayDownstreamHealthChecks(ServiceName); + services.AddHttpClient(GatewayHealthCheckExtensions.ClientName(ServiceName)) + .ConfigurePrimaryHttpMessageHandler(() => new RefusesHttp2Handler(recorder)); + await using var provider = services.BuildServiceProvider(); + var healthChecks = provider.GetRequiredService(); + + var first = await PollAsync(healthChecks); + var firstAttempts = recorder.TakeAll(); + var second = await PollAsync(healthChecks); + var secondAttempts = recorder.TakeAll(); + + using (new AssertionScope()) + { + first.Status.Should().Be(HealthStatus.Healthy); + firstAttempts.Should().Equal(Http2Attempt, Http11Attempt); + second.Status.Should().Be(HealthStatus.Healthy); + secondAttempts.Should().Equal( + [Http11Attempt], + "the first poll settled on HTTP/1.1, and that latch must outlive the poll: a check rebuilt per poll renegotiates (and re-sends the refused HTTP/2 attempt) every time"); + } + } + + [Fact] + public async Task ReadinessPolls_UnderServiceDefaults_FallBackWithoutARetryAndStayHealthyWithinTheProbeBudget() + { + var recorder = new AttemptRecorder(); + var builder = Host.CreateEmptyApplicationBuilder(new HostApplicationBuilderSettings()); + builder.AddServiceDefaults(); + builder.Services.AddGatewayDownstreamHealthChecks(ServiceName); + + // Only the transport is replaced; the resilience and service-discovery handlers that + // AddServiceDefaults() puts on every factory client stay in the probe pipeline. + builder.Services.AddHttpClient(GatewayHealthCheckExtensions.ClientName(ServiceName)) + .ConfigurePrimaryHttpMessageHandler(() => new RefusesHttp2Handler(recorder)); + using var host = builder.Build(); + var healthChecks = host.Services.GetRequiredService(); + + var first = await PollAsync(healthChecks); + var firstAttempts = recorder.TakeAll(); + var second = await PollAsync(healthChecks); + var secondAttempts = recorder.TakeAll(); + + using (new AssertionScope()) + { + firstAttempts.Should().Equal( + [Http2Attempt, Http11Attempt], + "a refused HTTP/2 attempt must go straight to the HTTP/1.1 fallback: a Polly retry of the refused attempt (about two seconds of backoff) spends the whole probe budget and reports a healthy downstream as Unhealthy"); + first.Status.Should().Be(HealthStatus.Healthy); + first.Duration.Should().BeLessThan(GatewayHealthCheckExtensions.ProbeTimeout); + secondAttempts.Should().Equal( + [Http11Attempt], + "the version latched on the first poll must survive to the next one, so the second poll sends no HTTP/2 attempt"); + second.Status.Should().Be(HealthStatus.Healthy); + second.Duration.Should().BeLessThan(GatewayHealthCheckExtensions.ProbeTimeout); + } + } + + private static async Task PollAsync(HealthCheckService healthChecks) + { + var checkName = GatewayHealthCheckExtensions.CheckName(ServiceName); + var report = await healthChecks.CheckHealthAsync( + registration => registration.Name == checkName, + TestContext.Current.CancellationToken); + return report.Entries[checkName]; + } + + /// One request that reached the transport, with the version profile it asked for. + /// The HTTP version on the request. + /// The version policy on the request. + private sealed record ProbeAttempt(Version Version, HttpVersionPolicy Policy); + + /// Collects the attempts that reached the transport, drained once per poll. + private sealed class AttemptRecorder + { + private readonly Lock _gate = new(); + private readonly List _attempts = []; + + public void Add(ProbeAttempt attempt) + { + lock (_gate) + { + _attempts.Add(attempt); + } + } + + public ProbeAttempt[] TakeAll() + { + lock (_gate) + { + ProbeAttempt[] taken = [.. _attempts]; + _attempts.Clear(); + return taken; + } + } + } + + /// + /// The transport of a cleartext Http1AndHttp2 downstream without ALPN: HTTP/2 is refused with + /// the protocol error, HTTP/1.1 answers 200. Each attempt is recorded BEFORE it is answered, so a + /// refused one still counts. + /// + private sealed class RefusesHttp2Handler(AttemptRecorder recorder) : HttpMessageHandler + { + protected override Task SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken) + { + recorder.Add(new ProbeAttempt(request.Version, request.VersionPolicy)); + + return request.Version == HttpVersion.Version20 + ? Task.FromException( + new HttpRequestException(HttpRequestError.VersionNegotiationError, "HTTP_1_1_REQUIRED")) + : Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)); + } + } +} diff --git a/Tests/Presentation/MMCA.Common.UI.Tests/Pages/Common/DataGridListPageBaseTests.cs b/Tests/Presentation/MMCA.Common.UI.Tests/Pages/Common/DataGridListPageBaseTests.cs index 6e646b7e..80037a18 100644 --- a/Tests/Presentation/MMCA.Common.UI.Tests/Pages/Common/DataGridListPageBaseTests.cs +++ b/Tests/Presentation/MMCA.Common.UI.Tests/Pages/Common/DataGridListPageBaseTests.cs @@ -448,13 +448,12 @@ public async Task LoadServerDataAsync_WhenASupersededLoadEnds_LoadingStaysOnUnti TaskCreationOptions.RunContinuationsAsynchronously); var newestStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var supersededEnded = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - _toast.Setup(t => t.Info(It.IsAny())).Callback(() => supersededEnded.TrySetResult()); var calls = 0; cut.Instance.Fetch = (_, _, _, _, _, _) => { if (++calls == 1) { - return supersededFetch.Task; + return SignalWhenEndedAsync(supersededFetch.Task, supersededEnded); } newestStarted.TrySetResult(); @@ -471,8 +470,9 @@ public async Task LoadServerDataAsync_WhenASupersededLoadEnds_LoadingStaysOnUnti await cut.InvokeAsync(() => { }); cut.Instance.LoadingNow.Should().BeTrue(); - // A ends now, while B is still in flight (its cancellation raises the info toast from its - // catch, right before its finally runs on the same dispatcher turn). + // A ends now, while B is still in flight. The signal fires as A's fetch ends, on the same + // dispatcher turn that then runs A's catch and finally (independent of whether a superseded + // load raises a toast, which it must not: see the O-39 tests below). supersededFetch.SetException(new OperationCanceledException()); await supersededEnded.Task.WaitAsync(TimeSpan.FromSeconds(10), Xunit.TestContext.Current.CancellationToken); await cut.InvokeAsync(() => { }); @@ -503,6 +503,97 @@ public async Task LoadServerDataAsync_WhenTheUserCancelsTheLatestLoad_LoadingEnd cut.Instance.LoadingNow.Should().BeFalse("a user cancel ends the latest load, and with it the loading state"); } + // == Cancel toast: only a cancel the user asked for announces itself (O-39) == + [Fact] + public async Task LoadServerDataAsync_WhenALoadIsSupersededByANewerLoad_RaisesNoCancelToastAndShowsTheNewerRows() + { + // A paged grid supersedes its own in-flight load on every pager, sort or filter change: the + // newer ServerData call's reset cancels the older load's token. That is not a cancel the user + // asked for, so the admin-page cancel toast (showCancelSnackbar: true) must stay quiet. + var cut = Render(); + var calls = 0; + cut.Instance.Fetch = (_, _, _, _, _, token) => + ++calls == 1 ? UntilCancelled(token) : Loaded(1, new WidgetRow(4, "Newer")); + + Task>? supersededLoad = null; + Task>? newerLoad = null; + await cut.InvokeAsync(() => + { + supersededLoad = cut.Instance.LoadAsync(State(page: 0, pageSize: 10), showCancelSnackbar: true); + newerLoad = cut.Instance.LoadAsync(State(page: 1, pageSize: 10), showCancelSnackbar: true); + }); + + var newer = await newerLoad!; + var superseded = await supersededLoad!; + + calls.Should().Be(2); + newer.Items.Should().ContainSingle().Which.Name.Should().Be("Newer"); + superseded.Items.Should().ContainSingle().Which.Name.Should().Be("Newer"); + _toast.Verify( + t => t.Info(It.IsAny()), + Times.Never, + "a load superseded by a newer load was not cancelled by the user, so it must not announce 'Loading cancelled.'"); + } + + [Fact] + public async Task LoadServerDataAsync_WhenTheUserCancelsALoad_RaisesExactlyOneLoadingCancelledToast() + { + var cut = Render(); + cut.Instance.Fetch = (_, _, _, _, _, token) => UntilCancelled(token); + + Task>? load = null; + await cut.InvokeAsync(() => { load = cut.Instance.LoadAsync(State(page: 0, pageSize: 10), showCancelSnackbar: true); }); + + await cut.InvokeAsync(cut.Instance.CancelLoading); + var data = await load!; + + data.Items.Should().BeEmpty(); + _toast.Verify(t => t.Info("Loading cancelled."), Times.Once); + _toast.Verify(t => t.Info(It.IsAny()), Times.Once); + } + + [Fact] + public async Task LoadServerDataAsync_WhenTheComponentIsDisposedMidLoad_RaisesNoCancelToast() + { + // Navigating away disposes the page, which cancels its in-flight load. The user is already on + // another page; a "Loading cancelled." toast there describes nothing they did. + var cut = Render(); + var fetchStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + cut.Instance.Fetch = (_, _, _, _, _, token) => + { + fetchStarted.TrySetResult(); + return UntilCancelled(token); + }; + + Task>? load = null; + await cut.InvokeAsync(() => { load = cut.Instance.LoadAsync(State(page: 0, pageSize: 10), showCancelSnackbar: true); }); + await fetchStarted.Task.WaitAsync(TimeSpan.FromSeconds(10), Xunit.TestContext.Current.CancellationToken); + + await cut.InvokeAsync(() => cut.Instance.DisposeAsync().AsTask()); + var data = await load!; + + data.Items.Should().BeEmpty(); + _toast.Verify( + t => t.Info(It.IsAny()), + Times.Never, + "a load cancelled by the component's own disposal was not cancelled by the user"); + } + + // Passes a fetch through unchanged and signals the moment it ends, however it ends. + private static async Task Items, int TotalItems)>> SignalWhenEndedAsync( + Task Items, int TotalItems)>> fetch, + TaskCompletionSource ended) + { + try + { + return await fetch; + } + finally + { + ended.TrySetResult(); + } + } + // A fetch that honors its token: it only ends, by cancellation, when the next load supersedes it. private static Task Items, int TotalItems)>> UntilCancelled(CancellationToken token) { diff --git a/Tests/Presentation/MMCA.Common.UI.Web.Tests/Services/BrowserForwardedForHandlerTests.cs b/Tests/Presentation/MMCA.Common.UI.Web.Tests/Services/BrowserOriginHandlerTests.cs similarity index 65% rename from Tests/Presentation/MMCA.Common.UI.Web.Tests/Services/BrowserForwardedForHandlerTests.cs rename to Tests/Presentation/MMCA.Common.UI.Web.Tests/Services/BrowserOriginHandlerTests.cs index 4be6b38b..b3199b02 100644 --- a/Tests/Presentation/MMCA.Common.UI.Web.Tests/Services/BrowserForwardedForHandlerTests.cs +++ b/Tests/Presentation/MMCA.Common.UI.Web.Tests/Services/BrowserOriginHandlerTests.cs @@ -7,15 +7,17 @@ namespace MMCA.Common.UI.Web.Tests.Services; /// -/// Pins : a server-side API call made for a visitor carries -/// that visitor's address in X-Forwarded-For as the single value, nothing is sent when no -/// visitor request is in scope, and AddCommonServerTokenStorage() composes the handler onto the -/// "APIClient" pipeline. +/// Pins : a server-side API call made for a visitor carries that +/// visitor's address in X-Forwarded-For and the browser's user-agent in User-Agent, each +/// as the single value, nothing is sent when no visitor request is in scope, and +/// AddCommonServerTokenStorage() composes the handler onto the "APIClient" pipeline. /// -public sealed class BrowserForwardedForHandlerTests +public sealed class BrowserOriginHandlerTests { private const string HeaderName = "X-Forwarded-For"; + private const string UserAgentHeaderName = "User-Agent"; private const string BrowserIp = "198.51.100.23"; + private const string BrowserUserAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36"; private static readonly Uri ApiUri = new("https://gateway.example.com/Auth/register"); [Fact] @@ -46,6 +48,7 @@ public async Task SendAsync_WithNoRequestInScope_SendsNoHeader() await client.GetAsync(ApiUri, TestContext.Current.CancellationToken); inner.LastRequest!.Headers.Contains(HeaderName).Should().BeFalse(); + inner.LastRequest.Headers.Contains(UserAgentHeaderName).Should().BeFalse(); } [Fact] @@ -84,6 +87,39 @@ public async Task SendAsync_IgnoresAForwardedForHeaderTheBrowserSentToThisHost() inner.LastRequest!.Headers.GetValues(HeaderName).Should().ContainSingle().Which.Should().Be(BrowserIp); } + [Fact] + public async Task SendAsync_WithAVisitorRequestInScope_ForwardsTheBrowserUserAgent() + { + var (client, inner) = CreateClient(VisitorContext(BrowserIp, BrowserUserAgent)); + + await client.PostAsync(ApiUri, content: null, TestContext.Current.CancellationToken); + + inner.LastRequest!.Headers.UserAgent.ToString().Should().Be(BrowserUserAgent); + } + + [Fact] + public async Task SendAsync_WhenTheRequestAlreadyCarriesAUserAgent_ReplacesIt() + { + var (client, inner) = CreateClient(VisitorContext(BrowserIp, BrowserUserAgent)); + using var request = new HttpRequestMessage(HttpMethod.Get, ApiUri); + request.Headers.TryAddWithoutValidation(UserAgentHeaderName, "MMCA-Host/1.0"); + + await client.SendAsync(request, TestContext.Current.CancellationToken); + + inner.LastRequest!.Headers.UserAgent.ToString().Should().Be(BrowserUserAgent); + } + + [Fact] + public async Task SendAsync_WhenTheBrowserSentNoUserAgent_SendsNone() + { + var (client, inner) = CreateClient(VisitorContext(BrowserIp, userAgent: " ")); + + await client.GetAsync(ApiUri, TestContext.Current.CancellationToken); + + inner.LastRequest!.Headers.Contains(UserAgentHeaderName).Should().BeFalse(); + inner.LastRequest.Headers.GetValues(HeaderName).Should().ContainSingle().Which.Should().Be(BrowserIp); + } + [Fact] public async Task AddCommonServerTokenStorage_ComposesTheHandlerOntoTheApiClient() { @@ -92,25 +128,31 @@ public async Task AddCommonServerTokenStorage_ComposesTheHandlerOntoTheApiClient services.AddCommonServerTokenStorage(); services.AddHttpClient("APIClient").ConfigurePrimaryHttpMessageHandler(() => inner); await using var provider = services.BuildServiceProvider(); - provider.GetRequiredService().HttpContext = VisitorContext(BrowserIp); + provider.GetRequiredService().HttpContext = VisitorContext(BrowserIp, BrowserUserAgent); using var client = provider.GetRequiredService().CreateClient("APIClient"); await client.GetAsync(ApiUri, TestContext.Current.CancellationToken); inner.LastRequest!.Headers.GetValues(HeaderName).Should().ContainSingle().Which.Should().Be(BrowserIp); + inner.LastRequest.Headers.UserAgent.ToString().Should().Be(BrowserUserAgent); } - private static DefaultHttpContext VisitorContext(string remoteIp) + private static DefaultHttpContext VisitorContext(string remoteIp, string? userAgent = null) { var context = new DefaultHttpContext(); context.Connection.RemoteIpAddress = IPAddress.Parse(remoteIp); + if (userAgent is not null) + { + context.Request.Headers.UserAgent = userAgent; + } + return context; } private static (HttpClient Client, CapturingHandler Inner) CreateClient(HttpContext? httpContext) { var inner = new CapturingHandler(); - var handler = new BrowserForwardedForHandler(new HttpContextAccessor { HttpContext = httpContext }) + var handler = new BrowserOriginHandler(new HttpContextAccessor { HttpContext = httpContext }) { InnerHandler = inner, };