From 078a18d0cb0e0cf6fdc70ff47dec5dfd75c6ab21 Mon Sep 17 00:00:00 2001 From: Aarnav Tale Date: Sat, 17 Oct 2026 14:30:05 -0700 Subject: [PATCH] fix(oidc): sync profile picture with the provider on every sign-in --- CHANGELOG.md | 1 + app/server/oidc/provider.ts | 6 ++++- app/server/web/auth.ts | 2 +- tests/unit/auth/auth-service.test.ts | 9 +++++++ tests/unit/oidc/provider.test.ts | 35 ++++++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abe3c3b2..49227d60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ ## Fixes +- Profile pictures from OIDC now correctly update on each sign-in. If the provider stops sending pictures, Headplane removes the picture instead of leaving a potentially broken image behind. - A machine's page now requires permission to view machines, like the machines list. Previously anyone signed in could open it by its address. - Errors on the machine page and in Settings no longer replace the whole app with an error message. The navigation bar and the Settings sidebar stay so you can move on. - The agent page loads while Headscale is unreachable, showing the agent's sync status without the machine details. diff --git a/app/server/oidc/provider.ts b/app/server/oidc/provider.ts index 74fa1e77..5fad830e 100644 --- a/app/server/oidc/provider.ts +++ b/app/server/oidc/provider.ts @@ -707,8 +707,12 @@ export function createOidcService(initialConfig: OidcConfig): OidcService { !claims.name && !claims.email && !claims.picture && !!resolveSubject(claims); const needsSubjectEnrichment = !resolveSubject(claims); const needsRoleEnrichment = !!config.roleClaim && claims[config.roleClaim] === undefined; + const needsPictureEnrichment = !claims.picture && config.profilePictureSource !== "gravatar"; if ( - (!needsEnrichment && !needsSubjectEnrichment && !needsRoleEnrichment) || + (!needsEnrichment && + !needsSubjectEnrichment && + !needsRoleEnrichment && + !needsPictureEnrichment) || !ep.userinfoEndpoint ) { return claims; diff --git a/app/server/web/auth.ts b/app/server/web/auth.ts index 152b0dde..5dc7af2a 100644 --- a/app/server/web/auth.ts +++ b/app/server/web/auth.ts @@ -609,7 +609,7 @@ export function createAuthService(opts: AuthServiceOptions): AuthService { .set({ name: profile?.name, email: profile?.email, - picture: profile?.picture, + picture: profile?.picture ?? null, ...(syncedRole && existing.role !== "owner" ? { role: syncedRole, caps: capsForRole(syncedRole) } : {}), diff --git a/tests/unit/auth/auth-service.test.ts b/tests/unit/auth/auth-service.test.ts index b32cd88b..43183f22 100644 --- a/tests/unit/auth/auth-service.test.ts +++ b/tests/unit/auth/auth-service.test.ts @@ -66,6 +66,15 @@ describe("findOrCreateUser", () => { expect(user?.email).toBe("new@test.com"); }); + test("clears picture when the provider stops sending one", async () => { + await auth.findOrCreateUser("sub-1", { name: "Alice", picture: "https://old.test/a.png" }); + await auth.findOrCreateUser("sub-1", { name: "Alice" }); + + const users = await auth.listUsers(); + const user = users.find((u) => u.sub === "sub-1"); + expect(user?.picture).toBeNull(); + }); + test("syncs an existing non-owner role when explicitly provided", async () => { await auth.findOrCreateUser("sub-owner"); await auth.findOrCreateUser("sub-1", { name: "User" }); diff --git a/tests/unit/oidc/provider.test.ts b/tests/unit/oidc/provider.test.ts index 4fcfe276..f9ccbc26 100644 --- a/tests/unit/oidc/provider.test.ts +++ b/tests/unit/oidc/provider.test.ts @@ -1084,6 +1084,41 @@ describe("userinfo enrichment", () => { expect(userinfoCalledCount).toBe(0); }); + test("fetches picture from userinfo when id token has profile claims without one", async () => { + const svc = createOidcService(testConfig({ usePkce: false })); + const flowResult = await svc.startFlow(); + if (!flowResult.ok) { + throw new Error("startFlow failed"); + } + + const { flowState } = flowResult.value; + const idToken = await signIdToken( + { sub: "user-123", name: "From Token", email: "token@example.com" }, + flowState.nonce, + ); + + tokenHandler = async (_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ access_token: "at", id_token: idToken, token_type: "Bearer" })); + }; + + userinfoHandler = (_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ name: "Should Not Use", picture: "https://example.com/pic.jpg" })); + }; + + const params = new URLSearchParams({ code: "c", state: flowState.state }); + const result = await svc.handleCallback(params, flowState); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + expect(result.value.name).toBe("From Token"); + expect(result.value.picture).toBe("https://example.com/pic.jpg"); + }); + test("userinfo failure does not block login", async () => { const svc = createOidcService(testConfig({ usePkce: false })); const flowResult = await svc.startFlow();