diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DagsoversigtWorksheetExportTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DagsoversigtWorksheetExportTests.cs index 8a0fd3a5..52801f00 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DagsoversigtWorksheetExportTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DagsoversigtWorksheetExportTests.cs @@ -11,7 +11,6 @@ using Microting.eForm.Infrastructure.Constants; using Microting.eFormApi.BasePn.Abstractions; using Microting.EformAngularFrontendBase.Infrastructure.Data; -using Microting.eFormApi.BasePn.Infrastructure.Database.Entities; using Microting.eFormApi.BasePn.Infrastructure.Helpers.PluginDbOptions; using Microting.TimePlanningBase.Infrastructure.Data.Entities; using NSubstitute; @@ -80,8 +79,7 @@ public async Task SetUpTest() // The all-workers export scopes its site list to the signed-in caller, // so these fixtures need a real one. Admin: scoping is a no-op. - var adminUserId = await GetBaseDbContextWithAdminAsync(); - userService.GetCurrentUserAsync().Returns(new EformUser { Id = adminUserId }); + await SeedAdminCallerAsync(userService); _service = new TimePlanningWorkingHoursService( Substitute.For>(), diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ExportTagFilterAndSiteTagsTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ExportTagFilterAndSiteTagsTests.cs index 16e5fd0d..21ab09d2 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ExportTagFilterAndSiteTagsTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ExportTagFilterAndSiteTagsTests.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Globalization; +using System.IO; using System.Linq; using System.Threading.Tasks; using DocumentFormat.OpenXml.Packaging; @@ -19,6 +20,7 @@ using Microting.eFormApi.BasePn.Abstractions; using Microting.eFormApi.BasePn.Infrastructure.Database.Entities; using Microting.eFormApi.BasePn.Infrastructure.Helpers.PluginDbOptions; +using Microting.eFormApi.BasePn.Infrastructure.Models.API; using Microting.EformAngularFrontendBase.Infrastructure.Data; using NSubstitute; using NUnit.Framework; @@ -102,8 +104,7 @@ public async Task SetUpTest() // test needs one. The default is an admin, for whom scoping is a no-op; // the scoping tests below seed a second, narrower caller into the same // context and re-point the substitute at it. - var adminUserId = await GetBaseDbContextWithAdminAsync(); - _userService.GetCurrentUserAsync().Returns(new EformUser { Id = adminUserId }); + await SeedAdminCallerAsync(_userService); _workingHoursService = new TimePlanningWorkingHoursService( Substitute.For>(), @@ -511,10 +512,324 @@ public async Task GetSiteTags_ManagerUser_ReturnsOnlySitesInTheirManagedTags() "A manager sees the sites in their managed tags plus their own — and nothing else"); } + // ------------------------------------------------------------------ + // 4. Single-worker export: the requested SiteId must be in the caller's + // scope. It arrives straight from the query string, so an unscoped + // overload let any signed-in user export any worker by guessing an id. + // ------------------------------------------------------------------ + + /// + /// The counterpart that decides whether the scoping is safe to ship: an + /// admin still exports any site they ask for, exactly as before. + /// + [Test] + public async Task SingleWorkerExport_AdminUser_CanExportAnySite() + { + var date = new DateTime(2026, 7, 26); + await SeedSiteAndPlanRegistration(siteUid: 9701, employeeNo: "1", date: date); + // A site belonging to somebody else entirely, and a manager elsewhere in + // the system: neither may narrow an ADMIN's reach. + await SeedSiteAndPlanRegistration( + siteUid: 9702, employeeNo: "2", date: date, email: "other9702@example.com", isManager: true); + + // The caller stays the admin seeded in SetUp. + await AssertExportSucceeded( + await SingleWorkerExport(9701, date), + "An admin must still export any site"); + await AssertExportSucceeded( + await SingleWorkerExport(9702, date), + "Scoping must be a no-op for an admin — including for another manager's site"); + } + + /// + /// A manager's reach on this endpoint is the same one the planning board + /// gives them: the sites carrying the tags they manage, plus their own. + /// + [Test] + public async Task SingleWorkerExport_ManagerUser_CanExportTheirOwnSiteAndOneInTheirManagedTags() + { + var date = new DateTime(2026, 7, 27); + await SeedManagerWithManagedTag( + email: "manager9711@example.com", + managerSiteUid: 9711, managedSiteUid: 9712, outsideSiteUid: 9713, date: date); + + await AssertExportSucceeded( + await SingleWorkerExport(9711, date), + "A manager must be able to export their own site"); + await AssertExportSucceeded( + await SingleWorkerExport(9712, date), + "A manager must be able to export a worker inside a tag they manage"); + } + + [Test] + public async Task SingleWorkerExport_ManagerUser_RefusesASiteOutsideTheirManagedTags() + { + var date = new DateTime(2026, 7, 28); + + // Seeded BEFORE the caller is switched, so the export below is first + // proved to work for the admin. Without that, a refusal could just as + // well mean the site was never exportable in this fixture. + await SeedSiteAndPlanRegistration(siteUid: 9723, employeeNo: "3", date: date); + await AssertExportSucceeded( + await SingleWorkerExport(9723, date), + "Precondition: the site must be exportable at all"); + + await SeedManagerWithManagedTag( + email: "manager9721@example.com", + managerSiteUid: 9721, managedSiteUid: 9722, outsideSiteUid: 9723, date: date, + seedOutsideSite: false); + + AssertExportRefused( + await SingleWorkerExport(9723, date), + "A site outside the manager's tags is not on their page and must not be exportable either"); + } + + /// + /// A plain worker sees exactly one row on the planning board. Unscoped, they + /// could download any colleague's hours by editing the site id in the URL. + /// + [Test] + public async Task SingleWorkerExport_PlainWorker_ExportsOwnSiteButIsRefusedAnother() + { + const string email = "worker9731@example.com"; + var date = new DateTime(2026, 7, 29); + await SeedSiteAndPlanRegistration( + siteUid: 9731, employeeNo: "1", date: date, email: email); + await SeedSiteAndPlanRegistration(siteUid: 9732, employeeNo: "2", date: date); + + // Same reasoning as above: prove the colleague's site is exportable + // before asserting that this caller cannot export it. + await AssertExportSucceeded( + await SingleWorkerExport(9732, date), + "Precondition: the colleague's site must be exportable at all"); + + await SeedNonAdminCallerAsync(email); + + await AssertExportSucceeded( + await SingleWorkerExport(9731, date), + "A worker must still be able to export their own hours"); + AssertExportRefused( + await SingleWorkerExport(9732, date), + "A plain worker must not be able to export a colleague by guessing their site id"); + } + + /// + /// The refusal must be blind. Out-of-scope and "no such site" answer with + /// the same body, or the difference between them IS an oracle: a caller + /// walks the id space and learns which workers exist. + /// + [Test] + public async Task SingleWorkerExport_UnknownSiteIdAndOutOfScopeSiteId_AnswerIdentically() + { + const string email = "worker9741@example.com"; + var date = new DateTime(2026, 7, 30); + await SeedSiteAndPlanRegistration( + siteUid: 9741, employeeNo: "1", date: date, email: email); + await SeedSiteAndPlanRegistration(siteUid: 9742, employeeNo: "2", date: date); + + await SeedNonAdminCallerAsync(email); + + var outOfScope = await SingleWorkerExport(9742, date); + // 999_941 is deliberately in no table at all. + var unknown = await SingleWorkerExport(999_941, date); + + AssertExportRefused(outOfScope, "A colleague's site must be refused"); + AssertExportRefused(unknown, "An id that names no site must be refused"); + Assert.That(unknown.Message, Is.EqualTo(outOfScope.Message), + "A real-but-forbidden id and a non-existent id must be indistinguishable, " + + "or the caller can enumerate which site ids exist"); + } + + // ------------------------------------------------------------------ + // 5. The same scope on the JSON grid behind POST working-hours/index. + // The export reads its rows from here, so leaving this open would + // hand back as JSON exactly what the export now refuses. + // ------------------------------------------------------------------ + + [Test] + public async Task WorkingHoursIndex_AdminUser_CanReadAnySite() + { + var date = new DateTime(2026, 8, 3); + await SeedSiteAndPlanRegistration(siteUid: 9751, employeeNo: "1", date: date); + await SeedSiteAndPlanRegistration( + siteUid: 9752, employeeNo: "2", date: date, email: "other9752@example.com", isManager: true); + + // The caller stays the admin seeded in SetUp. + AssertIndexSucceeded(await WorkingHoursIndex(9751, date), "An admin must still read any site"); + AssertIndexSucceeded(await WorkingHoursIndex(9752, date), + "Scoping must be a no-op for an admin — including for another manager's site"); + } + + [Test] + public async Task WorkingHoursIndex_ManagerUser_ReadsTheirOwnSiteAndOneInTheirManagedTags() + { + var date = new DateTime(2026, 8, 4); + await SeedManagerWithManagedTag( + email: "manager9761@example.com", + managerSiteUid: 9761, managedSiteUid: 9762, outsideSiteUid: 9763, date: date); + + AssertIndexSucceeded(await WorkingHoursIndex(9761, date), "A manager must be able to read their own site"); + AssertIndexSucceeded(await WorkingHoursIndex(9762, date), + "A manager must be able to read a worker inside a tag they manage"); + } + + [Test] + public async Task WorkingHoursIndex_ManagerUser_RefusesASiteOutsideTheirManagedTags() + { + var date = new DateTime(2026, 8, 5); + + // Seeded and read as the admin first, so the refusal below cannot be + // explained by the site simply not being readable in this fixture. + await SeedSiteAndPlanRegistration(siteUid: 9773, employeeNo: "3", date: date); + AssertIndexSucceeded(await WorkingHoursIndex(9773, date), + "Precondition: the site must be readable at all"); + + await SeedManagerWithManagedTag( + email: "manager9771@example.com", + managerSiteUid: 9771, managedSiteUid: 9772, outsideSiteUid: 9773, date: date, + seedOutsideSite: false); + + AssertIndexRefused(await WorkingHoursIndex(9773, date), + "A site outside the manager's tags is not on their page and must not be readable either"); + } + + [Test] + public async Task WorkingHoursIndex_PlainWorker_ReadsOwnSiteButIsRefusedAnother() + { + const string email = "worker9781@example.com"; + var date = new DateTime(2026, 8, 6); + await SeedSiteAndPlanRegistration( + siteUid: 9781, employeeNo: "1", date: date, email: email); + await SeedSiteAndPlanRegistration(siteUid: 9782, employeeNo: "2", date: date); + + AssertIndexSucceeded(await WorkingHoursIndex(9782, date), + "Precondition: the colleague's site must be readable at all"); + + await SeedNonAdminCallerAsync(email); + + AssertIndexSucceeded(await WorkingHoursIndex(9781, date), + "A worker must still be able to read their own hours"); + AssertIndexRefused(await WorkingHoursIndex(9782, date), + "A plain worker must not be able to read a colleague by guessing their site id"); + } + + /// The same blindness the export owes, on the JSON route. + [Test] + public async Task WorkingHoursIndex_UnknownSiteIdAndOutOfScopeSiteId_AnswerIdentically() + { + const string email = "worker9791@example.com"; + var date = new DateTime(2026, 8, 7); + await SeedSiteAndPlanRegistration( + siteUid: 9791, employeeNo: "1", date: date, email: email); + await SeedSiteAndPlanRegistration(siteUid: 9792, employeeNo: "2", date: date); + + await SeedNonAdminCallerAsync(email); + + var outOfScope = await WorkingHoursIndex(9792, date); + var unknown = await WorkingHoursIndex(999_991, date); + + AssertIndexRefused(outOfScope, "A colleague's site must be refused"); + AssertIndexRefused(unknown, "An id that names no site must be refused"); + Assert.That(unknown.Message, Is.EqualTo(outOfScope.Message), + "A real-but-forbidden id and a non-existent id must be indistinguishable, " + + "or the caller can enumerate which site ids exist"); + } + // ------------------------------------------------------------------ // Helpers // ------------------------------------------------------------------ + /// Runs the working-hours grid for one day — the JSON route the + /// export reads its rows from. + private async Task>> WorkingHoursIndex( + int siteUid, DateTime date) => + await _workingHoursService.Index( + new TimePlanningWorkingHoursRequestModel + { + SiteId = siteUid, + DateFrom = date, + DateTo = date, + }); + + private static void AssertIndexSucceeded( + OperationDataResult> result, string because) + { + Assert.That(result.Success, Is.True, $"{because} (message: {result.Message})"); + Assert.That(result.Model, Is.Not.Null, because); + } + + /// Pins the refusal itself: without the message assertion this + /// would also pass on the catch-all ErrorWhileObtainingPlannings a crash + /// returns, which is the very body the oracle tests forbid. + private static void AssertIndexRefused( + OperationDataResult> result, string because) + { + Assert.That(result.Success, Is.False, because); + Assert.That(result.Message, Is.EqualTo("SiteNotFound"), + "The refusal must be the plugin's own not-found message, not a generic read failure"); + Assert.That(result.Model, Is.Null, "A refused read must hand back no rows at all"); + } + + /// Runs the single-worker export for one day and hands back the raw + /// result, so a test can assert on a refusal as well as on a workbook. + private async Task> SingleWorkerExport(int siteUid, DateTime date) => + await _workingHoursService.GenerateExcelDashboard( + new TimePlanningWorkingHoursRequestModel + { + SiteId = siteUid, + DateFrom = date, + DateTo = date, + }); + + private static async Task AssertExportSucceeded(OperationDataResult result, string because) + { + Assert.That(result.Success, Is.True, $"{because} (message: {result.Message})"); + Assert.That(result.Model, Is.Not.Null, because); + await result.Model!.DisposeAsync(); + } + + /// Pins the refusal itself, not merely "not a success": asserting on + /// the message separates a deliberate scope refusal from the catch-all + /// ErrorWhileCreatingExcelFile a crash would return. (The fixture's + /// localization substitute echoes the key it is handed.) + private static void AssertExportRefused(OperationDataResult result, string because) + { + Assert.That(result.Success, Is.False, because); + Assert.That(result.Message, Is.EqualTo("SiteNotFound"), + "The refusal must be the plugin's own not-found message, not a generic export failure"); + Assert.That(result.Model, Is.Null, "A refused export must hand back no workbook at all"); + } + + /// Seeds a manager who manages the tag "EL", a site carrying it and + /// a site carrying "Brand" instead, then points the caller at the manager. + /// Pass false when the caller has already + /// seeded that site itself. + private async Task SeedManagerWithManagedTag( + string email, int managerSiteUid, int managedSiteUid, int outsideSiteUid, DateTime date, + bool seedOutsideSite = true) + { + var managerAssignedSite = await SeedSiteAndPlanRegistration( + siteUid: managerSiteUid, employeeNo: "1", date: date, email: email, isManager: true); + await SeedSiteAndPlanRegistration(siteUid: managedSiteUid, employeeNo: "2", date: date); + if (seedOutsideSite) + { + await SeedSiteAndPlanRegistration(siteUid: outsideSiteUid, employeeNo: "3", date: date); + } + + var elTagId = await TagSiteByUid(managedSiteUid, "EL"); + await TagSiteByUid(outsideSiteUid, "Brand"); + await new AssignedSiteManagingTagEntity + { + AssignedSiteId = managerAssignedSite.Id, + TagId = elTagId, + WorkflowState = Constants.WorkflowStates.Created, + CreatedByUserId = 1, + UpdatedByUserId = 1, + }.Create(TimePlanningPnDbContext!); + + await SeedNonAdminCallerAsync(email); + } + /// Seeds a second, non-admin eform user into the fixture's /// BaseDbContext and points the IUserService substitute at it, so the next /// service call resolves that caller instead of the default admin. Whether diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/MobileFlexRecomputeAndCascadeTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/MobileFlexRecomputeAndCascadeTests.cs index 09d4a245..321d67d9 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/MobileFlexRecomputeAndCascadeTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/MobileFlexRecomputeAndCascadeTests.cs @@ -59,6 +59,15 @@ public async Task SetUpTest() var sdkDb = core.DbContextHelper.GetDbContext(); // --- SDK graph: site + worker + siteworker, keyed by the user's email --- + // + // The email keying is load-bearing beyond mere lookup: this fixture's + // EformUser carries no admin role and no security groups, so the + // working-hours grid resolves it through the PLAIN-WORKER branch of the + // caller scope — own site only, named by the SDK Worker that matches + // this email. The Index call below asks for SiteUid, the very site this + // worker is linked to, which is the only reason it is not refused. + // Break the email match, or link the worker to a different site, and + // every scoped call in this fixture starts failing with "SiteNotFound". var language = await sdkDb.Languages.FirstOrDefaultAsync(l => l.LanguageCode == "da"); if (language == null) { diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ReconcileServiceTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ReconcileServiceTests.cs index 30d3cedd..68b370e6 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ReconcileServiceTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ReconcileServiceTests.cs @@ -826,6 +826,10 @@ async Task Stored(PlanRegistration row) => [Test] public async Task WorkingHoursIndex_MarksReconciledLockedDaysAsIsLocked() { + // Index scopes the requested site to the signed-in caller; this seeds + // the admin user and points _userService at it ("me"). + await using var baseDbContext = GetBaseDbContext(); + await BuildAdminIndexServiceAsync(baseDbContext); await SeedAssignedSiteAsync(930); // Keep the MaxDaysEditable window out of the way, so only the // reconciled lock can set IsLocked on these past days. @@ -835,7 +839,7 @@ public async Task WorkingHoursIndex_MarksReconciledLockedDaysAsIsLocked() await SeedReconciledBoundaryAsync(930, DateTime.Now.Date.AddDays(-3)); await SeedPlain(930, DateTime.Now.Date.AddDays(-1)); - var result = await BuildWorkingHoursService().Index(new TimePlanningWorkingHoursRequestModel + var result = await BuildWorkingHoursService(baseDbContext).Index(new TimePlanningWorkingHoursRequestModel { SiteId = 930, DateFrom = DateTime.Now.Date.AddDays(-10), diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/TestBaseSetup.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/TestBaseSetup.cs index 6e0a0b3f..3480a845 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/TestBaseSetup.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/TestBaseSetup.cs @@ -5,9 +5,11 @@ using eFormCore; using Microsoft.EntityFrameworkCore; using Microting.eForm.Infrastructure; +using Microting.eFormApi.BasePn.Abstractions; using Microting.eFormApi.BasePn.Infrastructure.Database.Entities; using Microting.EformAngularFrontendBase.Infrastructure.Data; using Microting.TimePlanningBase.Infrastructure.Data; +using NSubstitute; using NUnit.Framework; using Testcontainers.MariaDb; using TimePlanning.Pn.Infrastructure.Data.Seed; @@ -114,10 +116,9 @@ protected BaseDbContext GetBaseDbContext() /// /// Seeds a holding one admin user and /// returns that user's id. Services that scope their result to the signed-in - /// caller — the planning board, the site-tags lookup, the all-workers - /// export — need a real caller to resolve; this is the admin caller, for - /// whom scoping is a no-op. Point the IUserService substitute's - /// GetCurrentUserAsync at the returned id. + /// caller — the planning board, the site-tags lookup, the working-hours grid, + /// both exports — need a real caller to resolve; this is the admin caller, + /// for whom scoping is a no-op. /// protected async Task GetBaseDbContextWithAdminAsync(string email = "admin@example.com") { @@ -144,6 +145,20 @@ protected async Task GetBaseDbContextWithAdminAsync(string email = "admin@e return user.Id; } + /// + /// Seeds the admin caller AND points at it — + /// the two halves belong together, because a service handed + /// without a matching + /// GetCurrentUserAsync resolves no caller at all and every scoped + /// call fails. + /// + protected async Task SeedAdminCallerAsync( + IUserService userService, string email = "admin@example.com") + { + var adminUserId = await GetBaseDbContextWithAdminAsync(email); + userService.GetCurrentUserAsync().Returns(new EformUser { Id = adminUserId }); + } + /// /// The connection string of the plugin database /// migrates, for tests that must build a context the way production does diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursDisplayParityTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursDisplayParityTests.cs index 642b8358..8fab00b7 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursDisplayParityTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursDisplayParityTests.cs @@ -72,12 +72,16 @@ public async Task SetUpTest() SnapshotEnabled = "0" }); + // Index scopes the requested site to the signed-in caller, so this + // fixture needs a real one. Admin: scoping is a no-op. + await SeedAdminCallerAsync(userService); + _service = new TimePlanningWorkingHoursService( Substitute.For>(), TimePlanningPnDbContext!, userService, localizationService, - baseDbContext: null!, + baseDbContext: SeededBaseDbContext!, options, coreService); } diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelExportE2ETests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelExportE2ETests.cs index 3418f40b..b556e478 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelExportE2ETests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelExportE2ETests.cs @@ -74,12 +74,16 @@ public async Task SetUpTest() SnapshotEnabled = "0" }); + // The export scopes the requested site to the signed-in caller, so this + // fixture needs a real one. Admin: scoping is a no-op. + await SeedAdminCallerAsync(userService); + _service = new TimePlanningWorkingHoursService( Substitute.For>(), TimePlanningPnDbContext!, userService, localizationService, - baseDbContext: null!, + baseDbContext: SeededBaseDbContext!, options, coreService); } diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelExportTagsColumnTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelExportTagsColumnTests.cs index 8753e51e..1d204aff 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelExportTagsColumnTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelExportTagsColumnTests.cs @@ -10,7 +10,6 @@ using Microting.eForm.Infrastructure.Constants; using Microting.eFormApi.BasePn.Abstractions; using Microting.EformAngularFrontendBase.Infrastructure.Data; -using Microting.eFormApi.BasePn.Infrastructure.Database.Entities; using Microting.eFormApi.BasePn.Infrastructure.Helpers.PluginDbOptions; using NSubstitute; using NUnit.Framework; @@ -81,8 +80,7 @@ public async Task SetUpTest() // The all-workers export scopes its site list to the signed-in caller, // so these fixtures need a real one. Admin: scoping is a no-op. - var adminUserId = await GetBaseDbContextWithAdminAsync(); - userService.GetCurrentUserAsync().Returns(new EformUser { Id = adminUserId }); + await SeedAdminCallerAsync(userService); _service = new TimePlanningWorkingHoursService( Substitute.For>(), diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelHolidayColumnTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelHolidayColumnTests.cs index 3f511a9e..ebd697d1 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelHolidayColumnTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelHolidayColumnTests.cs @@ -10,7 +10,6 @@ using Microting.eForm.Infrastructure.Constants; using Microting.eFormApi.BasePn.Abstractions; using Microting.EformAngularFrontendBase.Infrastructure.Data; -using Microting.eFormApi.BasePn.Infrastructure.Database.Entities; using Microting.eFormApi.BasePn.Infrastructure.Helpers.PluginDbOptions; using NSubstitute; using NUnit.Framework; @@ -108,8 +107,7 @@ public async Task SetUpTest() // The all-workers export scopes its site list to the signed-in caller, // so these fixtures need a real one. Admin: scoping is a no-op. - var adminUserId = await GetBaseDbContextWithAdminAsync(); - userService.GetCurrentUserAsync().Returns(new EformUser { Id = adminUserId }); + await SeedAdminCallerAsync(userService); _service = new TimePlanningWorkingHoursService( Substitute.For>(), diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelShiftColumnOrderTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelShiftColumnOrderTests.cs index 8b886e57..d7e1320f 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelShiftColumnOrderTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelShiftColumnOrderTests.cs @@ -10,7 +10,6 @@ using Microting.eForm.Infrastructure.Constants; using Microting.eFormApi.BasePn.Abstractions; using Microting.EformAngularFrontendBase.Infrastructure.Data; -using Microting.eFormApi.BasePn.Infrastructure.Database.Entities; using Microting.eFormApi.BasePn.Infrastructure.Helpers.PluginDbOptions; using NSubstitute; using NUnit.Framework; @@ -104,8 +103,7 @@ public async Task SetUpTest() // The all-workers export scopes its site list to the signed-in caller, // so these fixtures need a real one. Admin: scoping is a no-op. - var adminUserId = await GetBaseDbContextWithAdminAsync(); - userService.GetCurrentUserAsync().Returns(new EformUser { Id = adminUserId }); + await SeedAdminCallerAsync(userService); _service = new TimePlanningWorkingHoursService( Substitute.For>(), diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs index b5eb4268..5d3999a0 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs @@ -30,6 +30,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE using DocumentFormat.OpenXml.Packaging; using DocumentFormat.OpenXml.Spreadsheet; using DocumentFormat.OpenXml.Validation; +using Microting.eForm.Infrastructure; using Microting.eForm.Infrastructure.Data.Entities; using Microting.EformAngularFrontendBase.Infrastructure.Data; using Microting.TimePlanningBase.Infrastructure.Data.Entities; @@ -69,8 +70,88 @@ public class TimePlanningWorkingHoursService( IEFormCoreService coreHelper) : ITimePlanningWorkingHoursService { + /// + /// The set of sites the signed-in caller may see — the same scope the + /// planning board and the export dialog's worker count apply. Check + /// before reading anything else. + /// + private async Task ResolveScopeAsync(MicrotingDbContext sdkContext) + { + var assignedSites = await dbContext.AssignedSites + .AsNoTracking() + .Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) + .ToListAsync(); + return await SiteScopeResolver + .ResolveForCurrentUserAsync(assignedSites, dbContext, sdkContext, baseDbContext, userService) + .ConfigureAwait(false); + } + + /// + /// Null when the signed-in caller may see , + /// otherwise the localization key to refuse with. + /// + /// + /// Every site-specific reason answers with the SAME key on purpose. An + /// unknown site id, a site whose AssignedSite is removed, and a real site + /// belonging to somebody else are indistinguishable to the caller — + /// otherwise the two different 400 bodies let anyone enumerate which site + /// ids exist. The caller-specific keys the resolver itself returns + /// (UserNotFound and friends) do not reopen that: they depend only on who + /// is asking, so they are the same for every id that caller tries. + /// + /// An unknown id is refused for an admin too, and that is what makes the + /// two cases collapse: no AssignedSite row means the id is in nobody's + /// scope, admin's included. + /// + private async Task ResolveSiteAccessErrorAsync(int siteId, MicrotingDbContext sdkContext) + { + var scope = await ResolveScopeAsync(sdkContext); + if (scope.ErrorKey != null) + { + return scope.ErrorKey; + } + + return scope.Narrow([siteId]).Count == 0 ? "SiteNotFound" : null; + } + + /// + /// The working-hours grid for one site. SiteId comes from the request body, + /// so the caller's scope is checked here before any data is read; the export + /// paths call instead, having resolved the very + /// same scope once for the whole workbook. + /// public async Task>> Index( TimePlanningWorkingHoursRequestModel model) + { + try + { + var core = await coreHelper.GetCore(); + await using var sdkDbContext = core.DbContextHelper.GetDbContext(); + var accessError = await ResolveSiteAccessErrorAsync(model.SiteId, sdkDbContext); + if (accessError != null) + { + return new OperationDataResult>( + false, localizationService.GetString(accessError)); + } + } + catch (Exception ex) + { + SentrySdk.CaptureException(ex); + logger.LogError(ex.Message); + return new OperationDataResult>( + false, localizationService.GetString("ErrorWhileObtainingPlannings")); + } + + return await IndexUnscoped(model); + } + + /// + /// without the caller-scope check. Private, and named + /// for what it omits: every caller must have established that the signed-in + /// user may see model.SiteId before calling it. + /// + private async Task>> IndexUnscoped( + TimePlanningWorkingHoursRequestModel model) { try { @@ -2735,6 +2816,26 @@ public async Task> GenerateExcelDashboard(TimePlanni { var core = await coreHelper.GetCore(); var sdkContext = core.DbContextHelper.GetDbContext(); + + // Scope to the caller, from the same code the planning board, the + // export dialog's worker count and the all-workers export use. + // SiteId arrives straight from the query string, so without this any + // signed-in user could export any worker's hours by guessing an id. + // + // Before the lookups below, not after: those throw into the catch + // for an id that names no site, and that second, distinguishable + // 400 body would tell the guesser which ids are real. Refusing here + // gives the unknown id and the out-of-scope id one answer. + var accessError = await ResolveSiteAccessErrorAsync(model.SiteId, sdkContext); + if (accessError != null) + { + // Refuse outright rather than narrow: an empty or substituted + // workbook would read as a successful export of the worker that + // was asked for. + return new OperationDataResult(false, + localizationService.GetString(accessError)); + } + var site = await sdkContext.Sites.FirstAsync(x => x.MicrotingUid == model.SiteId); var siteWorker = await sdkContext.SiteWorkers.FirstAsync(x => x.SiteId == site.Id); var worker = await sdkContext.Workers.FirstAsync(x => x.Id == siteWorker!.WorkerId); @@ -2778,8 +2879,10 @@ public async Task> GenerateExcelDashboard(TimePlanni var timeStamp = $"{DateTime.UtcNow:yyyyMMdd_HHmmss}_{Guid.NewGuid():N}"; var filePath = Path.Combine(Path.GetTempPath(), "results", $"{timeStamp}_.xlsx"); - // Fetch data early so we can pre-compute pay lines for header discovery - var content = await Index(model); + // Fetch data early so we can pre-compute pay lines for header + // discovery. Unscoped: the gate at the top of this method has + // already cleared this SiteId for this caller. + var content = await IndexUnscoped(model); if (!content.Success) return new OperationDataResult(false, content.Message); // remove the first entry from the content.Model @@ -3373,13 +3476,7 @@ public async Task> GenerateExcelDashboard( // export dialog's worker count use. Without this a manager saw "3 // workers" in the dialog and downloaded the whole organisation — // and any non-admin could export every worker in the system. - var assignedSites = await dbContext.AssignedSites - .AsNoTracking() - .Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) - .ToListAsync(); - var scope = await SiteScopeResolver - .ResolveForCurrentUserAsync(assignedSites, dbContext, sdkContext, baseDbContext, userService) - .ConfigureAwait(false); + var scope = await ResolveScopeAsync(sdkContext); if (scope.ErrorKey != null) { return new OperationDataResult(false, @@ -3469,7 +3566,10 @@ public async Task> GenerateExcelDashboard( } } - var dataResult = await Index(new TimePlanningWorkingHoursRequestModel + // Unscoped: siteIds was narrowed to the caller's scope above, and + // re-resolving that same scope once per site would be N round + // trips for an answer that cannot change between them. + var dataResult = await IndexUnscoped(new TimePlanningWorkingHoursRequestModel { DateFrom = model.DateFrom, DateTo = model.DateTo,