From af3d01c6ad3fe9a852fdbed61d057a72cfe3278a Mon Sep 17 00:00:00 2001 From: Patrick Erichsen Date: Fri, 17 Jul 2026 18:02:30 -0700 Subject: [PATCH] feat: verify organization GitHub profiles (#3169) --- .github/workflows/ci.yml | 1 + convex/_generated/api.d.ts | 4 + convex/auth.test.ts | 71 +++++- convex/auth.ts | 35 ++- convex/githubOrgMemberships.ts | 34 +++ convex/lib/githubOrgMemberships.test.ts | 137 +++++++++++ convex/lib/githubOrgMemberships.ts | 199 ++++++++++++++++ convex/lib/public.test.ts | 25 ++ convex/lib/public.ts | 13 +- convex/lib/retentionPolicy.ts | 4 + convex/publishers.test.ts | 133 +++++++++++ convex/publishers.ts | 54 +++++ convex/schema.ts | 18 ++ convex/users.test.ts | 19 +- convex/users.ts | 55 ++++- e2e/local-auth/helpers.ts | 38 ++-- .../publisher-github-profile.pw.test.ts | 49 ++++ specs/auth-identity.md | 14 ++ src/__tests__/user-profile-route.test.tsx | 19 ++ src/lib/publicUser.ts | 11 +- src/routes/-settings.test.tsx | 215 ++++++++++++++++++ src/routes/settings.tsx | 162 ++++++++++++- src/routes/user/$handle.tsx | 47 ++-- 23 files changed, 1307 insertions(+), 50 deletions(-) create mode 100644 convex/githubOrgMemberships.ts create mode 100644 convex/lib/githubOrgMemberships.test.ts create mode 100644 convex/lib/githubOrgMemberships.ts create mode 100644 e2e/local-auth/publisher-github-profile.pw.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d280f09e..3af65249 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -163,6 +163,7 @@ jobs: specs: | e2e/local-auth/header-profile-link.pw.test.ts e2e/local-auth/manage-context-proof.pw.test.ts + e2e/local-auth/publisher-github-profile.pw.test.ts - name: moderation-malicious specs: e2e/local-auth/malicious-skill-ban-flow.pw.test.ts - name: star-sync diff --git a/convex/_generated/api.d.ts b/convex/_generated/api.d.ts index 5065bc39..5ac5f0df 100644 --- a/convex/_generated/api.d.ts +++ b/convex/_generated/api.d.ts @@ -27,6 +27,7 @@ import type * as functions from "../functions.js"; import type * as githubAccountAgeBackfill from "../githubAccountAgeBackfill.js"; import type * as githubIdentity from "../githubIdentity.js"; import type * as githubImport from "../githubImport.js"; +import type * as githubOrgMemberships from "../githubOrgMemberships.js"; import type * as githubSkillSources from "../githubSkillSources.js"; import type * as githubSkillSync from "../githubSkillSync.js"; import type * as githubSkillSyncNode from "../githubSkillSyncNode.js"; @@ -70,6 +71,7 @@ import type * as lib_githubAuth from "../lib/githubAuth.js"; import type * as lib_githubHandoff from "../lib/githubHandoff.js"; import type * as lib_githubIdentity from "../lib/githubIdentity.js"; import type * as lib_githubImport from "../lib/githubImport.js"; +import type * as lib_githubOrgMemberships from "../lib/githubOrgMemberships.js"; import type * as lib_githubProfileSync from "../lib/githubProfileSync.js"; import type * as lib_githubSkillScans from "../lib/githubSkillScans.js"; import type * as lib_githubSkillSync from "../lib/githubSkillSync.js"; @@ -194,6 +196,7 @@ declare const fullApi: ApiFromModules<{ githubAccountAgeBackfill: typeof githubAccountAgeBackfill; githubIdentity: typeof githubIdentity; githubImport: typeof githubImport; + githubOrgMemberships: typeof githubOrgMemberships; githubSkillSources: typeof githubSkillSources; githubSkillSync: typeof githubSkillSync; githubSkillSyncNode: typeof githubSkillSyncNode; @@ -237,6 +240,7 @@ declare const fullApi: ApiFromModules<{ "lib/githubHandoff": typeof lib_githubHandoff; "lib/githubIdentity": typeof lib_githubIdentity; "lib/githubImport": typeof lib_githubImport; + "lib/githubOrgMemberships": typeof lib_githubOrgMemberships; "lib/githubProfileSync": typeof lib_githubProfileSync; "lib/githubSkillScans": typeof lib_githubSkillScans; "lib/githubSkillSync": typeof lib_githubSkillSync; diff --git a/convex/auth.test.ts b/convex/auth.test.ts index 3c39c80b..ebb9ec9e 100644 --- a/convex/auth.test.ts +++ b/convex/auth.test.ts @@ -156,6 +156,14 @@ describe("handleDeletedUserSignIn", () => { }); describe("GitHub auth provider", () => { + it("requests read-only GitHub organization membership access", () => { + const provider = createGitHubAuthProvider() as { + options?: { authorization?: { params?: { scope?: string } } }; + }; + + expect(provider.options?.authorization?.params?.scope?.split(" ")).toContain("read:org"); + }); + it("does not link ClawHub accounts by GitHub profile email", () => { const provider = createGitHubAuthProvider() as { options?: { allowDangerousEmailAccountLinking?: boolean }; @@ -181,19 +189,74 @@ describe("GitHub auth provider", () => { ); }); - it("fails closed when the GitHub provider receives a malformed profile", () => { + it("fails closed when the GitHub provider receives a malformed profile", async () => { const provider = createGitHubAuthProvider() as { - options?: { profile?: (profile: Record) => Record }; + options?: { + profile?: ( + profile: Record, + tokens: { access_token?: string }, + ) => Promise>; + }; }; - expect(() => provider.options?.profile?.({ message: "Bad credentials" })).toThrow( + await expect(provider.options?.profile?.({ message: "Bad credentials" }, {})).rejects.toThrow( "GitHub OAuth profile is missing a valid numeric id", ); - expect(provider.options?.profile?.({ id: 123456, login: "fixture-user" })).toEqual({ + await expect( + provider.options?.profile?.({ id: 123456, login: "fixture-user" }, {}), + ).resolves.toEqual({ id: "123456", name: "fixture-user", email: undefined, image: undefined, }); }); + + it("adds a verified GitHub organization snapshot to the OAuth profile", async () => { + const fetchMock = vi.spyOn(globalThis, "fetch").mockResolvedValueOnce( + new Response( + JSON.stringify([ + { + state: "active", + role: "member", + organization: { id: 42, login: "trycua" }, + }, + ]), + { status: 200 }, + ), + ); + const provider = createGitHubAuthProvider() as { + options?: { + profile?: ( + profile: Record, + tokens: { access_token?: string }, + ) => Promise>; + }; + }; + + await expect( + provider.options?.profile?.( + { id: 123456, login: "fixture-user" }, + { access_token: "test-token-placeholder" }, + ), + ).resolves.toEqual({ + id: "123456", + name: "fixture-user", + email: undefined, + image: undefined, + githubOrgMembershipSync: { + memberships: [ + { + githubOrgId: "42", + login: "trycua", + avatarUrl: undefined, + role: "member", + }, + ], + syncedAt: expect.any(Number), + truncated: false, + }, + }); + fetchMock.mockRestore(); + }); }); diff --git a/convex/auth.ts b/convex/auth.ts index 75c856b2..df558c45 100644 --- a/convex/auth.ts +++ b/convex/auth.ts @@ -6,6 +6,12 @@ import { ConvexError } from "convex/values"; import { internal } from "./_generated/api"; import type { DataModel, Id } from "./_generated/dataModel"; import { isLocalDevAuthEnabled } from "./lib/devAuth"; +import { + GITHUB_ORG_MEMBERSHIP_SYNC_PROFILE_KEY, + fetchActiveGitHubOrgMemberships, + readGitHubOrgMembershipSync, + replaceGitHubOrgMemberships, +} from "./lib/githubOrgMemberships"; import { shouldScheduleGitHubProfileSync } from "./lib/githubProfileSync"; export const BANNED_REAUTH_MESSAGE = @@ -39,15 +45,34 @@ export function createGitHubAuthProvider() { return GitHub({ clientId: process.env.AUTH_GITHUB_ID ?? "", clientSecret: process.env.AUTH_GITHUB_SECRET ?? "", + authorization: { + params: { scope: "read:user user:email read:org" }, + }, // GitHub's OAuth email must not be treated as a ClawHub account key. The // immutable GitHub provider account id is the only account-linking key. allowDangerousEmailAccountLinking: false, - profile(profile) { + async profile(profile, tokens) { + let githubOrgMembershipSync; + const accessToken = tokens.access_token?.trim(); + if (accessToken) { + try { + githubOrgMembershipSync = await fetchActiveGitHubOrgMemberships(accessToken); + } catch (error) { + console.warn( + `[auth] GitHub organization membership sync failed: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } return { id: normalizeGitHubProfileId(profile.id), name: profile.login, email: profile.email ?? undefined, image: profile.avatar_url, + ...(githubOrgMembershipSync + ? { [GITHUB_ORG_MEMBERSHIP_SYNC_PROFILE_KEY]: githubOrgMembershipSync } + : {}), }; }, }); @@ -120,6 +145,7 @@ function userDataFromAuthProfile(args: { const { emailVerified: profileEmailVerified, phoneVerified: profilePhoneVerified, + [GITHUB_ORG_MEMBERSHIP_SYNC_PROFILE_KEY]: _githubOrgMembershipSync, ...profile } = args.profile; const emailVerified = @@ -187,6 +213,7 @@ export const { auth, signIn, signOut, store, isAuthenticated } = convexAuth({ */ async createOrUpdateUser(ctx, args) { const userData = userDataFromAuthProfile(args); + const githubOrgMembershipSync = readGitHubOrgMembershipSync(args.profile); if (args.existingUserId !== null) { const userId = args.existingUserId as Id<"users">; const existingUser = await ctx.db.get(userId); @@ -194,11 +221,17 @@ export const { auth, signIn, signOut, store, isAuthenticated } = convexAuth({ return userId; } await ctx.db.patch(userId, userData); + if (githubOrgMembershipSync) { + await replaceGitHubOrgMemberships(ctx, userId, githubOrgMembershipSync); + } await schedulePostUserCreatedOrUpdated(ctx, userId, existingUser); return userId; } const userId = await ctx.db.insert("users", userData); + if (githubOrgMembershipSync) { + await replaceGitHubOrgMemberships(ctx, userId, githubOrgMembershipSync); + } const user = await ctx.db.get(userId); await schedulePostUserCreatedOrUpdated(ctx, userId, user); return userId; diff --git a/convex/githubOrgMemberships.ts b/convex/githubOrgMemberships.ts new file mode 100644 index 00000000..effcbdb1 --- /dev/null +++ b/convex/githubOrgMemberships.ts @@ -0,0 +1,34 @@ +import { query } from "./functions"; +import { getOptionalActiveAuthUserId } from "./lib/access"; + +export const listMine = query({ + args: {}, + handler: async (ctx) => { + const userId = await getOptionalActiveAuthUserId(ctx); + if (!userId) { + return { syncedAt: null, truncated: false, memberships: [] }; + } + const user = await ctx.db.get(userId); + if (!user || user.deletedAt || user.deactivatedAt) { + return { syncedAt: null, truncated: false, memberships: [] }; + } + + const memberships = await ctx.db + .query("githubOrgMemberships") + .withIndex("by_user", (q) => q.eq("userId", userId)) + .collect(); + memberships.sort((left, right) => left.login.localeCompare(right.login)); + + return { + syncedAt: user.githubOrgMembershipsSyncedAt ?? null, + truncated: user.githubOrgMembershipsTruncated ?? false, + memberships: memberships.map(({ githubOrgId, login, avatarUrl, role, syncedAt }) => ({ + githubOrgId, + login, + avatarUrl: avatarUrl ?? null, + role, + syncedAt, + })), + }; + }, +}); diff --git a/convex/lib/githubOrgMemberships.test.ts b/convex/lib/githubOrgMemberships.test.ts new file mode 100644 index 00000000..50eae55e --- /dev/null +++ b/convex/lib/githubOrgMemberships.test.ts @@ -0,0 +1,137 @@ +import { describe, expect, it, vi } from "vitest"; +import { + fetchActiveGitHubOrgMemberships, + readGitHubOrgMembershipSync, +} from "./githubOrgMemberships"; + +describe("GitHub organization memberships", () => { + it("loads active memberships from the authenticated GitHub API", async () => { + const fetchImpl = vi.fn(async () => { + return new Response( + JSON.stringify([ + { + state: "active", + role: "member", + organization: { + id: 2, + login: "trycua", + avatar_url: "https://avatars.githubusercontent.com/u/2", + }, + }, + { + state: "active", + role: "admin", + organization: { + id: 1, + login: "openclaw", + avatar_url: "https://avatars.githubusercontent.com/u/1", + }, + }, + { + state: "pending", + role: "member", + organization: { id: 3, login: "pending-org" }, + }, + ]), + { status: 200 }, + ); + }); + + const result = await fetchActiveGitHubOrgMemberships("github-token", { + fetchImpl: fetchImpl as typeof fetch, + now: 123, + }); + + expect(result).toEqual({ + syncedAt: 123, + truncated: false, + memberships: [ + { + githubOrgId: "1", + login: "openclaw", + avatarUrl: "https://avatars.githubusercontent.com/u/1", + role: "admin", + }, + { + githubOrgId: "2", + login: "trycua", + avatarUrl: "https://avatars.githubusercontent.com/u/2", + role: "member", + }, + ], + }); + expect(fetchImpl).toHaveBeenCalledWith( + expect.stringContaining("/user/memberships/orgs?state=active"), + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: "Bearer github-token", + }), + }), + ); + }); + + it("rejects GitHub API failures without accepting partial membership data", async () => { + const fetchImpl = vi.fn(async () => new Response("Forbidden", { status: 403 })); + + await expect( + fetchActiveGitHubOrgMemberships("github-token", { + fetchImpl: fetchImpl as typeof fetch, + }), + ).rejects.toThrow("GitHub organization membership lookup failed (403)"); + }); + + it("loads every GitHub organization membership page", async () => { + const firstPage = Array.from({ length: 100 }, (_, index) => ({ + state: "active", + role: "member", + organization: { id: index + 1, login: `org-${index + 1}` }, + })); + const fetchImpl = vi + .fn() + .mockResolvedValueOnce(new Response(JSON.stringify(firstPage), { status: 200 })) + .mockResolvedValueOnce( + new Response( + JSON.stringify([ + { + state: "active", + role: "admin", + organization: { id: 101, login: "org-101" }, + }, + ]), + { status: 200 }, + ), + ); + + const result = await fetchActiveGitHubOrgMemberships("github-token", { + fetchImpl: fetchImpl as typeof fetch, + now: 123, + }); + + expect(result.memberships).toHaveLength(101); + expect(result.truncated).toBe(false); + expect(fetchImpl).toHaveBeenCalledTimes(2); + expect(fetchImpl).toHaveBeenLastCalledWith( + expect.stringContaining("page=2"), + expect.any(Object), + ); + }); + + it("validates membership snapshots before they reach the database", () => { + expect( + readGitHubOrgMembershipSync({ + githubOrgMembershipSync: { + syncedAt: 123, + truncated: false, + memberships: [ + { githubOrgId: "1", login: "openclaw", role: "admin" }, + { githubOrgId: "invalid", login: "spoofed", role: "member" }, + ], + }, + }), + ).toEqual({ + syncedAt: 123, + truncated: false, + memberships: [{ githubOrgId: "1", login: "openclaw", role: "admin" }], + }); + }); +}); diff --git a/convex/lib/githubOrgMemberships.ts b/convex/lib/githubOrgMemberships.ts new file mode 100644 index 00000000..f821ab23 --- /dev/null +++ b/convex/lib/githubOrgMemberships.ts @@ -0,0 +1,199 @@ +import type { Id } from "../_generated/dataModel"; +import type { MutationCtx } from "../_generated/server"; + +const GITHUB_API = "https://api.github.com"; +const GITHUB_API_VERSION = "2022-11-28"; +const GITHUB_ORG_PAGE_SIZE = 100; + +export const GITHUB_ORG_MEMBERSHIP_VERIFICATION_MAX_AGE_MS = 15 * 60 * 1000; +export const GITHUB_ORG_MEMBERSHIP_SYNC_PROFILE_KEY = "githubOrgMembershipSync"; + +type FetchImpl = typeof fetch; + +export type GitHubOrgMembership = { + githubOrgId: string; + login: string; + avatarUrl?: string; + role: "admin" | "member"; +}; + +export type GitHubOrgMembershipSync = { + memberships: GitHubOrgMembership[]; + syncedAt: number; + truncated: boolean; +}; + +type GitHubMembershipPayload = { + state?: unknown; + role?: unknown; + organization?: { + id?: unknown; + login?: unknown; + avatar_url?: unknown; + }; +}; + +export async function fetchActiveGitHubOrgMemberships( + accessToken: string, + options: { fetchImpl?: FetchImpl; now?: number } = {}, +): Promise { + const fetchImpl = options.fetchImpl ?? fetch; + const memberships: GitHubOrgMembership[] = []; + + for (let page = 1; ; page += 1) { + const response = await fetchImpl( + `${GITHUB_API}/user/memberships/orgs?state=active&per_page=${GITHUB_ORG_PAGE_SIZE}&page=${page}`, + { + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${accessToken}`, + "User-Agent": "clawhub/github-org-memberships", + "X-GitHub-Api-Version": GITHUB_API_VERSION, + }, + }, + ); + if (!response.ok) { + throw new Error(`GitHub organization membership lookup failed (${response.status})`); + } + + const payload = (await response.json()) as unknown; + if (!Array.isArray(payload)) { + throw new Error("GitHub organization membership lookup returned invalid data"); + } + + for (const row of payload) { + const membership = parseGitHubOrgMembership(row); + if (!membership) continue; + memberships.push(membership); + } + + if (payload.length < GITHUB_ORG_PAGE_SIZE) { + return { + memberships: dedupeMemberships(memberships), + syncedAt: options.now ?? Date.now(), + truncated: false, + }; + } + } +} + +export function readGitHubOrgMembershipSync( + profile: Record, +): GitHubOrgMembershipSync | null { + const value = profile[GITHUB_ORG_MEMBERSHIP_SYNC_PROFILE_KEY]; + if (!value || typeof value !== "object") return null; + const candidate = value as Partial; + if ( + !Array.isArray(candidate.memberships) || + typeof candidate.syncedAt !== "number" || + !Number.isFinite(candidate.syncedAt) || + typeof candidate.truncated !== "boolean" + ) { + return null; + } + + const memberships = candidate.memberships + .map((membership) => parseStoredMembership(membership)) + .filter((membership): membership is GitHubOrgMembership => membership !== null); + return { + memberships: dedupeMemberships(memberships), + syncedAt: candidate.syncedAt, + truncated: candidate.truncated, + }; +} + +export async function replaceGitHubOrgMemberships( + ctx: Pick, + userId: Id<"users">, + sync: GitHubOrgMembershipSync, +) { + const existing = await ctx.db + .query("githubOrgMemberships") + .withIndex("by_user", (q) => q.eq("userId", userId)) + .collect(); + const existingByOrgId = new Map( + existing.map((membership) => [membership.githubOrgId, membership]), + ); + const nextIds = new Set(); + + for (const membership of sync.memberships) { + nextIds.add(membership.githubOrgId); + const current = existingByOrgId.get(membership.githubOrgId); + const value = { + userId, + githubOrgId: membership.githubOrgId, + login: membership.login, + avatarUrl: membership.avatarUrl, + role: membership.role, + syncedAt: sync.syncedAt, + }; + if (current) { + await ctx.db.patch(current._id, value); + } else { + await ctx.db.insert("githubOrgMemberships", value); + } + } + + for (const membership of existing) { + if (!nextIds.has(membership.githubOrgId)) { + await ctx.db.delete(membership._id); + } + } + + await ctx.db.patch(userId, { + githubOrgMembershipsSyncedAt: sync.syncedAt, + githubOrgMembershipsTruncated: sync.truncated || undefined, + }); +} + +function parseGitHubOrgMembership(value: unknown): GitHubOrgMembership | null { + if (!value || typeof value !== "object") return null; + const row = value as GitHubMembershipPayload; + if (row.state !== "active" || (row.role !== "admin" && row.role !== "member")) return null; + return parseStoredMembership({ + githubOrgId: normalizeNumericId(row.organization?.id), + login: row.organization?.login, + avatarUrl: row.organization?.avatar_url, + role: row.role, + }); +} + +function parseStoredMembership(value: unknown): GitHubOrgMembership | null { + if (!value || typeof value !== "object") return null; + const row = value as Partial; + const githubOrgId = normalizeNumericId(row.githubOrgId); + const login = typeof row.login === "string" ? row.login.trim() : ""; + const role = row.role; + if (!githubOrgId || !isGitHubLogin(login) || (role !== "admin" && role !== "member")) { + return null; + } + const avatarUrl = + typeof row.avatarUrl === "string" && isHttpsUrl(row.avatarUrl.trim()) + ? row.avatarUrl.trim() + : undefined; + return { githubOrgId, login, avatarUrl, role }; +} + +function normalizeNumericId(value: unknown) { + if (typeof value === "number" && Number.isSafeInteger(value) && value > 0) return String(value); + if (typeof value === "string" && /^[1-9]\d*$/.test(value.trim())) return value.trim(); + return null; +} + +function isGitHubLogin(value: string) { + return /^[a-z\d](?:[a-z\d-]{0,37}[a-z\d])?$/i.test(value); +} + +function isHttpsUrl(value: string) { + try { + return new URL(value).protocol === "https:"; + } catch { + return false; + } +} + +function dedupeMemberships(memberships: GitHubOrgMembership[]) { + return [ + ...new Map(memberships.map((membership) => [membership.githubOrgId, membership])).values(), + ].sort((left, right) => left.login.localeCompare(right.login)); +} diff --git a/convex/lib/public.test.ts b/convex/lib/public.test.ts index 178c1337..35070e16 100644 --- a/convex/lib/public.test.ts +++ b/convex/lib/public.test.ts @@ -133,4 +133,29 @@ describe("public publisher mapping", () => { expect(toPublicPublisher(publisher)).not.toHaveProperty("official"); expect(toPublicPublisher(publisher, { official: true })?.official).toBe(true); }); + + it("exposes a verified GitHub profile without exposing verification internals", () => { + const publisher = { + _id: "publishers:cua", + _creationTime: 1, + kind: "org", + handle: "cua", + displayName: "Cua", + githubHandle: "trycua", + githubOrgId: "42", + githubVerifiedAt: 123, + githubVerifiedByUserId: "users:admin", + createdAt: 1, + updatedAt: 1, + } as Doc<"publishers">; + + expect(toPublicPublisher(publisher)).toEqual( + expect.objectContaining({ + githubHandle: "trycua", + githubVerifiedAt: 123, + }), + ); + expect(toPublicPublisher(publisher)).not.toHaveProperty("githubOrgId"); + expect(toPublicPublisher(publisher)).not.toHaveProperty("githubVerifiedByUserId"); + }); }); diff --git a/convex/lib/public.ts b/convex/lib/public.ts index 5f89aef3..53bd7429 100644 --- a/convex/lib/public.ts +++ b/convex/lib/public.ts @@ -9,7 +9,16 @@ export type PublicUser = Pick< export type PublicPublisher = Pick< Doc<"publishers">, - "_id" | "_creationTime" | "kind" | "handle" | "displayName" | "image" | "bio" | "linkedUserId" + | "_id" + | "_creationTime" + | "kind" + | "handle" + | "displayName" + | "image" + | "bio" + | "linkedUserId" + | "githubHandle" + | "githubVerifiedAt" > & { official?: boolean }; export type PublicSkillStats = { @@ -130,6 +139,8 @@ export function toPublicPublisher( image: publisher.image, bio: publisher.bio, linkedUserId: publisher.linkedUserId, + githubHandle: publisher.githubHandle, + githubVerifiedAt: publisher.githubVerifiedAt, ...(options?.official ? { official: true } : {}), }; } diff --git a/convex/lib/retentionPolicy.ts b/convex/lib/retentionPolicy.ts index e24afafa..c6ddcb84 100644 --- a/convex/lib/retentionPolicy.ts +++ b/convex/lib/retentionPolicy.ts @@ -66,6 +66,10 @@ export const RETENTION_POLICIES = { retention: "Convex Auth session total duration.", }), authAccounts: permanent("Provider account links for active users."), + githubOrgMemberships: derived( + "Active GitHub organization memberships synced during OAuth.", + "Reconnect the GitHub account with read:org access.", + ), authRefreshTokens: ephemeral("Convex Auth refresh tokens expire after inactive duration.", { expirationField: "expirationTime", expirationIndex: "by_expiration_time", diff --git a/convex/publishers.test.ts b/convex/publishers.test.ts index 761e265d..908a6bbf 100644 --- a/convex/publishers.test.ts +++ b/convex/publishers.test.ts @@ -377,6 +377,7 @@ const updateProfileHandler = ( image?: string; imageStorageId?: string; imageUploadTicket?: string; + githubOrgId?: string | null; }> )._handler; @@ -6354,6 +6355,138 @@ describe("publishers membership controls", () => { ); }); + it("links only a freshly verified GitHub organization membership", async () => { + vi.mocked(getAuthUserId).mockResolvedValue("users:admin" as never); + const publisher = { + _id: "publishers:org", + kind: "org", + handle: "cua", + displayName: "Cua", + image: undefined, + bio: undefined, + }; + const patch = vi.fn(async () => {}); + const ctx = { + db: { + get: vi.fn(async (id: string) => { + if (id === "users:admin") return { _id: id }; + if (id === "publishers:org") return publisher; + return null; + }), + query: vi.fn((table: string) => { + if (table === "publisherMembers") { + return { + withIndex: vi.fn(() => ({ + unique: vi.fn().mockResolvedValue({ + _id: "publisherMembers:admin", + publisherId: "publishers:org", + userId: "users:admin", + role: "admin", + }), + })), + }; + } + if (table === "githubOrgMemberships") { + return { + withIndex: vi.fn(() => ({ + unique: vi.fn().mockResolvedValue({ + userId: "users:admin", + githubOrgId: "42", + login: "trycua", + role: "member", + syncedAt: Date.now(), + }), + })), + }; + } + if (table === "officialPublishers") return emptyOfficialPublishersQuery(); + throw new Error(`unexpected table ${table}`); + }), + patch, + insert: vi.fn(async () => "auditLogs:1"), + delete: vi.fn(), + replace: vi.fn(), + normalizeId: vi.fn(), + }, + }; + + await updateProfileHandler(ctx as never, { + publisherId: "publishers:org", + displayName: "Cua", + githubOrgId: "42", + }); + + expect(patch).toHaveBeenCalledWith( + "publishers:org", + expect.objectContaining({ + githubHandle: "trycua", + githubOrgId: "42", + githubVerifiedAt: expect.any(Number), + githubVerifiedByUserId: "users:admin", + }), + ); + }); + + it("rejects stale GitHub organization membership verification", async () => { + vi.mocked(getAuthUserId).mockResolvedValue("users:admin" as never); + const ctx = { + db: { + get: vi.fn(async (id: string) => { + if (id === "users:admin") return { _id: id }; + if (id === "publishers:org") { + return { + _id: id, + kind: "org", + handle: "cua", + displayName: "Cua", + }; + } + return null; + }), + query: vi.fn((table: string) => { + if (table === "publisherMembers") { + return { + withIndex: vi.fn(() => ({ + unique: vi.fn().mockResolvedValue({ + publisherId: "publishers:org", + userId: "users:admin", + role: "admin", + }), + })), + }; + } + if (table === "githubOrgMemberships") { + return { + withIndex: vi.fn(() => ({ + unique: vi.fn().mockResolvedValue({ + userId: "users:admin", + githubOrgId: "42", + login: "trycua", + role: "member", + syncedAt: Date.now() - 16 * 60 * 1000, + }), + })), + }; + } + throw new Error(`unexpected table ${table}`); + }), + patch: vi.fn(), + insert: vi.fn(), + delete: vi.fn(), + replace: vi.fn(), + normalizeId: vi.fn(), + }, + }; + + await expect( + updateProfileHandler(ctx as never, { + publisherId: "publishers:org", + displayName: "Cua", + githubOrgId: "42", + }), + ).rejects.toThrow("Reconnect GitHub to verify your organization membership"); + }); + it("issues logo upload tickets only to org admins", async () => { vi.mocked(getAuthUserId).mockResolvedValue("users:admin" as never); const insert = vi.fn(async () => "publisherImageUploadTickets:1"); diff --git a/convex/publishers.ts b/convex/publishers.ts index 77a056fb..0bbfb099 100644 --- a/convex/publishers.ts +++ b/convex/publishers.ts @@ -5,6 +5,7 @@ import type { Doc, Id } from "./_generated/dataModel"; import type { MutationCtx, QueryCtx } from "./_generated/server"; import { action, internalMutation, internalQuery, mutation, query } from "./functions"; import { assertAdmin, getOptionalActiveAuthUserId, requireUser } from "./lib/access"; +import { GITHUB_ORG_MEMBERSHIP_VERIFICATION_MAX_AGE_MS } from "./lib/githubOrgMemberships"; import { isPublicSkillDoc } from "./lib/globalStats"; import { isOfficialPublisher, toPublicPublisherWithOfficial } from "./lib/officialPublishers"; import { extractPackageDigestFields, upsertPackageSearchDigest } from "./lib/packageSearchDigest"; @@ -2273,6 +2274,7 @@ export const listMine = query({ publisher: { ...(includePublishedItems ? publicPublisher : withoutPublishedItems(publicPublisher)), imageStorageId: publisher?.imageStorageId, + githubOrgId: publisher?.githubOrgId, }, role: publisher?.kind === "user" ? "owner" : membership.role, }; @@ -2296,6 +2298,7 @@ export const listMine = query({ publisher: { ...(includePublishedItems ? personalPublisher : withoutPublishedItems(personalPublisher)), imageStorageId: personalPublisherDoc?.imageStorageId, + githubOrgId: personalPublisherDoc?.githubOrgId, }, role: "owner", }); @@ -3095,6 +3098,7 @@ export const updateProfile = mutation({ image: v.optional(v.string()), imageStorageId: v.optional(v.id("_storage")), imageUploadTicket: v.optional(v.id("publisherImageUploadTickets")), + githubOrgId: v.optional(v.union(v.string(), v.null())), }, handler: async (ctx, args) => { const { userId } = await requireUser(ctx); @@ -3178,11 +3182,59 @@ export const updateProfile = mutation({ } const now = Date.now(); + let githubPatch: + | { + githubHandle: string; + githubOrgId: string; + githubVerifiedAt: number; + githubVerifiedByUserId: Id<"users">; + } + | { + githubHandle: undefined; + githubOrgId: undefined; + githubVerifiedAt: undefined; + githubVerifiedByUserId: undefined; + } + | undefined; + if (args.githubOrgId === null) { + githubPatch = { + githubHandle: undefined, + githubOrgId: undefined, + githubVerifiedAt: undefined, + githubVerifiedByUserId: undefined, + }; + } else if (args.githubOrgId !== undefined) { + const githubOrgId = args.githubOrgId.trim(); + if (!/^[1-9]\d*$/.test(githubOrgId)) { + throw new ConvexError("Select a GitHub organization from your connected account"); + } + const githubMembership = await ctx.db + .query("githubOrgMemberships") + .withIndex("by_user_and_github_org", (q) => + q.eq("userId", userId).eq("githubOrgId", githubOrgId), + ) + .unique(); + if ( + !githubMembership || + now - githubMembership.syncedAt > GITHUB_ORG_MEMBERSHIP_VERIFICATION_MAX_AGE_MS + ) { + throw new ConvexError("Reconnect GitHub to verify your organization membership"); + } + githubPatch = { + githubHandle: githubMembership.login, + githubOrgId: githubMembership.githubOrgId, + githubVerifiedAt: now, + githubVerifiedByUserId: userId, + }; + } + const nextGithubHandle = githubPatch ? githubPatch.githubHandle : publisher.githubHandle; + const nextGithubOrgId = githubPatch ? githubPatch.githubOrgId : publisher.githubOrgId; await ctx.db.patch(publisher._id, { displayName, bio, image: imageUrl, imageStorageId, + ...githubPatch, updatedAt: now, }); if ( @@ -3202,6 +3254,8 @@ export const updateProfile = mutation({ bio, image: imageUrl, imageStorageId, + githubHandle: nextGithubHandle, + githubOrgId: nextGithubOrgId, }, createdAt: now, }); diff --git a/convex/schema.ts b/convex/schema.ts index 8db4c1d4..c3f24ca4 100644 --- a/convex/schema.ts +++ b/convex/schema.ts @@ -173,6 +173,8 @@ const users = defineTable({ githubCreatedAt: v.optional(v.number()), githubFetchedAt: v.optional(v.number()), githubProfileSyncedAt: v.optional(v.number()), + githubOrgMembershipsSyncedAt: v.optional(v.number()), + githubOrgMembershipsTruncated: v.optional(v.boolean()), trustedPublisher: v.optional(v.boolean()), publishedSkills: v.optional(v.number()), totalStars: v.optional(v.number()), @@ -218,6 +220,10 @@ const publishers = defineTable({ bio: v.optional(v.string()), image: v.optional(v.string()), imageStorageId: v.optional(v.id("_storage")), + githubHandle: v.optional(v.string()), + githubOrgId: v.optional(v.string()), + githubVerifiedAt: v.optional(v.number()), + githubVerifiedByUserId: v.optional(v.id("users")), linkedUserId: v.optional(v.id("users")), trustedPublisher: v.optional(v.boolean()), publishedSkills: v.optional(v.number()), @@ -254,6 +260,17 @@ const publishers = defineTable({ "updatedAt", ]); +const githubOrgMemberships = defineTable({ + userId: v.id("users"), + githubOrgId: v.string(), + login: v.string(), + avatarUrl: v.optional(v.string()), + role: v.union(v.literal("admin"), v.literal("member")), + syncedAt: v.number(), +}) + .index("by_user", ["userId"]) + .index("by_user_and_github_org", ["userId", "githubOrgId"]); + const publisherMembers = defineTable({ publisherId: v.id("publishers"), userId: v.id("users"), @@ -3329,6 +3346,7 @@ export default defineSchema({ authRefreshTokens, users, publishers, + githubOrgMemberships, publisherMembers, publisherInvites, publisherImageUploadTickets, diff --git a/convex/users.test.ts b/convex/users.test.ts index 0ec7e5a6..ba85f51c 100644 --- a/convex/users.test.ts +++ b/convex/users.test.ts @@ -333,11 +333,21 @@ function makeDevPersonaCtx() { }), }; } + if (table === "githubOrgMemberships") { + return { + withIndex: vi.fn((name: string) => { + if (name !== "by_user") { + throw new Error(`Unexpected githubOrgMemberships index ${name}`); + } + return { collect: vi.fn(async () => []) }; + }), + }; + } throw new Error(`Unexpected table ${table}`); }); return { - ctx: { db: { patch, get, insert, query, normalizeId: vi.fn() } } as never, + ctx: { db: { patch, get, insert, query, delete: vi.fn(), normalizeId: vi.fn() } } as never, auditLogs, inserts, patches, @@ -1996,6 +2006,13 @@ describe("users profile audit logs", () => { }), }; } + if (table === "githubOrgMemberships") { + return { + withIndex: () => ({ + collect: vi.fn(async () => []), + }), + }; + } if (table === "authAccounts" || table === "authSessions") { return { withIndex: () => ({ diff --git a/convex/users.ts b/convex/users.ts index deda0936..aeffc597 100644 --- a/convex/users.ts +++ b/convex/users.ts @@ -73,6 +73,7 @@ type DeletedAccountCleanupResult = { authVerificationCodes: number; authSessions: number; authRefreshTokens: number; + githubOrgMemberships: number; apiTokens: number; personalPublisherDeleted: boolean; }; @@ -360,6 +361,11 @@ async function hardDeleteSelfDeletedAccountState( .withIndex("by_user", (q) => q.eq("userId", user._id)) .collect(); for (const token of tokens) await ctx.db.delete(token._id); + const githubOrgMemberships = await ctx.db + .query("githubOrgMemberships") + .withIndex("by_user", (q) => q.eq("userId", user._id)) + .collect(); + for (const membership of githubOrgMemberships) await ctx.db.delete(membership._id); const personalPublisher = user.personalPublisherId ? await ctx.db.get(user.personalPublisherId) @@ -405,7 +411,12 @@ async function hardDeleteSelfDeletedAccountState( }); await ctx.runMutation(internal.telemetry.clearUserTelemetryInternal, { userId: user._id }); const authState = await purgeAuthStateForUser(ctx, user._id); - return { ...authState, apiTokens: tokens.length, personalPublisherDeleted }; + return { + ...authState, + githubOrgMemberships: githubOrgMemberships.length, + apiTokens: tokens.length, + personalPublisherDeleted, + }; } async function scrubDeletedUserTombstone(ctx: MutationCtx, user: Doc<"users">, deletedAt: number) { @@ -426,6 +437,8 @@ async function scrubDeletedUserTombstone(ctx: MutationCtx, user: Doc<"users">, d isAnonymous: undefined, bio: undefined, githubCreatedAt: undefined, + githubOrgMembershipsSyncedAt: undefined, + githubOrgMembershipsTruncated: undefined, updatedAt: deletedAt, }); } @@ -463,6 +476,20 @@ const DEV_OFFICIAL_ORG = { displayName: "Local Official Org", reason: "dev-persona.official-org-member", } as const; +const DEV_GITHUB_ORGS = [ + { + githubOrgId: "100000001", + login: "openclaw", + avatarUrl: "https://avatars.githubusercontent.com/u/188567264?v=4", + role: "admin" as const, + }, + { + githubOrgId: "100000002", + login: "trycua", + avatarUrl: "https://avatars.githubusercontent.com/u/175698028?v=4", + role: "member" as const, + }, +]; type DevPersona = keyof typeof DEV_PERSONAS; @@ -540,11 +567,35 @@ export const upsertDevPersonaInternal = internalMutation({ }); if (args.persona === "officialOrgMember") { await ensureDevOfficialOrgMembership(ctx, user, now); + await replaceDevGitHubOrgMemberships(ctx, user._id, now); } return userId; }, }); +async function replaceDevGitHubOrgMemberships( + ctx: MutationCtx, + userId: Id<"users">, + syncedAt: number, +) { + const existing = await ctx.db + .query("githubOrgMemberships") + .withIndex("by_user", (q) => q.eq("userId", userId)) + .collect(); + for (const membership of existing) await ctx.db.delete(membership._id); + for (const membership of DEV_GITHUB_ORGS) { + await ctx.db.insert("githubOrgMemberships", { + userId, + ...membership, + syncedAt, + }); + } + await ctx.db.patch(userId, { + githubOrgMembershipsSyncedAt: syncedAt, + githubOrgMembershipsTruncated: undefined, + }); +} + async function ensureDevOfficialOrgMembership(ctx: MutationCtx, user: Doc<"users">, now: number) { let publisher = await getPublisherByHandle(ctx, DEV_OFFICIAL_ORG.handle); let publisherId = publisher?._id; @@ -1071,6 +1122,8 @@ export const deleteAccount = mutation({ isAnonymous: undefined, bio: undefined, githubCreatedAt: undefined, + githubOrgMembershipsSyncedAt: undefined, + githubOrgMembershipsTruncated: undefined, updatedAt: now, }); await ctx.db.insert("auditLogs", { diff --git a/e2e/local-auth/helpers.ts b/e2e/local-auth/helpers.ts index de2270e8..f9824709 100644 --- a/e2e/local-auth/helpers.ts +++ b/e2e/local-auth/helpers.ts @@ -27,6 +27,8 @@ const FINGERPRINT_SALT_LINES = [ "### Local browser release evidence and storage handoff notes", ] as const; +type LocalAuthPersona = DevPersona | "officialOrgMember"; + function hashFixtureInput(value: string) { let hash = 0; for (const char of value) { @@ -273,15 +275,17 @@ export async function completeMockPrePublicationChecks(args: { return { ...completion, claim }; } -function devPersonaHeaderPattern(persona: DevPersona, expectedHandle: string) { +function devPersonaHeaderPattern(persona: LocalAuthPersona, expectedHandle: string) { const displayName = persona === "owner" ? "Local Owner" : persona === "user" ? "Local User" - : persona === "abusePublisher" - ? "Local Abuse Test Publisher" - : "Local Admin"; + : persona === "officialOrgMember" + ? "Local Official Org Member" + : persona === "abusePublisher" + ? "Local Abuse Test Publisher" + : "Local Admin"; const displayNamePattern = persona === "abusePublisher" ? `${escapeRegExp("Local Abuse Test Publishe")}.*` @@ -293,8 +297,9 @@ function devPersonaHeaderPattern(persona: DevPersona, expectedHandle: string) { return new RegExp(`@(?:${exactHandle}|${displayNamePattern})`, "i"); } -function devPersonaMenuLabel(persona: DevPersona) { +function devPersonaMenuLabel(persona: LocalAuthPersona) { if (persona === "abusePublisher") return "abuse publisher"; + if (persona === "officialOrgMember") return "org member"; return persona; } @@ -309,12 +314,14 @@ function parseSkillDetailPath(pathname: string) { throw new Error(`Expected skill detail path, received ${pathname}`); } -function devPersonaHandle(persona: DevPersona) { +function devPersonaHandle(persona: LocalAuthPersona) { return persona === "owner" ? "local" - : persona === "abusePublisher" - ? "local-abuse" - : `local-${persona}`; + : persona === "officialOrgMember" + ? "local-official-member" + : persona === "abusePublisher" + ? "local-abuse" + : `local-${persona}`; } export { @@ -358,20 +365,15 @@ export function escapeRegExp(value: string) { return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } -export async function expectLocalPersonaActive(page: Page, persona: DevPersona) { - const expectedHandle = - persona === "owner" - ? "local" - : persona === "abusePublisher" - ? "local-abuse" - : `local-${persona}`; +export async function expectLocalPersonaActive(page: Page, persona: LocalAuthPersona) { + const expectedHandle = devPersonaHandle(persona); await expect(page.locator("header .user-trigger")).toContainText( devPersonaHeaderPattern(persona, expectedHandle), { timeout: 15_000 }, ); } -export async function signInAsLocalPersona(page: Page, persona: DevPersona) { +export async function signInAsLocalPersona(page: Page, persona: LocalAuthPersona) { let lastError: unknown; for (let attempt = 1; attempt <= 3; attempt += 1) { try { @@ -512,7 +514,7 @@ async function waitForPublishSkillMetadataForm(page: Page) { await page.getByTestId("upload-input").waitFor({ state: "attached", timeout: 15_000 }); } -export async function signInAsLocalPublisher(page: Page, persona: DevPersona) { +export async function signInAsLocalPublisher(page: Page, persona: LocalAuthPersona) { await signInAsLocalPersona(page, persona); await page.goto("/skills/publish", { waitUntil: "domcontentloaded" }); await waitForPublishSkillForm(page); diff --git a/e2e/local-auth/publisher-github-profile.pw.test.ts b/e2e/local-auth/publisher-github-profile.pw.test.ts new file mode 100644 index 00000000..9229f177 --- /dev/null +++ b/e2e/local-auth/publisher-github-profile.pw.test.ts @@ -0,0 +1,49 @@ +import { expect, test } from "@playwright/test"; +import { expectHealthyPage, trackRuntimeErrors, waitForHydration } from "../helpers/runtimeErrors"; +import { signInAsLocalPersona } from "./helpers"; + +test.skip( + process.env.VITE_ENABLE_DEV_AUTH !== "1", + "publisher GitHub profile tests require the local dev auth runner", +); + +test.setTimeout(180_000); + +test("org admins can link a verified GitHub organization to the public profile", async ({ + page, +}, testInfo) => { + const errors = trackRuntimeErrors(page); + + await signInAsLocalPersona(page, "officialOrgMember"); + errors.length = 0; + + await page.goto("/settings?view=organizations", { waitUntil: "domcontentloaded" }); + await waitForHydration(page); + + const githubOrgSelect = page.getByRole("combobox", { name: "GitHub organization" }); + await expect(githubOrgSelect).toBeVisible({ timeout: 30_000 }); + + if ((await githubOrgSelect.textContent())?.includes("@trycua")) { + await githubOrgSelect.click(); + await page.getByRole("option", { name: "No GitHub organization" }).click(); + await page.getByRole("button", { name: "Save changes" }).click(); + await expect(page.getByText("Organization updated", { exact: true })).toBeVisible(); + } + + await githubOrgSelect.click(); + await page.getByRole("option", { name: "@trycua · member" }).click(); + await page.getByRole("button", { name: "Save changes" }).click(); + await expect(page.getByText("Organization updated", { exact: true })).toBeVisible(); + + await page.goto("/local-official-org", { waitUntil: "domcontentloaded" }); + await waitForHydration(page); + + const githubLink = page.getByRole("link", { name: "GitHub · @trycua" }); + await expect(githubLink).toHaveAttribute("href", "https://github.com/trycua"); + await page.screenshot({ + path: testInfo.outputPath("publisher-github-profile.png"), + fullPage: true, + }); + + await expectHealthyPage(page, errors); +}); diff --git a/specs/auth-identity.md b/specs/auth-identity.md index 33e31a4c..dd0256eb 100644 --- a/specs/auth-identity.md +++ b/specs/auth-identity.md @@ -40,6 +40,20 @@ derive the actor server-side from Convex Auth (`getAuthUserId` via `requireUser`/`getOptionalActiveAuthUserId`). They must not accept client-supplied user ids, usernames, handles, or emails for authorization. +## Organization GitHub profile verification + +Organization publisher GitHub links are profile metadata, not ClawHub account +identity or artifact provenance. GitHub OAuth requests `read:org` and uses the +access token only during the OAuth callback to fetch the signed-in user's active +organization memberships. ClawHub stores the resulting immutable GitHub +organization ids and current logins, but never stores the provider access token. + +Changing an organization publisher's GitHub link requires both ClawHub +owner/admin access and a fresh server-side GitHub membership snapshot for the +selected immutable organization id. The stored login may determine the public +profile URL, but it must not grant publishing authority, Official status, +trusted-publishing authority, or source/artifact provenance. + Staff recovery for a personal publisher whose GitHub principal is no longer accessible must not rewrite or merge Convex Auth `authAccounts` rows. The only supported permanent recovery path is an admin-only personal publisher recovery diff --git a/src/__tests__/user-profile-route.test.tsx b/src/__tests__/user-profile-route.test.tsx index 7077ecbd..1c4fb8b3 100644 --- a/src/__tests__/user-profile-route.test.tsx +++ b/src/__tests__/user-profile-route.test.tsx @@ -70,6 +70,7 @@ const publisher = { handle: "nvidia", image: null, kind: "org" as const, + githubHandle: "nvidia", official: true, publishedItems: [], stats: { @@ -127,6 +128,24 @@ describe("user profile route", () => { expect(screen.getByRole("link", { name: /github/i })).toBeTruthy(); }); + it("does not guess a GitHub profile for an unlinked organization", async () => { + const unlinkedPublisher = { ...publisher, githubHandle: undefined }; + loaderDataMock.mockReturnValue({ publisher: unlinkedPublisher }); + queryMock.mockImplementation((_query, args: Record | "skip") => { + if (args === "skip") return undefined; + if ("publisherHandle" in args) return { publisher: unlinkedPublisher, members: [] }; + if ("kind" in args) return null; + return unlinkedPublisher; + }); + + const route = await loadRoute(); + const Component = route.__config.component as ComponentType; + + render(); + + expect(screen.queryByRole("link", { name: /github/i })).toBeNull(); + }); + it("shows edit profile instead of report on the viewer's own publisher page", async () => { const personalPublisher = { ...publisher, diff --git a/src/lib/publicUser.ts b/src/lib/publicUser.ts index 82986165..e93b8335 100644 --- a/src/lib/publicUser.ts +++ b/src/lib/publicUser.ts @@ -7,7 +7,16 @@ export type PublicUser = Pick< export type PublicPublisher = Pick< Doc<"publishers">, - "_id" | "_creationTime" | "kind" | "handle" | "displayName" | "image" | "bio" | "linkedUserId" + | "_id" + | "_creationTime" + | "kind" + | "handle" + | "displayName" + | "image" + | "bio" + | "linkedUserId" + | "githubHandle" + | "githubVerifiedAt" > & { official?: boolean }; type PublicPublisherStats = { diff --git a/src/routes/-settings.test.tsx b/src/routes/-settings.test.tsx index fadc16b3..80b9befc 100644 --- a/src/routes/-settings.test.tsx +++ b/src/routes/-settings.test.tsx @@ -65,6 +65,8 @@ const orgMembership = { kind: "org", image: null, bio: "OpenClaw publisher", + githubHandle: null as string | null, + githubOrgId: null as string | null, official: true, }, role: "owner", @@ -78,6 +80,8 @@ const personalMembership = { kind: "user", image: null, bio: null, + githubHandle: null as string | null, + githubOrgId: null as string | null, official: false, }, role: "owner", @@ -136,6 +140,11 @@ function mockSignedInSettings({ githubSources = [], pendingInvites = [], myInvites = [], + githubOrgMemberships = { + syncedAt: null, + truncated: false, + memberships: [], + }, membersLoading = false, deletionInventoryLoading = false, }: { @@ -146,6 +155,17 @@ function mockSignedInSettings({ deletionInventoryLoading?: boolean; pendingInvites?: PublisherInviteFixture[]; myInvites?: PublisherInviteFixture[]; + githubOrgMemberships?: { + syncedAt: number | null; + truncated: boolean; + memberships: Array<{ + githubOrgId: string; + login: string; + avatarUrl: string | null; + role: "admin" | "member"; + syncedAt: number; + }>; + }; githubSources?: Array<{ _id: string; repo: string; @@ -198,6 +218,7 @@ function mockSignedInSettings({ if (args === "skip") return undefined; if (queryName === "tokens:listMine") return []; if (queryName === "publishers:listMine") return memberships; + if (queryName === "githubOrgMemberships:listMine") return githubOrgMemberships; if (queryName === "publishers:getDeletionInventory") { return deletionInventoryLoading ? undefined : []; } @@ -223,6 +244,7 @@ function getLastQueryArgs(functionName: string) { describe("Settings", () => { beforeEach(() => { + Element.prototype.scrollIntoView = vi.fn(); window.history.replaceState(null, "", "/settings"); useQueryMock.mockReset(); useMutationMock.mockReset(); @@ -316,6 +338,199 @@ describe("Settings", () => { }); }); + it("connects GitHub from organization settings when memberships are unavailable", () => { + const signIn = vi.fn().mockResolvedValue(undefined); + useAuthActionsMock.mockReturnValue({ + signIn, + signOut: vi.fn().mockResolvedValue(undefined), + }); + mockSignedInSettings({ search: { view: "organizations" } }); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Connect GitHub organizations" })); + + expect(signIn).toHaveBeenCalledWith("github", { + redirectTo: "/settings?view=organizations&ownerHandle=openclaw", + }); + }); + + it("selects a verified GitHub organization and saves its immutable id", async () => { + const updateOrgProfile = vi.fn().mockResolvedValue({ ok: true }); + useMutationMock.mockImplementation((mutation) => + getFunctionName(mutation) === "publishers:updateProfile" ? updateOrgProfile : vi.fn(), + ); + const syncedAt = Date.now(); + mockSignedInSettings({ + search: { view: "organizations" }, + githubOrgMemberships: { + syncedAt, + truncated: false, + memberships: [ + { + githubOrgId: "42", + login: "trycua", + avatarUrl: null, + role: "member", + syncedAt, + }, + ], + }, + }); + + render(); + + fireEvent.click(screen.getByLabelText("GitHub organization")); + fireEvent.click(await screen.findByText("@trycua · member")); + fireEvent.click(screen.getByRole("button", { name: "Save changes" })); + + await waitFor(() => { + expect(updateOrgProfile).toHaveBeenCalledWith( + expect.objectContaining({ + publisherId: "publisher_openclaw", + githubOrgId: "42", + }), + ); + }); + }); + + it("refreshes a linked GitHub organization handle after a rename", async () => { + const updateOrgProfile = vi.fn().mockResolvedValue({ ok: true }); + useMutationMock.mockImplementation((mutation) => + getFunctionName(mutation) === "publishers:updateProfile" ? updateOrgProfile : vi.fn(), + ); + const syncedAt = Date.now(); + mockSignedInSettings({ + search: { view: "organizations" }, + memberships: [ + { + ...orgMembership, + publisher: { + ...orgMembership.publisher, + githubHandle: "old-cua", + githubOrgId: "42", + }, + }, + ], + githubOrgMemberships: { + syncedAt, + truncated: false, + memberships: [ + { + githubOrgId: "42", + login: "trycua", + avatarUrl: null, + role: "member", + syncedAt, + }, + ], + }, + }); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Save changes" })); + + await waitFor(() => { + expect(updateOrgProfile).toHaveBeenCalledWith( + expect.objectContaining({ + publisherId: "publisher_openclaw", + githubOrgId: "42", + }), + ); + }); + }); + + it("saves unrelated profile changes when the linked GitHub organization is unavailable", async () => { + const updateOrgProfile = vi.fn().mockResolvedValue({ ok: true }); + useMutationMock.mockImplementation((mutation) => + getFunctionName(mutation) === "publishers:updateProfile" ? updateOrgProfile : vi.fn(), + ); + mockSignedInSettings({ + search: { view: "organizations" }, + memberships: [ + { + ...orgMembership, + publisher: { + ...orgMembership.publisher, + githubHandle: "trycua", + githubOrgId: "42", + }, + }, + ], + githubOrgMemberships: { + syncedAt: Date.now(), + truncated: false, + memberships: [], + }, + }); + + render(); + + fireEvent.change(screen.getByLabelText("Display name"), { + target: { value: "Renamed publisher" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Save changes" })); + + await waitFor(() => { + expect(updateOrgProfile).toHaveBeenCalledWith( + expect.objectContaining({ + displayName: "Renamed publisher", + githubOrgId: undefined, + }), + ); + }); + }); + + it("allows removing a GitHub organization after membership verification expires", async () => { + const updateOrgProfile = vi.fn().mockResolvedValue({ ok: true }); + useMutationMock.mockImplementation((mutation) => + getFunctionName(mutation) === "publishers:updateProfile" ? updateOrgProfile : vi.fn(), + ); + const staleSyncedAt = Date.now() - 16 * 60 * 1000; + mockSignedInSettings({ + search: { view: "organizations" }, + memberships: [ + { + ...orgMembership, + publisher: { + ...orgMembership.publisher, + githubHandle: "trycua", + githubOrgId: "42", + }, + }, + ], + githubOrgMemberships: { + syncedAt: staleSyncedAt, + truncated: false, + memberships: [ + { + githubOrgId: "42", + login: "trycua", + avatarUrl: null, + role: "member", + syncedAt: staleSyncedAt, + }, + ], + }, + }); + + render(); + + fireEvent.click(screen.getByLabelText("GitHub organization")); + fireEvent.click(await screen.findByText("No GitHub organization")); + fireEvent.click(screen.getByRole("button", { name: "Save changes" })); + + await waitFor(() => { + expect(updateOrgProfile).toHaveBeenCalledWith( + expect.objectContaining({ + publisherId: "publisher_openclaw", + githubOrgId: null, + }), + ); + }); + }); + it("lets organization owners confirm org deletion", async () => { const deleteOrg = vi.fn().mockResolvedValue({ deleted: true }); useMutationMock.mockReturnValue(deleteOrg); diff --git a/src/routes/settings.tsx b/src/routes/settings.tsx index 8446ec75..25e9112f 100644 --- a/src/routes/settings.tsx +++ b/src/routes/settings.tsx @@ -16,6 +16,7 @@ import { Package, Palette, Plus, + RefreshCw, Save, Send, ShieldAlert, @@ -113,6 +114,9 @@ type PublisherMembership = { image?: string | null; imageStorageId?: Id<"_storage"> | null; bio?: string | null; + githubHandle?: string | null; + githubOrgId?: string | null; + githubVerifiedAt?: number | null; official?: boolean; stats?: { skills: number; @@ -125,6 +129,18 @@ type PublisherMembership = { role: "owner" | "admin" | "publisher"; }; +type GitHubOrgMembershipsResult = { + syncedAt: number | null; + truncated: boolean; + memberships: Array<{ + githubOrgId: string; + login: string; + avatarUrl: string | null; + role: "admin" | "member"; + syncedAt: number; + }>; +}; + type PublisherDeletionInventory = { handle: string; stats: { @@ -253,7 +269,7 @@ const themeToggleItemClass = export function Settings() { const navigate = useNavigate(); - const { signOut } = useAuthActions(); + const { signIn, signOut } = useAuthActions(); const { isAuthenticated, isLoading: isAuthLoading, me } = useAuthStatus(); const updateProfile = useMutation(api.users.updateProfile); const deleteAccount = useMutation(api.users.deleteAccount); @@ -269,6 +285,10 @@ export function Settings() { api.publishers.listMine, shouldLoadAccountScopedQueries ? { includePublishedItems: false } : "skip", ) as Array | undefined; + const githubOrgMemberships = useQuery( + api.githubOrgMemberships.listMine, + shouldLoadAccountScopedQueries ? {} : "skip", + ) as GitHubOrgMembershipsResult | undefined; const createOrg = useMutation(api.publishers.createOrg); const deleteOrg = useMutation(api.publishers.deleteOrg); const createOrgImageUpload = useMutation(api.publishers.createImageUpload); @@ -295,6 +315,7 @@ export function Settings() { const [selectedOrgImage, setSelectedOrgImage] = useState(""); const [selectedOrgImageFile, setSelectedOrgImageFile] = useState(null); const [selectedOrgImagePreview, setSelectedOrgImagePreview] = useState(null); + const [selectedGitHubOrgId, setSelectedGitHubOrgId] = useState(""); const [isUploadingOrgImage, setIsUploadingOrgImage] = useState(false); const [selectedSourcePublisherId, setSelectedSourcePublisherId] = useState(""); const [githubRepo, setGithubRepo] = useState(""); @@ -334,10 +355,25 @@ export function Settings() { null; const selectedOrg = orgs.find((entry) => entry.publisher.handle === selectedOrgHandle) ?? orgs[0] ?? null; + const githubOrgMembershipsFresh = Boolean( + githubOrgMemberships?.syncedAt && Date.now() - githubOrgMemberships.syncedAt <= 15 * 60 * 1000, + ); + const selectedGitHubOrgMembership = githubOrgMemberships?.memberships.find( + (membership) => membership.githubOrgId === selectedGitHubOrgId, + ); + const linkedGitHubHandleNeedsRefresh = Boolean( + githubOrgMembershipsFresh && + selectedGitHubOrgId && + selectedGitHubOrgId === selectedOrg?.publisher.githubOrgId && + selectedGitHubOrgMembership && + selectedGitHubOrgMembership?.login !== selectedOrg.publisher.githubHandle, + ); const hasOrgProfileChanges = selectedOrg ? selectedOrgDisplayName !== (selectedOrg.publisher.displayName ?? "") || selectedOrgBio !== (selectedOrg.publisher.bio ?? "") || selectedOrgImage !== (selectedOrg.publisher.image ?? "") || + selectedGitHubOrgId !== (selectedOrg.publisher.githubOrgId ?? "") || + linkedGitHubHandleNeedsRefresh || selectedOrgImageFile !== null : false; const hasProfileChanges = me @@ -427,12 +463,14 @@ export function Settings() { setSelectedOrgDisplayName(""); setSelectedOrgBio(""); setSelectedOrgImage(""); + setSelectedGitHubOrgId(""); setSelectedOrgImageFile(null); return; } setSelectedOrgDisplayName(selectedOrg.publisher.displayName ?? ""); setSelectedOrgBio(selectedOrg.publisher.bio ?? ""); setSelectedOrgImage(selectedOrg.publisher.image ?? ""); + setSelectedGitHubOrgId(selectedOrg.publisher.githubOrgId ?? ""); setSelectedOrgImageFile(null); }, [selectedOrg]); @@ -466,6 +504,7 @@ export function Settings() { const activeSectionLoading = (activeView === "organizations" && (publisherMemberships === undefined || + githubOrgMemberships === undefined || (selectedOrg && selectedOrg.role !== "publisher" && orgMembers === undefined))) || (activeView === "tokens" && tokens === undefined); @@ -535,6 +574,11 @@ export function Settings() { if (!selectedOrg) return; setIsUploadingOrgImage(true); try { + const githubOrgId = + selectedGitHubOrgId === (selectedOrg.publisher.githubOrgId ?? "") && + !linkedGitHubHandleNeedsRefresh + ? undefined + : selectedGitHubOrgId || null; if (selectedOrgImageFile) { const upload = await createOrgImageUpload({ publisherId: selectedOrg.publisher._id, @@ -546,6 +590,7 @@ export function Settings() { bio: selectedOrgBio || undefined, imageStorageId: imageStorageId as Id<"_storage">, imageUploadTicket: upload.uploadTicket, + githubOrgId, }); } else { await updateOrgProfile({ @@ -556,6 +601,7 @@ export function Settings() { imageStorageId: selectedOrgImage ? (selectedOrg.publisher.imageStorageId ?? undefined) : undefined, + githubOrgId, }); } setSelectedOrgImageFile(null); @@ -567,6 +613,13 @@ export function Settings() { } } + async function onConnectGitHubOrganizations() { + const ownerHandle = selectedOrg?.publisher.handle; + const search = new URLSearchParams({ view: "organizations" }); + if (ownerHandle) search.set("ownerHandle", ownerHandle); + await signIn("github", { redirectTo: `/settings?${search.toString()}` }); + } + function onOrgImageFileChange(event: ChangeEvent) { const file = event.target.files?.[0] ?? null; event.target.value = ""; @@ -1104,6 +1157,105 @@ export function Settings() { ) : null} +
+ + {githubOrgMemberships?.syncedAt || + selectedOrg.publisher.githubOrgId ? ( + <> +
+ + +
+

+ {githubOrgMembershipsFresh + ? "Only organizations where your GitHub account is an active member are shown." + : "Reconnect GitHub to choose another organization. You can still remove the current link."} +

+ + ) : ( +
+ +

+ GitHub will ask for read-only access to your organization + memberships. +

+
+ )} + +