Ask for the picture the way providers actually offer it
Two assumptions, both mine, and either one was enough for an account to end up with no picture. It was looked for only in the ID token, which a provider is free to keep lean while holding the rest at userinfo — PocketID does exactly that — and the endpoint was never asked. Then the image was fetched with no credentials, while avatars are commonly behind the provider's own login. Userinfo is now asked when something is missing, and believed only when its sub matches the token whose signature was checked. The access token rides along with the picture request when, and only when, the picture lives at the provider. The third fault was the one that cost the time: it failed silently, so there was no way to tell which of the two it was. Every outcome is a line in the log now. And the test provider keeps its picture out of the token and behind its door, so the check passing is evidence for both new paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
a2b3f91e00
commit
1e2375f190
6 files changed
+152
-19
No files matched your search
@@ -14,6 +14,33 @@ a rebuild is not bit-identical.
|
||||
|
||||
---
|
||||
|
||||
## 2026-08-21 — Asking for the picture properly
|
||||
|
||||
**Images** — web `sha256:4310304d…`, server `sha256:7a486929…`
|
||||
|
||||
The server image changed; the web image only because `build.sh` builds both.
|
||||
|
||||
**Fixed**
|
||||
- **The profile picture never arrived**, and two assumptions of mine were why.
|
||||
The picture was only looked for in the ID token — a provider may keep its token
|
||||
lean and hold the rest at its `userinfo` endpoint, which is what PocketID does,
|
||||
and that endpoint was never asked. And the image was then fetched with no
|
||||
credentials at all, while providers commonly keep avatars behind their own
|
||||
login. The endpoint is now asked when something is missing (and its answer
|
||||
trusted only if the `sub` matches the verified token), and the access token
|
||||
goes with the picture request — but only when the picture lives at the
|
||||
provider, because a token sent to a third party is a credential given away.
|
||||
- **And it failed in silence**, which is the part that actually cost the time:
|
||||
there was no way to see which of those two it was. Every outcome is now one
|
||||
line in the server log — stored, no claim, refused, not an image, too large,
|
||||
unreachable.
|
||||
|
||||
The test provider now behaves like a lean one: it leaves the picture out of the
|
||||
token and hands it over only against the access token, so the check passing is
|
||||
evidence for both new paths.
|
||||
|
||||
---
|
||||
|
||||
## 2026-08-21 — The face from the provider, and whose business a password is
|
||||
|
||||
**Images** — web `sha256:e525b11d…`, server `sha256:95d56c46…`
|
||||
|
||||
+79
-11
@@ -290,16 +290,50 @@ async function completeLogin({ code, state }) {
|
||||
}
|
||||
if (!tokens.id_token) throw fail("token_exchange_failed", "no id_token in the response");
|
||||
|
||||
const claims = await verifyIdToken(tokens.id_token, attempt.nonce);
|
||||
const verified = await verifyIdToken(tokens.id_token, attempt.nonce);
|
||||
const claims = await withUserInfo(verified, tokens.access_token, doc);
|
||||
return {
|
||||
sub: String(claims.sub),
|
||||
sub: String(verified.sub),
|
||||
username: claims.preferred_username || null,
|
||||
email: claims.email || null,
|
||||
displayName: claims.name || claims.given_name || null,
|
||||
picture: typeof claims.picture === "string" ? claims.picture : null,
|
||||
/* Only for fetching a picture the provider keeps behind its own door, and
|
||||
* only ever sent back to the provider itself. */
|
||||
accessToken: tokens.access_token || null,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* What the ID token did not say.
|
||||
*
|
||||
* A provider is free to put the bare minimum in the token and keep the rest at
|
||||
* its userinfo endpoint, and several do — PocketID among them for the picture.
|
||||
* Asked only when something is actually missing, so a provider that says
|
||||
* everything up front is not made to answer a second request per sign-in.
|
||||
*
|
||||
* The reply is trusted only after its `sub` matches the token's: userinfo is an
|
||||
* ordinary API answer, and the token is the thing whose signature was checked.
|
||||
*/
|
||||
async function withUserInfo(claims, accessToken, doc) {
|
||||
const complete = claims.preferred_username && claims.name && claims.picture;
|
||||
if (complete || !accessToken || !doc.userinfo_endpoint) return claims;
|
||||
try {
|
||||
const info = await getJson(doc.userinfo_endpoint, {
|
||||
headers: { Authorization: `Bearer ${accessToken}`, Accept: "application/json" },
|
||||
});
|
||||
if (String(info.sub) !== String(claims.sub)) {
|
||||
console.warn("[oidc] userinfo is about somebody else — ignored");
|
||||
return claims;
|
||||
}
|
||||
// The signed token wins wherever both speak.
|
||||
return { ...info, ...claims };
|
||||
} catch (err) {
|
||||
console.warn("[oidc] userinfo could not be read:", err.message);
|
||||
return claims;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The profile picture, fetched by the server rather than by the browser.
|
||||
*
|
||||
@@ -313,21 +347,55 @@ async function completeLogin({ code, state }) {
|
||||
* 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;
|
||||
async function fetchPicture(url, { accessToken = null, maxBytes = 512 * 1024 } = {}) {
|
||||
const give_up = (why) => {
|
||||
console.warn(`[oidc] no profile picture: ${why}`);
|
||||
return null;
|
||||
};
|
||||
if (!url) return give_up("the provider sent no picture for this account");
|
||||
|
||||
/* Some providers hand over the image itself rather than an address for it. */
|
||||
const inline = /^data:(image\/[a-z+]+);base64,([\s\S]+)$/i.exec(url.trim());
|
||||
if (inline) {
|
||||
const bytes = Buffer.from(inline[2], "base64");
|
||||
if (!bytes.length || bytes.length > maxBytes) return give_up("the inline picture is empty or too large");
|
||||
return { bytes, type: inline[1].toLowerCase() };
|
||||
}
|
||||
|
||||
if (!/^https?:\/\//i.test(url)) return give_up(`the picture address is not one we can fetch (${url.slice(0, 40)})`);
|
||||
|
||||
/* The token goes with the request only when the picture lives at the provider
|
||||
* — which is where a picture behind a login lives. Never anywhere else: an
|
||||
* access token sent to a third party is a credential given away. */
|
||||
const sameHouse = (() => {
|
||||
try {
|
||||
return new URL(url).origin === new URL(ISSUER).origin;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
})();
|
||||
const headers = accessToken && sameHouse ? { Authorization: `Bearer ${accessToken}` } : {};
|
||||
|
||||
try {
|
||||
const res = await fetch(url, { signal: AbortSignal.timeout(4000), redirect: "follow" });
|
||||
if (!res.ok) return null;
|
||||
const res = await fetch(url, { headers, signal: AbortSignal.timeout(4000), redirect: "follow" });
|
||||
if (!res.ok) {
|
||||
return give_up(
|
||||
res.status === 401 || res.status === 403
|
||||
? `the provider would not hand it over (${res.status})${sameHouse ? "" : " — and it is hosted elsewhere, so no token was sent"}`
|
||||
: `the provider answered ${res.status}`
|
||||
);
|
||||
}
|
||||
const type = String(res.headers.get("content-type") || "").split(";")[0].trim().toLowerCase();
|
||||
if (!/^image\/(png|jpeg|webp|gif)$/.test(type)) return null;
|
||||
if (!/^image\/(png|jpeg|webp|gif|svg\+xml)$/.test(type)) return give_up(`it is not an image we keep (${type || "no content-type"})`);
|
||||
const declared = Number(res.headers.get("content-length"));
|
||||
if (Number.isFinite(declared) && declared > maxBytes) return null;
|
||||
if (Number.isFinite(declared) && declared > maxBytes) return give_up(`it is larger than ${maxBytes} bytes`);
|
||||
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;
|
||||
if (!bytes.length) return give_up("the answer was empty");
|
||||
if (bytes.length > maxBytes) return give_up(`it is larger than ${maxBytes} bytes`);
|
||||
return { bytes, type };
|
||||
} catch {
|
||||
return null;
|
||||
} catch (err) {
|
||||
return give_up(`it could not be fetched (${err.message})`);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+9
-3
@@ -895,7 +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);
|
||||
await rememberPicture(user.id, identity.picture, identity.accessToken);
|
||||
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.
|
||||
@@ -915,13 +915,14 @@ app.get("/api/auth/oidc/callback", async (req, res) => {
|
||||
* 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) {
|
||||
async function rememberPicture(userId, url, accessToken) {
|
||||
try {
|
||||
if (!url) {
|
||||
db.prepare("DELETE FROM user_avatars WHERE user_id = ?").run(userId);
|
||||
console.log(" ℹ️ the provider sent no picture for this account");
|
||||
return;
|
||||
}
|
||||
const picture = await oidc.fetchPicture(url);
|
||||
const picture = await oidc.fetchPicture(url, { accessToken });
|
||||
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)
|
||||
@@ -930,6 +931,7 @@ async function rememberPicture(userId, url) {
|
||||
content_type = excluded.content_type,
|
||||
updated_at = excluded.updated_at`
|
||||
).run(userId, encryptBuffer(picture.bytes), picture.type);
|
||||
console.log(` 🖼️ stored a profile picture from the provider (${picture.type}, ${picture.bytes.length} bytes)`);
|
||||
} catch (err) {
|
||||
console.warn("[oidc] could not store the profile picture:", err.message);
|
||||
}
|
||||
@@ -953,6 +955,10 @@ app.get("/api/users/:id/avatar", authMiddleware, (req, res) => {
|
||||
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");
|
||||
// It came from somewhere else, and one of the types kept here (SVG) is a
|
||||
// document when a browser is allowed to think so. It never is.
|
||||
res.set("X-Content-Type-Options", "nosniff");
|
||||
res.set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'; sandbox");
|
||||
res.send(decryptBuffer(row.bytes));
|
||||
} catch (err) {
|
||||
console.error("[avatar] could not be read:", err.message);
|
||||
|
||||
+21
-2
@@ -20,6 +20,7 @@ import crypto from "node:crypto";
|
||||
import http from "node:http";
|
||||
|
||||
const b64url = (buf) => Buffer.from(buf).toString("base64url");
|
||||
const ACCESS_TOKEN = "test-access-token";
|
||||
const ONE_PIXEL_PNG = Buffer.from(
|
||||
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==",
|
||||
"base64"
|
||||
@@ -89,7 +90,10 @@ export async function startFakeIdp({ port = 3011, issuer, clientId, clientSecret
|
||||
let misbehave = { wrongKey: false, wrongAudience: false };
|
||||
const codes = new Map(); // code -> { challenge, nonce, identity }
|
||||
|
||||
const idToken = ({ nonce, sub, ...rest }) => {
|
||||
const idToken = ({ nonce, sub, picture, ...rest }) => {
|
||||
// `picture` is deliberately left out of the token — it is offered at
|
||||
// /userinfo instead, which is what a lean provider does and what InkNote
|
||||
// used never to ask for.
|
||||
const key = misbehave.wrongKey ? other.privateKey : real.privateKey;
|
||||
const header = b64url(JSON.stringify({ alg: "RS256", typ: "JWT", kid }));
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
@@ -117,6 +121,7 @@ export async function startFakeIdp({ port = 3011, issuer, clientId, clientSecret
|
||||
authorization_endpoint: `${issuer}/authorize`,
|
||||
token_endpoint: `${issuer}/token`,
|
||||
jwks_uri: `${issuer}/jwks`,
|
||||
userinfo_endpoint: `${issuer}/userinfo`,
|
||||
response_types_supported: ["code"],
|
||||
id_token_signing_alg_values_supported: ["RS256"],
|
||||
});
|
||||
@@ -125,10 +130,24 @@ 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") {
|
||||
// Behind the provider's own door, like a real one's: without the access
|
||||
// token there is nothing here.
|
||||
if (req.headers.authorization !== `Bearer ${ACCESS_TOKEN}`) {
|
||||
return json(res, { error: "unauthorized" }, 401);
|
||||
}
|
||||
res.writeHead(200, { "Content-Type": "image/png" });
|
||||
return res.end(ONE_PIXEL_PNG);
|
||||
}
|
||||
|
||||
/* The rest of what is known about somebody. A provider is free to keep the
|
||||
picture here rather than in the token, and several do. */
|
||||
if (url.pathname === "/userinfo") {
|
||||
if (req.headers.authorization !== `Bearer ${ACCESS_TOKEN}`) {
|
||||
return json(res, { error: "unauthorized" }, 401);
|
||||
}
|
||||
return json(res, { sub: identity.sub, ...(identity.picture ? { picture: identity.picture } : {}) });
|
||||
}
|
||||
|
||||
if (url.pathname === "/jwks") {
|
||||
const jwk = real.publicKey.export({ format: "jwk" });
|
||||
return json(res, { keys: [{ ...jwk, kid, use: "sig", alg: "RS256" }] });
|
||||
@@ -183,7 +202,7 @@ export async function startFakeIdp({ port = 3011, issuer, clientId, clientSecret
|
||||
if (computed !== entry.challenge) return json(res, { error: "invalid_grant" }, 400);
|
||||
|
||||
return json(res, {
|
||||
access_token: "not-used-by-inknote",
|
||||
access_token: ACCESS_TOKEN,
|
||||
token_type: "Bearer",
|
||||
expires_in: 300,
|
||||
id_token: idToken({ nonce: entry.nonce, ...entry.identity }),
|
||||
|
||||
@@ -130,6 +130,9 @@ try {
|
||||
src: img.currentSrc || img.src,
|
||||
width: img.naturalWidth,
|
||||
}));
|
||||
/* The provider keeps this one out of the token and behind its own door, which
|
||||
is what a lean provider does — so this passing means both the second
|
||||
question to /userinfo and the token sent with the picture request. */
|
||||
check(drawn.width > 0, "the profile picture from the provider is drawn", JSON.stringify(drawn));
|
||||
check(
|
||||
drawn.src.startsWith("blob:"),
|
||||
|
||||
@@ -324,9 +324,19 @@ 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 picture comes along.** The server looks for a `picture` claim in the ID
|
||||
token and, when it is not there, asks the provider's `userinfo` endpoint — a
|
||||
provider is free to keep the token lean, and several do, PocketID among them.
|
||||
The answer is used only if its `sub` matches the token whose signature was
|
||||
checked. The picture is then fetched at each sign-in and stored, encrypted, in a
|
||||
table of its own; if it lives at the provider, the access token goes with the
|
||||
request, since avatars are often behind the provider's own login. That token is
|
||||
never sent anywhere but to the provider itself.
|
||||
|
||||
Whatever happens, the server log says so in one line — stored, or exactly why
|
||||
not: no claim, refused with a 401, not an image, too large, unreachable.
|
||||
|
||||
The picture is stored 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 `<img src>` cannot carry a bearer token; the
|
||||
|
||||
Reference in new issue
Block a user