fix(webapp,rbac): keep revoked personal access tokens revoked

Revoking a personal access token is now permanent. Logging in with the
CLI reuses your existing token only while it's still live; if you've
revoked it, the next login issues a new one and the revoked token stays
rejected. Previously a CLI login brought the revoked token back with the
same secret, so a leaked CLI token couldn't be rotated. To rotate one,
revoke it on the tokens page, then run `trigger logout` and `trigger
login`.

Also in this change:

- The endpoint the CLI polls during login no longer returns a token that
was revoked after you approved the login.
- Environment-scoped API access re-checks that the personal access token
behind a delegated token is still live.
- Approving a CLI login now respects your session duration setting: an
expired session has to sign in again first.

Mono-RevId: 22439c1ab597029283c471ba36fb40246d960531

Author: ericallam

.server-changes/permanent-personal-access-token-revocation.md

@@ -0,0 +1,6 @@
+---
+area: webapp
+type: fix
+---
+
+Revoking a personal access token is now permanent. Logging in with the CLI again issues a new token instead of reinstating the revoked one, so you can rotate a leaked CLI token by revoking it and running `trigger logout` then `trigger login`.

apps/webapp/app/routes/account.authorization-code.$authorizationCode/route.tsx

@@ -15,7 +15,7 @@ import {
   createPersonalAccessTokenFromAuthorizationCode,
   isAuthorizationCodeMintable,
 } from "~/services/personalAccessToken.server";
-import { requireUser, requireUserId } from "~/services/session.server";
+import { requireUser } from "~/services/session.server";
 import { pageMeta } from "~/utils/pageTitle";
 
 export const meta = pageMeta("Authorize login");
@@ -89,7 +89,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
 };
 
 export const action = async ({ request, params }: ActionFunctionArgs) => {
-  const userId = await requireUserId(request);
+  const { id: userId } = await requireUser(request);
 
   const { authorizationCode } = parseParams(params);
   const { source, clientName } = parseSearch(request);

apps/webapp/app/services/environmentVariableApiAccess.server.ts

@@ -9,6 +9,7 @@ import {
   type AuthenticationResult,
   type ScopedApiKeyAuthenticationDependencies,
 } from "~/services/apiAuth.server";
+import { resolveAndRecheckUserActorClaims } from "~/services/personalAccessToken.server";
 import { rbac } from "~/services/rbac.server";
 
 type EnvironmentScopedResource = "envvars" | "apiKeys" | "deployments" | "branches";
@@ -127,6 +128,11 @@ const RESOURCE_LABELS: Record<EnvironmentScopedResource, string> = {
  * stops a restricted role deploying via the CLI (deploy needs the
  * environment's secret key).
  *
+ * A user-actor token is stateless and lives for up to seven days, so its source
+ * personal access token is re-checked here on every call: revoking that PAT is
+ * the only way to retire the token, and this helper must not rely on a route
+ * preamble having done the check already.
+ *
  * Returns a `Response` to short-circuit with when access is denied, or
  * `undefined` when the request may proceed.
  */
@@ -156,7 +162,7 @@ export async function authorizePatEnvironmentAccess({
     .get("Authorization")
     ?.replace(/^Bearer /, "")
     .trim();
-  const isUat = !!bearer && isUserActorToken(bearer);
+  const userActorBearer = bearer && isUserActorToken(bearer) ? bearer : undefined;
 
   // Machine API keys are authorized by their controller ability. Root keys and
   // ungranted additional keys are permissive; granted keys are restricted.
@@ -174,17 +180,27 @@ export async function authorizePatEnvironmentAccess({
 
   // Org tokens carry no user role to enforce. A user-actor token carries a
   // user just like a PAT, so it's gated too.
-  if (authType !== "personalAccessToken" && !isUat) {
+  if (authType !== "personalAccessToken" && !userActorBearer) {
     return undefined;
   }
 
-  const userAuth = isUat
+  const userAuth = userActorBearer
     ? await rbac.authenticateUserActor(request, { organizationId, projectId })
     : await rbac.authenticatePat(request, { organizationId, projectId });
   if (!userAuth.ok) {
     return json({ error: userAuth.error }, { status: userAuth.status });
   }
 
+  if (
+    userActorBearer &&
+    !(await resolveAndRecheckUserActorClaims(
+      "claims" in userAuth ? userAuth.claims : undefined,
+      userActorBearer
+    ))
+  ) {
+    return json({ error: "Invalid user-actor token" }, { status: 401 });
+  }
+
   if (!resources.some((candidate) => userAuth.ability.can(action, { type: candidate, envType }))) {
     return json(
       {

apps/webapp/app/services/personalAccessToken.server.ts

@@ -77,7 +77,7 @@ export async function getPersonalAccessTokenFromAuthorizationCode(authorizationC
       },
     },
   });
-  if (!code) {
+  if (!code || code.personalAccessToken?.revokedAt) {
     throw new Error("Invalid authorization code, or code expired");
   }
 
@@ -379,6 +379,10 @@ export async function createPersonalAccessTokenFromAuthorizationCode(
     where: {
       userId,
       name: "cli",
+      revokedAt: null,
+    },
+    orderBy: {
+      createdAt: "desc",
     },
   });
 
@@ -394,18 +398,6 @@ export async function createPersonalAccessTokenFromAuthorizationCode(
       },
     });
 
-    if (existingCliPersonalAccessToken.revokedAt) {
-      // re-activate revoked CLI PAT so we can use it again
-      await prisma.personalAccessToken.update({
-        where: {
-          id: existingCliPersonalAccessToken.id,
-        },
-        data: {
-          revokedAt: null,
-        },
-      });
-    }
-
     //we don't return the decrypted token
     return {
       id: existingCliPersonalAccessToken.id,

apps/webapp/test/cliPersonalAccessTokenRevocation.test.ts

@@ -0,0 +1,200 @@
+import { beforeEach, describe, expect, vi } from "vitest";
+
+/**
+ * Revoking a CLI personal access token has to be permanent. Logging in again mints a new secret;
+ * it never reinstates the revoked row, and the revoked string never authenticates or gets handed
+ * back out over the unauthenticated token endpoint.
+ */
+
+const db = vi.hoisted(() => ({ client: null as any }));
+
+vi.mock("~/db.server", () => ({
+  get prisma() {
+    return db.client;
+  },
+  get $replica() {
+    return db.client;
+  },
+}));
+vi.mock("~/env.server", () => ({
+  env: {
+    SESSION_SECRET: "test-session-secret-for-cli-pat-revocation",
+    ENCRYPTION_KEY: "0123456789abcdef0123456789abcdef",
+  },
+}));
+vi.mock("~/services/logger.server", () => ({
+  logger: { debug: vi.fn(), error: vi.fn(), warn: vi.fn(), info: vi.fn() },
+}));
+vi.mock("~/services/rbac.server", () => ({
+  rbac: { isUsingPlugin: async () => false, setTokenRole: async () => ({ ok: true }) },
+}));
+
+import { postgresTest } from "@internal/testcontainers";
+import {
+  authenticatePersonalAccessToken,
+  createAuthorizationCode,
+  createPersonalAccessToken,
+  createPersonalAccessTokenFromAuthorizationCode,
+  getPersonalAccessTokenFromAuthorizationCode,
+  revokePersonalAccessToken,
+} from "~/services/personalAccessToken.server";
+
+vi.setConfig({ testTimeout: 60_000 });
+
+let userCounter = 0;
+
+async function createUser(prisma: any) {
+  userCounter += 1;
+  return prisma.user.create({
+    data: {
+      email: `cli-pat-revocation-${userCounter}-${Date.now()}@example.com`,
+      authenticationMethod: "MAGIC_LINK",
+    },
+  });
+}
+
+/** Runs the consent-screen mint the CLI login flow triggers, and returns what it minted. */
+async function cliLogin(userId: string) {
+  const code = await createAuthorizationCode();
+  const minted = await createPersonalAccessTokenFromAuthorizationCode(code.code, userId);
+  return { code: code.code, minted };
+}
+
+function secretOf(minted: unknown): string {
+  return (minted as { token: string }).token;
+}
+
+describe("CLI personal access token revocation", () => {
+  beforeEach(() => {
+    db.client = null;
+  });
+
+  postgresTest("a later login reuses a live CLI token", async ({ prisma }) => {
+    db.client = prisma;
+    const user = await createUser(prisma);
+
+    const first = await cliLogin(user.id);
+    const second = await cliLogin(user.id);
+
+    expect(second.minted.id).toBe(first.minted.id);
+    expect(await prisma.personalAccessToken.count({ where: { userId: user.id } })).toBe(1);
+  });
+
+  postgresTest(
+    "a revoked CLI token is not revived by a later login, and its secret stays rejected",
+    async ({ prisma }) => {
+      db.client = prisma;
+      const user = await createUser(prisma);
+
+      const first = await cliLogin(user.id);
+      const revokedSecret = secretOf(first.minted);
+      expect(await authenticatePersonalAccessToken(revokedSecret)).toMatchObject({
+        userId: user.id,
+      });
+
+      await revokePersonalAccessToken(first.minted.id, user.id);
+
+      const second = await cliLogin(user.id);
+      const freshSecret = secretOf(second.minted);
+
+      expect(second.minted.id).not.toBe(first.minted.id);
+      expect(freshSecret).not.toBe(revokedSecret);
+
+      const revokedRow = await prisma.personalAccessToken.findFirst({
+        where: { id: first.minted.id },
+      });
+      expect(revokedRow?.revokedAt).toBeInstanceOf(Date);
+      expect(revokedRow?.name).toBe("cli");
+
+      expect(await authenticatePersonalAccessToken(revokedSecret)).toBeUndefined();
+      expect(await authenticatePersonalAccessToken(freshSecret)).toMatchObject({
+        userId: user.id,
+      });
+    }
+  );
+
+  postgresTest("revoking again after re-login leaves both secrets dead", async ({ prisma }) => {
+    db.client = prisma;
+    const user = await createUser(prisma);
+
+    const first = await cliLogin(user.id);
+    await revokePersonalAccessToken(first.minted.id, user.id);
+
+    const second = await cliLogin(user.id);
+    await revokePersonalAccessToken(second.minted.id, user.id);
+
+    const third = await cliLogin(user.id);
+
+    expect(third.minted.id).not.toBe(first.minted.id);
+    expect(third.minted.id).not.toBe(second.minted.id);
+    expect(await authenticatePersonalAccessToken(secretOf(first.minted))).toBeUndefined();
+    expect(await authenticatePersonalAccessToken(secretOf(second.minted))).toBeUndefined();
+  });
+
+  postgresTest(
+    "a token revoked under a different name does not block a fresh CLI login",
+    async ({ prisma }) => {
+      db.client = prisma;
+      const user = await createUser(prisma);
+
+      const dashboardToken = await createPersonalAccessToken({ name: "laptop", userId: user.id });
+      await revokePersonalAccessToken(dashboardToken.id, user.id);
+
+      const login = await cliLogin(user.id);
+
+      expect(login.minted.id).not.toBe(dashboardToken.id);
+      expect(await authenticatePersonalAccessToken(dashboardToken.token)).toBeUndefined();
+    }
+  );
+});
+
+describe("the authorization code token endpoint", () => {
+  beforeEach(() => {
+    db.client = null;
+  });
+
+  postgresTest("discloses the token it minted", async ({ prisma }) => {
+    db.client = prisma;
+    const user = await createUser(prisma);
+
+    const login = await cliLogin(user.id);
+
+    const disclosed = await getPersonalAccessTokenFromAuthorizationCode(login.code);
+    expect(disclosed.token?.token).toBe(secretOf(login.minted));
+  });
+
+  postgresTest(
+    "stops disclosing a token that was revoked after the code was bound",
+    async ({ prisma }) => {
+      db.client = prisma;
+      const user = await createUser(prisma);
+
+      const login = await cliLogin(user.id);
+      await revokePersonalAccessToken(login.minted.id, user.id);
+
+      await expect(getPersonalAccessTokenFromAuthorizationCode(login.code)).rejects.toThrow(
+        "Invalid authorization code, or code expired"
+      );
+    }
+  );
+
+  postgresTest("discloses the live token a later login reused", async ({ prisma }) => {
+    db.client = prisma;
+    const user = await createUser(prisma);
+
+    const first = await cliLogin(user.id);
+    const second = await cliLogin(user.id);
+
+    const disclosed = await getPersonalAccessTokenFromAuthorizationCode(second.code);
+    expect(disclosed.token?.token).toBe(secretOf(first.minted));
+  });
+
+  postgresTest("reports an unapproved code as not ready yet", async ({ prisma }) => {
+    db.client = prisma;
+
+    const code = await createAuthorizationCode();
+
+    const disclosed = await getPersonalAccessTokenFromAuthorizationCode(code.code);
+    expect(disclosed.token).toBeNull();
+  });
+});

apps/webapp/test/dashboardAgentRoutes.test.ts

@@ -62,6 +62,7 @@ vi.mock("~/services/rbac.server", () => ({
     authenticateUserActor: async () => ({
       ok: true,
       userId: "usr_1",
+      claims: { userId: "usr_1" },
       ability: { can: () => true, canSuper: () => true },
     }),
     authenticatePat: async () => ({