feat/postit-acl #32

Merged
notazof merged 33 commits from feat/postit-acl into release/1.0.7 2026-08-18 16:14:32 +01:00
2 changed files with 61 additions and 10 deletions
Showing only changes of commit e376aed887 - Show all commits

fix(blogacl): restrict Circle + BlogAcl reads and writes to caller's own data

Closes the data-leak holes that survived the move of these controllers
from Yavsc.Api to Yavsc.Blogs. Circles are personal — a circle and its
membership should never be visible, modifiable, or deletable by anyone
other than its owner.

BlogAclApiController:
- GetBlogACL() was returning the full table; now filters by
  Allowed.OwnerId == caller's uid, with an Include(a => a.Allowed)
  so EF Core can push the filter into SQL instead of materialising
  the whole table.
- Other endpoints (GetById, Put, Post, Delete) already enforced
  ownership; left as is.

CircleApiController:
- GetCircle() (no id) now filters by OwnerId.
- GetCircle(id) now requires c.Id == id && c.OwnerId == uid;
  returns 404 (not 403) on miss to avoid leaking the existence of
  someone else's circle.
- PutCircle verifies the existing record is owned by the caller,
  then forces circle.OwnerId = uid on the body (the client's value
  is ignored). Returns ChallengeResult when the caller doesn't own
  the record.
- PostCircle forces circle.OwnerId = uid (was trusting the body).
- DeleteCircle now filters by OwnerId; 404 on miss.

All checks use the same source of truth (User.FindFirstValue(
ClaimTypes.NameIdentifier)) that the existing BlogAclApiController
authz code already relies on.
Paul Schneider 2026-08-17 23:36:20 +01:00
Signed by: notazof
GPG key ID: 1DD5D838E5343B06

View file

@ -1,3 +1,4 @@
using System.Linq;
using System.Security.Claims; using System.Security.Claims;
using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc;
using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore;
@ -18,11 +19,19 @@ namespace Yavsc.Blogs.Controllers
_context = context; _context = context;
} }
// GET: api/BlogAclApi /// <summary>
/// Returns the ACL entries for the caller's own blog posts.
/// Blog posts (and therefore their ACLs) are private to their
/// author — the API never exposes another user's ACL.
/// </summary>
// GET: api/blogacl
[HttpGet] [HttpGet]
public IEnumerable<CircleAuthorizationToBlogPost> GetBlogACL() public IEnumerable<CircleAuthorizationToBlogPost> GetBlogACL()
{ {
return _context.CircleAuthorizationToBlogPost; var uid = User.FindFirstValue(ClaimTypes.NameIdentifier);
return _context.CircleAuthorizationToBlogPost
.Include(a => a.Allowed)
.Where(a => a.Allowed.OwnerId == uid);
} }
// GET: api/BlogAclApi/5 // GET: api/BlogAclApi/5

View file

@ -1,3 +1,5 @@
using System.Linq;
using System.Security.Claims;
using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc;
using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore;
using Yavsc.Models; using Yavsc.Models;
@ -17,14 +19,22 @@ namespace Yavsc.Blogs.Controllers
_context = context; _context = context;
} }
// GET: api/CircleApi /// <summary>
/// Returns the caller's own circles. Circles are personal —
/// the API never exposes another user's circles, even by id.
/// </summary>
// GET: api/circle
[HttpGet] [HttpGet]
public IEnumerable<Circle> GetCircle() public IEnumerable<Circle> GetCircle()
{ {
return _context.Circle; var uid = User.FindFirstValue(ClaimTypes.NameIdentifier);
return _context.Circle.Where(c => c.OwnerId == uid);
} }
// GET: api/CircleApi/5 /// <summary>
/// Returns a single circle only when it belongs to the caller.
/// </summary>
// GET: api/circle/5
[HttpGet("{id}", Name = "GetCircle")] [HttpGet("{id}", Name = "GetCircle")]
public async Task<IActionResult> GetCircle([FromRoute] long id) public async Task<IActionResult> GetCircle([FromRoute] long id)
{ {
@ -33,7 +43,9 @@ namespace Yavsc.Blogs.Controllers
return BadRequest(ModelState); return BadRequest(ModelState);
} }
Circle circle = await _context.Circle.SingleAsync(m => m.Id == id); var uid = User.FindFirstValue(ClaimTypes.NameIdentifier);
Circle circle = await _context.Circle.SingleOrDefaultAsync(
m => m.Id == id && m.OwnerId == uid);
if (circle == null) if (circle == null)
{ {
@ -43,7 +55,12 @@ namespace Yavsc.Blogs.Controllers
return Ok(circle); return Ok(circle);
} }
// PUT: api/CircleApi/5 /// <summary>
/// Replaces a circle. The caller must own it; the server
/// reasserts ownership regardless of any OwnerId the client
/// tries to put in the body.
/// </summary>
// PUT: api/circle/5
[HttpPut("{id}")] [HttpPut("{id}")]
public async Task<IActionResult> PutCircle([FromRoute] long id, [FromBody] Circle circle) public async Task<IActionResult> PutCircle([FromRoute] long id, [FromBody] Circle circle)
{ {
@ -57,6 +74,16 @@ namespace Yavsc.Blogs.Controllers
return BadRequest(); return BadRequest();
} }
var uid = User.FindFirstValue(ClaimTypes.NameIdentifier);
var existing = await _context.Circle.SingleOrDefaultAsync(
c => c.Id == id && c.OwnerId == uid);
if (existing is null)
{
return new ChallengeResult();
}
// Force OwnerId to the caller; the body value is ignored.
circle.OwnerId = uid;
_context.Entry(circle).State = EntityState.Modified; _context.Entry(circle).State = EntityState.Modified;
try try
@ -78,7 +105,11 @@ namespace Yavsc.Blogs.Controllers
return new StatusCodeResult(StatusCodes.Status204NoContent); return new StatusCodeResult(StatusCodes.Status204NoContent);
} }
// POST: api/CircleApi /// <summary>
/// Creates a circle owned by the caller. The server overwrites
/// any OwnerId the client sends in the body.
/// </summary>
// POST: api/circle
[HttpPost] [HttpPost]
public async Task<IActionResult> PostCircle([FromBody] Circle circle) public async Task<IActionResult> PostCircle([FromBody] Circle circle)
{ {
@ -87,6 +118,9 @@ namespace Yavsc.Blogs.Controllers
return BadRequest(ModelState); return BadRequest(ModelState);
} }
var uid = User.FindFirstValue(ClaimTypes.NameIdentifier);
circle.OwnerId = uid;
_context.Circle.Add(circle); _context.Circle.Add(circle);
try try
{ {
@ -107,7 +141,13 @@ namespace Yavsc.Blogs.Controllers
return CreatedAtRoute("GetCircle", new { id = circle.Id }, circle); return CreatedAtRoute("GetCircle", new { id = circle.Id }, circle);
} }
// DELETE: api/CircleApi/5 /// <summary>
/// Deletes a circle only if the caller owns it. Returns 404
/// (not 403) when the circle does not exist or is not owned
/// by the caller, to avoid leaking the existence of someone
/// else's circle.
/// </summary>
// DELETE: api/circle/5
[HttpDelete("{id}")] [HttpDelete("{id}")]
public async Task<IActionResult> DeleteCircle([FromRoute] long id) public async Task<IActionResult> DeleteCircle([FromRoute] long id)
{ {
@ -116,7 +156,9 @@ namespace Yavsc.Blogs.Controllers
return BadRequest(ModelState); return BadRequest(ModelState);
} }
Circle circle = await _context.Circle.SingleAsync(m => m.Id == id); var uid = User.FindFirstValue(ClaimTypes.NameIdentifier);
Circle circle = await _context.Circle.SingleOrDefaultAsync(
m => m.Id == id && m.OwnerId == uid);
if (circle == null) if (circle == null)
{ {
return NotFound(); return NotFound();