Skip to content

Commit 65ed0b2

Browse files
NikaNatsNikaNats
committed
feat: Enhance document ownership checks in IDocumentStore and InMemoryDocumentStore, update DocumentsController to validate ownership, and add unit tests for ownership scenarios
1 parent 5c31bf0 commit 65ed0b2

13 files changed

Lines changed: 279 additions & 75 deletions

File tree

‎src/Sentinel.Application/Common/Abstractions/IDocumentStore.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace Sentinel.Application.Common.Abstractions;
55
public interface IDocumentStore
66
{
77
Task<IReadOnlyCollection<DocumentDto>> ListAsync(string ownerSub, CancellationToken cancellationToken);
8-
Task<DocumentDto?> GetByIdAsync(Guid id, CancellationToken cancellationToken);
8+
Task<DocumentDto?> GetByIdAsync(Guid id, string ownerSub, CancellationToken cancellationToken);
99
Task<DocumentDto> CreateAsync(string ownerSub, CreateDocumentRequest request, CancellationToken cancellationToken);
1010
Task<DocumentDto?> UpdateAsync(Guid id, string ownerSub, UpdateDocumentRequest request, CancellationToken cancellationToken);
1111
Task<bool> DeleteAsync(Guid id, string ownerSub, CancellationToken cancellationToken);

‎src/Sentinel.Infrastructure/Cache/InMemoryDocumentStore.cs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,17 @@ public Task<IReadOnlyCollection<DocumentDto>> ListAsync(string ownerSub, Cancell
2121
return Task.FromResult<IReadOnlyCollection<DocumentDto>>(results);
2222
}
2323

24-
public Task<DocumentDto?> GetByIdAsync(Guid id, CancellationToken cancellationToken)
24+
public Task<DocumentDto?> GetByIdAsync(Guid id, string ownerSub, CancellationToken cancellationToken)
2525
{
2626
cancellationToken.ThrowIfCancellationRequested();
27-
return Task.FromResult(documents.TryGetValue(id, out var state) ? Map(state) : null);
27+
28+
if (!documents.TryGetValue(id, out var state)
29+
|| !string.Equals(state.OwnerSub, ownerSub, StringComparison.Ordinal))
30+
{
31+
return Task.FromResult<DocumentDto?>(null);
32+
}
33+
34+
return Task.FromResult<DocumentDto?>(Map(state));
2835
}
2936

3037
public Task<DocumentDto> CreateAsync(string ownerSub, CreateDocumentRequest request, CancellationToken cancellationToken)
@@ -47,6 +54,7 @@ public Task<DocumentDto> CreateAsync(string ownerSub, CreateDocumentRequest requ
4754
public Task<DocumentDto?> UpdateAsync(Guid id, string ownerSub, UpdateDocumentRequest request, CancellationToken cancellationToken)
4855
{
4956
cancellationToken.ThrowIfCancellationRequested();
57+
var spinWait = new SpinWait();
5058

5159
while (documents.TryGetValue(id, out var current))
5260
{
@@ -66,6 +74,8 @@ public Task<DocumentDto> CreateAsync(string ownerSub, CreateDocumentRequest requ
6674
{
6775
return Task.FromResult<DocumentDto?>(Map(updated));
6876
}
77+
78+
spinWait.SpinOnce();
6979
}
7080

7181
return Task.FromResult<DocumentDto?>(null);

‎src/Sentinel.Infrastructure/Cache/SessionBlacklistCache.cs‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,20 @@
1-
using Microsoft.Extensions.Caching.Distributed;
21
using Sentinel.Application.Common.Abstractions;
2+
using StackExchange.Redis;
33

44
namespace Sentinel.Infrastructure.Cache;
55

6-
public sealed class SessionBlacklistCache(IDistributedCache cache, ILogger<SessionBlacklistCache> logger) : ISessionBlacklistCache
6+
public sealed class SessionBlacklistCache(IConnectionMultiplexer redis, ILogger<SessionBlacklistCache> logger) : ISessionBlacklistCache
77
{
8+
private readonly IDatabase db = redis.GetDatabase();
89
private static string GetKey(string sessionId) => $"blacklist:sid:{sessionId}";
910

1011
public async Task BlacklistSessionAsync(string sessionId, TimeSpan ttl, CancellationToken ct)
1112
{
13+
ct.ThrowIfCancellationRequested();
14+
1215
try
1316
{
14-
var options = new DistributedCacheEntryOptions
15-
{
16-
AbsoluteExpirationRelativeToNow = ttl
17-
};
18-
19-
await cache.SetAsync(GetKey(sessionId), [], options, ct);
17+
await db.StringSetAsync(GetKey(sessionId), RedisValue.EmptyString, ttl, When.Always, CommandFlags.None);
2018
logger.LogInformation("Session {SessionId} blacklisted for {TtlSeconds} seconds.", sessionId, ttl.TotalSeconds);
2119
}
2220
catch (Exception ex)
@@ -28,10 +26,11 @@ public async Task BlacklistSessionAsync(string sessionId, TimeSpan ttl, Cancella
2826

2927
public async ValueTask<bool> IsSessionBlacklistedAsync(string sessionId, CancellationToken ct)
3028
{
29+
ct.ThrowIfCancellationRequested();
30+
3131
try
3232
{
33-
var value = await cache.GetAsync(GetKey(sessionId), ct);
34-
return value is not null;
33+
return await db.KeyExistsAsync(GetKey(sessionId), CommandFlags.None);
3534
}
3635
catch (Exception ex)
3736
{

‎src/Sentinel.Presentation/Controllers/DocumentsController.cs‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,16 @@ namespace Sentinel.Controllers;
1111
[Route("v1/documents")]
1212
public sealed class DocumentsController(IDocumentStore documentStore, ILogger<DocumentsController> logger) : ControllerBase
1313
{
14+
private string? CurrentSub => User.FindFirst("sub")?.Value;
15+
1416
[HttpGet]
1517
[Authorize(Policy = Policies.DocumentsRead)]
1618
[ProducesResponseType(StatusCodes.Status200OK)]
1719
[ProducesResponseType(StatusCodes.Status401Unauthorized)]
1820
[ProducesResponseType(StatusCodes.Status403Forbidden)]
1921
public async Task<IActionResult> ListDocuments(CancellationToken cancellationToken)
2022
{
21-
var subject = User.FindFirst("sub")?.Value;
23+
var subject = CurrentSub;
2224
if (string.IsNullOrWhiteSpace(subject))
2325
{
2426
return Unauthorized();
@@ -36,14 +38,14 @@ public async Task<IActionResult> ListDocuments(CancellationToken cancellationTok
3638
[ProducesResponseType(StatusCodes.Status403Forbidden)]
3739
public async Task<IActionResult> GetDocument(Guid id, CancellationToken cancellationToken)
3840
{
39-
var subject = User.FindFirst("sub")?.Value;
41+
var subject = CurrentSub;
4042
if (string.IsNullOrWhiteSpace(subject))
4143
{
4244
return Unauthorized();
4345
}
4446

45-
var document = await documentStore.GetByIdAsync(id, cancellationToken);
46-
if (document is null || !string.Equals(document.OwnerSub, subject, StringComparison.Ordinal))
47+
var document = await documentStore.GetByIdAsync(id, subject, cancellationToken);
48+
if (document is null)
4749
{
4850
return NotFound();
4951
}
@@ -61,7 +63,7 @@ public async Task<IActionResult> GetDocument(Guid id, CancellationToken cancella
6163
[ProducesResponseType(StatusCodes.Status409Conflict)]
6264
public async Task<IActionResult> CreateDocument([FromBody] CreateDocumentRequest request, CancellationToken cancellationToken)
6365
{
64-
var subject = User.FindFirst("sub")?.Value;
66+
var subject = CurrentSub;
6567
if (string.IsNullOrWhiteSpace(subject))
6668
{
6769
return Unauthorized();
@@ -87,7 +89,7 @@ public async Task<IActionResult> CreateDocument([FromBody] CreateDocumentRequest
8789
[ProducesResponseType(StatusCodes.Status409Conflict)]
8890
public async Task<IActionResult> UpdateDocument(Guid id, [FromBody] UpdateDocumentRequest request, CancellationToken cancellationToken)
8991
{
90-
var subject = User.FindFirst("sub")?.Value;
92+
var subject = CurrentSub;
9193
if (string.IsNullOrWhiteSpace(subject))
9294
{
9395
return Unauthorized();
@@ -104,7 +106,6 @@ public async Task<IActionResult> UpdateDocument(Guid id, [FromBody] UpdateDocume
104106

105107
[HttpDelete("{id}")]
106108
[Authorize(Policy = Policies.DocumentsWrite)]
107-
[RequireIdempotency]
108109
[RequireMtlsBinding]
109110
[ProducesResponseType(StatusCodes.Status204NoContent)]
110111
[ProducesResponseType(StatusCodes.Status401Unauthorized)]
@@ -113,7 +114,7 @@ public async Task<IActionResult> UpdateDocument(Guid id, [FromBody] UpdateDocume
113114
[ProducesResponseType(StatusCodes.Status409Conflict)]
114115
public async Task<IActionResult> DeleteDocument(Guid id, CancellationToken cancellationToken)
115116
{
116-
var subject = User.FindFirst("sub")?.Value;
117+
var subject = CurrentSub;
117118
if (string.IsNullOrWhiteSpace(subject))
118119
{
119120
return Unauthorized();

‎src/Sentinel.Presentation/Controllers/ProfileController.cs‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,18 @@ public IActionResult GetProfile()
2121
var displayName = User.FindFirstValue("name") ?? string.Empty;
2222

2323
var rolesClaim = User.FindFirst("realm_access.roles")?.Value;
24-
var roles = string.IsNullOrWhiteSpace(rolesClaim)
25-
? []
26-
: JsonSerializer.Deserialize<string[]>(rolesClaim) ?? [];
24+
var roles = Array.Empty<string>();
25+
if (!string.IsNullOrWhiteSpace(rolesClaim))
26+
{
27+
try
28+
{
29+
roles = JsonSerializer.Deserialize<string[]>(rolesClaim) ?? [];
30+
}
31+
catch (JsonException)
32+
{
33+
roles = [];
34+
}
35+
}
2736

2837
if (string.IsNullOrWhiteSpace(sub))
2938
{

‎src/Sentinel.Presentation/DependencyInjection/ApiServiceCollectionExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,10 @@ public static WebApplication UseApiLayer(this WebApplication app)
9292
app.UseRouting();
9393

9494
app.UseAuthentication();
95-
app.UseRateLimiter();
9695
app.UseMiddleware<DpopValidationMiddleware>();
9796
app.UseMiddleware<MtlsBindingMiddleware>();
9897
app.UseMiddleware<AcrValidationMiddleware>();
98+
app.UseRateLimiter();
9999
app.UseAuthorization();
100100

101101
app.MapPrometheusScrapingEndpoint();

‎src/Sentinel.Presentation/Middleware/DpopValidationMiddleware.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ public async Task InvokeAsync(HttpContext context)
4646
}
4747

4848
var token = authHeader["DPoP ".Length..].Trim();
49+
// RFC 9449 section 4.2: htu excludes query string and fragment.
4950
var requestUrl = $"{context.Request.Scheme}://{context.Request.Host}{context.Request.Path}";
5051

5152
var thumbprint = TryExtractProofThumbprint(dpopProof);

‎src/Sentinel.Presentation/Middleware/Filters/RequireIdempotencyAttribute.cs‎

Lines changed: 70 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using Microsoft.AspNetCore.Mvc;
22
using Microsoft.AspNetCore.Mvc.Filters;
33
using Microsoft.AspNetCore.Mvc.Infrastructure;
4+
using Microsoft.Extensions.Logging.Abstractions;
45
using StackExchange.Redis;
56

67
namespace Sentinel.Middleware.Filters;
@@ -26,20 +27,43 @@ public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionE
2627
}
2728

2829
var idempotencyKey = keyValues.ToString();
30+
var logger = context.HttpContext.RequestServices.GetService<ILogger<RequireIdempotencyAttribute>>()
31+
?? NullLogger<RequireIdempotencyAttribute>.Instance;
2932
var redis = context.HttpContext.RequestServices.GetRequiredService<IConnectionMultiplexer>();
3033
var db = redis.GetDatabase();
3134
var sub = context.HttpContext.User.FindFirst("sub")?.Value ?? "anonymous";
3235
var redisKey = $"idempotency:{sub}:{idempotencyKey}";
3336

34-
bool lockAcquired = await db.StringSetAsync(
35-
redisKey,
36-
"IN_PROGRESS",
37-
TimeSpan.FromMinutes(5),
38-
When.NotExists);
37+
bool lockAcquired;
38+
try
39+
{
40+
lockAcquired = await db.StringSetAsync(
41+
redisKey,
42+
"IN_PROGRESS",
43+
TimeSpan.FromMinutes(5),
44+
When.NotExists);
45+
}
46+
catch (RedisException ex)
47+
{
48+
logger.LogCritical(ex, "Redis unavailable during idempotency check.");
49+
context.Result = BuildUnavailableResult();
50+
return;
51+
}
3952

4053
if (!lockAcquired)
4154
{
42-
var currentState = await db.StringGetAsync(redisKey);
55+
RedisValue currentState;
56+
try
57+
{
58+
currentState = await db.StringGetAsync(redisKey);
59+
}
60+
catch (RedisException ex)
61+
{
62+
logger.LogCritical(ex, "Redis unavailable during idempotency state read.");
63+
context.Result = BuildUnavailableResult();
64+
return;
65+
}
66+
4367
if (string.Equals(currentState.ToString(), "COMPLETED", StringComparison.Ordinal))
4468
{
4569
context.Result = new NoContentResult();
@@ -62,20 +86,57 @@ public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionE
6286

6387
if (executedContext.Exception is null && IsSuccessfulResult(executedContext.Result))
6488
{
65-
await db.StringSetAsync(redisKey, "COMPLETED", TimeSpan.FromHours(24), When.Always);
89+
try
90+
{
91+
await db.StringSetAsync(redisKey, "COMPLETED", TimeSpan.FromHours(24), When.Always);
92+
}
93+
catch (RedisException ex)
94+
{
95+
logger.LogCritical(ex, "Redis unavailable while marking idempotency request as completed.");
96+
executedContext.Result = BuildUnavailableResult();
97+
}
6698
}
6799
else
68100
{
69-
await db.KeyDeleteAsync(redisKey);
101+
try
102+
{
103+
await db.KeyDeleteAsync(redisKey);
104+
}
105+
catch (RedisException ex)
106+
{
107+
logger.LogCritical(ex, "Redis unavailable while releasing idempotency lock after unsuccessful request.");
108+
executedContext.Result = BuildUnavailableResult();
109+
}
70110
}
71111
}
72112
catch
73113
{
74-
await db.KeyDeleteAsync(redisKey);
114+
try
115+
{
116+
await db.KeyDeleteAsync(redisKey);
117+
}
118+
catch (RedisException ex)
119+
{
120+
logger.LogCritical(ex, "Redis unavailable while releasing idempotency lock after exception.");
121+
}
122+
75123
throw;
76124
}
77125
}
78126

127+
private static ObjectResult BuildUnavailableResult()
128+
{
129+
return new ObjectResult(new ProblemDetails
130+
{
131+
Type = "/errors/idempotency-unavailable",
132+
Title = "Idempotency service unavailable",
133+
Status = StatusCodes.Status503ServiceUnavailable
134+
})
135+
{
136+
StatusCode = StatusCodes.Status503ServiceUnavailable
137+
};
138+
}
139+
79140
private static bool IsSuccessfulResult(IActionResult? result)
80141
{
81142
if (result is IStatusCodeActionResult statusCodeResult)

‎tests/Sentinel.Tests/Integration/SecurityScenarioTests.cs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -184,23 +184,23 @@ public async Task S11_MissingScope_Returns403()
184184
[Fact]
185185
public async Task S07_RateLimitExceeded_Returns429()
186186
{
187-
using var ecdsa = ECDsa.Create(ECCurve.NamedCurves.nistP256);
188-
var jwk = JsonWebKeyConverter.ConvertFromECDsaSecurityKey(new ECDsaSecurityKey(ecdsa));
189-
var jwkObject = new Dictionary<string, string>
190-
{
191-
["crv"] = jwk.Crv!,
192-
["kty"] = jwk.Kty!,
193-
["x"] = jwk.X!,
194-
["y"] = jwk.Y!
195-
};
196-
var jkt = ComputeEcThumbprint(jwkObject);
197-
198187
var successCount = 0;
199188
var rateLimitedCount = 0;
200189
var requestUrl = new Uri(client.BaseAddress!, "/v1/profile").ToString();
201190

202-
var tasks = Enumerable.Range(0, 105).Select(async _ =>
191+
var tasks = Enumerable.Range(0, 160).Select(async _ =>
203192
{
193+
using var ecdsa = ECDsa.Create(ECCurve.NamedCurves.nistP256);
194+
var jwk = JsonWebKeyConverter.ConvertFromECDsaSecurityKey(new ECDsaSecurityKey(ecdsa));
195+
var jwkObject = new Dictionary<string, string>
196+
{
197+
["crv"] = jwk.Crv!,
198+
["kty"] = jwk.Kty!,
199+
["x"] = jwk.X!,
200+
["y"] = jwk.Y!
201+
};
202+
203+
var jkt = ComputeEcThumbprint(jwkObject);
204204
var accessToken = TestTokenIssuer.MintAccessToken(jkt);
205205
using var request = CreateSignedRequest(ecdsa, jwkObject, accessToken, HttpMethod.Get, requestUrl);
206206
var response = await client.SendAsync(request, TestContext.Current.CancellationToken);

0 commit comments

Comments
 (0)