diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..e7c1f136 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,26 @@ +version: 2 +updates: + - package-ecosystem: nuget + directory: / + schedule: + interval: weekly + groups: + nuget-minor-patch: + update-types: + - minor + - patch + + - package-ecosystem: npm + directory: / + schedule: + interval: weekly + groups: + npm-minor-patch: + update-types: + - minor + - patch + + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ef5640ed..471f4f05 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,6 +50,26 @@ jobs: - name: Test run: dotnet test --no-build + vulnerable-packages: + runs-on: ubuntu-latest + needs: lint + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-dotnet@v5 + with: + dotnet-version: '10.0.x' + + - run: dotnet restore + + - name: Audit NuGet packages for known vulnerabilities + run: | + dotnet list package --vulnerable --include-transitive 2>&1 | tee vulnerable.txt + if grep -q "has the following vulnerable packages" vulnerable.txt; then + echo "::error::Vulnerable NuGet packages found — see log above" + exit 1 + fi + e2e: runs-on: ubuntu-latest needs: lint diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 00000000..445e6dd0 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,40 @@ +name: CodeQL + +on: + push: + branches: [main] + pull_request: + branches: [main] + schedule: + - cron: '24 5 * * 1' + +jobs: + analyze: + name: Analyze (${{ matrix.language }}) + runs-on: ubuntu-latest + permissions: + security-events: write + packages: read + actions: read + contents: read + strategy: + fail-fast: false + matrix: + include: + - language: csharp + build-mode: none + - language: javascript-typescript + build-mode: none + steps: + - uses: actions/checkout@v6 + + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 + with: + category: '/language:${{ matrix.language }}' diff --git a/CLAUDE.md b/CLAUDE.md index 33307a11..8c5ef3d6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -87,34 +87,32 @@ dotnet test --filter "FullyQualifiedName~ClassName" # single test class dotnet test --filter "FullyQualifiedName~MethodName" # single test method ``` -Test stack: xUnit.v3, FluentAssertions, Bogus, Microsoft.AspNetCore.Mvc.Testing. SQLite in-memory for unit tests, PostgreSQL for integration tests in CI. +Test stack: xUnit.v3, FluentAssertions, Bogus, Microsoft.AspNetCore.Mvc.Testing. SQLite in-memory for unit and integration tests (a PostgreSQL CI leg is a known gap — the test factory is SQLite-only today). ### Benchmarks (BenchmarkDotNet) ```bash dotnet run -c Release --project tests/SimpleModule.Benchmarks # all benchmarks -dotnet run -c Release --project tests/SimpleModule.Benchmarks -- --filter "*Products*" # specific module +dotnet run -c Release --project tests/SimpleModule.Benchmarks -- --filter "*Users*" # specific module ``` -Micro-benchmarks for every module: endpoint latency (CRUD operations via in-process TestServer), JSON serialization/deserialization of DTOs. Uses `SimpleModuleWebApplicationFactory` with test auth headers for low-overhead measurement. +Micro-benchmarks for Admin, AuditLogs, FileStorage, Settings, and Users: endpoint latency (CRUD operations via in-process TestServer), JSON serialization/deserialization of DTOs. Uses `SimpleModuleWebApplicationFactory` with test auth headers for low-overhead measurement. ### Load Tests (NBomber) ```bash -dotnet test tests/SimpleModule.LoadTests # all 11 scenarios (50 concurrent, ~5 min) -dotnet test tests/SimpleModule.LoadTests --filter "Products_Crud" # single scenario +dotnet test tests/SimpleModule.LoadTests # all scenarios +dotnet test tests/SimpleModule.LoadTests --filter "Users_Crud" # single scenario ``` -HTTP load tests using real OAuth Bearer tokens acquired via ROPC (password grant) from OpenIddict. Runs against the full ASP.NET pipeline with file-based SQLite in WAL mode. 11 scenarios covering all modules at 50 concurrent copies: +HTTP load tests using real OAuth Bearer tokens acquired via ROPC (password grant) from OpenIddict. Runs against the full ASP.NET pipeline with file-based SQLite in WAL mode. Six scenarios (`tests/SimpleModule.LoadTests/Scenarios/`), each runnable individually or combined via `All_Scenarios`: -- **Products, Orders, Users** — full CRUD lifecycle (create → read → update → delete) -- **Settings** — read operations (settings, definitions, menus, available pages) -- **AuditLogs, FileStorage** — read operations (list, get by ID, folders) -- **PageBuilder** — full lifecycle (create → get → update → publish → unpublish → delete + tags/templates) -- **Admin** — role create/delete (handles 302 redirects) -- **FeatureFlags** — get all flags, check flag status -- **Marketplace** — search and browse (anonymous) -- **Mixed Realistic** — weighted workload (70% reads, 20% creates, 10% updates) +- **Users** (`Users_Crud`) — full CRUD lifecycle +- **Settings** (`Settings_Ops`) — read operations +- **AuditLogs** (`AuditLogs_Read`) — read operations +- **FileStorage** (`Files_Ops`) — file operations +- **Admin** (`Admin_Ops`) — role create/delete (handles 302 redirects) +- **FeatureFlags** (`FeatureFlags_Ops`) — get all flags, check flag status **Key infrastructure:** - `LoadTestWebApplicationFactory` — extends `WebApplicationFactory` with file-based SQLite + WAL, seeds OAuth client/user/permissions, acquires Bearer tokens via `/connect/token` @@ -131,13 +129,13 @@ HTTP load tests using real OAuth Bearer tokens acquired via ROPC (password grant ### Frontend (React + Inertia.js) -- **ClientApp** (`template/SimpleModule.Host/ClientApp/app.tsx`) — Inertia bootstrap. Resolves pages by splitting route name (e.g., `Products/Browse` → imports `/_content/Products/Products.pages.js`). +- **ClientApp** (`template/SimpleModule.Host/ClientApp/app.tsx`) — Inertia bootstrap. Resolves pages by splitting route name (e.g., `Tenants/Browse` → imports `/_content/Tenants/Tenants.pages.js`). - **Module pages** — Each module builds its React pages via Vite in library mode → `{ModuleName}.pages.js` in module's `wwwroot/`. Entry point: `Pages/index.ts` exporting a `pages` record mapping route names to components. - **Type generation** — `[Dto]` types → source generator embeds TS interfaces → `scripts/extract-ts-types.mjs` writes `.ts` files to `ClientApp/types/`. ### Request Flow -1. ASP.NET route handler calls `Inertia.Render("Products/Browse", props)` +1. ASP.NET route handler calls `Inertia.Render("Tenants/Browse", props)` 2. Inertia middleware renders static HTML shell with embedded JSON props 3. React ClientApp dynamically imports module's `pages.js` bundle 4. Component hydrates with server-provided props @@ -177,15 +175,15 @@ When you add a new `IViewEndpoint`, you **must** register it in your module's `P **Pattern:** ```typescript -// modules/Products/src/Products/Pages/index.ts -export const pages: Record = { - "Products/Browse": () => import("./Browse"), - "Products/Manage": () => import("./Manage"), - "Products/Create": () => import("./Create"), +// modules/Tenants/src/SimpleModule.Tenants/Pages/index.ts +export const pages: Record = { + 'Tenants/Browse': () => import('./Browse'), + 'Tenants/Manage': () => import('./Manage'), + 'Tenants/Create': () => import('./Create'), }; ``` -**The Rule:** For every `IViewEndpoint` with `Inertia.Render("Products/Something", ...)`, add a matching entry in `pages`. The component name in Inertia.Render (e.g., `"Products/Manage"`) is your key. +**The Rule:** For every `IViewEndpoint` with `Inertia.Render("Tenants/Something", ...)`, add a matching entry in `pages`. The component name in Inertia.Render (e.g., `"Tenants/Manage"`) is your key. **Validation:** After adding endpoints, run: diff --git a/docker-compose.yml b/docker-compose.yml index 97f4ed4c..84d4af01 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,14 +4,30 @@ services: ports: - "8080:8080" environment: - # Development enables auto-migration. For production, apply migrations - # externally and set to Production. + # Development enables auto-migration and ephemeral signing keys for a + # local quickstart. Do NOT expose this stack to the internet as-is: set + # ASPNETCORE_ENVIRONMENT=Production (which additionally enforces signing + # certificates and refuses the password grant), apply migrations + # externally, and trust your reverse proxy via a comma-separated + # ForwardedHeaders__KnownProxies=10.0.0.5,10.0.0.6 (or + # ForwardedHeaders__KnownNetworks=10.0.0.0/8). Without it X-Forwarded-For + # is ignored and per-IP rate limiting keys every client to the proxy IP. ASPNETCORE_ENVIRONMENT: Development Database__DefaultConnection: "Host=postgres;Port=5432;Database=simplemodule;Username=simplemodule;Password=${POSTGRES_PASSWORD:-simplemodule}" Database__Provider: PostgreSQL # Set to your public URL so OpenIddict registers correct redirect URIs. # Examples: https://app.simplemodule.dev, http://localhost:8080 OpenIddict__BaseUrl: ${APP_BASE_URL:-http://localhost:8080} + # The ROPC password grant stays off even in this Development quickstart — + # combined with seeded credentials it would hand out fully-privileged + # tokens with a single POST /connect/token. + OpenIddict__AllowPasswordGrant: "false" + # Required: password for the seeded admin account. There is deliberately + # no default — put SEED_ADMIN_PASSWORD in your .env file. + Seed__AdminPassword: ${SEED_ADMIN_PASSWORD:?Set SEED_ADMIN_PASSWORD in .env} + # Optional: password for the seeded demo user (outside Development the + # demo user is skipped entirely when this is unset). + Seed__UserPassword: ${SEED_USER_PASSWORD:-} # api stays in Producer mode — it enqueues jobs but never runs them. # All IModuleJob execution lives in the worker service below. BackgroundJobs__WorkerMode: Producer @@ -39,6 +55,13 @@ services: Database__DefaultConnection: "Host=postgres;Port=5432;Database=simplemodule;Username=simplemodule;Password=${POSTGRES_PASSWORD:-simplemodule}" Database__Provider: PostgreSQL BackgroundJobs__WorkerMode: Consumer + # The worker runs the same module set as the api, including the user + # seeder. It must see the SAME seed passwords — otherwise whichever + # process wins the startup race seeds the admin account, and a worker + # without these would silently seed the compiled-in default password, + # defeating the api's required SEED_ADMIN_PASSWORD. + Seed__AdminPassword: ${SEED_ADMIN_PASSWORD:?Set SEED_ADMIN_PASSWORD in .env} + Seed__UserPassword: ${SEED_USER_PASSWORD:-} volumes: - storage_data:/app/storage depends_on: diff --git a/docs/CONSTITUTION.md b/docs/CONSTITUTION.md index c22dcc2f..7a90f636 100644 --- a/docs/CONSTITUTION.md +++ b/docs/CONSTITUTION.md @@ -431,8 +431,8 @@ xUnit.v3, FluentAssertions, Bogus, `SimpleModuleWebApplicationFactory`. ### Database -- SQLite in-memory locally, PostgreSQL in CI. -- Both providers tested in CI. +- SQLite in-memory for unit and integration tests (shared connection per test factory). +- A PostgreSQL CI test leg is planned but not yet implemented — `SimpleModuleWebApplicationFactory` currently supports SQLite only. ### Rules @@ -600,7 +600,8 @@ All SM diagnostics are emitted by the Roslyn source generator at compile time. ` - `TreatWarningsAsErrors` is enabled globally via `Directory.Build.props`. - `AnalysisLevel=latest-all`, `AnalysisMode=All`. - Suppressed rules live in `.editorconfig`. -- Tests run against both SQLite and PostgreSQL in CI. +- CI tests run against SQLite. A PostgreSQL leg (postgres service + provider switch in the test factory) is a known gap. +- CodeQL, Dependabot, and a vulnerable-package audit run in CI for security scanning. --- diff --git a/framework/SimpleModule.Core/Hosting/HostEnvironmentExtensions.cs b/framework/SimpleModule.Core/Hosting/HostEnvironmentExtensions.cs new file mode 100644 index 00000000..fb6385ce --- /dev/null +++ b/framework/SimpleModule.Core/Hosting/HostEnvironmentExtensions.cs @@ -0,0 +1,34 @@ +using Microsoft.Extensions.Hosting; + +namespace SimpleModule.Core.Hosting; + +/// +/// Shared environment classification for the framework's fail-fast guards. +/// +public static class HostEnvironmentExtensions +{ + /// + /// The well-known name for the test environment used by the in-process + /// WebApplicationFactory harnesses. + /// + public const string TestingEnvironmentName = "Testing"; + + /// + /// True for the developer-machine and CI/test environments — Development and + /// Testing — where compiled-in defaults (seed passwords, ephemeral signing + /// keys, the ROPC password grant) are acceptable conveniences. + /// + /// Every other environment (Staging, QA, Production, or any custom name) is + /// treated as a real deployment: the security guards require explicit + /// configuration and refuse the unsafe defaults. Using a single predicate + /// keeps UserSeedService and OpenIddictProductionGuard + /// consistent — a deployment is never hardened by one guard and waved + /// through by the other. + /// + /// + public static bool IsLocalOrTest(this IHostEnvironment environment) + { + ArgumentNullException.ThrowIfNull(environment); + return environment.IsDevelopment() || environment.IsEnvironment(TestingEnvironmentName); + } +} diff --git a/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.Helpers.cs b/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.Helpers.cs index d21d4060..ef12d9b7 100644 --- a/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.Helpers.cs +++ b/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.Helpers.cs @@ -16,6 +16,30 @@ public static partial class SimpleModuleHostExtensions private const string ModuleContentPathPrefix = "/_content/"; private const string ModuleScriptExtension = ".mjs"; + /// + /// Reads a config list that may be expressed either as a JSON/indexed array + /// (ForwardedHeaders:KnownProxies:0) or as a single comma-separated + /// scalar (ForwardedHeaders__KnownProxies=10.0.0.5,10.0.0.6, the form + /// natural for environment variables). Binding only the array form silently + /// dropped scalar env vars, leaving the proxy untrusted. + /// + private static string[] ReadConfigList(IConfigurationSection section, string key) + { + var array = section.GetSection(key).Get(); + if (array is { Length: > 0 }) + { + return array; + } + + var scalar = section[key]; + return string.IsNullOrWhiteSpace(scalar) + ? [] + : scalar.Split( + ',', + StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries + ); + } + private static IResult RenderErrorPage(int statusCode) { var (title, message) = statusCode switch diff --git a/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs b/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs index 6ef608bf..c4c36e65 100644 --- a/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs +++ b/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs @@ -57,9 +57,49 @@ public static WebApplicationBuilder AddSimpleModuleInfrastructure( { fhOptions.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; - // Allow any proxy in containerized/cloud environments - fhOptions.KnownIPNetworks.Clear(); - fhOptions.KnownProxies.Clear(); + + // Proxy trust must be explicit. The previous behavior cleared + // KnownProxies/KnownIPNetworks, which let any client spoof + // X-Forwarded-For and bypass per-IP rate limiting. By default only + // loopback is trusted (the ASP.NET Core default); deployments behind + // a reverse proxy list it under ForwardedHeaders:KnownProxies / + // ForwardedHeaders:KnownNetworks, or — for closed networks where the + // proxy address is not static — opt into + // ForwardedHeaders:TrustAllProxies. + var section = builder.Configuration.GetSection("ForwardedHeaders"); + + if (section.GetValue("TrustAllProxies")) + { + fhOptions.KnownIPNetworks.Clear(); + fhOptions.KnownProxies.Clear(); + return; + } + + foreach (var proxy in ReadConfigList(section, "KnownProxies")) + { + if (!System.Net.IPAddress.TryParse(proxy, out var address)) + { + throw new InvalidOperationException( + $"ForwardedHeaders:KnownProxies contains '{proxy}', which is not a valid " + + "IP address." + ); + } + + fhOptions.KnownProxies.Add(address); + } + + foreach (var network in ReadConfigList(section, "KnownNetworks")) + { + if (!System.Net.IPNetwork.TryParse(network, out var ipNetwork)) + { + throw new InvalidOperationException( + $"ForwardedHeaders:KnownNetworks contains '{network}', which is not a valid " + + "CIDR network (e.g. 10.0.0.0/8)." + ); + } + + fhOptions.KnownIPNetworks.Add(ipNetwork); + } }); builder.Services.AddProblemDetails(); @@ -139,7 +179,10 @@ public static WebApplicationBuilder AddSimpleModuleInfrastructure( // cleared by `sm up`. Resolved as singleton because it caches state // for a short interval. builder.Services.Configure(_ => { }); - builder.Services.TryAddSingleton(); + builder.Services.TryAddSingleton< + IMaintenanceStateProvider, + FileSystemMaintenanceStateProvider + >(); if (options.EnableHealthChecks) { diff --git a/modules/Admin/src/SimpleModule.Admin/AdminService.cs b/modules/Admin/src/SimpleModule.Admin/AdminService.cs index 9844495d..57378cb6 100644 --- a/modules/Admin/src/SimpleModule.Admin/AdminService.cs +++ b/modules/Admin/src/SimpleModule.Admin/AdminService.cs @@ -10,14 +10,16 @@ public async Task GetAdminOverviewAsync( CancellationToken cancellationToken = default ) { - var usersTask = userAdmin.GetUsersPagedAsync(null, 1, 1); - var activeTask = userAdmin.GetUsersPagedAsync(null, 1, 1, filterStatus: "active"); - var rolesTask = roleAdmin.GetAllRolesAsync(); - await Task.WhenAll(usersTask, activeTask, rolesTask).ConfigureAwait(false); - - var usersPage = await usersTask.ConfigureAwait(false); - var activePage = await activeTask.ConfigureAwait(false); - var roles = await rolesTask.ConfigureAwait(false); + // Await sequentially, not via Task.WhenAll: IUserAdminContracts and + // IRoleAdminContracts are both backed by the same scoped UsersDbContext, + // and EF Core forbids concurrent operations on one DbContext instance. + // Parallel awaits here intermittently threw "A second operation was + // started on this context instance..." → HTTP 500 (same class as #242). + var usersPage = await userAdmin.GetUsersPagedAsync(null, 1, 1).ConfigureAwait(false); + var activePage = await userAdmin + .GetUsersPagedAsync(null, 1, 1, filterStatus: "active") + .ConfigureAwait(false); + var roles = await roleAdmin.GetAllRolesAsync().ConfigureAwait(false); return new AdminOverviewDto { diff --git a/modules/Admin/src/SimpleModule.Admin/Pages/Admin/UsersEditEndpoint.cs b/modules/Admin/src/SimpleModule.Admin/Pages/Admin/UsersEditEndpoint.cs index 66d21398..a5dea2f9 100644 --- a/modules/Admin/src/SimpleModule.Admin/Pages/Admin/UsersEditEndpoint.cs +++ b/modules/Admin/src/SimpleModule.Admin/Pages/Admin/UsersEditEndpoint.cs @@ -35,6 +35,13 @@ public void Map(IEndpointRouteBuilder app) if (user is null) return TypedResults.NotFound(); + // These three contracts are backed by DISTINCT stores — + // IRoleAdminContracts → UsersDbContext, IPermissionContracts → + // PermissionsDbContext, ISessionContracts → the OpenIddict token + // store / Keycloak — so unlike the AdminService case (#242) there + // is no shared-DbContext concurrency hazard and they run in + // parallel. (GetAdminUserByIdAsync above already completed, so no + // other in-flight UsersDbContext operation overlaps GetAllRolesAsync.) var rolesTask = roleAdmin.GetAllRolesAsync(); var permsTask = permissionContracts.GetPermissionsForUserAsync(UserId.From(id)); var sessionsTask = sessionContracts.GetActiveSessionsForUserAsync(id); diff --git a/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/UploadEndpoint.cs b/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/UploadEndpoint.cs index 11e6ce89..2338554f 100644 --- a/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/UploadEndpoint.cs +++ b/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/UploadEndpoint.cs @@ -1,6 +1,8 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using SimpleModule.Core; using SimpleModule.Core.Authorization; using SimpleModule.Core.Extensions; @@ -13,7 +15,23 @@ public class UploadEndpoint : IEndpoint public const string Route = FileStorageConstants.Routes.Upload; public const string Method = "POST"; - public void Map(IEndpointRouteBuilder app) => + public void Map(IEndpointRouteBuilder app) + { + // FileStorageModuleOptions is an IOptions singleton, so the size limit and + // the parsed extension allowlist are resolved once at registration time + // and captured — not re-split into a new HashSet on every upload request. + var options = app + .ServiceProvider.GetRequiredService>() + .Value; + var maxBytes = options.MaxFileSizeMb * 1024L * 1024L; + // An empty allowlist means "no restriction". + var allowedExtensions = options + .AllowedExtensions.Split( + ',', + StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries + ) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + app.MapPost( Route, async Task ( @@ -28,6 +46,30 @@ IFileStorageContracts files return TypedResults.BadRequest("A file is required."); } + if (file.Length > maxBytes) + { + return TypedResults.Problem( + detail: $"File exceeds the maximum allowed size of " + + $"{options.MaxFileSizeMb} MB.", + statusCode: StatusCodes.Status413PayloadTooLarge + ); + } + + var extension = Path.GetExtension(file.FileName); + if ( + allowedExtensions.Count > 0 + && ( + string.IsNullOrEmpty(extension) + || !allowedExtensions.Contains(extension) + ) + ) + { + return TypedResults.BadRequest( + $"File type '{extension}' is not allowed. Allowed extensions: " + + $"{options.AllowedExtensions}" + ); + } + var userId = context.User.GetUserId(); await using var stream = file.OpenReadStream(); var storedFile = await files.UploadFileAsync( @@ -42,4 +84,5 @@ IFileStorageContracts files ) .RequirePermission(FileStoragePermissions.Upload) .DisableAntiforgery(); + } } diff --git a/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageModule.cs b/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageModule.cs index 884ac064..d546640d 100644 --- a/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageModule.cs +++ b/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageModule.cs @@ -70,7 +70,7 @@ public void ConfigureSettings(ISettingsBuilder settings) "Comma-separated list of allowed file extensions (e.g., .jpg,.pdf,.zip).", Group = "FileStorage", Scope = SettingScope.Application, - DefaultValue = "\".jpg,.jpeg,.png,.gif,.pdf,.doc,.docx,.xls,.xlsx,.zip\"", + DefaultValue = ".txt,.csv,.jpg,.jpeg,.png,.gif,.pdf,.doc,.docx,.xls,.xlsx,.zip", Type = SettingType.Text, } ); diff --git a/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageModuleOptions.cs b/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageModuleOptions.cs index 77889411..f9d3ad11 100644 --- a/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageModuleOptions.cs +++ b/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageModuleOptions.cs @@ -13,9 +13,10 @@ public class FileStorageModuleOptions : IModuleOptions public int MaxFileSizeMb { get; set; } = 50; /// - /// Comma-separated list of allowed file extensions for uploads. - /// Default: ".jpg,.jpeg,.png,.gif,.pdf,.doc,.docx,.xls,.xlsx,.zip" + /// Comma-separated list of allowed file extensions for uploads. An empty + /// string disables the extension check (any type allowed, size limit still + /// applies). The default covers common documents, plain text, and images. /// public string AllowedExtensions { get; set; } = - ".jpg,.jpeg,.png,.gif,.pdf,.doc,.docx,.xls,.xlsx,.zip"; + ".txt,.csv,.jpg,.jpeg,.png,.gif,.pdf,.doc,.docx,.xls,.xlsx,.zip"; } diff --git a/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageService.cs b/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageService.cs index 28ebff02..56bf075f 100644 --- a/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageService.cs +++ b/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageService.cs @@ -3,14 +3,14 @@ using SimpleModule.FileStorage.Contracts; using SimpleModule.FileStorage.Contracts.Events; using SimpleModule.Storage; -using Wolverine; +using Wolverine.EntityFrameworkCore; namespace SimpleModule.FileStorage; public sealed partial class FileStorageService( FileStorageDbContext db, IStorageProvider storageProvider, - IMessageBus bus, + IDbContextOutbox outbox, ILogger logger ) : IFileStorageContracts { @@ -63,25 +63,28 @@ public async Task UploadFileAsync( var result = await storageProvider.SaveAsync(storagePath, content, contentType); + var storedFile = new StoredFile + { + FileName = fileName, + StoragePath = result.Path, + ContentType = contentType, + Size = result.Size, + Folder = normalizedFolder, + CreatedByUserId = userId, + CreatedAt = DateTimeOffset.UtcNow, + }; + + db.StoredFiles.Add(storedFile); + + // FileStorageId is database-generated, so the row must be saved before + // the event can carry it. The explicit transaction keeps the row and the + // outbox envelope atomic. + await using var transaction = await db.Database.BeginTransactionAsync(); + try { - var storedFile = new StoredFile - { - FileName = fileName, - StoragePath = result.Path, - ContentType = contentType, - Size = result.Size, - Folder = normalizedFolder, - CreatedByUserId = userId, - CreatedAt = DateTimeOffset.UtcNow, - }; - - db.StoredFiles.Add(storedFile); await db.SaveChangesAsync(); - - LogFileUploaded(logger, storedFile.Id, storedFile.FileName); - - await bus.PublishAsync( + await outbox.PublishAsync( new FileUploadedEvent( storedFile.Id, storedFile.FileName, @@ -89,14 +92,26 @@ await bus.PublishAsync( storedFile.ContentType ) ); - - return storedFile; } catch { + // The row is not yet committed — roll back the orphaned blob. The blob + // cleanup MUST stay scoped to the pre-commit window: once the row is + // committed below, deleting the blob would dangle a committed StoredFile + // against a missing file. await storageProvider.DeleteAsync(result.Path); throw; } + + // Commits the transaction (row + envelope), then flushes messages. A + // failure here is post-persist: the row may already be committed, so the + // blob must be left in place. The previous SaveChanges-then-PublishAsync + // pattern lost the event when the process died between the two calls. + await outbox.SaveChangesAndFlushMessagesAsync(); + + LogFileUploaded(logger, storedFile.Id, storedFile.FileName); + + return storedFile; } public async Task DeleteFileAsync(FileStorageId id) @@ -113,7 +128,16 @@ public async Task DeleteFileAsync(StoredFile file) var storagePath = file.StoragePath; db.StoredFiles.Remove(file); - await db.SaveChangesAsync(); + + // The DB row is the system of record: once it is gone, the file is deleted + // as far as the application is concerned. Publish through the outbox so the + // row removal and FileDeletedEvent commit atomically — previously the event + // fired only after blob deletion succeeded, so a crash (or a failed blob + // delete) after the DB commit silently dropped the event. Blob deletion + // below is best-effort cleanup; a failure there leaves an orphaned blob (a + // storage-sweep concern), not a lost event or a dangling row. + await outbox.PublishAsync(new FileDeletedEvent(file.Id, file.FileName)); + await outbox.SaveChangesAndFlushMessagesAsync(); try { @@ -128,8 +152,6 @@ public async Task DeleteFileAsync(StoredFile file) } LogFileDeleted(logger, file.Id, file.FileName); - - await bus.PublishAsync(new FileDeletedEvent(file.Id, file.FileName)); } public async Task DownloadFileAsync(FileStorageId id) diff --git a/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/FileStorageServiceTests.cs b/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/FileStorageServiceTests.cs index 96ca0575..bd2d2bfc 100644 --- a/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/FileStorageServiceTests.cs +++ b/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/FileStorageServiceTests.cs @@ -38,7 +38,7 @@ public FileStorageServiceTests() _service = new FileStorageService( _db, _storageProvider, - new TestMessageBus(), + new FakeDbContextOutbox(_db), NullLogger.Instance ); } @@ -229,7 +229,7 @@ public async Task UploadFileAsync_Cleans_Up_Storage_On_DB_Failure() var failingService = new FileStorageService( _db, failingProvider, - new TestMessageBus(), + new FakeDbContextOutbox(_db), NullLogger.Instance ); diff --git a/modules/OpenIddict/src/SimpleModule.OpenIddict.Contracts/ConfigKeys.cs b/modules/OpenIddict/src/SimpleModule.OpenIddict.Contracts/ConfigKeys.cs index 3a8598e0..6ef63970 100644 --- a/modules/OpenIddict/src/SimpleModule.OpenIddict.Contracts/ConfigKeys.cs +++ b/modules/OpenIddict/src/SimpleModule.OpenIddict.Contracts/ConfigKeys.cs @@ -3,6 +3,13 @@ namespace SimpleModule.OpenIddict.Contracts; public static class ConfigKeys { public const string OpenIddictBaseUrl = "OpenIddict:BaseUrl"; + + /// + /// Enables the ROPC (resource-owner password credentials) grant. Intended for + /// local load testing only — OpenIddictProductionGuard refuses it in + /// real deployments. + /// + public const string OpenIddictAllowPasswordGrant = "OpenIddict:AllowPasswordGrant"; public const string OpenIddictEncryptionCertPath = "OpenIddict:EncryptionCertificatePath"; public const string OpenIddictSigningCertPath = "OpenIddict:SigningCertificatePath"; public const string OpenIddictCertPassword = "OpenIddict:CertificatePassword"; diff --git a/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs b/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs index e100a4b9..7e44da80 100644 --- a/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs +++ b/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs @@ -66,8 +66,10 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config options.AllowRefreshTokenFlow(); - // Enable password grant in Development for load testing (k6, etc.) - if (configuration.GetValue("OpenIddict:AllowPasswordGrant")) + // Enable password grant in Development for load testing (k6, etc.). + // OpenIddictProductionGuard fails host startup if this is ever + // turned on in a real deployment. + if (configuration.GetValue(ConfigKeys.OpenIddictAllowPasswordGrant)) { options.AllowPasswordFlow(); } @@ -103,7 +105,9 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config } else { - // Development/Testing: use ephemeral keys (avoids macOS keychain issues) + // Development/Testing: use ephemeral keys (avoids macOS keychain + // issues). OpenIddictProductionGuard fails host startup if + // Production runs without certificates. options.AddEphemeralEncryptionKey().AddEphemeralSigningKey(); } @@ -127,6 +131,9 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config options.UseAspNetCore(); }); + // Refuses unsafe Production configurations (password grant, ephemeral keys) + services.AddHostedService(); + // Seed service services.AddHostedService(); diff --git a/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs b/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs new file mode 100644 index 00000000..d84c7461 --- /dev/null +++ b/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs @@ -0,0 +1,58 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Hosting; +using SimpleModule.Core.Hosting; +using SimpleModule.OpenIddict.Contracts; + +namespace SimpleModule.OpenIddict.Services; + +/// +/// Fails host startup when the OpenIddict configuration is unsafe for a real +/// deployment (anything but Development/Testing): the ROPC password grant must +/// stay off (it lets anyone exchange leaked or default credentials for a +/// fully-privileged token in a single request), and token signing/encryption +/// must use real certificates — ephemeral keys are regenerated on every restart, +/// invalidating all issued tokens, and signal a copy-pasted Development config. +/// Shares with +/// UserSeedService so the two guards never disagree about whether an +/// environment is a real deployment. +/// +public sealed class OpenIddictProductionGuard( + IConfiguration configuration, + IHostEnvironment environment +) : IHostedService +{ + public Task StartAsync(CancellationToken cancellationToken) + { + if (environment.IsLocalOrTest()) + { + return Task.CompletedTask; + } + + if (configuration.GetValue(ConfigKeys.OpenIddictAllowPasswordGrant)) + { + throw new InvalidOperationException( + $"'{ConfigKeys.OpenIddictAllowPasswordGrant}' must not be enabled in the " + + $"'{environment.EnvironmentName}' environment. The ROPC password grant " + + "exists for local load testing only." + ); + } + + var encryptionCertPath = configuration[ConfigKeys.OpenIddictEncryptionCertPath]; + var signingCertPath = configuration[ConfigKeys.OpenIddictSigningCertPath]; + + if (string.IsNullOrEmpty(encryptionCertPath) || string.IsNullOrEmpty(signingCertPath)) + { + throw new InvalidOperationException( + $"'{ConfigKeys.OpenIddictSigningCertPath}' and " + + $"'{ConfigKeys.OpenIddictEncryptionCertPath}' must be configured in " + + "Production. Without certificates OpenIddict falls back to ephemeral " + + "keys that are regenerated on every restart, invalidating all issued " + + "tokens." + ); + } + + return Task.CompletedTask; + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; +} diff --git a/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictSeedService.cs b/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictSeedService.cs index b8b3eba2..d69ef10b 100644 --- a/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictSeedService.cs +++ b/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictSeedService.cs @@ -70,7 +70,7 @@ CancellationToken cancellationToken }; // Allow password grant in Development for load testing (k6, etc.) - if (configuration.GetValue("OpenIddict:AllowPasswordGrant")) + if (configuration.GetValue(ConfigKeys.OpenIddictAllowPasswordGrant)) { descriptor.Permissions.Add(OpenIddictConstants.Permissions.GrantTypes.Password); } diff --git a/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs b/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs new file mode 100644 index 00000000..359d1a3e --- /dev/null +++ b/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs @@ -0,0 +1,93 @@ +using FluentAssertions; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.FileProviders; +using Microsoft.Extensions.Hosting; +using SimpleModule.OpenIddict.Contracts; +using SimpleModule.OpenIddict.Services; + +namespace SimpleModule.OpenIddict.Tests.Unit; + +public class OpenIddictProductionGuardTests +{ + private const string CertPath = "/certs/example.pfx"; + + [Theory] + [InlineData("Development")] + [InlineData("Testing")] + public async Task StartAsync_LocalOrTest_DoesNotThrow_EvenWithUnsafeConfig(string environment) + { + // Password grant on + no certs would be refused in a real deployment, + // but is tolerated locally. + var guard = CreateGuard(environment, (ConfigKeys.OpenIddictAllowPasswordGrant, "true")); + + await guard.Invoking(g => g.StartAsync(default)).Should().NotThrowAsync(); + } + + [Theory] + [InlineData("Production")] + [InlineData("Staging")] + [InlineData("QA")] + public async Task StartAsync_RealDeployment_PasswordGrantEnabled_Throws(string environment) + { + var guard = CreateGuard( + environment, + (ConfigKeys.OpenIddictAllowPasswordGrant, "true"), + (ConfigKeys.OpenIddictEncryptionCertPath, CertPath), + (ConfigKeys.OpenIddictSigningCertPath, CertPath) + ); + + ( + await guard + .Invoking(g => g.StartAsync(default)) + .Should() + .ThrowAsync() + ).WithMessage($"*{ConfigKeys.OpenIddictAllowPasswordGrant}*"); + } + + [Theory] + [InlineData("Production")] + [InlineData("Staging")] + public async Task StartAsync_RealDeployment_MissingCertificates_Throws(string environment) + { + var guard = CreateGuard(environment); // no cert paths configured + + ( + await guard + .Invoking(g => g.StartAsync(default)) + .Should() + .ThrowAsync() + ).WithMessage("*certificate*"); + } + + [Fact] + public async Task StartAsync_RealDeployment_FullyConfigured_DoesNotThrow() + { + var guard = CreateGuard( + "Production", + (ConfigKeys.OpenIddictEncryptionCertPath, CertPath), + (ConfigKeys.OpenIddictSigningCertPath, CertPath) + ); + + await guard.Invoking(g => g.StartAsync(default)).Should().NotThrowAsync(); + } + + private static OpenIddictProductionGuard CreateGuard( + string environment, + params (string Key, string Value)[] settings + ) + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(settings.ToDictionary(s => s.Key, s => (string?)s.Value)) + .Build(); + + return new OpenIddictProductionGuard(configuration, new FakeHostEnvironment(environment)); + } + + private sealed class FakeHostEnvironment(string environmentName) : IHostEnvironment + { + public string EnvironmentName { get; set; } = environmentName; + public string ApplicationName { get; set; } = "Tests"; + public string ContentRootPath { get; set; } = AppContext.BaseDirectory; + public IFileProvider ContentRootFileProvider { get; set; } = new NullFileProvider(); + } +} diff --git a/modules/Tenants/src/SimpleModule.Tenants/Endpoints/TenantFeatures/TenantFeatureHelper.cs b/modules/Tenants/src/SimpleModule.Tenants/Endpoints/TenantFeatures/TenantFeatureHelper.cs index 0061ce39..71a531f0 100644 --- a/modules/Tenants/src/SimpleModule.Tenants/Endpoints/TenantFeatures/TenantFeatureHelper.cs +++ b/modules/Tenants/src/SimpleModule.Tenants/Endpoints/TenantFeatures/TenantFeatureHelper.cs @@ -14,16 +14,24 @@ TenantId tenantId var tenantIdStr = tenantId.Value.ToString( System.Globalization.CultureInfo.InvariantCulture ); - var overrideTasks = flags - .Where(f => !f.IsDeprecated) - .Select(f => featureFlags.GetOverridesAsync(f.Name)); - var allOverrides = await Task.WhenAll(overrideTasks); - return allOverrides - .SelectMany(o => o) - .Where(o => - o.OverrideType == OverrideType.Tenant - && string.Equals(o.OverrideValue, tenantIdStr, StringComparison.Ordinal) - ) - .ToList(); + + // Await sequentially, not via Task.WhenAll: every GetOverridesAsync call + // hits the same scoped FeatureFlagsDbContext, and EF Core forbids + // concurrent operations on one context instance. With 2+ active flags + // the parallel version reliably threw "A second operation was started + // on this context instance..." → HTTP 500 (same class as #242). + var result = new List(); + foreach (var flag in flags.Where(f => !f.IsDeprecated)) + { + var overrides = await featureFlags.GetOverridesAsync(flag.Name); + result.AddRange( + overrides.Where(o => + o.OverrideType == OverrideType.Tenant + && string.Equals(o.OverrideValue, tenantIdStr, StringComparison.Ordinal) + ) + ); + } + + return result; } } diff --git a/modules/Tenants/src/SimpleModule.Tenants/TenantService.cs b/modules/Tenants/src/SimpleModule.Tenants/TenantService.cs index 09cfe0b7..b5509436 100644 --- a/modules/Tenants/src/SimpleModule.Tenants/TenantService.cs +++ b/modules/Tenants/src/SimpleModule.Tenants/TenantService.cs @@ -3,14 +3,12 @@ using SimpleModule.Core.Exceptions; using SimpleModule.Tenants.Contracts; using SimpleModule.Tenants.Contracts.Events; -using Wolverine; using Wolverine.EntityFrameworkCore; namespace SimpleModule.Tenants; public sealed partial class TenantService( TenantsDbContext db, - IMessageBus bus, IDbContextOutbox outbox, ILogger logger ) : ITenantContracts @@ -81,10 +79,19 @@ public async Task CreateTenantAsync(CreateTenantRequest request) } db.Tenants.Add(entity); + + // TenantId is database-generated, so the entity must be saved before the + // event can carry it. The explicit transaction keeps the tenant row and + // the outbox envelope atomic: SaveChangesAndFlushMessagesAsync persists + // the envelope, commits the open transaction, and only then releases the + // event to the bus. The previous SaveChanges-then-PublishAsync pattern + // lost the event when the process died between the two calls. + await using var transaction = await db.Database.BeginTransactionAsync(); await db.SaveChangesAsync(); + await outbox.PublishAsync(new TenantCreatedEvent(entity.Id, entity.Name, entity.Slug)); + await outbox.SaveChangesAndFlushMessagesAsync(); LogTenantCreated(logger, entity.Id, entity.Name); - await bus.PublishAsync(new TenantCreatedEvent(entity.Id, entity.Name, entity.Slug)); return MapToDto(entity); } diff --git a/modules/Tenants/tests/SimpleModule.Tenants.Tests/Unit/TenantServiceTests.cs b/modules/Tenants/tests/SimpleModule.Tenants.Tests/Unit/TenantServiceTests.cs index 88e27898..9159970e 100644 --- a/modules/Tenants/tests/SimpleModule.Tenants.Tests/Unit/TenantServiceTests.cs +++ b/modules/Tenants/tests/SimpleModule.Tenants.Tests/Unit/TenantServiceTests.cs @@ -2,13 +2,11 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; -using NSubstitute; using SimpleModule.Core.Exceptions; using SimpleModule.Database; using SimpleModule.Tenants; using SimpleModule.Tenants.Contracts; using SimpleModule.Tests.Shared.Fakes; -using Wolverine; namespace Tenants.Tests.Unit; @@ -16,7 +14,6 @@ public sealed class TenantServiceTests : IDisposable { private readonly TenantsDbContext _db; private readonly TenantService _sut; - private readonly IMessageBus _bus = Substitute.For(); private readonly FakeDbContextOutbox _outbox; public TenantServiceTests() @@ -37,7 +34,7 @@ public TenantServiceTests() _db.Database.OpenConnection(); _db.Database.EnsureCreated(); _outbox = new FakeDbContextOutbox(_db); - _sut = new TenantService(_db, _bus, _outbox, NullLogger.Instance); + _sut = new TenantService(_db, _outbox, NullLogger.Instance); } public void Dispose() => _db.Dispose(); @@ -89,10 +86,12 @@ public async Task CreateTenantAsync_CreatesAndReturnsTenant() tenant.Status.Should().Be(TenantStatus.Active); tenant.Hosts.Should().HaveCount(1); tenant.Hosts[0].HostName.Should().Be("new.localhost"); - await _bus.Received(1) - .PublishAsync( - Arg.Any(), - Arg.Any() + _outbox + .PublishedMessages.Should() + .ContainSingle(m => + m is SimpleModule.Tenants.Contracts.Events.TenantCreatedEvent + && ((SimpleModule.Tenants.Contracts.Events.TenantCreatedEvent)m).TenantId + == tenant.Id ); } @@ -105,9 +104,7 @@ public async Task UpdateTenantAsync_WithValidData_UpdatesTenant() updated.Name.Should().Be("Updated Acme"); _outbox .PublishedMessages.Should() - .ContainSingle(m => - m is SimpleModule.Tenants.Contracts.Events.TenantUpdatedEvent - ); + .ContainSingle(m => m is SimpleModule.Tenants.Contracts.Events.TenantUpdatedEvent); } [Fact] diff --git a/modules/Users/src/SimpleModule.Users/Services/SeedConfigurationException.cs b/modules/Users/src/SimpleModule.Users/Services/SeedConfigurationException.cs new file mode 100644 index 00000000..48665495 --- /dev/null +++ b/modules/Users/src/SimpleModule.Users/Services/SeedConfigurationException.cs @@ -0,0 +1,19 @@ +namespace SimpleModule.Users.Services; + +/// +/// Thrown when seeding cannot proceed safely — e.g. a required seed password is +/// missing outside Development. Unlike transient database errors (which the seed +/// service logs and tolerates), this exception is allowed to escape +/// so host startup fails loudly instead +/// of seeding a publicly known default credential. +/// +public sealed class SeedConfigurationException : InvalidOperationException +{ + public SeedConfigurationException() { } + + public SeedConfigurationException(string message) + : base(message) { } + + public SeedConfigurationException(string message, Exception innerException) + : base(message, innerException) { } +} diff --git a/modules/Users/src/SimpleModule.Users/Services/SeedPasswordOutcome.cs b/modules/Users/src/SimpleModule.Users/Services/SeedPasswordOutcome.cs new file mode 100644 index 00000000..1f1fdec2 --- /dev/null +++ b/modules/Users/src/SimpleModule.Users/Services/SeedPasswordOutcome.cs @@ -0,0 +1,20 @@ +namespace SimpleModule.Users.Services; + +/// +/// The decision made by for a +/// single seed account. +/// +internal enum SeedPasswordOutcome +{ + /// Seed the account with the resolved password. + Seed, + + /// Skip the (optional) account — unconfigured in a real deployment. + Skip, + + /// + /// Fail host startup — a required account is unconfigured in a real + /// deployment and must not fall back to the compiled-in default. + /// + Fail, +} diff --git a/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs b/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs index e42b05b2..3b201271 100644 --- a/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs +++ b/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs @@ -3,6 +3,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; +using SimpleModule.Core.Hosting; using SimpleModule.Users.Constants; using SimpleModule.Users.Contracts; @@ -43,7 +44,8 @@ await SeedUserAsync( SeedConstants.AdminDisplayName, ConfigKeys.SeedAdminPassword, SeedConstants.DefaultAdminPassword, - SeedConstants.AdminRole + SeedConstants.AdminRole, + requiredOutsideDevelopment: true ); await SeedUserAsync( userManager, @@ -51,11 +53,12 @@ await SeedUserAsync( SeedConstants.UserDisplayName, ConfigKeys.SeedUserPassword, SeedConstants.DefaultUserPassword, - SeedConstants.UserRole + SeedConstants.UserRole, + requiredOutsideDevelopment: false ); } #pragma warning disable CA1031 // Seed service must not crash the host on database errors - catch (Exception ex) + catch (Exception ex) when (ex is not SeedConfigurationException) #pragma warning restore CA1031 { LogSeedError(logger, ex.Message); @@ -99,12 +102,34 @@ private async Task SeedUserAsync( string displayName, string passwordConfigKey, string defaultPassword, - string role + string role, + bool requiredOutsideDevelopment ) { if (await userManager.FindByEmailAsync(email) is not null) return; + var outcome = ResolveSeedPassword( + configuration[passwordConfigKey], + defaultPassword, + environment.IsLocalOrTest(), + requiredOutsideDevelopment, + out var password + ); + + switch (outcome) + { + case SeedPasswordOutcome.Fail: + throw new SeedConfigurationException( + $"'{passwordConfigKey}' must be configured in the '{environment.EnvironmentName}' " + + $"environment. Refusing to create '{email}' with the compiled-in default " + + "password." + ); + case SeedPasswordOutcome.Skip: + LogSkippingSeedUser(logger, email, passwordConfigKey); + return; + } + LogSeedingUser(logger, email); var user = new ApplicationUser @@ -116,10 +141,9 @@ string role CreatedAt = DateTime.UtcNow, }; - var password = configuration[passwordConfigKey] ?? defaultPassword; - if (password == defaultPassword && !environment.IsDevelopment()) - LogDefaultPasswordWarning(logger, email, passwordConfigKey); - var result = await userManager.CreateAsync(user, password); + // Only SeedPasswordOutcome.Seed falls through the switch above, and it + // always yields a non-null password. + var result = await userManager.CreateAsync(user, password!); if (result.Succeeded) { await userManager.AddToRoleAsync(user, role); @@ -133,6 +157,47 @@ string role } } + /// + /// Decides how to seed a user's password. Extracted as a pure function so the + /// security-critical branching — never seed a real deployment with the + /// compiled-in default — is unit-testable without an Identity stack. + /// + /// The password from configuration, if any. + /// The compiled-in fallback (local/CI only). + /// True for Development/Testing environments. + /// + /// True for the admin account (must fail closed); false for the optional demo + /// user (skipped when unconfigured in a real deployment). + /// + /// The password to use when the outcome is Seed. + internal static SeedPasswordOutcome ResolveSeedPassword( + string? configuredPassword, + string defaultPassword, + bool isLocalOrTest, + bool requiredOutsideLocal, + out string? password + ) + { + if (!string.IsNullOrEmpty(configuredPassword)) + { + password = configuredPassword; + return SeedPasswordOutcome.Seed; + } + + // The compiled-in default passwords are a local/CI convenience only. In a + // real deployment (anything but Development/Testing) the configured + // password is mandatory: seeding the admin with a published default would + // leave it one POST /connect/token away from a fully-privileged token. + if (isLocalOrTest) + { + password = defaultPassword; + return SeedPasswordOutcome.Seed; + } + + password = null; + return requiredOutsideLocal ? SeedPasswordOutcome.Fail : SeedPasswordOutcome.Skip; + } + [LoggerMessage(Level = LogLevel.Information, Message = "Seeding role: {RoleName}")] private static partial void LogSeedingRole(ILogger logger, string roleName); @@ -140,14 +205,10 @@ string role private static partial void LogSeedingUser(ILogger logger, string email); [LoggerMessage( - Level = LogLevel.Warning, - Message = "Seeding {Email} with default password. Set '{ConfigKey}' in configuration before deploying to production." + Level = LogLevel.Information, + Message = "Skipping seed user {Email}: '{ConfigKey}' is not configured and default passwords are disabled outside Development." )] - private static partial void LogDefaultPasswordWarning( - ILogger logger, - string email, - string configKey - ); + private static partial void LogSkippingSeedUser(ILogger logger, string email, string configKey); [LoggerMessage(Level = LogLevel.Error, Message = "Seed error: {ErrorDescription}")] private static partial void LogSeedError(ILogger logger, string errorDescription); diff --git a/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserSeedServiceTests.cs b/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserSeedServiceTests.cs new file mode 100644 index 00000000..e349e9e7 --- /dev/null +++ b/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserSeedServiceTests.cs @@ -0,0 +1,95 @@ +using FluentAssertions; +using SimpleModule.Users.Services; + +namespace SimpleModule.Users.Tests.Unit; + +public class UserSeedServiceTests +{ + private const string Default = "Default123!"; + private const string Configured = "Configured123!"; + + [Theory] + [InlineData(true, true)] // local, required (admin) + [InlineData(true, false)] // local, optional (demo) + [InlineData(false, true)] // real deployment, required + [InlineData(false, false)] // real deployment, optional + public void ResolveSeedPassword_ConfiguredPassword_AlwaysUsed( + bool isLocalOrTest, + bool requiredOutsideLocal + ) + { + var outcome = UserSeedService.ResolveSeedPassword( + Configured, + Default, + isLocalOrTest, + requiredOutsideLocal, + out var password + ); + + outcome.Should().Be(SeedPasswordOutcome.Seed); + password.Should().Be(Configured); + } + + [Theory] + [InlineData(true)] // required (admin) + [InlineData(false)] // optional (demo) + public void ResolveSeedPassword_LocalOrTest_NoConfig_UsesDefault(bool requiredOutsideLocal) + { + var outcome = UserSeedService.ResolveSeedPassword( + configuredPassword: null, + Default, + isLocalOrTest: true, + requiredOutsideLocal, + out var password + ); + + outcome.Should().Be(SeedPasswordOutcome.Seed); + password.Should().Be(Default); + } + + [Fact] + public void ResolveSeedPassword_RealDeployment_RequiredAndUnconfigured_Fails() + { + // The security-critical path: the admin account must never fall back to + // the compiled-in default outside a local/test environment. + var outcome = UserSeedService.ResolveSeedPassword( + configuredPassword: null, + Default, + isLocalOrTest: false, + requiredOutsideLocal: true, + out var password + ); + + outcome.Should().Be(SeedPasswordOutcome.Fail); + password.Should().BeNull(); + } + + [Fact] + public void ResolveSeedPassword_RealDeployment_OptionalAndUnconfigured_Skips() + { + var outcome = UserSeedService.ResolveSeedPassword( + configuredPassword: null, + Default, + isLocalOrTest: false, + requiredOutsideLocal: false, + out var password + ); + + outcome.Should().Be(SeedPasswordOutcome.Skip); + password.Should().BeNull(); + } + + [Fact] + public void ResolveSeedPassword_EmptyConfiguredPassword_TreatedAsUnconfigured() + { + var outcome = UserSeedService.ResolveSeedPassword( + configuredPassword: "", + Default, + isLocalOrTest: false, + requiredOutsideLocal: true, + out _ + ); + + outcome.Should().Be(SeedPasswordOutcome.Fail); + } +} diff --git a/packages/SimpleModule.UI/components/layouts/layout-provider.tsx b/packages/SimpleModule.UI/components/layouts/layout-provider.tsx index 5def956a..3594be15 100644 --- a/packages/SimpleModule.UI/components/layouts/layout-provider.tsx +++ b/packages/SimpleModule.UI/components/layouts/layout-provider.tsx @@ -24,7 +24,9 @@ function AutoLayout({ children }: { children: React.ReactNode }) { } interface PageModule { - default: { layout?: (content: React.ReactNode) => React.ReactNode }; + default: React.ComponentType & { + layout?: (content: React.ReactNode) => React.ReactNode; + }; } export function resolveLayout(page: PageModule) { diff --git a/scripts/typecheck.mjs b/scripts/typecheck.mjs index 214766bc..52391ad5 100644 --- a/scripts/typecheck.mjs +++ b/scripts/typecheck.mjs @@ -3,9 +3,9 @@ /** * typecheck.mjs * - * Runs `tsc --noEmit` in every module and package that has a tsconfig.json. - * Each project is checked independently so @/* path aliases resolve correctly. - * All checks run in parallel for speed. + * Runs `tsc --noEmit` in every module and package that has a tsconfig.json, + * plus the Host ClientApp. Each project is checked independently so @/* path + * aliases resolve correctly. All checks run in parallel for speed. * * Exit codes: * 0 = All projects pass type checking @@ -68,9 +68,26 @@ function checkProject(dir) { }); } +// The Host ClientApp must always be in this list — it is the Inertia +// bootstrap that every module page loads through. It went unchecked once +// (no tsconfig.json, not listed here) and shipped a ReferenceError in its +// page-load error handler. +const clientAppDir = path.join( + projectRoot, + 'template', + 'SimpleModule.Host', + 'ClientApp', +); + +if (!fs.existsSync(path.join(clientAppDir, 'tsconfig.json'))) { + console.error(`Missing tsconfig.json in ${clientAppDir} — ClientApp must be type-checked.`); + process.exit(1); +} + const projects = [ ...findProjects(modulesDir, 'nested'), ...findProjects(packagesDir, 'flat'), + clientAppDir, ]; const results = await Promise.all(projects.map(checkProject)); diff --git a/scripts/validate-i18n.mjs b/scripts/validate-i18n.mjs index 54196cee..4957a7b3 100644 --- a/scripts/validate-i18n.mjs +++ b/scripts/validate-i18n.mjs @@ -56,9 +56,16 @@ function parseJsonFile(filePath) { // Main const localesDirs = findModuleLocales(modulesDir); +// Self-check: modules ship Locales directories today, so finding zero means the +// scan path drifted out from under this script — the same silent-no-op failure +// that let validate-pages.mjs validate nothing for months. Fail loudly rather +// than report success over an empty set. if (localesDirs.length === 0) { - console.log('No Locales directories found. Nothing to validate.'); - process.exit(0); + console.error( + `ERROR: No Locales directories found under '${modulesDir}'. The module layout ` + + 'has likely changed — update findModuleLocales / the scan path.', + ); + process.exit(1); } for (const { localesDir, project } of localesDirs) { diff --git a/tasks/todo.md b/tasks/todo.md index 9ad3a546..b6e7c32b 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -1,78 +1,79 @@ -# Task: Design-system consistency pass across all module pages +# Fix critical issues from framework review (2026-06-09) -Goal: every page uses the design system consistently → run /qa → open PR with screenshots. +## Critical 1 — page-registry guard broken on both ends +- [x] Fix `validate-pages.mjs` path: scan `modules/*/src/*/` (real layout is `src/SimpleModule.`), skip `obj`/`bin` +- [x] Add self-check: fail when zero C# files or zero view endpoints are found repo-wide (path drift can never silently disable the guard again) +- [x] Fix `app.tsx:232` `showErrorToast(...)` → `showToast({ variant: 'error', ... })` (ReferenceError on failed page load) +- [x] Add `ClientApp/tsconfig.json` and include ClientApp in `scripts/typecheck.mjs` -Scope: 13 tracked modules with `.tsx` source. The 8 untracked dirs (Agents, Chat, Datasets, -Map, Marketplace, Orders, PageBuilder, Products) are stale build artifacts with NO source — left untouched, flagged to user. +## Critical 2 — default deployment one request from admin token +- [x] `UserSeedService`: fail fast in non-Development when `Seed:AdminPassword` unset; never seed test user with default password outside Development +- [x] OpenIddict: refuse `AllowPasswordFlow` in Production; fail fast on ephemeral signing/encryption keys in Production +- [x] `SimpleModuleHostExtensions`: stop clearing `KnownProxies`/`KnownIPNetworks` unconditionally — config-driven (`ForwardedHeaders:KnownProxies`/`KnownNetworks`/`TrustAllProxies`) +- [x] `docker-compose.yml`: explicit `OpenIddict__AllowPasswordGrant: "false"`, required seed-password env vars +- [x] Enforce `FileStorageModuleOptions` (MaxFileSizeMb / AllowedExtensions) in UploadEndpoint -Out of scope (noted, not changed): i18n hardcoded-string gaps; the intentional centered-card -auth-page layout (only token/control bugs inside auth pages are fixed, not forced into PageShell). +## Critical 3 — remaining #242-class DbContext races +- [x] `AdminService.GetAdminOverviewAsync` — sequential awaits +- [x] `Admin/Pages/Admin/UsersEditEndpoint` — sequential awaits +- [x] `TenantFeatureHelper.GetOverridesForTenantAsync` — sequential awaits (throws with 2+ active flags) -## Fixes (from parallel line-level audit) +## Outbox gaps (events lost on crash after SaveChanges) +- [x] `TenantService.CreateTenantAsync` — use `IDbContextOutbox` like UpdateTenantAsync already does +- [x] `FileStorageService.UploadFileAsync` — same -### HIGH — color tokens breaking dark mode / raw controls -- [ ] Tenants/tenantStatus.ts — raw palette → Badge variant map (success/warning/danger) -- [ ] Tenants/Browse.tsx, Manage.tsx — status span → -- [ ] Tenants/Features.tsx — text-green/red-600 → -- [ ] BackgroundJobs/Dashboard.tsx — text-red-500, border-red-200, hover:bg-red-50 → semantic -- [ ] BackgroundJobs/Detail.tsx — text-red-600, border-red-200, bg-red-50/text-red-800 → semantic -- [ ] FeatureFlags/Manage.tsx:264 — raw → -- [ ] RateLimiting/components/RulesTable.tsx — text-muted-foreground (undefined) → text-text-muted -- [ ] Email/History.tsx — text-destructive (undefined) → text-danger -- [ ] OpenIddict/OAuthCallback.tsx — text-muted → text-text-muted -- [ ] Dashboard/Home.tsx — text-white → text-text-inverse -- [ ] Users/Login.tsx, Register.tsx — text-white + inline var → bg-primary text-text-inverse; raw checkbox → Checkbox -- [ ] Users/LoginWith2fa.tsx — raw checkbox → Checkbox +## Critical 4 — docs describe phantom modules +- [x] CLAUDE.md: load-test section lists 11 scenarios incl. Products/Orders/Marketplace/PageBuilder — only 6 exist (Admin, AuditLogs, FeatureFlags, FileStorage, Settings, Users) +- [x] CLAUDE.md: `modules/Products/src/Products` examples use wrong layout (`src/SimpleModule.`) — likely origin of the validate-pages bug +- [x] Sweep docs/CONSTITUTION.md + skills for phantom-module/wrong-layout references +- (untracked WIP dirs in the main checkout are stale build artifacts — left alone) -### MEDIUM — hand-rolled layout → PageShell; custom markup → DS components -- [ ] Settings/UserSettings.tsx — Container+h1 → PageShell; error banner → Alert -- [ ] Settings/AdminSettings.tsx — error banner → Alert -- [ ] Admin/RolesCreate, RolesEdit, UsersCreate, UsersEdit — Container+Breadcrumb+h1 → PageShell -- [ ] Admin/Roles.tsx — raw