+ Could not disconnect {failedTitle ?? "that account"}:{" "}
{unlink.error.message}
) : null}
diff --git a/app/src/lib/identity/queries.ts b/app/src/lib/identity/queries.ts
index 57e97fc93..95b8449cf 100644
--- a/app/src/lib/identity/queries.ts
+++ b/app/src/lib/identity/queries.ts
@@ -1,5 +1,5 @@
import { queryOptions } from "@tanstack/react-query";
-import { client } from "@/lib/client";
+import { tryClient } from "@/lib/client";
export type LinkedAccount = {
id: string;
@@ -15,12 +15,34 @@ export const identityKeys = {
links: () => [...identityKeys.all, "links"] as const,
};
+const LOAD_FALLBACK = "Could not load your linked accounts";
+
+/**
+ * A deployment without an identity store answers 503 "not available". That is the feature being
+ * absent, not the server breaking, so it reads as no links and the section hides itself.
+ */
+export async function linksFromResponse(
+ response: Response,
+): Promise {
+ if (response.status === 503) return [];
+ if (!response.ok) {
+ const message = await response
+ .json()
+ .then((body: { error?: string }) => body.error)
+ .catch(() => undefined);
+ throw new Error(message ?? LOAD_FALLBACK);
+ }
+ const body = (await response.json().catch(() => null)) as {
+ links?: LinkedAccount[];
+ } | null;
+ if (!body || !Array.isArray(body.links)) throw new Error(LOAD_FALLBACK);
+ return body.links;
+}
+
export function linkedAccountsQueryOptions() {
return queryOptions({
queryKey: identityKeys.links(),
queryFn: async (): Promise =>
- client("/api/identity/links", "links", {
- fallback: "Could not load your linked accounts",
- }),
+ linksFromResponse(await tryClient("/api/identity/links")),
});
}
From 93a2a621887a55bf61e8ddfeff5a69e6b54a04c1 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:16:25 -0300
Subject: [PATCH 030/101] fix(server): audit disconnecting a linked account
---
server/src/app.ts | 2 +-
server/src/audit.ts | 2 ++
server/src/identity/routes.ts | 27 ++++++++++++++++--
server/tests/identity-routes.test.ts | 41 +++++++++++++++++++++++-----
4 files changed, 61 insertions(+), 11 deletions(-)
diff --git a/server/src/app.ts b/server/src/app.ts
index 4dbf8be53..77941f983 100644
--- a/server/src/app.ts
+++ b/server/src/app.ts
@@ -685,7 +685,7 @@ export function createApp(
* A person's own linked accounts at outside providers (Slack, GitHub). Theirs alone: there is no
* administrator view and no linking on somebody else's behalf.
*/
- app.route("/api/identity", identityRoutes(requireUser, identity));
+ app.route("/api/identity", identityRoutes(requireUser, identity, auditStore));
app.get("/api/settings/instructions", requireUser, async (context) => {
if (!userInstructions) {
diff --git a/server/src/audit.ts b/server/src/audit.ts
index 990453831..50c454c9e 100644
--- a/server/src/audit.ts
+++ b/server/src/audit.ts
@@ -511,6 +511,8 @@ export const auditEventTypes = [
/* A Bot connected to, or disconnected from, a Slack, Teams or SMS conversation. */
"delivery.linked",
"delivery.unlinked",
+ /* A person removing their own linked account at an outside provider (Slack, GitHub). */
+ "identity.unlinked",
/* Routines, however they were made: by a person on the screen or by a Bot through its tools. */
"routine.created",
"routine.updated",
diff --git a/server/src/identity/routes.ts b/server/src/identity/routes.ts
index a11ec7a15..a0066c86c 100644
--- a/server/src/identity/routes.ts
+++ b/server/src/identity/routes.ts
@@ -1,4 +1,5 @@
import { Hono, type MiddlewareHandler } from "hono";
+import { type AuditStore, recordAuditEvent } from "../audit";
import type { AppVariables } from "../auth/guards";
import { PROVIDERS } from "./providers";
import type { IdentityStore } from "./store";
@@ -13,6 +14,7 @@ import type { IdentityStore } from "./store";
export function identityRoutes(
requireUser: MiddlewareHandler<{ Variables: AppVariables }>,
store?: Pick,
+ auditStore?: AuditStore,
) {
const app = new Hono<{ Variables: AppVariables }>();
app.use("*", async (context, next) => {
@@ -43,12 +45,31 @@ export function identityRoutes(
});
app.delete("/links/:id", async (context) => {
- const removed = await live.unlink(
- context.var.actor.id,
- context.req.param("id"),
+ const actorId = context.var.actor.id;
+ const id = context.req.param("id");
+ const link = (await live.identitiesFor(actorId)).find(
+ (candidate) => candidate.id === id,
);
+ const removed = await live.unlink(actorId, id);
if (!removed)
return context.json({ error: "Linked account not found." }, 404);
+ /*
+ * After the removal, and not caught: the same order and the same failure as the connected-account
+ * disconnect (`mcp.account_disconnected`), where an audit write that throws fails the request.
+ * Repeating it is safe because the link is already gone and answers 404.
+ *
+ * Names the link and its provider only. The subject, realm and credential id stay out of the
+ * trail, as they stay out of every response here.
+ */
+ if (auditStore) {
+ await recordAuditEvent(auditStore, {
+ eventType: "identity.unlinked",
+ targetType: "identity_link",
+ targetId: id,
+ actorUserId: actorId,
+ payload: { actor: actorId, provider: link?.provider },
+ });
+ }
return context.body(null, 204);
});
diff --git a/server/tests/identity-routes.test.ts b/server/tests/identity-routes.test.ts
index d10f0e1e9..07c3c8d14 100644
--- a/server/tests/identity-routes.test.ts
+++ b/server/tests/identity-routes.test.ts
@@ -1,5 +1,6 @@
import { expect, test } from "bun:test";
import type { MiddlewareHandler } from "hono";
+import type { AuditEventInput } from "../src/audit";
import type { AppVariables } from "../src/auth/guards";
import { identityRoutes } from "../src/identity/routes";
import type { IdentityLink } from "../src/identity/types";
@@ -31,14 +32,19 @@ const link: IdentityLink = {
function fixture() {
const calls: string[] = [];
- const app = identityRoutes(signedIn, {
- identitiesFor: async (userId) => (userId === "person" ? [link] : []),
- unlink: async (userId, id) => {
- calls.push(`unlink:${userId}:${id}`);
- return userId === "person" && id === "link-1";
+ const events: AuditEventInput[] = [];
+ const app = identityRoutes(
+ signedIn,
+ {
+ identitiesFor: async (userId) => (userId === "person" ? [link] : []),
+ unlink: async (userId, id) => {
+ calls.push(`unlink:${userId}:${id}`);
+ return userId === "person" && id === "link-1";
+ },
},
- });
- return { app, calls };
+ { insert: async (event) => void events.push(event) },
+ );
+ return { app, calls, events };
}
test("links lists the asker's links without subject, realm or credential", async () => {
@@ -103,3 +109,24 @@ test("auth runs before the missing-store 503 and responses are never cached", as
expect(response.status).toBe(401);
expect(response.headers.get("cache-control")).toBe("no-store");
});
+
+test("a successful DELETE records one event naming actor, link and provider only", async () => {
+ const f = fixture();
+ await f.app.request("/links/link-1", { method: "DELETE" });
+ expect(f.events).toHaveLength(1);
+ expect(f.events[0]).toMatchObject({
+ eventType: "identity.unlinked",
+ targetId: "link-1",
+ actorUserId: "person",
+ payload: { actor: "person", provider: "github" },
+ });
+ const text = JSON.stringify(f.events[0]);
+ for (const secret of ["secret-credential-id", "github.com", "42"])
+ expect(text).not.toContain(`"${secret}"`);
+});
+
+test("a 404 DELETE records nothing", async () => {
+ const f = fixture();
+ await f.app.request("/links/someone-elses", { method: "DELETE" });
+ expect(f.events).toEqual([]);
+});
From 25e6f6f5ec86ffd74f0a866cbb8e1ca6d2fd9afa Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:16:25 -0300
Subject: [PATCH 031/101] fix(server): refuse identity links to unusable
credentials
---
server/src/identity/store.ts | 29 ++++++++++++--
.../tests/identity-store.integration.test.ts | 39 +++++++++++++++++++
2 files changed, 64 insertions(+), 4 deletions(-)
diff --git a/server/src/identity/store.ts b/server/src/identity/store.ts
index 48634711b..df83ff55d 100644
--- a/server/src/identity/store.ts
+++ b/server/src/identity/store.ts
@@ -17,6 +17,7 @@ import {
type Transaction = Parameters[0]>[0];
type Revoke = Pick;
+type Vault = Pick;
const identityMatches = (identity: Identity) =>
and(
@@ -186,7 +187,7 @@ function codeHash(code: string) {
return createHash("sha256").update(code.toLowerCase()).digest("hex");
}
-export function createIdentityStore(database: Database, credentials: Revoke) {
+export function createIdentityStore(database: Database, credentials: Vault) {
return {
async linkedUser(
value: Identity,
@@ -232,6 +233,15 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
return links;
},
+ /**
+ * Link `value` to `userId` on proof the caller has already verified.
+ *
+ * A `credentialId`, when given, must name a live credential: it is checked in the link's own
+ * transaction, so a missing or revoked id is refused rather than stored as a token the link
+ * does not have. A "challenge" link proves control by code and carries no credential, so it
+ * refuses one. An "oauth" link is not required to carry one: GitHub sign-in may link before
+ * a token is stored.
+ */
async linkVerified(
value: Identity,
userId: string,
@@ -247,9 +257,20 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
throw new IdentityInputError(
"This provider does not accept that kind of proof.",
);
- return database.transaction((transaction) =>
- writeLink(transaction, credentials, identity, userId, proof),
- );
+ if (proof.credentialId && proof.method === "challenge")
+ throw new IdentityInputError(
+ "A challenge link does not carry a credential.",
+ );
+ return database.transaction(async (transaction) => {
+ if (
+ proof.credentialId &&
+ !(await credentials.isLive(proof.credentialId, transaction))
+ )
+ throw new IdentityInputError(
+ "The credential for this link is not available.",
+ );
+ return writeLink(transaction, credentials, identity, userId, proof);
+ });
},
async markNeedsReconnect(linkId: string): Promise {
diff --git a/server/tests/identity-store.integration.test.ts b/server/tests/identity-store.integration.test.ts
index 559014786..ddf94cd51 100644
--- a/server/tests/identity-store.integration.test.ts
+++ b/server/tests/identity-store.integration.test.ts
@@ -210,3 +210,42 @@ test("identitiesFor skips a link whose provider the registry does not know", asy
const links = await store.identitiesFor(person);
expect(links.map((link) => link.id)).toEqual([valid.id]);
});
+
+test("a credential that does not exist is refused and no link is written", async () => {
+ const identity = github();
+ await expect(
+ store.linkVerified(identity, user(), {
+ method: "oauth",
+ credentialId: randomUUID(),
+ }),
+ ).rejects.toThrow(IdentityInputError);
+ expect(await store.linkedUser(identity)).toBeNull();
+});
+
+test("a revoked credential is refused and no link is written", async () => {
+ const identity = github();
+ const id = await credential();
+ await vault.revoke(id);
+ await expect(
+ store.linkVerified(identity, user(), {
+ method: "oauth",
+ credentialId: id,
+ }),
+ ).rejects.toThrow(IdentityInputError);
+ expect(await store.linkedUser(identity)).toBeNull();
+});
+
+test("a challenge link refuses a credential", async () => {
+ const identity: Identity = {
+ provider: "slack",
+ realm,
+ subject: randomUUID(),
+ };
+ await expect(
+ store.linkVerified(identity, user(), {
+ method: "challenge",
+ credentialId: await credential(),
+ }),
+ ).rejects.toThrow(IdentityInputError);
+ expect(await store.linkedUser(identity)).toBeNull();
+});
From 3da32231592b1ccfb4ed695b49780ef2ee254e9e Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:23:41 -0300
Subject: [PATCH 032/101] docs: describe linked accounts as they stand
---
docs/configuration.md | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/docs/configuration.md b/docs/configuration.md
index 98a64aeac..588b13023 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -485,20 +485,23 @@ SMS through Twilio honours Advanced Opt-Out: `STOP` (and Twilio's other opt-out
number opted out, later messages show `opted_out` in the delivery history instead of being sent, and
`START` resumes. Twilio sends the confirmation reply itself, so OpenBot does not.
-### Linked accounts
-
-A person's accounts at outside providers — their Slack user, their GitHub account — are linked to
-their OpenBot user in one place, `identity_links`, and listed under **Linked accounts** on
-`/settings/connected-accounts`, where they can disconnect any of them. A link is proven one of two
-ways: by the provider's own sign-in (GitHub, when that integration is configured), or by a one-time
-code OpenBot issues to the signed-in person, which they send from their chat account (Slack). A
-code always links to the person it was issued to, so nobody can link an account on somebody else's
-behalf, and an outside account links to one OpenBot user at most. Linking providers are added by
-their integrations; until one is configured, the list is empty.
-
-This complements, rather than replaces, the OpenTag **Reachability** pairing above: that pairing
-binds a conversation and Bot for delivery, while a linked account records who a person is at the
-provider.
+## Linked accounts
+
+OpenBot keeps a record of a person's accounts at outside providers, such as their GitHub account
+or their Slack user. When a person has any, they appear under **Linked accounts** on
+`/settings/connected-accounts`, and each can be disconnected there. Disconnecting revokes any token
+stored with the link, and is audited. A person with none sees no such section, and a deployment
+without an identity store does not show it at all.
+
+Links are created by the integrations that support them, and none ship enabled yet, so the list is
+empty until one is. An integration proves an account in one of two ways: by the provider's own
+sign-in, or by a one-time code that OpenBot issues to the signed-in person and that they send from
+their chat account. A code links only to the person it was issued to, and an outside account links to
+one OpenBot user at most.
+
+This is separate from the OpenTag **Reachability** pairing above. That pairing binds a conversation
+for delivery and uses its own `link ` messages, shown on the **Reachability** screen; it is not
+recorded as a linked account.
## Text messages and push notifications
From 116d79d3f30dc1cfe573ce305c1cc492a96bb8df Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:23:46 -0300
Subject: [PATCH 033/101] fix(server): refuse blank identity parts
---
server/src/identity/providers.ts | 16 ++++++++---
server/tests/identity-providers.test.ts | 36 +++++++++++++++++++++++++
2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/server/src/identity/providers.ts b/server/src/identity/providers.ts
index 761d96c25..ab2025837 100644
--- a/server/src/identity/providers.ts
+++ b/server/src/identity/providers.ts
@@ -25,6 +25,8 @@ export const PROVIDERS: Readonly> =
github: { title: "GitHub", methods: ["oauth"] },
});
+const REALM_MAX = 512;
+
export const GITHUB_DOTCOM_REALM = "github.com";
export function isIdentityProvider(value: unknown): value is IdentityProvider {
@@ -46,15 +48,21 @@ export function slackRealm(parts: {
workspaceId: string;
}): string {
const values = [parts.connectionId, parts.installationId, parts.workspaceId];
- if (values.some((value) => !value.trim()))
+ if (values.some((value) => !value.trim() || value !== value.trim()))
throw new IdentityInputError("Slack identity is incomplete.");
- return values.map(encodeURIComponent).join(":");
+ const realm = values.map(encodeURIComponent).join(":");
+ if (realm.length > REALM_MAX)
+ throw new IdentityInputError("Slack identity is too long.");
+ return realm;
}
+// Provider ids never contain whitespace: reject blank values but keep others exactly as given.
+const nonBlank = (value: string) => value.trim().length > 0;
+
const identitySchema = z.object({
provider: z.string().refine(isIdentityProvider),
- realm: z.string().min(1).max(512),
- subject: z.string().min(1).max(256),
+ realm: z.string().max(REALM_MAX).refine(nonBlank),
+ subject: z.string().max(256).refine(nonBlank),
});
/** Explicit fields only, so extra runtime properties never reach the persistence contract. */
diff --git a/server/tests/identity-providers.test.ts b/server/tests/identity-providers.test.ts
index 129297415..91cb9ef7a 100644
--- a/server/tests/identity-providers.test.ts
+++ b/server/tests/identity-providers.test.ts
@@ -83,3 +83,39 @@ test("parseIdentity strips extra properties and bounds sizes", () => {
parseIdentity({ provider: "slack", realm: "", subject: "s" }),
).toThrow(IdentityInputError);
});
+
+test("parseIdentity refuses a whitespace-only realm or subject", () => {
+ expect(() =>
+ parseIdentity({ provider: "slack", realm: " ", subject: "U1" }),
+ ).toThrow(IdentityInputError);
+ expect(() =>
+ parseIdentity({ provider: "slack", realm: "T1", subject: " \t" }),
+ ).toThrow(IdentityInputError);
+});
+
+test("slackRealm refuses parts with surrounding whitespace", () => {
+ expect(() =>
+ slackRealm({
+ connectionId: "C1",
+ installationId: "I1",
+ workspaceId: " T1",
+ }),
+ ).toThrow(IdentityInputError);
+ expect(() =>
+ slackRealm({
+ connectionId: "C1 ",
+ installationId: "I1",
+ workspaceId: "T1",
+ }),
+ ).toThrow(IdentityInputError);
+});
+
+test("slackRealm refuses a realm longer than parseIdentity accepts", () => {
+ expect(() =>
+ slackRealm({
+ connectionId: "C".repeat(200),
+ installationId: "I".repeat(200),
+ workspaceId: "T".repeat(200),
+ }),
+ ).toThrow(IdentityInputError);
+});
From 01d825d9e2c0dc4c4783718e5adf27488b63bc8c Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:23:59 -0300
Subject: [PATCH 034/101] docs: warn that a linked-account code must not be
shared
---
docs/configuration.md | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/docs/configuration.md b/docs/configuration.md
index 588b13023..6918a4a8b 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -496,7 +496,9 @@ without an identity store does not show it at all.
Links are created by the integrations that support them, and none ship enabled yet, so the list is
empty until one is. An integration proves an account in one of two ways: by the provider's own
sign-in, or by a one-time code that OpenBot issues to the signed-in person and that they send from
-their chat account. A code links only to the person it was issued to, and an outside account links to
+their chat account. Whoever sends the code gets that chat account linked to the person it was issued
+to, so treat it like a password and do not share it. It is valid for ten minutes and works once, and
+issuing a new code for the same provider invalidates the earlier one. An outside account links to
one OpenBot user at most.
This is separate from the OpenTag **Reachability** pairing above. That pairing binds a conversation
From 7ec2bc4d4920f7fe2f459b6437627c98fa40db01 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:25:04 -0300
Subject: [PATCH 035/101] fix(server): retire linked accounts completely when a
person is removed
Removing a person now retires every linked account, whichever half fails:
- retireOwnedAccounts runs the plugin and identity retirements both, then
rethrows the first failure (AggregateError if both fail); the count is
the sum. A Composio refusal no longer leaves linked tokens live.
- retireIdentityLinks deletes the person's pending link challenges in the
same transaction, so a code issued before removal cannot be redeemed.
- Every link is marked needs_reconnect, including credential-less (Slack)
links; rows are kept so the outside account resolves to the removed
person and is refused.
- writeLink's per-(user, provider, realm) advisory key is taken for each
of the person's realms before the row locks; the residual window (a new
realm's OAuth link in flight) is documented.
- One identity.link_retired audit row per changed link: actor `by`, target
the link id, payload { actor, provider, reason: "person_removed",
credentialRevoked }. No subject, realm or credential id.
Call sites of changed exports:
- retireIdentityLinks (signature now database, credentials, auditStore,
userId, by): server/src/index.ts (people-store hook),
server/tests/identity-retire.integration.test.ts.
- retireOwnedAccounts (new): server/src/index.ts, the same test.
- auditEventTypes (+ "identity.link_retired"): closed union in
server/src/audit.ts; no app-side mirror references identity.* types.
---
server/src/audit.ts | 5 +
server/src/identity/retire.ts | 138 ++++++++++--
server/src/index.ts | 24 ++-
.../tests/identity-retire.integration.test.ts | 198 +++++++++++++++++-
4 files changed, 331 insertions(+), 34 deletions(-)
diff --git a/server/src/audit.ts b/server/src/audit.ts
index 50c454c9e..5d487905f 100644
--- a/server/src/audit.ts
+++ b/server/src/audit.ts
@@ -513,6 +513,11 @@ export const auditEventTypes = [
"delivery.unlinked",
/* A person removing their own linked account at an outside provider (Slack, GitHub). */
"identity.unlinked",
+ /*
+ * A removed person's linked account retired by the removal: marked `needs_reconnect`, its token (if
+ * any) revoked. Names the link, its provider and the remover only.
+ */
+ "identity.link_retired",
/* Routines, however they were made: by a person on the screen or by a Bot through its tools. */
"routine.created",
"routine.updated",
diff --git a/server/src/identity/retire.ts b/server/src/identity/retire.ts
index fed55d974..7b8fc1588 100644
--- a/server/src/identity/retire.ts
+++ b/server/src/identity/retire.ts
@@ -1,40 +1,89 @@
-import { eq } from "drizzle-orm";
+import { asc, eq, sql } from "drizzle-orm";
+import { type AuditStore, recordAuditEvent } from "../audit";
import { CredentialRefusedError, type CredentialStore } from "../credentials";
import type { Database } from "../db/client";
-import { identityLinks } from "../db/schema";
+import { identityLinkChallenges, identityLinks } from "../db/schema";
/**
- * Retires the credentials behind a removed person's identity links.
+ * Retires a removed person's identity links.
*
- * The link rows stay: `user_id` deliberately has no foreign key, so the outside account keeps
- * resolving to the removed person and is refused, rather than turning into an unlinked guest. What
- * must not stay live is the token each link holds, so every one is revoked, detached from its link,
- * and the link marked `needs_reconnect`. One transaction, so a failure leaves nothing half-retired.
+ * Every one of the person's links is marked `needs_reconnect`, whatever its provider: a link with a
+ * credential has that credential revoked and detached (`credential_id` null), and a link without one
+ * (Slack) changes status only. The rows are NOT deleted — `user_id` deliberately has no foreign key,
+ * so the outside account keeps resolving to the removed person and is refused, rather than turning
+ * into an unlinked guest. The person's pending link challenges are deleted, so a code issued before
+ * the removal cannot be redeemed into a fresh active link afterwards.
*
- * Returns how many links had a credential retired; a second call returns 0.
+ * One transaction, so a failure leaves nothing half-retired. One `identity.link_retired` audit row
+ * per link that changed, written after the commit (the audit store has its own handle — the same
+ * order `retireConnectionsFor` uses): actor `by`, target the link id, and the provider, the reason
+ * and whether a credential was revoked. Never the subject, realm or credential id.
+ *
+ * Concurrency. The challenges are deleted first: a `redeemChallenge` in flight holds its challenge
+ * row, so the delete waits for it to commit and the link it wrote is then seen below. Then, for
+ * every realm the person has a link in, the exact advisory key `writeLink` takes for
+ * `(userId, provider, realm)` is taken — in sorted order, before any row lock, the order `writeLink`
+ * uses — so a concurrent re-link in a known realm finishes first or waits for this. The residual
+ * window: a `writeLink` that is not a challenge redemption (an OAuth link) into a realm this person
+ * had no link in, still uncommitted when the realms are read, commits an active link after this
+ * returns. The person's sessions are already gone by then, so that needs a callback already past
+ * its session check.
+ *
+ * Returns how many links changed; a second call returns 0 and writes no audit rows.
*/
export async function retireIdentityLinks(
database: Database,
credentials: Pick,
+ auditStore: AuditStore,
userId: string,
+ by: string,
): Promise {
- return database.transaction(async (transaction) => {
+ const retired = await database.transaction(async (transaction) => {
+ await transaction
+ .delete(identityLinkChallenges)
+ .where(eq(identityLinkChallenges.userId, userId));
+
+ const realms = await transaction
+ .selectDistinct({
+ provider: identityLinks.provider,
+ realm: identityLinks.realm,
+ })
+ .from(identityLinks)
+ .where(eq(identityLinks.userId, userId))
+ .orderBy(asc(identityLinks.provider), asc(identityLinks.realm));
+ for (const { provider, realm } of realms) {
+ // writeLink's key, character for character (server/src/identity/store.ts).
+ await transaction.execute(
+ sql`select pg_advisory_xact_lock(hashtextextended(${`${userId}\u001f${provider}\u001f${realm}`}, 0))`,
+ );
+ }
+
const links = await transaction
.select({
id: identityLinks.id,
+ provider: identityLinks.provider,
+ status: identityLinks.status,
credentialId: identityLinks.credentialId,
})
.from(identityLinks)
.where(eq(identityLinks.userId, userId))
+ .orderBy(asc(identityLinks.id))
.for("update");
- let retired = 0;
+
+ const changed: {
+ id: string;
+ provider: string;
+ credentialRevoked: boolean;
+ }[] = [];
for (const link of links) {
- if (!link.credentialId) continue;
- try {
- await credentials.revoke(link.credentialId, transaction);
- } catch (error) {
- // Already revoked is the outcome we wanted.
- if (!(error instanceof CredentialRefusedError)) throw error;
+ if (!link.credentialId && link.status === "needs_reconnect") continue;
+ if (link.credentialId) {
+ try {
+ await credentials.revoke(link.credentialId, transaction);
+ } catch (error) {
+ // Already revoked is the outcome we wanted.
+ if (!(error instanceof CredentialRefusedError)) throw error;
+ }
}
await transaction
.update(identityLinks)
@@ -44,8 +93,61 @@ export async function retireIdentityLinks(
updatedAt: new Date(),
})
.where(eq(identityLinks.id, link.id));
- retired += 1;
+ changed.push({
+ id: link.id,
+ provider: link.provider,
+ credentialRevoked: Boolean(link.credentialId),
+ });
}
- return retired;
+ return changed;
});
+
+ for (const link of retired) {
+ await recordAuditEvent(auditStore, {
+ eventType: "identity.link_retired",
+ targetType: "identity_link",
+ targetId: link.id,
+ actorUserId: by,
+ payload: {
+ actor: by,
+ provider: link.provider,
+ reason: "person_removed",
+ credentialRevoked: link.credentialRevoked,
+ },
+ });
+ }
+ return retired.length;
+}
+
+/**
+ * Everything a removed person had connected, retired in full whichever half fails.
+ *
+ * Both halves always run: the plugin half rethrows on purpose (a broker refusal), and stopping there
+ * left every linked account's token live, with a retry hitting the same refusal first. The failure
+ * is still thrown afterwards — the first one alone, or an AggregateError of both — so the caller's
+ * error handling sees it. On success, the count is both halves together.
+ */
+export async function retireOwnedAccounts(retirers: {
+ plugins: () => Promise<{ retired: number }>;
+ identities: () => Promise;
+}): Promise<{ retired: number }> {
+ const failures: unknown[] = [];
+ let retired = 0;
+ try {
+ retired += (await retirers.plugins()).retired;
+ } catch (error) {
+ failures.push(error);
+ }
+ try {
+ retired += await retirers.identities();
+ } catch (error) {
+ failures.push(error);
+ }
+ if (failures.length === 1) throw failures[0];
+ if (failures.length > 1)
+ throw new AggregateError(
+ failures,
+ "Retiring a removed person's accounts failed in more than one place.",
+ );
+ return { retired };
}
diff --git a/server/src/index.ts b/server/src/index.ts
index 51516f4c6..1898acb67 100644
--- a/server/src/index.ts
+++ b/server/src/index.ts
@@ -176,7 +176,7 @@ import {
HostAccessRefusedError,
} from "./host-access/broker";
import { hostAccessTools } from "./host-access/tools";
-import { retireIdentityLinks } from "./identity/retire";
+import { retireIdentityLinks, retireOwnedAccounts } from "./identity/retire";
import { createIdentityStore } from "./identity/store";
import {
createIntelligenceClient,
@@ -401,12 +401,22 @@ const peopleStore = createPeopleStore(
* during module initialisation — it runs when an administrator removes somebody, over HTTP — so by
* then the binding is there.
*/
- async (userId, by) => {
- const plugins = await pluginStore.retireConnectionsFor(userId, by);
- // Their linked outside accounts keep resolving to them, now refused; only the tokens go.
- await retireIdentityLinks(database, credentialStore, userId);
- return plugins;
- },
+ /*
+ * Both halves run whichever fails, and the failure is rethrown after. Their linked outside
+ * accounts keep resolving to them, now refused; the tokens go and every link needs reconnecting.
+ */
+ (userId, by) =>
+ retireOwnedAccounts({
+ plugins: () => pluginStore.retireConnectionsFor(userId, by),
+ identities: () =>
+ retireIdentityLinks(
+ database,
+ credentialStore,
+ bootAuditStore,
+ userId,
+ by,
+ ),
+ }),
);
const identityProviderStore = createIdentityProviderStore(database);
/*
diff --git a/server/tests/identity-retire.integration.test.ts b/server/tests/identity-retire.integration.test.ts
index d1d802139..c7311196f 100644
--- a/server/tests/identity-retire.integration.test.ts
+++ b/server/tests/identity-retire.integration.test.ts
@@ -1,19 +1,35 @@
import { afterAll, expect, test } from "bun:test";
import { randomUUID } from "node:crypto";
import { eq, inArray } from "drizzle-orm";
+import type { AuditEventInput, AuditStore } from "../src/audit";
import { createCredentialStore } from "../src/credentials";
import { createDatabase } from "../src/db/client";
-import { credentials, identityLinks } from "../src/db/schema";
-import { retireIdentityLinks } from "../src/identity/retire";
+import {
+ credentials,
+ identityLinkChallenges,
+ identityLinks,
+} from "../src/db/schema";
+import {
+ retireIdentityLinks,
+ retireOwnedAccounts,
+} from "../src/identity/retire";
+import { createIdentityStore } from "../src/identity/store";
+import { IdentityLinkError } from "../src/identity/types";
import { TEST_POOL, testDatabaseUrl } from "./support/database";
const database = createDatabase(testDatabaseUrl(), TEST_POOL);
const vault = createCredentialStore(database);
const realm = `test-${randomUUID()}`;
const createdCredentials: string[] = [];
+const users: string[] = [];
+const ADMIN = "admin-remover";
afterAll(async () => {
await database.delete(identityLinks).where(eq(identityLinks.realm, realm));
+ if (users.length)
+ await database
+ .delete(identityLinkChallenges)
+ .where(inArray(identityLinkChallenges.userId, users));
if (createdCredentials.length)
await database
.delete(credentials)
@@ -21,8 +37,32 @@ afterAll(async () => {
await database.$client.close();
});
+const user = () => {
+ const id = randomUUID();
+ users.push(id);
+ return id;
+};
+
+function recorder() {
+ const rows: AuditEventInput[] = [];
+ const store: AuditStore = {
+ insert: async (event) => {
+ rows.push(event);
+ },
+ };
+ return { rows, store };
+}
+
+const statusOf = async (linkId: string) =>
+ (
+ await database
+ .select()
+ .from(identityLinks)
+ .where(eq(identityLinks.id, linkId))
+ )[0];
+
test("retiring a removed person's links revokes their tokens and keeps the rows", async () => {
- const userId = randomUUID();
+ const userId = user();
const credential = await vault.create({
kind: "connector",
provider: "github-user-token",
@@ -43,17 +83,157 @@ test("retiring a removed person's links revokes their tokens and keeps the rows"
status: "active",
});
expect(await vault.isLive(credential.id)).toBe(true);
+ const audit = recorder();
- expect(await retireIdentityLinks(database, vault, userId)).toBe(1);
+ expect(
+ await retireIdentityLinks(database, vault, audit.store, userId, ADMIN),
+ ).toBe(1);
expect(await vault.isLive(credential.id)).toBe(false);
- const [row] = await database
- .select()
- .from(identityLinks)
- .where(eq(identityLinks.id, linkId));
+ const row = await statusOf(linkId);
expect(row?.userId).toBe(userId);
expect(row?.credentialId).toBeNull();
expect(row?.status).toBe("needs_reconnect");
- expect(await retireIdentityLinks(database, vault, userId)).toBe(0);
+ expect(audit.rows).toHaveLength(1);
+ expect(audit.rows[0]).toMatchObject({
+ eventType: "identity.link_retired",
+ targetType: "identity_link",
+ targetId: linkId,
+ actorUserId: ADMIN,
+ payload: {
+ actor: ADMIN,
+ provider: "github",
+ reason: "person_removed",
+ credentialRevoked: true,
+ },
+ });
+ const payload = JSON.stringify(audit.rows[0]?.payload);
+ expect(payload).not.toContain(realm);
+ expect(payload).not.toContain(credential.id);
+
+ // A second pass changes nothing and records nothing.
+ expect(
+ await retireIdentityLinks(database, vault, audit.store, userId, ADMIN),
+ ).toBe(0);
+ expect(audit.rows).toHaveLength(1);
+});
+
+test("a link without a credential is also marked needs_reconnect, and audited without its subject", async () => {
+ const userId = user();
+ const linkId = randomUUID();
+ const subject = `U-${randomUUID()}`;
+ await database.insert(identityLinks).values({
+ id: linkId,
+ provider: "slack",
+ realm,
+ subject,
+ userId,
+ verifiedBy: "challenge",
+ status: "active",
+ });
+ const audit = recorder();
+
+ expect(
+ await retireIdentityLinks(database, vault, audit.store, userId, ADMIN),
+ ).toBe(1);
+
+ const row = await statusOf(linkId);
+ expect(row?.userId).toBe(userId);
+ expect(row?.status).toBe("needs_reconnect");
+ expect(audit.rows).toHaveLength(1);
+ expect(audit.rows[0]).toMatchObject({
+ eventType: "identity.link_retired",
+ targetId: linkId,
+ actorUserId: ADMIN,
+ payload: {
+ provider: "slack",
+ reason: "person_removed",
+ credentialRevoked: false,
+ },
+ });
+ const payload = JSON.stringify(audit.rows[0]?.payload);
+ expect(payload).not.toContain(subject);
+ expect(payload).not.toContain(realm);
+});
+
+test("a pending challenge cannot be redeemed after the person is removed", async () => {
+ const userId = user();
+ const identities = createIdentityStore(database, vault);
+ const { code } = await identities.issueChallenge(userId, "slack");
+
+ await retireIdentityLinks(database, vault, recorder().store, userId, ADMIN);
+
+ const pending = await database
+ .select()
+ .from(identityLinkChallenges)
+ .where(eq(identityLinkChallenges.userId, userId));
+ expect(pending).toHaveLength(0);
+ await expect(
+ identities.redeemChallenge(code, {
+ provider: "slack",
+ realm,
+ subject: randomUUID(),
+ }),
+ ).rejects.toBeInstanceOf(IdentityLinkError);
+});
+
+test("a plugin refusal still retires identity links, and is rethrown", async () => {
+ const refusal = new Error("composio refused");
+ const ran: string[] = [];
+ await expect(
+ retireOwnedAccounts({
+ plugins: async () => {
+ ran.push("plugins");
+ throw refusal;
+ },
+ identities: async () => {
+ ran.push("identities");
+ return 2;
+ },
+ }),
+ ).rejects.toBe(refusal);
+ expect(ran).toEqual(["plugins", "identities"]);
+});
+
+test("an identity failure still retires plugin connections, and is rethrown", async () => {
+ const failure = new Error("vault down");
+ const ran: string[] = [];
+ await expect(
+ retireOwnedAccounts({
+ plugins: async () => {
+ ran.push("plugins");
+ return { retired: 1 };
+ },
+ identities: async () => {
+ ran.push("identities");
+ throw failure;
+ },
+ }),
+ ).rejects.toBe(failure);
+ expect(ran).toEqual(["plugins", "identities"]);
+});
+
+test("both failing surfaces both", async () => {
+ const first = new Error("plugins");
+ const second = new Error("identities");
+ const error = await retireOwnedAccounts({
+ plugins: async () => {
+ throw first;
+ },
+ identities: async () => {
+ throw second;
+ },
+ }).catch((caught: unknown) => caught);
+ expect(error).toBeInstanceOf(AggregateError);
+ expect((error as AggregateError).errors).toEqual([first, second]);
+});
+
+test("both succeeding counts both", async () => {
+ expect(
+ await retireOwnedAccounts({
+ plugins: async () => ({ retired: 2 }),
+ identities: async () => 3,
+ }),
+ ).toEqual({ retired: 5 });
});
From 8264d2c45381348c6c1c65b73c78f54966d9a60c Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:27:52 -0300
Subject: [PATCH 036/101] fix(server): keep, lock and own identity link
credentials
Re-linking an identity without a credentialId used to revoke and null
its stored token, so a GitHub re-sign-in that minted no new token killed
the working one. Now an omitted credentialId (or handle) keeps what the
link has; a different credential replaces the old one and revokes it;
only unlink clears a credential. An explicit null handle clears it.
A credential belongs to one link: a new partial unique index
identity_links_credential_idx (credential_id where not null) backs a
check that refuses a credential already held by another link, so
unlinking, replacing or retiring one link can no longer revoke another
link's token. The unique violation maps to the same IdentityInputError.
The credential row is now locked live (select ... where revoked_at is
null for update) inside the link's transaction, after the advisory lock
and link-row locks (retireIdentityLinks' order), instead of an unlocked
isLive check a concurrent revoke could race. A malformed credentialId is
refused by shape before any query, so Postgres never echoes it back.
Replacing a person's older same-realm link no longer revokes the
credential when the new link is taking that same credential.
writeLink is module-private now. Call sites:
- linkVerified (store.ts) -> writeLink
- redeemChallenge (store.ts) -> writeLink, with no credential
- retire.ts only names writeLink's advisory key in comments; no import
linkVerified callers: server/tests/identity-store.integration.test.ts
only (no production caller yet).
Migration 0052 (branch-only) regenerated to include the new index.
---
server/drizzle/0052_identity_links.sql | 3 +-
server/drizzle/meta/0052_snapshot.json | 18 +-
server/drizzle/meta/_journal.json | 2 +-
server/src/db/schema/identity.ts | 5 +
server/src/identity/store.ts | 212 +++++++++++++-----
.../tests/identity-store.integration.test.ts | 89 +++++++-
6 files changed, 261 insertions(+), 68 deletions(-)
diff --git a/server/drizzle/0052_identity_links.sql b/server/drizzle/0052_identity_links.sql
index b0495c777..c2a6140d2 100644
--- a/server/drizzle/0052_identity_links.sql
+++ b/server/drizzle/0052_identity_links.sql
@@ -25,4 +25,5 @@ ALTER TABLE "identity_links" ADD CONSTRAINT "identity_links_credential_id_creden
CREATE INDEX "identity_link_challenges_expiry_idx" ON "identity_link_challenges" USING btree ("expires_at");--> statement-breakpoint
CREATE INDEX "identity_link_challenges_user_idx" ON "identity_link_challenges" USING btree ("user_id","provider");--> statement-breakpoint
CREATE UNIQUE INDEX "identity_links_identity_idx" ON "identity_links" USING btree ("provider","realm","subject");--> statement-breakpoint
-CREATE UNIQUE INDEX "identity_links_user_realm_idx" ON "identity_links" USING btree ("user_id","provider","realm");
\ No newline at end of file
+CREATE UNIQUE INDEX "identity_links_user_realm_idx" ON "identity_links" USING btree ("user_id","provider","realm");--> statement-breakpoint
+CREATE UNIQUE INDEX "identity_links_credential_idx" ON "identity_links" USING btree ("credential_id") WHERE "identity_links"."credential_id" IS NOT NULL;
\ No newline at end of file
diff --git a/server/drizzle/meta/0052_snapshot.json b/server/drizzle/meta/0052_snapshot.json
index 81b3e9634..203d7789a 100644
--- a/server/drizzle/meta/0052_snapshot.json
+++ b/server/drizzle/meta/0052_snapshot.json
@@ -1,5 +1,5 @@
{
- "id": "54122071-85f2-4f2b-a125-703090a744a2",
+ "id": "a5c96b02-0eca-4def-bcc4-590cdb66fb28",
"prevId": "b4bb328d-1308-403f-a2ac-f8a291aaabc5",
"version": "7",
"dialect": "postgresql",
@@ -9113,6 +9113,22 @@
"concurrently": false,
"method": "btree",
"with": {}
+ },
+ "identity_links_credential_idx": {
+ "name": "identity_links_credential_idx",
+ "columns": [
+ {
+ "expression": "credential_id",
+ "isExpression": false,
+ "asc": true,
+ "nulls": "last"
+ }
+ ],
+ "isUnique": true,
+ "where": "\"identity_links\".\"credential_id\" IS NOT NULL",
+ "concurrently": false,
+ "method": "btree",
+ "with": {}
}
},
"foreignKeys": {
diff --git a/server/drizzle/meta/_journal.json b/server/drizzle/meta/_journal.json
index 3e53b2e19..3a967466c 100644
--- a/server/drizzle/meta/_journal.json
+++ b/server/drizzle/meta/_journal.json
@@ -369,7 +369,7 @@
{
"idx": 52,
"version": "7",
- "when": 1791472331083,
+ "when": 1791473240143,
"tag": "0052_identity_links",
"breakpoints": true
}
diff --git a/server/src/db/schema/identity.ts b/server/src/db/schema/identity.ts
index 6d58e9521..2a96dff91 100644
--- a/server/src/db/schema/identity.ts
+++ b/server/src/db/schema/identity.ts
@@ -50,6 +50,11 @@ export const identityLinks = pgTable(
table.provider,
table.realm,
),
+ // One link per credential: unlinking, replacing or retiring a link revokes its credential, so a
+ // shared one would kill the other link's token.
+ uniqueIndex("identity_links_credential_idx")
+ .on(table.credentialId)
+ .where(sql`${table.credentialId} IS NOT NULL`),
check(
"identity_links_verified_by_check",
sql`${table.verifiedBy} IN ('challenge', 'oauth')`,
diff --git a/server/src/identity/store.ts b/server/src/identity/store.ts
index df83ff55d..161ae033f 100644
--- a/server/src/identity/store.ts
+++ b/server/src/identity/store.ts
@@ -1,8 +1,12 @@
import { createHash, randomUUID } from "node:crypto";
-import { and, eq, gt, lt, sql } from "drizzle-orm";
+import { and, eq, gt, isNull, lt, notInArray, sql } from "drizzle-orm";
import { CredentialRefusedError, type CredentialStore } from "../credentials";
import type { Database } from "../db/client";
-import { identityLinkChallenges, identityLinks } from "../db/schema";
+import {
+ credentials as credentialRows,
+ identityLinkChallenges,
+ identityLinks,
+} from "../db/schema";
import { acceptsMethod, isIdentityProvider, parseIdentity } from "./providers";
import {
type Identity,
@@ -17,7 +21,6 @@ import {
type Transaction = Parameters[0]>[0];
type Revoke = Pick;
-type Vault = Pick;
const identityMatches = (identity: Identity) =>
and(
@@ -80,8 +83,51 @@ async function revokeQuietly(
}
}
+const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+
+const CREDENTIAL_UNAVAILABLE = "The credential for this link is not available.";
+
/**
- * Link `identity` to `userId` inside `transaction`.
+ * Claim `credentialId` for the link being written, inside its transaction: the credential row is
+ * locked live (`for update`, so a concurrent revoke waits for this link to commit or sees it), and
+ * it must not already belong to a link other than `own` (the identity's own link and the person's
+ * older link in the realm, which this write replaces). The partial unique index
+ * identity_links_credential_idx is the backstop.
+ */
+async function claimCredential(
+ transaction: Transaction,
+ credentialId: string,
+ own: string[],
+) {
+ const [live] = await transaction
+ .select({ id: credentialRows.id })
+ .from(credentialRows)
+ .where(
+ and(
+ eq(credentialRows.id, credentialId),
+ isNull(credentialRows.revokedAt),
+ ),
+ )
+ .for("update");
+ if (!live) throw new IdentityInputError(CREDENTIAL_UNAVAILABLE);
+ const [taken] = await transaction
+ .select({ id: identityLinks.id })
+ .from(identityLinks)
+ .where(
+ own.length
+ ? and(
+ eq(identityLinks.credentialId, credentialId),
+ notInArray(identityLinks.id, own),
+ )
+ : eq(identityLinks.credentialId, credentialId),
+ )
+ .limit(1);
+ if (taken) throw new IdentityInputError(CREDENTIAL_UNAVAILABLE);
+}
+
+/**
+ * Link `identity` to `userId` inside `transaction`. Module-private: only `linkVerified` (which
+ * validates the proof first) and `redeemChallenge` (no credential) call it.
*
* Two races are handled. Two people racing for one identity: the identity row is locked first, and a
* racing insert surfaces as a unique violation on the identity index, which is a refusal
@@ -89,11 +135,47 @@ async function revokeQuietly(
* per-person-per-realm advisory lock taken before any row lock, so the later link sees the earlier
* one as `previous` and replaces it.
*
- * Under the lock: linked to somebody else: refused. Linked to this person: updated in place.
- * Otherwise this person's older link in the same realm, if any, is removed with its credential, and
- * the new link inserted.
+ * Locks are taken advisory lock, then link rows, then the credential row: the order
+ * retireIdentityLinks uses, so the two cannot deadlock.
+ *
+ * Under the lock: linked to somebody else: refused. Linked to this person: re-verified. Otherwise
+ * this person's older link in the same realm, if any, is removed and the new link inserted.
+ *
+ * Credential and handle: `undefined` keeps what the link has; a new credential replaces the old
+ * one, which is revoked; there is no clearing a credential here (unlink does that). An explicit
+ * null handle clears it. A replaced older link's credential is revoked unless the new link is
+ * taking that same credential.
*/
-export async function writeLink(
+async function writeLink(
+ transaction: Transaction,
+ credentials: Revoke,
+ identity: Identity,
+ userId: string,
+ proof: {
+ method: LinkMethod;
+ handle?: string | null;
+ credentialId?: string;
+ },
+): Promise {
+ try {
+ return await writeLinkLocked(
+ transaction,
+ credentials,
+ identity,
+ userId,
+ proof,
+ );
+ } catch (error) {
+ const constraint = uniqueViolationConstraint(error);
+ if (constraint === "identity_links_identity_idx")
+ throw new IdentityConflictError();
+ if (constraint === "identity_links_credential_idx")
+ throw new IdentityInputError(CREDENTIAL_UNAVAILABLE);
+ throw error;
+ }
+}
+
+async function writeLinkLocked(
transaction: Transaction,
credentials: Revoke,
identity: Identity,
@@ -101,7 +183,7 @@ export async function writeLink(
proof: {
method: LinkMethod;
handle?: string | null;
- credentialId?: string | null;
+ credentialId?: string;
},
): Promise {
await transaction.execute(
@@ -115,14 +197,17 @@ export async function writeLink(
if (existing && existing.userId !== userId) throw new IdentityConflictError();
const now = new Date();
if (existing) {
- if (existing.credentialId && existing.credentialId !== proof.credentialId)
+ if (proof.credentialId)
+ await claimCredential(transaction, proof.credentialId, [existing.id]);
+ const credentialId = proof.credentialId ?? existing.credentialId;
+ if (existing.credentialId && existing.credentialId !== credentialId)
await revokeQuietly(credentials, existing.credentialId, transaction);
const [updated] = await transaction
.update(identityLinks)
.set({
- handle: proof.handle ?? existing.handle,
+ handle: proof.handle === undefined ? existing.handle : proof.handle,
verifiedBy: proof.method,
- credentialId: proof.credentialId ?? null,
+ credentialId,
status: "active",
updatedAt: now,
})
@@ -144,42 +229,42 @@ export async function writeLink(
),
)
.for("update");
+ if (proof.credentialId)
+ await claimCredential(
+ transaction,
+ proof.credentialId,
+ previous ? [previous.id] : [],
+ );
if (previous) {
await transaction
.delete(identityLinks)
.where(eq(identityLinks.id, previous.id));
- await revokeQuietly(credentials, previous.credentialId, transaction);
- }
- try {
- const [inserted] = await transaction
- .insert(identityLinks)
- .values({
- id: randomUUID(),
- provider: identity.provider,
- realm: identity.realm,
- subject: identity.subject,
- userId,
- handle: proof.handle ?? null,
- verifiedBy: proof.method,
- credentialId: proof.credentialId ?? null,
- status: "active",
- createdAt: now,
- updatedAt: now,
- })
- .returning();
- return toLink(
- inserted as typeof identityLinks.$inferSelect,
- identity.provider,
- );
- } catch (error) {
- if (uniqueViolationConstraint(error) === "identity_links_identity_idx")
- throw new IdentityConflictError();
- throw error;
+ // The new link may be taking this very credential: then it is moved, not revoked.
+ if (previous.credentialId !== proof.credentialId)
+ await revokeQuietly(credentials, previous.credentialId, transaction);
}
+ const [inserted] = await transaction
+ .insert(identityLinks)
+ .values({
+ id: randomUUID(),
+ provider: identity.provider,
+ realm: identity.realm,
+ subject: identity.subject,
+ userId,
+ handle: proof.handle ?? null,
+ verifiedBy: proof.method,
+ credentialId: proof.credentialId ?? null,
+ status: "active",
+ createdAt: now,
+ updatedAt: now,
+ })
+ .returning();
+ return toLink(
+ inserted as typeof identityLinks.$inferSelect,
+ identity.provider,
+ );
}
-const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
-
/** Refuses anything that is not a code OpenBot could have issued before it reaches a query. */
function codeHash(code: string) {
if (typeof code !== "string" || !UUID.test(code))
@@ -187,7 +272,7 @@ function codeHash(code: string) {
return createHash("sha256").update(code.toLowerCase()).digest("hex");
}
-export function createIdentityStore(database: Database, credentials: Vault) {
+export function createIdentityStore(database: Database, credentials: Revoke) {
return {
async linkedUser(
value: Identity,
@@ -236,11 +321,15 @@ export function createIdentityStore(database: Database, credentials: Vault) {
/**
* Link `value` to `userId` on proof the caller has already verified.
*
- * A `credentialId`, when given, must name a live credential: it is checked in the link's own
- * transaction, so a missing or revoked id is refused rather than stored as a token the link
- * does not have. A "challenge" link proves control by code and carries no credential, so it
- * refuses one. An "oauth" link is not required to carry one: GitHub sign-in may link before
- * a token is stored.
+ * A `credentialId`, when given, must be a UUID naming a live credential that no other link
+ * holds: it is checked and locked in the link's own transaction, so a malformed, missing,
+ * revoked or already-owned id is refused rather than stored as a token the link does not
+ * have. Omitting it keeps the credential an existing link already has (a re-sign-in that mints
+ * no token must not kill the working one); a different id replaces it and revokes the old one.
+ * Only unlink clears a credential. `handle` likewise: omitted keeps, null clears.
+ *
+ * A "challenge" link proves control by code and carries no credential, so it refuses one. An
+ * "oauth" link is not required to carry one: GitHub sign-in may link before a token is stored.
*/
async linkVerified(
value: Identity,
@@ -248,7 +337,7 @@ export function createIdentityStore(database: Database, credentials: Vault) {
proof: {
method: LinkMethod;
handle?: string | null;
- credentialId?: string | null;
+ credentialId?: string;
},
): Promise {
const identity = parseIdentity(value);
@@ -257,20 +346,25 @@ export function createIdentityStore(database: Database, credentials: Vault) {
throw new IdentityInputError(
"This provider does not accept that kind of proof.",
);
- if (proof.credentialId && proof.method === "challenge")
+ if (proof.credentialId != null && proof.method === "challenge")
throw new IdentityInputError(
"A challenge link does not carry a credential.",
);
- return database.transaction(async (transaction) => {
- if (
- proof.credentialId &&
- !(await credentials.isLive(proof.credentialId, transaction))
- )
- throw new IdentityInputError(
- "The credential for this link is not available.",
- );
- return writeLink(transaction, credentials, identity, userId, proof);
- });
+ // Shape first, so a malformed id never reaches Postgres (and its error never echoes it).
+ if (
+ proof.credentialId != null &&
+ (typeof proof.credentialId !== "string" ||
+ !UUID.test(proof.credentialId))
+ )
+ throw new IdentityInputError(CREDENTIAL_UNAVAILABLE);
+ // Lowercase: uuid columns read back lowercase, and keep/move compare ids as strings.
+ const credentialId = proof.credentialId?.toLowerCase() ?? undefined;
+ return database.transaction((transaction) =>
+ writeLink(transaction, credentials, identity, userId, {
+ ...proof,
+ credentialId,
+ }),
+ );
},
async markNeedsReconnect(linkId: string): Promise {
diff --git a/server/tests/identity-store.integration.test.ts b/server/tests/identity-store.integration.test.ts
index ddf94cd51..1a7b7d80a 100644
--- a/server/tests/identity-store.integration.test.ts
+++ b/server/tests/identity-store.integration.test.ts
@@ -148,13 +148,90 @@ test("concurrent links by one person in one realm: the later replaces the earlie
expect(mine).toHaveLength(1);
const [remaining] = mine;
expect([linkA.id, linkB.id]).toContain(remaining?.id);
- const live = [first, second].filter((id) => id === remaining?.credentialId);
- expect(live).toHaveLength(1);
- const liveness = await Promise.all(
- [first, second].map((id) => vault.isLive(id)),
- );
- expect(liveness.filter(Boolean)).toHaveLength(1);
+ expect([first, second]).toContain(remaining?.credentialId as string);
+ const loser = remaining?.credentialId === first ? second : first;
expect(await vault.isLive(remaining?.credentialId as string)).toBe(true);
+ expect(await vault.isLive(loser)).toBe(false);
+});
+
+test("relinking without a credentialId keeps the stored token live and on the link", async () => {
+ const person = user();
+ const identity = github();
+ const token = await credential();
+ await store.linkVerified(identity, person, {
+ method: "oauth",
+ handle: "dana",
+ credentialId: token,
+ });
+ const link = await store.linkVerified(identity, person, { method: "oauth" });
+ expect(link.credentialId).toBe(token);
+ expect(link.handle).toBe("dana");
+ expect(await vault.isLive(token)).toBe(true);
+});
+
+test("relinking with an explicit null handle clears it", async () => {
+ const person = user();
+ const identity = github();
+ await store.linkVerified(identity, person, {
+ method: "oauth",
+ handle: "dana",
+ });
+ const link = await store.linkVerified(identity, person, {
+ method: "oauth",
+ handle: null,
+ });
+ expect(link.handle).toBeNull();
+});
+
+test("a credential already on another link is refused and stays live", async () => {
+ const token = await credential();
+ const owner = await store.linkVerified(github(), user(), {
+ method: "oauth",
+ credentialId: token,
+ });
+ const identity = github();
+ await expect(
+ store.linkVerified(identity, user(), {
+ method: "oauth",
+ credentialId: token,
+ }),
+ ).rejects.toThrow(IdentityInputError);
+ expect(await store.linkedUser(identity)).toBeNull();
+ expect(await vault.isLive(token)).toBe(true);
+ const [still] = await database
+ .select({ credentialId: identityLinks.credentialId })
+ .from(identityLinks)
+ .where(eq(identityLinks.id, owner.id));
+ expect(still?.credentialId).toBe(token);
+});
+
+test("a credentialId that is not a UUID is refused before any query", async () => {
+ const identity = github();
+ const attempt = store.linkVerified(identity, user(), {
+ method: "oauth",
+ credentialId: "not-a-uuid",
+ });
+ await expect(attempt).rejects.toThrow(IdentityInputError);
+ await expect(attempt).rejects.not.toThrow(/not-a-uuid/);
+ expect(await store.linkedUser(identity)).toBeNull();
+});
+
+test("replacing a person's older link while moving its credential to the new one keeps it live", async () => {
+ const person = user();
+ const old = github();
+ const token = await credential();
+ await store.linkVerified(old, person, {
+ method: "oauth",
+ credentialId: token,
+ });
+ const replacement = github();
+ const link = await store.linkVerified(replacement, person, {
+ method: "oauth",
+ credentialId: token,
+ });
+ expect(await store.linkedUser(old)).toBeNull();
+ expect(link.credentialId).toBe(token);
+ expect(await vault.isLive(token)).toBe(true);
});
test("markNeedsReconnect is visible to linkedUser; relinking makes it active again", async () => {
From 0541aaae994c42d83ad4bcc3d9547e8e3e0c315e Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:29:03 -0300
Subject: [PATCH 037/101] fix(server): one live link code per person and
provider
---
server/drizzle/0052_identity_links.sql | 2 +-
server/drizzle/meta/0052_snapshot.json | 8 +++----
server/drizzle/meta/_journal.json | 2 +-
server/src/db/schema/identity.ts | 6 ++++-
server/src/identity/store.ts | 21 +++++++++--------
.../identity-challenges.integration.test.ts | 23 +++++++++++++++++++
6 files changed, 46 insertions(+), 16 deletions(-)
diff --git a/server/drizzle/0052_identity_links.sql b/server/drizzle/0052_identity_links.sql
index c2a6140d2..95b1fc4a4 100644
--- a/server/drizzle/0052_identity_links.sql
+++ b/server/drizzle/0052_identity_links.sql
@@ -23,7 +23,7 @@ CREATE TABLE "identity_links" (
--> statement-breakpoint
ALTER TABLE "identity_links" ADD CONSTRAINT "identity_links_credential_id_credentials_id_fk" FOREIGN KEY ("credential_id") REFERENCES "public"."credentials"("id") ON DELETE no action ON UPDATE no action;--> statement-breakpoint
CREATE INDEX "identity_link_challenges_expiry_idx" ON "identity_link_challenges" USING btree ("expires_at");--> statement-breakpoint
-CREATE INDEX "identity_link_challenges_user_idx" ON "identity_link_challenges" USING btree ("user_id","provider");--> statement-breakpoint
+CREATE UNIQUE INDEX "identity_link_challenges_user_provider_idx" ON "identity_link_challenges" USING btree ("user_id","provider");--> statement-breakpoint
CREATE UNIQUE INDEX "identity_links_identity_idx" ON "identity_links" USING btree ("provider","realm","subject");--> statement-breakpoint
CREATE UNIQUE INDEX "identity_links_user_realm_idx" ON "identity_links" USING btree ("user_id","provider","realm");--> statement-breakpoint
CREATE UNIQUE INDEX "identity_links_credential_idx" ON "identity_links" USING btree ("credential_id") WHERE "identity_links"."credential_id" IS NOT NULL;
\ No newline at end of file
diff --git a/server/drizzle/meta/0052_snapshot.json b/server/drizzle/meta/0052_snapshot.json
index 203d7789a..bc1c9f44c 100644
--- a/server/drizzle/meta/0052_snapshot.json
+++ b/server/drizzle/meta/0052_snapshot.json
@@ -1,5 +1,5 @@
{
- "id": "a5c96b02-0eca-4def-bcc4-590cdb66fb28",
+ "id": "f5896210-489f-45e4-a318-4056d4d88cf3",
"prevId": "b4bb328d-1308-403f-a2ac-f8a291aaabc5",
"version": "7",
"dialect": "postgresql",
@@ -8956,8 +8956,8 @@
"method": "btree",
"with": {}
},
- "identity_link_challenges_user_idx": {
- "name": "identity_link_challenges_user_idx",
+ "identity_link_challenges_user_provider_idx": {
+ "name": "identity_link_challenges_user_provider_idx",
"columns": [
{
"expression": "user_id",
@@ -8972,7 +8972,7 @@
"nulls": "last"
}
],
- "isUnique": false,
+ "isUnique": true,
"concurrently": false,
"method": "btree",
"with": {}
diff --git a/server/drizzle/meta/_journal.json b/server/drizzle/meta/_journal.json
index 3a967466c..0124a41d0 100644
--- a/server/drizzle/meta/_journal.json
+++ b/server/drizzle/meta/_journal.json
@@ -369,7 +369,7 @@
{
"idx": 52,
"version": "7",
- "when": 1791473240143,
+ "when": 1791473326630,
"tag": "0052_identity_links",
"breakpoints": true
}
diff --git a/server/src/db/schema/identity.ts b/server/src/db/schema/identity.ts
index 2a96dff91..952d4626f 100644
--- a/server/src/db/schema/identity.ts
+++ b/server/src/db/schema/identity.ts
@@ -81,6 +81,10 @@ export const identityLinkChallenges = pgTable(
},
(table) => [
index("identity_link_challenges_expiry_idx").on(table.expiresAt),
- index("identity_link_challenges_user_idx").on(table.userId, table.provider),
+ // One live code per person per provider, enforced here rather than by the code that issues them.
+ uniqueIndex("identity_link_challenges_user_provider_idx").on(
+ table.userId,
+ table.provider,
+ ),
],
);
diff --git a/server/src/identity/store.ts b/server/src/identity/store.ts
index 161ae033f..db8ae7ed8 100644
--- a/server/src/identity/store.ts
+++ b/server/src/identity/store.ts
@@ -407,15 +407,6 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
);
const code = randomUUID();
return database.transaction(async (transaction) => {
- // One live code per person per provider: a new one replaces any earlier one.
- await transaction
- .delete(identityLinkChallenges)
- .where(
- and(
- eq(identityLinkChallenges.userId, userId),
- eq(identityLinkChallenges.provider, provider),
- ),
- );
// No sweep exists, so codes that were never sent would otherwise accumulate.
await transaction
.delete(identityLinkChallenges)
@@ -433,6 +424,18 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
userId,
expiresAt: sql`clock_timestamp() + interval '10 minutes'`,
})
+ // One live code per person per provider, enforced by the unique index: a new code
+ // replaces any earlier one, even when two requests race.
+ .onConflictDoUpdate({
+ target: [
+ identityLinkChallenges.userId,
+ identityLinkChallenges.provider,
+ ],
+ set: {
+ tokenHash: sql`excluded.token_hash`,
+ expiresAt: sql`excluded.expires_at`,
+ },
+ })
.returning({ expiresAt: identityLinkChallenges.expiresAt });
if (!challenge) throw new IdentityLinkError();
return { code, expiresAt: challenge.expiresAt };
diff --git a/server/tests/identity-challenges.integration.test.ts b/server/tests/identity-challenges.integration.test.ts
index e13c5b780..689368f3b 100644
--- a/server/tests/identity-challenges.integration.test.ts
+++ b/server/tests/identity-challenges.integration.test.ts
@@ -228,3 +228,26 @@ test("redeeming a second code in the same realm replaces the person's older link
expect(await store.linkedUser(first)).toBeNull();
expect((await store.linkedUser(second))?.userId).toBe(owner);
});
+
+test("concurrent requests still leave one live code per person and provider", async () => {
+ const owner = user();
+ const issued = await Promise.all(
+ Array.from({ length: 10 }, () => store.issueChallenge(owner, "slack")),
+ );
+ const rows = await database
+ .select()
+ .from(identityLinkChallenges)
+ .where(eq(identityLinkChallenges.userId, owner));
+ expect(rows).toHaveLength(1);
+ const live = issued.filter(({ code }) => hashOf(code) === rows[0]?.tokenHash);
+ expect(live).toHaveLength(1);
+ for (const { code } of issued) {
+ if (code === live[0]?.code) continue;
+ await expect(store.redeemChallenge(code, slack())).rejects.toBeInstanceOf(
+ IdentityLinkError,
+ );
+ }
+ expect(
+ (await store.redeemChallenge(live[0]?.code ?? "", slack())).userId,
+ ).toBe(owner);
+});
From 12482a8f774b8577ff49b628e4377e952f0cd0f5 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:29:50 -0300
Subject: [PATCH 038/101] fix(server): audit the link that was actually removed
unlink now returns the removed link's provider (read by DELETE ... RETURNING
in the same transaction) or null, and the DELETE route audits that instead of
a separate identitiesFor lookup that could miss a concurrently removed link or
a provider outside the registry. The comment about retries is corrected: the
audit is written after the unlink commits, so a failed write fails the request
with the link already gone, as for mcp.account_disconnected.
unlink call sites: server/src/identity/routes.ts (DELETE /links/:id),
server/tests/identity-routes.test.ts (fake store),
server/tests/identity-store.integration.test.ts.
---
server/src/identity/routes.ts | 8 +++-----
server/src/identity/store.ts | 15 +++++++++++----
server/tests/identity-routes.test.ts | 18 ++++++++++++++++--
.../tests/identity-store.integration.test.ts | 8 +++++---
4 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/server/src/identity/routes.ts b/server/src/identity/routes.ts
index a0066c86c..b95359a34 100644
--- a/server/src/identity/routes.ts
+++ b/server/src/identity/routes.ts
@@ -47,16 +47,14 @@ export function identityRoutes(
app.delete("/links/:id", async (context) => {
const actorId = context.var.actor.id;
const id = context.req.param("id");
- const link = (await live.identitiesFor(actorId)).find(
- (candidate) => candidate.id === id,
- );
const removed = await live.unlink(actorId, id);
if (!removed)
return context.json({ error: "Linked account not found." }, 404);
/*
* After the removal, and not caught: the same order and the same failure as the connected-account
* disconnect (`mcp.account_disconnected`), where an audit write that throws fails the request.
- * Repeating it is safe because the link is already gone and answers 404.
+ * The audit is written after the unlink commits, so such a failure leaves the link already gone
+ * and the request failed; a retry answers 404 and writes no audit row.
*
* Names the link and its provider only. The subject, realm and credential id stay out of the
* trail, as they stay out of every response here.
@@ -67,7 +65,7 @@ export function identityRoutes(
targetType: "identity_link",
targetId: id,
actorUserId: actorId,
- payload: { actor: actorId, provider: link?.provider },
+ payload: { actor: actorId, provider: removed.provider },
});
}
return context.body(null, 204);
diff --git a/server/src/identity/store.ts b/server/src/identity/store.ts
index db8ae7ed8..7f3ede92f 100644
--- a/server/src/identity/store.ts
+++ b/server/src/identity/store.ts
@@ -374,17 +374,24 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
.where(eq(identityLinks.id, linkId));
},
- async unlink(userId: string, linkId: string): Promise {
+ /** The removed link's provider as stored, or null when the asker has no such link. */
+ async unlink(
+ userId: string,
+ linkId: string,
+ ): Promise<{ provider: string } | null> {
return database.transaction(async (transaction) => {
const [removed] = await transaction
.delete(identityLinks)
.where(
and(eq(identityLinks.id, linkId), eq(identityLinks.userId, userId)),
)
- .returning({ credentialId: identityLinks.credentialId });
- if (!removed) return false;
+ .returning({
+ credentialId: identityLinks.credentialId,
+ provider: identityLinks.provider,
+ });
+ if (!removed) return null;
await revokeQuietly(credentials, removed.credentialId, transaction);
- return true;
+ return { provider: removed.provider };
});
},
diff --git a/server/tests/identity-routes.test.ts b/server/tests/identity-routes.test.ts
index 07c3c8d14..ef133dde4 100644
--- a/server/tests/identity-routes.test.ts
+++ b/server/tests/identity-routes.test.ts
@@ -30,7 +30,7 @@ const link: IdentityLink = {
updatedAt: new Date("2026-10-08T00:00:00Z"),
};
-function fixture() {
+function fixture(removedProvider = "github") {
const calls: string[] = [];
const events: AuditEventInput[] = [];
const app = identityRoutes(
@@ -39,7 +39,9 @@ function fixture() {
identitiesFor: async (userId) => (userId === "person" ? [link] : []),
unlink: async (userId, id) => {
calls.push(`unlink:${userId}:${id}`);
- return userId === "person" && id === "link-1";
+ return userId === "person" && id === "link-1"
+ ? { provider: removedProvider }
+ : null;
},
},
{ insert: async (event) => void events.push(event) },
@@ -130,3 +132,15 @@ test("a 404 DELETE records nothing", async () => {
await f.app.request("/links/someone-elses", { method: "DELETE" });
expect(f.events).toEqual([]);
});
+
+test("the audit names the provider the store removed, even one outside the registry", async () => {
+ const f = fixture("retired-provider");
+ expect(
+ (await f.app.request("/links/link-1", { method: "DELETE" })).status,
+ ).toBe(204);
+ expect(f.events).toHaveLength(1);
+ expect(f.events[0]?.payload).toEqual({
+ actor: "person",
+ provider: "retired-provider",
+ });
+});
diff --git a/server/tests/identity-store.integration.test.ts b/server/tests/identity-store.integration.test.ts
index 1a7b7d80a..212e8d68b 100644
--- a/server/tests/identity-store.integration.test.ts
+++ b/server/tests/identity-store.integration.test.ts
@@ -252,12 +252,14 @@ test("unlink removes only the asker's own link and revokes its credential", asyn
method: "oauth",
credentialId: token,
});
- expect(await store.unlink(user(), link.id)).toBe(false);
+ expect(await store.unlink(user(), link.id)).toBeNull();
expect(await store.linkedUser(identity)).not.toBeNull();
- expect(await store.unlink(person, link.id)).toBe(true);
+ expect(await store.unlink(person, link.id)).toEqual({
+ provider: "github",
+ });
expect(await store.linkedUser(identity)).toBeNull();
expect(await vault.isLive(token)).toBe(false);
- expect(await store.unlink(person, link.id)).toBe(false);
+ expect(await store.unlink(person, link.id)).toBeNull();
});
test("a link outlives its user: nothing cascades from users", async () => {
From 12be7b95ed1c88af973ca91165208d735a3995bf Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:31:20 -0300
Subject: [PATCH 039/101] fix(server): mark the linked-accounts 503 as not
configured
---
server/src/identity/routes.ts | 12 +++++++++++-
server/tests/identity-routes.test.ts | 10 +++++++++-
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/server/src/identity/routes.ts b/server/src/identity/routes.ts
index b95359a34..bb9f30b90 100644
--- a/server/src/identity/routes.ts
+++ b/server/src/identity/routes.ts
@@ -23,8 +23,18 @@ export function identityRoutes(
});
app.use("*", requireUser);
if (!store) {
+ /*
+ * The code is what tells the settings page "this deployment has no identity store" apart from a
+ * proxy or platform 503, which must surface as an error rather than hide the section.
+ */
app.all("*", (context) =>
- context.json({ error: "Linked accounts are not available." }, 503),
+ context.json(
+ {
+ error: "Linked accounts are not available.",
+ code: "identity_unavailable",
+ },
+ 503,
+ ),
);
return app;
}
diff --git a/server/tests/identity-routes.test.ts b/server/tests/identity-routes.test.ts
index ef133dde4..bb85b209f 100644
--- a/server/tests/identity-routes.test.ts
+++ b/server/tests/identity-routes.test.ts
@@ -100,7 +100,15 @@ test("responses are not cached", async () => {
test("without a store every route is 503", async () => {
const app = identityRoutes(signedIn);
- expect((await app.request("/links")).status).toBe(503);
+ const response = await app.request("/links");
+ expect(response.status).toBe(503);
+ expect(await response.json()).toEqual({
+ error: "Linked accounts are not available.",
+ code: "identity_unavailable",
+ });
+ const removal = await app.request("/links/x", { method: "DELETE" });
+ expect(removal.status).toBe(503);
+ expect((await removal.json()).code).toBe("identity_unavailable");
});
test("auth runs before the missing-store 503 and responses are never cached", async () => {
From 901f5976bc4316d5672170da89e3418aee14cc54 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:31:21 -0300
Subject: [PATCH 040/101] fix(app): per-row disconnect and honest
linked-account errors
---
.../settings/linked-accounts.test.tsx | 51 +++++++++-
.../components/settings/linked-accounts.tsx | 99 ++++++++++---------
app/src/lib/identity/mutations.ts | 31 ++++--
app/src/lib/identity/queries.ts | 24 +++--
4 files changed, 141 insertions(+), 64 deletions(-)
diff --git a/app/src/components/settings/linked-accounts.test.tsx b/app/src/components/settings/linked-accounts.test.tsx
index 29bc89ade..a1c8d948c 100644
--- a/app/src/components/settings/linked-accounts.test.tsx
+++ b/app/src/components/settings/linked-accounts.test.tsx
@@ -1,5 +1,6 @@
import { expect, test } from "bun:test";
import { IconBrandGithub, IconBrandSlack, IconLink } from "@tabler/icons-react";
+import { unlinkOutcome } from "@/lib/identity/mutations";
import { linksFromResponse } from "@/lib/identity/queries";
import { linkedAccountDescription, providerIcon } from "./linked-accounts";
@@ -48,14 +49,34 @@ test("providers map to their own icon with a neutral fallback", () => {
expect(providerIcon("gitlab")).toBe(IconLink);
});
-test("a 503 means the feature is absent, so no links", async () => {
+test("the not-configured 503 means the feature is absent, so no links", async () => {
const response = Response.json(
- { error: "Linked accounts are not available." },
+ {
+ error: "Linked accounts are not available.",
+ code: "identity_unavailable",
+ },
{ status: 503 },
);
expect(await linksFromResponse(response)).toEqual([]);
});
+test("any other 503 is an outage and throws its message", async () => {
+ const response = Response.json(
+ { error: "Service Unavailable" },
+ { status: 503 },
+ );
+ await expect(linksFromResponse(response)).rejects.toThrow(
+ "Service Unavailable",
+ );
+});
+
+test("a 503 with no JSON body throws the fallback", async () => {
+ const response = new Response("upstream down", { status: 503 });
+ await expect(linksFromResponse(response)).rejects.toThrow(
+ "Could not load your linked accounts",
+ );
+});
+
test("other failures still throw the server message", async () => {
const response = Response.json({ error: "boom" }, { status: 500 });
await expect(linksFromResponse(response)).rejects.toThrow("boom");
@@ -65,3 +86,29 @@ test("a success unwraps the links", async () => {
const links = [{ ...base, handle: "dana", status: "active" as const }];
expect(await linksFromResponse(Response.json({ links }))).toEqual(links);
});
+
+test("a 204 disconnect succeeds", async () => {
+ await expect(
+ unlinkOutcome(new Response(null, { status: 204 })),
+ ).resolves.toBeUndefined();
+});
+
+test("a 404 disconnect means the link is already gone, which is success", async () => {
+ await expect(
+ unlinkOutcome(
+ Response.json({ error: "Linked account not found." }, { status: 404 }),
+ ),
+ ).resolves.toBeUndefined();
+});
+
+test("any other disconnect failure throws the server message", async () => {
+ await expect(
+ unlinkOutcome(Response.json({ error: "boom" }, { status: 500 })),
+ ).rejects.toThrow("boom");
+});
+
+test("a disconnect failure without a message throws the fallback", async () => {
+ await expect(
+ unlinkOutcome(new Response("nope", { status: 502 })),
+ ).rejects.toThrow("The account could not be disconnected");
+});
diff --git a/app/src/components/settings/linked-accounts.tsx b/app/src/components/settings/linked-accounts.tsx
index 0ddaf63c2..a386e4ff8 100644
--- a/app/src/components/settings/linked-accounts.tsx
+++ b/app/src/components/settings/linked-accounts.tsx
@@ -48,64 +48,69 @@ export function providerIcon(provider: string) {
*/
export function LinkedAccountsSection() {
const links = useQuery(linkedAccountsQueryOptions());
- const unlink = useMutation(unlinkMutationOptions(queryClient));
- if (links.isPending) return null;
+ // Rows win over the error: a failed background refetch must not replace a list already on screen.
+ if (links.data) {
+ // Deliberately hidden when nothing is linked, or when this deployment has no identity store.
+ if (links.data.length === 0) return null;
+ return (
+
+
+ {links.data.map((account, index) => (
+
+ {index > 0 ? : null}
+
+
+ ))}
+
+
+ );
+ }
if (links.error) {
return (
- Your linked accounts could not be loaded.
+ Your linked accounts could not be loaded: {links.error.message}
);
}
- const failedTitle = links.data.find(
- (account) => account.id === unlink.variables,
- )?.title;
- // Deliberately hidden when nothing is linked, or when this deployment has no identity store.
- if (links.data.length === 0) return null;
+ return null;
+}
+
+/**
+ * One linked account with its own disconnect, so a second click elsewhere neither re-enables this
+ * row mid-request nor overwrites its error.
+ */
+function LinkedAccountRow({ account }: { account: LinkedAccount }) {
+ const unlink = useMutation(unlinkMutationOptions(queryClient));
+ const Icon = providerIcon(account.provider);
return (
-
-
- {links.data.map((account, index) => {
- const Icon = providerIcon(account.provider);
- return (
-
- Could not disconnect {failedTitle ?? "that account"}:{" "}
- {unlink.error.message}
+
+ Could not disconnect {account.title}: {unlink.error.message}
) : null}
-
+ >
);
}
diff --git a/app/src/lib/identity/mutations.ts b/app/src/lib/identity/mutations.ts
index 51e4a8a4c..c7d5557eb 100644
--- a/app/src/lib/identity/mutations.ts
+++ b/app/src/lib/identity/mutations.ts
@@ -1,15 +1,32 @@
import { mutationOptions, type QueryClient } from "@tanstack/react-query";
-import { client } from "@/lib/client";
+import { tryClient } from "@/lib/client";
import { identityKeys } from "./queries";
+const UNLINK_FALLBACK = "The account could not be disconnected";
+
+/**
+ * A 404 means the link is already gone — removed in another tab, or by a retry after a failed audit
+ * write — which is what the person asked for, so it is success rather than an error.
+ */
+export async function unlinkOutcome(response: Response): Promise {
+ if (response.ok || response.status === 404) return;
+ const message = await response
+ .json()
+ .then((body: { error?: unknown }) =>
+ typeof body?.error === "string" ? body.error : undefined,
+ )
+ .catch(() => undefined);
+ throw new Error(message ?? UNLINK_FALLBACK);
+}
+
export function unlinkMutationOptions(queryClient: QueryClient) {
return mutationOptions({
- mutationFn: async (id: string): Promise => {
- await client(`/api/identity/links/${encodeURIComponent(id)}`, {
- method: "DELETE",
- fallback: "The account could not be disconnected",
- });
- },
+ mutationFn: async (id: string): Promise =>
+ unlinkOutcome(
+ await tryClient(`/api/identity/links/${encodeURIComponent(id)}`, {
+ method: "DELETE",
+ }),
+ ),
onSettled: () =>
queryClient.invalidateQueries({ queryKey: identityKeys.links() }),
});
diff --git a/app/src/lib/identity/queries.ts b/app/src/lib/identity/queries.ts
index 95b8449cf..91dcb8a27 100644
--- a/app/src/lib/identity/queries.ts
+++ b/app/src/lib/identity/queries.ts
@@ -17,20 +17,28 @@ export const identityKeys = {
const LOAD_FALLBACK = "Could not load your linked accounts";
+/** The machine-readable code on the server's own "no identity store here" 503. */
+export const IDENTITY_UNAVAILABLE = "identity_unavailable";
+
/**
- * A deployment without an identity store answers 503 "not available". That is the feature being
- * absent, not the server breaking, so it reads as no links and the section hides itself.
+ * A deployment without an identity store answers 503 with `code: "identity_unavailable"`. That is
+ * the feature being absent, not the server breaking, so it reads as no links and the section hides
+ * itself. Any other failure, including a 503 from a proxy or the platform, throws so it is shown.
*/
export async function linksFromResponse(
response: Response,
): Promise {
- if (response.status === 503) return [];
if (!response.ok) {
- const message = await response
- .json()
- .then((body: { error?: string }) => body.error)
- .catch(() => undefined);
- throw new Error(message ?? LOAD_FALLBACK);
+ const body = (await response.json().catch(() => null)) as {
+ error?: unknown;
+ code?: unknown;
+ } | null;
+ if (response.status === 503 && body?.code === IDENTITY_UNAVAILABLE) {
+ return [];
+ }
+ throw new Error(
+ typeof body?.error === "string" ? body.error : LOAD_FALLBACK,
+ );
}
const body = (await response.json().catch(() => null)) as {
links?: LinkedAccount[];
From cd1d7aa675869765f53a873ed59ccc3c5713b884 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:46:13 -0300
Subject: [PATCH 041/101] fix(server): tag a missing linked account
---
server/src/identity/routes.ts | 9 ++++++++-
server/tests/identity-routes.test.ts | 14 +++++++++-----
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/server/src/identity/routes.ts b/server/src/identity/routes.ts
index bb9f30b90..8f6734e48 100644
--- a/server/src/identity/routes.ts
+++ b/server/src/identity/routes.ts
@@ -58,8 +58,15 @@ export function identityRoutes(
const actorId = context.var.actor.id;
const id = context.req.param("id");
const removed = await live.unlink(actorId, id);
+ /*
+ * The code is what tells the settings page "this link is already gone" apart from a proxy,
+ * missing-route or SPA-fallback 404, which must surface as a failed disconnect.
+ */
if (!removed)
- return context.json({ error: "Linked account not found." }, 404);
+ return context.json(
+ { error: "Linked account not found.", code: "identity_link_not_found" },
+ 404,
+ );
/*
* After the removal, and not caught: the same order and the same failure as the connected-account
* disconnect (`mcp.account_disconnected`), where an audit write that throws fails the request.
diff --git a/server/tests/identity-routes.test.ts b/server/tests/identity-routes.test.ts
index bb85b209f..f2fd0f945 100644
--- a/server/tests/identity-routes.test.ts
+++ b/server/tests/identity-routes.test.ts
@@ -74,11 +74,15 @@ test("DELETE of the asker's own link is 204", async () => {
expect(f.calls).toEqual(["unlink:person:link-1"]);
});
-test("DELETE of another person's link is 404", async () => {
- expect(
- (await fixture().app.request("/links/someone-elses", { method: "DELETE" }))
- .status,
- ).toBe(404);
+test("DELETE of another person's link is a tagged 404", async () => {
+ const response = await fixture().app.request("/links/someone-elses", {
+ method: "DELETE",
+ });
+ expect(response.status).toBe(404);
+ expect(await response.json()).toEqual({
+ error: "Linked account not found.",
+ code: "identity_link_not_found",
+ });
});
test("the chat-started confirmation routes are gone", async () => {
From 9dc92601e1c4f2baf09c3d4d0381ad0ebffdd8c2 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:46:26 -0300
Subject: [PATCH 042/101] fix(app): only the server's own not-found counts as
disconnected
---
.../settings/linked-accounts.test.tsx | 24 ++++++++++++++--
app/src/lib/identity/mutations.ts | 28 ++++++++++++-------
2 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/app/src/components/settings/linked-accounts.test.tsx b/app/src/components/settings/linked-accounts.test.tsx
index a1c8d948c..bea42fbe3 100644
--- a/app/src/components/settings/linked-accounts.test.tsx
+++ b/app/src/components/settings/linked-accounts.test.tsx
@@ -93,14 +93,34 @@ test("a 204 disconnect succeeds", async () => {
).resolves.toBeUndefined();
});
-test("a 404 disconnect means the link is already gone, which is success", async () => {
+test("the server's own 404 means the link is already gone, which is success", async () => {
await expect(
unlinkOutcome(
- Response.json({ error: "Linked account not found." }, { status: 404 }),
+ Response.json(
+ { error: "Linked account not found.", code: "identity_link_not_found" },
+ { status: 404 },
+ ),
),
).resolves.toBeUndefined();
});
+test("a 404 without the server's code is a failed disconnect", async () => {
+ await expect(
+ unlinkOutcome(Response.json({ error: "Not Found" }, { status: 404 })),
+ ).rejects.toThrow("Not Found");
+});
+
+test("an HTML 404 from a proxy or SPA fallback throws the fallback", async () => {
+ await expect(
+ unlinkOutcome(
+ new Response("", {
+ status: 404,
+ headers: { "content-type": "text/html" },
+ }),
+ ),
+ ).rejects.toThrow("The account could not be disconnected");
+});
+
test("any other disconnect failure throws the server message", async () => {
await expect(
unlinkOutcome(Response.json({ error: "boom" }, { status: 500 })),
diff --git a/app/src/lib/identity/mutations.ts b/app/src/lib/identity/mutations.ts
index c7d5557eb..28c212c2a 100644
--- a/app/src/lib/identity/mutations.ts
+++ b/app/src/lib/identity/mutations.ts
@@ -4,19 +4,27 @@ import { identityKeys } from "./queries";
const UNLINK_FALLBACK = "The account could not be disconnected";
+/** The machine-readable code on the server's own "no such link" 404. */
+export const IDENTITY_LINK_NOT_FOUND = "identity_link_not_found";
+
/**
- * A 404 means the link is already gone — removed in another tab, or by a retry after a failed audit
- * write — which is what the person asked for, so it is success rather than an error.
+ * The server's own 404, tagged `code: "identity_link_not_found"`, means the link is already gone —
+ * removed in another tab, or by a retry after a failed audit write — which is what the person asked
+ * for, so it is success rather than an error. Any other 404 (a proxy, a missing route, an SPA
+ * fallback page) says nothing about the link, so it throws: the credential may still be live.
*/
export async function unlinkOutcome(response: Response): Promise {
- if (response.ok || response.status === 404) return;
- const message = await response
- .json()
- .then((body: { error?: unknown }) =>
- typeof body?.error === "string" ? body.error : undefined,
- )
- .catch(() => undefined);
- throw new Error(message ?? UNLINK_FALLBACK);
+ if (response.ok) return;
+ const body = (await response.json().catch(() => null)) as {
+ error?: unknown;
+ code?: unknown;
+ } | null;
+ if (response.status === 404 && body?.code === IDENTITY_LINK_NOT_FOUND) {
+ return;
+ }
+ throw new Error(
+ typeof body?.error === "string" ? body.error : UNLINK_FALLBACK,
+ );
}
export function unlinkMutationOptions(queryClient: QueryClient) {
From 55d234c197a3ed669d4b1873955e3c2f11e73acb Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:46:50 -0300
Subject: [PATCH 043/101] fix(app): show linked-account load errors alongside
stale rows
---
.../settings/linked-accounts.test.tsx | 45 ++++++++++++++-
.../components/settings/linked-accounts.tsx | 55 ++++++++++++-------
2 files changed, 80 insertions(+), 20 deletions(-)
diff --git a/app/src/components/settings/linked-accounts.test.tsx b/app/src/components/settings/linked-accounts.test.tsx
index bea42fbe3..650b36ef6 100644
--- a/app/src/components/settings/linked-accounts.test.tsx
+++ b/app/src/components/settings/linked-accounts.test.tsx
@@ -2,7 +2,11 @@ import { expect, test } from "bun:test";
import { IconBrandGithub, IconBrandSlack, IconLink } from "@tabler/icons-react";
import { unlinkOutcome } from "@/lib/identity/mutations";
import { linksFromResponse } from "@/lib/identity/queries";
-import { linkedAccountDescription, providerIcon } from "./linked-accounts";
+import {
+ linkedAccountDescription,
+ linkedAccountsSectionState,
+ providerIcon,
+} from "./linked-accounts";
const base = {
id: "1",
@@ -87,6 +91,45 @@ test("a success unwraps the links", async () => {
expect(await linksFromResponse(Response.json({ links }))).toEqual(links);
});
+test("a 200 whose body has no links array throws the fallback", async () => {
+ await expect(
+ linksFromResponse(Response.json({ accounts: [] })),
+ ).rejects.toThrow("Could not load your linked accounts");
+ await expect(
+ linksFromResponse(new Response("", { status: 200 })),
+ ).rejects.toThrow("Could not load your linked accounts");
+});
+
+const row = { ...base, handle: "dana", status: "active" as const };
+
+test("the section hides while loading and when nothing is linked", () => {
+ expect(linkedAccountsSectionState({ data: undefined, error: null })).toBe(
+ null,
+ );
+ expect(linkedAccountsSectionState({ data: [], error: null })).toBe(null);
+});
+
+test("rows show on their own when the list loaded", () => {
+ expect(linkedAccountsSectionState({ data: [row], error: null })).toEqual({
+ rows: [row],
+ error: null,
+ });
+});
+
+test("a failed refetch keeps stale rows on screen and shows the error with them", () => {
+ expect(
+ linkedAccountsSectionState({ data: [row], error: new Error("boom") }),
+ ).toEqual({ rows: [row], error: "boom" });
+});
+
+test("an error with no data, or with an empty cached list, still shows", () => {
+ for (const data of [undefined, []]) {
+ expect(
+ linkedAccountsSectionState({ data, error: new Error("boom") }),
+ ).toEqual({ rows: [], error: "boom" });
+ }
+});
+
test("a 204 disconnect succeeds", async () => {
await expect(
unlinkOutcome(new Response(null, { status: 204 })),
diff --git a/app/src/components/settings/linked-accounts.tsx b/app/src/components/settings/linked-accounts.tsx
index a386e4ff8..2fa665297 100644
--- a/app/src/components/settings/linked-accounts.tsx
+++ b/app/src/components/settings/linked-accounts.tsx
@@ -40,6 +40,26 @@ export function providerIcon(provider: string) {
);
}
+/**
+ * What the section draws: nothing, rows, an error, or rows with an error under them.
+ *
+ * Hidden while loading and when nothing is linked (or the deployment has no identity store). A
+ * failed refetch keeps the rows already on screen — stale but visible — and says the list could not
+ * be refreshed, so a removed account that is still drawn is never shown without that warning. An
+ * error with no rows to keep, including over a cached empty list, shows on its own.
+ */
+export function linkedAccountsSectionState({
+ data,
+ error,
+}: {
+ data: LinkedAccount[] | undefined;
+ error: Error | null;
+}): { rows: LinkedAccount[]; error: string | null } | null {
+ const rows = data ?? [];
+ if (error) return { rows, error: error.message };
+ return rows.length > 0 ? { rows, error: null } : null;
+}
+
/**
* Who you are in the chat apps and code hosts this deployment talks to.
*
@@ -48,33 +68,30 @@ export function providerIcon(provider: string) {
*/
export function LinkedAccountsSection() {
const links = useQuery(linkedAccountsQueryOptions());
- // Rows win over the error: a failed background refetch must not replace a list already on screen.
- if (links.data) {
- // Deliberately hidden when nothing is linked, or when this deployment has no identity store.
- if (links.data.length === 0) return null;
- return (
-
+ const state = linkedAccountsSectionState({
+ data: links.data,
+ error: links.error,
+ });
+ if (!state) return null;
+ return (
+
+ {state.rows.length > 0 ? (
- {links.data.map((account, index) => (
+ {state.rows.map((account, index) => (
- Your linked accounts could not be loaded: {links.error.message}
+ ) : null}
+ {state.error ? (
+
+ Your linked accounts could not be loaded: {state.error}
-
- );
- }
- return null;
+ ) : null}
+
+ );
}
/**
From 1f3ae8872a0ed39c9ff705426ae00a5d90ad8959 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:46:55 -0300
Subject: [PATCH 044/101] test(server): pin which identity constraint fired
---
.../tests/identity-schema.integration.test.ts | 126 +++++++++++++++---
1 file changed, 106 insertions(+), 20 deletions(-)
diff --git a/server/tests/identity-schema.integration.test.ts b/server/tests/identity-schema.integration.test.ts
index 847c9f3f2..d8f45e10e 100644
--- a/server/tests/identity-schema.integration.test.ts
+++ b/server/tests/identity-schema.integration.test.ts
@@ -1,18 +1,52 @@
import { afterAll, expect, test } from "bun:test";
import { randomUUID } from "node:crypto";
-import { eq } from "drizzle-orm";
+import { eq, inArray } from "drizzle-orm";
+import { createCredentialStore } from "../src/credentials";
import { createDatabase } from "../src/db/client";
-import { identityLinks } from "../src/db/schema";
+import {
+ credentials,
+ identityLinkChallenges,
+ identityLinks,
+} from "../src/db/schema";
import { TEST_POOL, testDatabaseUrl } from "./support/database";
const database = createDatabase(testDatabaseUrl(), TEST_POOL);
+const credentialStore = createCredentialStore(database);
const realm = `test-${randomUUID()}`;
+const challengeUsers = [`user-${randomUUID()}`];
+const credentialIds: string[] = [];
afterAll(async () => {
await database.delete(identityLinks).where(eq(identityLinks.realm, realm));
+ await database
+ .delete(identityLinkChallenges)
+ .where(inArray(identityLinkChallenges.userId, challengeUsers));
+ if (credentialIds.length > 0)
+ await database
+ .delete(credentials)
+ .where(inArray(credentials.id, credentialIds));
await database.$client.close();
});
+/**
+ * Which constraint a rejected write tripped. Drizzle wraps the driver error, so SQLSTATE and
+ * constraint name sit on `cause`. A bare `rejects.toThrow()` would pass on any failure at all.
+ */
+async function violation(promise: Promise) {
+ const error = await promise.then(
+ () => {
+ throw new Error("expected the write to be rejected");
+ },
+ (caught: unknown) => caught,
+ );
+ const driver = (error as { cause?: unknown }).cause ?? error;
+ const { errno, constraint } = driver as {
+ errno?: unknown;
+ constraint?: unknown;
+ };
+ return { sqlState: errno, constraint };
+}
+
const row = (subject: string, userId: string) => ({
id: randomUUID(),
provider: "slack",
@@ -24,29 +58,81 @@ const row = (subject: string, userId: string) => ({
test("an outside account links to one user", async () => {
await database.insert(identityLinks).values(row("U1", "user-a"));
- await expect(
- database.insert(identityLinks).values(row("U1", "user-b")).execute(),
- ).rejects.toThrow();
+ expect(
+ await violation(
+ database.insert(identityLinks).values(row("U1", "user-b")).execute(),
+ ),
+ ).toEqual({ sqlState: "23505", constraint: "identity_links_identity_idx" });
});
test("a user has one account per provider and realm", async () => {
await database.insert(identityLinks).values(row("U2", "user-c"));
- await expect(
- database.insert(identityLinks).values(row("U3", "user-c")).execute(),
- ).rejects.toThrow();
+ expect(
+ await violation(
+ database.insert(identityLinks).values(row("U3", "user-c")).execute(),
+ ),
+ ).toEqual({ sqlState: "23505", constraint: "identity_links_user_realm_idx" });
+});
+
+test("one credential belongs to one link", async () => {
+ const { id } = await credentialStore.create({
+ kind: "connector",
+ provider: "github-user-token",
+ keyId: randomUUID(),
+ metadata: {},
+ encryptedValue: "x",
+ });
+ credentialIds.push(id);
+ await database
+ .insert(identityLinks)
+ .values({ ...row("U6", "user-f"), credentialId: id });
+ expect(
+ await violation(
+ database
+ .insert(identityLinks)
+ .values({ ...row("U7", "user-g"), credentialId: id })
+ .execute(),
+ ),
+ ).toEqual({ sqlState: "23505", constraint: "identity_links_credential_idx" });
+});
+
+test("a user has one live link code per provider", async () => {
+ const userId = challengeUsers[0] as string;
+ const challenge = () => ({
+ tokenHash: randomUUID(),
+ provider: "slack",
+ userId,
+ expiresAt: new Date(Date.now() + 600_000),
+ });
+ await database.insert(identityLinkChallenges).values(challenge());
+ expect(
+ await violation(
+ database.insert(identityLinkChallenges).values(challenge()).execute(),
+ ),
+ ).toEqual({
+ sqlState: "23505",
+ constraint: "identity_link_challenges_user_provider_idx",
+ });
});
test("verified_by and status are constrained", async () => {
- await expect(
- database
- .insert(identityLinks)
- .values({ ...row("U4", "user-d"), verifiedBy: "guess" as never })
- .execute(),
- ).rejects.toThrow();
- await expect(
- database
- .insert(identityLinks)
- .values({ ...row("U5", "user-e"), status: "gone" as never })
- .execute(),
- ).rejects.toThrow();
+ expect(
+ await violation(
+ database
+ .insert(identityLinks)
+ .values({ ...row("U4", "user-d"), verifiedBy: "guess" as never })
+ .execute(),
+ ),
+ ).toEqual({
+ sqlState: "23514",
+ constraint: "identity_links_verified_by_check",
+ });
+ expect(
+ await violation(
+ database
+ .insert(identityLinks)
+ .values({ ...row("U5", "user-e"), status: "gone" as never })
+ .execute(),
+ ),
+ ).toEqual({ sqlState: "23514", constraint: "identity_links_status_check" });
});
From 2d9cb634e6db1d60e5945a91e6437e34ece70a2a Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:46:59 -0300
Subject: [PATCH 045/101] docs: say what replacement and removal do to linked
accounts
---
docs/configuration.md | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/docs/configuration.md b/docs/configuration.md
index 6918a4a8b..37b73f791 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -435,8 +435,9 @@ A person the directory creates gets the `user` role, or `admin` when their addre
`INITIAL_ADMIN_EMAILS`, and still signs in through the company's identity provider: SCIM creates no
password. Their directory groups become their OpenBot groups, which per-group capability switches
and network policies read. Deactivating or deleting someone in the directory ends their sessions,
-deny-lists the address, retires the connector credentials they granted and stops their Bots'
-computers. Reactivating them lifts a deny-list entry SCIM wrote, never one an administrator wrote.
+deny-lists the address, retires the connector credentials they granted and their linked accounts
+(see Linked accounts), and stops their Bots' computers. Reactivating them lifts a deny-list entry
+SCIM wrote, never one an administrator wrote.
## One Bot handing work to another
@@ -490,16 +491,22 @@ number opted out, later messages show `opted_out` in the delivery history instea
OpenBot keeps a record of a person's accounts at outside providers, such as their GitHub account
or their Slack user. When a person has any, they appear under **Linked accounts** on
`/settings/connected-accounts`, and each can be disconnected there. Disconnecting revokes any token
-stored with the link, and is audited. A person with none sees no such section, and a deployment
-without an identity store does not show it at all.
+stored with the link, and is audited. A person with none sees no such section.
-Links are created by the integrations that support them, and none ship enabled yet, so the list is
-empty until one is. An integration proves an account in one of two ways: by the provider's own
+Links are created by the integrations that support them. Until one is configured, the list stays
+empty. An integration proves an account in one of two ways: by the provider's own
sign-in, or by a one-time code that OpenBot issues to the signed-in person and that they send from
their chat account. Whoever sends the code gets that chat account linked to the person it was issued
to, so treat it like a password and do not share it. It is valid for ten minutes and works once, and
issuing a new code for the same provider invalidates the earlier one. An outside account links to
-one OpenBot user at most.
+one OpenBot user at most. Linking a new account at a provider and workspace replaces the person's
+earlier link there and revokes its token.
+
+Removing a person, whether an administrator does it or a SCIM deactivation does, retires their
+links. Their unused link codes are deleted, any token stored with a link is revoked, and each link is
+marked as needing reconnection. The links are kept, not deleted, so the outside account still
+resolves to the removed person and is refused rather than treated as an unlinked guest. Each retired
+link is recorded in the audit trail.
This is separate from the OpenTag **Reachability** pairing above. That pairing binds a conversation
for delivery and uses its own `link ` messages, shown on the **Reachability** screen; it is not
From 58c62232d821f1f63e778206fcc4b8fbc3565c84 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:47:09 -0300
Subject: [PATCH 046/101] fix(server): accept only the provider's own token on
a link
---
server/src/identity/providers.ts | 12 ++++-
server/src/identity/store.ts | 36 +++++++++----
server/tests/identity-providers.test.ts | 5 ++
.../tests/identity-store.integration.test.ts | 50 +++++++++++++++++--
4 files changed, 90 insertions(+), 13 deletions(-)
diff --git a/server/src/identity/providers.ts b/server/src/identity/providers.ts
index ab2025837..83833cd27 100644
--- a/server/src/identity/providers.ts
+++ b/server/src/identity/providers.ts
@@ -12,6 +12,12 @@ type ProviderSpec = {
methods: readonly LinkMethod[];
/** For challenge providers: what the person sends from their account there, carrying the code. */
instruction?: (code: string) => string;
+ /**
+ * The vault `provider` of the connector credential this provider's links hold (its user token).
+ * A link may only carry a live connector credential with exactly this provider; a provider
+ * without one stores no token, so its links refuse any credential.
+ */
+ credentialProvider?: string;
};
export const PROVIDERS: Readonly> =
@@ -22,7 +28,11 @@ export const PROVIDERS: Readonly> =
instruction: (code) =>
`Send this message to the OpenBot bot in Slack: link ${code}`,
},
- github: { title: "GitHub", methods: ["oauth"] },
+ github: {
+ title: "GitHub",
+ methods: ["oauth"],
+ credentialProvider: "github-user-token",
+ },
});
const REALM_MAX = 512;
diff --git a/server/src/identity/store.ts b/server/src/identity/store.ts
index 7f3ede92f..b688f2ddf 100644
--- a/server/src/identity/store.ts
+++ b/server/src/identity/store.ts
@@ -7,7 +7,12 @@ import {
identityLinkChallenges,
identityLinks,
} from "../db/schema";
-import { acceptsMethod, isIdentityProvider, parseIdentity } from "./providers";
+import {
+ acceptsMethod,
+ isIdentityProvider,
+ PROVIDERS,
+ parseIdentity,
+} from "./providers";
import {
type Identity,
IdentityConflictError,
@@ -89,16 +94,21 @@ const CREDENTIAL_UNAVAILABLE = "The credential for this link is not available.";
/**
* Claim `credentialId` for the link being written, inside its transaction: the credential row is
- * locked live (`for update`, so a concurrent revoke waits for this link to commit or sees it), and
- * it must not already belong to a link other than `own` (the identity's own link and the person's
- * older link in the realm, which this write replaces). The partial unique index
- * identity_links_credential_idx is the backstop.
+ * locked live (`for update`, so a concurrent revoke waits for this link to commit or sees it), it
+ * must be this provider's own user token (kind "connector", vault provider equal to the registry's
+ * `credentialProvider`) so a link can never adopt, and a later unlink or replace never revoke, some
+ * other secret such as an operator key, and it must not already belong to a link other than `own`
+ * (the identity's own link and the person's older link in the realm, which this write replaces).
+ * The partial unique index identity_links_credential_idx is the backstop.
*/
async function claimCredential(
transaction: Transaction,
+ provider: IdentityProvider,
credentialId: string,
own: string[],
) {
+ const credentialProvider = PROVIDERS[provider].credentialProvider;
+ if (!credentialProvider) throw new IdentityInputError(CREDENTIAL_UNAVAILABLE);
const [live] = await transaction
.select({ id: credentialRows.id })
.from(credentialRows)
@@ -106,6 +116,8 @@ async function claimCredential(
and(
eq(credentialRows.id, credentialId),
isNull(credentialRows.revokedAt),
+ eq(credentialRows.kind, "connector"),
+ eq(credentialRows.provider, credentialProvider),
),
)
.for("update");
@@ -198,7 +210,12 @@ async function writeLinkLocked(
const now = new Date();
if (existing) {
if (proof.credentialId)
- await claimCredential(transaction, proof.credentialId, [existing.id]);
+ await claimCredential(
+ transaction,
+ identity.provider,
+ proof.credentialId,
+ [existing.id],
+ );
const credentialId = proof.credentialId ?? existing.credentialId;
if (existing.credentialId && existing.credentialId !== credentialId)
await revokeQuietly(credentials, existing.credentialId, transaction);
@@ -232,6 +249,7 @@ async function writeLinkLocked(
if (proof.credentialId)
await claimCredential(
transaction,
+ identity.provider,
proof.credentialId,
previous ? [previous.id] : [],
);
@@ -321,9 +339,9 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
/**
* Link `value` to `userId` on proof the caller has already verified.
*
- * A `credentialId`, when given, must be a UUID naming a live credential that no other link
- * holds: it is checked and locked in the link's own transaction, so a malformed, missing,
- * revoked or already-owned id is refused rather than stored as a token the link does not
+ * A `credentialId`, when given, must be a UUID naming a live credential that is this
+ * provider's own user token (see `credentialProvider`) and that no other link holds: it is checked and locked in the link's own transaction, so a malformed, missing,
+ * revoked, foreign or already-owned id is refused rather than stored as a token the link does not
* have. Omitting it keeps the credential an existing link already has (a re-sign-in that mints
* no token must not kill the working one); a different id replaces it and revokes the old one.
* Only unlink clears a credential. `handle` likewise: omitted keeps, null clears.
diff --git a/server/tests/identity-providers.test.ts b/server/tests/identity-providers.test.ts
index 91cb9ef7a..5dda798cf 100644
--- a/server/tests/identity-providers.test.ts
+++ b/server/tests/identity-providers.test.ts
@@ -119,3 +119,8 @@ test("slackRealm refuses a realm longer than parseIdentity accepts", () => {
}),
).toThrow(IdentityInputError);
});
+
+test("only a provider that stores a token names the credential its links may hold", () => {
+ expect(PROVIDERS.github.credentialProvider).toBe("github-user-token");
+ expect(PROVIDERS.slack.credentialProvider).toBeUndefined();
+});
diff --git a/server/tests/identity-store.integration.test.ts b/server/tests/identity-store.integration.test.ts
index 212e8d68b..615abc097 100644
--- a/server/tests/identity-store.integration.test.ts
+++ b/server/tests/identity-store.integration.test.ts
@@ -34,10 +34,13 @@ const github = (subject = randomUUID()): Identity => ({
});
const user = () => `user-${randomUUID()}`;
-async function credential() {
+async function credential(
+ kind: "connector" | "model" = "connector",
+ provider = "github-user-token",
+) {
const { id } = await vault.create({
- kind: "connector",
- provider: "github-user-token",
+ kind,
+ provider,
keyId: randomUUID(),
metadata: {},
encryptedValue: "not-a-real-ciphertext",
@@ -328,3 +331,44 @@ test("a challenge link refuses a credential", async () => {
).rejects.toThrow(IdentityInputError);
expect(await store.linkedUser(identity)).toBeNull();
});
+
+test("a live credential of another kind is refused and stays live", async () => {
+ const identity = github();
+ const operatorKey = await credential("model");
+ await expect(
+ store.linkVerified(identity, user(), {
+ method: "oauth",
+ credentialId: operatorKey,
+ }),
+ ).rejects.toThrow("The credential for this link is not available.");
+ expect(await store.linkedUser(identity)).toBeNull();
+ expect(await vault.isLive(operatorKey)).toBe(true);
+});
+
+test("a connector credential for another provider is refused and stays live", async () => {
+ const identity = github();
+ const person = user();
+ const other = await credential("connector", "slack-bot-token");
+ await expect(
+ store.linkVerified(identity, person, {
+ method: "oauth",
+ credentialId: other,
+ }),
+ ).rejects.toThrow("The credential for this link is not available.");
+ expect(await store.linkedUser(identity)).toBeNull();
+ expect(await vault.isLive(other)).toBe(true);
+ // Relinking an existing link with it is refused too, and the link's own token survives.
+ const own = await credential();
+ await store.linkVerified(identity, person, {
+ method: "oauth",
+ credentialId: own,
+ });
+ await expect(
+ store.linkVerified(identity, person, {
+ method: "oauth",
+ credentialId: other,
+ }),
+ ).rejects.toThrow(IdentityInputError);
+ expect(await vault.isLive(other)).toBe(true);
+ expect(await vault.isLive(own)).toBe(true);
+});
From f842c7652b5d6d9250eee5314b52167b3a008404 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:47:09 -0300
Subject: [PATCH 047/101] fix(app): fall back for unknown provider icons
---
app/src/components/settings/linked-accounts.test.tsx | 11 +++++++++++
app/src/components/settings/linked-accounts.tsx | 8 ++++----
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/app/src/components/settings/linked-accounts.test.tsx b/app/src/components/settings/linked-accounts.test.tsx
index 650b36ef6..f8b5c1ece 100644
--- a/app/src/components/settings/linked-accounts.test.tsx
+++ b/app/src/components/settings/linked-accounts.test.tsx
@@ -53,6 +53,17 @@ test("providers map to their own icon with a neutral fallback", () => {
expect(providerIcon("gitlab")).toBe(IconLink);
});
+test("Object prototype keys are unknown providers, not icons", () => {
+ for (const key of [
+ "constructor",
+ "toString",
+ "hasOwnProperty",
+ "__proto__",
+ ]) {
+ expect(providerIcon(key)).toBe(IconLink);
+ }
+});
+
test("the not-configured 503 means the feature is absent, so no links", async () => {
const response = Response.json(
{
diff --git a/app/src/components/settings/linked-accounts.tsx b/app/src/components/settings/linked-accounts.tsx
index 2fa665297..df68a1947 100644
--- a/app/src/components/settings/linked-accounts.tsx
+++ b/app/src/components/settings/linked-accounts.tsx
@@ -34,10 +34,10 @@ const providerIcons = {
/** A provider this build has no mark for gets a neutral link icon rather than someone else's logo. */
export function providerIcon(provider: string) {
- return (
- (providerIcons as Record)[provider] ??
- IconLink
- );
+ // Own keys only: "constructor" or "toString" would otherwise find an Object prototype member.
+ return Object.hasOwn(providerIcons, provider)
+ ? providerIcons[provider as keyof typeof providerIcons]
+ : IconLink;
}
/**
From c464bbcac48e0444949c1f3220bf34cdfcdeb6bd Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:47:39 -0300
Subject: [PATCH 048/101] fix(server): treat link handles the same on every
path
---
server/src/identity/store.ts | 25 +++++++++--
.../identity-challenges.integration.test.ts | 42 +++++++++++++++++++
.../tests/identity-store.integration.test.ts | 24 +++++++++++
3 files changed, 88 insertions(+), 3 deletions(-)
diff --git a/server/src/identity/store.ts b/server/src/identity/store.ts
index b688f2ddf..c42ebf535 100644
--- a/server/src/identity/store.ts
+++ b/server/src/identity/store.ts
@@ -88,6 +88,21 @@ async function revokeQuietly(
}
}
+const HANDLE_MAX = 256;
+
+/**
+ * One rule for a display handle on every path: omitted (`undefined`) keeps what the link has,
+ * null clears it, a string is trimmed and capped at 256 code points (never splitting a surrogate
+ * pair), and a blank one becomes null. Anything else is refused.
+ */
+function normalizeHandle(handle: unknown): string | null | undefined {
+ if (handle === undefined || handle === null) return handle;
+ if (typeof handle !== "string") throw new IdentityInputError();
+ const trimmed = handle.trim();
+ if (!trimmed) return null;
+ return Array.from(trimmed).slice(0, HANDLE_MAX).join("");
+}
+
const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
const CREDENTIAL_UNAVAILABLE = "The credential for this link is not available.";
@@ -377,9 +392,11 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
throw new IdentityInputError(CREDENTIAL_UNAVAILABLE);
// Lowercase: uuid columns read back lowercase, and keep/move compare ids as strings.
const credentialId = proof.credentialId?.toLowerCase() ?? undefined;
+ const handle = normalizeHandle(proof.handle);
return database.transaction((transaction) =>
writeLink(transaction, credentials, identity, userId, {
- ...proof,
+ method: proof.method,
+ handle,
credentialId,
}),
);
@@ -470,14 +487,16 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
/**
* Link `value`, the chat account the code arrived from, to the user the code was issued to.
* Consumption and link creation share one transaction: a refused link leaves the code unused.
+ * `handle` follows the same rule as linkVerified: omitted keeps the stored one, null clears.
*/
async redeemChallenge(
code: string,
value: Identity,
- handle: string | null = null,
+ handle?: string | null,
): Promise {
const hash = codeHash(code);
const identity = parseIdentity(value);
+ const normalized = normalizeHandle(handle);
return database.transaction(async (transaction) => {
const [challenge] = await transaction
.select()
@@ -499,7 +518,7 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
if (!consumed) throw new IdentityLinkError();
return writeLink(transaction, credentials, identity, consumed.userId, {
method: "challenge",
- handle: handle?.slice(0, 256) ?? null,
+ handle: normalized,
});
});
},
diff --git a/server/tests/identity-challenges.integration.test.ts b/server/tests/identity-challenges.integration.test.ts
index 689368f3b..415ca833d 100644
--- a/server/tests/identity-challenges.integration.test.ts
+++ b/server/tests/identity-challenges.integration.test.ts
@@ -251,3 +251,45 @@ test("concurrent requests still leave one live code per person and provider", as
(await store.redeemChallenge(live[0]?.code ?? "", slack())).userId,
).toBe(owner);
});
+
+test("redeeming again without a handle keeps the stored one; an explicit null clears it", async () => {
+ const owner = user();
+ const sender = slack();
+ const issue = async () => (await store.issueChallenge(owner, "slack")).code;
+ await store.redeemChallenge(await issue(), sender, "dana");
+ expect((await store.redeemChallenge(await issue(), sender)).handle).toBe(
+ "dana",
+ );
+ expect(
+ (await store.redeemChallenge(await issue(), sender, undefined)).handle,
+ ).toBe("dana");
+ expect(
+ (await store.redeemChallenge(await issue(), sender, null)).handle,
+ ).toBeNull();
+});
+
+test("a redeemed handle is capped at 256 code points without splitting a pair", async () => {
+ const owner = user();
+ const link = await store.redeemChallenge(
+ (await store.issueChallenge(owner, "slack")).code,
+ slack(),
+ "\u{1F600}".repeat(300),
+ );
+ expect(Array.from(link.handle ?? "")).toHaveLength(256);
+ expect(link.handle).toBe("\u{1F600}".repeat(256));
+});
+
+test("a blank redeemed handle is stored as none, and a non-string one is refused", async () => {
+ const owner = user();
+ const link = await store.redeemChallenge(
+ (await store.issueChallenge(owner, "slack")).code,
+ slack(),
+ " ",
+ );
+ expect(link.handle).toBeNull();
+ const { code } = await store.issueChallenge(owner, "slack");
+ await expect(
+ store.redeemChallenge(code, slack(), 42 as unknown as string),
+ ).rejects.toThrow(IdentityInputError);
+ expect(await rowFor(code)).toBeDefined();
+});
diff --git a/server/tests/identity-store.integration.test.ts b/server/tests/identity-store.integration.test.ts
index 615abc097..29ae6e0c4 100644
--- a/server/tests/identity-store.integration.test.ts
+++ b/server/tests/identity-store.integration.test.ts
@@ -372,3 +372,27 @@ test("a connector credential for another provider is refused and stays live", as
expect(await vault.isLive(other)).toBe(true);
expect(await vault.isLive(own)).toBe(true);
});
+
+test("a linked handle is capped at 256 code points like a redeemed one", async () => {
+ const link = await store.linkVerified(github(), user(), {
+ method: "oauth",
+ handle: `${"a".repeat(255)}\u{1F600}\u{1F600}`,
+ });
+ expect(link.handle).toBe(`${"a".repeat(255)}\u{1F600}`);
+});
+
+test("a blank linked handle is stored as none, and a non-string one is refused", async () => {
+ const blank = await store.linkVerified(github(), user(), {
+ method: "oauth",
+ handle: " ",
+ });
+ expect(blank.handle).toBeNull();
+ const identity = github();
+ await expect(
+ store.linkVerified(identity, user(), {
+ method: "oauth",
+ handle: { name: "dana" } as unknown as string,
+ }),
+ ).rejects.toThrow(IdentityInputError);
+ expect(await store.linkedUser(identity)).toBeNull();
+});
From 5a30d2364f4982a3f250c486af956c67a3e16127 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:48:23 -0300
Subject: [PATCH 049/101] fix(server): serialize link codes with person removal
---
server/src/identity/store.ts | 47 ++++++++++++++++++-
.../identity-challenges.integration.test.ts | 31 +++++++++++-
2 files changed, 75 insertions(+), 3 deletions(-)
diff --git a/server/src/identity/store.ts b/server/src/identity/store.ts
index c42ebf535..9f2aa5b37 100644
--- a/server/src/identity/store.ts
+++ b/server/src/identity/store.ts
@@ -26,6 +26,34 @@ import {
type Transaction = Parameters[0]>[0];
type Revoke = Pick;
+type Locker = Pick;
+
+/**
+ * The per-person identity lock: a transaction-scoped advisory lock on `userId`. issueChallenge
+ * takes it, and person removal (retireIdentityLinks) is meant to take the same key, so a link code
+ * is issued wholly before a removal (which then deletes it) or wholly after, never alongside.
+ */
+export async function identityUserLock(transaction: Locker, userId: string) {
+ await transaction.execute(
+ sql`select pg_advisory_xact_lock(hashtextextended(${`identity-user\u001f${userId}`}, 0))`,
+ );
+}
+
+/**
+ * The per-person-per-realm lock writeLink takes before any row lock, serializing one person's
+ * links in one `(provider, realm)`. `provider` is text so a caller reading stored rows, whose
+ * provider may be one this build does not know, can take the same key.
+ */
+export async function identityRealmLock(
+ transaction: Locker,
+ userId: string,
+ provider: string,
+ realm: string,
+) {
+ await transaction.execute(
+ sql`select pg_advisory_xact_lock(hashtextextended(${`${userId}\u001f${provider}\u001f${realm}`}, 0))`,
+ );
+}
const identityMatches = (identity: Identity) =>
and(
@@ -213,8 +241,11 @@ async function writeLinkLocked(
credentialId?: string;
},
): Promise {
- await transaction.execute(
- sql`select pg_advisory_xact_lock(hashtextextended(${`${userId}\u001f${identity.provider}\u001f${identity.realm}`}, 0))`,
+ await identityRealmLock(
+ transaction,
+ userId,
+ identity.provider,
+ identity.realm,
);
const [existing] = await transaction
.select()
@@ -307,6 +338,12 @@ function codeHash(code: string) {
export function createIdentityStore(database: Database, credentials: Revoke) {
return {
+ /**
+ * Who `value` is linked to, and the link's status. A removed person's links persist by design
+ * (retirement marks them needs_reconnect rather than deleting them, so the outside account
+ * keeps resolving to the removed person and is refused rather than becoming a guest), so
+ * callers must also check the returned user is still an active person before acting for them.
+ */
async linkedUser(
value: Identity,
): Promise<{ userId: string; status: LinkStatus } | null> {
@@ -434,6 +471,11 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
* A one-time code for the signed-in `userId`. The link is made when the code arrives from the
* person's account at `provider` (redeemChallenge), and always links to the user it was issued
* to, so a code started by somebody else can never attach their account to this person.
+ *
+ * Issued under identityUserLock, the lock person removal takes, so a code is never issued
+ * alongside a removal: one issued before it is deleted by it. A code issued after a removal
+ * (by a caller that skipped its session check) can still link, but only to the removed person,
+ * whom linkedUser's callers must refuse.
*/
async issueChallenge(
userId: string,
@@ -449,6 +491,7 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
);
const code = randomUUID();
return database.transaction(async (transaction) => {
+ await identityUserLock(transaction, userId);
// No sweep exists, so codes that were never sent would otherwise accumulate.
await transaction
.delete(identityLinkChallenges)
diff --git a/server/tests/identity-challenges.integration.test.ts b/server/tests/identity-challenges.integration.test.ts
index 415ca833d..e90eed4cb 100644
--- a/server/tests/identity-challenges.integration.test.ts
+++ b/server/tests/identity-challenges.integration.test.ts
@@ -4,7 +4,7 @@ import { eq, inArray, sql } from "drizzle-orm";
import { createCredentialStore } from "../src/credentials";
import { createDatabase } from "../src/db/client";
import { identityLinkChallenges, identityLinks } from "../src/db/schema";
-import { createIdentityStore } from "../src/identity/store";
+import { createIdentityStore, identityUserLock } from "../src/identity/store";
import {
type Identity,
IdentityConflictError,
@@ -293,3 +293,32 @@ test("a blank redeemed handle is stored as none, and a non-string one is refused
).rejects.toThrow(IdentityInputError);
expect(await rowFor(code)).toBeDefined();
});
+
+test("issuing a code waits for the person's identity lock, which removal holds", async () => {
+ const owner = user();
+ let release = () => {};
+ const released = new Promise((resolve) => {
+ release = resolve;
+ });
+ let locked = () => {};
+ const isLocked = new Promise((resolve) => {
+ locked = resolve;
+ });
+ const holder = database.transaction(async (transaction) => {
+ await identityUserLock(transaction, owner);
+ locked();
+ await released;
+ });
+ await isLocked;
+ let issued = false;
+ const issuing = store.issueChallenge(owner, "slack").then((result) => {
+ issued = true;
+ return result;
+ });
+ await Bun.sleep(200);
+ expect(issued).toBe(false);
+ release();
+ await holder;
+ const { code } = await issuing;
+ expect(await rowFor(code)).toBeDefined();
+});
From 347f11bda310341ec5ea4637858ab32266b63092 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:49:11 -0300
Subject: [PATCH 050/101] refactor(server): tidy identity store edges
---
server/src/identity/store.ts | 24 ++++++++++-----
.../identity-challenges.integration.test.ts | 17 +++++++++++
.../tests/identity-store.integration.test.ts | 29 +++++++++++++++++++
3 files changed, 63 insertions(+), 7 deletions(-)
diff --git a/server/src/identity/store.ts b/server/src/identity/store.ts
index 9f2aa5b37..ad473bc14 100644
--- a/server/src/identity/store.ts
+++ b/server/src/identity/store.ts
@@ -131,6 +131,12 @@ function normalizeHandle(handle: unknown): string | null | undefined {
return Array.from(trimmed).slice(0, HANDLE_MAX).join("");
}
+/** A user id is an opaque, non-blank string with no surrounding whitespace, never coerced. */
+function requireUserId(userId: unknown): asserts userId is string {
+ if (typeof userId !== "string" || !userId.trim() || userId !== userId.trim())
+ throw new IdentityInputError();
+}
+
const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
const CREDENTIAL_UNAVAILABLE = "The credential for this link is not available.";
@@ -140,8 +146,9 @@ const CREDENTIAL_UNAVAILABLE = "The credential for this link is not available.";
* locked live (`for update`, so a concurrent revoke waits for this link to commit or sees it), it
* must be this provider's own user token (kind "connector", vault provider equal to the registry's
* `credentialProvider`) so a link can never adopt, and a later unlink or replace never revoke, some
- * other secret such as an operator key, and it must not already belong to a link other than `own`
- * (the identity's own link and the person's older link in the realm, which this write replaces).
+ * other secret such as an operator key, and it must not already belong to any link outside `own`
+ * (the one link this write updates or replaces: the identity's own link on a relink, else the
+ * person's older link in the realm, if any).
* The partial unique index identity_links_credential_idx is the backstop.
*/
async function claimCredential(
@@ -182,7 +189,8 @@ async function claimCredential(
/**
* Link `identity` to `userId` inside `transaction`. Module-private: only `linkVerified` (which
- * validates the proof first) and `redeemChallenge` (no credential) call it.
+ * checks the proof's method against the provider and the credential id's shape first; the proof
+ * itself is the caller's to verify) and `redeemChallenge` (no credential) call it.
*
* Two races are handled. Two people racing for one identity: the identity row is locked first, and a
* racing insert surfaces as a unique violation on the identity index, which is a refusal
@@ -411,7 +419,7 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
},
): Promise {
const identity = parseIdentity(value);
- if (!userId.trim()) throw new IdentityInputError();
+ requireUserId(userId);
if (!acceptsMethod(identity.provider, proof.method))
throw new IdentityInputError(
"This provider does not accept that kind of proof.",
@@ -481,7 +489,7 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
userId: string,
provider: IdentityProvider,
): Promise<{ code: string; expiresAt: Date }> {
- if (!userId.trim()) throw new IdentityInputError();
+ requireUserId(userId);
if (
!isIdentityProvider(provider) ||
!acceptsMethod(provider, "challenge")
@@ -492,7 +500,8 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
const code = randomUUID();
return database.transaction(async (transaction) => {
await identityUserLock(transaction, userId);
- // No sweep exists, so codes that were never sent would otherwise accumulate.
+ // Nothing else deletes an expired code (a person has at most one per provider, but one
+ // never redeemed stays forever), so each issue clears those over a day stale.
await transaction
.delete(identityLinkChallenges)
.where(
@@ -522,7 +531,8 @@ export function createIdentityStore(database: Database, credentials: Revoke) {
},
})
.returning({ expiresAt: identityLinkChallenges.expiresAt });
- if (!challenge) throw new IdentityLinkError();
+ // An upsert always returns its row; this only narrows the type.
+ if (!challenge) throw new Error("The link code was not stored.");
return { code, expiresAt: challenge.expiresAt };
});
},
diff --git a/server/tests/identity-challenges.integration.test.ts b/server/tests/identity-challenges.integration.test.ts
index e90eed4cb..bdf790b64 100644
--- a/server/tests/identity-challenges.integration.test.ts
+++ b/server/tests/identity-challenges.integration.test.ts
@@ -322,3 +322,20 @@ test("issuing a code waits for the person's identity lock, which removal holds",
const { code } = await issuing;
expect(await rowFor(code)).toBeDefined();
});
+
+test("an uppercase code redeems", async () => {
+ const owner = user();
+ const { code } = await store.issueChallenge(owner, "slack");
+ expect(
+ (await store.redeemChallenge(code.toUpperCase(), slack())).userId,
+ ).toBe(owner);
+});
+
+test.each([42, undefined, " user-padded", "user-padded "])(
+ "issuing for a userId of %p is refused as input",
+ async (userId) => {
+ await expect(
+ store.issueChallenge(userId as unknown as string, "slack"),
+ ).rejects.toThrow(IdentityInputError);
+ },
+);
diff --git a/server/tests/identity-store.integration.test.ts b/server/tests/identity-store.integration.test.ts
index 29ae6e0c4..0a5d3c7a7 100644
--- a/server/tests/identity-store.integration.test.ts
+++ b/server/tests/identity-store.integration.test.ts
@@ -396,3 +396,32 @@ test("a blank linked handle is stored as none, and a non-string one is refused",
).rejects.toThrow(IdentityInputError);
expect(await store.linkedUser(identity)).toBeNull();
});
+
+test("an uppercase credentialId is kept on relink, not revoked as a different token", async () => {
+ const person = user();
+ const identity = github();
+ const token = await credential();
+ await store.linkVerified(identity, person, {
+ method: "oauth",
+ credentialId: token.toUpperCase(),
+ });
+ const link = await store.linkVerified(identity, person, {
+ method: "oauth",
+ credentialId: token.toUpperCase(),
+ });
+ expect(link.credentialId).toBe(token);
+ expect(await vault.isLive(token)).toBe(true);
+});
+
+test.each([42, null, " user-padded", "user-padded "])(
+ "a userId of %p is refused as input",
+ async (userId) => {
+ const identity = github();
+ await expect(
+ store.linkVerified(identity, userId as unknown as string, {
+ method: "oauth",
+ }),
+ ).rejects.toThrow(IdentityInputError);
+ expect(await store.linkedUser(identity)).toBeNull();
+ },
+);
From 09bb63737158369fdcd9a8e7e777d95c2435107c Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:51:12 -0300
Subject: [PATCH 051/101] refactor(server): share identity lock keys with
retirement
---
server/src/identity/retire.ts | 32 ++++++++---------
.../tests/identity-retire.integration.test.ts | 36 ++++++++++++++++++-
2 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/server/src/identity/retire.ts b/server/src/identity/retire.ts
index 7b8fc1588..577d0a07a 100644
--- a/server/src/identity/retire.ts
+++ b/server/src/identity/retire.ts
@@ -1,8 +1,9 @@
-import { asc, eq, sql } from "drizzle-orm";
+import { asc, eq } from "drizzle-orm";
import { type AuditStore, recordAuditEvent } from "../audit";
import { CredentialRefusedError, type CredentialStore } from "../credentials";
import type { Database } from "../db/client";
import { identityLinkChallenges, identityLinks } from "../db/schema";
+import { identityRealmLock, identityUserLock } from "./store";
/**
* Retires a removed person's identity links.
@@ -19,15 +20,17 @@ import { identityLinkChallenges, identityLinks } from "../db/schema";
* order `retireConnectionsFor` uses): actor `by`, target the link id, and the provider, the reason
* and whether a credential was revoked. Never the subject, realm or credential id.
*
- * Concurrency. The challenges are deleted first: a `redeemChallenge` in flight holds its challenge
- * row, so the delete waits for it to commit and the link it wrote is then seen below. Then, for
- * every realm the person has a link in, the exact advisory key `writeLink` takes for
- * `(userId, provider, realm)` is taken — in sorted order, before any row lock, the order `writeLink`
- * uses — so a concurrent re-link in a known realm finishes first or waits for this. The residual
- * window: a `writeLink` that is not a challenge redemption (an OAuth link) into a realm this person
- * had no link in, still uncommitted when the realms are read, commits an active link after this
- * returns. The person's sessions are already gone by then, so that needs a callback already past
- * its session check.
+ * Concurrency. Locks are taken in this order: `identityUserLock`, the lock `issueChallenge` takes, so
+ * no code is issued alongside the removal; then the challenges are deleted, which waits for a
+ * `redeemChallenge` in flight (it holds its challenge row) to commit, so the link it wrote is seen
+ * below; then `identityRealmLock` for every `(provider, realm)` the person has a link in, in sorted
+ * order (writeLink takes the one key for the realm it writes, so sorting only keeps two retirements
+ * of one person from deadlocking each other); then the link rows. A writeLink into one of those realms
+ * that is already running finishes first and is retired here. One that arrives later waits, and once
+ * this commits it goes ahead: it re-links that realm for the removed person, active again, so
+ * refusing it is left to linkedUser's callers, which refuse a removed person. The same holds for a
+ * writeLink (an OAuth link) into a realm the person had no link in when the realms were read. Both
+ * need a callback already past its session check, since the person's sessions are gone by then.
*
* Returns how many links changed; a second call returns 0 and writes no audit rows.
*/
@@ -39,6 +42,7 @@ export async function retireIdentityLinks(
by: string,
): Promise {
const retired = await database.transaction(async (transaction) => {
+ await identityUserLock(transaction, userId);
await transaction
.delete(identityLinkChallenges)
.where(eq(identityLinkChallenges.userId, userId));
@@ -51,12 +55,8 @@ export async function retireIdentityLinks(
.from(identityLinks)
.where(eq(identityLinks.userId, userId))
.orderBy(asc(identityLinks.provider), asc(identityLinks.realm));
- for (const { provider, realm } of realms) {
- // writeLink's key, character for character (server/src/identity/store.ts).
- await transaction.execute(
- sql`select pg_advisory_xact_lock(hashtextextended(${`${userId}\u001f${provider}\u001f${realm}`}, 0))`,
- );
- }
+ for (const { provider, realm } of realms)
+ await identityRealmLock(transaction, userId, provider, realm);
const links = await transaction
.select({
diff --git a/server/tests/identity-retire.integration.test.ts b/server/tests/identity-retire.integration.test.ts
index c7311196f..dcb813094 100644
--- a/server/tests/identity-retire.integration.test.ts
+++ b/server/tests/identity-retire.integration.test.ts
@@ -13,7 +13,7 @@ import {
retireIdentityLinks,
retireOwnedAccounts,
} from "../src/identity/retire";
-import { createIdentityStore } from "../src/identity/store";
+import { createIdentityStore, identityUserLock } from "../src/identity/store";
import { IdentityLinkError } from "../src/identity/types";
import { TEST_POOL, testDatabaseUrl } from "./support/database";
@@ -178,6 +178,40 @@ test("a pending challenge cannot be redeemed after the person is removed", async
).rejects.toBeInstanceOf(IdentityLinkError);
});
+test("retirement waits on the per-person identity lock that issuing a code takes", async () => {
+ const userId = user();
+ let release = () => {};
+ const released = new Promise((resolve) => {
+ release = resolve;
+ });
+ let locked = () => {};
+ const isLocked = new Promise((resolve) => {
+ locked = resolve;
+ });
+ const holder = database.transaction(async (transaction) => {
+ await identityUserLock(transaction, userId);
+ locked();
+ await released;
+ });
+ await isLocked;
+ let finished = false;
+ const retiring = retireIdentityLinks(
+ database,
+ vault,
+ recorder().store,
+ userId,
+ ADMIN,
+ ).then((count) => {
+ finished = true;
+ return count;
+ });
+ await Bun.sleep(200);
+ expect(finished).toBe(false);
+ release();
+ await holder;
+ expect(await retiring).toBe(0);
+});
+
test("a plugin refusal still retires identity links, and is rethrown", async () => {
const refusal = new Error("composio refused");
const ran: string[] = [];
From 56888be9509b4002381d5e76cdec352040b79b24 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:52:06 -0300
Subject: [PATCH 052/101] fix(server): audit linked-account retirement inside
its transaction
---
server/src/identity/retire.ts | 55 ++++----
server/src/index.ts | 2 +-
.../tests/identity-retire.integration.test.ts | 121 ++++++++++++++++--
3 files changed, 138 insertions(+), 40 deletions(-)
diff --git a/server/src/identity/retire.ts b/server/src/identity/retire.ts
index 577d0a07a..8f9cf2344 100644
--- a/server/src/identity/retire.ts
+++ b/server/src/identity/retire.ts
@@ -15,10 +15,14 @@ import { identityRealmLock, identityUserLock } from "./store";
* into an unlinked guest. The person's pending link challenges are deleted, so a code issued before
* the removal cannot be redeemed into a fresh active link afterwards.
*
- * One transaction, so a failure leaves nothing half-retired. One `identity.link_retired` audit row
- * per link that changed, written after the commit (the audit store has its own handle — the same
- * order `retireConnectionsFor` uses): actor `by`, target the link id, and the provider, the reason
- * and whether a credential was revoked. Never the subject, realm or credential id.
+ * One transaction, the audit rows included: `auditFor` is handed the transaction and returns the
+ * store that writes into it (`createAuditStore(transaction)`, as approvals do), so a link is retired
+ * exactly when its `identity.link_retired` row is written. A failure anywhere, an audit row
+ * included, rolls the whole retirement back and a retry starts over and audits every link; writing
+ * the rows after the commit instead lost the rest for good when one failed, because the retry found
+ * every link already retired. One row per link that changed: actor `by`, target the link id, and the
+ * provider, the reason and whether a credential was revoked. Never the subject, realm or credential
+ * id.
*
* Concurrency. Locks are taken in this order: `identityUserLock`, the lock `issueChallenge` takes, so
* no code is issued alongside the removal; then the challenges are deleted, which waits for a
@@ -37,11 +41,12 @@ import { identityRealmLock, identityUserLock } from "./store";
export async function retireIdentityLinks(
database: Database,
credentials: Pick,
- auditStore: AuditStore,
+ auditFor: (executor: Database) => AuditStore,
userId: string,
by: string,
): Promise {
- const retired = await database.transaction(async (transaction) => {
+ return database.transaction(async (transaction) => {
+ const auditStore = auditFor(transaction as unknown as Database);
await identityUserLock(transaction, userId);
await transaction
.delete(identityLinkChallenges)
@@ -70,11 +75,7 @@ export async function retireIdentityLinks(
.orderBy(asc(identityLinks.id))
.for("update");
- const changed: {
- id: string;
- provider: string;
- credentialRevoked: boolean;
- }[] = [];
+ let changed = 0;
for (const link of links) {
if (!link.credentialId && link.status === "needs_reconnect") continue;
if (link.credentialId) {
@@ -93,30 +94,22 @@ export async function retireIdentityLinks(
updatedAt: new Date(),
})
.where(eq(identityLinks.id, link.id));
- changed.push({
- id: link.id,
- provider: link.provider,
- credentialRevoked: Boolean(link.credentialId),
+ await recordAuditEvent(auditStore, {
+ eventType: "identity.link_retired",
+ targetType: "identity_link",
+ targetId: link.id,
+ actorUserId: by,
+ payload: {
+ actor: by,
+ provider: link.provider,
+ reason: "person_removed",
+ credentialRevoked: Boolean(link.credentialId),
+ },
});
+ changed += 1;
}
return changed;
});
-
- for (const link of retired) {
- await recordAuditEvent(auditStore, {
- eventType: "identity.link_retired",
- targetType: "identity_link",
- targetId: link.id,
- actorUserId: by,
- payload: {
- actor: by,
- provider: link.provider,
- reason: "person_removed",
- credentialRevoked: link.credentialRevoked,
- },
- });
- }
- return retired.length;
}
/**
diff --git a/server/src/index.ts b/server/src/index.ts
index 1898acb67..c187f8bd2 100644
--- a/server/src/index.ts
+++ b/server/src/index.ts
@@ -412,7 +412,7 @@ const peopleStore = createPeopleStore(
retireIdentityLinks(
database,
credentialStore,
- bootAuditStore,
+ (executor) => createAuditStore(executor),
userId,
by,
),
diff --git a/server/tests/identity-retire.integration.test.ts b/server/tests/identity-retire.integration.test.ts
index dcb813094..cb1d75bd5 100644
--- a/server/tests/identity-retire.integration.test.ts
+++ b/server/tests/identity-retire.integration.test.ts
@@ -1,10 +1,15 @@
import { afterAll, expect, test } from "bun:test";
import { randomUUID } from "node:crypto";
-import { eq, inArray } from "drizzle-orm";
-import type { AuditEventInput, AuditStore } from "../src/audit";
+import { eq, inArray, like } from "drizzle-orm";
+import {
+ type AuditEventInput,
+ type AuditStore,
+ createAuditStore,
+} from "../src/audit";
import { createCredentialStore } from "../src/credentials";
import { createDatabase } from "../src/db/client";
import {
+ auditEvents,
credentials,
identityLinkChallenges,
identityLinks,
@@ -25,7 +30,9 @@ const users: string[] = [];
const ADMIN = "admin-remover";
afterAll(async () => {
- await database.delete(identityLinks).where(eq(identityLinks.realm, realm));
+ await database
+ .delete(identityLinks)
+ .where(like(identityLinks.realm, `${realm}%`));
if (users.length)
await database
.delete(identityLinkChallenges)
@@ -86,7 +93,13 @@ test("retiring a removed person's links revokes their tokens and keeps the rows"
const audit = recorder();
expect(
- await retireIdentityLinks(database, vault, audit.store, userId, ADMIN),
+ await retireIdentityLinks(
+ database,
+ vault,
+ () => audit.store,
+ userId,
+ ADMIN,
+ ),
).toBe(1);
expect(await vault.isLive(credential.id)).toBe(false);
@@ -114,7 +127,13 @@ test("retiring a removed person's links revokes their tokens and keeps the rows"
// A second pass changes nothing and records nothing.
expect(
- await retireIdentityLinks(database, vault, audit.store, userId, ADMIN),
+ await retireIdentityLinks(
+ database,
+ vault,
+ () => audit.store,
+ userId,
+ ADMIN,
+ ),
).toBe(0);
expect(audit.rows).toHaveLength(1);
});
@@ -135,7 +154,13 @@ test("a link without a credential is also marked needs_reconnect, and audited wi
const audit = recorder();
expect(
- await retireIdentityLinks(database, vault, audit.store, userId, ADMIN),
+ await retireIdentityLinks(
+ database,
+ vault,
+ () => audit.store,
+ userId,
+ ADMIN,
+ ),
).toBe(1);
const row = await statusOf(linkId);
@@ -157,12 +182,92 @@ test("a link without a credential is also marked needs_reconnect, and audited wi
expect(payload).not.toContain(realm);
});
+test("an audit row that fails to write rolls the retirement back, so a retry retires and audits every link", async () => {
+ const userId = user();
+ const linkIds = [randomUUID(), randomUUID()];
+ for (const id of linkIds)
+ await database.insert(identityLinks).values({
+ id,
+ provider: "slack",
+ realm: `${realm}-${id}`,
+ subject: randomUUID(),
+ userId,
+ verifiedBy: "challenge",
+ status: "active",
+ });
+ let written = 0;
+ const failing: AuditStore = {
+ insert: async () => {
+ written += 1;
+ if (written === 2) throw new Error("audit store down");
+ },
+ };
+
+ await expect(
+ retireIdentityLinks(database, vault, () => failing, userId, ADMIN),
+ ).rejects.toThrow("audit store down");
+ for (const id of linkIds) expect((await statusOf(id))?.status).toBe("active");
+
+ const audit = recorder();
+ expect(
+ await retireIdentityLinks(
+ database,
+ vault,
+ () => audit.store,
+ userId,
+ ADMIN,
+ ),
+ ).toBe(2);
+ expect(audit.rows.map((row) => row.targetId).sort()).toEqual(
+ [...linkIds].sort(),
+ );
+ for (const id of linkIds)
+ expect((await statusOf(id))?.status).toBe("needs_reconnect");
+});
+
+test("the audit trail's own store writes the rows inside the retirement's transaction", async () => {
+ const userId = user();
+ const linkId = randomUUID();
+ await database.insert(identityLinks).values({
+ id: linkId,
+ provider: "slack",
+ realm,
+ subject: randomUUID(),
+ userId,
+ verifiedBy: "challenge",
+ status: "active",
+ });
+
+ expect(
+ await retireIdentityLinks(
+ database,
+ vault,
+ (executor) => createAuditStore(executor),
+ userId,
+ ADMIN,
+ ),
+ ).toBe(1);
+
+ const rows = await database
+ .select()
+ .from(auditEvents)
+ .where(eq(auditEvents.targetId, linkId));
+ expect(rows).toHaveLength(1);
+ expect(rows[0]?.eventType).toBe("identity.link_retired");
+});
+
test("a pending challenge cannot be redeemed after the person is removed", async () => {
const userId = user();
const identities = createIdentityStore(database, vault);
const { code } = await identities.issueChallenge(userId, "slack");
- await retireIdentityLinks(database, vault, recorder().store, userId, ADMIN);
+ await retireIdentityLinks(
+ database,
+ vault,
+ () => recorder().store,
+ userId,
+ ADMIN,
+ );
const pending = await database
.select()
@@ -198,7 +303,7 @@ test("retirement waits on the per-person identity lock that issuing a code takes
const retiring = retireIdentityLinks(
database,
vault,
- recorder().store,
+ () => recorder().store,
userId,
ADMIN,
).then((count) => {
From 7c697ac80769bbf01f06f58160caf8c51c58b9f1 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:53:14 -0300
Subject: [PATCH 053/101] fix(server): record linked-account retirement
accurately
---
server/src/audit.ts | 12 ++-
server/src/identity/retire.ts | 55 ++++++++++----
.../tests/identity-retire.integration.test.ts | 73 ++++++++++++++++++-
3 files changed, 119 insertions(+), 21 deletions(-)
diff --git a/server/src/audit.ts b/server/src/audit.ts
index 5d487905f..36c4ecb33 100644
--- a/server/src/audit.ts
+++ b/server/src/audit.ts
@@ -511,11 +511,17 @@ export const auditEventTypes = [
/* A Bot connected to, or disconnected from, a Slack, Teams or SMS conversation. */
"delivery.linked",
"delivery.unlinked",
- /* A person removing their own linked account at an outside provider (Slack, GitHub). */
+ /*
+ * A person removing their own link to an outside account (Slack, GitHub). The link and its stored
+ * token go here; nothing is changed at the provider.
+ */
"identity.unlinked",
/*
- * A removed person's linked account retired by the removal: marked `needs_reconnect`, its token (if
- * any) revoked. Names the link, its provider and the remover only.
+ * A removed person's linked account retired by the removal: marked `needs_reconnect`, its stored
+ * token (if any) revoked here. Target the link id; payload `actor` (the remover, not always a user
+ * id, so never `actorUserId`), `owner` (the removed person), `provider`, `reason`
+ * ("person_removed") and `credentialRevoked` (whether this removal revoked a live token). Never the
+ * subject, realm or credential id.
*/
"identity.link_retired",
/* Routines, however they were made: by a person on the screen or by a Bot through its tools. */
diff --git a/server/src/identity/retire.ts b/server/src/identity/retire.ts
index 8f9cf2344..b99d71290 100644
--- a/server/src/identity/retire.ts
+++ b/server/src/identity/retire.ts
@@ -1,10 +1,31 @@
import { asc, eq } from "drizzle-orm";
import { type AuditStore, recordAuditEvent } from "../audit";
-import { CredentialRefusedError, type CredentialStore } from "../credentials";
+import {
+ type CredentialExecutor,
+ CredentialRefusedError,
+ type CredentialStore,
+} from "../credentials";
import type { Database } from "../db/client";
import { identityLinkChallenges, identityLinks } from "../db/schema";
import { identityRealmLock, identityUserLock } from "./store";
+/** Revokes a live credential; false when there is none or it was already revoked. */
+async function revokeIfLive(
+ credentials: Pick,
+ id: string | null,
+ executor: CredentialExecutor,
+): Promise {
+ if (!id) return false;
+ try {
+ await credentials.revoke(id, executor);
+ return true;
+ } catch (error) {
+ // Already revoked is the outcome wanted, but not this call's doing.
+ if (error instanceof CredentialRefusedError) return false;
+ throw error;
+ }
+}
+
/**
* Retires a removed person's identity links.
*
@@ -20,16 +41,21 @@ import { identityRealmLock, identityUserLock } from "./store";
* exactly when its `identity.link_retired` row is written. A failure anywhere, an audit row
* included, rolls the whole retirement back and a retry starts over and audits every link; writing
* the rows after the commit instead lost the rest for good when one failed, because the retry found
- * every link already retired. One row per link that changed: actor `by`, target the link id, and the
- * provider, the reason and whether a credential was revoked. Never the subject, realm or credential
- * id.
+ * every link already retired.
+ *
+ * One row per link that changed, shaped like `retireConnectionsFor`'s `mcp.account_disconnected`
+ * rows: target the link id; payload `actor` (the remover, `by`), `owner` (the removed person),
+ * `provider`, `reason` ("person_removed") and `credentialRevoked`, true only when this call revoked
+ * a live credential (one already revoked is detached but not reported as revoked). The remover goes
+ * in the payload and not in `actorUserId`, because `by` need not be a user id: a SCIM removal passes
+ * "scim:directory". Never the subject, realm or credential id.
*
* Concurrency. Locks are taken in this order: `identityUserLock`, the lock `issueChallenge` takes, so
* no code is issued alongside the removal; then the challenges are deleted, which waits for a
* `redeemChallenge` in flight (it holds its challenge row) to commit, so the link it wrote is seen
* below; then `identityRealmLock` for every `(provider, realm)` the person has a link in, in sorted
- * order (writeLink takes the one key for the realm it writes, so sorting only keeps two retirements
- * of one person from deadlocking each other); then the link rows. A writeLink into one of those realms
+ * order (defensive: writeLink takes only the one key for the realm it writes, and two retirements of
+ * one person already queue on the user lock); then the link rows. A writeLink into one of those realms
* that is already running finishes first and is retired here. One that arrives later waits, and once
* this commits it goes ahead: it re-links that realm for the removed person, active again, so
* refusing it is left to linkedUser's callers, which refuse a removed person. The same holds for a
@@ -78,14 +104,11 @@ export async function retireIdentityLinks(
let changed = 0;
for (const link of links) {
if (!link.credentialId && link.status === "needs_reconnect") continue;
- if (link.credentialId) {
- try {
- await credentials.revoke(link.credentialId, transaction);
- } catch (error) {
- // Already revoked is the outcome we wanted.
- if (!(error instanceof CredentialRefusedError)) throw error;
- }
- }
+ const credentialRevoked = await revokeIfLive(
+ credentials,
+ link.credentialId,
+ transaction,
+ );
await transaction
.update(identityLinks)
.set({
@@ -98,12 +121,12 @@ export async function retireIdentityLinks(
eventType: "identity.link_retired",
targetType: "identity_link",
targetId: link.id,
- actorUserId: by,
payload: {
actor: by,
+ owner: userId,
provider: link.provider,
reason: "person_removed",
- credentialRevoked: Boolean(link.credentialId),
+ credentialRevoked,
},
});
changed += 1;
diff --git a/server/tests/identity-retire.integration.test.ts b/server/tests/identity-retire.integration.test.ts
index cb1d75bd5..c4cb1ac1f 100644
--- a/server/tests/identity-retire.integration.test.ts
+++ b/server/tests/identity-retire.integration.test.ts
@@ -113,14 +113,15 @@ test("retiring a removed person's links revokes their tokens and keeps the rows"
eventType: "identity.link_retired",
targetType: "identity_link",
targetId: linkId,
- actorUserId: ADMIN,
payload: {
actor: ADMIN,
+ owner: userId,
provider: "github",
reason: "person_removed",
credentialRevoked: true,
},
});
+ expect(audit.rows[0]?.actorUserId).toBeUndefined();
const payload = JSON.stringify(audit.rows[0]?.payload);
expect(payload).not.toContain(realm);
expect(payload).not.toContain(credential.id);
@@ -170,8 +171,9 @@ test("a link without a credential is also marked needs_reconnect, and audited wi
expect(audit.rows[0]).toMatchObject({
eventType: "identity.link_retired",
targetId: linkId,
- actorUserId: ADMIN,
payload: {
+ actor: ADMIN,
+ owner: userId,
provider: "slack",
reason: "person_removed",
credentialRevoked: false,
@@ -182,6 +184,73 @@ test("a link without a credential is also marked needs_reconnect, and audited wi
expect(payload).not.toContain(realm);
});
+test("a credential revoked before the removal is not reported as revoked by it", async () => {
+ const userId = user();
+ const credential = await vault.create({
+ kind: "connector",
+ provider: "github-user-token",
+ keyId: randomUUID(),
+ metadata: {},
+ encryptedValue: "x",
+ });
+ createdCredentials.push(credential.id);
+ await vault.revoke(credential.id);
+ const linkId = randomUUID();
+ await database.insert(identityLinks).values({
+ id: linkId,
+ provider: "github",
+ realm,
+ subject: randomUUID(),
+ userId,
+ verifiedBy: "oauth",
+ credentialId: credential.id,
+ status: "active",
+ });
+ const audit = recorder();
+
+ expect(
+ await retireIdentityLinks(
+ database,
+ vault,
+ () => audit.store,
+ userId,
+ ADMIN,
+ ),
+ ).toBe(1);
+
+ expect((await statusOf(linkId))?.credentialId).toBeNull();
+ expect(audit.rows[0]?.payload).toMatchObject({ credentialRevoked: false });
+});
+
+test("a directory remover is recorded in the payload only, never as the acting user", async () => {
+ const userId = user();
+ await database.insert(identityLinks).values({
+ id: randomUUID(),
+ provider: "slack",
+ realm,
+ subject: randomUUID(),
+ userId,
+ verifiedBy: "challenge",
+ status: "active",
+ });
+ const audit = recorder();
+
+ await retireIdentityLinks(
+ database,
+ vault,
+ () => audit.store,
+ userId,
+ "scim:directory",
+ );
+
+ expect(audit.rows).toHaveLength(1);
+ expect(audit.rows[0]?.actorUserId).toBeUndefined();
+ expect(audit.rows[0]?.payload).toMatchObject({
+ actor: "scim:directory",
+ owner: userId,
+ });
+});
+
test("an audit row that fails to write rolls the retirement back, so a retry retires and audits every link", async () => {
const userId = user();
const linkIds = [randomUUID(), randomUUID()];
From 927bf12a4e88659c3be19e16ebd108a35c390815 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 12:54:02 -0300
Subject: [PATCH 054/101] fix(server): name both failures when retiring a
person's accounts
---
server/src/identity/retire.ts | 10 +++++++---
server/src/index.ts | 18 +++++++++---------
.../tests/identity-retire.integration.test.ts | 3 +++
3 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/server/src/identity/retire.ts b/server/src/identity/retire.ts
index b99d71290..b43c3c01d 100644
--- a/server/src/identity/retire.ts
+++ b/server/src/identity/retire.ts
@@ -140,8 +140,9 @@ export async function retireIdentityLinks(
*
* Both halves always run: the plugin half rethrows on purpose (a broker refusal), and stopping there
* left every linked account's token live, with a retry hitting the same refusal first. The failure
- * is still thrown afterwards — the first one alone, or an AggregateError of both — so the caller's
- * error handling sees it. On success, the count is both halves together.
+ * is still thrown afterwards — the first one alone, or an AggregateError of both whose message
+ * names both causes (callers log `String(error)`, which shows the message and not `.errors`) — so
+ * the caller's error handling sees it. On success, the count is both halves together.
*/
export async function retireOwnedAccounts(retirers: {
plugins: () => Promise<{ retired: number }>;
@@ -163,7 +164,10 @@ export async function retireOwnedAccounts(retirers: {
if (failures.length > 1)
throw new AggregateError(
failures,
- "Retiring a removed person's accounts failed in more than one place.",
+ `Retiring a removed person's accounts failed in both halves: plugins: ${messageOf(failures[0])}; linked accounts: ${messageOf(failures[1])}`,
);
return { retired };
}
+
+const messageOf = (error: unknown) =>
+ error instanceof Error ? error.message : String(error);
diff --git a/server/src/index.ts b/server/src/index.ts
index c187f8bd2..f843ff077 100644
--- a/server/src/index.ts
+++ b/server/src/index.ts
@@ -394,16 +394,16 @@ const peopleStore = createPeopleStore(
database,
config.auth?.initialAdminEmails ?? [],
/*
- * Removing somebody retires the credentials they granted this deployment.
+ * Removing somebody retires what they connected: their plugin credentials and connections, and
+ * their linked outside accounts (which keep resolving to them, now refused; the tokens are revoked
+ * and every link needs reconnecting). Both halves run whichever fails, and the failure is rethrown
+ * after (see retireOwnedAccounts).
*
- * A closure rather than the method itself, because the plugin store is built further down: this
- * has to exist before `auth` does, and that one needs the vault and the policy. Nothing calls this
- * during module initialisation — it runs when an administrator removes somebody, over HTTP — so by
- * then the binding is there.
- */
- /*
- * Both halves run whichever fails, and the failure is rethrown after. Their linked outside
- * accounts keep resolving to them, now refused; the tokens go and every link needs reconnecting.
+ * A closure, because `pluginStore` is built further down: this has to exist before `auth` does,
+ * and that one needs the vault and the policy. `pluginStore` is read only when the closure runs —
+ * an administrator or the directory removing somebody, never during module initialisation — so by
+ * then the binding is there. The linked-account half writes its audit rows through a store made
+ * on its own transaction, so it reads no later binding (`bootAuditStore` included).
*/
(userId, by) =>
retireOwnedAccounts({
diff --git a/server/tests/identity-retire.integration.test.ts b/server/tests/identity-retire.integration.test.ts
index c4cb1ac1f..fce05f306 100644
--- a/server/tests/identity-retire.integration.test.ts
+++ b/server/tests/identity-retire.integration.test.ts
@@ -435,6 +435,9 @@ test("both failing surfaces both", async () => {
}).catch((caught: unknown) => caught);
expect(error).toBeInstanceOf(AggregateError);
expect((error as AggregateError).errors).toEqual([first, second]);
+ // Callers log String(error), which must name both causes.
+ expect(String(error)).toContain("plugins");
+ expect(String(error)).toContain("identities");
});
test("both succeeding counts both", async () => {
From 1af385aadec26ea8f9eb9bc604ef6453624cabf1 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:37:50 -0300
Subject: [PATCH 055/101] docs: document the GitHub App sign-in variables
---
.env.example | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/.env.example b/.env.example
index 478ddab20..c70ef2afb 100644
--- a/.env.example
+++ b/.env.example
@@ -436,6 +436,19 @@ WORKER_SHARED_SECRET=
# with every call, so one account's connections are never reachable from another's.
COMPOSIO_API_KEY=
+# ---------------------------------------------------------------------------
+# Linking people's GitHub accounts
+# ---------------------------------------------------------------------------
+
+# A GitHub App lets people link their GitHub account under Settings -> Connected accounts -> Linked
+# accounts. Register the App's callback URL as /api/identity/github/callback and keep
+# "Expire user authorization tokens" on. A webhook is not needed for linking. Set both the client id
+# and the client secret, or neither: one without the other stops the server at start-up. The slug is
+# optional and is used for links to the App.
+# GITHUB_APP_CLIENT_ID=
+# GITHUB_APP_CLIENT_SECRET=
+# GITHUB_APP_SLUG=
+
# ---------------------------------------------------------------------------
# Reaching people outside the app: Slack, Teams, SMS, push
# ---------------------------------------------------------------------------
From 36c16e7f5e8c559fef20b9e1644050e02682303a Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:38:05 -0300
Subject: [PATCH 056/101] feat(server): read GitHub App sign-in settings
---
server/src/config.ts | 30 +++++++++++++
server/tests/config-github-app.test.ts | 59 ++++++++++++++++++++++++++
2 files changed, 89 insertions(+)
create mode 100644 server/tests/config-github-app.test.ts
diff --git a/server/src/config.ts b/server/src/config.ts
index bb557c663..d8196bea5 100644
--- a/server/src/config.ts
+++ b/server/src/config.ts
@@ -90,6 +90,12 @@ export type AuthProviderId = "google" | "microsoft" | "okta";
/** An OAuth client, as every provider here needs one. */
export type OAuthClient = { clientId: string; clientSecret: string };
+export type GithubAppConfig = {
+ clientId: string;
+ clientSecret: string;
+ slug?: string;
+};
+
export type AuthConfig = {
baseUrl: string;
secret: string;
@@ -217,6 +223,8 @@ export type DeploymentConfig = {
* where a key that was revoked last week would have surfaced regardless.
*/
composioApiKey: string | undefined;
+ /** GitHub App user sign-in, so people can link their GitHub account. Both or neither of GITHUB_APP_CLIENT_ID / GITHUB_APP_CLIENT_SECRET. */
+ githubApp?: GithubAppConfig;
/**
* Where this deployment is reached from outside, with no trailing slash.
*
@@ -664,6 +672,26 @@ function oauthClient(
return clientId && clientSecret ? { clientId, clientSecret } : undefined;
}
+function githubAppConfig(
+ environment: Environment,
+): GithubAppConfig | undefined {
+ const clientId = optional(environment, "GITHUB_APP_CLIENT_ID");
+ const clientSecret = optional(environment, "GITHUB_APP_CLIENT_SECRET");
+ const slug = optional(environment, "GITHUB_APP_SLUG");
+
+ // Both or neither. One alone is a half-configured sign-in that fails at the first attempt rather
+ // than at start-up, which is the worst moment to discover it.
+ if (Boolean(clientId) !== Boolean(clientSecret)) {
+ throw new Error(
+ "GITHUB_APP_CLIENT_ID and GITHUB_APP_CLIENT_SECRET must be set together",
+ );
+ }
+
+ return clientId && clientSecret
+ ? { clientId, clientSecret, ...(slug ? { slug } : {}) }
+ : undefined;
+}
+
function commaSeparated(environment: Environment, name: string): string[] {
return (optional(environment, name) ?? "")
.split(",")
@@ -1269,6 +1297,7 @@ export function loadConfig(
? organizationAuthority(organizationAuthValue)
: undefined;
const managedAgent = managedAgentConfig(environment);
+ const githubApp = githubAppConfig(environment);
const workerSharedSecret = optional(environment, "WORKER_SHARED_SECRET");
return {
@@ -1278,6 +1307,7 @@ export function loadConfig(
databaseUrl: required(environment, "DATABASE_URL"),
keyEncryptionKey: keyEncryptionKey(environment),
...(managedAgent ? { managedAgent } : {}),
+ ...(githubApp ? { githubApp } : {}),
agentEndpointAllowedHosts: agentEndpointAllowedHosts(environment),
deploymentId: optional(environment, "DEPLOYMENT_ID"),
composioApiKey: optional(environment, "COMPOSIO_API_KEY"),
diff --git a/server/tests/config-github-app.test.ts b/server/tests/config-github-app.test.ts
new file mode 100644
index 000000000..5af173e0c
--- /dev/null
+++ b/server/tests/config-github-app.test.ts
@@ -0,0 +1,59 @@
+import { describe, expect, test } from "bun:test";
+import { loadConfig } from "../src/config";
+
+const baseEnvironment = {
+ DATABASE_URL: "postgres://openbot:openbot@localhost:5432/openbot",
+ KEY_ENCRYPTION_KEY: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=",
+ GOOGLE_OAUTH_CLIENT_ID: "google-client-id",
+ GOOGLE_OAUTH_CLIENT_SECRET: "google-client-secret",
+ BETTER_AUTH_SECRET: "a-long-enough-local-development-auth-secret",
+ BETTER_AUTH_URL: "http://localhost:3001",
+ INITIAL_ADMIN_EMAILS: "admin@openbot.test",
+ INTELLIGENCE_API_URL: "http://localhost:7100",
+ INTELLIGENCE_GATEWAY_WS_URL: "ws://localhost:7103",
+ INTELLIGENCE_API_KEY: "tenant-api-key",
+ COPILOTKIT_LICENSE_TOKEN: "license-token",
+ MANAGED_AGENT_AG_UI_URL: " http://localhost:4200/ag-ui ",
+ MANAGED_AGENT_TOKEN: "managed-agent-token",
+};
+
+describe("GitHub App sign-in configuration", () => {
+ test("is absent when no GITHUB_APP_* is set", () => {
+ expect(loadConfig(baseEnvironment).githubApp).toBeUndefined();
+ });
+
+ test("reads the client id and secret", () => {
+ const config = loadConfig({
+ ...baseEnvironment,
+ GITHUB_APP_CLIENT_ID: "gh-id",
+ GITHUB_APP_CLIENT_SECRET: "gh-secret",
+ });
+ expect(config.githubApp).toEqual({
+ clientId: "gh-id",
+ clientSecret: "gh-secret",
+ });
+ });
+
+ test("includes the slug when set", () => {
+ const config = loadConfig({
+ ...baseEnvironment,
+ GITHUB_APP_CLIENT_ID: "gh-id",
+ GITHUB_APP_CLIENT_SECRET: "gh-secret",
+ GITHUB_APP_SLUG: "openbot",
+ });
+ expect(config.githubApp).toEqual({
+ clientId: "gh-id",
+ clientSecret: "gh-secret",
+ slug: "openbot",
+ });
+ });
+
+ test.each([
+ { GITHUB_APP_CLIENT_ID: "gh-id", GITHUB_APP_CLIENT_SECRET: "" },
+ { GITHUB_APP_CLIENT_ID: "", GITHUB_APP_CLIENT_SECRET: "gh-secret" },
+ ])("refuses only one half of the pair", (half) => {
+ expect(() => loadConfig({ ...baseEnvironment, ...half })).toThrow(
+ "GITHUB_APP_CLIENT_ID and GITHUB_APP_CLIENT_SECRET must be set together",
+ );
+ });
+});
From 1d1ea8d5fe9236663d92771913539d15fd669bb9 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:38:26 -0300
Subject: [PATCH 057/101] docs: explain linking Slack and GitHub accounts
---
docs/configuration.md | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/docs/configuration.md b/docs/configuration.md
index 37b73f791..7ea1aed06 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -489,12 +489,12 @@ number opted out, later messages show `opted_out` in the delivery history instea
## Linked accounts
OpenBot keeps a record of a person's accounts at outside providers, such as their GitHub account
-or their Slack user. When a person has any, they appear under **Linked accounts** on
-`/settings/connected-accounts`, and each can be disconnected there. Disconnecting revokes any token
-stored with the link, and is audited. A person with none sees no such section.
+or their Slack user. Whenever the deployment has an identity store, the section appears under
+**Linked accounts** on `/settings/connected-accounts`. It lists the person's links and offers only
+the actions this deployment can complete, and each link can be disconnected there. Disconnecting
+revokes any token stored with the link, and is audited.
-Links are created by the integrations that support them. Until one is configured, the list stays
-empty. An integration proves an account in one of two ways: by the provider's own
+See "Linking Slack and GitHub accounts" below for setting up each provider. An integration proves an account in one of two ways: by the provider's own
sign-in, or by a one-time code that OpenBot issues to the signed-in person and that they send from
their chat account. Whoever sends the code gets that chat account linked to the person it was issued
to, so treat it like a password and do not share it. It is valid for ten minutes and works once, and
@@ -508,9 +508,26 @@ marked as needing reconnection. The links are kept, not deleted, so the outside
resolves to the removed person and is refused rather than treated as an unlinked guest. Each retired
link is recorded in the audit trail.
-This is separate from the OpenTag **Reachability** pairing above. That pairing binds a conversation
-for delivery and uses its own `link ` messages, shown on the **Reachability** screen; it is not
-recorded as a linked account.
+### Linking Slack and GitHub accounts
+
+Slack requires the OpenTag pairing above, so `OPENTAG_SHARED_SECRET` must be set. The person clicks
+**Link Slack**, which shows a message of the form `link `, and sends it to the OpenBot app in
+Slack within ten minutes. Teams cannot be linked this way.
+
+GitHub uses a GitHub App. Create one under GitHub Settings, Developer settings, GitHub Apps, and set:
+
+- The callback URL to `/api/identity/github/callback`, where the public URL is
+ `OPENBOT_PUBLIC_URL` or, if that is unset, `BETTER_AUTH_URL`.
+- "Expire user authorization tokens" on. A webhook is not required for linking.
+- A client secret, generated on the App's page.
+
+Then set `GITHUB_APP_CLIENT_ID` and `GITHUB_APP_CLIENT_SECRET`; set both or neither. `GITHUB_APP_SLUG`
+is optional. People then use **Connect GitHub** while signed in to OpenBot in the same browser. The
+GitHub user token is stored encrypted with the link.
+
+Reachability codes and Linked accounts codes look alike, and Reachability's are checked first. A
+Reachability `link ` binds a conversation for delivery and is not recorded as a linked
+account; a `link ` from **Linked accounts** links the Slack user.
## Text messages and push notifications
From a222ab885c762d435e2787fce396cbe2de471290 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:38:34 -0300
Subject: [PATCH 058/101] feat(server): let the OpenTag handler redeem identity
link codes
---
server/src/delivery/opentag-agent.ts | 24 +++++++
server/src/delivery/routes.ts | 3 +
server/tests/delivery-routes.test.ts | 96 +++++++++++++++++++++++++++-
3 files changed, 122 insertions(+), 1 deletion(-)
diff --git a/server/src/delivery/opentag-agent.ts b/server/src/delivery/opentag-agent.ts
index a3d1494f6..1e6109aac 100644
--- a/server/src/delivery/opentag-agent.ts
+++ b/server/src/delivery/opentag-agent.ts
@@ -92,6 +92,16 @@ export type OpenTagAgentDeps = {
approvals?: PairedApprovals;
/** Slack trigger ingest; defaults to the responsibilities lane's `ingestSlackEvent`. */
ingestSlack?: (event: SlackTriggerEvent) => Promise;
+ /**
+ * Redeems OpenBot linked-account codes (Settings → Connected accounts) for a Slack sender.
+ * Reachability codes are always checked first; this only sees codes that are not one.
+ */
+ identity?: {
+ redeem(
+ code: string,
+ sender: { teamId: string; userId: string },
+ ): Promise<"linked" | "conflict" | "invalid">;
+ };
};
const LINK =
@@ -356,6 +366,20 @@ async function link(
code: string,
) {
const challenge = await deps.store.readChallenge(code);
+ if (!challenge && sender.transport === "slack" && deps.identity) {
+ const outcome = await deps.identity.redeem(code, {
+ teamId: sender.realm,
+ userId: sender.identity,
+ });
+ if (outcome === "linked")
+ return {
+ text: "Linked your Slack account to OpenBot. You can see it under Settings → Connected accounts.",
+ };
+ if (outcome === "conflict")
+ return {
+ text: "This Slack account is already linked to another OpenBot user.",
+ };
+ }
if (!challenge || challenge.transport !== sender.transport)
return {
text: "That link code expired, was already used, or is for another app. Create a new one in OpenBot.",
diff --git a/server/src/delivery/routes.ts b/server/src/delivery/routes.ts
index 13dc0fb64..3823ede4c 100644
--- a/server/src/delivery/routes.ts
+++ b/server/src/delivery/routes.ts
@@ -281,6 +281,8 @@ export function createDeliveryWebhookRoutes(deps: {
approvals?: PairedApprovals;
/** Test seam; production uses the responsibilities lane's `ingestSlackEvent`. */
ingestSlack?: OpenTagAgentDeps["ingestSlack"];
+ /** Redeems OpenBot linked-account codes sent to the Slack app. */
+ identity?: OpenTagAgentDeps["identity"];
}) {
const routes = new Hono<{ Variables: AppVariables }>();
// A paired chat run carries its conversation's history, so it gets a larger envelope.
@@ -303,6 +305,7 @@ export function createDeliveryWebhookRoutes(deps: {
scopeFor: deps.scopeFor,
...(deps.approvals ? { approvals: deps.approvals } : {}),
...(deps.ingestSlack ? { ingestSlack: deps.ingestSlack } : {}),
+ ...(deps.identity ? { identity: deps.identity } : {}),
},
c.req.raw,
),
diff --git a/server/tests/delivery-routes.test.ts b/server/tests/delivery-routes.test.ts
index 677ac5650..156874001 100644
--- a/server/tests/delivery-routes.test.ts
+++ b/server/tests/delivery-routes.test.ts
@@ -20,7 +20,19 @@ const bound: DeliveryBinding = {
address: "U1",
enabled: true,
};
-function fixture(options: { converse?: ConverseResult; bound?: boolean } = {}) {
+type IdentityAdapter = {
+ redeem(
+ code: string,
+ sender: { teamId: string; userId: string },
+ ): Promise<"linked" | "conflict" | "invalid">;
+};
+function fixture(
+ options: {
+ converse?: ConverseResult;
+ bound?: boolean;
+ identity?: IdentityAdapter;
+ } = {},
+) {
const calls: unknown[] = [];
const decisions: unknown[] = [];
const twilio = createTwilioTransport({
@@ -97,6 +109,7 @@ function fixture(options: { converse?: ConverseResult; bound?: boolean } = {}) {
return { matched: 0, deliveries: [] };
},
twilio,
+ ...(options.identity ? { identity: options.identity } : {}),
approvals: {
store: {
get: async (owner, id) => {
@@ -223,6 +236,87 @@ test("a link code binds the verified sender to the challenge's own scope", async
},
]);
});
+const IDENTITY_CODE = "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee";
+const EXPIRED =
+ "That link code expired, was already used, or is for another app. Create a new one in OpenBot.";
+function identityFixture(outcome?: "linked" | "conflict" | "invalid") {
+ const redeemed: unknown[] = [];
+ const f = fixture({
+ bound: false,
+ ...(outcome
+ ? {
+ identity: {
+ redeem: async (code, who) => {
+ redeemed.push([code, who]);
+ return outcome;
+ },
+ },
+ }
+ : {}),
+ });
+ return { f, redeemed };
+}
+const linkMessage = (code: string) => ({
+ messages: [
+ { id: "m1", role: "user", content: `<@UBOT> link ${code.toUpperCase()}` },
+ ],
+});
+test("a Reachability code is checked before the identity adapter", async () => {
+ const { f, redeemed } = identityFixture("linked");
+ const result = await run(f, {
+ context: [sender()],
+ ...linkMessage("11111111-2222-4333-8444-555555555555"),
+ });
+ expect(said(result.events)).toContain("Linked");
+ expect(f.calls).toHaveLength(1);
+ expect(redeemed).toEqual([]);
+});
+test("an identity code from a Slack sender is redeemed through the adapter", async () => {
+ const { f, redeemed } = identityFixture("linked");
+ const result = await run(f, {
+ context: [sender()],
+ ...linkMessage(IDENTITY_CODE),
+ });
+ expect(said(result.events)).toBe(
+ "Linked your Slack account to OpenBot. You can see it under Settings → Connected accounts.",
+ );
+ expect(redeemed).toEqual([[IDENTITY_CODE, { teamId: "T1", userId: "U1" }]]);
+});
+test("an identity code conflict names the other user", async () => {
+ const { f } = identityFixture("conflict");
+ const result = await run(f, {
+ context: [sender()],
+ ...linkMessage(IDENTITY_CODE),
+ });
+ expect(said(result.events)).toBe(
+ "This Slack account is already linked to another OpenBot user.",
+ );
+});
+test("an invalid identity code gets the expired reply", async () => {
+ const { f } = identityFixture("invalid");
+ const result = await run(f, {
+ context: [sender()],
+ ...linkMessage(IDENTITY_CODE),
+ });
+ expect(said(result.events)).toBe(EXPIRED);
+});
+test("a Teams sender never reaches the identity adapter", async () => {
+ const { f, redeemed } = identityFixture("linked");
+ const result = await run(f, {
+ context: [sender("teams:tenant1:user1")],
+ ...linkMessage(IDENTITY_CODE),
+ });
+ expect(said(result.events)).toBe(EXPIRED);
+ expect(redeemed).toEqual([]);
+});
+test("without an identity adapter an unknown code gets the expired reply", async () => {
+ const { f } = identityFixture();
+ const result = await run(f, {
+ context: [sender()],
+ ...linkMessage(IDENTITY_CODE),
+ });
+ expect(said(result.events)).toBe(EXPIRED);
+});
test("bound sender runs the canonical turn and an approval becomes an OpenTag card", async () => {
const f = fixture({
converse: {
From 911b726e5e245d6d1616af2fa2a5631d64904cae Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:38:36 -0300
Subject: [PATCH 059/101] feat(server): add GitHub App user sign-in client
---
server/src/identity/github-oauth.ts | 209 ++++++++++++++++++++
server/tests/identity-github-oauth.test.ts | 213 +++++++++++++++++++++
2 files changed, 422 insertions(+)
create mode 100644 server/src/identity/github-oauth.ts
create mode 100644 server/tests/identity-github-oauth.test.ts
diff --git a/server/src/identity/github-oauth.ts b/server/src/identity/github-oauth.ts
new file mode 100644
index 000000000..9057c00c0
--- /dev/null
+++ b/server/src/identity/github-oauth.ts
@@ -0,0 +1,209 @@
+import { z } from "zod";
+import { seal, unseal } from "../auth/signed-value";
+
+/**
+ * Signing a person in with the deployment's GitHub App.
+ *
+ * Four steps: build the authorize URL, remember who started it in a sealed state, trade the code
+ * for tokens, and ask GitHub who the token belongs to. Every failure is a `GithubOAuthError` with a
+ * fixed message: the vendor's body, the code, the tokens and the client secret never appear in one.
+ */
+
+export const GITHUB_CALLBACK_PATH = "/api/identity/github/callback";
+
+const STATE_LABEL = "identity-github-connect";
+const STATE_TTL_MS = 10 * 60_000;
+const REQUEST_TIMEOUT_MS = 15_000;
+
+const AUTHORIZE_URL = "https://github.com/login/oauth/authorize";
+const TOKEN_URL = "https://github.com/login/oauth/access_token";
+const USER_URL = "https://api.github.com/user";
+
+export class GithubOAuthError extends Error {
+ constructor(message: string) {
+ super(message);
+ this.name = "GithubOAuthError";
+ }
+}
+
+export function githubRedirectUri(publicUrl: string): string {
+ return `${publicUrl.replace(/\/+$/, "")}${GITHUB_CALLBACK_PATH}`;
+}
+
+const stateSchema = z.object({
+ userId: z.string().min(1),
+ issuedAt: z.iso.datetime(),
+});
+
+export async function sealGithubState(
+ userId: string,
+ encryptionKey: string,
+ now = new Date(),
+): Promise {
+ return seal(
+ JSON.stringify({ userId, issuedAt: now.toISOString() }),
+ encryptionKey,
+ STATE_LABEL,
+ );
+}
+
+/** The person a state was issued to, or nothing for every way of being unusable. */
+export async function openGithubState(
+ sealed: string | undefined,
+ encryptionKey: string,
+ now = new Date(),
+): Promise<{ userId: string } | null> {
+ const value = await unseal(sealed, encryptionKey, STATE_LABEL);
+ if (!value) return null;
+
+ let json: unknown;
+ try {
+ json = JSON.parse(value);
+ } catch {
+ return null;
+ }
+
+ const parsed = stateSchema.safeParse(json);
+ if (!parsed.success) return null;
+
+ const age = now.getTime() - new Date(parsed.data.issuedAt).getTime();
+ if (age < 0 || age > STATE_TTL_MS) return null;
+
+ return { userId: parsed.data.userId };
+}
+
+/** No scope: a GitHub App's permissions come from its installation, not from the request. */
+export function githubAuthorizationUrl(input: {
+ clientId: string;
+ redirectUri: string;
+ state: string;
+}): string {
+ const url = new URL(AUTHORIZE_URL);
+ url.searchParams.set("client_id", input.clientId);
+ url.searchParams.set("redirect_uri", input.redirectUri);
+ url.searchParams.set("state", input.state);
+ return url.toString();
+}
+
+export type GithubTokens = {
+ accessToken: string;
+ refreshToken: string | null;
+ expiresAt: string | null;
+ refreshTokenExpiresAt: string | null;
+};
+
+const tokenResponseSchema = z.object({
+ access_token: z.string().min(1),
+ refresh_token: z.string().min(1).optional(),
+ expires_in: z.number().positive().optional(),
+ refresh_token_expires_in: z.number().positive().optional(),
+});
+
+function secondsFrom(now: Date, seconds: number | undefined): string | null {
+ return seconds === undefined
+ ? null
+ : new Date(now.getTime() + seconds * 1000).toISOString();
+}
+
+export async function exchangeGithubCode(
+ input: {
+ clientId: string;
+ clientSecret: string;
+ code: string;
+ redirectUri: string;
+ },
+ fetchImpl: typeof fetch = fetch,
+ now = new Date(),
+): Promise {
+ let response: Response;
+ try {
+ response = await fetchImpl(TOKEN_URL, {
+ method: "POST",
+ headers: {
+ Accept: "application/json",
+ "Content-Type": "application/json",
+ },
+ body: JSON.stringify({
+ client_id: input.clientId,
+ client_secret: input.clientSecret,
+ code: input.code,
+ redirect_uri: input.redirectUri,
+ }),
+ // The request carries a client secret and a code; a redirect is a refusal, not a detour.
+ redirect: "manual",
+ signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
+ });
+ } catch {
+ throw new GithubOAuthError("GitHub could not be reached");
+ }
+
+ if (!response.ok) {
+ throw new GithubOAuthError("GitHub refused the authorization code");
+ }
+
+ let body: unknown;
+ try {
+ body = await response.json();
+ } catch {
+ throw new GithubOAuthError("GitHub returned an unreadable response");
+ }
+
+ // GitHub reports a bad code as a 200 with an `error` field, so no access_token is the refusal.
+ const parsed = tokenResponseSchema.safeParse(body);
+ if (!parsed.success) {
+ throw new GithubOAuthError("GitHub did not return an access token");
+ }
+
+ return {
+ accessToken: parsed.data.access_token,
+ refreshToken: parsed.data.refresh_token ?? null,
+ expiresAt: secondsFrom(now, parsed.data.expires_in),
+ refreshTokenExpiresAt: secondsFrom(
+ now,
+ parsed.data.refresh_token_expires_in,
+ ),
+ };
+}
+
+const userSchema = z.object({
+ id: z.number().int().positive(),
+ login: z.string().min(1).max(39),
+});
+
+export async function fetchGithubUser(
+ accessToken: string,
+ fetchImpl: typeof fetch = fetch,
+): Promise<{ id: number; login: string }> {
+ let response: Response;
+ try {
+ response = await fetchImpl(USER_URL, {
+ method: "GET",
+ headers: {
+ Authorization: `Bearer ${accessToken}`,
+ Accept: "application/vnd.github+json",
+ "X-GitHub-Api-Version": "2022-11-28",
+ },
+ redirect: "manual",
+ signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
+ });
+ } catch {
+ throw new GithubOAuthError("GitHub could not be reached");
+ }
+
+ if (!response.ok) {
+ throw new GithubOAuthError("GitHub refused to identify the account");
+ }
+
+ let body: unknown;
+ try {
+ body = await response.json();
+ } catch {
+ throw new GithubOAuthError("GitHub returned an unreadable response");
+ }
+
+ const parsed = userSchema.safeParse(body);
+ if (!parsed.success) {
+ throw new GithubOAuthError("GitHub returned an unexpected account");
+ }
+ return { id: parsed.data.id, login: parsed.data.login };
+}
diff --git a/server/tests/identity-github-oauth.test.ts b/server/tests/identity-github-oauth.test.ts
new file mode 100644
index 000000000..b38a0d43f
--- /dev/null
+++ b/server/tests/identity-github-oauth.test.ts
@@ -0,0 +1,213 @@
+import { describe, expect, test } from "bun:test";
+import { seal } from "../src/auth/signed-value";
+import {
+ exchangeGithubCode,
+ fetchGithubUser,
+ GithubOAuthError,
+ githubAuthorizationUrl,
+ githubRedirectUri,
+ openGithubState,
+ sealGithubState,
+} from "../src/identity/github-oauth";
+
+const key = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
+const now = new Date("2026-01-01T00:00:00.000Z");
+
+type Call = { url: string; init: RequestInit };
+
+function stub(response: () => Response) {
+ const calls: Call[] = [];
+ const fetchImpl = (async (
+ url: string | URL | Request,
+ init?: RequestInit,
+ ) => {
+ calls.push({ url: String(url), init: init ?? {} });
+ return response();
+ }) as unknown as typeof fetch;
+ return { calls, fetchImpl };
+}
+
+const input = {
+ clientId: "client-id",
+ clientSecret: "super-secret-value",
+ code: "one-time-code-123",
+ redirectUri: "https://o.test/api/identity/github/callback",
+};
+
+describe("GitHub authorize URL and redirect URI", () => {
+ test("authorize URL carries exactly client_id, redirect_uri and state", () => {
+ const url = new URL(
+ githubAuthorizationUrl({
+ clientId: "cid",
+ redirectUri: "https://o.test/cb",
+ state: "st",
+ }),
+ );
+ expect(`${url.origin}${url.pathname}`).toBe(
+ "https://github.com/login/oauth/authorize",
+ );
+ expect([...url.searchParams.keys()].sort()).toEqual([
+ "client_id",
+ "redirect_uri",
+ "state",
+ ]);
+ expect(url.searchParams.get("redirect_uri")).toBe("https://o.test/cb");
+ });
+
+ test("redirect URI strips trailing slashes", () => {
+ expect(githubRedirectUri("https://o.test")).toBe(
+ "https://o.test/api/identity/github/callback",
+ );
+ expect(githubRedirectUri("https://o.test//")).toBe(
+ "https://o.test/api/identity/github/callback",
+ );
+ });
+});
+
+describe("GitHub state", () => {
+ test("round-trips to the user", async () => {
+ const sealed = await sealGithubState("u1", key, now);
+ expect(await openGithubState(sealed, key, now)).toEqual({ userId: "u1" });
+ });
+
+ test("expires after ten minutes", async () => {
+ const sealed = await sealGithubState("u1", key, now);
+ const later = new Date(now.getTime() + 11 * 60_000);
+ expect(await openGithubState(sealed, key, later)).toBeNull();
+ });
+
+ test("is rejected from the future", async () => {
+ const sealed = await sealGithubState("u1", key, now);
+ const earlier = new Date(now.getTime() - 60_000);
+ expect(await openGithubState(sealed, key, earlier)).toBeNull();
+ });
+
+ test("missing and garbage values are null", async () => {
+ expect(await openGithubState(undefined, key, now)).toBeNull();
+ expect(await openGithubState("garbage", key, now)).toBeNull();
+ });
+
+ test("another label is null", async () => {
+ const sealed = await seal(
+ JSON.stringify({ userId: "u", issuedAt: now.toISOString() }),
+ key,
+ "other-label",
+ );
+ expect(await openGithubState(sealed, key, now)).toBeNull();
+ });
+
+ test("wrong shape is null", async () => {
+ const sealed = await seal(
+ JSON.stringify({ userId: "", issuedAt: now.toISOString() }),
+ key,
+ "identity-github-connect",
+ );
+ expect(await openGithubState(sealed, key, now)).toBeNull();
+ });
+});
+
+describe("exchangeGithubCode", () => {
+ test("posts JSON and maps expiries from now", async () => {
+ const { calls, fetchImpl } = stub(() =>
+ Response.json({
+ access_token: "at",
+ refresh_token: "rt",
+ expires_in: 28800,
+ refresh_token_expires_in: 15811200,
+ }),
+ );
+ const tokens = await exchangeGithubCode(input, fetchImpl, now);
+ expect(tokens).toEqual({
+ accessToken: "at",
+ refreshToken: "rt",
+ expiresAt: new Date(now.getTime() + 28800_000).toISOString(),
+ refreshTokenExpiresAt: new Date(
+ now.getTime() + 15811200_000,
+ ).toISOString(),
+ });
+ const call = calls[0];
+ expect(call.url).toBe("https://github.com/login/oauth/access_token");
+ expect(call.init.method).toBe("POST");
+ expect(call.init.redirect).toBe("manual");
+ const headers = new Headers(call.init.headers);
+ expect(headers.get("accept")).toBe("application/json");
+ expect(headers.get("content-type")).toBe("application/json");
+ expect(JSON.parse(String(call.init.body))).toEqual({
+ client_id: "client-id",
+ client_secret: "super-secret-value",
+ code: "one-time-code-123",
+ redirect_uri: input.redirectUri,
+ });
+ });
+
+ test("absent expiries and refresh token become null", async () => {
+ const { fetchImpl } = stub(() => Response.json({ access_token: "at" }));
+ expect(await exchangeGithubCode(input, fetchImpl, now)).toEqual({
+ accessToken: "at",
+ refreshToken: null,
+ expiresAt: null,
+ refreshTokenExpiresAt: null,
+ });
+ });
+
+ test("a 200 error body throws without echoing secrets", async () => {
+ const { fetchImpl } = stub(() =>
+ Response.json({ error: "bad_verification_code" }),
+ );
+ const error = await exchangeGithubCode(input, fetchImpl, now).catch(
+ (e) => e,
+ );
+ expect(error).toBeInstanceOf(GithubOAuthError);
+ expect(error.message).not.toContain(input.code);
+ expect(error.message).not.toContain(input.clientSecret);
+ expect(error.message).not.toContain("bad_verification_code");
+ });
+
+ test("non-ok status throws", async () => {
+ const { fetchImpl } = stub(() => new Response("nope", { status: 500 }));
+ await expect(exchangeGithubCode(input, fetchImpl, now)).rejects.toThrow(
+ GithubOAuthError,
+ );
+ });
+
+ test("transport failure throws GithubOAuthError", async () => {
+ const fetchImpl = (async () => {
+ throw new Error(`connect failed ${input.clientSecret}`);
+ }) as unknown as typeof fetch;
+ const error = await exchangeGithubCode(input, fetchImpl, now).catch(
+ (e) => e,
+ );
+ expect(error).toBeInstanceOf(GithubOAuthError);
+ expect(error.message).not.toContain(input.clientSecret);
+ });
+});
+
+describe("fetchGithubUser", () => {
+ test("sends bearer headers and returns id and login", async () => {
+ const { calls, fetchImpl } = stub(() =>
+ Response.json({ id: 42, login: "octocat", email: "x@y.z" }),
+ );
+ expect(await fetchGithubUser("tok", fetchImpl)).toEqual({
+ id: 42,
+ login: "octocat",
+ });
+ const headers = new Headers(calls[0].init.headers);
+ expect(calls[0].url).toBe("https://api.github.com/user");
+ expect(headers.get("authorization")).toBe("Bearer tok");
+ expect(headers.get("accept")).toBe("application/vnd.github+json");
+ expect(headers.get("x-github-api-version")).toBe("2022-11-28");
+ expect(calls[0].init.redirect).toBe("manual");
+ });
+
+ test.each([
+ ["non-integer id", () => Response.json({ id: 1.5, login: "a" })],
+ ["string id", () => Response.json({ id: "1", login: "a" })],
+ ["empty login", () => Response.json({ id: 1, login: "" })],
+ ["non-ok", () => new Response("x", { status: 401 })],
+ ])("%s throws", async (_name, response) => {
+ const { fetchImpl } = stub(response);
+ await expect(fetchGithubUser("tok", fetchImpl)).rejects.toThrow(
+ GithubOAuthError,
+ );
+ });
+});
From e8a18d03851a405ab74c3f52b4f25bed5604d189 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:40:06 -0300
Subject: [PATCH 060/101] feat(app): ask which account types a person can link
---
app/src/lib/identity/queries.test.ts | 42 ++++++++++++++++++++++++++
app/src/lib/identity/queries.ts | 44 ++++++++++++++++++++++++++++
2 files changed, 86 insertions(+)
create mode 100644 app/src/lib/identity/queries.test.ts
diff --git a/app/src/lib/identity/queries.test.ts b/app/src/lib/identity/queries.test.ts
new file mode 100644
index 000000000..8b5bf534e
--- /dev/null
+++ b/app/src/lib/identity/queries.test.ts
@@ -0,0 +1,42 @@
+import { describe, expect, test } from "bun:test";
+import { providersFromResponse } from "./queries";
+
+describe("providersFromResponse", () => {
+ test("reads the provider flags", async () => {
+ const response = Response.json({
+ providers: { slack: true, github: false },
+ });
+ expect(await providersFromResponse(response)).toEqual({
+ slack: true,
+ github: false,
+ });
+ });
+
+ test("treats the tagged 503 as nothing linkable", async () => {
+ const response = Response.json(
+ { error: "No identity store", code: "identity_unavailable" },
+ { status: 503 },
+ );
+ expect(await providersFromResponse(response)).toEqual({
+ slack: false,
+ github: false,
+ });
+ });
+
+ test("throws the server's message for any other 503", async () => {
+ const response = Response.json({ error: "Bad gateway" }, { status: 503 });
+ await expect(providersFromResponse(response)).rejects.toThrow(
+ "Bad gateway",
+ );
+ });
+
+ test("throws when providers is missing or not boolean-shaped", async () => {
+ const message = "Could not load the account types you can link";
+ await expect(providersFromResponse(Response.json({}))).rejects.toThrow(
+ message,
+ );
+ await expect(
+ providersFromResponse(Response.json({ providers: { slack: "yes" } })),
+ ).rejects.toThrow(message);
+ });
+});
diff --git a/app/src/lib/identity/queries.ts b/app/src/lib/identity/queries.ts
index 91dcb8a27..3d20e3b99 100644
--- a/app/src/lib/identity/queries.ts
+++ b/app/src/lib/identity/queries.ts
@@ -13,6 +13,7 @@ export type LinkedAccount = {
export const identityKeys = {
all: ["identity"] as const,
links: () => [...identityKeys.all, "links"] as const,
+ providers: () => [...identityKeys.all, "providers"] as const,
};
const LOAD_FALLBACK = "Could not load your linked accounts";
@@ -54,3 +55,46 @@ export function linkedAccountsQueryOptions() {
linksFromResponse(await tryClient("/api/identity/links")),
});
}
+
+/** Which account types this deployment can link a person to. */
+export type IdentityProviders = { slack: boolean; github: boolean };
+
+const PROVIDERS_FALLBACK = "Could not load the account types you can link";
+
+/** The tagged 503 means no identity store, so nothing is linkable; other failures throw. */
+export async function providersFromResponse(
+ response: Response,
+): Promise {
+ if (!response.ok) {
+ const body = (await response.json().catch(() => null)) as {
+ error?: unknown;
+ code?: unknown;
+ } | null;
+ if (response.status === 503 && body?.code === IDENTITY_UNAVAILABLE) {
+ return { slack: false, github: false };
+ }
+ throw new Error(
+ typeof body?.error === "string" ? body.error : PROVIDERS_FALLBACK,
+ );
+ }
+ const body = (await response.json().catch(() => null)) as {
+ providers?: { slack?: unknown; github?: unknown };
+ } | null;
+ const providers = body?.providers;
+ if (
+ !providers ||
+ typeof providers.slack !== "boolean" ||
+ typeof providers.github !== "boolean"
+ ) {
+ throw new Error(PROVIDERS_FALLBACK);
+ }
+ return { slack: providers.slack, github: providers.github };
+}
+
+export function identityProvidersQueryOptions() {
+ return queryOptions({
+ queryKey: identityKeys.providers(),
+ queryFn: async (): Promise =>
+ providersFromResponse(await tryClient("/api/identity/providers")),
+ });
+}
From db8b61b9dbcf7db0b1b4cfd9d93ee4190cea2de9 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:40:09 -0300
Subject: [PATCH 061/101] feat(app): add Slack link code and GitHub connect
requests
---
app/src/lib/identity/mutations.ts | 40 ++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/app/src/lib/identity/mutations.ts b/app/src/lib/identity/mutations.ts
index 28c212c2a..a0fdeea36 100644
--- a/app/src/lib/identity/mutations.ts
+++ b/app/src/lib/identity/mutations.ts
@@ -1,5 +1,5 @@
import { mutationOptions, type QueryClient } from "@tanstack/react-query";
-import { tryClient } from "@/lib/client";
+import { client, tryClient } from "@/lib/client";
import { identityKeys } from "./queries";
const UNLINK_FALLBACK = "The account could not be disconnected";
@@ -39,3 +39,41 @@ export function unlinkMutationOptions(queryClient: QueryClient) {
queryClient.invalidateQueries({ queryKey: identityKeys.links() }),
});
}
+
+/** A one-time code the person sends to the bot in Slack to link their account. */
+export type SlackLinkCode = {
+ code: string;
+ expiresAt: string;
+ instruction: string;
+};
+
+export function issueSlackCodeMutationOptions() {
+ return mutationOptions({
+ mutationFn: (): Promise =>
+ client("/api/identity/challenges", "challenge", {
+ method: "POST",
+ body: { provider: "slack" },
+ fallback: "Could not create a Slack link code",
+ }),
+ });
+}
+
+/** Resolves to the GitHub consent URL; the caller navigates, not this factory. */
+export function connectGithubMutationOptions() {
+ return mutationOptions({
+ mutationFn: async (): Promise => {
+ const { authorizationUrl } = await client<{ authorizationUrl: unknown }>(
+ "/api/identity/github/connect",
+ "connect",
+ {
+ method: "POST",
+ fallback: "Could not start connecting GitHub",
+ },
+ );
+ if (typeof authorizationUrl !== "string") {
+ throw new Error("Could not start connecting GitHub");
+ }
+ return authorizationUrl;
+ },
+ });
+}
From e82a92818646927fd3d65a0dd6e09ff6db763438 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:43:49 -0300
Subject: [PATCH 062/101] feat(server): report which account types can be
linked
---
server/src/identity/routes.ts | 15 +++++++++++
server/tests/identity-routes.test.ts | 37 ++++++++++++++++++++++++++--
2 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/server/src/identity/routes.ts b/server/src/identity/routes.ts
index 8f6734e48..a4dddde3b 100644
--- a/server/src/identity/routes.ts
+++ b/server/src/identity/routes.ts
@@ -4,6 +4,11 @@ import type { AppVariables } from "../auth/guards";
import { PROVIDERS } from "./providers";
import type { IdentityStore } from "./store";
+export type IdentityRouteOptions = {
+ slackLinking?: boolean;
+ github?: { clientId: string; publicUrl: string; encryptionKey: string };
+};
+
/**
* A person's own linked accounts.
*
@@ -15,6 +20,7 @@ export function identityRoutes(
requireUser: MiddlewareHandler<{ Variables: AppVariables }>,
store?: Pick,
auditStore?: AuditStore,
+ options: IdentityRouteOptions = {},
) {
const app = new Hono<{ Variables: AppVariables }>();
app.use("*", async (context, next) => {
@@ -40,6 +46,15 @@ export function identityRoutes(
}
const live = store;
+ app.get("/providers", (context) =>
+ context.json({
+ providers: {
+ slack: Boolean(options.slackLinking),
+ github: Boolean(options.github),
+ },
+ }),
+ );
+
app.get("/links", async (context) => {
const links = await live.identitiesFor(context.var.actor.id);
return context.json({
diff --git a/server/tests/identity-routes.test.ts b/server/tests/identity-routes.test.ts
index f2fd0f945..2c7fea7bd 100644
--- a/server/tests/identity-routes.test.ts
+++ b/server/tests/identity-routes.test.ts
@@ -2,7 +2,10 @@ import { expect, test } from "bun:test";
import type { MiddlewareHandler } from "hono";
import type { AuditEventInput } from "../src/audit";
import type { AppVariables } from "../src/auth/guards";
-import { identityRoutes } from "../src/identity/routes";
+import {
+ type IdentityRouteOptions,
+ identityRoutes,
+} from "../src/identity/routes";
import type { IdentityLink } from "../src/identity/types";
const signedIn: MiddlewareHandler<{ Variables: AppVariables }> = async (
@@ -30,7 +33,7 @@ const link: IdentityLink = {
updatedAt: new Date("2026-10-08T00:00:00Z"),
};
-function fixture(removedProvider = "github") {
+function fixture(removedProvider = "github", options?: IdentityRouteOptions) {
const calls: string[] = [];
const events: AuditEventInput[] = [];
const app = identityRoutes(
@@ -45,6 +48,7 @@ function fixture(removedProvider = "github") {
},
},
{ insert: async (event) => void events.push(event) },
+ options,
);
return { app, calls, events };
}
@@ -156,3 +160,32 @@ test("the audit names the provider the store removed, even one outside the regis
provider: "retired-provider",
});
});
+
+const github = {
+ clientId: "Iv1.x",
+ publicUrl: "https://o.test",
+ encryptionKey: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=",
+};
+
+test("GET /providers reports which account types can be linked", async () => {
+ const cases: [IdentityRouteOptions, { slack: boolean; github: boolean }][] = [
+ [{}, { slack: false, github: false }],
+ [{ slackLinking: true }, { slack: true, github: false }],
+ [{ github }, { slack: false, github: true }],
+ ];
+ for (const [options, providers] of cases) {
+ const response = await fixture("github", options).app.request("/providers");
+ expect(response.status).toBe(200);
+ expect(response.headers.get("cache-control")).toBe("no-store");
+ expect(await response.json()).toEqual({ providers });
+ }
+});
+
+test("GET /providers without a store is 503", async () => {
+ const response = await identityRoutes(signedIn).request("/providers");
+ expect(response.status).toBe(503);
+ expect(await response.json()).toEqual({
+ error: "Linked accounts are not available.",
+ code: "identity_unavailable",
+ });
+});
From 5db674f1334518590c4da7d3849b5da984189079 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:44:06 -0300
Subject: [PATCH 063/101] feat(server): redeem Slack link codes into identity
links
---
server/src/audit.ts | 5 +
server/src/identity/slack-redeem.ts | 59 ++++++++++
server/tests/identity-slack-redeem.test.ts | 119 +++++++++++++++++++++
3 files changed, 183 insertions(+)
create mode 100644 server/src/identity/slack-redeem.ts
create mode 100644 server/tests/identity-slack-redeem.test.ts
diff --git a/server/src/audit.ts b/server/src/audit.ts
index 36c4ecb33..d53ad7722 100644
--- a/server/src/audit.ts
+++ b/server/src/audit.ts
@@ -511,6 +511,11 @@ export const auditEventTypes = [
/* A Bot connected to, or disconnected from, a Slack, Teams or SMS conversation. */
"delivery.linked",
"delivery.unlinked",
+ /*
+ * A person linking an outside account to themselves (a Slack link code, a GitHub sign-in). Target
+ * the link id; payload `actor` and `provider` only, never the subject, realm or credential id.
+ */
+ "identity.linked",
/*
* A person removing their own link to an outside account (Slack, GitHub). The link and its stored
* token go here; nothing is changed at the provider.
diff --git a/server/src/identity/slack-redeem.ts b/server/src/identity/slack-redeem.ts
new file mode 100644
index 000000000..37b156235
--- /dev/null
+++ b/server/src/identity/slack-redeem.ts
@@ -0,0 +1,59 @@
+import { type AuditStore, recordAuditEvent } from "../audit";
+import { slackRealm } from "./providers";
+import type { IdentityStore } from "./store";
+import {
+ IdentityConflictError,
+ IdentityInputError,
+ IdentityLinkError,
+} from "./types";
+
+/**
+ * Redeems a link code a person sent from Slack. Failures a person can cause become a result; any
+ * other error propagates. The audit event is written after the link commits and is not caught, as
+ * for unlinking: it names the link and provider, never the realm or Slack subject.
+ */
+export function slackCodeRedeemer(
+ store: Pick,
+ auditStore?: AuditStore,
+): {
+ redeem(
+ code: string,
+ sender: { teamId: string; userId: string },
+ ): Promise<"linked" | "conflict" | "invalid">;
+} {
+ return {
+ async redeem(code, sender) {
+ let link: Awaited>;
+ try {
+ const realm = slackRealm({
+ connectionId: "opentag",
+ installationId: "opentag",
+ workspaceId: sender.teamId,
+ });
+ link = await store.redeemChallenge(code, {
+ provider: "slack",
+ realm,
+ subject: sender.userId,
+ });
+ } catch (error) {
+ if (
+ error instanceof IdentityLinkError ||
+ error instanceof IdentityInputError
+ )
+ return "invalid";
+ if (error instanceof IdentityConflictError) return "conflict";
+ throw error;
+ }
+ if (auditStore) {
+ await recordAuditEvent(auditStore, {
+ eventType: "identity.linked",
+ targetType: "identity_link",
+ targetId: link.id,
+ actorUserId: link.userId,
+ payload: { actor: link.userId, provider: "slack" },
+ });
+ }
+ return "linked";
+ },
+ };
+}
diff --git a/server/tests/identity-slack-redeem.test.ts b/server/tests/identity-slack-redeem.test.ts
new file mode 100644
index 000000000..b5e790ee5
--- /dev/null
+++ b/server/tests/identity-slack-redeem.test.ts
@@ -0,0 +1,119 @@
+import { describe, expect, test } from "bun:test";
+import type { AuditEventInput, AuditStore } from "../src/audit";
+import { slackCodeRedeemer } from "../src/identity/slack-redeem";
+import type { IdentityStore } from "../src/identity/store";
+import {
+ IdentityConflictError,
+ IdentityInputError,
+ type IdentityLink,
+ IdentityLinkError,
+} from "../src/identity/types";
+
+const sender = { teamId: "T1", userId: "U1" };
+
+function setup(behavior: () => Promise) {
+ const calls: unknown[][] = [];
+ const events: AuditEventInput[] = [];
+ const store = {
+ redeemChallenge: async (...args: unknown[]) => {
+ calls.push(args);
+ return behavior();
+ },
+ } as unknown as Pick;
+ const auditStore: AuditStore = {
+ insert: async (event) => {
+ events.push(event);
+ },
+ };
+ return { calls, events, store, auditStore };
+}
+
+const link = {
+ id: "link-1",
+ userId: "user-1",
+ provider: "slack",
+ realm: "opentag:opentag:T1",
+ subject: "U1",
+} as unknown as IdentityLink;
+
+describe("slackCodeRedeemer", () => {
+ test("links, audits without realm or subject", async () => {
+ const { calls, events, store, auditStore } = setup(async () => link);
+ const result = await slackCodeRedeemer(store, auditStore).redeem(
+ "CODE",
+ sender,
+ );
+ expect(result).toBe("linked");
+ expect(calls).toHaveLength(1);
+ expect(calls[0]).toHaveLength(2);
+ expect(calls[0]).toEqual([
+ "CODE",
+ { provider: "slack", realm: "opentag:opentag:T1", subject: "U1" },
+ ]);
+ expect(events).toHaveLength(1);
+ expect(events[0]).toEqual({
+ eventType: "identity.linked",
+ targetType: "identity_link",
+ targetId: "link-1",
+ actorUserId: "user-1",
+ payload: { actor: "user-1", provider: "slack" },
+ });
+ const json = JSON.stringify(events[0]);
+ expect(json).not.toContain("opentag:opentag");
+ expect(json).not.toContain("U1");
+ });
+
+ test("an invalid code is invalid and not audited", async () => {
+ const { events, store, auditStore } = setup(async () => {
+ throw new IdentityLinkError();
+ });
+ expect(await slackCodeRedeemer(store, auditStore).redeem("X", sender)).toBe(
+ "invalid",
+ );
+ expect(events).toHaveLength(0);
+ });
+
+ test("an input error is invalid", async () => {
+ const { store, auditStore } = setup(async () => {
+ throw new IdentityInputError();
+ });
+ expect(await slackCodeRedeemer(store, auditStore).redeem("X", sender)).toBe(
+ "invalid",
+ );
+ });
+
+ test("an incomplete sender is invalid", async () => {
+ const { calls, store, auditStore } = setup(async () => link);
+ expect(
+ await slackCodeRedeemer(store, auditStore).redeem("X", {
+ teamId: " ",
+ userId: "U1",
+ }),
+ ).toBe("invalid");
+ expect(calls).toHaveLength(0);
+ });
+
+ test("a conflict is reported and not audited", async () => {
+ const { events, store, auditStore } = setup(async () => {
+ throw new IdentityConflictError();
+ });
+ expect(await slackCodeRedeemer(store, auditStore).redeem("X", sender)).toBe(
+ "conflict",
+ );
+ expect(events).toHaveLength(0);
+ });
+
+ test("other errors rethrow", async () => {
+ const { store, auditStore } = setup(async () => {
+ throw new Error("db down");
+ });
+ await expect(
+ slackCodeRedeemer(store, auditStore).redeem("X", sender),
+ ).rejects.toThrow("db down");
+ });
+
+ test("works without an audit store", async () => {
+ const { store } = setup(async () => link);
+ expect(await slackCodeRedeemer(store).redeem("X", sender)).toBe("linked");
+ });
+});
From e0217089c262f50c2290e0f115dd66d793de639b Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:44:07 -0300
Subject: [PATCH 064/101] feat(app): show linked accounts whenever linking is
available
---
.../settings/linked-accounts.test.tsx | 26 +++++++++++++++++
.../components/settings/linked-accounts.tsx | 28 ++++++++++++++++---
2 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/app/src/components/settings/linked-accounts.test.tsx b/app/src/components/settings/linked-accounts.test.tsx
index f8b5c1ece..429cbfb53 100644
--- a/app/src/components/settings/linked-accounts.test.tsx
+++ b/app/src/components/settings/linked-accounts.test.tsx
@@ -120,6 +120,32 @@ test("the section hides while loading and when nothing is linked", () => {
expect(linkedAccountsSectionState({ data: [], error: null })).toBe(null);
});
+test("the section shows an empty list whenever linking is available", () => {
+ for (const providers of [
+ { slack: true, github: false },
+ { slack: false, github: true },
+ ]) {
+ for (const data of [undefined, []]) {
+ expect(
+ linkedAccountsSectionState({ data, error: null, providers }),
+ ).toEqual({ rows: [], error: null });
+ }
+ }
+});
+
+test("the section stays hidden when no provider is available", () => {
+ expect(
+ linkedAccountsSectionState({
+ data: [],
+ error: null,
+ providers: { slack: false, github: false },
+ }),
+ ).toBe(null);
+ expect(
+ linkedAccountsSectionState({ data: [], error: null, providers: undefined }),
+ ).toBe(null);
+});
+
test("rows show on their own when the list loaded", () => {
expect(linkedAccountsSectionState({ data: [row], error: null })).toEqual({
rows: [row],
diff --git a/app/src/components/settings/linked-accounts.tsx b/app/src/components/settings/linked-accounts.tsx
index df68a1947..5707359a1 100644
--- a/app/src/components/settings/linked-accounts.tsx
+++ b/app/src/components/settings/linked-accounts.tsx
@@ -1,6 +1,10 @@
import { IconBrandGithub, IconBrandSlack, IconLink } from "@tabler/icons-react";
import { useMutation, useQuery } from "@tanstack/react-query";
-import { PageRows, PageSection } from "@/components/layout/page-shell";
+import {
+ PageEmpty,
+ PageRows,
+ PageSection,
+} from "@/components/layout/page-shell";
import { Button } from "@/components/ui/button";
import {
Item,
@@ -13,6 +17,8 @@ import {
import { Separator } from "@/components/ui/separator";
import { unlinkMutationOptions } from "@/lib/identity/mutations";
import {
+ type IdentityProviders,
+ identityProvidersQueryOptions,
type LinkedAccount,
linkedAccountsQueryOptions,
} from "@/lib/identity/queries";
@@ -43,7 +49,8 @@ export function providerIcon(provider: string) {
/**
* What the section draws: nothing, rows, an error, or rows with an error under them.
*
- * Hidden while loading and when nothing is linked (or the deployment has no identity store). A
+ * Hidden while loading and when nothing is linked and nothing can be (no identity store, or no
+ * provider available to link); shown empty when a provider is available. A
* failed refetch keeps the rows already on screen — stale but visible — and says the list could not
* be refreshed, so a removed account that is still drawn is never shown without that warning. An
* error with no rows to keep, including over a cached empty list, shows on its own.
@@ -51,13 +58,18 @@ export function providerIcon(provider: string) {
export function linkedAccountsSectionState({
data,
error,
+ providers,
}: {
data: LinkedAccount[] | undefined;
error: Error | null;
+ providers?: IdentityProviders;
}): { rows: LinkedAccount[]; error: string | null } | null {
const rows = data ?? [];
if (error) return { rows, error: error.message };
- return rows.length > 0 ? { rows, error: null } : null;
+ if (rows.length > 0 || providers?.slack || providers?.github) {
+ return { rows, error: null };
+ }
+ return null;
}
/**
@@ -68,13 +80,21 @@ export function linkedAccountsSectionState({
*/
export function LinkedAccountsSection() {
const links = useQuery(linkedAccountsQueryOptions());
+ const providers = useQuery(identityProvidersQueryOptions());
const state = linkedAccountsSectionState({
data: links.data,
error: links.error,
+ providers: providers.data,
});
if (!state) return null;
return (
-
+ {/* actions */}}
+ >
+ {state.rows.length === 0 && !state.error ? (
+ No linked accounts yet.
+ ) : null}
{state.rows.length > 0 ? (
{state.rows.map((account, index) => (
From 791a4c28c9e72d4513e0d9844e2f4532c130c444 Mon Sep 17 00:00:00 2001
From: Guido Vizoso
Date: Thu, 8 Oct 2026 15:44:07 -0300
Subject: [PATCH 065/101] feat(app): report the linked-account result on
connected accounts
---
.../components/settings/linked-notice.test.ts | 25 +++++++++++++++
app/src/components/settings/linked-notice.ts | 18 +++++++++++
.../settings/connected-accounts/index.tsx | 32 +++++++++++++++++--
3 files changed, 72 insertions(+), 3 deletions(-)
create mode 100644 app/src/components/settings/linked-notice.test.ts
create mode 100644 app/src/components/settings/linked-notice.ts
diff --git a/app/src/components/settings/linked-notice.test.ts b/app/src/components/settings/linked-notice.test.ts
new file mode 100644
index 000000000..7bde1471d
--- /dev/null
+++ b/app/src/components/settings/linked-notice.test.ts
@@ -0,0 +1,25 @@
+import { describe, expect, test } from "bun:test";
+import { linkedNotice } from "./linked-notice";
+
+describe("linkedNotice", () => {
+ test("reports a linked GitHub account", () => {
+ expect(linkedNotice("github")).toEqual({
+ tone: "success",
+ text: "Your GitHub account is linked.",
+ });
+ });
+
+ test("reports a failed link", () => {
+ expect(linkedNotice("failed")).toEqual({
+ tone: "error",
+ text: "That account could not be linked. Nothing was saved — try again.",
+ });
+ });
+
+ test.each([undefined, "", "slack", "__proto__"])(
+ "says nothing for %p",
+ (value) => {
+ expect(linkedNotice(value)).toBeNull();
+ },
+ );
+});
diff --git a/app/src/components/settings/linked-notice.ts b/app/src/components/settings/linked-notice.ts
new file mode 100644
index 000000000..02472c581
--- /dev/null
+++ b/app/src/components/settings/linked-notice.ts
@@ -0,0 +1,18 @@
+export type LinkedNotice = { tone: "success" | "error"; text: string };
+
+/**
+ * What the connected accounts page says about the `?linked=` the GitHub callback redirected back
+ * with. Only the two values the callback sends have a sentence; anything else says nothing.
+ */
+export function linkedNotice(value: string | undefined): LinkedNotice | null {
+ if (value === "github") {
+ return { tone: "success", text: "Your GitHub account is linked." };
+ }
+ if (value === "failed") {
+ return {
+ tone: "error",
+ text: "That account could not be linked. Nothing was saved — try again.",
+ };
+ }
+ return null;
+}
diff --git a/app/src/routes/_authed/settings/connected-accounts/index.tsx b/app/src/routes/_authed/settings/connected-accounts/index.tsx
index f18a1327a..9466d59fb 100644
--- a/app/src/routes/_authed/settings/connected-accounts/index.tsx
+++ b/app/src/routes/_authed/settings/connected-accounts/index.tsx
@@ -16,6 +16,7 @@ import {
import { RowMark } from "@/components/layout/row-mark";
import { PluginLogo } from "@/components/plugins/plugin-logo";
import { LinkedAccountsSection } from "@/components/settings/linked-accounts";
+import { linkedNotice } from "@/components/settings/linked-notice";
import {
InputGroup,
InputGroupAddon,
@@ -52,8 +53,14 @@ export const Route = createFileRoute("/_authed/settings/connected-accounts/")({
* The key is omitted rather than set to undefined. Present-but-undefined makes `search` a required
* prop on every Link to this route, which is a lot of ripple for a parameter only the callback sets.
*/
- validateSearch: (search: Record): { connected?: string } =>
- typeof search.connected === "string" ? { connected: search.connected } : {},
+ validateSearch: (
+ search: Record,
+ ): { connected?: string; linked?: string } => ({
+ ...(typeof search.connected === "string"
+ ? { connected: search.connected }
+ : {}),
+ ...(typeof search.linked === "string" ? { linked: search.linked } : {}),
+ }),
});
/** The same marks the admin connector list uses: these are the same vendors seen from your side. */
@@ -81,7 +88,15 @@ export function brokeredAccountsListedOn(
}
function RouteComponent() {
- const { connected: outcome } = Route.useSearch();
+ const { connected: outcome, linked } = Route.useSearch();
+ /* Read once, then dropped from the URL, so a refresh does not repeat the news. */
+ const [notice] = React.useState(() => linkedNotice(linked));
+ const navigate = Route.useNavigate();
+ React.useEffect(() => {
+ if (linked !== undefined) {
+ navigate({ search: ({ linked: _l, ...rest }) => rest, replace: true });
+ }
+ }, [linked, navigate]);
const [search, setSearch] = React.useState("");
const plugins = useQuery(pluginsPageQueryOptions());
const connections = useQuery(connectionsQueryOptions());
@@ -174,6 +189,17 @@ function RouteComponent() {
That account could not be connected. Nothing was saved — try again.