Skip to content

Commit 1c0696a

Browse files
Correct UX bugs relating to context status.
1 parent c9f0955 commit 1c0696a

20 files changed

Lines changed: 88 additions & 69 deletions

src/Stott.Security.Optimizely.Test/Features/Csp/Reporting/ViolationReportSummaryTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public void SanitizedSourceReturnsEitherTheFullDomainOrOriginalString(
1818
string expectedSanitizedSource)
1919
{
2020
// Arrange
21-
var summary = new ViolationReportSummary(Guid.NewGuid(), source, string.Empty, 1, DateTime.Now);
21+
var summary = new ViolationReportSummary(Guid.NewGuid(), source, string.Empty, null, null, 1, DateTime.Now);
2222

2323
// Assert
2424
Assert.That(summary.SanitizedSource, Is.EqualTo(expectedSanitizedSource));
@@ -29,7 +29,7 @@ public void SanitizedSourceReturnsEitherTheFullDomainOrOriginalString(
2929
public void CreatesAppropriateSuggestionsForWildCardDomains(string source, IList<string> expectedSuggestions)
3030
{
3131
// Arrange
32-
var summary = new ViolationReportSummary(Guid.NewGuid(), source, string.Empty, 1, DateTime.Now);
32+
var summary = new ViolationReportSummary(Guid.NewGuid(), source, string.Empty, null, null, 1, DateTime.Now);
3333

3434
// Assert
3535
Assert.That(summary.SourceSuggestions, Is.EquivalentTo(expectedSuggestions));
@@ -40,7 +40,7 @@ public void CreatesAppropriateSuggestionsForWildCardDomains(string source, IList
4040
public void CreatesASingleSuggestionMatchingTheSourceWhenSourceIsNotAUrl(string source)
4141
{
4242
// Arrange
43-
var summary = new ViolationReportSummary(Guid.NewGuid(), source, string.Empty, 1, DateTime.Now);
43+
var summary = new ViolationReportSummary(Guid.NewGuid(), source, string.Empty, null, null, 1, DateTime.Now);
4444

4545
// Assert
4646
Assert.Multiple(() =>
@@ -55,7 +55,7 @@ public void CreatesASingleSuggestionMatchingTheSourceWhenSourceIsNotAUrl(string
5555
public void CreatesAppropriateSuggestionsForDirectives(string directive, IList<string> expectedDirectives)
5656
{
5757
// Arrange
58-
var summary = new ViolationReportSummary(Guid.NewGuid(), string.Empty, directive, 1, DateTime.Now);
58+
var summary = new ViolationReportSummary(Guid.NewGuid(), string.Empty, directive, null, null, 1, DateTime.Now);
5959

6060
// Assert
6161
Assert.That(summary.DirectiveSuggestions, Is.EquivalentTo(expectedDirectives));

src/Stott.Security.Optimizely.Test/Features/PermissionPolicy/PermissionPolicyControllerTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,31 +235,31 @@ public void DeleteDirectives_WhenServiceThrowsException_ThenExceptionIsRethrown(
235235
public async Task GetSettings_WhenOverrideExists_ThenIsInheritedIsFalse()
236236
{
237237
// Arrange
238-
_mockService.Setup(x => x.HasOverrideAsync("app1", null)).ReturnsAsync(true);
238+
_mockService.Setup(x => x.ExistsForContextAsync("app1", null)).ReturnsAsync(true);
239239
_mockService.Setup(x => x.GetPermissionPolicySettingsAsync("app1", null))
240240
.ReturnsAsync(new PermissionPolicySettingsModel { IsEnabled = true });
241241

242242
// Act
243243
var result = await _controller.GetSettings("app1", null);
244244

245245
// Assert
246-
_mockService.Verify(x => x.HasOverrideAsync("app1", null), Times.Once);
246+
_mockService.Verify(x => x.ExistsForContextAsync("app1", null), Times.Once);
247247
_mockService.Verify(x => x.GetPermissionPolicySettingsAsync("app1", null), Times.Once);
248248
}
249249

250250
[Test]
251251
public async Task GetSettings_WhenNoOverrideExists_ThenIsInheritedIsTrue()
252252
{
253253
// Arrange
254-
_mockService.Setup(x => x.HasOverrideAsync("app1", null)).ReturnsAsync(false);
254+
_mockService.Setup(x => x.ExistsForContextAsync("app1", null)).ReturnsAsync(false);
255255
_mockService.Setup(x => x.GetPermissionPolicySettingsAsync("app1", null))
256256
.ReturnsAsync(new PermissionPolicySettingsModel { IsEnabled = false });
257257

258258
// Act
259259
var result = await _controller.GetSettings("app1", null);
260260

261261
// Assert
262-
_mockService.Verify(x => x.HasOverrideAsync("app1", null), Times.Once);
262+
_mockService.Verify(x => x.ExistsForContextAsync("app1", null), Times.Once);
263263
_mockService.Verify(x => x.GetPermissionPolicySettingsAsync("app1", null), Times.Once);
264264
}
265265

src/Stott.Security.Optimizely.Test/Features/PermissionPolicy/Services/PermissionPolicyServiceTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ public async Task HasOverrideAsync_WhenSettingsByContextReturnsNonNull_ThenRetur
446446
.ReturnsAsync(new PermissionPolicySettingsModel());
447447

448448
// Act
449-
var result = await _service.HasOverrideAsync("app1", null);
449+
var result = await _service.ExistsForContextAsync("app1", null);
450450

451451
// Assert
452452
Assert.That(result, Is.True);
@@ -460,7 +460,7 @@ public async Task HasOverrideAsync_WhenSettingsByContextReturnsNull_ThenReturnsF
460460
.ReturnsAsync((PermissionPolicySettingsModel?)null);
461461

462462
// Act
463-
var result = await _service.HasOverrideAsync("app1", null);
463+
var result = await _service.ExistsForContextAsync("app1", null);
464464

465465
// Assert
466466
Assert.That(result, Is.False);

src/Stott.Security.Optimizely/Features/Csp/Reporting/Repository/CspViolationReportRepository.cs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,28 +86,29 @@ public async Task<IList<ViolationReportSummary>> GetReportAsync(string? source,
8686
// - Include host-specific records when both appId and hostName are provided
8787
if (!string.IsNullOrWhiteSpace(appId) && !string.IsNullOrWhiteSpace(hostName))
8888
{
89-
query = query.Where(x => x.AppId == null || x.AppId == string.Empty
90-
|| (x.AppId == appId && (x.HostName == null || x.HostName == string.Empty))
91-
|| (x.AppId == appId && x.HostName == hostName));
89+
query = query.Where(x => x.AppId == appId && x.HostName == hostName);
9290
}
9391
else if (!string.IsNullOrWhiteSpace(appId))
9492
{
95-
query = query.Where(x => x.AppId == null || x.AppId == string.Empty
96-
|| x.AppId == appId);
93+
query = query.Where(x => x.AppId == appId);
9794
}
9895

9996
// Groups violations by BlockedUri and Violated Directive and gets the latest stats.
10097
var violations = await (from violation in query
10198
group violation by new
10299
{
103100
violation.BlockedUri,
104-
violation.ViolatedDirective
101+
violation.ViolatedDirective,
102+
violation.AppId,
103+
violation.HostName
105104
} into violationGroup
106105
select new
107106
{
108107
Id = violationGroup.Select(x => x.Id).First(),
109108
Source = violationGroup.Key.BlockedUri,
110109
Directive = violationGroup.Key.ViolatedDirective,
110+
violationGroup.Key.AppId,
111+
violationGroup.Key.HostName,
111112
Violations = violationGroup.Sum(y => y.Instances),
112113
LastViolated = violationGroup.Max(y => y.LastReported)
113114
}).ToListAsync();
@@ -124,7 +125,7 @@ public async Task<IList<ViolationReportSummary>> GetReportAsync(string? source,
124125

125126
// Convert to a model collection with a unique Id per row.
126127
return violations.Where(x => x.LastViolated >= threshold)
127-
.Select(x => new ViolationReportSummary(x.Id, x.Source, x.Directive, x.Violations, x.LastViolated))
128+
.Select(x => new ViolationReportSummary(x.Id, x.Source, x.Directive, x.AppId, x.HostName, x.Violations, x.LastViolated))
128129
.OrderByDescending(x => x.LastViolated)
129130
.ToList();
130131
}

src/Stott.Security.Optimizely/Features/Csp/Reporting/ViolationReportSummary.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ public sealed class ViolationReportSummary
1414

1515
public string SanitizedSource { get; }
1616

17+
public string? AppId { get; }
18+
19+
public string? HostName { get; }
20+
1721
public IList<string> SourceSuggestions { get; }
1822

1923
public string Directive { get; set; }
@@ -28,12 +32,16 @@ public ViolationReportSummary(
2832
Guid key,
2933
string? source,
3034
string? directive,
35+
string? appId,
36+
string? hostName,
3137
int violations,
3238
DateTime lastViolated)
3339
{
3440
Key = key;
3541
Source = source ?? string.Empty;
3642
Directive = directive ?? string.Empty;
43+
AppId = appId;
44+
HostName = hostName;
3745
Violations = violations;
3846
LastViolated = lastViolated;
3947

src/Stott.Security.Optimizely/Features/Csp/Sandbox/CspSandboxController.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,10 @@ public async Task<IActionResult> Get(string? appId, string? hostName)
2424
{
2525
try
2626
{
27-
var sanitizedHost = hostName.GetSanitizedHostDomain();
28-
var contextData = await service.GetByContextAsync(appId, sanitizedHost);
29-
var isInherited = contextData == null && (!string.IsNullOrWhiteSpace(appId) || !string.IsNullOrWhiteSpace(sanitizedHost));
30-
contextData ??= await service.GetAsync(appId, sanitizedHost);
31-
var data = CspSandboxMapper.MapToResponse(contextData, isInherited);
27+
var sanitizedHostName = hostName.GetSanitizedHostDomain();
28+
var existsForContext = await service.ExistsForContextAsync(appId, sanitizedHostName);
29+
var contextData = await service.GetAsync(appId, sanitizedHostName);
30+
var data = CspSandboxMapper.MapToResponse(contextData, !existsForContext);
3231

3332
return CreateSuccessJson(data);
3433
}

src/Stott.Security.Optimizely/Features/Csp/Sandbox/Service/CspSandboxService.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,6 @@ public async Task<SandboxModel> GetAsync(string? appId, string? hostName)
2626
return settings;
2727
}
2828

29-
public async Task<SandboxModel?> GetByContextAsync(string? appId, string? hostName)
30-
{
31-
return await repository.GetByContextAsync(appId, hostName);
32-
}
33-
3429
public async Task SaveAsync(SandboxModel model, string? modifiedBy, string? appId, string? hostName)
3530
{
3631
if (string.IsNullOrWhiteSpace(modifiedBy))
@@ -55,6 +50,17 @@ public async Task DeleteByContextAsync(string? appId, string? hostName, string?
5550
cacheWrapper.RemoveAll();
5651
}
5752

53+
public async Task<bool> ExistsForContextAsync(string? appId, string? hostName)
54+
{
55+
if (string.IsNullOrWhiteSpace(appId) && string.IsNullOrWhiteSpace(hostName))
56+
{
57+
return true;
58+
}
59+
60+
var actualSettings = await repository.GetByContextAsync(appId, hostName);
61+
return actualSettings is not null;
62+
}
63+
5864
private static string GetCacheKey(string? appId, string? hostName)
5965
{
6066
return $"{CacheKeyPrefix}.{appId ?? "global"}.{hostName ?? "all"}";

src/Stott.Security.Optimizely/Features/Csp/Sandbox/Service/ICspSandboxService.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,6 @@ public interface ICspSandboxService
1616
/// <returns></returns>
1717
Task<SandboxModel> GetAsync(string? appId, string? hostName);
1818

19-
/// <summary>
20-
/// Gets sandbox settings without using fallbacks.
21-
/// </summary>
22-
/// <param name="appId"></param>
23-
/// <param name="hostName"></param>
24-
/// <returns></returns>
25-
Task<SandboxModel?> GetByContextAsync(string? appId, string? hostName);
26-
2719
/// <summary>
2820
/// Saves a specific sandbox settings for a given application and host context.
2921
/// </summary>
@@ -42,4 +34,12 @@ public interface ICspSandboxService
4234
/// <param name="deletedBy"></param>
4335
/// <returns></returns>
4436
Task DeleteByContextAsync(string? appId, string? hostName, string? deletedBy);
37+
38+
/// <summary>
39+
/// Determines whether sandbox settings exist for a specific context (appId and hostName) that would override inherited settings.
40+
/// </summary>
41+
/// <param name="appId"></param>
42+
/// <param name="hostName"></param>
43+
/// <returns></returns>
44+
Task<bool> ExistsForContextAsync(string? appId, string? hostName);
4545
}

src/Stott.Security.Optimizely/Features/Csp/Settings/CspSettingsController.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ public async Task<IActionResult> Get(string? appId, string? hostName)
2424
{
2525
try
2626
{
27-
var contextData = await service.GetByContextAsync(appId, hostName);
28-
var isInherited = contextData == null && (!string.IsNullOrWhiteSpace(appId) || !string.IsNullOrWhiteSpace(hostName));
29-
var data = contextData ?? await service.GetAsync(appId, hostName.GetSanitizedHostDomain());
27+
var sanitizedHostName = hostName.GetSanitizedHostDomain();
28+
var existsForContext = await service.ExistsForContextAsync(appId, sanitizedHostName);
29+
var data = await service.GetAsync(appId, sanitizedHostName);
3030

3131
return CreateSuccessJson(new CspSettingsResponseModel
3232
{
@@ -38,7 +38,7 @@ public async Task<IActionResult> Get(string? appId, string? hostName)
3838
UseInternalReporting = data.UseInternalReporting,
3939
UseExternalReporting = data.UseExternalReporting,
4040
ExternalReportToUrl = data.ExternalReportToUrl ?? string.Empty,
41-
IsInherited = isInherited
41+
IsInherited = !existsForContext
4242
});
4343
}
4444
catch (Exception exception)

src/Stott.Security.Optimizely/Features/Csp/Settings/Service/CspSettingsService.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,6 @@ public async Task<CspSettings> GetAsync(string? appId, string? hostName)
2626
return settings;
2727
}
2828

29-
public async Task<CspSettings?> GetByContextAsync(string? appId, string? hostName)
30-
{
31-
return await repository.GetByContextAsync(appId, hostName);
32-
}
33-
3429
public async Task SaveAsync(ICspSettings? cspSettings, string? modifiedBy, string? appId, string? hostName)
3530
{
3631
if (cspSettings is null || string.IsNullOrWhiteSpace(modifiedBy))
@@ -55,6 +50,17 @@ public async Task DeleteByContextAsync(string? appId, string? hostName, string?
5550
cache.RemoveAll();
5651
}
5752

53+
public async Task<bool> ExistsForContextAsync(string? appId, string? hostName)
54+
{
55+
if (string.IsNullOrWhiteSpace(appId) && string.IsNullOrWhiteSpace(hostName))
56+
{
57+
return false;
58+
}
59+
60+
var actualSettings = await repository.GetByContextAsync(appId, hostName);
61+
return actualSettings is not null;
62+
}
63+
5864
private static string GetCacheKey(string? appId, string? hostName)
5965
{
6066
return $"{CacheKeyPrefix}.{appId ?? "global"}.{hostName ?? "all"}";

0 commit comments

Comments
 (0)