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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ and is safe to rerun. It kills a port holder only once that process has identifi
OpenBot, so an unrelated process on 3010 is named and left alone rather than killed. Nothing is
deleted: the database, the Bots' files and their browser profiles are volumes. `--keep-computers`
leaves the browsers signed in.
### The trail says when an identity provider was added, not only when one was taken away

Whoever holds an identity provider decides who can sign in at all, and the audit trail recorded only
half of that. Removing one through the administration screen was written down; registering one was
not, because registration is the sign-in library's own endpoint and nothing this deployment owns ran
on the way through. The event type for it had been declared and never written. Removing a provider
through the library's endpoint rather than the screen was unrecorded for the same reason. Both are
now written where the deployment already stands in front of those routes to check that the person
asking is an administrator, so a provider appearing or disappearing names itself and whoever did it.

### Two workers on one machine can no longer fire the same routine twice

Every process that claims work from the shared queue named itself after its hostname, and the queue
Expand Down
44 changes: 43 additions & 1 deletion server/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { BotAccessCheck } from "./agents/profile-policy";
import type { AgentProfileStore } from "./agents/profile-store";
import { createAgentRoutes } from "./agents/routes";
import {
type AuditEventType,
type AuditReader,
type AuditStore,
AuditQueryError,
Expand Down Expand Up @@ -276,6 +277,11 @@ export function createApp(
"/api/auth/sso/delete-provider",
]);

const AUTH_ROUTE_EVENTS: Record<string, AuditEventType | undefined> = {
"/api/auth/sso/register": "identity_provider.registered",
"/api/auth/sso/delete-provider": "identity_provider.removed",
};

app.on(["GET", "POST"], "/api/auth/*", async (context) => {
if (!auth) {
return context.json(
Expand All @@ -301,7 +307,43 @@ export function createApp(
}
}

return auth.handler(context.req.raw);
const eventType =
AUTH_ROUTE_EVENTS[new URL(context.req.url).pathname] ?? undefined;

// Read before the handler runs, because it consumes the stream: a clone taken afterwards is of
// a request whose body is already gone, and the row would name no provider.
const named =
auditStore && eventType
? ((await context.req.raw
.clone()
.json()
.catch(() => null)) as { providerId?: unknown } | null)
: null;

const answer = await auth.handler(context.req.raw);

if (auditStore && eventType && answer.ok) {
const session = await auth.api.getSession({
headers: context.req.raw.headers,
query: { disableCookieCache: true },
});
await recordAuditEvent(auditStore, {
eventType,
targetType: "identity_provider",
...(typeof named?.providerId === "string"
? { targetId: named.providerId }
: {}),
...(session?.user ? { actorUserId: session.user.id } : {}),
payload: {
...(typeof named?.providerId === "string"
? { providerId: named.providerId }
: {}),
...(session?.user?.email ? { by: session.user.email } : {}),
},
});
}

return answer;
});

const authenticationUnavailable: MiddlewareHandler<{
Expand Down
115 changes: 115 additions & 0 deletions server/tests/identity-provider-audit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import { describe, expect, test } from "bun:test";
import { createApp } from "../src/app";
import type { AuditEventInput, AuditStore } from "../src/audit";
import { loadConfig } from "../src/config";
import { testEnvironment } from "./support/environment";

const ADMIN = { id: "admin", email: "admin@openbot.test" };

function app(role = "admin") {
const rows: AuditEventInput[] = [];
const auditStore: AuditStore = {
insert: async (event) => void rows.push(event),
};
const hono = createApp(
loadConfig(testEnvironment()),
{
/*
* Reads the body, as the real sign-in library does. A handler that leaves the stream
* untouched hides the ordering this depends on: a clone taken after the handler has run is of
* a consumed request, and the row would name no provider.
*/
handler: async (request: Request) => {
await request.json().catch(() => null);
return new Response(JSON.stringify({ ok: true }), { status: 200 });
},
api: { getSession: async () => ({ user: ADMIN }) },
} as never,
{ rolesForUser: async () => [role] },
// Positions 4-12 are the other stores; auditStore is 13.
...(Array.from({ length: 9 }) as never[]),
auditStore as never,
);
return { rows, hono };
}

const REGISTER_BODY = {
providerId: "acme",
issuer: "https://login.acme.test",
domain: "acme.test",
};

describe("the trail says how an identity provider came to exist", () => {
test("records the registration of an identity provider", async () => {
const { rows, hono } = app();
const response = await hono.request("/api/auth/sso/register", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify(REGISTER_BODY),
});
expect(response.status).toBe(200);
expect(rows.map((row) => row.eventType)).toContain(
"identity_provider.registered",
);
});

test("names who registered it and which provider", async () => {
const { rows, hono } = app();
await hono.request("/api/auth/sso/register", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify(REGISTER_BODY),
});
const row = rows.find(
(candidate) => candidate.eventType === "identity_provider.registered",
);
expect(row?.targetId).toBe("acme");
expect(row?.actorUserId).toBe("admin");
});

test("records a removal made through the library's own endpoint", async () => {
const { rows, hono } = app();
await hono.request("/api/auth/sso/delete-provider", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify({ providerId: "acme" }),
});
expect(rows.map((row) => row.eventType)).toContain(
"identity_provider.removed",
);
});

test("writes nothing when the library refuses the change", async () => {
const rows: AuditEventInput[] = [];
const hono = createApp(
loadConfig(testEnvironment()),
{
handler: async (request: Request) => {
await request.json().catch(() => null);
return new Response("no", { status: 400 });
},
api: { getSession: async () => ({ user: ADMIN }) },
} as never,
{ rolesForUser: async () => ["admin"] },
...(Array.from({ length: 9 }) as never[]),
{ insert: async (event) => void rows.push(event) } as never,
);
await hono.request("/api/auth/sso/register", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify(REGISTER_BODY),
});
expect(rows).toEqual([]);
});

test("writes nothing when a non-administrator is refused", async () => {
const { rows, hono } = app("user");
const response = await hono.request("/api/auth/sso/register", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify(REGISTER_BODY),
});
expect(response.status).toBe(403);
expect(rows).toEqual([]);
});
});