From a2b3f91e00f49cdb5159becf1f13f8923e422d15 Mon Sep 17 00:00:00 2001 From: marc Date: Fri, 21 Aug 2026 18:30:58 +0200 Subject: [PATCH] The face from the provider, and whose business a password is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The password tab was hidden for accounts with no password, which is not the same as accounts that sign in through the provider — an account linked to one kept its password and therefore kept the tab. That is precisely the account the person setting this up is using. It goes by "signs in over there" now; the password it kept is the emergency door, set by an administrator. And the picture. The provider sends one, the server fetches it at each sign-in and stores it encrypted in a table beside the users rather than in them, and serves it back through the API — so the page never talks to the provider, no third-party cookie is needed for a face to appear, and it survives the provider being down like everything else here. Fetched once per account into an object URL, since an img tag cannot carry a bearer token. The suite's provider now serves a picture too, and the check asks the browser whether it actually decoded rather than whether the element exists. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 24 +++++++++++ backend/oidc.js | 33 +++++++++++++++ backend/server.js | 74 +++++++++++++++++++++++++++++++++ e2e/fakeIdp.mjs | 18 +++++++- e2e/oidc.mjs | 35 ++++++++++++++++ frontend/src/App.jsx | 1 + frontend/src/syncEngine.ts | 33 +++++++++++++++ frontend/src/ui/AccountMenu.jsx | 37 ++++++++++------- frontend/src/ui/AdminPanel.jsx | 5 +++ frontend/src/ui/Avatar.jsx | 50 ++++++++++++++++++++++ frontend/src/ui/Sidebar.jsx | 7 +++- readme.md | 30 ++++++++++--- 12 files changed, 325 insertions(+), 22 deletions(-) create mode 100644 frontend/src/ui/Avatar.jsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 710275e..9deed3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,30 @@ a rebuild is not bit-identical. --- +## 2026-08-21 — The face from the provider, and whose business a password is + +**Images** — web `sha256:e525b11d…`, server `sha256:95d56c46…` + +The server image changed: a table for profile pictures and the route that serves +them. + +- **The profile picture comes along.** When the provider sends one — PocketID + does — the server fetches it at each sign-in and keeps it, encrypted, in a + table of its own rather than in `users`, where a blob would ride along with + every `SELECT *` for the sake of one corner of the screen. It is served back + through the API: nothing on the page talks to the provider, no third-party + cookie has to survive for a face to appear, and the picture keeps working when + the provider does not. It shows in the sidebar, in the account panel and beside + the name in the administration list, and arrives with the **next** sign-in — + the one moment this app speaks to the provider at all. +- **An account that signs in through the provider has no password tab**, and that + now includes one that had a password before it was linked — the usual case for + whoever set this up, and the reason the tab was still there. The password it + kept is the emergency door rather than a second thing for its owner to + maintain: it is set by an administrator, and the account panel says so. + +--- + ## 2026-08-18 — Which accounts belong to the provider **Images** — web `sha256:5fb51ed4…`, server `sha256:ce1f4649…` diff --git a/backend/oidc.js b/backend/oidc.js index b254716..3ca2f80 100644 --- a/backend/oidc.js +++ b/backend/oidc.js @@ -296,9 +296,41 @@ async function completeLogin({ code, state }) { username: claims.preferred_username || null, email: claims.email || null, displayName: claims.name || claims.given_name || null, + picture: typeof claims.picture === "string" ? claims.picture : null, }; } +/** + * The profile picture, fetched by the server rather than by the browser. + * + * Three reasons it happens here. The address is often behind the provider's own + * session, which this app's visitors may not have in a third-party context; the + * browser would have to talk to the provider on every page, which is a request + * to somewhere else on every load; and an image that lives here keeps working + * when the provider does not, like everything else in this app. + * + * Best effort in the strict sense: a slow, missing, oversized or non-image + * answer gives null and the sign-in carries on. A picture is never a reason to + * fail to let somebody in. + */ +async function fetchPicture(url, { maxBytes = 512 * 1024 } = {}) { + if (!url || !/^https?:\/\//i.test(url)) return null; + try { + const res = await fetch(url, { signal: AbortSignal.timeout(4000), redirect: "follow" }); + if (!res.ok) return null; + const type = String(res.headers.get("content-type") || "").split(";")[0].trim().toLowerCase(); + if (!/^image\/(png|jpeg|webp|gif)$/.test(type)) return null; + const declared = Number(res.headers.get("content-length")); + if (Number.isFinite(declared) && declared > maxBytes) return null; + const bytes = Buffer.from(await res.arrayBuffer()); + // The header may have lied or been absent; the bytes are the truth. + if (!bytes.length || bytes.length > maxBytes) return null; + return { bytes, type }; + } catch { + return null; + } +} + /* ─── The ticket the browser carries home ─── */ function issueTicket(payload) { @@ -325,6 +357,7 @@ module.exports = { describeConfiguration, beginLogin, completeLogin, + fetchPicture, issueTicket, redeemTicket, }; diff --git a/backend/server.js b/backend/server.js index bb2bdf9..a894a9e 100644 --- a/backend/server.js +++ b/backend/server.js @@ -287,6 +287,10 @@ const publicUser = (row) => ({ * second decides whether there is a password to offer changing. */ linkedToProvider: !!row.oidc_sub, signsInWithProvider: isProviderOnly(row), + /* Whether there is a picture to ask for. The bytes are fetched separately — + * this only decides whether anything asks at all, so a row without one draws + * its initial instead of firing a request that answers 404. */ + hasAvatar: !!hasAvatar.get(row.id), }); /** @@ -480,6 +484,20 @@ if (!tableColumns("notes").includes("icon_color")) { db.exec("ALTER TABLE notes ADD COLUMN icon_color TEXT"); } +/* A profile picture, kept out of the users table on purpose: every query that + * reads a user does `SELECT *`, and a blob in that row would be carried through + * the admin list, every sign-in and every `/auth/me` for the sake of something + * shown in one corner. Encrypted at rest like every other file here — it comes + * from outside and is a picture of a person. */ +db.exec(` + CREATE TABLE IF NOT EXISTS user_avatars ( + user_id TEXT PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, + bytes BLOB NOT NULL, + content_type TEXT NOT NULL, + updated_at TEXT DEFAULT (datetime('now')) + ); +`); + /* Which identity at the provider this account is, once it has signed in that * way. Kept on the account rather than in a table of its own: a person has one * provider here, and the column is what makes the link survive a rename — the @@ -704,6 +722,7 @@ function authMiddleware(req, res, next) { } } const userExists = db.prepare("SELECT 1 FROM users WHERE id = ?").pluck(); +const hasAvatar = db.prepare("SELECT 1 FROM user_avatars WHERE user_id = ?").pluck(); /* ─────────── Express app ─────────── */ const app = express(); @@ -876,6 +895,7 @@ app.get("/api/auth/oidc/callback", async (req, res) => { try { const identity = await oidc.completeLogin({ code: req.query.code, state: req.query.state }); const user = accountForIdentity(identity); + await rememberPicture(user.id, identity.picture); const token = jwt.sign({ userId: user.id }, JWT_SECRET, { expiresIn: "30d" }); // The token itself never rides in the URL: a redirect lands in the history, // in the proxy's log, and in the next request's referrer. @@ -886,6 +906,60 @@ app.get("/api/auth/oidc/callback", async (req, res) => { } }); +/** + * Keeps the picture the provider has of somebody, or clears it when they take + * theirs away. + * + * On every sign-in rather than only the first: a picture that changed over there + * and never here is worse than none, and this is the one moment we are talking + * to the provider anyway. Failure is silence — the account is already signed in + * by the time this runs, and a portrait is not worth a failed sign-in. + */ +async function rememberPicture(userId, url) { + try { + if (!url) { + db.prepare("DELETE FROM user_avatars WHERE user_id = ?").run(userId); + return; + } + const picture = await oidc.fetchPicture(url); + if (!picture) return; // unreachable or not an image: keep whatever is stored + db.prepare( + `INSERT INTO user_avatars (user_id, bytes, content_type, updated_at) + VALUES (?, ?, ?, datetime('now')) + ON CONFLICT(user_id) DO UPDATE SET bytes = excluded.bytes, + content_type = excluded.content_type, + updated_at = excluded.updated_at` + ).run(userId, encryptBuffer(picture.bytes), picture.type); + } catch (err) { + console.warn("[oidc] could not store the profile picture:", err.message); + } +} + +/** + * The picture itself. + * + * Yourself always; anybody else only for an administrator, who already sees the + * list of accounts. It is not a public avatar service — everything in this app + * is behind the same door. + */ +app.get("/api/users/:id/avatar", authMiddleware, (req, res) => { + const mine = req.params.id === req.userId; + if (!mine && !db.prepare("SELECT is_admin FROM users WHERE id = ?").get(req.userId)?.is_admin) { + return res.status(403).json({ error: "Not allowed" }); + } + const row = db.prepare("SELECT bytes, content_type, updated_at FROM user_avatars WHERE user_id = ?").get(req.params.id); + if (!row) return res.status(404).json({ error: "No picture" }); + try { + res.set("Content-Type", row.content_type); + // Private: it is fetched with a bearer token and belongs to one person. + res.set("Cache-Control", "private, max-age=300"); + res.send(decryptBuffer(row.bytes)); + } catch (err) { + console.error("[avatar] could not be read:", err.message); + res.status(500).json({ error: "Could not read the picture" }); + } +}); + /* The ticket is swapped for the token by the app itself, once, within a minute. */ app.post("/api/auth/oidc/claim", (req, res) => { const payload = oidc.redeemTicket(req.body?.ticket); diff --git a/e2e/fakeIdp.mjs b/e2e/fakeIdp.mjs index 6086aa9..c77d2a3 100644 --- a/e2e/fakeIdp.mjs +++ b/e2e/fakeIdp.mjs @@ -20,6 +20,10 @@ import crypto from "node:crypto"; import http from "node:http"; const b64url = (buf) => Buffer.from(buf).toString("base64url"); +const ONE_PIXEL_PNG = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==", + "base64" +); const json = (res, body, status = 200) => { res.writeHead(status, { "Content-Type": "application/json" }); res.end(JSON.stringify(body)); @@ -76,7 +80,12 @@ export async function startFakeIdp({ port = 3011, issuer, clientId, clientSecret /* Who the next sign-in is, and what the provider should do wrong this time. Set by the suite through POST /who before each attempt. */ - let identity = { sub: "subject-1", preferred_username: "someone", name: "Some One" }; + let identity = { + sub: "subject-1", + preferred_username: "someone", + name: "Some One", + picture: `${issuer}/picture.png`, + }; let misbehave = { wrongKey: false, wrongAudience: false }; const codes = new Map(); // code -> { challenge, nonce, identity } @@ -113,6 +122,13 @@ export async function startFakeIdp({ port = 3011, issuer, clientId, clientSecret }); } + /* A picture to be somebody's face — one green pixel, blown up by the + browser. The point is the journey, not the portrait. */ + if (url.pathname === "/picture.png") { + res.writeHead(200, { "Content-Type": "image/png" }); + return res.end(ONE_PIXEL_PNG); + } + if (url.pathname === "/jwks") { const jwk = real.publicKey.export({ format: "jwk" }); return json(res, { keys: [{ ...jwk, kid, use: "sig", alg: "RS256" }] }); diff --git a/e2e/oidc.mjs b/e2e/oidc.mjs index 63e7d81..84c7953 100644 --- a/e2e/oidc.mjs +++ b/e2e/oidc.mjs @@ -121,6 +121,21 @@ try { "by name", await page.locator("[data-account-sso]").innerText() ); + + /* The picture the provider has of them. It is fetched from *this* server with + a bearer token — an `` cannot carry one, so the bytes come through + the API and become an object URL. Which is also why "it is on the page" is + not enough: ask the browser whether it actually decoded. */ + const drawn = await page.locator("[data-avatar]").first().evaluate((img) => ({ + src: img.currentSrc || img.src, + width: img.naturalWidth, + })); + check(drawn.width > 0, "the profile picture from the provider is drawn", JSON.stringify(drawn)); + check( + drawn.src.startsWith("blob:"), + "from bytes this server handed over, not from the provider's address", + drawn.src.slice(0, 40) + ); await page.keyboard.press("Escape"); await page.waitForTimeout(500); @@ -143,6 +158,10 @@ try { (await adminPage.locator("body").innerText()).includes("Give a password"), "and is offered to give it a password rather than to reset one" ); + check( + (await adminPage.locator(`[data-avatar="${created.id}"]`).count()) === 1, + "and sees their picture beside the name" + ); await adminPage.keyboard.press("Escape"); await adminPage.waitForTimeout(400); await adminPage.keyboard.press("Escape"); @@ -205,6 +224,22 @@ try { await second.waitForSelector("text=Dashboard", { timeout: 20000 }); await second.waitForTimeout(1200); + /* An account that had a password and was then linked: it keeps the password, + and still manages it nowhere but through an administrator. */ + await second.getByTitle("Account").click(); + await second.waitForTimeout(900); + check( + !(await second.getByRole("button", { name: "Password", exact: true }).count()), + "an account linked to the provider is offered no password tab either" + ); + check( + (await second.locator("[data-account-sso]").innerText()).includes("administrator"), + "and is told who changes the one it has", + await second.locator("[data-account-sso]").innerText() + ); + await second.keyboard.press("Escape"); + await second.waitForTimeout(400); + const linked = (await accounts()).filter((u) => u.username === existing); check(linked.length === 1, "the identity is linked to it rather than duplicated", `${linked.length}`); check( diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 2de6e1d..a37d0ea 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -999,6 +999,7 @@ export default function App() { setDark(!dark)} isMobile={isMobile} diff --git a/frontend/src/syncEngine.ts b/frontend/src/syncEngine.ts index b41c77a..c9fdaec 100644 --- a/frontend/src/syncEngine.ts +++ b/frontend/src/syncEngine.ts @@ -427,6 +427,7 @@ export async function restoreSession(): Promise | null> { export async function logout(): Promise { api.forgetToken(); + forgetAvatars(); await db.clearAll(); } @@ -1461,6 +1462,38 @@ export async function fetchAttachmentBlob(id: string): Promise { return api.getBlob(`/attachments/${id}`); } +/* ─── Profile pictures ─── + * + * From the identity provider, stored on our own server and served from there, + * so nothing on the page talks to the provider and a picture keeps working when + * it is unreachable. It comes through the API with a bearer token, which an + * `` cannot carry — so the bytes are fetched once and turned into an + * object URL, exactly as an attached image is. + * + * Cached by account for the life of the page, since it is drawn in two places + * and changes only at the next sign-in. + */ +const avatarUrls = new Map>(); + +export function avatarUrl(userId: string): Promise { + const known = avatarUrls.get(userId); + if (known) return known; + const pending = api + .getBlob(`/users/${userId}/avatar`) + .then((blob) => URL.createObjectURL(blob)) + .catch(() => null); + avatarUrls.set(userId, pending); + return pending; +} + +/** Forgets them, so signing out does not leave one person's face for the next. */ +export function forgetAvatars(): void { + for (const pending of avatarUrls.values()) { + pending.then((url) => url && URL.revokeObjectURL(url)).catch(() => {}); + } + avatarUrls.clear(); +} + /* ─── PDFs ─── * * A PDF rides on the attachment machinery — encrypted at rest, named by content diff --git a/frontend/src/ui/AccountMenu.jsx b/frontend/src/ui/AccountMenu.jsx index 7e7abfa..cc3d0ed 100644 --- a/frontend/src/ui/AccountMenu.jsx +++ b/frontend/src/ui/AccountMenu.jsx @@ -1,6 +1,7 @@ import { useEffect, useMemo, useState } from "react"; import * as sync from "../syncEngine"; import { I } from "./icons.jsx"; +import { Avatar } from "./Avatar.jsx"; import { useBackdropClose } from "./backdrop"; /* ═══════ ACCOUNT MENU ═══════ */ @@ -176,17 +177,20 @@ export function AccountMenu({ user, onClose, onLogout, onUserUpdate, onOpenAdmin borderBottom: `1px solid ${T.border}`, }} > -

- Account -

+
+ +

+ Account +

+
- {!viaProvider && ( + {/* Not shown to an account that signs in through the provider — even + one that still has a password here. Passwords for those accounts + are an administrator's business: the one that exists is the way + back in if the provider is unreachable, not a second thing for its + owner to keep. */} + {!linked && ( diff --git a/readme.md b/readme.md index 126db04..3d49c66 100644 --- a/readme.md +++ b/readme.md @@ -310,11 +310,29 @@ administrator — PocketID, Authentik, Keycloak as they are normally run — tha exactly the intent. Do not point this at one where people choose their own name freely. -**Both doors stay open.** An account created by the provider has no password here -at all: the interface stops offering to change one, and the login route refuses -it exactly as it refuses a wrong password, at the same speed. An administrator -can still *set* one for such an account, and that is deliberate — it is the way -back in on the day the provider is unreachable. +**Both doors stay open, and passwords become an administrator's business.** An +account created by the provider has no password here at all: the login route +refuses one exactly as it refuses a wrong password, at the same speed. An +administrator can still *set* one — deliberately, since that is the way back in +on the day the provider is unreachable — and the button in the list says "Give a +password" rather than offering to reset something that does not exist. + +An account that signs in through the provider is offered no password tab of its +own, and that includes one which had a password before it was linked. The +password it kept is the emergency door, not a second thing for its owner to +maintain; the account panel says so, and names the provider it signs in through. +The administration list marks every linked account, and says which of them have +no password here. + +**The picture comes along.** If the provider sends a `picture` claim — PocketID +does — the server fetches it at each sign-in and stores it, encrypted, in a table +of its own. It is served back through the API, so nothing on the page talks to +the provider, no third-party cookie is needed for it to appear, and it keeps +working when the provider does not. The bytes are fetched once per account and +turned into an object URL, since an `` cannot carry a bearer token; the +picture then shows in the sidebar, in the account panel and beside the name in +the administration list. It arrives with the *next* sign-in: that is the one +moment this app talks to the provider at all. **The refresh token is deliberately unused.** The provider is asked once, at sign-in; InkNote's own token carries the session from then on, and nothing here @@ -566,7 +584,7 @@ docker compose -f docker-compose.dev.yml up -d --build # must be running | Suite | Checks | What it holds down | | --- | --- | --- | | `admin` | 15 | no sign-up anywhere; `/api/admin/*` refused for non-admins even when called directly | -| `oidc` | 26 | the whole redirect journey against a provider that signs real tokens — and the four ways it must fail: a refusal, a replayed ticket, a token signed with an unpublished key, a token addressed to another client | +| `oidc` | 39 | the whole redirect journey against a provider that signs real tokens — and the four ways it must fail: a refusal, a replayed ticket, a token signed with an unpublished key, a token addressed to another client | | `editor` | 40 | tables and their right-click menu, task lists, note links, backlinks — and that each survives a reload | | `search-attachments` | 12 | body-only matches, ranking, umlauts; images as blob URLs, not inline bytes | | `history` | 10 | a large deletion is snapshotted, restoring works, restoring is itself undoable |