From 091d503627d57b162479aac4febc7ebe6712e3ab Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 19 May 2026 14:05:21 -0400 Subject: [PATCH 1/9] feat(dirt): enforce validated state in download and upload endpoints Adds two feature-flag-gated guards to the self-hosted report-file endpoints under pm-31920-access-intelligence-azure-file-storage, implementing the Download and Upload endpoint sections of the PM-34909 acceptance criteria. The Get-latest read-path work from the same AC is deferred to the follow-up commits on this branch. Download guard -------------- In DownloadReportFileAsync, after the existing fileData == null check, throw NotFoundException when the V2 flag is ON and the ReportFile's Validated flag is false. The endpoint is reachable by report ID, independent of the filtered GetLatest read path introduced in later commits, so this guard is load-bearing: a caller cannot bypass it by hitting download directly. Upload guard ------------ In UploadReportFileAsync, extend the existing nullable / ID- mismatch guard with || fileData.Validated. Once the size- validation pass flips Validated to true, the report's file slot is locked. A second upload attempt to the same report ID throws NotFoundException, preventing overwrite of a confirmed file. Mirrors RenewFileUploadUrlAsync, which already refuses to mint a fresh upload URL for a validated file. Note: the guard checks the per-report Validated flag, not file content. Uploads to a distinct report ID are unaffected. Flag-off behavior ----------------- When pm-31920-access-intelligence-azure-file-storage is OFF, both endpoints behave exactly as on main: the download endpoint serves whatever file is on the row, and the upload endpoint accepts the upload regardless of the Validated flag. Rollout is revert-safe. Test coverage ------------- Four xUnit tests added to OrganizationReportsControllerTests, mapping 1:1 to the AC bullets: - DownloadReportFileAsync_FlagOn_UnvalidatedFile_ThrowsNotFoundException - DownloadReportFileAsync_FlagOn_ValidatedFile_ReturnsFile - DownloadReportFileAsync_FlagOff_UnvalidatedFile_ReturnsFile - UploadReportFileAsync_AlreadyValidated_ThrowsNotFoundException All four use NSubstitute + SutProvider per the existing patterns in the test file. Microsoft.AspNetCore.Http is added as a using directive for DefaultHttpContext, which the upload test needs to set ContentType = "multipart/form-data" on the request. References ---------- PR review thread on whether the download guard is needed (this commit keeps it; reasoning above): https://github.com/bitwarden/server/pull/7465#discussion_r3236449979 Feature flag: pm-31920-access-intelligence-azure-file-storage Prerequisite (already merged): #7228 [PM-34909] --- .../OrganizationReportsController.cs | 7 +- .../OrganizationReportsControllerTests.cs | 120 ++++++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/src/Api/Dirt/Controllers/OrganizationReportsController.cs b/src/Api/Dirt/Controllers/OrganizationReportsController.cs index e13b4c32d20c..c32be66703ff 100644 --- a/src/Api/Dirt/Controllers/OrganizationReportsController.cs +++ b/src/Api/Dirt/Controllers/OrganizationReportsController.cs @@ -393,7 +393,7 @@ public async Task UploadReportFileAsync(Guid organizationId, Guid reportId, [Fro } var fileData = report.GetReportFile(); - if (fileData == null || fileData.Id != reportFileId) + if (fileData == null || fileData.Id != reportFileId || fileData.Validated) { throw new NotFoundException(); } @@ -449,6 +449,11 @@ public async Task DownloadReportFileAsync(Guid organizationId, Gu throw new NotFoundException(); } + if (_featureService.IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2) && !fileData.Validated) + { + throw new NotFoundException(); + } + var stream = await _storageService.GetReportReadStreamAsync(report, fileData); if (stream == null) { diff --git a/test/Api.Test/Dirt/OrganizationReportsControllerTests.cs b/test/Api.Test/Dirt/OrganizationReportsControllerTests.cs index 0b6c5b828fe3..3286a1a0f26f 100644 --- a/test/Api.Test/Dirt/OrganizationReportsControllerTests.cs +++ b/test/Api.Test/Dirt/OrganizationReportsControllerTests.cs @@ -15,6 +15,7 @@ using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using NSubstitute; @@ -1401,6 +1402,125 @@ await sutProvider.GetDependency() .CreateAsync(Arg.Any()); } + // DownloadReportFileAsync - validated file enforcement + + [Theory, BitAutoData] + public async Task DownloadReportFileAsync_FlagOn_UnvalidatedFile_ThrowsNotFoundException( + SutProvider sutProvider, + Guid orgId, + OrganizationReport report) + { + // Arrange + report.OrganizationId = orgId; + var fileData = new ReportFile { Id = "file-id", FileName = "report.json", Size = 1024, Validated = false }; + report.SetReportFile(fileData); + + SetupV2Authorization(sutProvider, orgId); + + sutProvider.GetDependency() + .GetOrganizationReportAsync(report.Id) + .Returns(report); + + // Act & Assert + await Assert.ThrowsAsync(() => + sutProvider.Sut.DownloadReportFileAsync(orgId, report.Id)); + + await sutProvider.GetDependency() + .DidNotReceive() + .GetReportReadStreamAsync(Arg.Any(), Arg.Any()); + } + + [Theory, BitAutoData] + public async Task DownloadReportFileAsync_FlagOn_ValidatedFile_ReturnsFile( + SutProvider sutProvider, + Guid orgId, + OrganizationReport report) + { + // Arrange + report.OrganizationId = orgId; + var fileData = new ReportFile { Id = "file-id", FileName = "report.json", Size = 1024, Validated = true }; + report.SetReportFile(fileData); + + SetupV2Authorization(sutProvider, orgId); + + sutProvider.GetDependency() + .GetOrganizationReportAsync(report.Id) + .Returns(report); + + var stream = new MemoryStream(new byte[] { 1, 2, 3 }); + sutProvider.GetDependency() + .GetReportReadStreamAsync(report, Arg.Any()) + .Returns(stream); + + // Act + var result = await sutProvider.Sut.DownloadReportFileAsync(orgId, report.Id); + + // Assert + var fileResult = Assert.IsType(result); + Assert.Equal("application/octet-stream", fileResult.ContentType); + } + + [Theory, BitAutoData] + public async Task DownloadReportFileAsync_FlagOff_UnvalidatedFile_ReturnsFile( + SutProvider sutProvider, + Guid orgId, + OrganizationReport report) + { + // Arrange + report.OrganizationId = orgId; + var fileData = new ReportFile { Id = "file-id", FileName = "report.json", Size = 1024, Validated = false }; + report.SetReportFile(fileData); + + SetupAuthorization(sutProvider, orgId); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2) + .Returns(false); + + sutProvider.GetDependency() + .GetOrganizationReportAsync(report.Id) + .Returns(report); + + var stream = new MemoryStream(new byte[] { 1, 2, 3 }); + sutProvider.GetDependency() + .GetReportReadStreamAsync(report, Arg.Any()) + .Returns(stream); + + // Act + var result = await sutProvider.Sut.DownloadReportFileAsync(orgId, report.Id); + + // Assert + Assert.IsType(result); + } + + // UploadReportFileAsync - re-upload guard + + [Theory, BitAutoData] + public async Task UploadReportFileAsync_AlreadyValidated_ThrowsNotFoundException( + SutProvider sutProvider, + Guid orgId, + OrganizationReport report) + { + // Arrange + report.OrganizationId = orgId; + var fileData = new ReportFile { Id = "file-id", FileName = "report.json", Size = 1024, Validated = true }; + report.SetReportFile(fileData); + + SetupV2Authorization(sutProvider, orgId); + + sutProvider.GetDependency() + .GetOrganizationReportAsync(report.Id) + .Returns(report); + + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = "multipart/form-data"; + sutProvider.Sut.ControllerContext = new ControllerContext { HttpContext = httpContext }; + + // Act & Assert + await Assert.ThrowsAsync(() => + sutProvider.Sut.UploadReportFileAsync(orgId, report.Id, "file-id")); + } + // Helper methods for authorization mocks private static void SetupAuthorization( From d69fab54a12785a7443d9c8504cf15f2b8242304 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 19 May 2026 14:49:56 -0400 Subject: [PATCH 2/9] feat(dirt): add ReadLatest SP and skip empty ReportData in V1 SP Introduces a two-SP design for serving the latest organization report under the V2 file-storage feature flag. Implements the Get-latest endpoint section of the PM-34909 acceptance criteria. The controller dispatch that wires the V2 flag to the new SP is deferred to the next commit on this branch. New SP: OrganizationReport_ReadLatestByOrganizationId ----------------------------------------------------- Always filters by JSON_VALUE([ReportFile], '$.Validated') = 'true'. No parameter switch. Single execution plan. Used by the V2 read path (once commit 3 wires the controller dispatch to call it). This addresses mkincaid-bw's PR comment on the SP conditional-WHERE anti-pattern by structure: each SP has one shape, one execution plan. mkincaid-bw's own comment predicted this resolution. V1 SP: OrganizationReport_GetLatestByOrganizationId --------------------------------------------------- Adds [ReportData] <> '' to the existing WHERE clause. The OrganizationReport.ReportData column is NVARCHAR(MAX) NOT NULL with entity default = string.Empty; V2 CreateOrganizationReportCommand doesn't set ReportData, so V2 file-storage rows are written with ReportData = '' (empty string, not NULL). The new predicate skips those rows so the V1 read path can never return an empty-payload row to a V1 caller after a flag-flip from V2 ON to V2 OFF. Note: the Jira AC text says "WHERE ReportData IS NOT NULL" but that's a no-op against a NOT NULL column. Shipping predicate is <> ''. AC text will be updated post-merge. DbUp migrations --------------- Twin scripts mirror each SP body: - util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql - util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql Both use CREATE OR ALTER PROCEDURE for idempotency. Rename the date prefix at push time if the merge date shifts. Repositories ------------ Both repository implementations get a new ReadLatestByOrganizationIdAsync method calling the new SP / equivalent LINQ query. The EF Core implementation also gets the empty-ReportData filter on the existing GetLatestByOrganizationIdAsync method so Postgres / MySQL / SQLite deployments share the SQL Server guarantee. This addresses Banrion's PR comment on the EF Core conditional Where by structure: each method is a single top-to-bottom query, no shared IQueryable with conditional chaining. - src/Infrastructure.Dapper/Dirt/OrganizationReportRepository.cs - src/Infrastructure.EntityFramework/Dirt/Repositories/OrganizationReportRepository.cs - src/Core/Dirt/Repositories/IOrganizationReportRepository.cs What this commit does NOT do ---------------------------- - Does not change the controller dispatch. GetLatestOrganizationReportAsync still routes through the single existing query method on this branch; the V2 ternary lands in commit 3 along with the new query interface member. - Does not change upload or download endpoints (commit 1 owned those). - Does not add tests. Controller dispatch tests and query unit tests land in commit 3. Per the locked test-depth decision, no repo integration tests are added; the flag-flip regression is covered by the manual recipe in pr-comments-7465's parent directory at local-testing-recipe.md section 14. References ---------- PR review threads addressed by this commit: - SP conditional WHERE anti-pattern (mkincaid-bw): https://github.com/bitwarden/server/pull/7465#discussion_r3238180218 - EF Core conditional Where readability (Banrion): https://github.com/bitwarden/server/pull/7465#discussion_r3242001772 Scope-change rationale (Leslie, 2026-05-14): the flag-flip regression that motivated the two-SP design rather than a single conditional SP. Feature flag: pm-31920-access-intelligence-azure-file-storage Prerequisite (already merged): #7228 [PM-34909] --- .../IOrganizationReportRepository.cs | 1 + .../Dirt/OrganizationReportRepository.cs | 13 +++++++++++ .../OrganizationReportRepository.cs | 22 ++++++++++++++++++- ...zationReport_GetLatestByOrganizationId.sql | 1 + ...ationReport_ReadLatestByOrganizationId.sql | 13 +++++++++++ ...9_00_AddOrganizationReportReadLatestSp.sql | 14 ++++++++++++ ...tionReportGetLatestSkipEmptyReportData.sql | 14 ++++++++++++ 7 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql create mode 100644 util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql create mode 100644 util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql diff --git a/src/Core/Dirt/Repositories/IOrganizationReportRepository.cs b/src/Core/Dirt/Repositories/IOrganizationReportRepository.cs index fb85361a63ff..85baa07ed36f 100644 --- a/src/Core/Dirt/Repositories/IOrganizationReportRepository.cs +++ b/src/Core/Dirt/Repositories/IOrganizationReportRepository.cs @@ -9,6 +9,7 @@ public interface IOrganizationReportRepository : IRepository GetLatestByOrganizationIdAsync(Guid organizationId); + Task ReadLatestByOrganizationIdAsync(Guid organizationId); // SummaryData methods Task> GetSummaryDataByDateRangeAsync(Guid organizationId, DateTime startDate, DateTime endDate); diff --git a/src/Infrastructure.Dapper/Dirt/OrganizationReportRepository.cs b/src/Infrastructure.Dapper/Dirt/OrganizationReportRepository.cs index 2a6ee83985f7..066d0ee71db3 100644 --- a/src/Infrastructure.Dapper/Dirt/OrganizationReportRepository.cs +++ b/src/Infrastructure.Dapper/Dirt/OrganizationReportRepository.cs @@ -38,6 +38,19 @@ public async Task GetLatestByOrganizationIdAsync(Guid organi } } + public async Task ReadLatestByOrganizationIdAsync(Guid organizationId) + { + using (var connection = new SqlConnection(ReadOnlyConnectionString)) + { + var result = await connection.QuerySingleOrDefaultAsync( + $"[{Schema}].[OrganizationReport_ReadLatestByOrganizationId]", + new { OrganizationId = organizationId }, + commandType: CommandType.StoredProcedure); + + return result; + } + } + public async Task UpdateSummaryDataAsync(Guid organizationId, Guid reportId, string summaryData) { using (var connection = new SqlConnection(ConnectionString)) diff --git a/src/Infrastructure.EntityFramework/Dirt/Repositories/OrganizationReportRepository.cs b/src/Infrastructure.EntityFramework/Dirt/Repositories/OrganizationReportRepository.cs index 6691079179ec..fdb9df5113a2 100644 --- a/src/Infrastructure.EntityFramework/Dirt/Repositories/OrganizationReportRepository.cs +++ b/src/Infrastructure.EntityFramework/Dirt/Repositories/OrganizationReportRepository.cs @@ -27,7 +27,27 @@ public async Task GetLatestByOrganizationIdAsync(Guid organi { var dbContext = GetDatabaseContext(scope); var result = await dbContext.OrganizationReports - .Where(p => p.OrganizationId == organizationId) + .Where(p => p.OrganizationId == organizationId + && p.ReportData != string.Empty) + .OrderByDescending(p => p.RevisionDate) + .Take(1) + .FirstOrDefaultAsync(); + + if (result == null) return default; + + return Mapper.Map(result); + } + } + + public async Task ReadLatestByOrganizationIdAsync(Guid organizationId) + { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + var result = await dbContext.OrganizationReports + .Where(p => p.OrganizationId == organizationId + && p.ReportFile != null + && p.ReportFile.Contains("\"Validated\":true")) .OrderByDescending(p => p.RevisionDate) .Take(1) .FirstOrDefaultAsync(); diff --git a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql index fca8788ce6f7..10926f3c6443 100644 --- a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql +++ b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql @@ -8,5 +8,6 @@ BEGIN * FROM [dbo].[OrganizationReportView] WHERE [OrganizationId] = @OrganizationId + AND [ReportData] <> '' ORDER BY [RevisionDate] DESC END diff --git a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql new file mode 100644 index 000000000000..717bb9b23e57 --- /dev/null +++ b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql @@ -0,0 +1,13 @@ +CREATE PROCEDURE [dbo].[OrganizationReport_ReadLatestByOrganizationId] + @OrganizationId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT TOP 1 + * + FROM [dbo].[OrganizationReportView] + WHERE [OrganizationId] = @OrganizationId + AND JSON_VALUE([ReportFile], '$.Validated') = 'true' + ORDER BY [RevisionDate] DESC +END diff --git a/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql b/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql new file mode 100644 index 000000000000..9291ff0cc3bd --- /dev/null +++ b/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql @@ -0,0 +1,14 @@ +CREATE OR ALTER PROCEDURE [dbo].[OrganizationReport_ReadLatestByOrganizationId] + @OrganizationId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT TOP 1 + * + FROM [dbo].[OrganizationReportView] + WHERE [OrganizationId] = @OrganizationId + AND JSON_VALUE([ReportFile], '$.Validated') = 'true' + ORDER BY [RevisionDate] DESC +END +GO diff --git a/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql b/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql new file mode 100644 index 000000000000..747e2a554b72 --- /dev/null +++ b/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql @@ -0,0 +1,14 @@ +CREATE OR ALTER PROCEDURE [dbo].[OrganizationReport_GetLatestByOrganizationId] + @OrganizationId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT TOP 1 + * + FROM [dbo].[OrganizationReportView] + WHERE [OrganizationId] = @OrganizationId + AND [ReportData] <> '' + ORDER BY [RevisionDate] DESC +END +GO From c50e572d82a1cf29a06f63095c88ddcf9d378bfa Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 19 May 2026 15:35:20 -0400 Subject: [PATCH 3/9] feat(dirt): dispatch GetLatest endpoint by V2 feature flag Wires the controller to the two-SP foundation introduced in the previous commit. When pm-31920-access-intelligence-azure-file-storage is ON, GetLatestOrganizationReportAsync dispatches to a new ReadLatestOrganizationReportAsync query method that calls the validated-file SP. When the flag is OFF, the existing GetLatestOrganizationReportAsync query method continues to call the V1 SP, which (per the previous commit) now skips empty-ReportData rows. Completes the Get-latest endpoint section of the PM-34909 acceptance criteria. Three controller deltas ----------------------- 1. Adds an isAccessIntelligenceV2 local + ternary dispatch. The local is named after the gate, not after a parameter shape, addressing Banrion's review comment on filterByValidated naming: https://github.com/bitwarden/server/pull/7465#discussion_r3236438521 2. Replaces the inner `if (fileData is { Validated: true })` with `if (fileData != null)`. The enclosing block is already gated by `if (isAccessIntelligenceV2)`, so it only runs on the V2 path, and the V2 path always reaches the controller via ReadLatest's SP which has already filtered to Validated = true. Addresses Banrion's redundant-Validated-check comment: https://github.com/bitwarden/server/pull/7465#discussion_r3236445435 3. Removes the controller-level `if (latestReport == null) throw new NotFoundException();` after the query call. The query layer owns the null-to-NotFoundException contract for both methods; double-checking obscures ownership. Addresses Banrion's null- check comment: https://github.com/bitwarden/server/pull/7465#discussion_r3236423489 Query layer ----------- IGetOrganizationReportQuery gains a ReadLatestOrganizationReportAsync method declaration. GetOrganizationReportQuery provides the implementation: logs the read with the BypassFiltersEventId marker, calls the new repo method, throws NotFoundException on null. Mirrors the existing GetLatestOrganizationReportAsync exactly except for the repo method called and the log/exception message wording. Test coverage ------------- OrganizationReportsControllerTests: - Existing _WithValidatedFile_ReturnsOkWithDownloadUrl: mock swapped from GetLatestOrganizationReportAsync to ReadLatestOrganizationReportAsync because the test enables V2 and the controller now dispatches there. - NEW _FlagOn_CallsReadLatest: V2 ON, asserts ReadLatest was called once and GetLatest was not called. - NEW _FlagOff_CallsGetLatest: V2 OFF, mirror of the above. GetOrganizationReportQueryTests (NEW file): - ReadLatestOrganizationReportAsync_ReturnsRepoResult - ReadLatestOrganizationReportAsync_NullResult_ThrowsNotFoundException - GetLatestOrganizationReportAsync_ReturnsRepoResult - GetLatestOrganizationReportAsync_NullResult_ThrowsNotFoundException The V1 null-throw test is what backs the response to Banrion's null- check comment: "the query layer owns this contract" is now a provable claim with a test that fails if the throw is removed. No repo integration tests are added per the locked test-depth decision; the flag-flip regression is covered by the manual recipe in local-testing-recipe.md section 14. What this commit does NOT do ---------------------------- - Does not add or modify SPs, migrations, or repos (previous commit owned those). - Does not touch the upload or download endpoints (first commit on this branch owned those). - Does not touch any other controller method. References ---------- Feature flag: pm-31920-access-intelligence-azure-file-storage Prerequisite (already merged): #7228 After this commit the branch is feature-complete: git log --oneline origin/main..HEAD d69fab54a feat(dirt): add ReadLatest SP and skip empty ReportData in V1 SP 091d50362 feat(dirt): enforce validated state in download and upload endpoints The new commit will be the third entry above it. [PM-34909] --- .../OrganizationReportsController.cs | 13 ++-- .../GetOrganizationReportQuery.cs | 15 ++++ .../Interfaces/IGetOrganizationReportQuery.cs | 1 + .../OrganizationReportsControllerTests.cs | 73 ++++++++++++++++++- .../GetOrganizationReportQueryTests.cs | 70 ++++++++++++++++++ 5 files changed, 164 insertions(+), 8 deletions(-) create mode 100644 test/Core.Test/Dirt/ReportFeatures/GetOrganizationReportQueryTests.cs diff --git a/src/Api/Dirt/Controllers/OrganizationReportsController.cs b/src/Api/Dirt/Controllers/OrganizationReportsController.cs index c32be66703ff..41794eb6df31 100644 --- a/src/Api/Dirt/Controllers/OrganizationReportsController.cs +++ b/src/Api/Dirt/Controllers/OrganizationReportsController.cs @@ -141,19 +141,18 @@ public async Task GetLatestOrganizationReportAsync(Guid organizat await AuthorizeAsync(organizationId); - var latestReport = await _getOrganizationReportQuery.GetLatestOrganizationReportAsync(organizationId); + var isAccessIntelligenceV2 = _featureService.IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2); - if (latestReport == null) - { - throw new NotFoundException(); - } + var latestReport = isAccessIntelligenceV2 + ? await _getOrganizationReportQuery.ReadLatestOrganizationReportAsync(organizationId) + : await _getOrganizationReportQuery.GetLatestOrganizationReportAsync(organizationId); var response = new OrganizationReportResponseModel(latestReport); - if (_featureService.IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2)) + if (isAccessIntelligenceV2) { var fileData = latestReport.GetReportFile(); - if (fileData is { Validated: true }) + if (fileData != null) { response.ReportFileDownloadUrl = await _storageService.GetReportDataDownloadUrlAsync(latestReport, fileData); response.FileUploadType = _storageService.FileUploadType; diff --git a/src/Core/Dirt/Reports/ReportFeatures/GetOrganizationReportQuery.cs b/src/Core/Dirt/Reports/ReportFeatures/GetOrganizationReportQuery.cs index 7928b2956f87..f4563db3bc15 100644 --- a/src/Core/Dirt/Reports/ReportFeatures/GetOrganizationReportQuery.cs +++ b/src/Core/Dirt/Reports/ReportFeatures/GetOrganizationReportQuery.cs @@ -32,6 +32,21 @@ public async Task GetLatestOrganizationReportAsync(Guid orga return result; } + public async Task ReadLatestOrganizationReportAsync(Guid organizationId) + { + _logger.LogInformation(Constants.BypassFiltersEventId, + "Reading latest validated-file organization report for organization {organizationId}", + organizationId); + var result = await _organizationReportRepo.ReadLatestByOrganizationIdAsync(organizationId); + + if (result == null) + { + throw new NotFoundException($"No validated report found for organization: {organizationId}"); + } + + return result; + } + public async Task GetOrganizationReportAsync(Guid reportId) { _logger.LogInformation(Constants.BypassFiltersEventId, "Fetching organization reports for organization by Id: {reportId}", reportId); diff --git a/src/Core/Dirt/Reports/ReportFeatures/Interfaces/IGetOrganizationReportQuery.cs b/src/Core/Dirt/Reports/ReportFeatures/Interfaces/IGetOrganizationReportQuery.cs index b72fdd25b565..eb7ce6735105 100644 --- a/src/Core/Dirt/Reports/ReportFeatures/Interfaces/IGetOrganizationReportQuery.cs +++ b/src/Core/Dirt/Reports/ReportFeatures/Interfaces/IGetOrganizationReportQuery.cs @@ -6,4 +6,5 @@ public interface IGetOrganizationReportQuery { Task GetOrganizationReportAsync(Guid organizationId); Task GetLatestOrganizationReportAsync(Guid organizationId); + Task ReadLatestOrganizationReportAsync(Guid organizationId); } diff --git a/test/Api.Test/Dirt/OrganizationReportsControllerTests.cs b/test/Api.Test/Dirt/OrganizationReportsControllerTests.cs index 3286a1a0f26f..1b0249355898 100644 --- a/test/Api.Test/Dirt/OrganizationReportsControllerTests.cs +++ b/test/Api.Test/Dirt/OrganizationReportsControllerTests.cs @@ -49,7 +49,7 @@ public async Task GetLatestOrganizationReportAsync_WithValidatedFile_ReturnsOkWi .Returns(true); sutProvider.GetDependency() - .GetLatestOrganizationReportAsync(orgId) + .ReadLatestOrganizationReportAsync(orgId) .Returns(expectedReport); sutProvider.GetDependency() @@ -70,6 +70,77 @@ public async Task GetLatestOrganizationReportAsync_WithValidatedFile_ReturnsOkWi Assert.Equal(FileUploadType.Azure, response.FileUploadType); } + [Theory, BitAutoData] + public async Task GetLatestOrganizationReportAsync_FlagOn_CallsReadLatest( + SutProvider sutProvider, + Guid orgId, + OrganizationReport expectedReport) + { + // Arrange + expectedReport.OrganizationId = orgId; + var reportFile = new ReportFile { Id = "file-id", FileName = "report.json", Size = 1024, Validated = true }; + expectedReport.SetReportFile(reportFile); + + SetupAuthorization(sutProvider, orgId); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2) + .Returns(true); + + sutProvider.GetDependency() + .ReadLatestOrganizationReportAsync(orgId) + .Returns(expectedReport); + + sutProvider.GetDependency() + .GetReportDataDownloadUrlAsync(expectedReport, Arg.Any()) + .Returns("https://download-url"); + + // Act + var result = await sutProvider.Sut.GetLatestOrganizationReportAsync(orgId); + + // Assert + Assert.IsType(result); + await sutProvider.GetDependency() + .Received(1) + .ReadLatestOrganizationReportAsync(orgId); + await sutProvider.GetDependency() + .DidNotReceive() + .GetLatestOrganizationReportAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task GetLatestOrganizationReportAsync_FlagOff_CallsGetLatest( + SutProvider sutProvider, + Guid orgId, + OrganizationReport expectedReport) + { + // Arrange + expectedReport.OrganizationId = orgId; + expectedReport.ReportFile = null; + + SetupAuthorization(sutProvider, orgId); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2) + .Returns(false); + + sutProvider.GetDependency() + .GetLatestOrganizationReportAsync(orgId) + .Returns(expectedReport); + + // Act + var result = await sutProvider.Sut.GetLatestOrganizationReportAsync(orgId); + + // Assert + Assert.IsType(result); + await sutProvider.GetDependency() + .Received(1) + .GetLatestOrganizationReportAsync(orgId); + await sutProvider.GetDependency() + .DidNotReceive() + .ReadLatestOrganizationReportAsync(Arg.Any()); + } + [Theory, BitAutoData] public async Task GetLatestOrganizationReportAsync_WithNoFile_ReturnsOkWithNullDownloadUrl( SutProvider sutProvider, diff --git a/test/Core.Test/Dirt/ReportFeatures/GetOrganizationReportQueryTests.cs b/test/Core.Test/Dirt/ReportFeatures/GetOrganizationReportQueryTests.cs new file mode 100644 index 000000000000..8989a5383cd8 --- /dev/null +++ b/test/Core.Test/Dirt/ReportFeatures/GetOrganizationReportQueryTests.cs @@ -0,0 +1,70 @@ +using Bit.Core.Dirt.Entities; +using Bit.Core.Dirt.Reports.ReportFeatures; +using Bit.Core.Dirt.Repositories; +using Bit.Core.Exceptions; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.Dirt.ReportFeatures; + +[SutProviderCustomize] +public class GetOrganizationReportQueryTests +{ + [Theory, BitAutoData] + public async Task ReadLatestOrganizationReportAsync_ReturnsRepoResult( + SutProvider sutProvider, + Guid orgId, + OrganizationReport expectedReport) + { + sutProvider.GetDependency() + .ReadLatestByOrganizationIdAsync(orgId) + .Returns(expectedReport); + + var result = await sutProvider.Sut.ReadLatestOrganizationReportAsync(orgId); + + Assert.Equal(expectedReport, result); + } + + [Theory, BitAutoData] + public async Task ReadLatestOrganizationReportAsync_NullResult_ThrowsNotFoundException( + SutProvider sutProvider, + Guid orgId) + { + sutProvider.GetDependency() + .ReadLatestByOrganizationIdAsync(orgId) + .Returns((OrganizationReport)null); + + await Assert.ThrowsAsync(() => + sutProvider.Sut.ReadLatestOrganizationReportAsync(orgId)); + } + + [Theory, BitAutoData] + public async Task GetLatestOrganizationReportAsync_ReturnsRepoResult( + SutProvider sutProvider, + Guid orgId, + OrganizationReport expectedReport) + { + sutProvider.GetDependency() + .GetLatestByOrganizationIdAsync(orgId) + .Returns(expectedReport); + + var result = await sutProvider.Sut.GetLatestOrganizationReportAsync(orgId); + + Assert.Equal(expectedReport, result); + } + + [Theory, BitAutoData] + public async Task GetLatestOrganizationReportAsync_NullResult_ThrowsNotFoundException( + SutProvider sutProvider, + Guid orgId) + { + sutProvider.GetDependency() + .GetLatestByOrganizationIdAsync(orgId) + .Returns((OrganizationReport)null); + + await Assert.ThrowsAsync(() => + sutProvider.Sut.GetLatestOrganizationReportAsync(orgId)); + } +} From 8561e0098e702477327facb44f97c8b0c65bd7a9 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 20 May 2026 01:47:52 -0400 Subject: [PATCH 4/9] docs(dirt): document JSON substring coupling on EF ReadLatest filter The EF Core ReadLatestByOrganizationIdAsync method substring-matches "Validated":true against the ReportFile JSON column, while the Dapper/MSSQL counterpart uses JSON_VALUE. The substring approach works because SetReportFile (OrganizationReport.cs) is the sole writer and uses JsonHelpers.IgnoreWritingNull (PascalCase, no whitespace); changing those serializer options would silently return zero rows on non-MSSQL providers without affecting the MSSQL path. Add a brief inline comment near the predicate documenting the coupling and noting the format-agnostic Dapper counterpart. No functional change. --- .../Dirt/Repositories/OrganizationReportRepository.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Infrastructure.EntityFramework/Dirt/Repositories/OrganizationReportRepository.cs b/src/Infrastructure.EntityFramework/Dirt/Repositories/OrganizationReportRepository.cs index fdb9df5113a2..248ee2515c8c 100644 --- a/src/Infrastructure.EntityFramework/Dirt/Repositories/OrganizationReportRepository.cs +++ b/src/Infrastructure.EntityFramework/Dirt/Repositories/OrganizationReportRepository.cs @@ -44,6 +44,10 @@ public async Task ReadLatestByOrganizationIdAsync(Guid organ using (var scope = ServiceScopeFactory.CreateScope()) { var dbContext = GetDatabaseContext(scope); + // Substring match is coupled to SetReportFile (OrganizationReport.cs) writing via + // JsonHelpers.IgnoreWritingNull (PascalCase, no whitespace). The Dapper/MSSQL path + // uses JSON_VALUE which is format-agnostic; changing the serializer options would + // silently return zero rows here without affecting the MSSQL path. var result = await dbContext.OrganizationReports .Where(p => p.OrganizationId == organizationId && p.ReportFile != null From d5c0bd9b2d73776175e314711fd98ba22bd867a3 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 21 May 2026 10:28:42 -0400 Subject: [PATCH 5/9] style(dirt): reformat OrganizationReport SPs per SQL keyword style Place each top-level SQL keyword (SELECT, FROM, WHERE, ORDER BY) on its own line with arguments indented, per the Bitwarden SQL style guide (contributing.bitwarden.com/contributing/code-style/sql/). Reformats all four files this PR touches in the SP/migration pair: - src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql - src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql - util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql - util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql No functional change. --- ...OrganizationReport_GetLatestByOrganizationId.sql | 13 ++++++++----- ...rganizationReport_ReadLatestByOrganizationId.sql | 13 ++++++++----- ...6-05-19_00_AddOrganizationReportReadLatestSp.sql | 13 ++++++++----- ...ganizationReportGetLatestSkipEmptyReportData.sql | 13 ++++++++----- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql index 10926f3c6443..46a0842a9bfe 100644 --- a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql +++ b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql @@ -4,10 +4,13 @@ AS BEGIN SET NOCOUNT ON - SELECT TOP 1 - * - FROM [dbo].[OrganizationReportView] - WHERE [OrganizationId] = @OrganizationId + SELECT + TOP 1 * + FROM + [dbo].[OrganizationReportView] + WHERE + [OrganizationId] = @OrganizationId AND [ReportData] <> '' - ORDER BY [RevisionDate] DESC + ORDER BY + [RevisionDate] DESC END diff --git a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql index 717bb9b23e57..56ed462147e6 100644 --- a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql +++ b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql @@ -4,10 +4,13 @@ AS BEGIN SET NOCOUNT ON - SELECT TOP 1 - * - FROM [dbo].[OrganizationReportView] - WHERE [OrganizationId] = @OrganizationId + SELECT + TOP 1 * + FROM + [dbo].[OrganizationReportView] + WHERE + [OrganizationId] = @OrganizationId AND JSON_VALUE([ReportFile], '$.Validated') = 'true' - ORDER BY [RevisionDate] DESC + ORDER BY + [RevisionDate] DESC END diff --git a/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql b/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql index 9291ff0cc3bd..926c23ab507f 100644 --- a/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql +++ b/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql @@ -4,11 +4,14 @@ AS BEGIN SET NOCOUNT ON - SELECT TOP 1 - * - FROM [dbo].[OrganizationReportView] - WHERE [OrganizationId] = @OrganizationId + SELECT + TOP 1 * + FROM + [dbo].[OrganizationReportView] + WHERE + [OrganizationId] = @OrganizationId AND JSON_VALUE([ReportFile], '$.Validated') = 'true' - ORDER BY [RevisionDate] DESC + ORDER BY + [RevisionDate] DESC END GO diff --git a/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql b/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql index 747e2a554b72..1bec17ef23c5 100644 --- a/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql +++ b/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql @@ -4,11 +4,14 @@ AS BEGIN SET NOCOUNT ON - SELECT TOP 1 - * - FROM [dbo].[OrganizationReportView] - WHERE [OrganizationId] = @OrganizationId + SELECT + TOP 1 * + FROM + [dbo].[OrganizationReportView] + WHERE + [OrganizationId] = @OrganizationId AND [ReportData] <> '' - ORDER BY [RevisionDate] DESC + ORDER BY + [RevisionDate] DESC END GO From dcd4ec0a8646b1981bfc957ee0ee46f2f20ea959 Mon Sep 17 00:00:00 2001 From: mkincaid-bw Date: Thu, 21 May 2026 11:16:28 -0700 Subject: [PATCH 6/9] Update src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql --- .../OrganizationReport_GetLatestByOrganizationId.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql index 46a0842a9bfe..41bae326a25d 100644 --- a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql +++ b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_GetLatestByOrganizationId.sql @@ -4,8 +4,8 @@ AS BEGIN SET NOCOUNT ON - SELECT - TOP 1 * + SELECT TOP 1 + * FROM [dbo].[OrganizationReportView] WHERE From d3e0cf9fceb1440344fc8e7adbd8e8ede4b330f1 Mon Sep 17 00:00:00 2001 From: mkincaid-bw Date: Thu, 21 May 2026 11:16:50 -0700 Subject: [PATCH 7/9] Update src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql --- .../OrganizationReport_ReadLatestByOrganizationId.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql index 56ed462147e6..0c2ad783143b 100644 --- a/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql +++ b/src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadLatestByOrganizationId.sql @@ -4,8 +4,8 @@ AS BEGIN SET NOCOUNT ON - SELECT - TOP 1 * + SELECT TOP 1 + * FROM [dbo].[OrganizationReportView] WHERE From c38911aef6690ddfe194b5eea50626335a7fdcf7 Mon Sep 17 00:00:00 2001 From: mkincaid-bw Date: Thu, 21 May 2026 11:19:39 -0700 Subject: [PATCH 8/9] Update util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql --- .../2026-05-19_00_AddOrganizationReportReadLatestSp.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql b/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql index 926c23ab507f..e70550d0743a 100644 --- a/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql +++ b/util/Migrator/DbScripts/2026-05-19_00_AddOrganizationReportReadLatestSp.sql @@ -4,8 +4,8 @@ AS BEGIN SET NOCOUNT ON - SELECT - TOP 1 * + SELECT TOP 1 + * FROM [dbo].[OrganizationReportView] WHERE From 06adae03fd5c7a4f6e9584ba6e85e9637359bc1d Mon Sep 17 00:00:00 2001 From: mkincaid-bw Date: Thu, 21 May 2026 11:19:56 -0700 Subject: [PATCH 9/9] Update util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql --- ...1_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql b/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql index 1bec17ef23c5..e2a531e41a01 100644 --- a/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql +++ b/util/Migrator/DbScripts/2026-05-19_01_UpdateOrganizationReportGetLatestSkipEmptyReportData.sql @@ -4,8 +4,8 @@ AS BEGIN SET NOCOUNT ON - SELECT - TOP 1 * + SELECT TOP 1 + * FROM [dbo].[OrganizationReportView] WHERE