Keep only the bot cookie for the last visited bot
CI / check (push) Canceled after 0s

A workspace conversation is never remembered, so /admin/bots always
reopens the bot's own chat.
This commit is contained in:
2026-10-22 13:39:36 +01:00
parent 133e6b87c9
commit 63a93a4262
13 changed files with 48 additions and 147 deletions
-3
View File
@@ -13,9 +13,6 @@ describe("AppData auth constants", () => {
expect(AppData["LastVisitedBotCookieName"]).toBe(
`${SiteData["SiteSlug"]}_last_visited_bot_id`,
);
expect(AppData["LastVisitedWorkspaceCookieName"]).toBe(
`${SiteData["SiteSlug"]}_last_visited_workspace_id`,
);
});
test("expiry and limits are positive", () => {
+3 -5
View File
@@ -55,14 +55,12 @@ export const AppData = {
BuildCookieName: `${SiteData["SiteSlug"]}_build`,
BuildCookieMaxAgeSeconds: 365 * 24 * 60 * 60,
/**
* The conversation this browser was last in: `/admin/bots` opens it
* The bot this browser was last in: `/admin/bots` opens its own chat
* again, falling back to the coordinator (super admin) or the latest
* bot when the grant is gone. Two small ids, so the workspace one
* stays empty for a bot's own chat. `HttpOnly`: only the server reads
* them.
* bot when the grant is gone. One small id. `HttpOnly`: only the server
* reads it.
*/
LastVisitedBotCookieName: `${SiteData["SiteSlug"]}_last_visited_bot_id`,
// LastVisitedWorkspaceCookieName: `${SiteData["SiteSlug"]}_last_visited_workspace_id`,
LastVisitedCookieMaxAgeSeconds: 365 * 24 * 60 * 60,
AuthExpiryDays: 60,
APICrudBasePath: "/api/admin/crud",
@@ -3,38 +3,30 @@ import grabUserConversation from "./grab-user-conversation";
type Params = {
user_id: number | string;
bot_id: number | string;
/**
* Null or undefined: the bot's own chat.
*/
workspace_id?: number | string | null;
};
export type ReachableConversation = {
bot_id: number;
workspace_id: number | null;
};
/**
* A conversation from the last-visited cookie, only while the user still
* The bot's own chat from the last-visited cookie, only while the user still
* reaches it: `grabUserConversation` is the same access check the
* conversation pages use, so a revoked grant, a deleted bot or an archived
* workspace answers null and the landing page falls back instead of
* redirecting into a page that would bounce it back.
* conversation pages use, so a revoked grant or a deleted bot answers null
* and the landing page falls back instead of redirecting into a page that
* would bounce it back.
*/
export default async function grabReachableConversation({
user_id,
bot_id,
workspace_id,
}: Params): Promise<ReachableConversation | null> {
const resolved_workspace_id = workspace_id ? Number(workspace_id) : null;
const conversation = await grabUserConversation({
user_id,
bot_id,
workspace_id: resolved_workspace_id,
workspace_id: null,
}).catch(() => null);
if (!conversation?.bot.id) return null;
return { bot_id: conversation.bot.id, workspace_id: conversation.workspace_id };
return { bot_id: conversation.bot.id };
}
@@ -6,30 +6,20 @@ type Params = {
req: Request;
res: Response;
bot_id: number | string;
/**
* Null or undefined: the bot's own chat.
*/
workspace_id?: number | string | null;
};
/**
* The page's response with the last-visited cookies set, so `/admin/bots`
* opens this conversation again next time. Called from the conversation
* The page's response with the last-visited cookie set, so `/admin/bots`
* opens this bot's own chat again next time. Called from the conversation
* pages' `res_transform`, which is why it takes and returns the response.
*/
export default function withLastVisitedBotCookies({
req,
res,
bot_id,
workspace_id,
}: Params): Response {
export default function withLastVisitedBotCookies({ req, res, bot_id }: Params): Response {
const new_res = res.clone();
setCookies(
new_res,
lastVisitedBotCookies({
bot_id,
workspace_id,
secure: isSecureRequest({ req }),
}),
);
+4 -7
View File
@@ -12,8 +12,8 @@ import railWorkspaces from "@/src/utils/rail-workspaces";
import readLastVisitedBot from "@/src/utils/read-last-visited-bot";
/**
* Opens the conversation this browser was last in, from the last-visited
* cookies the conversation pages set, while the user still reaches it.
* Opens the bot's own chat this browser was last in, from the last-visited
* cookie the conversation pages set, while the user still reaches it.
* Otherwise the super admin lands on the coordinator, which (with the main
* computer) is created first if the install has none; anyone else lands on
* the latest bot they chat with, else their first workspace; with neither,
@@ -62,7 +62,7 @@ export const server: BunextPageServerFn<PagePropsType> = async ({ req }) => {
};
/**
* The path of the conversation this browser was last in, if the user still
* The path of the bot's own chat this browser was last in, if the user still
* reaches it (`grabReachableConversation`, the conversation pages' own
* access check). Null sends the page on to its usual fallback.
*/
@@ -74,12 +74,9 @@ async function grabLastVisited({ user_id, req }: { user_id: number | string; req
const conversation = await grabReachableConversation({
user_id,
bot_id: visited.bot_id,
workspace_id: visited.workspace_id,
});
if (!conversation) return null;
return conversation.workspace_id
? `/admin/workspaces/${conversation.workspace_id}/bots/${conversation.bot_id}`
: `/admin/bots/${conversation.bot_id}`;
return `/admin/bots/${conversation.bot_id}`;
}
@@ -9,8 +9,8 @@ import type { BunextPageServerFn } from "@moduletrace/bunext/types";
* A bot's conversation inside a workspace: the bot works with the
* workspace's context, computers and models. Only the coordinator and the
* workspace's bots have one; anyone else goes back to the workspace
* settings (managers) or their bots. Rendering it records the conversation
* in the last-visited cookies, so `/admin/bots` opens it again.
* settings (managers) or their bots. Rendering it records the bot in the
* last-visited cookie, so `/admin/bots` opens its own chat again.
*/
export const server: BunextPageServerFn<PagePropsType> = async (params) => {
const query: PageQueryObject = params.query;
@@ -37,12 +37,11 @@ export const server: BunextPageServerFn<PagePropsType> = async (params) => {
}
const bot_id = props.bot.id;
const workspace_id = props.workspace?.id ?? query.workspace_id;
return {
props,
res_transform(res) {
return withLastVisitedBotCookies({ req: params.req, res, bot_id, workspace_id });
return withLastVisitedBotCookies({ req: params.req, res, bot_id });
},
};
};
-4
View File
@@ -36,10 +36,6 @@ export const server: BunextPageServerFn = async ({ req }) => {
name: AppData["LastVisitedBotCookieName"],
httpOnly: true,
},
{
name: AppData["LastVisitedWorkspaceCookieName"],
httpOnly: true,
},
]);
return new_res;
},
+7 -27
View File
@@ -5,14 +5,10 @@ import { setCookies } from "./cookies";
import readLastVisitedBot from "./read-last-visited-bot";
const bot_cookie = AppData["LastVisitedBotCookieName"];
const workspace_cookie = AppData["LastVisitedWorkspaceCookieName"];
function requestWithCookies(): Request {
const res = new Response();
setCookies(
res,
lastVisitedBotCookies({ bot_id: 4, workspace_id: 9, secure: false }),
);
setCookies(res, lastVisitedBotCookies({ bot_id: 4, secure: false }));
const cookie_header = res.headers
.getSetCookie()
.map((header) => header.split(";")[0])
@@ -24,39 +20,23 @@ function requestWithCookies(): Request {
}
describe("last visited bot cookies", () => {
test("round-trip a workspace conversation", () => {
expect(readLastVisitedBot({ req: requestWithCookies() })).toEqual({
bot_id: 4,
workspace_id: 9,
});
test("round-trip a bot", () => {
expect(readLastVisitedBot({ req: requestWithCookies() })).toEqual({ bot_id: 4 });
});
test("a bot's own chat leaves the workspace cookie empty", () => {
test("only the bot cookie is written, never a workspace one", () => {
const res = new Response();
setCookies(res, lastVisitedBotCookies({ bot_id: 4, secure: false }));
const cookie_header = res.headers
.getSetCookie()
.map((header) => header.split(";")[0])
.join("; ");
const [header, ...others] = res.headers.getSetCookie();
const req = new Request("http://localhost", {
headers: { Cookie: cookie_header },
});
expect(readLastVisitedBot({ req })).toEqual({ bot_id: 4, workspace_id: null });
expect(others).toHaveLength(0);
expect(header?.startsWith(`${bot_cookie}=4`)).toBe(true);
});
test("no cookies at all", () => {
expect(readLastVisitedBot({ req: new Request("http://localhost") })).toBeNull();
});
test("only the workspace cookie names no conversation", () => {
const req = new Request("http://localhost", {
headers: { Cookie: `${workspace_cookie}=9` },
});
expect(readLastVisitedBot({ req })).toBeNull();
});
test("a tampered bot id is ignored", () => {
const req = new Request("http://localhost", {
headers: { Cookie: `${bot_cookie}=1%20OR%201=1` },
+7 -20
View File
@@ -4,26 +4,19 @@ import type { CookieOptions } from "./cookies";
type Params = {
bot_id: number | string;
/**
* Null or undefined: the bot's own chat.
*/
workspace_id?: number | string | null;
/**
* From `isSecureRequest`, so plain HTTP over a VPN still keeps them.
* From `isSecureRequest`, so plain HTTP over a VPN still keeps it.
*/
secure: boolean;
};
/**
* The cookies that remember the conversation this browser is in, for
* `setCookies`: read back by `readLastVisitedBot` on the bots' landing page.
* `HttpOnly`, since only the server reads them, and they hold nothing but
* two ids.
* The cookie that remembers the bot this browser is in, for `setCookies`:
* read back by `readLastVisitedBot` on the bots' landing page. `HttpOnly`,
* since only the server reads it, and it holds nothing but an id. A
* conversation inside a workspace is never remembered, so the landing page
* always reopens the bot's own chat.
*/
export default function lastVisitedBotCookies({
bot_id,
workspace_id,
secure,
}: Params): CookieOptions[] {
export default function lastVisitedBotCookies({ bot_id, secure }: Params): CookieOptions[] {
return [
{
name: AppData["LastVisitedBotCookieName"],
@@ -31,11 +24,5 @@ export default function lastVisitedBotCookies({
secure,
maxAge: AppData["LastVisitedCookieMaxAgeSeconds"],
},
// {
// name: AppData["LastVisitedWorkspaceCookieName"],
// value: workspace_id ? String(workspace_id) : "",
// secure,
// maxAge: AppData["LastVisitedCookieMaxAgeSeconds"],
// },
];
}
+5 -26
View File
@@ -2,27 +2,14 @@ import { describe, expect, test } from "bun:test";
import parseLastVisitedBot from "./parse-last-visited-bot";
describe("parseLastVisitedBot", () => {
test("a bot's own chat", () => {
expect(parseLastVisitedBot({ bot_id: "3" })).toEqual({ bot_id: 3, workspace_id: null });
expect(parseLastVisitedBot({ bot_id: 3, workspace_id: "" })).toEqual({
bot_id: 3,
workspace_id: null,
});
expect(parseLastVisitedBot({ bot_id: 3, workspace_id: null })).toEqual({
bot_id: 3,
workspace_id: null,
});
});
test("a workspace conversation", () => {
expect(parseLastVisitedBot({ bot_id: "3", workspace_id: "7" })).toEqual({
bot_id: 3,
workspace_id: 7,
});
test("a bot id becomes the bot", () => {
expect(parseLastVisitedBot({ bot_id: "3" })).toEqual({ bot_id: 3 });
expect(parseLastVisitedBot({ bot_id: 3 })).toEqual({ bot_id: 3 });
});
test("rejects ids that aren't positive whole numbers", () => {
expect(parseLastVisitedBot({})).toBeNull();
expect(parseLastVisitedBot({ bot_id: null })).toBeNull();
expect(parseLastVisitedBot({ bot_id: "" })).toBeNull();
expect(parseLastVisitedBot({ bot_id: "0" })).toBeNull();
expect(parseLastVisitedBot({ bot_id: "-1" })).toBeNull();
@@ -32,15 +19,7 @@ describe("parseLastVisitedBot", () => {
expect(parseLastVisitedBot({ bot_id: "99999999999999999999" })).toBeNull();
});
test("a bad workspace falls back to nothing, never to its own chat", () => {
expect(parseLastVisitedBot({ bot_id: "3", workspace_id: "abc" })).toBeNull();
expect(parseLastVisitedBot({ bot_id: "3", workspace_id: "0" })).toBeNull();
});
test("trims what a cookie header may carry", () => {
expect(parseLastVisitedBot({ bot_id: " 3 ", workspace_id: " 7 " })).toEqual({
bot_id: 3,
workspace_id: 7,
});
expect(parseLastVisitedBot({ bot_id: " 3 " })).toEqual({ bot_id: 3 });
});
});
+6 -19
View File
@@ -1,36 +1,23 @@
export type LastVisitedBot = {
bot_id: number;
/**
* Null: the bot's own chat.
*/
workspace_id: number | null;
};
type Params = {
bot_id?: string | number | null;
workspace_id?: string | number | null;
};
/**
* The conversation the last-visited cookies name, or null when they don't
* name one. Ids are checked to be positive whole numbers, so a hand-edited
* cookie never reaches a query; whether the user still reaches that
* conversation is the caller's check.
* The bot the last-visited cookie names, or null when it doesn't name one.
* The id is checked to be a positive whole number, so a hand-edited cookie
* never reaches a query; whether the user still reaches that bot's own chat
* is the caller's check.
*/
export default function parseLastVisitedBot({ bot_id, workspace_id }: Params): LastVisitedBot | null {
export default function parseLastVisitedBot({ bot_id }: Params): LastVisitedBot | null {
const bot = parseId(bot_id);
if (bot == null) return null;
if (workspace_id === undefined || workspace_id === null || workspace_id === "") {
return { bot_id: bot, workspace_id: null };
}
const workspace = parseId(workspace_id);
if (workspace == null) return null;
return { bot_id: bot, workspace_id: workspace };
return { bot_id: bot };
}
function parseId(value?: string | number | null): number | null {
+3 -4
View File
@@ -7,14 +7,13 @@ type Params = {
};
/**
* The conversation this browser was last in, from the last-visited cookies
* (see `lastVisitedBotCookies`). Null when it has none, or when they don't
* name a conversation. Whether the user still reaches it is the caller's
* The bot this browser was last in, from the last-visited cookie (see
* `lastVisitedBotCookies`). Null when it has none, or when it doesn't name
* a bot. Whether the user still reaches that bot's own chat is the caller's
* check: a stale grant must fall back, never redirect.
*/
export default function readLastVisitedBot({ req }: Params): LastVisitedBot | null {
return parseLastVisitedBot({
bot_id: getCookie(req, AppData["LastVisitedBotCookieName"]),
workspace_id: getCookie(req, AppData["LastVisitedWorkspaceCookieName"]),
});
}