Skip to content

Commit c697304

Browse files
Seperate Exec membership from Org admin (#343)
This allows us to have some people who are not exec representatives but do have org admin privileges. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Exec Council membership is now synced asynchronously via a background workflow; a new background sync operation can be triggered. * **Refactor** * Voting-lead discovery now aggregates leads across organizations with stronger consistency and expanded lead details. * GitHub team parent reference can be omitted and parent-team config key renamed for clarity. * **Style** * Non-voting member wording updated to reference the specific organization. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 2891801 commit c697304

File tree

12 files changed

+534
-79
lines changed

12 files changed

+534
-79
lines changed

src/api/functions/github.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Octokit } from "octokit";
66
export interface CreateGithubTeamInputs {
77
githubToken: string;
88
orgId: string;
9-
parentTeamId: number;
9+
parentTeamId?: number;
1010
name: string;
1111
description?: string;
1212
privacy?: "secret" | "closed";

src/api/functions/organizations.ts

Lines changed: 101 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,13 @@ export async function getOrgInfo({
101101
name: x.name,
102102
username: x.username,
103103
title: x.title,
104-
}) as { name: string; username: string; title: string | undefined },
104+
nonVotingMember: x.nonVotingMember || false,
105+
}) as {
106+
name: string;
107+
username: string;
108+
title: string | undefined;
109+
nonVotingMember: boolean;
110+
},
105111
);
106112
response = { ...response, leads: unmarshalledLeads };
107113
}
@@ -525,42 +531,106 @@ export const removeLead = async ({
525531
};
526532

527533
/**
528-
* Returns the Microsoft 365 Dynamic User query to return all members of all lead groups.
529-
* Currently used to setup the Exec member list.
534+
* Returns all voting org leads across all organizations.
535+
* Uses consistent reads to avoid eventual consistency issues.
530536
* @param dynamoClient A DynamoDB client.
531-
* @param includeGroupIds Used to ensure that a specific group ID is included (Scan could be eventually consistent.)
537+
* @param logger A logger instance.
532538
*/
533-
export async function getLeadsM365DynamicQuery({
539+
export async function getAllVotingLeads({
534540
dynamoClient,
535-
includeGroupIds,
541+
logger,
536542
}: {
537543
dynamoClient: DynamoDBClient;
538-
includeGroupIds?: string[];
539-
}): Promise<string | null> {
540-
const command = new ScanCommand({
541-
TableName: genericConfig.SigInfoTableName,
542-
IndexName: "LeadsGroupIdIndex",
544+
logger: ValidLoggers;
545+
}): Promise<
546+
Array<{ username: string; org: string; name: string; title: string }>
547+
> {
548+
// Query all organizations in parallel for better performance
549+
const queryPromises = AllOrganizationNameList.map(async (orgName) => {
550+
const leadsQuery = new QueryCommand({
551+
TableName: genericConfig.SigInfoTableName,
552+
KeyConditionExpression: "primaryKey = :leadName",
553+
ExpressionAttributeValues: {
554+
":leadName": { S: `LEAD#${orgName}` },
555+
},
556+
ConsistentRead: true,
557+
});
558+
559+
try {
560+
const responseMarshall = await dynamoClient.send(leadsQuery);
561+
if (responseMarshall.Items) {
562+
return responseMarshall.Items.map((x) => unmarshall(x))
563+
.filter((x) => x.username && !x.nonVotingMember)
564+
.map((x) => ({
565+
username: x.username as string,
566+
org: orgName,
567+
name: x.name as string,
568+
title: x.title as string,
569+
}));
570+
}
571+
return [];
572+
} catch (e) {
573+
if (e instanceof BaseError) {
574+
throw e;
575+
}
576+
logger.error(e);
577+
throw new DatabaseFetchError({
578+
message: `Failed to get leads for org ${orgName}.`,
579+
});
580+
}
543581
});
544-
const results = await dynamoClient.send(command);
545-
if (!results || !results.Items || results.Items.length === 0) {
546-
return null;
547-
}
548-
const entries = results.Items.map((x) => unmarshall(x)) as {
549-
primaryKey: string;
550-
leadsEntraGroupId: string;
551-
}[];
552-
const groupIds = entries
553-
.filter((x) => x.primaryKey.startsWith("DEFINE#"))
554-
.map((x) => x.leadsEntraGroupId);
555-
556-
if (groupIds.length === 0) {
557-
return null;
582+
583+
const results = await Promise.all(queryPromises);
584+
return results.flat();
585+
}
586+
587+
/**
588+
* Checks if a user should remain in exec council by verifying they are a voting lead of at least one org.
589+
* Uses consistent reads to avoid eventual consistency issues.
590+
* @param username The username to check.
591+
* @param dynamoClient A DynamoDB client.
592+
* @param logger A logger instance.
593+
*/
594+
export async function shouldBeInExecCouncil({
595+
username,
596+
dynamoClient,
597+
logger,
598+
}: {
599+
username: string;
600+
dynamoClient: DynamoDBClient;
601+
logger: ValidLoggers;
602+
}): Promise<boolean> {
603+
// Query all orgs to see if this user is a voting lead of any org
604+
for (const orgName of AllOrganizationNameList) {
605+
const leadsQuery = new QueryCommand({
606+
TableName: genericConfig.SigInfoTableName,
607+
KeyConditionExpression: "primaryKey = :leadName AND entryId = :username",
608+
ExpressionAttributeValues: {
609+
":leadName": { S: `LEAD#${orgName}` },
610+
":username": { S: username },
611+
},
612+
ConsistentRead: true,
613+
});
614+
615+
try {
616+
const responseMarshall = await dynamoClient.send(leadsQuery);
617+
if (responseMarshall.Items && responseMarshall.Items.length > 0) {
618+
const lead = unmarshall(responseMarshall.Items[0]);
619+
// If they're a lead and not a non-voting member, they should be in exec
620+
if (!lead.nonVotingMember) {
621+
return true;
622+
}
623+
}
624+
} catch (e) {
625+
if (e instanceof BaseError) {
626+
throw e;
627+
}
628+
logger.error(e);
629+
throw new DatabaseFetchError({
630+
message: `Failed to check lead status for ${username} in org ${orgName}.`,
631+
});
632+
}
558633
}
559634

560-
const formattedGroupIds = [
561-
...new Set([...(includeGroupIds || []), ...groupIds]),
562-
]
563-
.map((id) => `'${id}'`)
564-
.join(", ");
565-
return `user.memberOf -any (group.objectId -in [${formattedGroupIds}])`;
635+
return false;
566636
}

src/api/routes/organizations.ts

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
1-
import { FastifyError, FastifyPluginAsync } from "fastify";
2-
import {
3-
AllOrganizationNameList,
4-
getOrgByName,
5-
Organizations,
6-
} from "@acm-uiuc/js-shared";
1+
import { FastifyPluginAsync } from "fastify";
2+
import { AllOrganizationNameList, getOrgByName } from "@acm-uiuc/js-shared";
73
import rateLimiter from "api/plugins/rateLimiter.js";
84
import { withRoles, withTags } from "api/components/index.js";
95
import { z } from "zod/v4";
@@ -23,7 +19,6 @@ import {
2319
} from "common/errors/index.js";
2420
import {
2521
addLead,
26-
getLeadsM365DynamicQuery,
2722
getOrgInfo,
2823
removeLead,
2924
SQSMessage,
@@ -35,8 +30,6 @@ import {
3530
TransactWriteItemsCommand,
3631
} from "@aws-sdk/client-dynamodb";
3732
import {
38-
execCouncilGroupId,
39-
execCouncilTestingGroupId,
4033
genericConfig,
4134
notificationRecipients,
4235
roleArns,
@@ -46,20 +39,12 @@ import { buildAuditLogTransactPut } from "api/functions/auditLog.js";
4639
import { Modules } from "common/modules.js";
4740
import { authorizeByOrgRoleOrSchema } from "api/functions/authorization.js";
4841
import { checkPaidMembership } from "api/functions/membership.js";
49-
import {
50-
createM365Group,
51-
getEntraIdToken,
52-
setGroupMembershipRule,
53-
} from "api/functions/entraId.js";
42+
import { createM365Group, getEntraIdToken } from "api/functions/entraId.js";
5443
import { SecretsManagerClient } from "@aws-sdk/client-secrets-manager";
5544
import { getRoleCredentials } from "api/functions/sts.js";
56-
import { SendMessageCommand, SQSClient } from "@aws-sdk/client-sqs";
45+
import { SQSClient } from "@aws-sdk/client-sqs";
5746
import { sendSqsMessagesInBatches } from "api/functions/sqs.js";
5847
import { retryDynamoTransactionWithBackoff } from "api/utils.js";
59-
import {
60-
assignIdpGroupsToTeam,
61-
createGithubTeam,
62-
} from "api/functions/github.js";
6348
import { SKIP_EXTERNAL_ORG_LEAD_UPDATE } from "common/overrides.js";
6449
import { AvailableSQSFunctions, SQSPayload } from "common/types/sqsMessage.js";
6550

@@ -499,20 +484,8 @@ const organizationsPlugin: FastifyPluginAsync = async (fastify, _options) => {
499484
`Store Entra group ID for ${request.params.orgId}`,
500485
);
501486

502-
// Update dynamic membership query
503-
const newQuery = await getLeadsM365DynamicQuery({
504-
dynamoClient: fastify.dynamoClient,
505-
includeGroupIds: [entraGroupId],
506-
});
507-
if (newQuery) {
508-
const groupToUpdate =
509-
fastify.runEnvironment === "prod"
510-
? execCouncilGroupId
511-
: execCouncilTestingGroupId;
512-
request.log.info("Changing Exec group membership dynamic query...");
513-
await setGroupMembershipRule(entraIdToken, groupToUpdate, newQuery);
514-
request.log.info("Changed Exec group membership dynamic query!");
515-
}
487+
// Note: Exec council membership is now managed via SQS sync handler
488+
// instead of dynamic membership rules
516489
} catch (e) {
517490
request.log.error(e, "Failed to create Entra group");
518491
throw new InternalServerError({
@@ -589,6 +562,19 @@ const organizationsPlugin: FastifyPluginAsync = async (fastify, _options) => {
589562
};
590563
sqsPayloads.push(sqsPayload);
591564
}
565+
566+
// Queue exec council sync to ensure voting members are added/removed
567+
const syncExecPayload: SQSPayload<AvailableSQSFunctions.SyncExecCouncil> =
568+
{
569+
function: AvailableSQSFunctions.SyncExecCouncil,
570+
metadata: {
571+
initiator: request.username!,
572+
reqId: request.id,
573+
},
574+
payload: {},
575+
};
576+
sqsPayloads.push(syncExecPayload);
577+
592578
if (sqsPayloads.length > 0) {
593579
await sendSqsMessagesInBatches({
594580
sqsClient: fastify.sqsClient,

src/api/sqs/handlers/createOrgGithubTeam.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export const createOrgGithubTeamHandler: SQSHandlerFunction<
9191
const { updated, id: teamId } = await createGithubTeam({
9292
orgId: currentEnvironmentConfig.GithubOrgName,
9393
githubToken: secretConfig.github_pat,
94-
parentTeamId: currentEnvironmentConfig.ExecGithubTeam,
94+
parentTeamId: currentEnvironmentConfig.OrgAdminGithubParentTeam,
9595
name: finalName,
9696
description: githubTeamDescription,
9797
logger,

src/api/sqs/handlers/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@ export { provisionNewMemberHandler } from "./provisionNewMember.js";
44
export { sendSaleEmailHandler } from "./sendSaleEmailHandler.js";
55
export { emailNotificationsHandler } from "./emailNotifications.js";
66
export { createOrgGithubTeamHandler } from "./createOrgGithubTeam.js";
7+
export { syncExecCouncilHandler } from "./syncExecCouncil.js";

0 commit comments

Comments
 (0)