Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .server-changes/throttle-token-last-accessed-at.md
Original file line number Diff line number Diff line change
@@ -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.
18 changes: 17 additions & 1 deletion apps/webapp/app/services/organizationAccessToken.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) } },
],
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
data: {
lastAccessedAt: new Date(),
Expand Down
18 changes: 17 additions & 1 deletion apps/webapp/app/services/personalAccessToken.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) } },
],
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
data: {
lastAccessedAt: new Date(),
Expand Down
89 changes: 89 additions & 0 deletions apps/webapp/test/services/organizationAccessToken.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
96 changes: 96 additions & 0 deletions apps/webapp/test/services/personalAccessToken.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
Loading