diff --git a/.server-changes/throttle-token-last-accessed-at.md b/.server-changes/throttle-token-last-accessed-at.md new file mode 100644 index 00000000000..53379622cc3 --- /dev/null +++ b/.server-changes/throttle-token-last-accessed-at.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: improvement +--- + +Throttle `PersonalAccessToken.lastAccessedAt` and `OrganizationAccessToken.lastAccessedAt` writes to at most once per 5 minutes per token. Eliminates ~95% of writes on two narrow hot tables that were autovacuuming every ~5 minutes — same denormalization-on-the-hot-path shape as the schedule engine fix in TRI-8891. The settings UI continues to display "last used" with at most 5-minute lag. diff --git a/apps/webapp/app/services/organizationAccessToken.server.ts b/apps/webapp/app/services/organizationAccessToken.server.ts index ba11374c24d..77519ef8d1f 100644 --- a/apps/webapp/app/services/organizationAccessToken.server.ts +++ b/apps/webapp/app/services/organizationAccessToken.server.ts @@ -8,6 +8,12 @@ const tokenValueLength = 40; //lowercase only, removed 0 and l to avoid confusion const tokenGenerator = customAlphabet("123456789abcdefghijkmnopqrstuvwxyz", tokenValueLength); +// Skip the lastAccessedAt write if the existing value is already within this +// window. Eliminates per-auth UPDATE churn on a small narrow hot table; the +// settings UI reads this field at human granularity so a few-minute +// staleness is fine. +export const OAT_LAST_ACCESSED_THROTTLE_MS = 5 * 60 * 1000; + type CreateOrganizationAccessTokenOptions = { name: string; organizationId: string; @@ -105,9 +111,19 @@ export async function authenticateOrganizationAccessToken( return; } - await prisma.organizationAccessToken.update({ + // Conditional updateMany — only writes if the existing lastAccessedAt is + // null or older than the throttle window. The WHERE runs inside the UPDATE + // so concurrent auths don't race into a double-write. `revokedAt: null` + // matches the findFirst guard above so a token revoked between the read + // and write doesn't get a stale lastAccessedAt update. + await prisma.organizationAccessToken.updateMany({ where: { id: organizationAccessToken.id, + revokedAt: null, + OR: [ + { lastAccessedAt: null }, + { lastAccessedAt: { lt: new Date(Date.now() - OAT_LAST_ACCESSED_THROTTLE_MS) } }, + ], }, data: { lastAccessedAt: new Date(), diff --git a/apps/webapp/app/services/personalAccessToken.server.ts b/apps/webapp/app/services/personalAccessToken.server.ts index cceb576c9d8..e781cdfeb7a 100644 --- a/apps/webapp/app/services/personalAccessToken.server.ts +++ b/apps/webapp/app/services/personalAccessToken.server.ts @@ -10,6 +10,12 @@ const tokenValueLength = 40; //lowercase only, removed 0 and l to avoid confusion const tokenGenerator = customAlphabet("123456789abcdefghijkmnopqrstuvwxyz", tokenValueLength); +// Skip the lastAccessedAt write if the existing value is already within this +// window. Eliminates per-auth UPDATE churn on a small narrow hot table; the +// /account/tokens UI reads this field at human granularity so a few-minute +// staleness is fine. +export const PAT_LAST_ACCESSED_THROTTLE_MS = 5 * 60 * 1000; + type CreatePersonalAccessTokenOptions = { name: string; userId: string; @@ -205,9 +211,19 @@ export async function authenticatePersonalAccessToken( return; } - await prisma.personalAccessToken.update({ + // Conditional updateMany — only writes if the existing lastAccessedAt is + // null or older than the throttle window. The WHERE runs inside the UPDATE + // so concurrent auths don't race into a double-write. `revokedAt: null` + // matches the findFirst guard above so a token revoked between the read + // and write doesn't get a stale lastAccessedAt update. + await prisma.personalAccessToken.updateMany({ where: { id: personalAccessToken.id, + revokedAt: null, + OR: [ + { lastAccessedAt: null }, + { lastAccessedAt: { lt: new Date(Date.now() - PAT_LAST_ACCESSED_THROTTLE_MS) } }, + ], }, data: { lastAccessedAt: new Date(), diff --git a/apps/webapp/test/services/organizationAccessToken.test.ts b/apps/webapp/test/services/organizationAccessToken.test.ts new file mode 100644 index 00000000000..cf3900f4c47 --- /dev/null +++ b/apps/webapp/test/services/organizationAccessToken.test.ts @@ -0,0 +1,89 @@ +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: { + organizationAccessToken: { + findFirst: findFirstMock, + updateMany: updateManyMock, + }, + }, + $replica: {}, +})); + +vi.mock("~/utils/tokens.server", () => ({ + hashToken: (t: string) => `hashed:${t}`, +})); + +vi.mock("./logger.server", () => ({ + logger: { warn: vi.fn(), error: vi.fn() }, +})); + +import { + authenticateOrganizationAccessToken, + OAT_LAST_ACCESSED_THROTTLE_MS, +} from "~/services/organizationAccessToken.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("authenticateOrganizationAccessToken — lastAccessedAt throttle", () => { + test("issues a conditional updateMany that skips writes when lastAccessedAt is recent", async () => { + findFirstMock.mockResolvedValueOnce({ + id: "oat_123", + organizationId: "org_1", + hashedToken: "hashed:tr_oat_validtoken", + }); + + const result = await authenticateOrganizationAccessToken("tr_oat_validtoken"); + + expect(result).toEqual({ organizationId: "org_1" }); + expect(updateManyMock).toHaveBeenCalledTimes(1); + + const call = updateManyMock.mock.calls[0][0]; + expect(call.where.id).toBe("oat_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() - OAT_LAST_ACCESSED_THROTTLE_MS); + }); + + test("skips updateMany when token is not found", async () => { + findFirstMock.mockResolvedValueOnce(null); + + const result = await authenticateOrganizationAccessToken("tr_oat_validtoken"); + + expect(result).toBeUndefined(); + expect(updateManyMock).not.toHaveBeenCalled(); + }); + + test("skips updateMany when token doesn't start with prefix", async () => { + const result = await authenticateOrganizationAccessToken("not_an_oat"); + + expect(result).toBeUndefined(); + expect(findFirstMock).not.toHaveBeenCalled(); + expect(updateManyMock).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/webapp/test/services/personalAccessToken.test.ts b/apps/webapp/test/services/personalAccessToken.test.ts new file mode 100644 index 00000000000..0b52a7bf706 --- /dev/null +++ b/apps/webapp/test/services/personalAccessToken.test.ts @@ -0,0 +1,96 @@ +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(); + }); +});