Skip to content

Commit 4e0a5fd

Browse files
authored
Merge pull request #25 from hack-dance/codex/fix-authorization-bypass-for-team-management
fix(auth): restore team-scoped authorization
2 parents dfe78c6 + 3c4eee5 commit 4e0a5fd

2 files changed

Lines changed: 15 additions & 27 deletions

File tree

services/auth-broker/src/modules/orgs/service.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -233,23 +233,19 @@ export class InMemoryOrgTeamsStore implements OrgTeamsStore {
233233
readonly orgKey: string | null;
234234
readonly actorUserId: string;
235235
}): MaybePromise<readonly TeamRecord[]> {
236-
const allowedOrgIds = new Set(
237-
[...this.activeMemberships.values()]
238-
.filter(
239-
(membership) =>
240-
membership.scope === "organization" &&
241-
membership.userId === input.actorUserId
242-
)
243-
.map((membership) => membership.organizationId)
244-
);
245236
const organization = input.orgKey
246237
? this.findOrganization({ orgKey: input.orgKey })
247238
: null;
248239
return [...this.teams.values()].filter((team) => {
249240
if (organization && team.organizationId !== organization.id) {
250241
return false;
251242
}
252-
return allowedOrgIds.has(team.organizationId);
243+
return [...this.activeMemberships.values()].some(
244+
(membership) =>
245+
membership.scope === "team" &&
246+
membership.teamId === team.id &&
247+
membership.userId === input.actorUserId
248+
);
253249
});
254250
}
255251

@@ -267,8 +263,8 @@ export class InMemoryOrgTeamsStore implements OrgTeamsStore {
267263
}
268264
const allowed = [...this.activeMemberships.values()].some(
269265
(membership) =>
270-
membership.scope === "organization" &&
271-
membership.organizationId === team.organizationId &&
266+
membership.scope === "team" &&
267+
membership.teamId === team.id &&
272268
membership.userId === input.actorUserId
273269
);
274270
return allowed ? team : null;

services/auth-broker/tests/org-team-membership.test.ts

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ describe("org and team membership broker routes", () => {
249249
expect(listMembersResponse.status).toBe(404);
250250
});
251251

252-
test("active org members can administer teams without direct team membership", async () => {
252+
test("active org members cannot administer teams without direct team membership", async () => {
253253
const store = new InMemoryOrgTeamsStore();
254254
const ownerApp = createAuthBrokerApp({
255255
config: createTestConfig(),
@@ -300,27 +300,27 @@ describe("org and team membership broker routes", () => {
300300
})
301301
);
302302

303-
const orgAdminApp = createAuthBrokerApp({
303+
const orgMemberApp = createAuthBrokerApp({
304304
config: createTestConfig(),
305305
betterAuthRuntime: createBetterAuthRuntimeWithSession(
306306
createSession({
307307
userId: "user-456",
308-
email: "org-admin@example.com",
308+
email: "org-member@example.com",
309309
})
310310
),
311311
orgTeamsStore: store,
312312
});
313313

314-
const listTeamsResponse = await orgAdminApp.handle(
314+
const listTeamsResponse = await orgMemberApp.handle(
315315
new Request("http://localhost/v1/auth/teams?org=hack")
316316
);
317317
expect(listTeamsResponse.status).toBe(200);
318318
const listedTeams = (await listTeamsResponse.json()) as {
319319
readonly teams?: ReadonlyArray<{ readonly slug?: string }>;
320320
};
321-
expect(listedTeams.teams?.map((team) => team.slug)).toEqual(["cli"]);
321+
expect(listedTeams.teams).toEqual([]);
322322

323-
const addTeamMemberResponse = await orgAdminApp.handle(
323+
const addTeamMemberResponse = await orgMemberApp.handle(
324324
new Request("http://localhost/v1/auth/teams/cli/members/add", {
325325
method: "POST",
326326
headers: { "content-type": "application/json" },
@@ -330,15 +330,7 @@ describe("org and team membership broker routes", () => {
330330
}),
331331
})
332332
);
333-
expect(addTeamMemberResponse.status).toBe(200);
334-
const addedMembership = (await addTeamMemberResponse.json()) as {
335-
readonly membership?: {
336-
readonly scope?: string;
337-
readonly state?: string;
338-
};
339-
};
340-
expect(addedMembership.membership?.scope).toBe("team");
341-
expect(addedMembership.membership?.state).toBe("active");
333+
expect(addTeamMemberResponse.status).toBe(404);
342334
});
343335

344336
test("team membership changes require an active parent org membership", async () => {

0 commit comments

Comments
 (0)