diff --git a/prisma/migrations/0491_google_health_user_id_repair/migration.sql b/prisma/migrations/0491_google_health_user_id_repair/migration.sql new file mode 100644 index 000000000..39653968a --- /dev/null +++ b/prisma/migrations/0491_google_health_user_id_repair/migration.sql @@ -0,0 +1,30 @@ +-- Google Health user id repair. +-- +-- `google_health_connections.google_user_id` held the last path segment of +-- the profile's resource name `users/{user}/profile`, which is the constant +-- `profile` for every account. Since the one-account-per-record check every +-- connection therefore looked like the same Google account, and a second +-- record on the instance could not connect or reconnect (`already_linked`). +-- +-- The column becomes nullable (null = id unknown, which the check treats as +-- no evidence of a second holder) and the placeholder values are cleared. The +-- next connect of each record stores the real id. The table holds one row per +-- connected record, so the update is small; the lock wait is bounded. +-- +-- Reversibility: the cleared values were the same constant on every row and +-- identified nothing; restoring NOT NULL needs a value on every row first. +-- +-- Both statements are repeatable, so a run stopped by the lock timeout is +-- safe to retry. +-- @retry-on-lock-timeout + +DO $migration$ +BEGIN + PERFORM set_config('lock_timeout', '5s', true); + ALTER TABLE "google_health_connections" + ALTER COLUMN "google_user_id" DROP NOT NULL; + UPDATE "google_health_connections" + SET "google_user_id" = NULL + WHERE "google_user_id" IN ('profile', 'me', ''); +END +$migration$; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 0e566b0a6..4454303e9 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -4614,7 +4614,7 @@ model FitbitOAuthState { model GoogleHealthConnection { id String @id @default(cuid()) userId String @unique @map("user_id") - googleUserId String @map("google_user_id") // Google external account id + googleUserId String? @map("google_user_id") // Google Health user id; null when the connect could not read it accessToken String @map("access_token") // encrypted at app level refreshToken String @map("refresh_token") // encrypted at app level tokenExpiresAt DateTime @map("token_expires_at") diff --git a/src/app/api/google-health/callback/route.ts b/src/app/api/google-health/callback/route.ts index 97947aa72..c30af506d 100644 --- a/src/app/api/google-health/callback/route.ts +++ b/src/app/api/google-health/callback/route.ts @@ -9,10 +9,7 @@ import { verifyCallbackActor } from "@/lib/integrations/oauth-kit/connect-target import { annotate, getEvent } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; import { encrypt } from "@/lib/crypto"; -import { - fetchProfile, - resolveGoogleHealthUserId, -} from "@/lib/google-health/client"; +import { fetchProfile, accountIdFor } from "@/lib/google-health/client"; import { googleHealthOAuth } from "@/lib/google-health/oauth"; import { getGoogleHealthScopeString } from "@/lib/google-health/oauth"; import { getUserGoogleHealthCredentials } from "@/lib/google-health/credentials"; @@ -179,7 +176,7 @@ export const GET = apiHandler(async (request: NextRequest) => { } const profile = await fetchProfile(tokens.access_token); - const googleUserId = resolveGoogleHealthUserId(profile); + const googleUserId = await accountIdFor(profile, tokens.access_token); // One Google account, one record. if ( await accountLinkedElsewhere( diff --git a/src/lib/google-health/__tests__/__golden__/map-client-helpers.json b/src/lib/google-health/__tests__/__golden__/map-client-helpers.json index e124b324e..ac0e114bc 100644 --- a/src/lib/google-health/__tests__/__golden__/map-client-helpers.json +++ b/src/lib/google-health/__tests__/__golden__/map-client-helpers.json @@ -1,10 +1,19 @@ { "resolveGoogleHealthUserId": [ + { + "profile": { "name": "users/1234567890/profile" }, + "userId": "1234567890" + }, + { "profile": { "name": "users/me/profile" }, "userId": null }, + { + "profile": { "name": "users/me/profile", "id": "id-0001" }, + "userId": "id-0001" + }, { "profile": { "name": "users/1234567890" }, "userId": "1234567890" }, { "profile": { "name": "users/", "id": "id-0002" }, "userId": "id-0002" }, - { "profile": { "name": "plain-0003" }, "userId": "plain-0003" }, + { "profile": { "name": "plain-0003" }, "userId": null }, { "profile": { "id": "id-0004" }, "userId": "id-0004" }, - { "profile": {}, "userId": "me" } + { "profile": {}, "userId": null } ], "formatCivilBound": [ { diff --git a/src/lib/google-health/__tests__/map-fixtures.test.ts b/src/lib/google-health/__tests__/map-fixtures.test.ts index 289a099fc..a3eaa9493 100644 --- a/src/lib/google-health/__tests__/map-fixtures.test.ts +++ b/src/lib/google-health/__tests__/map-fixtures.test.ts @@ -337,6 +337,9 @@ describe("Google Health mapper goldens", () => { ]; const output = { resolveGoogleHealthUserId: [ + { name: "users/1234567890/profile" }, + { name: "users/me/profile" }, + { name: "users/me/profile", id: "id-0001" }, { name: "users/1234567890" }, { name: "users/", id: "id-0002" }, { name: "plain-0003" }, diff --git a/src/lib/google-health/client.ts b/src/lib/google-health/client.ts index 0228a7a53..7ab93267c 100644 --- a/src/lib/google-health/client.ts +++ b/src/lib/google-health/client.ts @@ -34,20 +34,15 @@ import { import { GOOGLE_HEALTH_API_BASE } from "./request"; /** - * The Google Health profile carries the external user id used as the - * connection's `externalUserId`. A single GET, not a paginated collection. + * The Google Health profile. Its resource `name` is `users/{user}/profile`, + * where `{user}` is the Google Health user id, or the literal `me` when the + * profile was read through the `users/me` alias. + * Docs: https://developers.google.com/health/reference/rest/v4/Profile */ export interface GoogleHealthProfile { - /** - * External user identifier. Google returns `me`-relative profile data; the - * stable id is surfaced under `name` (a `users/{id}` resource name) or `id` - * depending on the API surface — both are captured here and resolved at the - * call site. Re-verify the exact field against a live account at build. - */ name?: string; id?: string; } - export async function fetchProfile( accessToken: string, ): Promise { @@ -89,19 +84,68 @@ export async function fetchProfile( } /** - * Resolve the stable external user id from a Google Health profile. The - * `users/{id}` resource name is the canonical anchor; fall back to a bare `id` - * or "me" so the connection always persists a non-empty `externalUserId`. + * The Google Health user id in a profile, or null when it carries none. + * + * The id is the `{user}` segment of `users/{user}/profile`. Until v1.46.4 the + * last path segment was taken, which is the constant `profile` for every + * account: every connection stored the same id, and since the one-account- + * per-record check (v1.44.0) a second profile on the same instance could not + * connect at all. `me` is the alias, not an id, so it resolves to null. */ export function resolveGoogleHealthUserId( profile: GoogleHealthProfile, -): string { - if (profile.name) { - const tail = profile.name.split("/").pop(); - if (tail) return tail; +): string | null { + const segment = profile.name?.match(/^users\/([^/]+)/)?.[1]; + if (segment && segment !== "me") return segment; + if (profile.id && profile.id !== "me") return profile.id; + return null; +} + +/** + * The Google Health user id from `users/me/identity` (`healthUserId`), or + * null when the read fails or carries none. Used when the profile names the + * account only through the `me` alias. Best-effort: an account whose id + * cannot be read still connects, with the id left unknown. + * Docs: https://developers.google.com/health/reference/rest/v4/users/getIdentity + */ +export async function fetchHealthUserId( + accessToken: string, +): Promise { + const start = performance.now(); + try { + const res = await safeFetch(`${GOOGLE_HEALTH_API_BASE}/users/me/identity`, { + method: "GET", + headers: { Authorization: `Bearer ${accessToken}` }, + }); + getEvent()?.addExternalCall({ + service: "google-health", + method: "fetchIdentity", + duration_ms: Math.round(performance.now() - start), + status: res.status, + }); + if (!res.ok) return null; + const json = (await res.json().catch(() => null)) as { + healthUserId?: unknown; + } | null; + const id = json?.healthUserId; + return typeof id === "string" && id !== "" && id !== "me" ? id : null; + } catch { + return null; } - if (profile.id) return profile.id; - return "me"; +} + +/** + * The id a connection stores: the profile's, else the identity's, else null. + * An id neither read yields is stored as unknown rather than as a shared + * placeholder, so it never makes two records look like one Google account. + */ +export async function accountIdFor( + profile: GoogleHealthProfile, + accessToken: string, +): Promise { + return ( + resolveGoogleHealthUserId(profile) ?? (await fetchHealthUserId(accessToken)) + ); } // ─── Daily roll-up reads (`POST …/dataPoints:dailyRollUp`) ───────────────── diff --git a/src/lib/google-health/sync-core.ts b/src/lib/google-health/sync-core.ts index 7553f1268..82895bc36 100644 --- a/src/lib/google-health/sync-core.ts +++ b/src/lib/google-health/sync-core.ts @@ -119,7 +119,7 @@ export interface GoogleHealthTokenInfo { accessToken: string; /** When `accessToken` stops being accepted. */ expiresAt: Date; - connection: { id: string; googleUserId: string }; + connection: { id: string; googleUserId: string | null }; } /** diff --git a/tests/integration/google-health-account-id.test.ts b/tests/integration/google-health-account-id.test.ts new file mode 100644 index 000000000..5c0ec2cd1 --- /dev/null +++ b/tests/integration/google-health-account-id.test.ts @@ -0,0 +1,216 @@ +/** + * Two records on one instance connect two different Google accounts, against + * real Postgres and the real callback route. + * + * The id a connection stores is the `{user}` segment of the profile's + * resource name `users/{user}/profile`. Until v1.46.4 the last segment was + * stored, the constant `profile`, so every connection looked like the same + * Google account and the one-account-per-record check refused the second + * record on the instance with `already_linked`, also on a reconnect. + */ +import { NextRequest } from "next/server"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +process.env.ENCRYPTION_KEY ??= + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; +process.env.API_TOKEN_HMAC_KEY ??= + "integration-hmac-key-0123456789abcdef0123456789"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/jobs/boss-ready", () => ({ + enqueueWhenBossReady: vi.fn().mockResolvedValue("skipped"), +})); + +const TOKEN_URL = "https://oauth2.googleapis.com/token"; +const PROFILE_URL = "https://health.googleapis.com/v4/users/me/profile"; +const IDENTITY_URL = "https://health.googleapis.com/v4/users/me/identity"; +const FIRST = "gh-first"; +const SECOND = "gh-second"; + +function urlOf(input: unknown): string { + return typeof input === "string" ? input : (input as Request).url; +} + +function json(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); +} + +/** Google as one account sees it: its profile and identity. */ +function googleAs( + healthUserId: string, + opts: { profileName?: string; identityStatus?: number } = {}, +) { + vi.mocked(globalThis.fetch).mockImplementation(async (input) => { + const url = urlOf(input); + if (url === TOKEN_URL) { + return json({ + access_token: `access-${healthUserId}`, + refresh_token: `refresh-${healthUserId}`, + expires_in: 3600, + }); + } + if (url === PROFILE_URL) { + return json({ + name: opts.profileName ?? `users/${healthUserId}/profile`, + age: 40, + }); + } + if (url === IDENTITY_URL) { + if (opts.identityStatus && opts.identityStatus !== 200) { + return json( + { error: { code: opts.identityStatus } }, + opts.identityStatus, + ); + } + return json({ + name: "users/me/identity", + healthUserId, + legacyUserId: "LEGACY", + }); + } + throw new Error(`Unexpected fetch: ${url}`); + }); +} + +async function connect(userId: string): Promise { + const prisma = getPrismaClient(); + const session = await prisma.session.create({ + data: { userId, expiresAt: new Date(Date.now() + 60_000) }, + }); + cookieJar.set("healthlog_session", session.id); + const nonce = `nonce-${userId}-${Math.random().toString(36).slice(2)}`; + await prisma.googleHealthOAuthState.create({ + data: { + nonce, + userId, + codeVerifier: "verifier", + expiresAt: new Date(Date.now() + 60_000), + }, + }); + const { GET } = await import("@/app/api/google-health/callback/route"); + const params = new URLSearchParams({ code: "code", state: nonce }); + const res = await GET( + new NextRequest(`http://localhost/api/google-health/callback?${params}`, { + method: "GET", + headers: { cookie: `google_health_state=${nonce}` }, + }), + ); + return res.headers.get("location"); +} + +async function storedId(userId: string) { + const row = await getPrismaClient().googleHealthConnection.findUnique({ + where: { userId }, + select: { googleUserId: true }, + }); + return row?.googleUserId; +} + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + cookieJar.clear(); + headerJar.clear(); + process.env.NEXT_PUBLIC_APP_URL = "http://localhost:3000"; + const { encrypt } = await import("@/lib/crypto"); + for (const id of [FIRST, SECOND]) { + await getPrismaClient().user.create({ + data: { + id, + username: id, + email: `${id}@example.test`, + role: "USER", + googleHealthClientIdEncrypted: encrypt("client-id"), + googleHealthClientSecretEncrypted: encrypt("client-secret"), + }, + }); + } + vi.spyOn(globalThis, "fetch"); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("Google Health account id", () => { + it("connects two records with two Google accounts and stores each id", async () => { + googleAs("111"); + expect(await connect(FIRST)).toContain("googleHealth=connected"); + googleAs("222"); + expect(await connect(SECOND)).toContain("googleHealth=connected"); + expect(await storedId(FIRST)).toBe("111"); + expect(await storedId(SECOND)).toBe("222"); + }); + + it("still refuses the same Google account on a second record", async () => { + googleAs("111"); + await connect(FIRST); + const location = await connect(SECOND); + expect(location).toContain("reason=already_linked"); + expect(await storedId(SECOND)).toBeUndefined(); + }); + + it("reconnects a record whose stored id is the old placeholder, next to another one", async () => { + const { encrypt } = await import("@/lib/crypto"); + // The state the repair migration leaves: ids cleared to unknown. + for (const userId of [FIRST, SECOND]) { + await getPrismaClient().googleHealthConnection.create({ + data: { + userId, + googleUserId: null, + accessToken: encrypt("old"), + refreshToken: encrypt("old"), + tokenExpiresAt: new Date(), + needsReauth: true, + }, + }); + } + googleAs("222"); + expect(await connect(SECOND)).toContain("googleHealth=connected"); + expect(await storedId(SECOND)).toBe("222"); + }); + + it("reads the id from the identity when the profile answers through the me alias", async () => { + googleAs("333", { profileName: "users/me/profile" }); + expect(await connect(FIRST)).toContain("googleHealth=connected"); + expect(await storedId(FIRST)).toBe("333"); + }); + + it("connects with the id unknown when neither read names the account", async () => { + googleAs("444", { profileName: "users/me/profile", identityStatus: 403 }); + expect(await connect(FIRST)).toContain("googleHealth=connected"); + expect(await storedId(FIRST)).toBeNull(); + googleAs("555", { profileName: "users/me/profile", identityStatus: 403 }); + expect(await connect(SECOND)).toContain("googleHealth=connected"); + expect(await storedId(SECOND)).toBeNull(); + }); +});