diff --git a/Cleanup.md b/Cleanup.md new file mode 100644 index 00000000..ed8f8eeb --- /dev/null +++ b/Cleanup.md @@ -0,0 +1,59 @@ +# Backend Cleanup Audit + +Audit of the backend layers (`FinanceManager.Api`, `FinanceManager.Application`, +`FinanceManager.Domain`, `FinanceManager.Infrastructure`). Migrations and generated +files are excluded. + +## Overall assessment + +The backend is in good shape. Clean-architecture boundaries are respected: + +- `FinanceManager.Domain` has **no** EF Core / ASP.NET references (verified — the only + `AppDbContext` / `EntityFrameworkCore` mentions in `Application` are inside code comments, + not real dependencies). +- No business logic / DB access leaks into controllers of note; controllers stay thin. +- Naming conventions are consistent: private fields are `_camelCase`, interfaces `IPascalCase`, + namespaces match folders. No violations found by scanning private/public field declarations. + +The findings below are therefore mostly about **complexity / size** and one **latent bug**, +not systemic architecture problems. + +## Findings & tasks + +### 1. `BondEntryRepository.AddLabel` is a no-op (latent bug) — DONE +`Infrastructure/Repositories/Account/Entry/BondEntryRepository.cs:445` fetches the entry and +the label, then calls `SaveChangesAsync()` **without ever adding the label to the entry**. It +returns `true` while doing nothing. `CurrencyEntryRepository.AddLabel` implements the same +contract correctly. `AddLabel` currently has no production call sites (only the bulk `AddLabels` +path is used), so the bug is latent — but it should be fixed and covered by a test. +- **Action**: mirror the correct `CurrencyEntryRepository` implementation; add a unit test. + +### 2. `Api/Program.cs` (339 lines) — too much inline configuration — DONE +The startup file inlines options binding for ~12 option types, the full JWT/CORS/forwarded-headers +setup, and a long block of hosted-service registrations. This is readable but monolithic. +- **Action**: extract cohesive `IServiceCollection` extension methods + (`AddApiOptions`, `AddApiSecurity`, `AddApiBackgroundServices`) so `Program.cs` reads as a + high-level outline. Pure move — no behaviour change. + +### 3. `Application/.../Stock/Resolution/InstrumentResolver.cs` (400 lines) — mixed responsibilities (recommended follow-up) +The resolver does three jobs: (a) orchestrate the AV + OpenFIGI fan-out and reconcile candidates, +(b) **persist** the auto-resolved instrument into the asset model (`PersistInvestmentModel`, +`BuildIdentifiers`, `ResolveVenue`, `MapAssetType`, `PriceMultiplierFor`), and (c) manage the +OpenFIGI failure cooldown. The persistence + mapping half is a distinct responsibility. +- **Action**: extract a `ResolvedInstrumentPersister` collaborator (with the venue/type/identifier + mapping helpers) injected into the resolver. Left as a follow-up because it sits on the pricing + hot path and changes the manually-wired constructor in `Infrastructure/ServiceCollectionExtension.cs`; + it warrants its own focused PR even though `InstrumentResolverTests` already exists. + +### 4. `BondEntryRepository` (567 lines) — duplicated neighbour-lookup logic (recommended follow-up) +The class carries two implementations of the per-instrument "next older / next younger" lookup: +the explicit-interface `IBondAccountEntryRepository.GetNextOlder/GetNextYounger(accountId, date)` +run an **N+1 query per BondDetailsId in a loop**, while `GetNextOlderPerInstrument` / +`GetNextYoungerPerInstrument` already express the same intent as a single set-based query. +- **Action**: collapse the looping explicit-interface implementations onto the set-based query to + remove the N+1 and the duplication. Left as a follow-up (needs repository test coverage first). + +### 5. Large infrastructure files worth watching (no action now) +`CurrencyEntryRepository.cs` (484), `AccountRepository.cs` (337), +`CachedAccountEntryRepository.cs` (336), `Api/Services/CurrencyImportJobStore.cs` (277), +`AlphaVantageClient.cs` (220). These are cohesive; noted for future monitoring, not split now. diff --git a/code/Directory.Packages.props b/code/Directory.Packages.props index b7c9d5ff..6a7cd83f 100644 --- a/code/Directory.Packages.props +++ b/code/Directory.Packages.props @@ -28,6 +28,9 @@ + + diff --git a/code/FinanceManager.Api/ApiConfigurationExtensions.cs b/code/FinanceManager.Api/ApiConfigurationExtensions.cs new file mode 100644 index 00000000..2760eb5c --- /dev/null +++ b/code/FinanceManager.Api/ApiConfigurationExtensions.cs @@ -0,0 +1,286 @@ +using FinanceManager.Api.Logging; +using FinanceManager.Api.Services; +using FinanceManager.Api.Services.Guest; +using FinanceManager.Application.Shared.Options; +using FinanceManager.Domain.Identity.Services; +using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.AspNetCore.HttpOverrides; +using Microsoft.Extensions.Hosting; +using Microsoft.IdentityModel.Tokens; +using System.Security.Claims; +using System.Text; + +namespace FinanceManager.Api; + +/// +/// Groups the API host's DI wiring into cohesive extension methods so Program.cs reads as a +/// high-level outline. These are pure moves of the original inline registrations — no behaviour change. +/// +public static class ApiConfigurationExtensions +{ + /// + /// Binds and validates the strongly-typed options used across the API (JWT, refresh/reset tokens, + /// account lockout, external providers, AI provider fallback) and configures antiforgery. + /// + public static IServiceCollection AddApiOptions(this IServiceCollection services, IConfiguration configuration) + { + services.Configure(configuration.GetSection("JwtConfig")); + services.Configure(configuration.GetSection(RefreshTokenOptions.SectionName)); + services.Configure(configuration.GetSection(PasswordResetOptions.SectionName)); + services.AddAntiforgery(options => + { + options.HeaderName = "X-CSRF-Token"; + options.Cookie.Name = "fm_antiforgery"; + options.Cookie.HttpOnly = true; + options.Cookie.SameSite = SameSiteMode.Strict; + options.Cookie.SecurePolicy = CookieSecurePolicy.SameAsRequest; + }); + services.AddOptions() + .Bind(configuration.GetSection(AccountLockoutOptions.SectionName)) + .Validate(o => o.MaxFailedAttempts > 0, "AccountLockout:MaxFailedAttempts must be greater than 0.") + .Validate(o => o.LockoutDuration > TimeSpan.Zero, "AccountLockout:LockoutDuration must be greater than 0.") + .ValidateOnStart(); + services.AddOptions() + .Bind(configuration.GetSection("StockApi")) + .Validate(o => !string.IsNullOrWhiteSpace(o.BaseUrl), "StockApi:BaseUrl must be configured.") + .ValidateOnStart(); + services.Configure(configuration.GetSection("OpenFigi")); + services.AddOptions() + .Bind(configuration.GetSection("Eodhd")) + .Validate(o => !string.IsNullOrWhiteSpace(o.BaseUrl), "Eodhd:BaseUrl must be configured.") + .ValidateOnStart(); + services.Configure(configuration.GetSection("LmStudio")); + services.AddOptions() + .Bind(configuration.GetSection("OpenRouter")) + .Validate(o => !string.IsNullOrWhiteSpace(o.BaseUrl), "OpenRouter:BaseUrl must be configured.") + .Validate(o => o.RequestTimeoutSeconds > 0, "OpenRouter:RequestTimeoutSeconds must be greater than 0.") + .ValidateOnStart(); + services.AddOptions() + .Bind(configuration.GetSection("GitHubModels")) + .Validate(o => !string.IsNullOrWhiteSpace(o.BaseUrl), "GitHubModels:BaseUrl must be configured.") + .Validate(o => o.RequestTimeoutSeconds > 0, "GitHubModels:RequestTimeoutSeconds must be greater than 0.") + .ValidateOnStart(); + services.AddOptions() + .Bind(configuration.GetSection("Ollama")) + .Validate(o => !string.IsNullOrWhiteSpace(o.BaseUrl), "Ollama:BaseUrl must be configured.") + .ValidateOnStart(); + services.Configure(configuration.GetSection("AiProvider")); + services.Configure>(configuration.GetSection("AIProviderFallbackStrategies")); + + return services; + } + + /// + /// Resolves and validates the JWT signing key, then wires CORS, JWT bearer authentication (including + /// the SignalR access-token and guest-session hooks) and reverse-proxy forwarded-header handling. + /// Fails fast outside Development when required secrets/allowlists are missing. + /// + public static IServiceCollection AddApiSecurity(this IServiceCollection services, IConfiguration configuration, IHostEnvironment environment) + { + // The JWT signing key must never be committed. It is supplied via environment variable / User Secrets. + // Fail fast outside Development so we never silently fall back to an insecure default; in Development + // fall back to a clearly-insecure local key so the app still runs without configured secrets. + var jwtSigningKey = configuration["JwtConfig:Key"]; + if (string.IsNullOrWhiteSpace(jwtSigningKey)) + { + if (!environment.IsDevelopment()) + throw new InvalidOperationException( + "JwtConfig:Key is not configured. Provide a signing key via the JwtConfig__Key environment variable or User Secrets."); + + jwtSigningKey = "development-only-insecure-jwt-signing-key-do-not-use-in-production"; + configuration["JwtConfig:Key"] = jwtSigningKey; + } + else if (Encoding.UTF8.GetByteCount(jwtSigningKey) < 64) + { + // HMAC-SHA512 (used by JwtTokenGenerator) needs a 512-bit (64-byte) key; reject anything + // weaker so a short env var can't silently produce forgeable tokens or crash at sign time. + throw new InvalidOperationException( + "JwtConfig:Key must be at least 64 bytes (512 bits) for HMAC-SHA512 signing."); + } + + var tokenValidityMins = configuration.GetValue("JwtConfig:TokenValidityMins"); + if (tokenValidityMins <= 0) + throw new InvalidOperationException( + "JwtConfig:TokenValidityMins must be greater than 0. Set it to a positive number (e.g. 15) in appsettings or User Secrets."); + + // ValidateIssuer/ValidateAudience are enabled below, so an empty issuer/audience would reject + // every token at request time. Fail fast at startup instead — JwtTokenGenerator mints tokens + // with these same values, so both must be configured for auth to work at all. + var jwtIssuer = configuration["JwtConfig:Issuer"]; + if (string.IsNullOrWhiteSpace(jwtIssuer)) + throw new InvalidOperationException( + "JwtConfig:Issuer is not configured. Set it in appsettings or User Secrets."); + + var jwtAudience = configuration["JwtConfig:Audience"]; + if (string.IsNullOrWhiteSpace(jwtAudience)) + throw new InvalidOperationException( + "JwtConfig:Audience is not configured. Set it in appsettings or User Secrets."); + + var allowedCorsOrigins = configuration.GetSection("Cors:AllowedOrigins").Get() + ?.Where(origin => !string.IsNullOrWhiteSpace(origin)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToArray(); + + // Outside Development an empty/missing allowlist must fail fast: never silently fall back to + // AllowAnyOrigin() in production, which would let any site call the API cross-origin. + if (allowedCorsOrigins is not { Length: > 0 } && !environment.IsDevelopment()) + throw new InvalidOperationException( + "Cors:AllowedOrigins is not configured. Provide explicit allowed origins via the " + + "Cors__AllowedOrigins configuration; AllowAnyOrigin is only permitted in Development."); + + services.AddCors(options => + { + options.AddPolicy("ApiCorsPolicy", + corsPolicyBuilder => + { + if (allowedCorsOrigins is { Length: > 0 }) + { + corsPolicyBuilder.WithOrigins(allowedCorsOrigins) + .AllowAnyHeader() + .AllowAnyMethod(); + + return; + } + + // Reachable only in Development (the guard above throws otherwise): allow any origin + // so local tooling and ad-hoc clients work without configuring an allowlist. + corsPolicyBuilder.AllowAnyOrigin() + .AllowAnyHeader() + .AllowAnyMethod(); + }); + }).AddAuthentication(options => + { + options.DefaultAuthenticateScheme = JwtBearerDefaults.AuthenticationScheme; + options.DefaultChallengeScheme = JwtBearerDefaults.AuthenticationScheme; + options.DefaultScheme = JwtBearerDefaults.AuthenticationScheme; + }).AddJwtBearer(options => + { + options.RequireHttpsMetadata = !environment.IsDevelopment(); + options.SaveToken = true; + options.TokenValidationParameters = new TokenValidationParameters + { + ValidIssuer = jwtIssuer, + ValidAudience = jwtAudience, + IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(jwtSigningKey)), + ValidateIssuer = true, + ValidateAudience = true, + ValidateLifetime = true, + ValidateIssuerSigningKey = true, + }; + + options.Events = new JwtBearerEvents + { + OnMessageReceived = context => + { + var accessToken = context.Request.Query["access_token"]; + var path = context.HttpContext.Request.Path; + if (!string.IsNullOrWhiteSpace(accessToken) && + (path.StartsWithSegments("/hubs/currency-import") + || path.StartsWithSegments("/hubs/label-setter-progress") + || path.StartsWithSegments("/hubs/admin-logs"))) + context.Token = accessToken; + + return Task.CompletedTask; + }, + OnTokenValidated = context => + { + var principal = context.Principal; + var isGuestClaim = principal?.FindFirst(GuestClaims.IsGuest)?.Value; + if (!string.Equals(isGuestClaim, "true", StringComparison.OrdinalIgnoreCase)) + return Task.CompletedTask; + + var idClaim = principal?.FindFirst(ClaimTypes.NameIdentifier)?.Value; + if (!int.TryParse(idClaim, out var guestUserId)) + { + context.Fail("Guest token is missing a user id."); + return Task.CompletedTask; + } + + var store = context.HttpContext.RequestServices.GetRequiredService(); + if (!store.IsActive(guestUserId)) + context.Fail("Guest session has expired."); + + return Task.CompletedTask; + } + }; + }); + + // The app runs behind a TLS-terminating reverse proxy (Cloudflare) in production, so the request + // arriving at Kestrel is plain HTTP from the proxy. Honour X-Forwarded-Proto/For so Request.IsHttps, + // Request.Scheme and the remote IP reflect the original client request — this is what makes the + // Secure refresh-token cookie, HTTPS redirect and per-client rate limiting behave correctly. + // Only trust headers forwarded by the declared proxy addresses/networks; trusting any source would + // allow a client that reaches Kestrel directly to spoof scheme or IP. + var reverseProxyIps = configuration.GetSection("ReverseProxy:KnownProxies").Get() ?? []; + var reverseProxyNetworks = configuration.GetSection("ReverseProxy:KnownNetworks").Get() ?? []; + + if (!environment.IsDevelopment() && reverseProxyIps.Length == 0 && reverseProxyNetworks.Length == 0) + throw new InvalidOperationException( + "ReverseProxy:KnownProxies or ReverseProxy:KnownNetworks must be configured outside Development. " + + "Set these to the IP addresses or CIDR ranges of your reverse proxy (e.g. Cloudflare's published ranges)."); + + // Parse and validate every entry up front. A silently-dropped typo would shrink the trusted-proxy + // set (or empty it entirely), leaving the app unable to see the real client scheme/IP — fail fast. + var knownProxies = new List(); + foreach (var proxy in reverseProxyIps) + { + if (!System.Net.IPAddress.TryParse(proxy, out var ip)) + throw new InvalidOperationException( + $"ReverseProxy:KnownProxies contains an invalid IP address: '{proxy}'."); + knownProxies.Add(ip); + } + + var knownNetworks = new List(); + foreach (var network in reverseProxyNetworks) + { + if (!System.Net.IPNetwork.TryParse(network, out var net)) + throw new InvalidOperationException( + $"ReverseProxy:KnownNetworks contains an invalid CIDR network: '{network}'."); + knownNetworks.Add(net); + } + + services.Configure(options => + { + options.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; + options.KnownIPNetworks.Clear(); + options.KnownProxies.Clear(); + + foreach (var ip in knownProxies) + options.KnownProxies.Add(ip); + + foreach (var net in knownNetworks) + options.KnownIPNetworks.Add(net); + }); + + return services; + } + + /// + /// Registers the hosted background services and their channels/stores (guest cleanup, insights + /// generation, label setting, currency import) plus database-backed logging and log retention. + /// + public static IServiceCollection AddApiBackgroundServices(this IServiceCollection services, IConfiguration configuration) + { + services.AddScoped(); + services.AddSingleton(); + services.AddScoped(); + services.AddHostedService(); + services.AddSingleton(); + services.AddHostedService(); + services.AddSingleton(); + services.AddSingleton(); + services.AddHostedService(); + services.AddHostedService(); + services.AddSingleton(); + services.AddSingleton(); + services.AddHostedService(); + + services.Configure(configuration.GetSection(LogRetentionOptions.SectionName)); + services.AddSingleton(); + services.AddSingleton(sp => new DatabaseLoggerProvider(sp.GetRequiredService())); + services.AddHostedService(); + services.AddHostedService(); + + return services; + } +} \ No newline at end of file diff --git a/code/FinanceManager.Api/FinanceManager.Api.csproj b/code/FinanceManager.Api/FinanceManager.Api.csproj index 4b3d3779..d618f1a9 100644 --- a/code/FinanceManager.Api/FinanceManager.Api.csproj +++ b/code/FinanceManager.Api/FinanceManager.Api.csproj @@ -12,6 +12,9 @@ + + diff --git a/code/FinanceManager.Api/Program.cs b/code/FinanceManager.Api/Program.cs index af210f61..4fb7785f 100644 --- a/code/FinanceManager.Api/Program.cs +++ b/code/FinanceManager.Api/Program.cs @@ -1,26 +1,16 @@ using FinanceManager.Api; using FinanceManager.Api.Logging; using FinanceManager.Api.Middleware; -using FinanceManager.Api.Services; -using FinanceManager.Api.Services.Guest; using FinanceManager.Application; using FinanceManager.Application.Shared.Diagnostics; using FinanceManager.Application.Shared.Options; -using FinanceManager.Domain.FinancialAccounts.Shared.Services; -using FinanceManager.Domain.Identity.Services; using FinanceManager.Infrastructure; -using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Http.Features; -using Microsoft.AspNetCore.HttpOverrides; -using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.Options; -using Microsoft.IdentityModel.Tokens; using OpenTelemetry.Metrics; using OpenTelemetry.Trace; using Scalar.AspNetCore; using ServiceDefaults; -using System.Security.Claims; -using System.Text; var builder = WebApplication.CreateBuilder(args); builder.AddServiceDefaults(); @@ -31,32 +21,6 @@ .WithMetrics(metrics => metrics.AddMeter(FinanceManagerTelemetry.MeterName)) .WithTracing(tracing => tracing.AddSource(FinanceManagerTelemetry.ActivitySourceName)); -// The JWT signing key must never be committed. It is supplied via environment variable / User Secrets. -// Fail fast outside Development so we never silently fall back to an insecure default; in Development -// fall back to a clearly-insecure local key so the app still runs without configured secrets. -var jwtSigningKey = builder.Configuration["JwtConfig:Key"]; -if (string.IsNullOrWhiteSpace(jwtSigningKey)) -{ - if (!builder.Environment.IsDevelopment()) - throw new InvalidOperationException( - "JwtConfig:Key is not configured. Provide a signing key via the JwtConfig__Key environment variable or User Secrets."); - - jwtSigningKey = "development-only-insecure-jwt-signing-key-do-not-use-in-production"; - builder.Configuration["JwtConfig:Key"] = jwtSigningKey; -} -else if (Encoding.UTF8.GetByteCount(jwtSigningKey) < 64) -{ - // HMAC-SHA512 (used by JwtTokenGenerator) needs a 512-bit (64-byte) key; reject anything - // weaker so a short env var can't silently produce forgeable tokens or crash at sign time. - throw new InvalidOperationException( - "JwtConfig:Key must be at least 64 bytes (512 bits) for HMAC-SHA512 signing."); -} - -var tokenValidityMins = builder.Configuration.GetValue("JwtConfig:TokenValidityMins"); -if (tokenValidityMins <= 0) - throw new InvalidOperationException( - "JwtConfig:TokenValidityMins must be greater than 0. Set it to a positive number (e.g. 15) in appsettings or User Secrets."); - builder.Services .AddProblemDetails() .AddExceptionHandler() @@ -82,167 +46,8 @@ options.Limits.MaxRequestBodySize = RequestBodySizeLimits.GlobalRequestBodyBytes; }); -builder.Services.Configure(builder.Configuration.GetSection("JwtConfig")); -builder.Services.Configure(builder.Configuration.GetSection(RefreshTokenOptions.SectionName)); -builder.Services.Configure(builder.Configuration.GetSection(PasswordResetOptions.SectionName)); -builder.Services.AddAntiforgery(options => -{ - options.HeaderName = "X-CSRF-Token"; - options.Cookie.Name = "fm_antiforgery"; - options.Cookie.HttpOnly = true; - options.Cookie.SameSite = SameSiteMode.Strict; - options.Cookie.SecurePolicy = CookieSecurePolicy.SameAsRequest; -}); -builder.Services.AddOptions() - .Bind(builder.Configuration.GetSection(AccountLockoutOptions.SectionName)) - .Validate(o => o.MaxFailedAttempts > 0, "AccountLockout:MaxFailedAttempts must be greater than 0.") - .Validate(o => o.LockoutDuration > TimeSpan.Zero, "AccountLockout:LockoutDuration must be greater than 0.") - .ValidateOnStart(); -builder.Services.AddOptions() - .Bind(builder.Configuration.GetSection("StockApi")) - .Validate(o => !string.IsNullOrWhiteSpace(o.BaseUrl), "StockApi:BaseUrl must be configured.") - .ValidateOnStart(); -builder.Services.Configure(builder.Configuration.GetSection("OpenFigi")); -builder.Services.AddOptions() - .Bind(builder.Configuration.GetSection("Eodhd")) - .Validate(o => !string.IsNullOrWhiteSpace(o.BaseUrl), "Eodhd:BaseUrl must be configured.") - .ValidateOnStart(); -builder.Services.Configure(builder.Configuration.GetSection("LmStudio")); -builder.Services.AddOptions() - .Bind(builder.Configuration.GetSection("OpenRouter")) - .Validate(o => !string.IsNullOrWhiteSpace(o.BaseUrl), "OpenRouter:BaseUrl must be configured.") - .Validate(o => o.RequestTimeoutSeconds > 0, "OpenRouter:RequestTimeoutSeconds must be greater than 0.") - .ValidateOnStart(); -builder.Services.AddOptions() - .Bind(builder.Configuration.GetSection("GitHubModels")) - .Validate(o => !string.IsNullOrWhiteSpace(o.BaseUrl), "GitHubModels:BaseUrl must be configured.") - .Validate(o => o.RequestTimeoutSeconds > 0, "GitHubModels:RequestTimeoutSeconds must be greater than 0.") - .ValidateOnStart(); -builder.Services.AddOptions() - .Bind(builder.Configuration.GetSection("Ollama")) - .Validate(o => !string.IsNullOrWhiteSpace(o.BaseUrl), "Ollama:BaseUrl must be configured.") - .ValidateOnStart(); -builder.Services.Configure(builder.Configuration.GetSection("AiProvider")); -builder.Services.Configure>(builder.Configuration.GetSection("AIProviderFallbackStrategies")); - -var allowedCorsOrigins = builder.Configuration.GetSection("Cors:AllowedOrigins").Get() - ?.Where(origin => !string.IsNullOrWhiteSpace(origin)) - .Distinct(StringComparer.OrdinalIgnoreCase) - .ToArray(); - -// Outside Development an empty/missing allowlist must fail fast: never silently fall back to -// AllowAnyOrigin() in production, which would let any site call the API cross-origin. -if (allowedCorsOrigins is not { Length: > 0 } && !builder.Environment.IsDevelopment()) - throw new InvalidOperationException( - "Cors:AllowedOrigins is not configured. Provide explicit allowed origins via the " + - "Cors__AllowedOrigins configuration; AllowAnyOrigin is only permitted in Development."); - - -builder.Services.AddCors(options => -{ - options.AddPolicy("ApiCorsPolicy", - corsPolicyBuilder => - { - if (allowedCorsOrigins is { Length: > 0 }) - { - corsPolicyBuilder.WithOrigins(allowedCorsOrigins) - .AllowAnyHeader() - .AllowAnyMethod(); - - return; - } - - // Reachable only in Development (the guard above throws otherwise): allow any origin - // so local tooling and ad-hoc clients work without configuring an allowlist. - corsPolicyBuilder.AllowAnyOrigin() - .AllowAnyHeader() - .AllowAnyMethod(); - }); -}).AddAuthentication(options => -{ - options.DefaultAuthenticateScheme = JwtBearerDefaults.AuthenticationScheme; - options.DefaultChallengeScheme = JwtBearerDefaults.AuthenticationScheme; - options.DefaultScheme = JwtBearerDefaults.AuthenticationScheme; -}).AddJwtBearer(options => -{ - options.RequireHttpsMetadata = !builder.Environment.IsDevelopment(); - options.SaveToken = true; - options.TokenValidationParameters = new TokenValidationParameters - { - ValidIssuer = builder.Configuration["JwtConfig:Issuer"], - ValidAudience = builder.Configuration["JwtConfig:Audience"], - IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(jwtSigningKey)), - ValidateIssuer = true, - ValidateAudience = true, - ValidateLifetime = true, - ValidateIssuerSigningKey = true, - }; - - options.Events = new JwtBearerEvents - { - OnMessageReceived = context => - { - var accessToken = context.Request.Query["access_token"]; - var path = context.HttpContext.Request.Path; - if (!string.IsNullOrWhiteSpace(accessToken) && - (path.StartsWithSegments("/hubs/currency-import") - || path.StartsWithSegments("/hubs/label-setter-progress") - || path.StartsWithSegments("/hubs/admin-logs"))) - context.Token = accessToken; - - return Task.CompletedTask; - }, - OnTokenValidated = context => - { - var principal = context.Principal; - var isGuestClaim = principal?.FindFirst(GuestClaims.IsGuest)?.Value; - if (!string.Equals(isGuestClaim, "true", StringComparison.OrdinalIgnoreCase)) - return Task.CompletedTask; - - var idClaim = principal?.FindFirst(ClaimTypes.NameIdentifier)?.Value; - if (!int.TryParse(idClaim, out var guestUserId)) - { - context.Fail("Guest token is missing a user id."); - return Task.CompletedTask; - } - - var store = context.HttpContext.RequestServices.GetRequiredService(); - if (!store.IsActive(guestUserId)) - context.Fail("Guest session has expired."); - - return Task.CompletedTask; - } - }; -}); - -// The app runs behind a TLS-terminating reverse proxy (Cloudflare) in production, so the request -// arriving at Kestrel is plain HTTP from the proxy. Honour X-Forwarded-Proto/For so Request.IsHttps, -// Request.Scheme and the remote IP reflect the original client request — this is what makes the -// Secure refresh-token cookie, HTTPS redirect and per-client rate limiting behave correctly. -// Only trust headers forwarded by the declared proxy addresses/networks; trusting any source would -// allow a client that reaches Kestrel directly to spoof scheme or IP. -var reverseProxyIps = builder.Configuration.GetSection("ReverseProxy:KnownProxies").Get() ?? []; -var reverseProxyNetworks = builder.Configuration.GetSection("ReverseProxy:KnownNetworks").Get() ?? []; - -if (!builder.Environment.IsDevelopment() && reverseProxyIps.Length == 0 && reverseProxyNetworks.Length == 0) - throw new InvalidOperationException( - "ReverseProxy:KnownProxies or ReverseProxy:KnownNetworks must be configured outside Development. " + - "Set these to the IP addresses or CIDR ranges of your reverse proxy (e.g. Cloudflare's published ranges)."); - -builder.Services.Configure(options => -{ - options.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; - options.KnownIPNetworks.Clear(); - options.KnownProxies.Clear(); - - foreach (var proxy in reverseProxyIps) - if (System.Net.IPAddress.TryParse(proxy, out var ip)) - options.KnownProxies.Add(ip); - - foreach (var network in reverseProxyNetworks) - if (System.Net.IPNetwork.TryParse(network, out var net)) - options.KnownIPNetworks.Add(net); -}); +builder.Services.AddApiOptions(builder.Configuration); +builder.Services.AddApiSecurity(builder.Configuration, builder.Environment); builder.Services.AddApiRateLimiting(builder.Configuration); builder.Services.AddMemoryCache(); @@ -250,28 +55,13 @@ builder.Services.AddSignalR(); builder.Services.AddAuthorization(); builder.Services.AddApiHealthChecks(); -builder.Services.AddScoped(); -builder.Services.AddSingleton(); -builder.Services.AddScoped(); -builder.Services.AddHostedService(); -builder.Services.AddSingleton(); -builder.Services.AddHostedService(); -builder.Services.AddSingleton(); -builder.Services.AddSingleton(); -builder.Services.AddHostedService(); -builder.Services.AddHostedService(); -builder.Services.AddSingleton(); -builder.Services.AddSingleton(); -builder.Services.AddHostedService(); +builder.Services.AddApiBackgroundServices(builder.Configuration); -builder.Services.Configure(builder.Configuration.GetSection(LogRetentionOptions.SectionName)); -builder.Services.AddSingleton(); -builder.Services.AddSingleton(sp => new DatabaseLoggerProvider(sp.GetRequiredService())); +// Filters must be applied to builder.Logging (not the service collection): keep EF Core, SignalR and +// connection noise out of the database-backed log sink to avoid recursive/self-amplifying logging. builder.Logging.AddFilter("Microsoft.EntityFrameworkCore", LogLevel.None); builder.Logging.AddFilter("Microsoft.AspNetCore.SignalR", LogLevel.None); builder.Logging.AddFilter("Microsoft.AspNetCore.Http.Connections", LogLevel.None); -builder.Services.AddHostedService(); -builder.Services.AddHostedService(); var app = builder.Build(); diff --git a/code/FinanceManager.Infrastructure/Repositories/Account/Entry/BondEntryRepository.cs b/code/FinanceManager.Infrastructure/Repositories/Account/Entry/BondEntryRepository.cs index aeddb191..f30a0f4a 100644 --- a/code/FinanceManager.Infrastructure/Repositories/Account/Entry/BondEntryRepository.cs +++ b/code/FinanceManager.Infrastructure/Repositories/Account/Entry/BondEntryRepository.cs @@ -444,11 +444,17 @@ private async Task RecalculateValuesInMemory(int accountId, DateTime startDate) public async Task AddLabel(int entryId, int labelId) { - var entry = await context.BondEntries.FirstOrDefaultAsync(e => e.EntryId == entryId); + var entry = await context.BondEntries + .Include(e => e.Labels) + .FirstOrDefaultAsync(e => e.EntryId == entryId); var label = await context.FinancialLabels.FirstOrDefaultAsync(l => l.Id == labelId); if (entry is null || label is null) return false; + // Idempotent: don't add a label the entry already carries. + if (entry.Labels.Any(l => l.Id == labelId)) return true; + + entry.Labels.Add(label); await context.SaveChangesAsync(); return true; diff --git a/code/FinanceManager.Tests.Unit/Infrastructure/Repositories/BondEntryRepositoryLabelTests.cs b/code/FinanceManager.Tests.Unit/Infrastructure/Repositories/BondEntryRepositoryLabelTests.cs new file mode 100644 index 00000000..cff172e5 --- /dev/null +++ b/code/FinanceManager.Tests.Unit/Infrastructure/Repositories/BondEntryRepositoryLabelTests.cs @@ -0,0 +1,72 @@ +using FinanceManager.Domain.FinancialAccounts.Bond.Entities; +using FinanceManager.Domain.FinancialAccounts.Shared.Entities; +using FinanceManager.Infrastructure.Contexts; +using FinanceManager.Infrastructure.Repositories.Account.Entry; +using Microsoft.EntityFrameworkCore; + +namespace FinanceManager.Tests.Unit.Infrastructure.Repositories; + +[Collection("Infrastructure")] +[Trait("Category", "Unit")] +public class BondEntryRepositoryLabelTests +{ + private static AppDbContext CreateContext() => + new(new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options); + + [Fact] + public async Task AddLabel_AttachesLabelToEntry() + { + await using var context = CreateContext(); + var entry = new BondAccountEntry(1, 0, new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc), 0, 100, 5); + context.BondEntries.Add(entry); + var label = new FinancialLabel { Name = "Coupon" }; + context.FinancialLabels.Add(label); + await context.SaveChangesAsync(TestContext.Current.CancellationToken); + + var repository = new BondEntryRepository(context); + + var result = await repository.AddLabel(entry.EntryId, label.Id); + + Assert.True(result); + var stored = await context.BondEntries.Include(e => e.Labels) + .FirstAsync(e => e.EntryId == entry.EntryId, TestContext.Current.CancellationToken); + Assert.Contains(stored.Labels, l => l.Id == label.Id); + } + + [Fact] + public async Task AddLabel_IsIdempotent() + { + await using var context = CreateContext(); + var entry = new BondAccountEntry(1, 0, new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc), 0, 100, 5); + context.BondEntries.Add(entry); + var label = new FinancialLabel { Name = "Coupon" }; + context.FinancialLabels.Add(label); + await context.SaveChangesAsync(TestContext.Current.CancellationToken); + + var repository = new BondEntryRepository(context); + await repository.AddLabel(entry.EntryId, label.Id); + var result = await repository.AddLabel(entry.EntryId, label.Id); + + Assert.True(result); + var stored = await context.BondEntries.Include(e => e.Labels) + .FirstAsync(e => e.EntryId == entry.EntryId, TestContext.Current.CancellationToken); + Assert.Single(stored.Labels); + } + + [Fact] + public async Task AddLabel_ReturnsFalse_WhenLabelMissing() + { + await using var context = CreateContext(); + var entry = new BondAccountEntry(1, 0, new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc), 0, 100, 5); + context.BondEntries.Add(entry); + await context.SaveChangesAsync(TestContext.Current.CancellationToken); + + var repository = new BondEntryRepository(context); + + var result = await repository.AddLabel(entry.EntryId, labelId: 999); + + Assert.False(result); + } +} \ No newline at end of file