mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 06:26:26 +00:00
refactor(db): TrekRepository base — every repository path validates the request context and reads bypass the identity map by default
This commit is contained in:
@@ -87,8 +87,21 @@ describe('Passkey management — non-numeric id parity (Plan 3b Task 3 review, F
|
||||
expect(res.body).toEqual({ error: 'Passkey not found' });
|
||||
});
|
||||
|
||||
it('PASSKEY-INT-003 — PATCH /auth/passkey/credentials/0x10 (hex-literal id) returns the legacy 404, not a 500', async () => {
|
||||
// task-3-rereview.md R1: on the passkey credential routes the LEGACY bound
|
||||
// `Number(id)` (unlike the invite route's raw-string bind PASSKEY-INT-003's
|
||||
// sibling cases guard against), so `Number('0x10') === 16` and the legacy
|
||||
// statement would have ACTED ON credential 16, not answered a 404 — this
|
||||
// case pins the deliberate `toRowId` narrowing the fix round chose
|
||||
// (accepted deviation), not "the legacy 404". Credential 16 is seeded so
|
||||
// the case is load-bearing: dropping the `toRowId` guard makes
|
||||
// `renamePasskey` fall back to `Number(id)`, which would rename credential
|
||||
// 16 instead of 404ing — verified by hand, recorded in the task report.
|
||||
it('PASSKEY-INT-003 — PATCH /auth/passkey/credentials/0x10 (hex-literal id) is refused by the toRowId narrowing, even though credential 16 exists', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
testDb.prepare(
|
||||
`INSERT INTO webauthn_credentials (id, user_id, credential_id, public_key, name)
|
||||
VALUES (16, ?, 'cred-16', X'00', 'Original16')`,
|
||||
).run(user.id);
|
||||
|
||||
const res = await request(app)
|
||||
.patch('/api/auth/passkey/credentials/0x10')
|
||||
@@ -97,6 +110,8 @@ describe('Passkey management — non-numeric id parity (Plan 3b Task 3 review, F
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Passkey not found' });
|
||||
const row = testDb.prepare('SELECT name FROM webauthn_credentials WHERE id = 16').get() as { name: string };
|
||||
expect(row.name).toBe('Original16'); // untouched — not renamed to 'Ghost'
|
||||
});
|
||||
|
||||
it('PASSKEY-INT-004 — DELETE /auth/passkey/credentials/abc (non-numeric id, correct password) returns the legacy 404, not a 500', async () => {
|
||||
|
||||
@@ -169,6 +169,17 @@ describe('InviteTokensRepository', () => {
|
||||
expect(sql).toMatch(/left join `?trips`?/i);
|
||||
expect(sql).not.toMatch(/left join `?users`?/i);
|
||||
});
|
||||
|
||||
it('INVREPO-016: findWithCreatorAndTrip\'s generated SQL joins users with an INNER JOIN and trips with a LEFT JOIN — pins RI5\'s join TYPE (task-3-rereview.md R2; a `leftJoin` rewrite of the users join must fail this)', async () => {
|
||||
const { user: admin } = createUser(testDb, { username: 'join-type-check-single' });
|
||||
const invite = createInviteToken(testDb, { token: 'join-type-check-single', created_by: admin.id });
|
||||
|
||||
const { sql } = await captureSql(() => invites.findWithCreatorAndTrip(invite.id));
|
||||
|
||||
expect(sql).toMatch(/inner join `?users`?/i);
|
||||
expect(sql).toMatch(/left join `?trips`?/i);
|
||||
expect(sql).not.toMatch(/left join `?users`?/i);
|
||||
});
|
||||
});
|
||||
|
||||
// RI6/RI7 — deleteInvite's 404 check and the delete itself.
|
||||
|
||||
@@ -210,15 +210,26 @@ describe('OauthClientsRepository', () => {
|
||||
});
|
||||
|
||||
describe('identity-map write-back regression (program RULING, D-shape)', () => {
|
||||
it('OAUTHCLIENTREPO-016: listByUser then findAuthRow then updateSecretHash inside uow.transactional — the write survives', async () => {
|
||||
// task-4-review.md F1: the D-shape needs the `nativeUpdate`'s target
|
||||
// column in the FIRST, WIDER projection and absent from the SECOND,
|
||||
// narrower one — a stale write-back only reverts a column the second
|
||||
// read's snapshot doesn't already carry fresh. The original ordering
|
||||
// here (`listByUser`, narrow, then `findAuthRow`, wide) had it backwards:
|
||||
// `findAuthRow`'s own re-snapshot of `client_secret_hash` left that
|
||||
// column clean, so the mutation this test exists to catch (reverting
|
||||
// `disableIdentityMap: true` back to `refresh: true`) passed anyway.
|
||||
// `findAuthRow` (wide, carries `client_secret_hash`) now goes FIRST and
|
||||
// `findOwned` (narrow, no `client_secret_hash`) second — mutation-proven
|
||||
// load-bearing in the task report.
|
||||
it('OAUTHCLIENTREPO-016: findAuthRow then findOwned then updateSecretHash inside uow.transactional — the write survives', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const { id, clientId } = seedClient({ userId: user.id, secretHash: 'original', name: 'kept-name' });
|
||||
|
||||
await withRequestContext(t.orm, async () => {
|
||||
// Projection A
|
||||
await clients.listByUser(user.id);
|
||||
// Projection B — a different field set on the same row
|
||||
// Projection A — wide, carries client_secret_hash.
|
||||
await clients.findAuthRow(clientId);
|
||||
// Projection B — narrow, a different (smaller) field set on the same row.
|
||||
await clients.findOwned(id, user.id);
|
||||
// The intended write, inside the request's own transaction.
|
||||
await uow.transactional(async () => {
|
||||
await clients.updateSecretHash(id, 'rotated');
|
||||
|
||||
@@ -85,22 +85,33 @@ describe('OauthConsentsRepository', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('identity-map isolation (program RULING, D-shape)', () => {
|
||||
it('OAUTHCONSENTREPO-007: findScopes leaves no managed entity behind — a raw write to the same row on the same transaction connection survives the transaction\'s closing flush', async () => {
|
||||
// task-4-review.md F2: this is NOT a B1/D-shape regression test — that
|
||||
// shape needs TWO managed reads with different field sets on the same
|
||||
// row (task-1-rereview.md M-A/M-B), and `OauthConsentsRepository` has
|
||||
// exactly one read method, so the D-shape is not constructible here at
|
||||
// all. Mutating `findScopes`'s `disableIdentityMap: true` to `refresh:
|
||||
// true` leaves this case green either way — it was never testing the
|
||||
// mechanism its old name/comment claimed. What it DOES pin, genuinely:
|
||||
// a single `findScopes` read leaves nothing dirty (the entity's snapshot
|
||||
// and current data are identical, so the change-set computer finds no
|
||||
// diff), and a raw write to the same row on the transaction's own
|
||||
// connection, made afterward, is never fought by the closing flush.
|
||||
describe('findScopes does not dirty the entity it reads', () => {
|
||||
it('OAUTHCONSENTREPO-007: findScopes then a raw write on the same transaction connection — the raw write survives the transaction\'s closing flush', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
seedClient(user.id, 'proto-7');
|
||||
testDb.prepare(`INSERT INTO oauth_consents (client_id, user_id, scopes) VALUES (?, ?, ?)`).run('proto-7', user.id, '["a"]');
|
||||
|
||||
await withRequestContext(t.orm, async () => {
|
||||
await uow.transactional(async () => {
|
||||
// Projection: the only read shape this repository has.
|
||||
// The only read shape this repository has — a single projection,
|
||||
// so no second, differently-projected read of the same row can
|
||||
// ever be lingering in the identity map for this one to collide
|
||||
// with.
|
||||
await consents.findScopes('proto-7', user.id);
|
||||
// A write bypassing the ORM entirely, on the SAME single SQLite
|
||||
// connection the open transaction holds (D3/D6) — the shape any
|
||||
// nativeUpdate write from elsewhere in the same request takes. If
|
||||
// findScopes had left a managed entity in the identity map (the
|
||||
// B1 mechanism), the transaction's closing flush would try to
|
||||
// write the READ snapshot back over this change.
|
||||
// nativeUpdate write from elsewhere in the same request takes.
|
||||
testDb.prepare('UPDATE oauth_consents SET scopes = ? WHERE client_id = ? AND user_id = ?').run('["a","b"]', 'proto-7', user.id);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,223 @@
|
||||
/**
|
||||
* `TrekRepository` — the base class every repository under
|
||||
* `src/db/repositories/*.ts` extends (Plan 3b interlude B,
|
||||
* `.superpowers/sdd/2026-09-22-orm-phase3b/task-base-repo-brief.md`).
|
||||
*
|
||||
* Restores the program's D6 ratchet ("an unwrapped entrypoint fails
|
||||
* loudly") for every repository path, closing the gap
|
||||
* `task-1-rereview.md` F-R1 found: the `disableIdentityMap: true` ruling
|
||||
* (Plan 3b Task 1, B1) resolves its `EntityManager` with MikroORM's
|
||||
* non-validating `getContext(false)`, so a converted read stopped failing
|
||||
* closed outside a request context, and the write paths
|
||||
* (`nativeUpdate`/`nativeDelete`/`count`/`insert`/`upsert`) never validated
|
||||
* in the first place (`task-1-review.md` F7 INFO). This file drives every
|
||||
* overridden method against a real ORM with `allowGlobalContext: false`
|
||||
* (the production setting) — outside any request context each one must
|
||||
* reject with MikroORM's own `cannotUseGlobalContext` `ValidationError`;
|
||||
* inside `withRequestContext`/`uow.transactional` each must succeed exactly
|
||||
* as before.
|
||||
*
|
||||
* Uses the two concrete repositories the base class actually protects
|
||||
* (`UsersRepository` for find/findOne/count/nativeUpdate/nativeDelete/
|
||||
* insert/upsert/qb, `WebauthnChallengesRepository.claimChallenge` for the
|
||||
* one `kysely()` statement) rather than instantiating the abstract class
|
||||
* directly — `TrekRepository` is never instantiated on its own in
|
||||
* production either.
|
||||
*
|
||||
* **D-shape RULE for every regression test in the program written against
|
||||
* the `disableIdentityMap` ruling (task-4-review.md F1):** the column the
|
||||
* `nativeUpdate` targets must be in the FIRST, WIDER projection and absent
|
||||
* from the SECOND, narrower one. A stale write-back can only revert a
|
||||
* column whose value the second read's own snapshot does not already carry
|
||||
* fresh — if the write's target column is in the narrower, later-read
|
||||
* projection instead, that read's own snapshot is already correct for it
|
||||
* and no staleness is observable no matter what `disableIdentityMap` does.
|
||||
* `OAUTHCLIENTREPO-016` (`OauthClients.repository.test.ts`) had this
|
||||
* backwards until this task; `OAUTHTOKREPO-029` (`OauthTokens.repository
|
||||
* .test.ts`) already had it right and is the shape to copy.
|
||||
*/
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
import { createUser } from '../../../helpers/factories';
|
||||
import { Users } from '../../../../src/db/entities/Users.entity';
|
||||
import type { UsersRepository } from '../../../../src/db/repositories/Users.repository';
|
||||
import { WebauthnChallenges } from '../../../../src/db/entities/WebauthnChallenges.entity';
|
||||
import type { WebauthnChallengesRepository } from '../../../../src/db/repositories/WebauthnChallenges.repository';
|
||||
import { UnitOfWork } from '../../../../src/nest/database/unit-of-work';
|
||||
import { withRequestContext } from '../../../../src/nest/database/request-context';
|
||||
|
||||
const GLOBAL_CONTEXT_ERROR = /global EntityManager|global context/i;
|
||||
|
||||
const testDb = createSnapshotTestDb();
|
||||
let t: TestOrm;
|
||||
let users: UsersRepository;
|
||||
let challenges: WebauthnChallengesRepository;
|
||||
let uow: UnitOfWork;
|
||||
|
||||
// Global context DISALLOWED here on purpose — the production setting, like
|
||||
// tests/unit/nest/database/request-context.test.ts and
|
||||
// tests/unit/nest/cron-registrar.service.test.ts's D6 describe block — so a
|
||||
// repository call with no wrapper around it genuinely throws.
|
||||
beforeAll(async () => {
|
||||
t = await createTestOrm(testDb, { allowGlobalContext: false });
|
||||
users = t.repo(Users);
|
||||
challenges = t.repo(WebauthnChallenges);
|
||||
uow = new UnitOfWork(t.em);
|
||||
});
|
||||
// `t.clear()` is deliberately NOT called here (unlike most repository test
|
||||
// files): `EntityManager.clear()` itself calls the VALIDATING `getContext()`
|
||||
// (`node_modules/@mikro-orm/core/EntityManager.js:1785`), so it would throw
|
||||
// the same `cannotUseGlobalContext` this file exists to test, outside any
|
||||
// request context. Every read in this file passes through
|
||||
// `disableIdentityMap: true` (the base class's default), so nothing ever
|
||||
// populates the identity map for a stale entry to clear between tests.
|
||||
beforeEach(() => {
|
||||
resetTestDb(testDb);
|
||||
});
|
||||
afterAll(async () => {
|
||||
await t.close();
|
||||
testDb.close();
|
||||
});
|
||||
|
||||
describe('TrekRepository — every overridden path fails closed outside a request context', () => {
|
||||
it('TREKREPO-001: findOne rejects', async () => {
|
||||
await expect(users.findOne({ id: 1 })).rejects.toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-002: find rejects', async () => {
|
||||
await expect(users.find({})).rejects.toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-003: findAll rejects', async () => {
|
||||
await expect(users.findAll()).rejects.toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-004: count rejects', async () => {
|
||||
await expect(users.count({})).rejects.toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-005: nativeUpdate rejects', async () => {
|
||||
await expect(users.nativeUpdate({ id: 1 }, { username: 'outside-context' })).rejects.toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-006: nativeDelete rejects', async () => {
|
||||
await expect(users.nativeDelete({ id: -1 })).rejects.toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-007: insert rejects', async () => {
|
||||
await expect(
|
||||
users.insert({ username: 'outside-context', email: 'outside@example.test', password_hash: 'h', role: 'user', first_seen_version: '1.0' }),
|
||||
).rejects.toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-008: upsert rejects', async () => {
|
||||
await expect(
|
||||
users.upsert({ id: 1, username: 'outside-context' }, { onConflictFields: ['id'], onConflictAction: 'merge' }),
|
||||
).rejects.toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-009: a QueryBuilder read (qb) rejects', () => {
|
||||
expect(() => users.qb('u')).toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-010: createQueryBuilder rejects', () => {
|
||||
expect(() => users.createQueryBuilder('u')).toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-011: a Kysely statement (the protected kysely() helper) rejects', async () => {
|
||||
await expect(challenges.claimChallenge('outside-context', 'registration', Date.now())).rejects.toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
|
||||
it('TREKREPO-012: getEntityManager() itself rejects', () => {
|
||||
expect(() => users.getEntityManager()).toThrow(GLOBAL_CONTEXT_ERROR);
|
||||
});
|
||||
});
|
||||
|
||||
describe('TrekRepository — every overridden path succeeds inside withRequestContext', () => {
|
||||
it('TREKREPO-013: findOne/find/findAll/count/nativeUpdate/nativeDelete/insert/upsert/qb/kysely all succeed', async () => {
|
||||
const { user } = createUser(testDb, { username: 'inside-before', email: 'inside-before@example.test' });
|
||||
|
||||
await withRequestContext(t.orm, async () => {
|
||||
await expect(users.findOne({ id: user.id })).resolves.not.toBeNull();
|
||||
await expect(users.find({ id: user.id })).resolves.toHaveLength(1);
|
||||
await expect(users.findAll({ limit: 1 })).resolves.toHaveLength(1);
|
||||
await expect(users.count({})).resolves.toBeGreaterThan(0);
|
||||
await expect(users.nativeUpdate({ id: user.id }, { username: 'inside-after' })).resolves.toBe(1);
|
||||
|
||||
const qbRow = await users
|
||||
.qb('u')
|
||||
.select('u.id')
|
||||
.where({ id: user.id })
|
||||
.execute<{ id: number } | undefined>('get', false);
|
||||
expect(qbRow?.id).toBe(user.id);
|
||||
|
||||
const cbRow = await users
|
||||
.createQueryBuilder('u')
|
||||
.select('u.id')
|
||||
.where({ id: user.id })
|
||||
.execute<{ id: number } | undefined>('get', false);
|
||||
expect(cbRow?.id).toBe(user.id);
|
||||
|
||||
// Kysely: proving the statement runs at all (no throw), not its result shape.
|
||||
await expect(challenges.claimChallenge('inside-context', 'registration', Date.now())).resolves.toBeNull();
|
||||
|
||||
const inserted = await users.insert({
|
||||
username: 'inside-new',
|
||||
email: 'inside-new@example.test',
|
||||
password_hash: 'h',
|
||||
role: 'user',
|
||||
first_seen_version: '1.0',
|
||||
});
|
||||
expect(typeof inserted).toBe('number');
|
||||
|
||||
await users.upsert(
|
||||
{ id: user.id, username: 'inside-upserted', email: user.email, password_hash: 'h', role: 'user', first_seen_version: '1.0' },
|
||||
{ onConflictFields: ['id'], onConflictAction: 'merge' },
|
||||
);
|
||||
|
||||
expect(users.getEntityManager()).toBeDefined();
|
||||
|
||||
await users.nativeDelete({ id: inserted as number });
|
||||
});
|
||||
|
||||
const row = testDb.prepare('SELECT username FROM users WHERE id = ?').get(user.id) as { username: string };
|
||||
expect(row.username).toBe('inside-upserted'); // the upsert's merge is the last write and must stick
|
||||
});
|
||||
});
|
||||
|
||||
describe('TrekRepository — inside uow.transactional', () => {
|
||||
it('TREKREPO-014: a disableIdentityMap read sees the transaction\'s own uncommitted write', async () => {
|
||||
const { user } = createUser(testDb, { username: 'tx-before' });
|
||||
let seenInsideTx: string | undefined;
|
||||
|
||||
await withRequestContext(t.orm, async () => {
|
||||
await uow.transactional(async () => {
|
||||
await users.nativeUpdate({ id: user.id }, { username: 'tx-uncommitted' });
|
||||
const row = await users.findOne({ id: user.id }, { fields: ['username'] });
|
||||
seenInsideTx = row?.username;
|
||||
});
|
||||
});
|
||||
|
||||
expect(seenInsideTx).toBe('tx-uncommitted');
|
||||
});
|
||||
|
||||
it('TREKREPO-015: disableIdentityMap is in effect by default — a raw UPDATE between two reads is seen, no stale write-back on the closing flush (reuses identity-map-writeback.test.ts\'s shape)', async () => {
|
||||
const { user } = createUser(testDb, { username: 'before', email: 'before@example.test' });
|
||||
|
||||
await withRequestContext(t.orm, async () => {
|
||||
// Two differently-projected reads of the same row — the D-shape B1
|
||||
// depends on, if the entity were left managed.
|
||||
await users.findOne({ id: user.id }, { fields: ['username'] });
|
||||
await users.findOne({ id: user.id }, { fields: ['email'] });
|
||||
await uow.transactional(async () => {
|
||||
await users.nativeUpdate({ id: user.id }, { username: 'after' });
|
||||
});
|
||||
});
|
||||
|
||||
const row = testDb.prepare('SELECT username, email FROM users WHERE id = ?').get(user.id) as { username: string; email: string };
|
||||
expect(row.username).toBe('after'); // the nativeUpdate must stick
|
||||
expect(row.email).toBe('before@example.test'); // untouched by either read
|
||||
});
|
||||
});
|
||||
@@ -207,6 +207,31 @@ describe('CronRegistrarService', () => {
|
||||
expect(caught).toBeUndefined();
|
||||
expect(Array.isArray(result)).toBe(true);
|
||||
});
|
||||
|
||||
// Plan 3b interlude B mutation proof (b): the SAME wrapper, but through a
|
||||
// real TrekRepository-backed call rather than a bare `em.find` — proving
|
||||
// the wrapper is load-bearing for the base class's write paths too
|
||||
// (`count`/`nativeUpdate`/`nativeDelete`/`insert`/`upsert`), which never
|
||||
// validated their own context before this class existed
|
||||
// (task-1-review.md F7 INFO) and so would NOT have failed this way on
|
||||
// the pre-interlude-B tree.
|
||||
it('CRONREG-011B — with MikroORM injected, a TrekRepository-backed count() inside onTick succeeds; dropping the wrapper (mutation, not committed) makes it throw cannotUseGlobalContext', async () => {
|
||||
const registry = new SchedulerRegistry();
|
||||
const registrar = new CronRegistrarService(registry, { isTest: () => false } as RuntimeEnvService, t.orm);
|
||||
const users = t.repo(Users);
|
||||
let result: unknown;
|
||||
let caught: unknown;
|
||||
registrar.register('job', '0 2 * * *', async () => {
|
||||
try {
|
||||
result = await users.count({});
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
});
|
||||
await h.jobs[0].onTick();
|
||||
expect(caught).toBeUndefined();
|
||||
expect(typeof result).toBe('number');
|
||||
});
|
||||
});
|
||||
|
||||
describe('runOnBoot (task-6-fix-brief.md item 7 — the boot-sweep choke point)', () => {
|
||||
|
||||
@@ -75,13 +75,20 @@ import type { TripFile, User } from '../../../src/types';
|
||||
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service';
|
||||
import type { EntityManager } from '@mikro-orm/core';
|
||||
import { Users } from '../../../src/db/entities/Users.entity';
|
||||
|
||||
const storageDelete = vi.fn();
|
||||
const storageStub = { delete: storageDelete } as unknown as import('../../../src/nest/storage/storage.service').StorageService;
|
||||
// EntityManager stub (Plan 3b Task 1 RULING): verifyJwtAndLoadUser is
|
||||
// fully mocked above, so `em.getRepository` never needs to return
|
||||
// anything meaningful; it just has to not throw when the service calls it.
|
||||
const emStub = { getRepository: () => ({}) } as unknown as EntityManager;
|
||||
// `getRepository` is a spy, not a plain arrow (task-1-rereview.md F-R4): a
|
||||
// fixed `() => ({})` pinned only the RETURN value, not which entity class
|
||||
// `files.service.ts` asks for — the assertion below would hold even if the
|
||||
// production code asked for `Trips`. The spy lets FILE-SVC-032/033 pin
|
||||
// `Users` specifically.
|
||||
const getRepository = vi.fn(() => ({}));
|
||||
const emStub = { getRepository } as unknown as EntityManager;
|
||||
const svc = new FilesService(new DatabaseService(testDb), permissionsStub, new RealtimeService(), new EphemeralTokenService(), storageStub, emStub);
|
||||
|
||||
beforeAll(() => {
|
||||
@@ -564,13 +571,18 @@ describe('authenticateDownload', () => {
|
||||
// always returns a fresh `{}` (task-1-review.md F6) — deep-equal, not a
|
||||
// reference match, so this pins WHICH repository (an empty object, this
|
||||
// suite's stand-in for UsersRepository), not merely "a repository".
|
||||
// `getRepository` itself is a spy (task-1-rereview.md F-R4), so this ALSO
|
||||
// pins which entity class was asked for: the deep-equal alone would still
|
||||
// hold if the production code called `em.getRepository(Trips)`.
|
||||
expect(verifyJwtAndLoadUser).toHaveBeenCalledWith('cookie-jwt', {});
|
||||
expect(getRepository).toHaveBeenCalledWith(Users);
|
||||
});
|
||||
|
||||
it('FILE-SVC-033: a bearer token is used when no cookie is present; invalid JWTs 401', async () => {
|
||||
verifyJwtAndLoadUser.mockReturnValue({ id: 7 });
|
||||
expect(await svc.authenticateDownload(req({ bearer: 'bearer-jwt' }))).toEqual({ userId: 7 });
|
||||
expect(verifyJwtAndLoadUser).toHaveBeenCalledWith('bearer-jwt', {}); // see FILE-SVC-032's comment
|
||||
expect(getRepository).toHaveBeenCalledWith(Users);
|
||||
|
||||
verifyJwtAndLoadUser.mockReturnValue(null);
|
||||
expect(await svc.authenticateDownload(req({ bearer: 'stale' }))).toEqual({ error: 'Invalid or expired token', status: 401 });
|
||||
|
||||
@@ -726,10 +726,19 @@ describe('renamePasskey', () => {
|
||||
expect(await svc.renamePasskey(user.id, '999999', 'Ghost')).toEqual({ error: 'Passkey not found', status: 404 });
|
||||
});
|
||||
|
||||
it('PASSKEY-SVC-042: 404s (not a 500) on a non-numeric or prefixed-numeric id — Plan 3b Task 3 review, F1', async () => {
|
||||
it('PASSKEY-SVC-042: 404s (not a 500) on a non-numeric id — Plan 3b Task 3 review, F1', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
expect(await svc.renamePasskey(user.id, 'abc', 'Ghost')).toEqual({ error: 'Passkey not found', status: 404 });
|
||||
expect(await svc.renamePasskey(user.id, '1abc', 'Ghost')).toEqual({ error: 'Passkey not found', status: 404 });
|
||||
});
|
||||
|
||||
// task-3-rereview.md R1: the legacy `renamePasskey` bound `Number(id)`
|
||||
// (unlike the invite route's raw-string bind), so `Number('0x10') === 16`
|
||||
// — the legacy statement would have ACTED on credential 16, not 404'd.
|
||||
// This pins the deliberate `toRowId` narrowing the fix round chose
|
||||
// (accepted deviation, not parity), not "the legacy 404".
|
||||
it('PASSKEY-SVC-042B: refuses a prefixed-numeric id (0x10) via the toRowId narrowing', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
expect(await svc.renamePasskey(user.id, '0x10', 'Ghost')).toEqual({ error: 'Passkey not found', status: 404 });
|
||||
});
|
||||
});
|
||||
@@ -759,6 +768,13 @@ describe('deletePasskey', () => {
|
||||
it('PASSKEY-SVC-043: 404s (not a 500) on a non-numeric id once the password check passes — Plan 3b Task 3 review, F1', async () => {
|
||||
const { user, password } = createUser(testDb);
|
||||
expect(await svc.deletePasskey(user.id, 'abc', password)).toEqual({ error: 'Passkey not found', status: 404 });
|
||||
});
|
||||
|
||||
// task-3-rereview.md R1: same distinction as PASSKEY-SVC-042B —
|
||||
// `deletePasskey`'s legacy bind was `Number(id)`, so `'0x10'` would have
|
||||
// acted on credential 16, not 404'd. This pins the `toRowId` narrowing.
|
||||
it('PASSKEY-SVC-043B: refuses a prefixed-numeric id (0x10) via the toRowId narrowing, once the password check passes', async () => {
|
||||
const { user, password } = createUser(testDb);
|
||||
expect(await svc.deletePasskey(user.id, '0x10', password)).toEqual({ error: 'Passkey not found', status: 404 });
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user