mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-24 19:35:28 -06:00
fix(gateway): reuse verified GitHub profiles during outages (#128833)
This commit is contained in:
committed by
GitHub
parent
e1a700840a
commit
6fe4e11a78
@@ -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<string, str
|
||||
});
|
||||
}
|
||||
|
||||
function githubBodyReadFailure() {
|
||||
return new Response(
|
||||
new ReadableStream({
|
||||
start(controller) {
|
||||
controller.error(new Error("body read failed"));
|
||||
},
|
||||
}),
|
||||
{ status: 200 },
|
||||
);
|
||||
}
|
||||
|
||||
function accessAssertion(issuer: unknown): string {
|
||||
const payload = Buffer.from(JSON.stringify({ iss: issuer })).toString("base64url");
|
||||
return `header.${payload}.signature`;
|
||||
@@ -250,6 +262,23 @@ describe("authenticated GitHub identity sync", () => {
|
||||
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<ControlUiGitHubError>);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("redacts the Access assertion from network failures and retries on the same connection", async () => {
|
||||
await withOpenClawTestState({ scenario: "minimal" }, async () => {
|
||||
|
||||
@@ -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<ResolvedGitHubUserIdentity> {
|
||||
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 },
|
||||
|
||||
@@ -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[],
|
||||
|
||||
Reference in New Issue
Block a user