mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(polar): lift invalid_grant to reauth and harden the credentials surface
classifyPolarError now lifts a 400 carrying invalid_grant on the token endpoint from persistent to reauth_required, mirroring Oura via the shared isOAuthInvalidGrant helper. A stale or replayed authorization code at connect time now surfaces the reconnect prompt instead of silently parking after 24 h. The BYO-credentials DELETE now audit-logs and parks the integration ledger at disconnected for parity with the dedicated disconnect route, and the PUT audit-logs the credential write. Refresh the connect-route comment that still described Polar as env-only OAuth; it has resolved BYO-first since v1.17.1. Add the invalid_grant classification test plus the previously-missing callback (happy path, CSRF mismatch, cross-user, off-spec token), connect, and credentials route tests.
This commit is contained in:
1 parent
574b860eb4
commit
87e570abfe
7 files changed
+348
-5
No files matched your search
@@ -0,0 +1,102 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
const {
|
||||
stateMatchesCookieMock,
|
||||
verifySignedStateMock,
|
||||
getSessionMock,
|
||||
exchangeCodeMock,
|
||||
registerUserMock,
|
||||
getCredsMock,
|
||||
markReconnectedMock,
|
||||
} = vi.hoisted(() => ({
|
||||
stateMatchesCookieMock: vi.fn(),
|
||||
verifySignedStateMock: vi.fn(),
|
||||
getSessionMock: vi.fn(),
|
||||
exchangeCodeMock: vi.fn(),
|
||||
registerUserMock: vi.fn(),
|
||||
getCredsMock: vi.fn(),
|
||||
markReconnectedMock: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api-handler", () => ({
|
||||
apiHandler: <T extends (...args: unknown[]) => unknown>(fn: T) => fn,
|
||||
}));
|
||||
vi.mock("@/lib/db", () => ({ prisma: { user: { update: vi.fn() } } }));
|
||||
vi.mock("@/lib/logging/context", () => ({
|
||||
annotate: vi.fn(),
|
||||
getEvent: () => ({ setAuth: vi.fn(), setError: vi.fn() }),
|
||||
}));
|
||||
vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() }));
|
||||
vi.mock("@/lib/crypto", () => ({ encrypt: (s: string) => `enc:${s}` }));
|
||||
vi.mock("@/lib/auth/session", () => ({ getSession: getSessionMock }));
|
||||
vi.mock("@/lib/polar/client", () => ({
|
||||
exchangeCode: exchangeCodeMock,
|
||||
registerUser: registerUserMock,
|
||||
}));
|
||||
vi.mock("@/lib/polar/credentials", () => ({
|
||||
getPolarClientCredentials: getCredsMock,
|
||||
}));
|
||||
vi.mock("@/lib/oauth/signed-state", () => ({
|
||||
oauthStateCookieName: () => "polar_oauth_state",
|
||||
stateMatchesCookie: stateMatchesCookieMock,
|
||||
verifySignedState: verifySignedStateMock,
|
||||
}));
|
||||
vi.mock("@/lib/integrations/status", () => ({
|
||||
markReconnected: markReconnectedMock,
|
||||
}));
|
||||
|
||||
import { GET } from "../route";
|
||||
|
||||
process.env.NEXT_PUBLIC_APP_URL = "https://app.example";
|
||||
|
||||
function makeReq(): Parameters<typeof GET>[0] {
|
||||
return {
|
||||
url: "https://app.example/api/polar/callback?code=abc&state=s",
|
||||
cookies: { get: () => ({ value: "s" }) },
|
||||
} as unknown as Parameters<typeof GET>[0];
|
||||
}
|
||||
|
||||
const run = GET as unknown as (req: ReturnType<typeof makeReq>) => Promise<{
|
||||
status: number;
|
||||
headers: { get(k: string): string | null };
|
||||
}>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
stateMatchesCookieMock.mockReturnValue(true);
|
||||
verifySignedStateMock.mockReturnValue({ userId: "u1" });
|
||||
getSessionMock.mockResolvedValue(null);
|
||||
getCredsMock.mockResolvedValue({ clientId: "c", clientSecret: "s" });
|
||||
exchangeCodeMock.mockResolvedValue({ access_token: "tok", x_user_id: 42 });
|
||||
registerUserMock.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
describe("GET /api/polar/callback", () => {
|
||||
it("completes the handshake and redirects to connected", async () => {
|
||||
const res = await run(makeReq());
|
||||
expect(res.status).toBe(307);
|
||||
expect(res.headers.get("location")).toContain("polar=connected");
|
||||
expect(markReconnectedMock).toHaveBeenCalledWith("u1", "polar");
|
||||
});
|
||||
|
||||
it("rejects a CSRF cookie/param mismatch before exchanging the code", async () => {
|
||||
stateMatchesCookieMock.mockReturnValue(false);
|
||||
const res = await run(makeReq());
|
||||
expect(res.headers.get("location")).toContain("reason=csrf1");
|
||||
expect(exchangeCodeMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a cross-user session completing another user's handshake", async () => {
|
||||
getSessionMock.mockResolvedValue({ user: { id: "other" } });
|
||||
const res = await run(makeReq());
|
||||
expect(res.headers.get("location")).toContain("reason=cross_user");
|
||||
expect(exchangeCodeMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects an off-spec token body missing x_user_id", async () => {
|
||||
exchangeCodeMock.mockResolvedValue({ access_token: "tok" });
|
||||
const res = await run(makeReq());
|
||||
expect(res.headers.get("location")).toContain("reason=token");
|
||||
expect(registerUserMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,69 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
const { rateLimitMock, getCredsMock, mintMock, authUrlMock } = vi.hoisted(
|
||||
() => ({
|
||||
rateLimitMock: vi.fn(),
|
||||
getCredsMock: vi.fn(),
|
||||
mintMock: vi.fn(() => "signed-state"),
|
||||
authUrlMock: vi.fn(() => "https://flow.polar.com/oauth2/authorization?x=1"),
|
||||
}),
|
||||
);
|
||||
|
||||
vi.mock("@/lib/api-handler", () => ({
|
||||
apiHandler: <T extends (...args: unknown[]) => unknown>(fn: T) => fn,
|
||||
requireAuth: vi.fn(async () => ({ user: { id: "u1" } })),
|
||||
}));
|
||||
vi.mock("@/lib/api-response", () => ({
|
||||
apiError: (error: string, status: number) => ({ data: null, error, status }),
|
||||
}));
|
||||
vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() }));
|
||||
vi.mock("@/lib/auth/secure-cookie", () => ({
|
||||
shouldEmitSecureCookie: () => true,
|
||||
}));
|
||||
vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: rateLimitMock }));
|
||||
vi.mock("@/lib/polar/client", () => ({ getAuthorizationUrl: authUrlMock }));
|
||||
vi.mock("@/lib/polar/credentials", () => ({
|
||||
getPolarClientCredentials: getCredsMock,
|
||||
}));
|
||||
vi.mock("@/lib/oauth/signed-state", () => ({
|
||||
OAUTH_STATE_TTL_MS: 600_000,
|
||||
mintSignedState: mintMock,
|
||||
oauthStateCookieName: () => "polar_oauth_state",
|
||||
}));
|
||||
|
||||
import { GET } from "../route";
|
||||
|
||||
process.env.NEXT_PUBLIC_APP_URL = "https://app.example";
|
||||
|
||||
const run = GET as unknown as () => Promise<{
|
||||
status: number;
|
||||
headers: { get(k: string): string | null };
|
||||
cookies: { get(k: string): { value: string } | undefined };
|
||||
}>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
rateLimitMock.mockResolvedValue({ allowed: true });
|
||||
getCredsMock.mockResolvedValue({ clientId: "c", clientSecret: "s" });
|
||||
});
|
||||
|
||||
describe("GET /api/polar/connect", () => {
|
||||
it("redirects to the consent screen and sets the state cookie", async () => {
|
||||
const res = await run();
|
||||
expect(res.headers.get("location")).toContain("flow.polar.com");
|
||||
expect(res.cookies.get("polar_oauth_state")?.value).toBe("signed-state");
|
||||
});
|
||||
|
||||
it("redirects to an error when the bucket is exhausted", async () => {
|
||||
rateLimitMock.mockResolvedValue({ allowed: false });
|
||||
const res = await run();
|
||||
expect(res.headers.get("location")).toContain("reason=rate_limited");
|
||||
expect(getCredsMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns 400 when neither BYO nor env credentials resolve", async () => {
|
||||
getCredsMock.mockResolvedValue(null);
|
||||
const res = (await run()) as unknown as { status: number };
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
});
|
||||
@@ -15,11 +15,12 @@ import { NextResponse } from "next/server";
|
||||
/**
|
||||
* v1.17.0 (F4) — redirect the user to the Polar AccessLink consent screen.
|
||||
*
|
||||
* Full OAuth from env: the shared client id/secret come from `POLAR_CLIENT_ID`
|
||||
* / `POLAR_CLIENT_SECRET` (not per-user BYO). The CSRF defence is a stateless
|
||||
* signed state (`src/lib/oauth/signed-state.ts`): the same token is the `state`
|
||||
* URL param AND an httpOnly cookie, and the callback enforces both signature
|
||||
* validity and a byte-exact cookie match.
|
||||
* BYO-key OAuth: the client id/secret resolve DB-first then env via
|
||||
* `getPolarClientCredentials` — a user's own AccessLink app when stored,
|
||||
* otherwise the shared `POLAR_CLIENT_ID` / `POLAR_CLIENT_SECRET`. The CSRF
|
||||
* defence is a stateless signed state (`src/lib/oauth/signed-state.ts`): the
|
||||
* same token is the `state` URL param AND an httpOnly cookie, and the callback
|
||||
* enforces both signature validity and a byte-exact cookie match.
|
||||
*
|
||||
* Rate-limited per user so a session can't spam the consent redirect.
|
||||
*/
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
const { storeMock, clearMock, auditMock, markDisconnectedMock } = vi.hoisted(
|
||||
() => ({
|
||||
storeMock: vi.fn(),
|
||||
clearMock: vi.fn(),
|
||||
auditMock: vi.fn(),
|
||||
markDisconnectedMock: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
vi.mock("@/lib/api-handler", () => ({
|
||||
apiHandler: <T extends (...args: unknown[]) => unknown>(fn: T) => fn,
|
||||
requireAuth: vi.fn(async () => ({ user: { id: "u1" } })),
|
||||
}));
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: { user: { findUnique: vi.fn(), update: vi.fn() } },
|
||||
}));
|
||||
vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() }));
|
||||
vi.mock("@/lib/auth/audit", () => ({ auditLog: auditMock }));
|
||||
vi.mock("@/lib/integrations/status", () => ({
|
||||
markDisconnected: markDisconnectedMock,
|
||||
}));
|
||||
vi.mock("@/lib/polar/credentials", () => ({
|
||||
storePolarClientCredentials: storeMock,
|
||||
clearPolarClientCredentials: clearMock,
|
||||
}));
|
||||
vi.mock("@/lib/api-response", () => ({
|
||||
apiSuccess: (data: unknown) => ({ data, error: null, status: 200 }),
|
||||
apiError: (error: string, status: number) => ({ data: null, error, status }),
|
||||
safeJson: vi.fn(async (req: { _body: unknown }) => ({
|
||||
data: req._body,
|
||||
error: null,
|
||||
})),
|
||||
}));
|
||||
|
||||
import { PUT, DELETE } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
|
||||
type RouteResult = { data: unknown; error: string | null; status: number };
|
||||
const userUpdate = prisma.user.update as ReturnType<typeof vi.fn>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("PUT /api/polar/credentials", () => {
|
||||
it("stores credentials and audit-logs the mutation", async () => {
|
||||
const req = { _body: { clientId: "cid", clientSecret: "csecret" } };
|
||||
const put = PUT as unknown as (r: typeof req) => Promise<RouteResult>;
|
||||
const res = await put(req);
|
||||
expect(res.status).toBe(200);
|
||||
expect(storeMock).toHaveBeenCalledWith("u1", "cid", "csecret");
|
||||
expect(auditMock).toHaveBeenCalledWith("polar.credentials.update", {
|
||||
userId: "u1",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects an invalid body with a 422", async () => {
|
||||
const req = { _body: { clientId: "" } };
|
||||
const put = PUT as unknown as (r: typeof req) => Promise<RouteResult>;
|
||||
const res = await put(req);
|
||||
expect(res.status).toBe(422);
|
||||
expect(storeMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("DELETE /api/polar/credentials", () => {
|
||||
it("clears creds + token and parks the ledger at disconnected", async () => {
|
||||
const del = DELETE as unknown as () => Promise<RouteResult>;
|
||||
const res = await del();
|
||||
expect(res.status).toBe(200);
|
||||
// The active token + member id are nulled.
|
||||
expect(userUpdate).toHaveBeenCalledWith({
|
||||
where: { id: "u1" },
|
||||
data: { polarAccessTokenEncrypted: null, polarUserIdEncrypted: null },
|
||||
});
|
||||
expect(clearMock).toHaveBeenCalledWith("u1");
|
||||
expect(auditMock).toHaveBeenCalledWith("polar.credentials.delete", {
|
||||
userId: "u1",
|
||||
});
|
||||
expect(markDisconnectedMock).toHaveBeenCalledWith("u1", "polar");
|
||||
});
|
||||
});
|
||||
@@ -1,7 +1,9 @@
|
||||
import { prisma } from "@/lib/db";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { apiSuccess, apiError, safeJson } from "@/lib/api-response";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { markDisconnected } from "@/lib/integrations/status";
|
||||
import {
|
||||
storePolarClientCredentials,
|
||||
clearPolarClientCredentials,
|
||||
@@ -57,12 +59,18 @@ export const PUT = apiHandler(async (request: NextRequest) => {
|
||||
result.data.clientId,
|
||||
result.data.clientSecret,
|
||||
);
|
||||
await auditLog("polar.credentials.update", { userId: user.id });
|
||||
|
||||
return apiSuccess({ updated: true });
|
||||
});
|
||||
|
||||
/**
|
||||
* Delete Polar credentials and the active connection.
|
||||
*
|
||||
* Removing the BYO client id/secret also drops the live grant: a token minted
|
||||
* under the user's own AccessLink app would be orphaned once the keys are gone.
|
||||
* Parks the integration ledger at `disconnected` for parity with the dedicated
|
||||
* disconnect route so the status snapshot does not linger at a stale state.
|
||||
*/
|
||||
export const DELETE = apiHandler(async () => {
|
||||
const { user } = await requireAuth();
|
||||
@@ -76,6 +84,8 @@ export const DELETE = apiHandler(async () => {
|
||||
},
|
||||
});
|
||||
await clearPolarClientCredentials(user.id);
|
||||
await auditLog("polar.credentials.delete", { userId: user.id });
|
||||
await markDisconnected(user.id, "polar");
|
||||
|
||||
return apiSuccess({ deleted: true });
|
||||
});
|
||||
@@ -0,0 +1,64 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
PolarApiError,
|
||||
classifyPolarError,
|
||||
isInvalidGrant,
|
||||
} from "../response-classifier";
|
||||
|
||||
describe("classifyPolarError — invalid_grant reauth lift (H-1)", () => {
|
||||
it("lifts a 400 invalid_grant on the token endpoint to reauth_required", () => {
|
||||
const err = new PolarApiError({
|
||||
verb: "exchangeCode",
|
||||
classification: "persistent",
|
||||
httpStatus: 400,
|
||||
reason: "http_400",
|
||||
upstreamError: "invalid_grant",
|
||||
});
|
||||
expect(isInvalidGrant(err)).toBe(true);
|
||||
expect(classifyPolarError(err)).toBe("reauth_required");
|
||||
});
|
||||
|
||||
it("recovers the verdict from the message shape after a lost prototype", () => {
|
||||
// A pg-boss retry can strip the PolarApiError prototype; the message-shape
|
||||
// fallback must still recognise the invalid_grant signal.
|
||||
const bare = new Error("Polar exchangeCode error: 400 - invalid_grant");
|
||||
expect(isInvalidGrant(bare)).toBe(true);
|
||||
expect(classifyPolarError(bare)).toBe("reauth_required");
|
||||
});
|
||||
|
||||
it("leaves a non-invalid_grant 400 as persistent", () => {
|
||||
const err = new PolarApiError({
|
||||
verb: "exchangeCode",
|
||||
classification: "persistent",
|
||||
httpStatus: 400,
|
||||
reason: "http_400",
|
||||
upstreamError: "invalid_client",
|
||||
});
|
||||
expect(isInvalidGrant(err)).toBe(false);
|
||||
expect(classifyPolarError(err)).toBe("persistent");
|
||||
});
|
||||
|
||||
it("keeps a 401 reauth and a 500 transient classification", () => {
|
||||
expect(
|
||||
classifyPolarError(
|
||||
new PolarApiError({
|
||||
verb: "fetchSleeps",
|
||||
classification: "reauth_required",
|
||||
httpStatus: 401,
|
||||
reason: "http_401",
|
||||
}),
|
||||
),
|
||||
).toBe("reauth_required");
|
||||
expect(
|
||||
classifyPolarError(
|
||||
new PolarApiError({
|
||||
verb: "fetchSleeps",
|
||||
classification: "transient",
|
||||
httpStatus: 500,
|
||||
reason: "http_500",
|
||||
}),
|
||||
),
|
||||
).toBe("transient");
|
||||
});
|
||||
});
|
||||
@@ -20,6 +20,7 @@ import {
|
||||
classifyHttpStatus,
|
||||
classifyIntegrationError,
|
||||
IntegrationApiError,
|
||||
isOAuthInvalidGrant,
|
||||
type ClassifiedHttpResponse,
|
||||
type IntegrationClassification,
|
||||
} from "@/lib/integrations/http-status-classifier";
|
||||
@@ -48,6 +49,17 @@ export class PolarApiError extends IntegrationApiError {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* True when a caught error is an OAuth `invalid_grant` on the token endpoint —
|
||||
* a stale/replayed authorization code on Polar's only token call
|
||||
* (`exchangeCode`). Lifts a 400 carrying `invalid_grant` from `persistent` to
|
||||
* `reauth_required` so the connection surfaces the reconnect prompt instead of
|
||||
* silently parking after 24 h.
|
||||
*/
|
||||
export function isInvalidGrant(err: unknown): boolean {
|
||||
return isOAuthInvalidGrant(err, "Polar");
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the classification from any caught error. A non-`PolarApiError` defaults
|
||||
* to `transient` so a plain `Error` retries rather than disabling the
|
||||
@@ -55,5 +67,6 @@ export class PolarApiError extends IntegrationApiError {
|
||||
* caller that lost the prototype across a pg-boss retry.
|
||||
*/
|
||||
export function classifyPolarError(err: unknown): PolarClassification {
|
||||
if (isInvalidGrant(err)) return "reauth_required";
|
||||
return classifyIntegrationError(err, "Polar");
|
||||
}
|
||||
Reference in new issue
Block a user