diff --git a/src/gateway/github-user-identity.test.ts b/src/gateway/github-user-identity.test.ts index f2d64e12eac8..388d97af66f9 100644 --- a/src/gateway/github-user-identity.test.ts +++ b/src/gateway/github-user-identity.test.ts @@ -3,6 +3,7 @@ import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js import { ensureProfileForTailscaleIdentity, getUserProfileListItem, + syncGitHubIdentity, } from "../state/user-profiles.js"; import { withOpenClawTestState } from "../test-utils/openclaw-test-state.js"; import { ControlUiGitHubError } from "./control-ui-github-api.js"; @@ -15,6 +16,17 @@ function githubResponse(body: unknown, status = 200, headers: Record { emails: ["ada@example.com"], githubIdentity: { login: "steipete" }, }); + + fetchMock + .mockResolvedValueOnce( + githubResponse({ + id: 58493, + email: "ada@example.com", + idp: { type: "github" }, + }), + ) + .mockResolvedValueOnce(githubResponse({ id: 58493, login: "steipete-renamed" })); + await expect(cloudflareSync({})?.()).resolves.toMatchObject({ + profileId: result!.profileId, + }); + expect(getUserProfileListItem(result!.profileId).githubIdentity).toMatchObject({ + login: "steipete-renamed", + }); + expect(fetchMock).toHaveBeenCalledTimes(4); }); }); @@ -352,39 +381,116 @@ describe("authenticated GitHub identity sync", () => { "x-ratelimit-remaining": "0", }), }, + { name: "GitHub upstream failure", githubResult: githubResponse({}, 503) }, { name: "GitHub network failure", githubError: new Error("network unavailable") }, - ])("preserves prior identity after a $name", async ({ githubResult, githubError }) => { - await withOpenClawTestState({ scenario: "minimal" }, async () => { - const fetchMock = vi - .spyOn(globalThis, "fetch") - .mockResolvedValueOnce( + ])( + "reattaches the exact cached verified identity after a $name", + async ({ githubResult, githubError }) => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + const fetchMock = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + githubResponse({ + id: 58493, + email: "ada@example.com", + idp: { type: "github" }, + }), + ) + .mockResolvedValueOnce(githubResponse({ id: 58493, login: "steipete" })); + const first = await cloudflareSync({})?.(); + fetchMock.mockResolvedValueOnce( githubResponse({ id: 58493, email: "ada@example.com", idp: { type: "github" }, }), - ) - .mockResolvedValueOnce(githubResponse({ id: 58493, login: "steipete" })); - const first = await cloudflareSync({})?.(); - fetchMock.mockResolvedValueOnce( - githubResponse({ - id: 58493, - email: "ada@example.com", - idp: { type: "github" }, - }), - ); - if (githubError) { - fetchMock.mockRejectedValueOnce(githubError); - } else { - fetchMock.mockResolvedValueOnce(githubResult!); - } + ); + if (githubError) { + fetchMock.mockRejectedValueOnce(githubError); + } else { + fetchMock.mockResolvedValueOnce(githubResult!); + } - await expect(cloudflareSync({})?.()).rejects.toThrow(); - expect(getUserProfileListItem(first!.profileId).githubIdentity).toMatchObject({ - login: "steipete", + await expect(cloudflareSync({ principal: "ADA@Example.COM" })?.()).resolves.toEqual(first); + expect(getUserProfileListItem(first!.profileId).githubIdentity).toMatchObject({ + login: "steipete", + }); + expect(fetchMock).toHaveBeenCalledTimes(4); }); - }); - }); + }, + ); + + it.each([ + { name: "malformed GitHub response", githubStatus: 200, expectedStatus: 502, malformed: true }, + { + name: "GitHub body read failure", + githubStatus: 200, + expectedStatus: 502, + bodyReadFailure: true, + }, + { name: "non-retryable GitHub request", githubStatus: 400, expectedStatus: 502 }, + { name: "unauthorized GitHub account", githubStatus: 401 }, + { name: "GitHub permission denial", githubStatus: 403 }, + { name: "deleted GitHub account", githubStatus: 404 }, + { name: "different cached account", githubStatus: 429, accessAccountId: 99999 }, + { name: "different cached email", githubStatus: 429, principal: "mallory@example.com" }, + { + name: "email and account on different profiles", + githubStatus: 429, + accessAccountId: 99999, + otherProfile: true, + }, + { name: "missing cached identity", githubStatus: 429, seedCache: false }, + ])( + "fails closed for a $name", + async ({ + githubStatus, + expectedStatus, + malformed, + bodyReadFailure, + accessAccountId, + principal, + otherProfile, + seedCache, + }) => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + if (seedCache !== false) { + syncGitHubIdentity({ + identity: { accountId: 58493, login: "steipete" }, + authenticationAlias: { kind: "email", email: "ada@example.com" }, + }); + } + if (otherProfile) { + syncGitHubIdentity({ + identity: { accountId: 99999, login: "mallory" }, + authenticationAlias: { kind: "email", email: "mallory@example.com" }, + }); + } + const authenticatedEmail = principal ?? "ada@example.com"; + const fetchMock = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + githubResponse({ + id: accessAccountId ?? 58493, + email: authenticatedEmail, + idp: { type: "github" }, + }), + ) + .mockResolvedValueOnce( + bodyReadFailure + ? githubBodyReadFailure() + : malformed + ? new Response("{", { status: githubStatus }) + : githubResponse({}, githubStatus), + ); + + await expect(cloudflareSync({ principal: authenticatedEmail })?.()).rejects.toMatchObject({ + statusCode: expectedStatus ?? githubStatus, + } satisfies Partial); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + }, + ); it("redacts the Access assertion from network failures and retries on the same connection", async () => { await withOpenClawTestState({ scenario: "minimal" }, async () => { diff --git a/src/gateway/github-user-identity.ts b/src/gateway/github-user-identity.ts index 1c3439dfdfa8..4b0dc4d905cb 100644 --- a/src/gateway/github-user-identity.ts +++ b/src/gateway/github-user-identity.ts @@ -2,16 +2,19 @@ import type { IncomingHttpHeaders } from "node:http"; import { isRecord } from "@openclaw/normalization-core/record-coerce"; import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce"; import type { GatewayAuthConfig } from "../config/types.gateway.js"; +import { resolveCachedGitHubIdentity } from "../state/user-profile-github-identity.js"; import { classifyTailscaleLogin } from "../state/user-profiles-tailscale-login.js"; import { syncGitHubIdentity } from "../state/user-profiles.js"; import { normalizeGitHubLogin } from "../utils/github-login.js"; import type { GatewayAuthResult } from "./auth.js"; import { ControlUiGitHubError, + fetchGitHubApi, fetchGitHubJson, GITHUB_API_ORIGIN, GITHUB_REQUEST_TIMEOUT_MS, readBoundedResponse, + readGitHubJsonResponse, } from "./control-ui-github-api.js"; const CLOUDFLARE_ACCESS_USER_HEADER = "cf-access-authenticated-user-email"; @@ -142,18 +145,10 @@ async function resolveGitHubUserIdentityByLogin( return { accountId, login }; } -async function resolveGitHubUserIdentityById( +function resolveGitHubUserIdentityById( accountId: number, -): Promise { - let payload: unknown; - try { - payload = await fetchGitHubJson(`${GITHUB_API_ORIGIN}/user/${accountId}`, fetch, undefined); - } catch (error) { - if (error instanceof ControlUiGitHubError) { - throw error; - } - throw new ControlUiGitHubError(502, "GitHub request failed"); - } + payload: unknown, +): ResolvedGitHubUserIdentity { if (!isRecord(payload) || payload.id !== accountId) { throw new ControlUiGitHubError(502, "GitHub account id did not match"); } @@ -246,7 +241,35 @@ export function createAuthenticatedGitHubIdentitySync(params: { access.assertion, access.principal, ); - const identity = await resolveGitHubUserIdentityById(accessIdentity.accountId); + let response: Response | undefined; + let payload: unknown; + try { + response = await fetchGitHubApi( + `${GITHUB_API_ORIGIN}/user/${accessIdentity.accountId}`, + fetch, + ); + payload = await readGitHubJsonResponse(response); + } catch (error) { + const retryable = response + ? response.status === 429 || + response.status >= 500 || + (error instanceof ControlUiGitHubError && error.statusCode === 429) + : !(error instanceof ControlUiGitHubError); + if (retryable) { + // Retry failures may reuse only the exact verified email + immutable-account binding. + const cached = resolveCachedGitHubIdentity({ + accountId: accessIdentity.accountId, + email: access.principal, + }); + if (cached) { + return cached; + } + } + throw error instanceof ControlUiGitHubError + ? error + : new ControlUiGitHubError(502, "GitHub request failed"); + } + const identity = resolveGitHubUserIdentityById(accessIdentity.accountId, payload); const profile = syncGitHubIdentity({ identity, authenticationAlias: { kind: "email", email: access.principal }, diff --git a/src/state/user-profile-github-identity.ts b/src/state/user-profile-github-identity.ts index 27a7d8db2ca7..f60588b1f07c 100644 --- a/src/state/user-profile-github-identity.ts +++ b/src/state/user-profile-github-identity.ts @@ -60,6 +60,34 @@ function selectStoredGitHubIdentities( ); } +export function resolveCachedGitHubIdentity( + params: { accountId: number; email: string }, + options: OpenClawStateDatabaseOptions = {}, +): { profileId: string; updatedAt: number } | undefined { + const email = params.email.trim().toLowerCase(); + if (!email || !Number.isSafeInteger(params.accountId) || params.accountId <= 0) { + return undefined; + } + const database = openOpenClawStateDatabase(options); + ensureUserProfilesSchema(options, database); + const { db } = database; + const alias = executeSqliteQueryTakeFirstSync( + db, + userProfilesDb(db) + .selectFrom("user_profile_emails") + .select("profile_id") + .where("email", "=", email), + ); + const profile = alias ? selectResolvedUserProfileById(db, alias.profile_id) : undefined; + if (!profile) { + return undefined; + } + const identity = selectStoredGitHubIdentities(db, [profile.id]).get(profile.id); + return identity?.accountId === params.accountId + ? { profileId: profile.id, updatedAt: profile.updated_at } + : undefined; +} + function deleteProfileGitHubIdentities( db: DatabaseSync, profileIds: readonly string[],