mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
fix(withings): skip activity + sleep sync when OAuth scope lacks user.activity
Legacy v1.4.24-and-earlier Withings connections sit on `user.metrics`
only. W17b/c added `syncUserActivity` + `syncUserSleep` against the
scope-gated `/v2/measure?action=getactivity` and `/v2/sleep?action=get`
endpoints without a scope guard, so every cron tick for a legacy user
hit a guaranteed 403. The catch-block classified the 403 as transient
(the refresh token still worked; only the resource call failed), pg-boss
retried, and the 3-strike admin alert paged Telegram for every affected
user.
Both routines now short-circuit at entry: read `WithingsConnection.scope`,
call `hasActivityScope` from `src/lib/withings/client.ts`, and on miss
park at `error_reauth` via `recordSyncFailure({ kind: "reauth_required",
errorCode: "scope_missing" })` and return 0. The Settings reconnect
banner already in place tells the user what to do; subsequent cron ticks
short-circuit cheaply on `isReauthRequired`.
Defence-in-depth: a 403 reaching the catch-block (race where scope was
revoked between OAuth and the next sync without invalidating the refresh
token) now classifies as `reauth_required` instead of `transient`, so
pg-boss stops retrying and the 3-strike alert stays quiet.
Tests: +4 scope-skip / 403-classification cases per routine (8 total),
defaulting the existing field-mapping + idempotency tests to a
scope-is-fine connection so they stay focused on the write path.
This commit is contained in:
1 parent
ac2142e19a
commit
fec02aa573
4 files changed
+262
-10
No files matched your search
@@ -45,7 +45,7 @@ vi.mock("@/lib/logging/context", () => ({
|
||||
}));
|
||||
|
||||
import { prisma } from "@/lib/db";
|
||||
import { recordSyncSuccess } from "@/lib/integrations/status";
|
||||
import { recordSyncFailure, recordSyncSuccess } from "@/lib/integrations/status";
|
||||
|
||||
import { fetchWithingsActivity, syncUserActivity } from "../sync-activity";
|
||||
|
||||
@@ -74,6 +74,14 @@ function installFetchMock(entries: FakeActivity[]) {
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
// v1.4.26 — every syncUserActivity call now reads the connection's
|
||||
// scope to short-circuit legacy `user.metrics`-only connections.
|
||||
// Default-mock to "scope is fine" so the existing field-mapping +
|
||||
// idempotency tests stay focused on the write path. The scope-skip
|
||||
// case has its own dedicated tests below.
|
||||
vi.mocked(prisma.withingsConnection.findUnique).mockResolvedValue({
|
||||
scope: "user.metrics,user.activity",
|
||||
} as never);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -255,3 +263,88 @@ describe("syncUserActivity — field mapping + idempotency", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("syncUserActivity — scope-skip guard (v1.4.26)", () => {
|
||||
it("returns 0 without firing the Withings call when scope lacks user.activity", async () => {
|
||||
// Legacy v1.4.24- connection — scope holds `user.metrics` only.
|
||||
// The sync must short-circuit BEFORE the upstream fetch so a
|
||||
// guaranteed 403 doesn't park as `transient` and spam pg-boss
|
||||
// retries.
|
||||
vi.mocked(prisma.withingsConnection.findUnique).mockResolvedValue({
|
||||
scope: "user.metrics",
|
||||
} as never);
|
||||
const fetchMock = vi.fn();
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
const imported = await syncUserActivity("user-1");
|
||||
|
||||
expect(imported).toBe(0);
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
expect(prisma.measurement.create).not.toHaveBeenCalled();
|
||||
expect(recordSyncSuccess).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("parks the connection at error_reauth with kind=reauth_required + errorCode=scope_missing", async () => {
|
||||
vi.mocked(prisma.withingsConnection.findUnique).mockResolvedValue({
|
||||
scope: "user.metrics",
|
||||
} as never);
|
||||
vi.stubGlobal("fetch", vi.fn());
|
||||
|
||||
await syncUserActivity("user-1");
|
||||
|
||||
expect(recordSyncFailure).toHaveBeenCalledTimes(1);
|
||||
expect(recordSyncFailure).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
userId: "user-1",
|
||||
integration: "withings",
|
||||
kind: "reauth_required",
|
||||
errorCode: "scope_missing",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("treats a null scope (pre-v1.4.25 connection) as missing user.activity", async () => {
|
||||
vi.mocked(prisma.withingsConnection.findUnique).mockResolvedValue({
|
||||
scope: null,
|
||||
} as never);
|
||||
const fetchMock = vi.fn();
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
const imported = await syncUserActivity("user-1");
|
||||
|
||||
expect(imported).toBe(0);
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
expect(recordSyncFailure).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ kind: "reauth_required" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("classifies a Withings 403 in the catch-block as reauth_required (defence-in-depth)", async () => {
|
||||
// The scope-skip above is the primary guard. This test covers the
|
||||
// race where Withings revokes scope between OAuth and the next
|
||||
// sync without invalidating the refresh token — `getValidToken`
|
||||
// succeeds, the connection scope still claims `user.activity`,
|
||||
// but the resource call 403s. Must park at reauth_required, not
|
||||
// transient, so pg-boss stops retrying.
|
||||
vi.mocked(prisma.withingsConnection.findUnique).mockResolvedValue({
|
||||
scope: "user.metrics,user.activity",
|
||||
} as never);
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => ({
|
||||
status: 200,
|
||||
json: async () => ({ status: 403, error: "insufficient scope" }),
|
||||
})),
|
||||
);
|
||||
|
||||
await expect(syncUserActivity("user-1")).rejects.toThrow(
|
||||
/Withings activity error: 403/,
|
||||
);
|
||||
expect(recordSyncFailure).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
kind: "reauth_required",
|
||||
errorCode: "403",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -44,7 +44,7 @@ vi.mock("@/lib/logging/context", () => ({
|
||||
}));
|
||||
|
||||
import { prisma } from "@/lib/db";
|
||||
import { recordSyncSuccess } from "@/lib/integrations/status";
|
||||
import { recordSyncFailure, recordSyncSuccess } from "@/lib/integrations/status";
|
||||
|
||||
import {
|
||||
fetchWithingsSleep,
|
||||
@@ -73,6 +73,14 @@ function installFetchMock(segments: FakeSegment[]) {
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
// v1.4.26 — every syncUserSleep call now reads the connection's
|
||||
// scope to short-circuit legacy `user.metrics`-only connections.
|
||||
// Default-mock to "scope is fine" so the existing segment-mapping
|
||||
// tests stay focused on the write path. The scope-skip case has
|
||||
// its own dedicated tests below.
|
||||
vi.mocked(prisma.withingsConnection.findUnique).mockResolvedValue({
|
||||
scope: "user.metrics,user.activity",
|
||||
} as never);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -254,3 +262,82 @@ describe("syncUserSleep — segment writes + idempotency", () => {
|
||||
expect(stages).not.toContain("REM");
|
||||
});
|
||||
});
|
||||
|
||||
describe("syncUserSleep — scope-skip guard (v1.4.26)", () => {
|
||||
it("returns 0 without firing the Withings call when scope lacks user.activity", async () => {
|
||||
vi.mocked(prisma.withingsConnection.findUnique).mockResolvedValue({
|
||||
scope: "user.metrics",
|
||||
} as never);
|
||||
const fetchMock = vi.fn();
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
const imported = await syncUserSleep("user-1");
|
||||
|
||||
expect(imported).toBe(0);
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
expect(prisma.measurement.create).not.toHaveBeenCalled();
|
||||
expect(recordSyncSuccess).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("parks the connection at error_reauth with kind=reauth_required + errorCode=scope_missing", async () => {
|
||||
vi.mocked(prisma.withingsConnection.findUnique).mockResolvedValue({
|
||||
scope: "user.metrics",
|
||||
} as never);
|
||||
vi.stubGlobal("fetch", vi.fn());
|
||||
|
||||
await syncUserSleep("user-1");
|
||||
|
||||
expect(recordSyncFailure).toHaveBeenCalledTimes(1);
|
||||
expect(recordSyncFailure).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
userId: "user-1",
|
||||
integration: "withings",
|
||||
kind: "reauth_required",
|
||||
errorCode: "scope_missing",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("treats a null scope (pre-v1.4.25 connection) as missing user.activity", async () => {
|
||||
vi.mocked(prisma.withingsConnection.findUnique).mockResolvedValue({
|
||||
scope: null,
|
||||
} as never);
|
||||
const fetchMock = vi.fn();
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
const imported = await syncUserSleep("user-1");
|
||||
|
||||
expect(imported).toBe(0);
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
expect(recordSyncFailure).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ kind: "reauth_required" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("classifies a Withings 403 in the catch-block as reauth_required (defence-in-depth)", async () => {
|
||||
// Scope claim says we're fine but the resource call 403s — the
|
||||
// race where Withings revokes scope without invalidating the
|
||||
// refresh token. Must park at reauth_required, not transient,
|
||||
// so pg-boss stops retrying.
|
||||
vi.mocked(prisma.withingsConnection.findUnique).mockResolvedValue({
|
||||
scope: "user.metrics,user.activity",
|
||||
} as never);
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => ({
|
||||
status: 200,
|
||||
json: async () => ({ status: 403, error: "insufficient scope" }),
|
||||
})),
|
||||
);
|
||||
|
||||
await expect(syncUserSleep("user-1")).rejects.toThrow(
|
||||
/Withings sleep error: 403/,
|
||||
);
|
||||
expect(recordSyncFailure).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
kind: "reauth_required",
|
||||
errorCode: "403",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -39,6 +39,7 @@ import { prisma } from "@/lib/db";
|
||||
import { getEvent } from "@/lib/logging/context";
|
||||
import { getUnitForType } from "@/lib/validations/measurement";
|
||||
|
||||
import { hasActivityScope } from "./client";
|
||||
import {
|
||||
extractWithingsStatus,
|
||||
isWithingsRefreshReauthFailure,
|
||||
@@ -202,6 +203,40 @@ export async function syncUserActivity(
|
||||
const tokenInfo = await getValidToken(userId);
|
||||
if (!tokenInfo) return 0;
|
||||
|
||||
// v1.4.26 — scope-skip guard. The W17b activity endpoint is gated on
|
||||
// `user.activity`; legacy v1.4.24-and-earlier connections sit on
|
||||
// `user.metrics` only and Withings returns HTTP 200 / status 403 for
|
||||
// every request. Without this short-circuit the call fires, the
|
||||
// catch-block classifies the 403 as `"transient"` (the refresh-token
|
||||
// worked; only the resource call failed), pg-boss retries, and the
|
||||
// 3-strike alert fires for every legacy user that hasn't reconnected
|
||||
// since W5d. The Settings reconnect-banner is already in place — the
|
||||
// sync just has to stop hammering the endpoint until the user acts.
|
||||
//
|
||||
// Park behaviour: mark the connection `error_reauth` so the next
|
||||
// hourly cron tick also short-circuits via `isReauthRequired` above
|
||||
// (cheap query, no upstream call). `getValidToken` already fetched
|
||||
// the connection row to refresh the access token; we re-read the
|
||||
// `scope` column with a `select` so the call is one indexed lookup.
|
||||
const connection = await prisma.withingsConnection.findUnique({
|
||||
where: { userId },
|
||||
select: { scope: true },
|
||||
});
|
||||
if (!hasActivityScope(connection?.scope ?? null)) {
|
||||
getEvent()?.addWarning(
|
||||
`withings activity sync skipped for ${userId}: missing user.activity scope (legacy connection — reconnect required)`,
|
||||
);
|
||||
await recordSyncFailure({
|
||||
userId,
|
||||
integration: "withings",
|
||||
kind: "reauth_required",
|
||||
message:
|
||||
"Withings connection is missing the user.activity scope. Reconnect Withings in Settings to enable activity sync.",
|
||||
errorCode: "scope_missing",
|
||||
});
|
||||
return 0;
|
||||
}
|
||||
|
||||
// 30-day rolling window keeps the call cheap (≤ one page) and
|
||||
// self-heals stale rows from a clock skew or a backfill replay.
|
||||
// The webhook-driven path narrows naturally to today/yesterday;
|
||||
@@ -222,14 +257,22 @@ export async function syncUserActivity(
|
||||
);
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
const status = extractWithingsStatus(message);
|
||||
// v1.4.26 defence-in-depth — a 403 from a scope-gated endpoint
|
||||
// always means scope-missing or token-revoked. Park at
|
||||
// `error_reauth` rather than `transient` so pg-boss stops
|
||||
// retrying and the 3-strike alert doesn't fire. The early
|
||||
// scope-skip above covers the common case; this catches the
|
||||
// race where Withings revokes scope between OAuth and the next
|
||||
// sync without invalidating the refresh token.
|
||||
const isReauth =
|
||||
isWithingsRefreshReauthFailure(message) || status === "403";
|
||||
await recordSyncFailure({
|
||||
userId,
|
||||
integration: "withings",
|
||||
kind: isWithingsRefreshReauthFailure(message)
|
||||
? "reauth_required"
|
||||
: "transient",
|
||||
kind: isReauth ? "reauth_required" : "transient",
|
||||
message,
|
||||
errorCode: extractWithingsStatus(message),
|
||||
errorCode: status,
|
||||
});
|
||||
throw err;
|
||||
}
|
||||
|
||||
@@ -37,6 +37,7 @@ import { prisma } from "@/lib/db";
|
||||
import { getEvent } from "@/lib/logging/context";
|
||||
import { getUnitForType } from "@/lib/validations/measurement";
|
||||
|
||||
import { hasActivityScope } from "./client";
|
||||
import {
|
||||
extractWithingsStatus,
|
||||
isWithingsRefreshReauthFailure,
|
||||
@@ -158,6 +159,31 @@ export async function syncUserSleep(
|
||||
const tokenInfo = await getValidToken(userId);
|
||||
if (!tokenInfo) return 0;
|
||||
|
||||
// v1.4.26 — scope-skip guard. Sleep v2 shares the `user.activity`
|
||||
// scope gate with the activity endpoints; legacy v1.4.24- connections
|
||||
// 403 on every call. Park at `error_reauth` rather than retrying so
|
||||
// pg-boss stops queuing and the 3-strike alert doesn't fire for users
|
||||
// who haven't reconnected since W5d. See `sync-activity.ts` for the
|
||||
// full rationale.
|
||||
const connection = await prisma.withingsConnection.findUnique({
|
||||
where: { userId },
|
||||
select: { scope: true },
|
||||
});
|
||||
if (!hasActivityScope(connection?.scope ?? null)) {
|
||||
getEvent()?.addWarning(
|
||||
`withings sleep sync skipped for ${userId}: missing user.activity scope (legacy connection — reconnect required)`,
|
||||
);
|
||||
await recordSyncFailure({
|
||||
userId,
|
||||
integration: "withings",
|
||||
kind: "reauth_required",
|
||||
message:
|
||||
"Withings connection is missing the user.activity scope. Reconnect Withings in Settings to enable sleep sync.",
|
||||
errorCode: "scope_missing",
|
||||
});
|
||||
return 0;
|
||||
}
|
||||
|
||||
void opts.fullSync;
|
||||
const now = new Date();
|
||||
const start = new Date(
|
||||
@@ -175,14 +201,17 @@ export async function syncUserSleep(
|
||||
);
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
const status = extractWithingsStatus(message);
|
||||
// v1.4.26 defence-in-depth — see sync-activity.ts. A 403 on a
|
||||
// scope-gated endpoint always means scope-missing or token-revoked.
|
||||
const isReauth =
|
||||
isWithingsRefreshReauthFailure(message) || status === "403";
|
||||
await recordSyncFailure({
|
||||
userId,
|
||||
integration: "withings",
|
||||
kind: isWithingsRefreshReauthFailure(message)
|
||||
? "reauth_required"
|
||||
: "transient",
|
||||
kind: isReauth ? "reauth_required" : "transient",
|
||||
message,
|
||||
errorCode: extractWithingsStatus(message),
|
||||
errorCode: status,
|
||||
});
|
||||
throw err;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user