Skip to content

Commit af3d01c

Browse files
feat: verify organization GitHub profiles (#3169)
1 parent da965d6 commit af3d01c

23 files changed

Lines changed: 1307 additions & 50 deletions

.github/workflows/ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ jobs:
163163
specs: |
164164
e2e/local-auth/header-profile-link.pw.test.ts
165165
e2e/local-auth/manage-context-proof.pw.test.ts
166+
e2e/local-auth/publisher-github-profile.pw.test.ts
166167
- name: moderation-malicious
167168
specs: e2e/local-auth/malicious-skill-ban-flow.pw.test.ts
168169
- name: star-sync

convex/_generated/api.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import type * as functions from "../functions.js";
2727
import type * as githubAccountAgeBackfill from "../githubAccountAgeBackfill.js";
2828
import type * as githubIdentity from "../githubIdentity.js";
2929
import type * as githubImport from "../githubImport.js";
30+
import type * as githubOrgMemberships from "../githubOrgMemberships.js";
3031
import type * as githubSkillSources from "../githubSkillSources.js";
3132
import type * as githubSkillSync from "../githubSkillSync.js";
3233
import type * as githubSkillSyncNode from "../githubSkillSyncNode.js";
@@ -70,6 +71,7 @@ import type * as lib_githubAuth from "../lib/githubAuth.js";
7071
import type * as lib_githubHandoff from "../lib/githubHandoff.js";
7172
import type * as lib_githubIdentity from "../lib/githubIdentity.js";
7273
import type * as lib_githubImport from "../lib/githubImport.js";
74+
import type * as lib_githubOrgMemberships from "../lib/githubOrgMemberships.js";
7375
import type * as lib_githubProfileSync from "../lib/githubProfileSync.js";
7476
import type * as lib_githubSkillScans from "../lib/githubSkillScans.js";
7577
import type * as lib_githubSkillSync from "../lib/githubSkillSync.js";
@@ -194,6 +196,7 @@ declare const fullApi: ApiFromModules<{
194196
githubAccountAgeBackfill: typeof githubAccountAgeBackfill;
195197
githubIdentity: typeof githubIdentity;
196198
githubImport: typeof githubImport;
199+
githubOrgMemberships: typeof githubOrgMemberships;
197200
githubSkillSources: typeof githubSkillSources;
198201
githubSkillSync: typeof githubSkillSync;
199202
githubSkillSyncNode: typeof githubSkillSyncNode;
@@ -237,6 +240,7 @@ declare const fullApi: ApiFromModules<{
237240
"lib/githubHandoff": typeof lib_githubHandoff;
238241
"lib/githubIdentity": typeof lib_githubIdentity;
239242
"lib/githubImport": typeof lib_githubImport;
243+
"lib/githubOrgMemberships": typeof lib_githubOrgMemberships;
240244
"lib/githubProfileSync": typeof lib_githubProfileSync;
241245
"lib/githubSkillScans": typeof lib_githubSkillScans;
242246
"lib/githubSkillSync": typeof lib_githubSkillSync;

convex/auth.test.ts

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,14 @@ describe("handleDeletedUserSignIn", () => {
156156
});
157157

158158
describe("GitHub auth provider", () => {
159+
it("requests read-only GitHub organization membership access", () => {
160+
const provider = createGitHubAuthProvider() as {
161+
options?: { authorization?: { params?: { scope?: string } } };
162+
};
163+
164+
expect(provider.options?.authorization?.params?.scope?.split(" ")).toContain("read:org");
165+
});
166+
159167
it("does not link ClawHub accounts by GitHub profile email", () => {
160168
const provider = createGitHubAuthProvider() as {
161169
options?: { allowDangerousEmailAccountLinking?: boolean };
@@ -181,19 +189,74 @@ describe("GitHub auth provider", () => {
181189
);
182190
});
183191

184-
it("fails closed when the GitHub provider receives a malformed profile", () => {
192+
it("fails closed when the GitHub provider receives a malformed profile", async () => {
185193
const provider = createGitHubAuthProvider() as {
186-
options?: { profile?: (profile: Record<string, unknown>) => Record<string, unknown> };
194+
options?: {
195+
profile?: (
196+
profile: Record<string, unknown>,
197+
tokens: { access_token?: string },
198+
) => Promise<Record<string, unknown>>;
199+
};
187200
};
188201

189-
expect(() => provider.options?.profile?.({ message: "Bad credentials" })).toThrow(
202+
await expect(provider.options?.profile?.({ message: "Bad credentials" }, {})).rejects.toThrow(
190203
"GitHub OAuth profile is missing a valid numeric id",
191204
);
192-
expect(provider.options?.profile?.({ id: 123456, login: "fixture-user" })).toEqual({
205+
await expect(
206+
provider.options?.profile?.({ id: 123456, login: "fixture-user" }, {}),
207+
).resolves.toEqual({
208+
id: "123456",
209+
name: "fixture-user",
210+
email: undefined,
211+
image: undefined,
212+
});
213+
});
214+
215+
it("adds a verified GitHub organization snapshot to the OAuth profile", async () => {
216+
const fetchMock = vi.spyOn(globalThis, "fetch").mockResolvedValueOnce(
217+
new Response(
218+
JSON.stringify([
219+
{
220+
state: "active",
221+
role: "member",
222+
organization: { id: 42, login: "trycua" },
223+
},
224+
]),
225+
{ status: 200 },
226+
),
227+
);
228+
const provider = createGitHubAuthProvider() as {
229+
options?: {
230+
profile?: (
231+
profile: Record<string, unknown>,
232+
tokens: { access_token?: string },
233+
) => Promise<Record<string, unknown>>;
234+
};
235+
};
236+
237+
await expect(
238+
provider.options?.profile?.(
239+
{ id: 123456, login: "fixture-user" },
240+
{ access_token: "test-token-placeholder" },
241+
),
242+
).resolves.toEqual({
193243
id: "123456",
194244
name: "fixture-user",
195245
email: undefined,
196246
image: undefined,
247+
githubOrgMembershipSync: {
248+
memberships: [
249+
{
250+
githubOrgId: "42",
251+
login: "trycua",
252+
avatarUrl: undefined,
253+
role: "member",
254+
},
255+
],
256+
syncedAt: expect.any(Number),
257+
truncated: false,
258+
},
197259
});
260+
fetchMock.mockRestore();
198261
});
199262
});

convex/auth.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@ import { ConvexError } from "convex/values";
66
import { internal } from "./_generated/api";
77
import type { DataModel, Id } from "./_generated/dataModel";
88
import { isLocalDevAuthEnabled } from "./lib/devAuth";
9+
import {
10+
GITHUB_ORG_MEMBERSHIP_SYNC_PROFILE_KEY,
11+
fetchActiveGitHubOrgMemberships,
12+
readGitHubOrgMembershipSync,
13+
replaceGitHubOrgMemberships,
14+
} from "./lib/githubOrgMemberships";
915
import { shouldScheduleGitHubProfileSync } from "./lib/githubProfileSync";
1016

1117
export const BANNED_REAUTH_MESSAGE =
@@ -39,15 +45,34 @@ export function createGitHubAuthProvider() {
3945
return GitHub({
4046
clientId: process.env.AUTH_GITHUB_ID ?? "",
4147
clientSecret: process.env.AUTH_GITHUB_SECRET ?? "",
48+
authorization: {
49+
params: { scope: "read:user user:email read:org" },
50+
},
4251
// GitHub's OAuth email must not be treated as a ClawHub account key. The
4352
// immutable GitHub provider account id is the only account-linking key.
4453
allowDangerousEmailAccountLinking: false,
45-
profile(profile) {
54+
async profile(profile, tokens) {
55+
let githubOrgMembershipSync;
56+
const accessToken = tokens.access_token?.trim();
57+
if (accessToken) {
58+
try {
59+
githubOrgMembershipSync = await fetchActiveGitHubOrgMemberships(accessToken);
60+
} catch (error) {
61+
console.warn(
62+
`[auth] GitHub organization membership sync failed: ${
63+
error instanceof Error ? error.message : String(error)
64+
}`,
65+
);
66+
}
67+
}
4668
return {
4769
id: normalizeGitHubProfileId(profile.id),
4870
name: profile.login,
4971
email: profile.email ?? undefined,
5072
image: profile.avatar_url,
73+
...(githubOrgMembershipSync
74+
? { [GITHUB_ORG_MEMBERSHIP_SYNC_PROFILE_KEY]: githubOrgMembershipSync }
75+
: {}),
5176
};
5277
},
5378
});
@@ -120,6 +145,7 @@ function userDataFromAuthProfile(args: {
120145
const {
121146
emailVerified: profileEmailVerified,
122147
phoneVerified: profilePhoneVerified,
148+
[GITHUB_ORG_MEMBERSHIP_SYNC_PROFILE_KEY]: _githubOrgMembershipSync,
123149
...profile
124150
} = args.profile;
125151
const emailVerified =
@@ -187,18 +213,25 @@ export const { auth, signIn, signOut, store, isAuthenticated } = convexAuth({
187213
*/
188214
async createOrUpdateUser(ctx, args) {
189215
const userData = userDataFromAuthProfile(args);
216+
const githubOrgMembershipSync = readGitHubOrgMembershipSync(args.profile);
190217
if (args.existingUserId !== null) {
191218
const userId = args.existingUserId as Id<"users">;
192219
const existingUser = await ctx.db.get(userId);
193220
if (existingUser?.deletedAt || existingUser?.deactivatedAt) {
194221
return userId;
195222
}
196223
await ctx.db.patch(userId, userData);
224+
if (githubOrgMembershipSync) {
225+
await replaceGitHubOrgMemberships(ctx, userId, githubOrgMembershipSync);
226+
}
197227
await schedulePostUserCreatedOrUpdated(ctx, userId, existingUser);
198228
return userId;
199229
}
200230

201231
const userId = await ctx.db.insert("users", userData);
232+
if (githubOrgMembershipSync) {
233+
await replaceGitHubOrgMemberships(ctx, userId, githubOrgMembershipSync);
234+
}
202235
const user = await ctx.db.get(userId);
203236
await schedulePostUserCreatedOrUpdated(ctx, userId, user);
204237
return userId;

convex/githubOrgMemberships.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { query } from "./functions";
2+
import { getOptionalActiveAuthUserId } from "./lib/access";
3+
4+
export const listMine = query({
5+
args: {},
6+
handler: async (ctx) => {
7+
const userId = await getOptionalActiveAuthUserId(ctx);
8+
if (!userId) {
9+
return { syncedAt: null, truncated: false, memberships: [] };
10+
}
11+
const user = await ctx.db.get(userId);
12+
if (!user || user.deletedAt || user.deactivatedAt) {
13+
return { syncedAt: null, truncated: false, memberships: [] };
14+
}
15+
16+
const memberships = await ctx.db
17+
.query("githubOrgMemberships")
18+
.withIndex("by_user", (q) => q.eq("userId", userId))
19+
.collect();
20+
memberships.sort((left, right) => left.login.localeCompare(right.login));
21+
22+
return {
23+
syncedAt: user.githubOrgMembershipsSyncedAt ?? null,
24+
truncated: user.githubOrgMembershipsTruncated ?? false,
25+
memberships: memberships.map(({ githubOrgId, login, avatarUrl, role, syncedAt }) => ({
26+
githubOrgId,
27+
login,
28+
avatarUrl: avatarUrl ?? null,
29+
role,
30+
syncedAt,
31+
})),
32+
};
33+
},
34+
});
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
import {
3+
fetchActiveGitHubOrgMemberships,
4+
readGitHubOrgMembershipSync,
5+
} from "./githubOrgMemberships";
6+
7+
describe("GitHub organization memberships", () => {
8+
it("loads active memberships from the authenticated GitHub API", async () => {
9+
const fetchImpl = vi.fn(async () => {
10+
return new Response(
11+
JSON.stringify([
12+
{
13+
state: "active",
14+
role: "member",
15+
organization: {
16+
id: 2,
17+
login: "trycua",
18+
avatar_url: "https://avatars.githubusercontent.com/u/2",
19+
},
20+
},
21+
{
22+
state: "active",
23+
role: "admin",
24+
organization: {
25+
id: 1,
26+
login: "openclaw",
27+
avatar_url: "https://avatars.githubusercontent.com/u/1",
28+
},
29+
},
30+
{
31+
state: "pending",
32+
role: "member",
33+
organization: { id: 3, login: "pending-org" },
34+
},
35+
]),
36+
{ status: 200 },
37+
);
38+
});
39+
40+
const result = await fetchActiveGitHubOrgMemberships("github-token", {
41+
fetchImpl: fetchImpl as typeof fetch,
42+
now: 123,
43+
});
44+
45+
expect(result).toEqual({
46+
syncedAt: 123,
47+
truncated: false,
48+
memberships: [
49+
{
50+
githubOrgId: "1",
51+
login: "openclaw",
52+
avatarUrl: "https://avatars.githubusercontent.com/u/1",
53+
role: "admin",
54+
},
55+
{
56+
githubOrgId: "2",
57+
login: "trycua",
58+
avatarUrl: "https://avatars.githubusercontent.com/u/2",
59+
role: "member",
60+
},
61+
],
62+
});
63+
expect(fetchImpl).toHaveBeenCalledWith(
64+
expect.stringContaining("/user/memberships/orgs?state=active"),
65+
expect.objectContaining({
66+
headers: expect.objectContaining({
67+
Authorization: "Bearer github-token",
68+
}),
69+
}),
70+
);
71+
});
72+
73+
it("rejects GitHub API failures without accepting partial membership data", async () => {
74+
const fetchImpl = vi.fn(async () => new Response("Forbidden", { status: 403 }));
75+
76+
await expect(
77+
fetchActiveGitHubOrgMemberships("github-token", {
78+
fetchImpl: fetchImpl as typeof fetch,
79+
}),
80+
).rejects.toThrow("GitHub organization membership lookup failed (403)");
81+
});
82+
83+
it("loads every GitHub organization membership page", async () => {
84+
const firstPage = Array.from({ length: 100 }, (_, index) => ({
85+
state: "active",
86+
role: "member",
87+
organization: { id: index + 1, login: `org-${index + 1}` },
88+
}));
89+
const fetchImpl = vi
90+
.fn()
91+
.mockResolvedValueOnce(new Response(JSON.stringify(firstPage), { status: 200 }))
92+
.mockResolvedValueOnce(
93+
new Response(
94+
JSON.stringify([
95+
{
96+
state: "active",
97+
role: "admin",
98+
organization: { id: 101, login: "org-101" },
99+
},
100+
]),
101+
{ status: 200 },
102+
),
103+
);
104+
105+
const result = await fetchActiveGitHubOrgMemberships("github-token", {
106+
fetchImpl: fetchImpl as typeof fetch,
107+
now: 123,
108+
});
109+
110+
expect(result.memberships).toHaveLength(101);
111+
expect(result.truncated).toBe(false);
112+
expect(fetchImpl).toHaveBeenCalledTimes(2);
113+
expect(fetchImpl).toHaveBeenLastCalledWith(
114+
expect.stringContaining("page=2"),
115+
expect.any(Object),
116+
);
117+
});
118+
119+
it("validates membership snapshots before they reach the database", () => {
120+
expect(
121+
readGitHubOrgMembershipSync({
122+
githubOrgMembershipSync: {
123+
syncedAt: 123,
124+
truncated: false,
125+
memberships: [
126+
{ githubOrgId: "1", login: "openclaw", role: "admin" },
127+
{ githubOrgId: "invalid", login: "spoofed", role: "member" },
128+
],
129+
},
130+
}),
131+
).toEqual({
132+
syncedAt: 123,
133+
truncated: false,
134+
memberships: [{ githubOrgId: "1", login: "openclaw", role: "admin" }],
135+
});
136+
});
137+
});

0 commit comments

Comments
 (0)