diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c0f7f21..5b252df 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,10 +11,37 @@ jobs: - uses: actions/setup-dotnet@v4 with: dotnet-version: '10.0.x' - - name: Build API starter - run: dotnet build backend/Platform.Api/Platform.Api.csproj --configuration Release - - name: Test - run: dotnet test backend/Platform.Tests/Platform.Tests.csproj --configuration Release + - name: Build backend solution + run: dotnet build backend/SecurityLab.slnx --configuration Release + - name: Test backend solution + run: > + dotnet test backend/SecurityLab.slnx --configuration Release --no-build + --logger "trx" --results-directory TestResults + - name: Verify tests were discovered and executed + shell: bash + run: | + mapfile -t files < <(find TestResults -name '*.trx' 2>/dev/null) + if [ "${#files[@]}" -eq 0 ]; then + echo "::error::No TRX result files were produced." + exit 1 + fi + total=0 + for f in "${files[@]}"; do + n=$(grep -o 'executed="[0-9]*"' "$f" | head -n 1 | grep -o '[0-9]*' || true) + total=$((total + ${n:-0})) + done + echo "Executed tests: $total" + if [ "$total" -eq 0 ]; then + echo "::error::Zero tests were executed; a green run with no tests proves nothing." + exit 1 + fi + - name: Upload backend test results + if: always() + uses: actions/upload-artifact@v4 + with: + name: backend-test-results-${{ github.sha }} + path: TestResults/ + if-no-files-found: error frontend: runs-on: ubuntu-latest steps: diff --git a/backend/Platform.Api/Platform.Api.csproj b/backend/Platform.Api/Platform.Api.csproj index 0c96897..af34409 100644 --- a/backend/Platform.Api/Platform.Api.csproj +++ b/backend/Platform.Api/Platform.Api.csproj @@ -6,5 +6,6 @@ + diff --git a/backend/Platform.Api/Program.cs b/backend/Platform.Api/Program.cs index 47b0fbc..513139b 100644 --- a/backend/Platform.Api/Program.cs +++ b/backend/Platform.Api/Program.cs @@ -5,6 +5,7 @@ using Platform.Api.Domain.Identity; using Platform.Api.Middlewares; using Platform.Api.Services.Identity; +using Platform.Audit.Composition; // Oba modula imaju interfejs IAccessDecision, pa se koriste aliasi da ime ne bude dvosmisleno. using IdentityAccessDecision = Platform.Api.Services.Identity.IAccessDecision; @@ -13,6 +14,7 @@ var builder = WebApplication.CreateBuilder(args); builder.Services.AddAccessControl(); +builder.Services.AddAudit(); builder.Services.AddControllers(); builder.Services.AddHttpContextAccessor(); builder.Services.AddSingleton(TimeProvider.System); diff --git a/backend/Platform.Audit.Tests/CompositionScopeTests.cs b/backend/Platform.Audit.Tests/CompositionScopeTests.cs new file mode 100644 index 0000000..806587a --- /dev/null +++ b/backend/Platform.Audit.Tests/CompositionScopeTests.cs @@ -0,0 +1,48 @@ +using Microsoft.Extensions.DependencyInjection; +using NUnit.Framework; +using Platform.Access.Composition; +using Platform.Access.Contracts; +using Platform.Audit.Composition; + +namespace Platform.Audit.Tests; + +// Module registrations must survive strict DI validation (composition level). +[TestFixture] +public sealed class CompositionScopeTests +{ + private static readonly ServiceProviderOptions StrictOptions = new() + { + ValidateScopes = true, + ValidateOnBuild = true, + }; + + [Test] + public void AccessAndAuditRegistrations_PassScopeAndBuildValidation() + { + var services = new ServiceCollection(); + services.AddAccessControl(); + services.AddAudit(); + + using var provider = services.BuildServiceProvider(StrictOptions); + using var scope = provider.CreateScope(); + + var decision = scope.ServiceProvider.GetRequiredService(); + + Assert.That(decision, Is.Not.Null); + } + + [Test] + public void CaptiveDependency_IsRejectedByValidation() + { + var services = new ServiceCollection(); + services.AddScoped(); + services.AddSingleton(); + + var exception = Assert.Catch(() => + { + services.BuildServiceProvider(StrictOptions); + }); + + Assert.That(exception?.ToString(), Does.Contain("scoped").IgnoreCase); + } +} diff --git a/backend/Platform.Audit.Tests/HostStartupScopeTests.cs b/backend/Platform.Audit.Tests/HostStartupScopeTests.cs new file mode 100644 index 0000000..49b916c --- /dev/null +++ b/backend/Platform.Audit.Tests/HostStartupScopeTests.cs @@ -0,0 +1,56 @@ +using System.Net; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using NUnit.Framework; + +namespace Platform.Audit.Tests; + +// The real Platform.Api host must start with strict DI validation (host level). +// The Development environment enables ValidateScopes and ValidateOnBuild by default. +internal sealed class StrictHostFactory(bool addCaptiveDependency = false) + : WebApplicationFactory +{ + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + builder.UseEnvironment("Development"); + builder.ConfigureLogging(logging => logging.ClearProviders()); + + if (addCaptiveDependency) + { + builder.ConfigureServices(services => + { + services.AddScoped(); + services.AddSingleton(); + }); + } + } +} + +[TestFixture] +public sealed class HostStartupScopeTests +{ + [Test] + public async Task Host_StartsWithScopeValidation_AndHealthResponds() + { + using var factory = new StrictHostFactory(); + using var client = factory.CreateClient(); + + using var response = await client.GetAsync("/api/health"); + + Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.OK)); + } + + [Test] + public void Host_WithCaptiveDependency_FailsToStart() + { + var exception = Assert.Catch(() => + { + using var factory = new StrictHostFactory(addCaptiveDependency: true); + using var client = factory.CreateClient(); + }); + + Assert.That(exception?.ToString(), Does.Contain("scoped").IgnoreCase); + } +} diff --git a/backend/Platform.Audit.Tests/Platform.Audit.Tests.csproj b/backend/Platform.Audit.Tests/Platform.Audit.Tests.csproj new file mode 100644 index 0000000..9220093 --- /dev/null +++ b/backend/Platform.Audit.Tests/Platform.Audit.Tests.csproj @@ -0,0 +1,20 @@ + + + net10.0 + enable + enable + true + false + + + + + + + + + + + + + diff --git a/backend/Platform.Audit.Tests/ScopeValidationFixtures.cs b/backend/Platform.Audit.Tests/ScopeValidationFixtures.cs new file mode 100644 index 0000000..b8b81b0 --- /dev/null +++ b/backend/Platform.Audit.Tests/ScopeValidationFixtures.cs @@ -0,0 +1,12 @@ +namespace Platform.Audit.Tests; + +// Deliberate misconfiguration used to prove that scope validation is really +// switched on: a singleton that captures a scoped service (a "captive dependency"). +internal sealed class ScopedDependency +{ +} + +internal sealed class SingletonConsumer(ScopedDependency dependency) +{ + public ScopedDependency Dependency { get; } = dependency; +} diff --git a/backend/Platform.Audit/Application/.gitkeep b/backend/Platform.Audit/Application/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/backend/Platform.Audit/Composition/AuditServiceCollectionExtensions.cs b/backend/Platform.Audit/Composition/AuditServiceCollectionExtensions.cs new file mode 100644 index 0000000..fd3886b --- /dev/null +++ b/backend/Platform.Audit/Composition/AuditServiceCollectionExtensions.cs @@ -0,0 +1,23 @@ +using Microsoft.Extensions.DependencyInjection; + +namespace Platform.Audit.Composition; + +public static class AuditServiceCollectionExtensions +{ + /// + /// Registers the Audit module with the host. This is the single entry point + /// that Platform.Api calls; the host never registers Audit internals directly. + /// + /// + /// Skeleton only: nothing is registered yet. The audit contract, the writer and + /// the storage add their registrations here. Lifetimes follow + /// docs/components/audit/DI-GUIDELINES.md and are verified by the + /// scope-validation tests in Platform.Audit.Tests. + /// + public static IServiceCollection AddAudit(this IServiceCollection services) + { + ArgumentNullException.ThrowIfNull(services); + + return services; + } +} diff --git a/backend/Platform.Audit/Contracts/.gitkeep b/backend/Platform.Audit/Contracts/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/backend/Platform.Audit/Domain/.gitkeep b/backend/Platform.Audit/Domain/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/backend/Platform.Audit/Infrastructure/.gitkeep b/backend/Platform.Audit/Infrastructure/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/backend/Platform.Audit/Platform.Audit.csproj b/backend/Platform.Audit/Platform.Audit.csproj new file mode 100644 index 0000000..fc34a2b --- /dev/null +++ b/backend/Platform.Audit/Platform.Audit.csproj @@ -0,0 +1,10 @@ + + + net10.0 + enable + enable + + + + + diff --git a/backend/SecurityLab.slnx b/backend/SecurityLab.slnx new file mode 100644 index 0000000..c8a3250 --- /dev/null +++ b/backend/SecurityLab.slnx @@ -0,0 +1,7 @@ + + + + + + + diff --git a/docs/components/audit/DI-GUIDELINES.md b/docs/components/audit/DI-GUIDELINES.md new file mode 100644 index 0000000..49c22c1 --- /dev/null +++ b/docs/components/audit/DI-GUIDELINES.md @@ -0,0 +1,74 @@ +# Audit module: dependency injection lifetimes + +**Status:** proposal by the Audit component (AUD-01, AUD-06). It becomes a rule only after +it is agreed with the owners of `Platform.Access` and the Identity/Membership modules. +Nothing here is claimed as already accepted by them. + +## Why this matters + +Audit and authorization run on every request and will soon touch scoped services +(`DbContext`, the current identity, course membership). A singleton that captures a scoped +service reads one request's state on behalf of another request. In an authorization or +audit path that is a security defect, not only a bug. + +## Lifetime rules + +| Kind of service | Lifetime | Notes | +| --- | --- | --- | +| Read-only catalogs and registries, filled once during composition | Singleton | No per-request state. Safe for concurrent use. Registration happens before the first request; a duplicate fails at startup. | +| Anything that uses `DbContext`, the current identity, membership or a per-request policy | Scoped | Includes the audit writer once it persists to PostgreSQL. | +| Stateless helpers and validators | Singleton or transient | Only if they hold no request data. | +| Background work (outbox worker, delegation expiry) | Own scope per unit of work | Create it with `IServiceScopeFactory`; never use the root provider for scoped services. | + +Forbidden: + +- Injecting a scoped service into a singleton (captive dependency). +- Resolving a scoped service from the root provider. +- Relying on "last registration wins" to replace an adapter. Replace it explicitly. + +## Current state (checked on `main`, commit `e7474c7`) + +- `AddAccessControl()` registers `IAccessDecision` and `IAccessAuditWriter` as singletons. + This is valid today because their dependencies are singletons too. +- It stops being valid when a real audit adapter backed by a scoped `DbContext` replaces + `NullAccessAuditWriter`. `IAccessDecision` and `IAccessAuditWriter` must then become + scoped together, otherwise the host will not start under validation. This change belongs + to the Access integration and needs review by the Access team. +- `Platform.Api` now also registers the Identity services. Identity has its own `IAuditWriter` + (a singleton `LoggerAuditWriter`) and its own `IAccessDecision`, both in the + `Platform.Api.Services.Identity` namespace. They are separate from the Access contracts and + from the future Audit contract. Which audit contract the Identity code should use is an open + question for the Audit and Identity owners. +- `AddAudit()` registers nothing yet. It is the only entry point `Platform.Api` calls for the + Audit module. + +## Proposed registration order + +`AddAccessControl()` first, then `AddAudit()`. A later adapter that replaces +`NullAccessAuditWriter` must do so with an explicit, documented registration, not by +accident of ordering. + +## How this is verified + +`backend/Platform.Audit.Tests` runs with `ValidateScopes` and `ValidateOnBuild` enabled: + +- `CompositionScopeTests` builds the Access and Audit registrations and resolves + `IAccessDecision` inside a scope. +- `HostStartupScopeTests` starts the real `Platform.Api` host in the Development + environment (strict validation is the default there) and calls `/api/health`. +- Each level has a negative test with a deliberate captive dependency, which proves the + validation is actually active. Without it a passing test would prove nothing. + +Run from the repository root: + +```bash +dotnet test backend/SecurityLab.slnx --configuration Release +``` + +## Open questions + +- Should the audit writer be scoped, or a singleton that opens its own short-lived + `DbContext` per call through `IDbContextFactory`? Decide when the storage and the + writer are designed. +- Who owns the final registration order in `Program.cs`? Proposal: the Audit component + owner, with review by the Access team for any change to Access composition.