diff --git a/docs/ACCOUNTING_AUDIT.md b/docs/ACCOUNTING_AUDIT.md index 3ebad00..80d9f67 100644 --- a/docs/ACCOUNTING_AUDIT.md +++ b/docs/ACCOUNTING_AUDIT.md @@ -118,13 +118,17 @@ Verification of O3+O4: `dotnet build` clean; `dotnet test tests/PowderCoating.Un --- -## Remaining (non-O1–O4) — known lower-risk follow-up -Pure **read-path dropdown/display** account lookups still rely on the global tenant filter (correct for -non-SuperAdmin; only a SuperAdmin acting inside a company could see another tenant's accounts in a picker). -These are app-wide, not specific to the accounting posting path, e.g. `VendorCreditsController` -PopulateDropdowns/Details, `BillsController` PopulateDropdowns and Index/edit account lists, -`ExpensesController` filter + categorization pickers. Tracked here so they aren't mistaken for a regression; -fold into a broader defense-in-depth pass if/when desired. +## Read-path account lookup sweep (Tier 1–3) — **RESOLVED** +Completed the app-wide defense-in-depth pass: every `Accounts` lookup in a controller now carries an +explicit `CompanyId` predicate, matching the standing rule in CLAUDE.md. ~19 lookups across 12 controllers: +- **Tier 1 (write-path validation):** `AccountsController` duplicate account-number check on Create/Edit. +- **Tier 2 (dropdowns/lists):** `AccountsController` (Index/year-end/parent dropdown), `BankReconciliations`, + `Bills` (bank list + receipt-scan + suggest), `Budgets`, `CatalogItems`, `Expenses`, `FixedAssets`, + `Inventory`, `JournalEntries` (chart dropdown), `Vendors`. +- **Tier 3 (`accountIds.Contains` display maps):** `JournalEntries` Details, `Reports` budget vs actual, + `VendorCredits` Details — scoped via the in-scope entity's `CompanyId` for uniformity. +`companyId` source per controller: `_tenantContext.GetCurrentCompanyId()` where available, else the +in-scope entity's `CompanyId`, else `_userManager` current user. Build clean; 291 unit tests pass. ## Status All audit findings **O1–O4 are resolved** on `dev`. Original audit numbering #1–3/#5/#6/#8 remains diff --git a/src/PowderCoating.Web/Controllers/AccountsController.cs b/src/PowderCoating.Web/Controllers/AccountsController.cs index b97608c..847eedc 100644 --- a/src/PowderCoating.Web/Controllers/AccountsController.cs +++ b/src/PowderCoating.Web/Controllers/AccountsController.cs @@ -55,7 +55,8 @@ public class AccountsController : Controller // GET: /Accounts public async Task Index() { - var accounts = await _unitOfWork.Accounts.GetAllAsync(false, a => a.ParentAccount); + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; + var accounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId, false, a => a.ParentAccount); var dtos = _mapper.Map>(accounts.OrderBy(a => a.AccountNumber).ToList()); @@ -134,7 +135,7 @@ public class AccountsController : Controller var currentUser = await _userManager.GetUserAsync(User); // Check for duplicate account number - var existing = await _unitOfWork.Accounts.FindAsync(a => a.AccountNumber == dto.AccountNumber); + var existing = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == currentUser!.CompanyId && a.AccountNumber == dto.AccountNumber); if (existing.Any()) { ModelState.AddModelError(nameof(dto.AccountNumber), "An account with this number already exists."); @@ -213,7 +214,7 @@ public class AccountsController : Controller // Check duplicate number (excluding self) var existing = await _unitOfWork.Accounts.FindAsync( - a => a.AccountNumber == dto.AccountNumber && a.Id != id); + a => a.CompanyId == account.CompanyId && a.AccountNumber == dto.AccountNumber && a.Id != id); if (existing.Any()) { ModelState.AddModelError(nameof(dto.AccountNumber), "An account with this number already exists."); @@ -472,7 +473,7 @@ public class AccountsController : Controller } // Load all active accounts with balances - var accounts = (await _unitOfWork.Accounts.FindAsync(a => a.IsActive)).ToList(); + var accounts = (await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && a.IsActive)).ToList(); var revenueAccounts = accounts.Where(a => a.AccountType == AccountType.Revenue).ToList(); var expenseAccounts = accounts.Where(a => @@ -616,7 +617,8 @@ public class AccountsController : Controller /// private async Task PopulateDropdownsAsync(int? excludeId = null) { - var allAccounts = await _unitOfWork.Accounts.FindAsync(a => excludeId == null || a.Id != excludeId.Value); + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; + var allAccounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && (excludeId == null || a.Id != excludeId.Value)); ViewBag.ParentAccounts = allAccounts .OrderBy(a => a.AccountNumber) diff --git a/src/PowderCoating.Web/Controllers/BankReconciliationsController.cs b/src/PowderCoating.Web/Controllers/BankReconciliationsController.cs index f9324f4..9a8fe73 100644 --- a/src/PowderCoating.Web/Controllers/BankReconciliationsController.cs +++ b/src/PowderCoating.Web/Controllers/BankReconciliationsController.cs @@ -365,8 +365,9 @@ public class BankReconciliationsController : Controller private async Task PopulateAccountDropdownAsync() { + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; var accounts = await _unitOfWork.Accounts.FindAsync( - a => a.IsActive + a => a.CompanyId == companyId && a.IsActive && (a.AccountSubType == AccountSubType.Checking || a.AccountSubType == AccountSubType.Savings || a.AccountSubType == AccountSubType.Cash)); diff --git a/src/PowderCoating.Web/Controllers/BillsController.cs b/src/PowderCoating.Web/Controllers/BillsController.cs index a276713..4121d29 100644 --- a/src/PowderCoating.Web/Controllers/BillsController.cs +++ b/src/PowderCoating.Web/Controllers/BillsController.cs @@ -453,10 +453,11 @@ public class BillsController : Controller // Payment form defaults var bankAccounts = (await _unitOfWork.Accounts.FindAsync( - a => a.AccountSubType == AccountSubType.Cash || + a => a.CompanyId == bill.CompanyId && + (a.AccountSubType == AccountSubType.Cash || a.AccountSubType == AccountSubType.Checking || a.AccountSubType == AccountSubType.Savings || - a.AccountSubType == AccountSubType.CreditCard)) + a.AccountSubType == AccountSubType.CreditCard))) .OrderBy(a => a.AccountNumber) .ToList(); @@ -1077,7 +1078,8 @@ public class BillsController : Controller return Json(new { success = false, error = "File must be under 10 MB." }); // Load expense accounts for matching - var allAccounts = await _unitOfWork.Accounts.FindAsync(a => a.IsActive); + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; + var allAccounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && a.IsActive); var expenseAccounts = allAccounts .Where(a => a.AccountType == AccountType.Expense || a.AccountType == AccountType.CostOfGoods || @@ -1097,7 +1099,6 @@ public class BillsController : Controller var imageBytes = ms.ToArray(); var result = await _accountingAi.ScanReceiptAsync(imageBytes, receiptImage.ContentType, expenseAccounts); - var companyId = int.TryParse(User.FindFirst("CompanyId")?.Value, out var cid) ? cid : 0; var userId = User.FindFirst(System.Security.Claims.ClaimTypes.NameIdentifier)?.Value ?? ""; await _usageLogger.LogAsync(companyId, userId, AppConstants.AiFeatures.ReceiptScan, inputLength: (int)receiptImage.Length); return Json(result); @@ -1124,7 +1125,8 @@ public class BillsController : Controller // Load expense accounts if not supplied if (!request.AvailableAccounts.Any()) { - var allAccounts = await _unitOfWork.Accounts.FindAsync(a => a.IsActive); + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; + var allAccounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && a.IsActive); request.AvailableAccounts = allAccounts .Where(a => a.AccountType == AccountType.Expense || a.AccountType == AccountType.CostOfGoods || diff --git a/src/PowderCoating.Web/Controllers/BudgetsController.cs b/src/PowderCoating.Web/Controllers/BudgetsController.cs index 45ca7c0..9f68b52 100644 --- a/src/PowderCoating.Web/Controllers/BudgetsController.cs +++ b/src/PowderCoating.Web/Controllers/BudgetsController.cs @@ -246,8 +246,9 @@ public class BudgetsController : Controller private async Task> GetBudgetableAccountsAsync() { + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; var accounts = await _unitOfWork.Accounts.FindAsync( - a => a.IsActive && (a.AccountType == AccountType.Revenue || a.AccountType == AccountType.Expense)); + a => a.CompanyId == companyId && a.IsActive && (a.AccountType == AccountType.Revenue || a.AccountType == AccountType.Expense)); return accounts.OrderBy(a => a.AccountNumber).ToList(); } diff --git a/src/PowderCoating.Web/Controllers/CatalogItemsController.cs b/src/PowderCoating.Web/Controllers/CatalogItemsController.cs index 151af7b..e8d4c39 100644 --- a/src/PowderCoating.Web/Controllers/CatalogItemsController.cs +++ b/src/PowderCoating.Web/Controllers/CatalogItemsController.cs @@ -670,7 +670,8 @@ namespace PowderCoating.Web.Controllers return; } - var accounts = await _unitOfWork.Accounts.FindAsync(a => a.IsActive); + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; + var accounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && a.IsActive); var revenueAccounts = accounts .Where(a => a.AccountType == PowderCoating.Core.Enums.AccountType.Revenue) diff --git a/src/PowderCoating.Web/Controllers/ExpensesController.cs b/src/PowderCoating.Web/Controllers/ExpensesController.cs index 2499f51..d6674c5 100644 --- a/src/PowderCoating.Web/Controllers/ExpensesController.cs +++ b/src/PowderCoating.Web/Controllers/ExpensesController.cs @@ -105,8 +105,9 @@ public class ExpensesController : Controller ViewBag.To = to?.ToString("yyyy-MM-dd"); ViewBag.TotalAmount = dtos.Sum(e => e.Amount); + var legacyUser = await _userManager.GetUserAsync(User); var expenseAccounts = (await _unitOfWork.Accounts.FindAsync( - a => a.IsActive && + a => a.CompanyId == legacyUser!.CompanyId && a.IsActive && (a.AccountType == AccountType.Expense || a.AccountType == AccountType.CostOfGoods))) .OrderBy(a => a.AccountNumber) .ToList(); @@ -479,7 +480,8 @@ public class ExpensesController : Controller if (!request.AvailableAccounts.Any()) { - var allAccounts = await _unitOfWork.Accounts.FindAsync(a => a.IsActive); + var currentUser = await _userManager.GetUserAsync(User); + var allAccounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == currentUser!.CompanyId && a.IsActive); request.AvailableAccounts = allAccounts .Where(a => a.AccountType == AccountType.Expense || a.AccountType == AccountType.CostOfGoods) diff --git a/src/PowderCoating.Web/Controllers/FixedAssetsController.cs b/src/PowderCoating.Web/Controllers/FixedAssetsController.cs index 9eae731..9318460 100644 --- a/src/PowderCoating.Web/Controllers/FixedAssetsController.cs +++ b/src/PowderCoating.Web/Controllers/FixedAssetsController.cs @@ -313,7 +313,8 @@ public class FixedAssetsController : Controller private async Task PopulateAccountsAsync() { - var accounts = await _unitOfWork.Accounts.FindAsync(a => a.IsActive); + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; + var accounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && a.IsActive); var list = accounts.OrderBy(a => a.AccountNumber).ThenBy(a => a.Name).ToList(); ViewBag.AssetAccounts = list diff --git a/src/PowderCoating.Web/Controllers/InventoryController.cs b/src/PowderCoating.Web/Controllers/InventoryController.cs index 10255f6..697a058 100644 --- a/src/PowderCoating.Web/Controllers/InventoryController.cs +++ b/src/PowderCoating.Web/Controllers/InventoryController.cs @@ -1640,7 +1640,7 @@ public class InventoryController : Controller new SelectListItem { Value = "rolls", Text = "Rolls" } }; - var accounts = await _unitOfWork.Accounts.FindAsync(a => a.IsActive); + var accounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && a.IsActive); ViewBag.InventoryAccounts = accounts .Where(a => a.AccountType == AccountType.Asset diff --git a/src/PowderCoating.Web/Controllers/JournalEntriesController.cs b/src/PowderCoating.Web/Controllers/JournalEntriesController.cs index 5686756..47eb0cc 100644 --- a/src/PowderCoating.Web/Controllers/JournalEntriesController.cs +++ b/src/PowderCoating.Web/Controllers/JournalEntriesController.cs @@ -118,7 +118,7 @@ public class JournalEntriesController : Controller // Load account names for lines var accountIds = je.Lines.Select(l => l.AccountId).Distinct().ToList(); - var accounts = await _unitOfWork.Accounts.FindAsync(a => accountIds.Contains(a.Id)); + var accounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == je.CompanyId && accountIds.Contains(a.Id)); ViewBag.AccountMap = accounts.ToDictionary(a => a.Id, a => $"{a.AccountNumber} – {a.Name}"); // Reversal metadata @@ -474,7 +474,8 @@ public class JournalEntriesController : Controller private async Task PopulateAccountDropdownAsync() { - var accounts = await _unitOfWork.Accounts.FindAsync(a => a.IsActive); + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; + var accounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && a.IsActive); ViewBag.AccountSelectList = accounts .OrderBy(a => a.AccountNumber) .Select(a => new SelectListItem diff --git a/src/PowderCoating.Web/Controllers/ReportsController.cs b/src/PowderCoating.Web/Controllers/ReportsController.cs index 485578a..2a52736 100644 --- a/src/PowderCoating.Web/Controllers/ReportsController.cs +++ b/src/PowderCoating.Web/Controllers/ReportsController.cs @@ -2558,7 +2558,7 @@ public class ReportsController : Controller // Load account metadata for budget lines var accountIds = budget.Lines.Select(l => l.AccountId).Distinct().ToList(); - var accounts = (await _unitOfWork.Accounts.FindAsync(a => accountIds.Contains(a.Id))) + var accounts = (await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == budget.CompanyId && accountIds.Contains(a.Id))) .ToDictionary(a => a.Id); var rows = new List(); diff --git a/src/PowderCoating.Web/Controllers/VendorCreditsController.cs b/src/PowderCoating.Web/Controllers/VendorCreditsController.cs index 97613db..dda5e1e 100644 --- a/src/PowderCoating.Web/Controllers/VendorCreditsController.cs +++ b/src/PowderCoating.Web/Controllers/VendorCreditsController.cs @@ -132,7 +132,7 @@ public class VendorCreditsController : Controller .Select(l => l.AccountId!.Value) .Distinct() .ToList(); - var accounts = await _unitOfWork.Accounts.FindAsync(a => accountIds.Contains(a.Id)); + var accounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == vc.CompanyId && accountIds.Contains(a.Id)); ViewBag.AccountMap = accounts.ToDictionary(a => a.Id, a => $"{a.AccountNumber} – {a.Name}"); // Load bills referenced by applications @@ -357,8 +357,9 @@ public class VendorCreditsController : Controller private async Task PopulateDropdownsAsync() { + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; var vendors = await _unitOfWork.Vendors.FindAsync(v => v.IsActive); - var accounts = await _unitOfWork.Accounts.FindAsync(a => a.IsActive); + var accounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && a.IsActive); ViewBag.VendorList = vendors .OrderBy(v => v.CompanyName) diff --git a/src/PowderCoating.Web/Controllers/VendorsController.cs b/src/PowderCoating.Web/Controllers/VendorsController.cs index f1f8d54..e93a652 100644 --- a/src/PowderCoating.Web/Controllers/VendorsController.cs +++ b/src/PowderCoating.Web/Controllers/VendorsController.cs @@ -463,8 +463,9 @@ public class VendorsController : Controller private async Task PopulateExpenseAccountsAsync() { + var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; var accounts = (await _unitOfWork.Accounts.FindAsync( - a => a.IsActive && (a.AccountType == AccountType.Expense || + a => a.CompanyId == companyId && a.IsActive && (a.AccountType == AccountType.Expense || a.AccountType == AccountType.CostOfGoods || a.AccountType == AccountType.Asset))) .OrderBy(a => a.AccountNumber)