fix(security): additive permitted-sites filter on secured-write query/count (plan R2-07 T5)
This commit is contained in:
+16
-1
@@ -46,6 +46,11 @@ public interface ISecuredWriteRepository
|
|||||||
/// <param name="siteId">Site id filter; <c>null</c> matches every site.</param>
|
/// <param name="siteId">Site id filter; <c>null</c> matches every site.</param>
|
||||||
/// <param name="skip">Number of rows to skip (offset paging).</param>
|
/// <param name="skip">Number of rows to skip (offset paging).</param>
|
||||||
/// <param name="take">Maximum number of rows to return.</param>
|
/// <param name="take">Maximum number of rows to return.</param>
|
||||||
|
/// <param name="permittedSiteIds">When non-null, only rows whose <c>SiteId</c> (site
|
||||||
|
/// identifier) is in the set match; <c>null</c> matches every site. Applied IN ADDITION
|
||||||
|
/// to <paramref name="siteId"/>. Used by the ManagementActor to constrain an unfiltered
|
||||||
|
/// list to a site-scoped caller's permitted sites at the query level (arch-review R2 N3),
|
||||||
|
/// so paging and totals stay correct.</param>
|
||||||
/// <param name="ct">Cancellation token.</param>
|
/// <param name="ct">Cancellation token.</param>
|
||||||
/// <returns>A task that resolves to a page of matching rows, newest submission first.</returns>
|
/// <returns>A task that resolves to a page of matching rows, newest submission first.</returns>
|
||||||
Task<IReadOnlyList<PendingSecuredWrite>> QueryAsync(
|
Task<IReadOnlyList<PendingSecuredWrite>> QueryAsync(
|
||||||
@@ -53,6 +58,7 @@ public interface ISecuredWriteRepository
|
|||||||
string? siteId,
|
string? siteId,
|
||||||
int skip,
|
int skip,
|
||||||
int take,
|
int take,
|
||||||
|
IReadOnlyCollection<string>? permittedSiteIds = null,
|
||||||
CancellationToken ct = default);
|
CancellationToken ct = default);
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
@@ -106,7 +112,16 @@ public interface ISecuredWriteRepository
|
|||||||
/// </summary>
|
/// </summary>
|
||||||
/// <param name="status">Status filter; <c>null</c> matches every status.</param>
|
/// <param name="status">Status filter; <c>null</c> matches every status.</param>
|
||||||
/// <param name="siteId">Site id filter; <c>null</c> matches every site.</param>
|
/// <param name="siteId">Site id filter; <c>null</c> matches every site.</param>
|
||||||
|
/// <param name="permittedSiteIds">When non-null, only rows whose <c>SiteId</c> (site
|
||||||
|
/// identifier) is in the set match; <c>null</c> matches every site. Applied IN ADDITION
|
||||||
|
/// to <paramref name="siteId"/>. Used by the ManagementActor to constrain an unfiltered
|
||||||
|
/// list to a site-scoped caller's permitted sites at the query level (arch-review R2 N3),
|
||||||
|
/// so paging and totals stay correct.</param>
|
||||||
/// <param name="ct">Cancellation token.</param>
|
/// <param name="ct">Cancellation token.</param>
|
||||||
/// <returns>A task that resolves to the count of matching rows.</returns>
|
/// <returns>A task that resolves to the count of matching rows.</returns>
|
||||||
Task<int> CountAsync(string? status, string? siteId, CancellationToken ct = default);
|
Task<int> CountAsync(
|
||||||
|
string? status,
|
||||||
|
string? siteId,
|
||||||
|
IReadOnlyCollection<string>? permittedSiteIds = null,
|
||||||
|
CancellationToken ct = default);
|
||||||
}
|
}
|
||||||
|
|||||||
+16
-1
@@ -57,6 +57,7 @@ public class SecuredWriteRepository : ISecuredWriteRepository
|
|||||||
string? siteId,
|
string? siteId,
|
||||||
int skip,
|
int skip,
|
||||||
int take,
|
int take,
|
||||||
|
IReadOnlyCollection<string>? permittedSiteIds = null,
|
||||||
CancellationToken ct = default)
|
CancellationToken ct = default)
|
||||||
{
|
{
|
||||||
IQueryable<PendingSecuredWrite> query = _context.Set<PendingSecuredWrite>()
|
IQueryable<PendingSecuredWrite> query = _context.Set<PendingSecuredWrite>()
|
||||||
@@ -72,6 +73,11 @@ public class SecuredWriteRepository : ISecuredWriteRepository
|
|||||||
query = query.Where(p => p.SiteId == siteId);
|
query = query.Where(p => p.SiteId == siteId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (permittedSiteIds is not null)
|
||||||
|
{
|
||||||
|
query = query.Where(p => permittedSiteIds.Contains(p.SiteId));
|
||||||
|
}
|
||||||
|
|
||||||
return await query
|
return await query
|
||||||
.OrderByDescending(p => p.SubmittedAtUtc)
|
.OrderByDescending(p => p.SubmittedAtUtc)
|
||||||
.ThenByDescending(p => p.Id)
|
.ThenByDescending(p => p.Id)
|
||||||
@@ -128,7 +134,11 @@ WHERE Id = {id}
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
public async Task<int> CountAsync(string? status, string? siteId, CancellationToken ct = default)
|
public async Task<int> CountAsync(
|
||||||
|
string? status,
|
||||||
|
string? siteId,
|
||||||
|
IReadOnlyCollection<string>? permittedSiteIds = null,
|
||||||
|
CancellationToken ct = default)
|
||||||
{
|
{
|
||||||
IQueryable<PendingSecuredWrite> query = _context.Set<PendingSecuredWrite>()
|
IQueryable<PendingSecuredWrite> query = _context.Set<PendingSecuredWrite>()
|
||||||
.AsNoTracking();
|
.AsNoTracking();
|
||||||
@@ -143,6 +153,11 @@ WHERE Id = {id}
|
|||||||
query = query.Where(p => p.SiteId == siteId);
|
query = query.Where(p => p.SiteId == siteId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (permittedSiteIds is not null)
|
||||||
|
{
|
||||||
|
query = query.Where(p => permittedSiteIds.Contains(p.SiteId));
|
||||||
|
}
|
||||||
|
|
||||||
return await query.CountAsync(ct);
|
return await query.CountAsync(ct);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+66
@@ -164,6 +164,72 @@ public class SecuredWriteRepositoryTests : IClassFixture<MsSqlMigrationFixture>
|
|||||||
Assert.Equal(0, await repo.CountAsync(status: "Executed", siteId: siteId));
|
Assert.Equal(0, await repo.CountAsync(status: "Executed", siteId: siteId));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[SkippableFact]
|
||||||
|
public async Task QueryAsync_PermittedSiteIds_ReturnsOnlyPermittedRows()
|
||||||
|
{
|
||||||
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
||||||
|
|
||||||
|
// Distinct per-run site identifiers stand in for "SITE1"/"SITE2"/"SITE3" so the
|
||||||
|
// permitted-set filter is asserted deterministically against a shared MSSQL.
|
||||||
|
var site1 = NewSiteId();
|
||||||
|
var site2 = NewSiteId();
|
||||||
|
var site3 = NewSiteId();
|
||||||
|
await using var context = CreateContext();
|
||||||
|
var repo = new SecuredWriteRepository(context);
|
||||||
|
|
||||||
|
await repo.AddAsync(NewRow(site1, status: "Pending"));
|
||||||
|
await repo.AddAsync(NewRow(site1, status: "Pending"));
|
||||||
|
await repo.AddAsync(NewRow(site2, status: "Pending"));
|
||||||
|
await repo.AddAsync(NewRow(site3, status: "Pending"));
|
||||||
|
|
||||||
|
var rows = await repo.QueryAsync(status: null, siteId: null, skip: 0, take: 100,
|
||||||
|
permittedSiteIds: new[] { site1, site3 });
|
||||||
|
Assert.Equal(3, rows.Count);
|
||||||
|
Assert.DoesNotContain(rows, r => r.SiteId == site2);
|
||||||
|
}
|
||||||
|
|
||||||
|
[SkippableFact]
|
||||||
|
public async Task CountAsync_PermittedSiteIds_CountsOnlyPermittedRows()
|
||||||
|
{
|
||||||
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
||||||
|
|
||||||
|
var site1 = NewSiteId();
|
||||||
|
var site2 = NewSiteId();
|
||||||
|
var site3 = NewSiteId();
|
||||||
|
await using var context = CreateContext();
|
||||||
|
var repo = new SecuredWriteRepository(context);
|
||||||
|
|
||||||
|
await repo.AddAsync(NewRow(site1, status: "Pending"));
|
||||||
|
await repo.AddAsync(NewRow(site1, status: "Pending"));
|
||||||
|
await repo.AddAsync(NewRow(site2, status: "Pending"));
|
||||||
|
await repo.AddAsync(NewRow(site3, status: "Pending"));
|
||||||
|
|
||||||
|
var count = await repo.CountAsync(status: null, siteId: null,
|
||||||
|
permittedSiteIds: new[] { site1, site3 });
|
||||||
|
Assert.Equal(3, count);
|
||||||
|
}
|
||||||
|
|
||||||
|
[SkippableFact]
|
||||||
|
public async Task QueryAsync_NullPermittedSiteIds_IsUnfiltered_BackCompat()
|
||||||
|
{
|
||||||
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
||||||
|
|
||||||
|
// 4 rows on a single unique site; null permittedSiteIds must not filter them
|
||||||
|
// out — exactly today's behavior.
|
||||||
|
var siteId = NewSiteId();
|
||||||
|
await using var context = CreateContext();
|
||||||
|
var repo = new SecuredWriteRepository(context);
|
||||||
|
|
||||||
|
await repo.AddAsync(NewRow(siteId, status: "Pending"));
|
||||||
|
await repo.AddAsync(NewRow(siteId, status: "Pending"));
|
||||||
|
await repo.AddAsync(NewRow(siteId, status: "Executed"));
|
||||||
|
await repo.AddAsync(NewRow(siteId, status: "Rejected"));
|
||||||
|
|
||||||
|
var rows = await repo.QueryAsync(status: null, siteId: siteId, skip: 0, take: 100,
|
||||||
|
permittedSiteIds: null);
|
||||||
|
Assert.Equal(4, rows.Count);
|
||||||
|
}
|
||||||
|
|
||||||
// --- helpers ------------------------------------------------------------
|
// --- helpers ------------------------------------------------------------
|
||||||
|
|
||||||
private ScadaBridgeDbContext CreateContext()
|
private ScadaBridgeDbContext CreateContext()
|
||||||
|
|||||||
Reference in New Issue
Block a user