Skip to content

Commit c25ae2d

Browse files
author
hack-cli-tests
committed
fix(auth): allow org admins to administer teams
1 parent 2a16e27 commit c25ae2d

2 files changed

Lines changed: 186 additions & 65 deletions

File tree

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

Lines changed: 94 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -233,15 +233,14 @@ export class InMemoryOrgTeamsStore implements OrgTeamsStore {
233233
readonly orgKey: string | null;
234234
readonly actorUserId: string;
235235
}): MaybePromise<readonly TeamRecord[]> {
236-
const allowedTeamIds = new Set(
236+
const allowedOrgIds = new Set(
237237
[...this.activeMemberships.values()]
238238
.filter(
239239
(membership) =>
240-
membership.scope === "team" &&
240+
membership.scope === "organization" &&
241241
membership.userId === input.actorUserId
242242
)
243-
.map((membership) => membership.teamId)
244-
.filter((teamId): teamId is string => typeof teamId === "string")
243+
.map((membership) => membership.organizationId)
245244
);
246245
const organization = input.orgKey
247246
? this.findOrganization({ orgKey: input.orgKey })
@@ -250,7 +249,7 @@ export class InMemoryOrgTeamsStore implements OrgTeamsStore {
250249
if (organization && team.organizationId !== organization.id) {
251250
return false;
252251
}
253-
return allowedTeamIds.has(team.id);
252+
return allowedOrgIds.has(team.organizationId);
254253
});
255254
}
256255

@@ -268,8 +267,8 @@ export class InMemoryOrgTeamsStore implements OrgTeamsStore {
268267
}
269268
const allowed = [...this.activeMemberships.values()].some(
270269
(membership) =>
271-
membership.scope === "team" &&
272-
membership.teamId === team.id &&
270+
membership.scope === "organization" &&
271+
membership.organizationId === team.organizationId &&
273272
membership.userId === input.actorUserId
274273
);
275274
return allowed ? team : null;
@@ -410,66 +409,30 @@ export class InMemoryOrgTeamsStore implements OrgTeamsStore {
410409
return null;
411410
}
412411
const normalizedTarget = input.target.trim();
413-
for (const invitation of this.invitations.values()) {
414-
if (
415-
invitation.status === "pending" &&
416-
invitation.scope === input.scope &&
417-
invitation.organizationId === organization.id &&
418-
invitation.teamId === (team?.id ?? null) &&
419-
invitation.email === normalizedTarget
420-
) {
421-
const removed: InvitationRecord = {
422-
...invitation,
423-
status: "removed",
424-
updatedAt: new Date().toISOString(),
425-
};
426-
this.invitations.set(invitation.id, removed);
427-
const membership = this.toRemovedMembership({
428-
scope: invitation.scope,
429-
organizationId: invitation.organizationId,
430-
teamId: invitation.teamId,
431-
userId: null,
432-
email: invitation.email,
433-
target: invitation.email,
434-
});
435-
this.removedMemberships.set(membership.id, membership);
436-
if (input.scope === "organization") {
437-
this.cascadeRemovedOrganizationTarget({
438-
organizationId: organization.id,
439-
target: normalizedTarget,
440-
});
441-
}
442-
return membership;
443-
}
412+
const teamId = team?.id ?? null;
413+
const removedMembership =
414+
this.removePendingInvitationMembership({
415+
scope: input.scope,
416+
organizationId: organization.id,
417+
teamId,
418+
target: normalizedTarget,
419+
}) ??
420+
this.removeActiveMembership({
421+
scope: input.scope,
422+
organizationId: organization.id,
423+
teamId,
424+
target: normalizedTarget,
425+
});
426+
if (!removedMembership) {
427+
return null;
444428
}
445-
446-
for (const [key, membership] of this.activeMemberships.entries()) {
447-
if (
448-
membership.scope === input.scope &&
449-
membership.organizationId === organization.id &&
450-
membership.teamId === (team?.id ?? null) &&
451-
membership.target === normalizedTarget
452-
) {
453-
this.activeMemberships.delete(key);
454-
const removed = this.toRemovedMembership({
455-
scope: membership.scope,
456-
organizationId: membership.organizationId,
457-
teamId: membership.teamId,
458-
userId: membership.userId,
459-
email: membership.email,
460-
target: membership.target,
461-
});
462-
this.removedMemberships.set(removed.id, removed);
463-
if (input.scope === "organization") {
464-
this.cascadeRemovedOrganizationTarget({
465-
organizationId: organization.id,
466-
target: normalizedTarget,
467-
});
468-
}
469-
return removed;
470-
}
429+
if (input.scope === "organization") {
430+
this.cascadeRemovedOrganizationTarget({
431+
organizationId: organization.id,
432+
target: normalizedTarget,
433+
});
471434
}
472-
return null;
435+
return removedMembership;
473436
}
474437

475438
listInvitationsForEmail(input: {
@@ -636,6 +599,72 @@ export class InMemoryOrgTeamsStore implements OrgTeamsStore {
636599
};
637600
}
638601

602+
private removePendingInvitationMembership(input: {
603+
readonly scope: "organization" | "team";
604+
readonly organizationId: string;
605+
readonly teamId: string | null;
606+
readonly target: string;
607+
}): MembershipRecord | null {
608+
for (const invitation of this.invitations.values()) {
609+
if (
610+
invitation.status !== "pending" ||
611+
invitation.scope !== input.scope ||
612+
invitation.organizationId !== input.organizationId ||
613+
invitation.teamId !== input.teamId ||
614+
invitation.email !== input.target
615+
) {
616+
continue;
617+
}
618+
const removed: InvitationRecord = {
619+
...invitation,
620+
status: "removed",
621+
updatedAt: new Date().toISOString(),
622+
};
623+
this.invitations.set(invitation.id, removed);
624+
const membership = this.toRemovedMembership({
625+
scope: invitation.scope,
626+
organizationId: invitation.organizationId,
627+
teamId: invitation.teamId,
628+
userId: null,
629+
email: invitation.email,
630+
target: invitation.email,
631+
});
632+
this.removedMemberships.set(membership.id, membership);
633+
return membership;
634+
}
635+
return null;
636+
}
637+
638+
private removeActiveMembership(input: {
639+
readonly scope: "organization" | "team";
640+
readonly organizationId: string;
641+
readonly teamId: string | null;
642+
readonly target: string;
643+
}): MembershipRecord | null {
644+
for (const [key, membership] of this.activeMemberships.entries()) {
645+
if (
646+
membership.scope !== input.scope ||
647+
membership.organizationId !== input.organizationId ||
648+
membership.teamId !== input.teamId ||
649+
membership.target !== input.target
650+
) {
651+
continue;
652+
}
653+
this.activeMemberships.delete(key);
654+
const removed = this.toRemovedMembership({
655+
scope: membership.scope,
656+
organizationId: membership.organizationId,
657+
teamId: membership.teamId,
658+
userId: membership.userId,
659+
email: membership.email,
660+
target: membership.target,
661+
});
662+
this.removedMemberships.set(removed.id, removed);
663+
return removed;
664+
}
665+
return null;
666+
}
667+
639668
private cascadeRemovedOrganizationTarget(input: {
640669
readonly organizationId: string;
641670
readonly target: string;

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

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,98 @@ 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 () => {
253+
const store = new InMemoryOrgTeamsStore();
254+
const ownerApp = createAuthBrokerApp({
255+
config: createTestConfig(),
256+
betterAuthRuntime: createBetterAuthRuntimeWithSession(createSession()),
257+
orgTeamsStore: store,
258+
});
259+
260+
await ownerApp.handle(
261+
new Request("http://localhost/v1/auth/orgs", {
262+
method: "POST",
263+
headers: { "content-type": "application/json" },
264+
body: JSON.stringify({
265+
slug: "hack",
266+
name: "Hack",
267+
}),
268+
})
269+
);
270+
271+
await ownerApp.handle(
272+
new Request("http://localhost/v1/auth/teams", {
273+
method: "POST",
274+
headers: { "content-type": "application/json" },
275+
body: JSON.stringify({
276+
slug: "cli",
277+
org: "hack",
278+
name: "CLI",
279+
}),
280+
})
281+
);
282+
283+
await ownerApp.handle(
284+
new Request("http://localhost/v1/auth/orgs/hack/members/add", {
285+
method: "POST",
286+
headers: { "content-type": "application/json" },
287+
body: JSON.stringify({
288+
target: "user-456",
289+
}),
290+
})
291+
);
292+
293+
await ownerApp.handle(
294+
new Request("http://localhost/v1/auth/orgs/hack/members/add", {
295+
method: "POST",
296+
headers: { "content-type": "application/json" },
297+
body: JSON.stringify({
298+
target: "user-789",
299+
}),
300+
})
301+
);
302+
303+
const orgAdminApp = createAuthBrokerApp({
304+
config: createTestConfig(),
305+
betterAuthRuntime: createBetterAuthRuntimeWithSession(
306+
createSession({
307+
userId: "user-456",
308+
email: "org-admin@example.com",
309+
})
310+
),
311+
orgTeamsStore: store,
312+
});
313+
314+
const listTeamsResponse = await orgAdminApp.handle(
315+
new Request("http://localhost/v1/auth/teams?org=hack")
316+
);
317+
expect(listTeamsResponse.status).toBe(200);
318+
const listedTeams = (await listTeamsResponse.json()) as {
319+
readonly teams?: ReadonlyArray<{ readonly slug?: string }>;
320+
};
321+
expect(listedTeams.teams?.map((team) => team.slug)).toEqual(["cli"]);
322+
323+
const addTeamMemberResponse = await orgAdminApp.handle(
324+
new Request("http://localhost/v1/auth/teams/cli/members/add", {
325+
method: "POST",
326+
headers: { "content-type": "application/json" },
327+
body: JSON.stringify({
328+
org: "hack",
329+
target: "user-789",
330+
}),
331+
})
332+
);
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");
342+
});
343+
252344
test("team membership changes require an active parent org membership", async () => {
253345
const store = new InMemoryOrgTeamsStore();
254346
const ownerApp = createAuthBrokerApp({

0 commit comments

Comments
 (0)