From 2033aea31b4ffc90db8c9d28471355fff7b26b65 Mon Sep 17 00:00:00 2001 From: cesnimda Date: Tue, 10 Nov 2026 12:48:38 +0100 Subject: [PATCH] fix(security): integrate mail subject isolation into A3 --- .../AdminProcessingControllerTests.cs | 82 +++- .../Controllers/AdminAuditController.cs | 3 +- .../ADMIN_AUDIT_ISOLATION_VALIDATION.md | 9 + .../ui-modernisation/REFINEMENT_AND_EUROPE.md | 2 +- .../admin-audit-isolation/INTEGRATED.trx | 370 ++++++++++++++++++ .../evidence/admin-audit-isolation/TESTS.txt | Bin 0 -> 2108 bytes 6 files changed, 463 insertions(+), 3 deletions(-) create mode 100644 docs/ui-modernisation/ADMIN_AUDIT_ISOLATION_VALIDATION.md create mode 100644 docs/ui-modernisation/evidence/admin-audit-isolation/INTEGRATED.trx create mode 100644 docs/ui-modernisation/evidence/admin-audit-isolation/TESTS.txt diff --git a/JobTrackerApi.Tests/AdminProcessingControllerTests.cs b/JobTrackerApi.Tests/AdminProcessingControllerTests.cs index 204d50b0..754ecdf6 100644 --- a/JobTrackerApi.Tests/AdminProcessingControllerTests.cs +++ b/JobTrackerApi.Tests/AdminProcessingControllerTests.cs @@ -159,6 +159,82 @@ public sealed class AdminProcessingControllerTests Assert.Equal(expected, (await client.GetAsync(route)).StatusCode); } + [Fact] + public async Task Audit_log_keeps_mail_subjects_in_the_owner_record() + { + await using var host = await Host.Start(); + const string privateSubject = "Synthetic private correspondence subject"; + const string operationalNote = "Synthetic workflow status change"; + await using (var scope = host.Services.CreateAsyncScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + db.JobEvents.AddRange( + new JobEvent { JobApplicationId = 2, Type = "ReplyReceived", NewValue = "True:2026-11-03", Note = privateSubject, At = DateTime.UtcNow }, + new JobEvent { JobApplicationId = 2, Type = "StatusChanged", OldValue = "Applied", NewValue = "Interview", Note = operationalNote, At = DateTime.UtcNow }); + await db.SaveChangesAsync(); + } + + using var anonymous = host.Client(null); + using var ordinary = host.Client("owner"); + foreach (var (client, status) in new[] { (anonymous, HttpStatusCode.Unauthorized), (ordinary, HttpStatusCode.Forbidden) }) + { + using var response = await client.GetAsync("/api/admin/audit"); + Assert.Equal(status, response.StatusCode); + Assert.DoesNotContain(privateSubject, await response.Content.ReadAsStringAsync()); + } + + using var admin = host.Client("admin"); + var audit = await admin.GetFromJsonAsync("/api/admin/audit"); + var events = audit.EnumerateArray().ToDictionary(item => item.GetProperty("type").GetString()!); + Assert.DoesNotContain(privateSubject, audit.GetRawText()); + var reply = events["ReplyReceived"]; + Assert.Equal(JsonValueKind.Null, reply.GetProperty("note").ValueKind); + Assert.Equal("True:2026-11-03", reply.GetProperty("newValue").GetString()); + Assert.Equal(2, reply.GetProperty("jobApplicationId").GetInt32()); + Assert.Equal(operationalNote, events["StatusChanged"].GetProperty("note").GetString()); + + await using var verifyScope = host.Services.CreateAsyncScope(); + var verifyDb = verifyScope.ServiceProvider.GetRequiredService(); + Assert.Equal(privateSubject, await verifyDb.JobEvents.IgnoreQueryFilters().Where(item => item.Type == "ReplyReceived").Select(item => item.Note).SingleAsync()); + } + + [Fact] + public async Task User_polling_does_not_expose_another_owners_notifications_or_processing_records() + { + await using var host = await Host.Start(); + var foreignNoticeId = Guid.NewGuid(); + const string foreignNotice = "Synthetic notice for the other owner"; + await using (var scope = host.Services.CreateAsyncScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + db.UserNotifications.AddRange( + new UserNotification { Id = Guid.NewGuid(), OwnerUserId = "owner", Kind = "operation_failed", Title = "Operation failed", Message = "Synthetic owner notice", CreatedAtUtc = DateTime.UtcNow }, + new UserNotification { Id = foreignNoticeId, OwnerUserId = "other", Kind = "operation_failed", Title = "Operation failed", Message = foreignNotice, CreatedAtUtc = DateTime.UtcNow }); + await db.SaveChangesAsync(); + } + + using var owner = host.Client("owner"); + var notices = await owner.GetFromJsonAsync("/api/notifications?owner=other"); + Assert.Equal("Synthetic owner notice", Assert.Single(notices.EnumerateArray()).GetProperty("message").GetString()); + var unread = await owner.GetFromJsonAsync("/api/notifications/unread-count?owner=other"); + Assert.Equal(1, unread.GetProperty("count").GetInt32()); + Assert.Equal(HttpStatusCode.NotFound, (await owner.PostAsync($"/api/notifications/{foreignNoticeId}/read", null)).StatusCode); + Assert.Equal(HttpStatusCode.NotFound, (await owner.DeleteAsync($"/api/notifications/{foreignNoticeId}")).StatusCode); + Assert.Equal(HttpStatusCode.NotFound, (await owner.GetAsync($"/api/operations/{host.ForeignSubjectOperationId}")).StatusCode); + var ownOperation = await owner.GetStringAsync($"/api/operations/{host.OperationId}"); + foreach (var internalValue in new[] { "SECRET_TOKEN", "SECRET_EXCEPTION", "SECRET_RESULT", "SECRET_CV_CONTENT" }) + Assert.DoesNotContain(internalValue, ownOperation); + + using var other = host.Client("other"); + notices = await other.GetFromJsonAsync("/api/notifications"); + Assert.Equal(foreignNotice, Assert.Single(notices.EnumerateArray()).GetProperty("message").GetString()); + await using var verifyScope = host.Services.CreateAsyncScope(); + var verifyDb = verifyScope.ServiceProvider.GetRequiredService(); + var notice = await verifyDb.UserNotifications.IgnoreQueryFilters().SingleAsync(item => item.Id == foreignNoticeId); + Assert.Null(notice.ReadAtUtc); + Assert.Null(notice.DismissedAtUtc); + } + [Fact] public async Task Admin_projection_filters_owners_and_never_returns_raw_processing_contents_or_lease_data() { @@ -216,7 +292,9 @@ public sealed class AdminProcessingControllerTests var builder = WebApplication.CreateBuilder(); builder.Logging.ClearProviders(); builder.WebHost.UseUrls("http://127.0.0.1:0"); builder.Services.AddControllers().AddApplicationPart(typeof(AuthController).Assembly); - builder.Services.AddAuthentication("test").AddScheme("test", _ => { }); + builder.Services.AddAuthentication("test") + .AddScheme("test", _ => { }) + .AddScheme("local", _ => { }); builder.Services.AddAuthorization(); builder.Services.AddHttpContextAccessor(); builder.Services.AddRateLimiter(options => { @@ -226,6 +304,8 @@ public sealed class AdminProcessingControllerTests _ => new FixedWindowRateLimiterOptions { PermitLimit = permits, Window = TimeSpan.FromMinutes(1), QueueLimit = 0 })); }); builder.Services.AddScoped(); + builder.Services.AddScoped(); + builder.Services.AddScoped(); builder.Services.AddSingleton(TimeProvider.System); builder.Services.AddDbContext(options => options.UseSqlite($"Data Source={Path.Combine(directory, "processing.db")};Pooling=False")); var app = builder.Build(); diff --git a/JobTrackerApi/Controllers/AdminAuditController.cs b/JobTrackerApi/Controllers/AdminAuditController.cs index e8817d24..7468db9a 100644 --- a/JobTrackerApi/Controllers/AdminAuditController.cs +++ b/JobTrackerApi/Controllers/AdminAuditController.cs @@ -51,7 +51,8 @@ public sealed class AdminAuditController : ControllerBase e.Type, e.OldValue, e.NewValue, - e.Note, + // Imported replies keep the mailbox subject in the owner's timeline. + Note = e.Type == "ReplyReceived" ? null : e.Note, e.At, e.JobApplicationId }) diff --git a/docs/ui-modernisation/ADMIN_AUDIT_ISOLATION_VALIDATION.md b/docs/ui-modernisation/ADMIN_AUDIT_ISOLATION_VALIDATION.md new file mode 100644 index 00000000..7695c36e --- /dev/null +++ b/docs/ui-modernisation/ADMIN_AUDIT_ISOLATION_VALIDATION.md @@ -0,0 +1,9 @@ +# Admin Audit isolation on the A3 integration branch + +Applied the controller and regression-test changes from security checkpoint `d4c486681ad51424bfd33545d518e87193ffbc56` to A3 base `2717c6afaf6375e2e1abd7d63cbfa613a802ce83`. Git comparisons confirmed both input files matched the security patch's parent and both outputs matched its committed files. No other security branch changes were integrated. + +Admin Audit now omits the mailbox subject from imported `ReplyReceived` notes. The loopback HTTP fixture verified anonymous and non-admin denial, the redacted admin response, retained non-private audit details and the unchanged stored owner note. It also verified owner-scoped notification reads and writes and processing detail access. There was no migration or data rewrite. + +**PASSED 57 selected backend tests**, zero failures or skips, pinned .NET SDK 9.0.317 in Release with no restore. The native [TRX](evidence/admin-audit-isolation/INTEGRATED.trx) records 31 AdminProcessing, 19 Gmail and 7 Users cases. The seven Users cases overlap the separate read-only 2FA checkpoint. The native hash before Git text normalisation is `840180a639bbc63f9869ae013faae33f10507a0152524c89590f44357fcacea5`. + +The fixtures used an isolated local database and synthetic users. Broader audit minimisation, real MariaDB and live provider checks, approved support access and final integrated security testing remain open. diff --git a/docs/ui-modernisation/REFINEMENT_AND_EUROPE.md b/docs/ui-modernisation/REFINEMENT_AND_EUROPE.md index 114b3dc7..2e2f49d4 100644 --- a/docs/ui-modernisation/REFINEMENT_AND_EUROPE.md +++ b/docs/ui-modernisation/REFINEMENT_AND_EUROPE.md @@ -4,7 +4,7 @@ Owner addendum received 9 November 2026. Source `E:/Storage/Projects/Active/temp | Work package | Next independently eligible slice | Gate or residual requirement | | --- | --- | --- | -| Admin information isolation | First slice published on `codex/security-offline-preflight` at `ecadc29b`. Reproduced and removed imported mail subjects from the Admin Audit projection. 62 selected offline tests passed, including owner-bound notices and operations. | Integrated security, remaining delivery paths, broader minimisation and deployment checks remain separate. | +| Admin information isolation | The original redaction patch passed 62 tests on the security branch. [A3 integration](ADMIN_AUDIT_ISOLATION_VALIDATION.md) matched the exact source/test patch and passed 57 selected tests. The runs overlap and remain separate evidence. | Integrated security, remaining delivery paths, broader minimisation and deployment checks remain separate. | | Discover Jobs | [Search-first layout](visual-correction/DISCOVERY_SEARCH_FIRST.md) moves the query above filters and qualifies the existing NAV request, keyboard submit and source disclosure. | Expose new filters, map and multilingual suggestions only with qualified data and the existing GJ architecture. Mobile filter drawer and focused detail remain open. | | Applications | [Title-first rows and explicit bulk mode](visual-correction/APPLICATION_SELECTION_REVIEW.md) qualified at 36 configurations. | Secondary action consolidation and final native/visual review remain open. | | Admin Users and Audit | [Aligned rows](visual-correction/ADMIN_USER_ALIGNMENT_REVIEW.md) and [read-only 2FA status](visual-correction/ADMIN_USER_2FA_REVIEW.md) each passed their own 36-configuration checks. Compact user actions and audit filter/record presentation remain open. | 2FA recovery and private support access require explicit owner approval of their technical and privacy designs. | diff --git a/docs/ui-modernisation/evidence/admin-audit-isolation/INTEGRATED.trx b/docs/ui-modernisation/evidence/admin-audit-isolation/INTEGRATED.trx new file mode 100644 index 00000000..3d0809e3 --- /dev/null +++ b/docs/ui-modernisation/evidence/admin-audit-isolation/INTEGRATED.trx @@ -0,0 +1,370 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 9.0.19) +[xUnit.net 00:00:00.19] Discovering: JobTrackerApi.Tests +[xUnit.net 00:00:00.33] Discovered: JobTrackerApi.Tests +[xUnit.net 00:00:00.34] Starting: JobTrackerApi.Tests +[xUnit.net 00:00:08.38] Finished: JobTrackerApi.Tests + + + + \ No newline at end of file diff --git a/docs/ui-modernisation/evidence/admin-audit-isolation/TESTS.txt b/docs/ui-modernisation/evidence/admin-audit-isolation/TESTS.txt new file mode 100644 index 0000000000000000000000000000000000000000..faa7dab28022a07e0218740ea51669501e554f79 GIT binary patch literal 2108 zcmdUwO>fgc5QgVmiT|*@fz(8eM4_z&Qs@UN2UL-?CtpnLz^Z9%#SYD{r#$a&+!#o! zR9uKwUeC_#ygNHHJ9GcjnX^w;*jF3c%u*ZKjpa~%E6pS8*n8#^JGR8GtO~LU`UIUR zqhN1hUGxgekSLe;IN>HjOY94H6TUQ9oxsb%Il|Wc2jq1KUF@1LUf2#dcXr8oY#)NncS{cpK$>&X z_W7qvoE4EeMCGZwZR*LVF#h!AdVkyWKHG8L zG{ZK_n#NPkUh>92+ZmazDy6EUYF65wT|>{v{8(S!*^gyCcokmzwGAF#heOuh>#S#Q z!R^`WFzb|sy#BpqFIBlA;f;b zDBz{+blhgl+zZ?Wd5n(gTm@J4wC0rWth2C7ltWgA(>?lrDJ#QXSvbSmc%Euzg^d$X zgm=EgK8F6^dO(IZMp&6(ucwFHvQ`JuZ9M#TAIXWP57H?*IXbcXC{7*dIWl=(ou~q5 z@|*Sr*)$}dKGIYAfjY%s>TZh=B}M8Ijfx?^@8T`hF!@grRdg~ssJL