Compare commits

...

2 Commits

Author SHA1 Message Date
cesnimda 4db8c08958 fix(security): evaluate tenant CurrentUserId live, not at construction
CI and Deploy / test (push) Successful in 2m38s
CI and Deploy / deploy (push) Failing after 7s
Production POST /api/cv/variants returned 200 but GET /api/cv/variants/{id}
returned 404, with the query logged as `... FROM CvVariants WHERE FALSE`
(no parameters). The created row had a correct OwnerUserId; the read was
excluded by the global query filter because CurrentUserId was null at
query time. Reproduced locally: it affected EVERY tenant-filtered read
(CV list returned 0 after creating 5, JobApplications returned total 0),
not just CV -- writes worked, reads came back empty.

Root cause: the "local" JwtBearer OnTokenValidated resolves the
request-scoped JobTrackerContext (to run LocalSessionValidator) BEFORE the
authentication middleware assigns HttpContext.User. JobTrackerContext
captured CurrentUserId in its constructor from ICurrentUserService.UserId,
which reads HttpContext.User -- still unauthenticated at that point -- so
CurrentUserId froze to null. That same scoped instance is reused by the
controller, so `CurrentUserId != null && OwnerUserId == CurrentUserId`
compiled to WHERE FALSE for the whole request. POST worked because
CreateAsync sets OwnerUserId from the controller-resolved user, and
inserts are not filtered.

Fix: make CurrentUserId a computed property that reads
ICurrentUserService.UserId live, so the query filters see the
authenticated user at query-execution time. Deny-on-null is preserved
(still null for an unauthenticated principal). LocalSessionValidator is
unaffected -- it already uses IgnoreQueryFilters and queries by explicit
sid.

Verified on a real MariaDB 11 container end to end: create then read a
variant returns 200, the variant list returns all rows, and
GET /api/jobapplications reads normally. Added
CurrentUserIdLiveEvaluationTests pinning the live-evaluation behaviour
(both fail against a constructor snapshot). 422 tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 00:20:09 +02:00
cesnimda 474654b1c1 fix(cv): add LongTailJson to reconciler CareerProfiles schema
Production GET /api/cv/outline returned 500 "Unknown column
'c.LongTailJson'". CareerProfiles is reconciler-owned, but the
reconciler's CREATE TABLE (both the SQLite and MySQL branches) only
listed Id, OwnerUserId, ProfileJson, Version, CreatedAtUtc, UpdatedAtUtc.
LongTailJson was added to the CareerProfile model in Phase 3 but neither
CREATE was updated and no column-repair existed, so:

- existing databases (prod): the MySQL CREATE is guarded on
  !HasMySqlTable, so it never runs once the table exists, and nothing
  adds the column -> LoadStructuredAsync selects a column that isn't
  there.
- fresh databases: the CREATE itself omitted the column, so even a brand
  new MariaDB/SQLite was missing it. The 420 tests never caught this
  because they build tables from the EF model, not the reconciler DDL.
  The release audit missed it because it never exercised /api/cv/outline.

Add LongTailJson to both CREATE statements and add an additive repair
(EnsureColumn / EnsureMySqlColumn) for existing tables. DEFAULT ''
backfills existing rows and matches the non-nullable model property.
This is the sanctioned reconciler repair path, not a manual ALTER, and
preserves existing data (ADD COLUMN is non-destructive).

Verified on a real MariaDB 11 container: an existing 6-column
CareerProfiles gains LongTailJson on startup (repair path), a fresh DB
gets it from the CREATE (longtext), and GET /api/cv/outline returns 200.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 00:19:52 +02:00
3 changed files with 96 additions and 2 deletions
+13 -2
View File
@@ -6,11 +6,22 @@ namespace JobTrackerApi.Data
{
public class JobTrackerContext : IdentityDbContext<ApplicationUser>
{
public string? CurrentUserId { get; }
private readonly JobTrackerApi.Services.ICurrentUserService _currentUser;
// Evaluated live on each access, NOT captured in the constructor. The "local" JwtBearer
// OnTokenValidated resolves this request-scoped DbContext to run LocalSessionValidator BEFORE
// the authentication middleware assigns HttpContext.User. A constructor snapshot therefore froze
// CurrentUserId to null for the whole request, and the same scoped instance is reused by the
// controller — so every tenant-filtered read compiled to `WHERE FALSE` and returned nothing
// (created rows 404'd on read, lists came back empty) even though writes set OwnerUserId
// correctly from the controller-resolved user. Reading it live means the query filters see the
// authenticated user at query-execution time. Deny-on-null is preserved: it is still null for an
// unauthenticated principal.
public string? CurrentUserId => _currentUser.UserId;
public JobTrackerContext(DbContextOptions<JobTrackerContext> options, JobTrackerApi.Services.ICurrentUserService currentUser) : base(options)
{
CurrentUserId = currentUser.UserId;
_currentUser = currentUser;
}
public DbSet<Company> Companies => Set<Company>();
@@ -0,0 +1,69 @@
using JobTrackerApi.Data;
using JobTrackerApi.Models;
using JobTrackerApi.Services;
using Microsoft.EntityFrameworkCore;
using Moq;
using Xunit;
namespace JobTrackerApi.Tests;
// Regression guard for the production tenant-read failure: the "local" JwtBearer OnTokenValidated
// resolves the request-scoped JobTrackerContext to run session validation BEFORE the auth middleware
// sets HttpContext.User. When CurrentUserId was captured in the DbContext constructor, that froze it
// to null for the whole request, and every global-query-filtered read returned WHERE FALSE — created
// rows 404'd on read and lists came back empty, while writes (which set OwnerUserId from the
// controller-resolved user) still succeeded.
//
// The fix makes CurrentUserId read ICurrentUserService.UserId live on each access. These tests fail
// if it reverts to a constructor snapshot.
public sealed class CurrentUserIdLiveEvaluationTests
{
private static (JobTrackerContext db, Mock<ICurrentUserService> user) New(string? initialUserId)
{
var options = new DbContextOptionsBuilder<JobTrackerContext>()
.UseInMemoryDatabase(Guid.NewGuid().ToString()).Options;
var user = new Mock<ICurrentUserService>();
user.SetupGet(s => s.UserId).Returns(initialUserId);
return (new JobTrackerContext(options, user.Object), user);
}
[Fact]
public void CurrentUserId_reflects_the_user_resolved_after_construction()
{
// Constructed while unauthenticated (UserId null), exactly as OnTokenValidated does.
var (db, user) = New(initialUserId: null);
Assert.Null(db.CurrentUserId);
// The auth middleware then assigns the principal; UserId becomes non-null on the SAME instance.
user.SetupGet(s => s.UserId).Returns("user-1");
Assert.Equal("user-1", db.CurrentUserId);
}
[Fact]
public async Task A_row_created_while_unauthenticated_context_is_readable_once_the_user_resolves()
{
var (db, user) = New(initialUserId: null);
// Write happens with an explicit owner (mirrors CreateAsync taking the controller-resolved id),
// while the context was built before the user resolved.
db.CvVariants.Add(new CvVariant
{
OwnerUserId = "user-1",
Name = "Test",
PublicSlug = "slug-1",
SettingsJson = "{}",
Version = 1,
});
await db.SaveChangesAsync();
// Before the user resolves, the tenant filter denies (deny-on-null preserved).
Assert.Empty(await db.CvVariants.ToListAsync());
// Once the middleware sets the user, the same context reads the row — not WHERE FALSE.
user.SetupGet(s => s.UserId).Returns("user-1");
var visible = await db.CvVariants.ToListAsync();
Assert.Single(visible);
Assert.Equal("user-1", visible[0].OwnerUserId);
}
}
@@ -773,12 +773,20 @@ public static class StartupInitializationExtensions
"Id" INTEGER NOT NULL CONSTRAINT "PK_CareerProfiles" PRIMARY KEY AUTOINCREMENT,
"OwnerUserId" TEXT NOT NULL,
"ProfileJson" TEXT NOT NULL,
"LongTailJson" TEXT NOT NULL DEFAULT '',
"Version" INTEGER NOT NULL,
"CreatedAtUtc" TEXT NOT NULL,
"UpdatedAtUtc" TEXT NOT NULL
);
""");
// LongTailJson was added to the CareerProfile model (Phase 3) but this CREATE and
// the MySQL one were never updated to match, and no column-repair existed — so a
// CareerProfiles table created before this line lacks the column and /api/cv/outline
// (CareerProfileService.LoadStructuredAsync) fails with "Unknown column LongTailJson".
// Additive repair for existing databases.
EnsureColumn(c, "CareerProfiles", "LongTailJson", """ALTER TABLE "CareerProfiles" ADD COLUMN "LongTailJson" TEXT NOT NULL DEFAULT '';""");
Exec(c, """
CREATE TABLE IF NOT EXISTS "CareerProfileVersions" (
"Id" INTEGER NOT NULL CONSTRAINT "PK_CareerProfileVersions" PRIMARY KEY AUTOINCREMENT,
@@ -1620,6 +1628,7 @@ public static class StartupInitializationExtensions
`Id` int NOT NULL AUTO_INCREMENT,
`OwnerUserId` varchar(255) NOT NULL,
`ProfileJson` longtext NOT NULL,
`LongTailJson` longtext NOT NULL,
`Version` int NOT NULL,
`CreatedAtUtc` datetime(6) NOT NULL,
`UpdatedAtUtc` datetime(6) NOT NULL,
@@ -1628,6 +1637,11 @@ public static class StartupInitializationExtensions
cmd.ExecuteNonQuery();
}
// Additive repair for a CareerProfiles table created before LongTailJson was added
// to the model. Without it, /api/cv/outline fails with "Unknown column LongTailJson".
// DEFAULT '' backfills existing rows and matches the non-nullable model property.
EnsureMySqlColumn(conn, "CareerProfiles", "LongTailJson", "ALTER TABLE `CareerProfiles` ADD COLUMN `LongTailJson` longtext NOT NULL DEFAULT '';");
if (!HasMySqlTable(conn, "CareerProfileVersions") && HasMySqlTable(conn, "CareerProfiles"))
{
using var cmd = conn.CreateCommand();