Eric Allam
04bdf4b90b
## Summary
Each successful PAT (`PersonalAccessToken`) or OAT
(`OrganizationAccessToken`) authentication issues a `prisma.X.update({
lastAccessedAt: new Date() })` to bump the timestamp. For tokens used at
high frequency (CLI clients, integrations) this generates a per-request
DB write that is mostly redundant — the `lastAccessedAt` field is only
surfaced on the settings page so users can decide which tokens to
revoke, and "within the last 5 minutes" is plenty of granularity for
that.
## Design
Replace each unconditional `update` with a conditional `updateMany`
whose `WHERE` requires the existing `lastAccessedAt` to be `NULL` or
strictly older than 5 minutes:
```ts
await prisma.personalAccessToken.updateMany({
where: {
id: personalAccessToken.id,
OR: [
{ lastAccessedAt: null },
{ lastAccessedAt: { lt: new Date(Date.now() - PAT_LAST_ACCESSED_THROTTLE_MS) } },
],
},
data: { lastAccessedAt: new Date() },
});
```
The conditional runs inside the SQL `UPDATE`, so concurrent auths can't
race into a double-write.
No schema change. No migration. No new infrastructure. Throttle is a
hardcoded constant (`5 * 60 * 1000`) — easy to revisit.
## Test plan
- [x] `pnpm run typecheck --filter webapp`
- [x] `pnpm vitest run ./test/services/personalAccessToken.test.ts
./test/services/organizationAccessToken.test.ts` — 6/6 pass, verifying
the throttle `WHERE` clause is constructed correctly and the `update` is
skipped on token-not-found / wrong-prefix paths
97 lines
2.9 KiB
TypeScript
97 lines
2.9 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
|
|
|
|
const { findFirstMock, updateManyMock } = vi.hoisted(() => ({
|
|
findFirstMock: vi.fn(),
|
|
updateManyMock: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("~/db.server", () => ({
|
|
prisma: {
|
|
personalAccessToken: {
|
|
findFirst: findFirstMock,
|
|
updateMany: updateManyMock,
|
|
},
|
|
},
|
|
$replica: {},
|
|
}));
|
|
|
|
vi.mock("~/env.server", () => ({
|
|
env: { ENCRYPTION_KEY: "0".repeat(64) },
|
|
}));
|
|
|
|
vi.mock("~/utils/tokens.server", () => ({
|
|
hashToken: (t: string) => `hashed:${t}`,
|
|
encryptToken: () => ({ nonce: "n", ciphertext: "c", tag: "t" }),
|
|
decryptToken: () => "tr_pat_validtoken",
|
|
}));
|
|
|
|
vi.mock("./logger.server", () => ({
|
|
logger: { warn: vi.fn(), error: vi.fn() },
|
|
}));
|
|
|
|
import {
|
|
authenticatePersonalAccessToken,
|
|
PAT_LAST_ACCESSED_THROTTLE_MS,
|
|
} from "~/services/personalAccessToken.server";
|
|
|
|
beforeEach(() => {
|
|
vi.useFakeTimers();
|
|
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
|
findFirstMock.mockReset();
|
|
updateManyMock.mockReset();
|
|
updateManyMock.mockResolvedValue({ count: 1 });
|
|
});
|
|
|
|
afterEach(() => {
|
|
vi.useRealTimers();
|
|
});
|
|
|
|
describe("authenticatePersonalAccessToken — lastAccessedAt throttle", () => {
|
|
test("issues a conditional updateMany that skips writes when lastAccessedAt is recent", async () => {
|
|
findFirstMock.mockResolvedValueOnce({
|
|
id: "pat_123",
|
|
userId: "user_1",
|
|
hashedToken: "hashed:tr_pat_validtoken",
|
|
encryptedToken: { nonce: "n", ciphertext: "c", tag: "t" },
|
|
});
|
|
|
|
const result = await authenticatePersonalAccessToken("tr_pat_validtoken");
|
|
|
|
expect(result).toEqual({ userId: "user_1" });
|
|
expect(updateManyMock).toHaveBeenCalledTimes(1);
|
|
|
|
const call = updateManyMock.mock.calls[0][0];
|
|
expect(call.where.id).toBe("pat_123");
|
|
expect(call.where.revokedAt).toBeNull();
|
|
expect(call.data.lastAccessedAt).toBeInstanceOf(Date);
|
|
|
|
// The WHERE clause should require the existing lastAccessedAt to be null
|
|
// or strictly older than the throttle window — that's the entire point.
|
|
expect(call.where.OR).toEqual([
|
|
{ lastAccessedAt: null },
|
|
{ lastAccessedAt: { lt: expect.any(Date) } },
|
|
]);
|
|
|
|
// With fake timers, the cutoff lands exactly throttle-ms before "now".
|
|
const cutoff = call.where.OR[1].lastAccessedAt.lt as Date;
|
|
expect(cutoff.getTime()).toBe(Date.now() - PAT_LAST_ACCESSED_THROTTLE_MS);
|
|
});
|
|
|
|
test("skips updateMany when token is not found", async () => {
|
|
findFirstMock.mockResolvedValueOnce(null);
|
|
|
|
const result = await authenticatePersonalAccessToken("tr_pat_validtoken");
|
|
|
|
expect(result).toBeUndefined();
|
|
expect(updateManyMock).not.toHaveBeenCalled();
|
|
});
|
|
|
|
test("skips updateMany when token doesn't start with prefix", async () => {
|
|
const result = await authenticatePersonalAccessToken("not_a_pat");
|
|
|
|
expect(result).toBeUndefined();
|
|
expect(findFirstMock).not.toHaveBeenCalled();
|
|
expect(updateManyMock).not.toHaveBeenCalled();
|
|
});
|
|
});
|