mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(sharing): let an expired grant be re-granted instead of colliding
The live-pair uniqueness is a partial index (WHERE revoked_at IS NULL), which cannot read the clock: an expired but never-revoked grant kept occupying the slot, so re-inviting the same person after expiry always failed as a duplicate though they had no access. Close the lapsed row at the moment of re-invite — conditionally, the expiry state in the WHERE — so revocation still never deletes and the fresh invitation mints its row. The revoker is stamped GRANTOR because the paired revoked-by check requires both columns set and the enum has no expiry member. An audit row records the close. Proven against the real index: the guard was watched fail with a duplicate before the fix landed.
This commit is contained in:
1 parent
ee561eaa23
commit
de83954a7f
2 files changed
+100
No files matched your search
@@ -379,6 +379,42 @@ export async function inviteGrant(
|
||||
}
|
||||
|
||||
const now = new Date();
|
||||
|
||||
// An expired grant confers nothing (see `grantState`), but its row still
|
||||
// sits in the live-pair slot the partial unique index guards: the index
|
||||
// predicate is `WHERE revoked_at IS NULL` and cannot read the clock. Without
|
||||
// ending the lapsed row here, a re-invitation for the same pair after expiry
|
||||
// always collides on P2002 and 409s, though the person genuinely has no
|
||||
// access anymore. Close it now — conditionally, the expiry state in the
|
||||
// WHERE — so "revocation never deletes" still holds and the fresh invitation
|
||||
// can mint its row. A live (unexpired) row is not matched, so its P2002 stays
|
||||
// correct. `revokedBy` is stamped GRANTOR rather than left null because the
|
||||
// paired CHECK (`account_grants_revoked_by_check`) requires revoked_at and
|
||||
// revoked_by to be both set or both null and the revoker enum has no expiry
|
||||
// member; the owner re-inviting is the actor that ends the lapsed row.
|
||||
const closed = await db.accountGrant.updateMany({
|
||||
where: {
|
||||
grantorId: input.grantorId,
|
||||
granteeId: input.granteeId,
|
||||
revokedAt: null,
|
||||
expiresAt: { lte: now },
|
||||
},
|
||||
data: { revokedAt: now, revokedBy: "GRANTOR" },
|
||||
});
|
||||
if (closed.count > 0) {
|
||||
// Keep "who had access, from when to when, and who ended it" answering
|
||||
// truthfully: the lapsed row ended by its own expiry, closed on re-invite.
|
||||
await auditLog("sharing.grant.expired_closed", {
|
||||
userId: input.grantorId,
|
||||
details: {
|
||||
granteeId: input.granteeId,
|
||||
closedCount: closed.count,
|
||||
reason: "expired",
|
||||
},
|
||||
client: db,
|
||||
}).catch(() => {});
|
||||
}
|
||||
|
||||
try {
|
||||
return await db.accountGrant.create({
|
||||
data: {
|
||||
|
||||
@@ -439,6 +439,70 @@ describe("account grant lifecycle", () => {
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("re-invites after a grant EXPIRED, closing the lapsed row instead of colliding", async () => {
|
||||
// The bug carrier is the partial unique index `WHERE revoked_at IS NULL`:
|
||||
// it cannot read the clock, so an expired-but-never-revoked row keeps
|
||||
// occupying the live-pair slot and every re-invitation 409s. A mocked
|
||||
// Prisma cannot reproduce this — the index is the thing under test — so it
|
||||
// lives here, against real Postgres.
|
||||
for (const accepted of [true, false]) {
|
||||
const owner = await makeUser("owner");
|
||||
const delegate = await makeUser("delegate");
|
||||
|
||||
const first = await inviteGrant({
|
||||
grantorId: owner.id,
|
||||
granteeId: delegate.id,
|
||||
access: "READ",
|
||||
scope: null,
|
||||
expiresAt: new Date(Date.now() - 1000),
|
||||
});
|
||||
if (accepted) {
|
||||
// Acceptance is refused once lapsed, so stamp it directly to build the
|
||||
// "accepted then expired" row the report describes.
|
||||
await getPrismaClient().accountGrant.update({
|
||||
where: { id: first.id },
|
||||
data: { acceptedAt: new Date(Date.now() - 2000) },
|
||||
});
|
||||
}
|
||||
|
||||
// The lapsed row confers nothing already.
|
||||
expect(
|
||||
await findActiveGrant({ grantorId: owner.id, granteeId: delegate.id }),
|
||||
).toBeNull();
|
||||
|
||||
// Re-inviting the same pair must succeed rather than 409 on the index.
|
||||
const second = await inviteGrant({
|
||||
grantorId: owner.id,
|
||||
granteeId: delegate.id,
|
||||
access: "READ",
|
||||
scope: null,
|
||||
});
|
||||
expect(second.id).not.toBe(first.id);
|
||||
expect(second.revokedAt).toBeNull();
|
||||
|
||||
// History kept, not deleted: the old row is now closed by expiry, and its
|
||||
// revoker is stamped so the paired CHECK holds.
|
||||
const old = await getPrismaClient().accountGrant.findUniqueOrThrow({
|
||||
where: { id: first.id },
|
||||
});
|
||||
expect(old.revokedAt).not.toBeNull();
|
||||
expect(old.revokedBy).toBe("GRANTOR");
|
||||
|
||||
// Two rows for the pair: the ended one and the fresh one.
|
||||
expect(
|
||||
await getPrismaClient().accountGrant.count({
|
||||
where: { grantorId: owner.id, granteeId: delegate.id },
|
||||
}),
|
||||
).toBe(2);
|
||||
|
||||
// And the close is on the audit trail.
|
||||
const trail = await getPrismaClient().auditLog.findMany({
|
||||
where: { action: "sharing.grant.expired_closed", userId: owner.id },
|
||||
});
|
||||
expect(trail).toHaveLength(1);
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps history without limit — a third grant does not collide with two ended ones", async () => {
|
||||
const owner = await makeUser("owner");
|
||||
const delegate = await makeUser("delegate");
|
||||
|
||||
Reference in new issue
Block a user