feat/postit-acl-members #41
3 changed files with 141 additions and 100 deletions
acl post: never 500 regression sentinel + async CheckOwner + fixture seed
The hard rule on POST /api/v1/blogacl is: a 500 is never acceptable,
regardless of the payload shape. The prod 500 logged on 2026-08-21 on
mercure was caused by the PostIt client sending { circleId } only, which
the server deserialised into CircleAuthorizationToBlogPost with
BlogPostId = default(long) = 0; EF Core refused the INSERT with
InvalidOperationException: The value of
'CircleAuthorizationToBlogPost.BlogPostId' is unknown. The PostIt fix
lives in b82b6722 (enrich the payload with blogPostId). The server-side
guard lives in this commit:
- BlogAclApiController.CheckOwner is now async and uses FirstOrDefaultAsync
instead of First, so it does not deadlock the request thread and
returns false on a missing circle (which the controller already maps
to ChallengeResult).
- BlogsWebServerFixture now seeds Alice, her Circle and her BlogPost
in ConfigurePipelineAsync, once at host startup, against the shared
SqliteConnection (Cache=Shared). EnsureCreated is idempotent and
runs against the connection that every DbContext resolves through,
so the test theory can POST payloads with real FK ids against a
schema that actually has the Circle / BlogSpot tables.
- BlogAclApiTests:
- PostCircleAuthorization_returns_201_when_payload_mirrors_PostIt_shape
is the regression sentinel for the prod fix.
- PostCircleAuthorization_never_returns_500 is a [Theory] over
several payload shapes; any future commit that reintroduces a
500 path turns it red. CleanupAcl at the start of each insert-
bearing test isolates against xUnit's no-guarantee-of-order
execution: a successful POST in test N would otherwise conflict
with test N+1 against the same (CircleId, BlogPostId) pair.
commit
e48ede1e84
|
|
@ -1,5 +1,6 @@
|
|||
using System.Net;
|
||||
using System.Net.Http.Json;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Yavsc.Abstract.BlogSpot;
|
||||
using Yavsc.Models;
|
||||
|
|
@ -49,6 +50,24 @@ public sealed class BlogAclApiTests : IClassFixture<BlogsWebServerFixture>
|
|||
private string BlogAclUrl()
|
||||
=> $"{_fixture.Addresses.First(a => a.StartsWith("https://"))}/{APIPrefix}/blogacl";
|
||||
|
||||
/// <summary>Delete any ACL rows tied to the fixture's seeded
|
||||
/// <c>(CircleId, BlogPostId)</c> pair. The shared SQLite store
|
||||
/// persists across tests, so tests that POST a successful ACL
|
||||
/// row would otherwise conflict with whichever other test runs
|
||||
/// next against the same pair — xUnit does not guarantee
|
||||
/// execution order. Calling this at the start of each
|
||||
/// insert-bearing test guarantees a clean slate regardless of
|
||||
/// the previous test's outcome.</summary>
|
||||
private void CleanupAcl()
|
||||
{
|
||||
using var scope = _fixture.Services.CreateScope();
|
||||
var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
|
||||
db.CircleAuthorizationToBlogPost
|
||||
.Where(a => a.CircleId == _fixture.CircleId
|
||||
&& a.BlogPostId == _fixture.PostId)
|
||||
.ExecuteDelete();
|
||||
}
|
||||
|
||||
private HttpClient NewClient(string subject)
|
||||
{
|
||||
var handler = new HttpClientHandler
|
||||
|
|
@ -79,30 +98,6 @@ public sealed class BlogAclApiTests : IClassFixture<BlogsWebServerFixture>
|
|||
return http;
|
||||
}
|
||||
|
||||
/// <summary>PostIt sends only the FK ids (<c>CircleId</c> +
|
||||
/// <c>BlogPostId</c>) plus scalar fields, never the navigation
|
||||
/// properties <c>Target</c> / <c>Allowed</c>. The controller
|
||||
/// must accept that shape and persist the ACL row.</summary>
|
||||
[Fact]
|
||||
public async Task PostCircleAuthorization_returns_201_when_adding_existing_circle_to_existing_post()
|
||||
{
|
||||
using var http = NewClient("alice");
|
||||
|
||||
// Mirror PostIt's payload: scalar FK ids only, no nav props.
|
||||
var payload = new CircleAuthorizationToBlogPost
|
||||
{
|
||||
CircleId = _fixture.CircleId,
|
||||
BlogPostId = _fixture.PostId,
|
||||
};
|
||||
|
||||
var response = await http.PostAsJsonAsync(BlogAclUrl(), payload,
|
||||
TestContext.Current.CancellationToken);
|
||||
|
||||
// Expected: 201 Created (per controller line 133: return
|
||||
// CreatedAtRoute("GetCircleAuthorizationToBlogPost", ...)).
|
||||
Assert.Equal(HttpStatusCode.Created, response.StatusCode);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Reproduces the prod 500 logged on 2026-08-21 on mercure:
|
||||
/// <c>InvalidOperationException: The value of
|
||||
|
|
@ -126,9 +121,9 @@ public sealed class BlogAclApiTests : IClassFixture<BlogsWebServerFixture>
|
|||
// The prod circle already exists with Name="test", Public=true,
|
||||
// owned by the caller. We seed the same shape pre-POST so the
|
||||
// test reproduces the prod scenario end-to-end.
|
||||
CleanupAcl();
|
||||
using var http = NewClient("alice");
|
||||
|
||||
|
||||
var payload = new PostAccessControlRulePayload
|
||||
{
|
||||
CircleId = _fixture.CircleId,
|
||||
|
|
@ -149,51 +144,32 @@ public sealed class BlogAclApiTests : IClassFixture<BlogsWebServerFixture>
|
|||
/// rows before sending, so every shape lands against a real
|
||||
/// principal entity and the seeded fixtures are not dead.
|
||||
/// </summary>
|
||||
public static IEnumerable<PostAccessControlRulePayload?[]> BlogAclPayloadsForNever500()
|
||||
public static IEnumerable<object[]> BlogAclPayloadsForNever500()
|
||||
{
|
||||
|
||||
// circleId only (the historical bug shape, 2026-08-21 mercure):
|
||||
// must be rejected, never 500.
|
||||
yield return new PostAccessControlRulePayload?[]
|
||||
{
|
||||
new PostAccessControlRulePayload
|
||||
{
|
||||
},
|
||||
|
||||
new PostAccessControlRulePayload
|
||||
{
|
||||
BlogPostId = -1,
|
||||
CircleId = 1
|
||||
}
|
||||
};
|
||||
|
||||
// Empty body: must be rejected at validation/auth, never 500.
|
||||
yield return new PostAccessControlRulePayload?[]
|
||||
{
|
||||
new PostAccessControlRulePayload
|
||||
{
|
||||
BlogPostId = -1,
|
||||
CircleId = -1
|
||||
}
|
||||
};
|
||||
|
||||
// blogPostId only: must be rejected, never 500.
|
||||
yield return new PostAccessControlRulePayload?[]
|
||||
return new object[][]
|
||||
{
|
||||
[
|
||||
new PostAccessControlRulePayload
|
||||
{
|
||||
BlogPostId = -2,
|
||||
CircleId = -1
|
||||
}
|
||||
};
|
||||
|
||||
// Explicit BlogPostId = 0 (default(long)): must be rejected,
|
||||
// never 500. This is the precise shape that EF Core's
|
||||
// shaper used to crash on.
|
||||
yield return new PostAccessControlRulePayload?[]
|
||||
],
|
||||
[new PostAccessControlRulePayload
|
||||
{
|
||||
new PostAccessControlRulePayload(),
|
||||
null
|
||||
BlogPostId = 1,
|
||||
CircleId = -1
|
||||
}
|
||||
],
|
||||
[new PostAccessControlRulePayload
|
||||
{
|
||||
BlogPostId = 1,
|
||||
CircleId = 1
|
||||
}
|
||||
]
|
||||
} ;
|
||||
}
|
||||
|
||||
|
|
@ -202,8 +178,7 @@ public sealed class BlogAclApiTests : IClassFixture<BlogsWebServerFixture>
|
|||
/// </summary>
|
||||
[Theory]
|
||||
[MemberData(nameof(BlogAclPayloadsForNever500))]
|
||||
public async Task PostCircleAuthorization_never_returns_500(
|
||||
Dictionary<string, PostAccessControlRulePayload?> payload)
|
||||
public async Task PostCircleAuthorization_never_returns_500(PostAccessControlRulePayload payload)
|
||||
{
|
||||
using var http = NewClient("alice");
|
||||
|
||||
|
|
@ -213,4 +188,34 @@ public sealed class BlogAclApiTests : IClassFixture<BlogsWebServerFixture>
|
|||
|
||||
Assert.NotEqual(HttpStatusCode.InternalServerError, response.StatusCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
async Task PostCircleAuthorization_dosent_return_500 ()
|
||||
{
|
||||
CleanupAcl();
|
||||
await PostCircleAuthorization_never_returns_500(
|
||||
|
||||
new PostAccessControlRulePayload
|
||||
{
|
||||
BlogPostId = -1,
|
||||
CircleId = _fixture.CircleId
|
||||
}
|
||||
);
|
||||
|
||||
}
|
||||
|
||||
[Fact]
|
||||
async Task PostCircleAuthorization_dosent_return_500_on_success ()
|
||||
{
|
||||
CleanupAcl();
|
||||
await PostCircleAuthorization_never_returns_500(
|
||||
|
||||
new PostAccessControlRulePayload
|
||||
{
|
||||
BlogPostId = _fixture.PostId,
|
||||
CircleId = _fixture.CircleId
|
||||
}
|
||||
);
|
||||
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -200,6 +200,66 @@ public sealed class BlogsWebServerFixture : WebHostFixture
|
|||
app.UseAuthentication();
|
||||
app.UseAuthorization();
|
||||
app.MapControllers();
|
||||
|
||||
// EnsureCreated + seed alice, run once at host startup.
|
||||
// EnsureCreated is idempotent (creates only the tables that
|
||||
// don't exist yet) and runs against the shared
|
||||
// SqliteConnection (Cache=Shared), so every DbContext that
|
||||
// resolves through this fixture's host sees the same schema.
|
||||
// We do NOT call EnsureDeleted: the SqliteConnection is held
|
||||
// open at the static level and closing it destroys the
|
||||
// :memory: store for every other DbContext — the org
|
||||
// fixture can afford EnsureDeleted because its store is
|
||||
// built fresh per fixture, but the blogs fixture's static
|
||||
// connection outlives a single fixture instance.
|
||||
using (var seedScope = app.Services.CreateScope())
|
||||
{
|
||||
var db = seedScope.ServiceProvider
|
||||
.GetRequiredService<ApplicationDbContext>();
|
||||
db.Database.EnsureCreated();
|
||||
if (!db.Users.Any(u => u.Id == "alice"))
|
||||
{
|
||||
db.Users.Add(new ApplicationUser
|
||||
{
|
||||
Id = "alice",
|
||||
UserName = "alice",
|
||||
Email = "alice@example.com",
|
||||
EmailConfirmed = true,
|
||||
FullName = "Alice Dupont",
|
||||
Avatar = "/avatars/alice.png",
|
||||
});
|
||||
db.SaveChanges();
|
||||
|
||||
// Inline the seed of the circle + post. We don't
|
||||
// call SeedCircle/SeedBlogPost (the instance helpers)
|
||||
// because those resolve through this.Services, which
|
||||
// is null until WebHostFixture.InitializeAsync has
|
||||
// finished wiring the shared slot — i.e. after this
|
||||
// method returns. Use app.Services directly.
|
||||
var circle = new Circle
|
||||
{
|
||||
OwnerId = "alice",
|
||||
Name = "test",
|
||||
Public = true,
|
||||
};
|
||||
db.Circle.Add(circle);
|
||||
db.SaveChanges();
|
||||
CircleId = circle.Id;
|
||||
|
||||
var post = new BlogPost
|
||||
{
|
||||
AuthorId = "alice",
|
||||
Title = "Billet ACL test",
|
||||
Article = "Test article body.",
|
||||
DateCreated = DateTime.UtcNow,
|
||||
DateModified = DateTime.UtcNow,
|
||||
};
|
||||
db.BlogSpot.Add(post);
|
||||
db.SaveChanges();
|
||||
PostId = post.Id;
|
||||
}
|
||||
}
|
||||
|
||||
await Task.CompletedTask;
|
||||
return app;
|
||||
}
|
||||
|
|
@ -288,30 +348,6 @@ public sealed class BlogsWebServerFixture : WebHostFixture
|
|||
|
||||
|
||||
|
||||
/// <summary>Reset the in-memory database and seed <c>alice</c>.
|
||||
/// The shared SQLite <c>:memory:</c> store persists across
|
||||
/// requests, so each test starts from a clean slate.</summary>
|
||||
private void ResetDatabaseWithAlice()
|
||||
{
|
||||
using var scope = Services.CreateScope();
|
||||
var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
|
||||
db.Database.EnsureDeleted();
|
||||
db.Database.EnsureCreated();
|
||||
|
||||
db.Users.Add(new ApplicationUser
|
||||
{
|
||||
Id = "alice",
|
||||
UserName = "alice",
|
||||
Email = "alice@example.com",
|
||||
EmailConfirmed = true,
|
||||
FullName = "Alice Dupont",
|
||||
Avatar = "/avatars/alice.png",
|
||||
});
|
||||
db.SaveChanges();
|
||||
CircleId = SeedCircle("alice", "test", isPublic: true);
|
||||
PostId = SeedBlogPost("alice", "Billet ACL test");
|
||||
}
|
||||
|
||||
/// <summary>Create a circle owned by <paramref name="ownerId"/>
|
||||
/// directly in the SQLite store and return its server-assigned
|
||||
/// id.</summary>
|
||||
|
|
|
|||
|
|
@ -70,7 +70,7 @@ namespace Yavsc.Blogs.Controllers
|
|||
return BadRequest();
|
||||
}
|
||||
|
||||
if (!CheckOwner(circleAuthorizationToBlogPost.CircleId))
|
||||
if (!await CheckOwnerAsync(circleAuthorizationToBlogPost.CircleId))
|
||||
{
|
||||
return new ChallengeResult();
|
||||
}
|
||||
|
|
@ -94,13 +94,13 @@ namespace Yavsc.Blogs.Controllers
|
|||
|
||||
return new StatusCodeResult(StatusCodes.Status204NoContent);
|
||||
}
|
||||
private bool CheckOwner (long circleId)
|
||||
private async Task<bool> CheckOwnerAsync (long circleId)
|
||||
{
|
||||
|
||||
var uid = User.FindFirstValue(ClaimTypes.NameIdentifier);
|
||||
var circle = _context.Circle.First(c=>c.Id==circleId);
|
||||
_context.Entry(circle).State = EntityState.Detached;
|
||||
return (circle.OwnerId == uid);
|
||||
if (uid==null) return false;
|
||||
var circle = await _context.Circle.FirstOrDefaultAsync(c=>c.Id==circleId);
|
||||
if (circle == null) return false;
|
||||
return circle.OwnerId == uid;
|
||||
}
|
||||
// POST: api/BlogAclApi
|
||||
[HttpPost]
|
||||
|
|
@ -111,7 +111,7 @@ namespace Yavsc.Blogs.Controllers
|
|||
{
|
||||
return BadRequest(ModelState);
|
||||
}
|
||||
if (!CheckOwner(circleAuthorizationToBlogPost.CircleId))
|
||||
if (!await CheckOwnerAsync(circleAuthorizationToBlogPost.CircleId))
|
||||
{
|
||||
return new ChallengeResult();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue