Skip to content

Commit abce643

Browse files
ralyodioclaude
andcommitted
fix(pwa): unbreak sign-out, and stop echoing $PUBLIC_ORIGIN in the CLI hint
Two bugs on the dashboard, both fixed by handing appBar/CLI_HINT the request. Sign-out was broken for everyone. csrfGuard rejects any POST whose _csrf does not match the mc_csrf cookie, and /auth/logout is a POST that is not on the exempt list, but the sign-out form carried no hidden field -- every click answered 403 "bad csrf token". appBar now takes the request rather than the user, because it needs the token as well as the identity. The field is written out instead of reusing csrfInput(): html.mjs is the view layer and imports nothing, and pulling in session.mjs would drag the database driver with it. The "Connect the CLI" snippet still printed $PUBLIC_ORIGIN, so users on app.logicsrc.com were told to point LOGICSRC_API at the generated Railway hostname. #105 added requestOrigin() for exactly this and fixed the device-flow URLs; the dashboard hint was missed. It now follows the request too, which is not a hardcode swap -- the same deployment answering on its Railway hostname still self-describes correctly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3649f78 commit abce643

4 files changed

Lines changed: 56 additions & 8 deletions

File tree

apps/pwa/src/lib/html.mjs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,22 @@ ${head}
101101
</html>`;
102102
}
103103

104-
export function appBar(user) {
104+
// Takes the request, not just the user: signing out is a POST, and csrfGuard
105+
// rejects any POST whose _csrf does not match the cookie. Without the hidden
106+
// field here every "Sign out" click answered "bad csrf token".
107+
//
108+
// The field is written out rather than imported from session.mjs on purpose --
109+
// this module is the view layer and has no imports, and pulling in session.mjs
110+
// would drag the database driver along with it.
111+
export function appBar(req) {
112+
const user = req?.user;
105113
return `<header class="bar"><div class="wrap bar-inner">
106114
<a class="brand" href="/"><span class="mark">LS</span>LogicSRC<span class="app">credentials</span></a>
107115
<div class="bar-right">
108116
${user
109117
? `<span class="mono faint" style="font-size:.78rem">${esc(user.email || user.display_name || "signed in")}</span>
110118
<a class="btn" href="/settings">Settings</a>
111-
<form method="post" action="/auth/logout" style="margin:0"><button class="btn">Sign out</button></form>`
119+
<form method="post" action="/auth/logout" style="margin:0"><input type="hidden" name="_csrf" value="${esc(req?.csrfToken)}"><button class="btn">Sign out</button></form>`
112120
: `<a class="btn acid" href="/">Sign in</a>`}
113121
</div>
114122
</div></header>`;

apps/pwa/src/routes/cli.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ cliRouter.get("/cli/authorize", requireAuth, (req, res) => {
3636
return res.status(400).type("html").send(page({ body: `<main class="wrap" style="padding-top:12vh"><h1>Bad CLI request</h1><p class="dim mono">missing/invalid redirect_uri, state, or code_challenge.</p></main>` }));
3737
}
3838
const name = String(req.query.name || "logicsrc cli").slice(0, 40);
39-
const body = `${appBar(req.user)}
39+
const body = `${appBar(req)}
4040
<main class="wrap" style="max-width:460px;padding-top:8vh">
4141
<div class="card"><div class="card-body" style="text-align:center">
4242
<div style="font-size:2rem">🔑</div>
@@ -142,7 +142,7 @@ cliRouter.post("/cli/device/code", async (req, res) => {
142142
});
143143

144144
const devicePage = (req, body) =>
145-
page({ title: "LogicSRC ▸ authorize CLI", body: `${appBar(req.user)}<main class="wrap" style="max-width:460px;padding-top:8vh">${body}</main>${footer}` });
145+
page({ title: "LogicSRC ▸ authorize CLI", body: `${appBar(req)}<main class="wrap" style="max-width:460px;padding-top:8vh">${body}</main>${footer}` });
146146

147147
const deviceResult = (req, res, status, heading, detail) =>
148148
res.status(status).type("html").send(devicePage(req, `<div class="card"><div class="card-body" style="text-align:center">

apps/pwa/src/routes/pages.mjs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { id, token, sha256 } from "../lib/crypto.mjs";
77
import { page, footer, appBar, esc } from "../lib/html.mjs";
88
import { requireAuth, csrfInput } from "../lib/session.mjs";
99
import { createApiKey, listApiKeys, revokeApiKey } from "../lib/apikey.mjs";
10+
import { requestOrigin } from "../lib/origin.mjs";
1011
import { config } from "../config.mjs";
1112

1213
export const pagesRouter = Router();
@@ -60,10 +61,10 @@ export async function dashboardHandler(req, res) {
6061
for (const t of teams) cards += await teamCard(t, uid);
6162
cards = cards.split(CSRF).join(csrfInput(req));
6263

63-
const body = `${appBar(req.user)}
64+
const body = `${appBar(req)}
6465
<main class="wrap" style="max-width:820px;padding:26px 0 40px">
6566
<div class="section-title"><h1 style="font-size:1.6rem">Your teams</h1><span class="count">${teams.length}</span></div>
66-
${CLI_HINT(config.origin)}
67+
${CLI_HINT(requestOrigin(req, config.origin))}
6768
${cards || `<div class="card"><div class="card-body dim">You're not on any teams yet. Create one below or accept an invite.</div></div>`}
6869
<div class="card" style="margin-top:22px"><div class="card-head"><span class="h">New team</span></div>
6970
<div class="card-body"><form method="post" action="/teams" style="display:flex;gap:8px">${csrfInput(req)}
@@ -109,7 +110,7 @@ pagesRouter.get("/teams/accept", requireAuth, (req, res) => {
109110
const tok = String(req.query.token || "");
110111
const shared = req.query.shared;
111112
const err = req.query.err;
112-
const body = `${appBar(req.user)}
113+
const body = `${appBar(req)}
113114
<main class="wrap" style="max-width:460px;padding-top:8vh">
114115
<div class="card"><div class="card-body" style="text-align:center">
115116
<h1 style="font-size:1.4rem;margin-bottom:12px">Accept team invite</h1>
@@ -143,7 +144,7 @@ pagesRouter.get("/settings", requireAuth, async (req, res) => {
143144
<span style="flex:1">${esc(k.name)} <span class="faint">${esc(k.prefix)}…</span></span>
144145
<form method="post" action="/settings/apikeys/${k.id}/delete" style="margin:0">${csrfInput(req)}<button class="btn danger" style="padding:5px 10px;font-size:.72rem">revoke</button></form>
145146
</div>`).join("") : `<div class="faint mono" style="font-size:.78rem;padding:6px 0">no keys yet</div>`;
146-
const body = `${appBar(req.user)}
147+
const body = `${appBar(req)}
147148
<main class="wrap" style="max-width:640px;padding-top:30px">
148149
<h1 style="font-size:1.5rem;margin-bottom:20px">Settings</h1>
149150
${newKey ? `<div class="notice ok">New API key (copy it now — shown once):<br><b class="mono" style="word-break:break-all">${esc(newKey)}</b></div>` : ""}

apps/pwa/test/appbar.test.mjs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Signing out is a POST and csrfGuard rejects any POST whose _csrf does not
2+
// match the mc_csrf cookie. The Sign out form shipped without that field, so
3+
// every click answered "bad csrf token" and nobody could log out. These pin the
4+
// hidden input in place.
5+
import assert from "node:assert/strict";
6+
import test from "node:test";
7+
8+
import { appBar } from "../src/lib/html.mjs";
9+
10+
const req = (extra = {}) => ({ csrfToken: "deadbeefdeadbeef", user: { email: "a@example.com" }, ...extra });
11+
12+
test("the sign-out form carries the CSRF token", () => {
13+
const html = appBar(req());
14+
assert.match(html, /action="\/auth\/logout"/);
15+
assert.match(html, /<input type="hidden" name="_csrf" value="deadbeefdeadbeef">/);
16+
// the field has to be inside the form, not merely somewhere on the page
17+
const form = html.slice(html.indexOf('action="/auth/logout"'));
18+
assert.ok(
19+
form.indexOf('name="_csrf"') < form.indexOf("</form>"),
20+
"the _csrf input must be inside the sign-out form",
21+
);
22+
});
23+
24+
test("signed-out visitors get no sign-out form at all", () => {
25+
const html = appBar({ user: null, csrfToken: "x" });
26+
assert.doesNotMatch(html, /\/auth\/logout/);
27+
assert.match(html, /Sign in/);
28+
});
29+
30+
test("survives a request with no CSRF token rather than printing undefined", () => {
31+
const html = appBar(req({ csrfToken: undefined }));
32+
assert.match(html, /name="_csrf" value=""/);
33+
});
34+
35+
test("the signed-in identity is escaped", () => {
36+
const html = appBar(req({ user: { email: '<script>alert(1)</script>' } }));
37+
assert.doesNotMatch(html, /<script>alert/);
38+
assert.match(html, /&lt;script&gt;/);
39+
});

0 commit comments

Comments
 (0)