Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/api/functions/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export async function createGithubTeam({

if (existingTeam) {
logger.info(`Team "${name}" already exists with id: ${existingTeam.id}`);
return existingTeam.id;
return { updated: false, id: existingTeam.id };
}
logger.info(`Creating GitHub team "${name}"`);
const response = await octokit.request("POST /orgs/{org}/teams", {
Expand Down Expand Up @@ -196,7 +196,7 @@ export async function createGithubTeam({
logger.warn(`Failed to remove user from team ${newTeamId}:`, removeError);
}

return newTeamId;
return { updated: true, id: newTeamId };
} catch (e) {
if (e instanceof BaseError) {
throw e;
Expand Down
4 changes: 2 additions & 2 deletions src/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"pino": "^9.6.0",
"pluralize": "^8.0.0",
"qrcode": "^1.5.4",
"redlock-universal": "^0.6.0",
"redlock-universal": "^0.6.4",
"sanitize-html": "^2.17.0",
"stripe": "^18.0.0",
"uuid": "^11.1.0",
Expand All @@ -74,4 +74,4 @@
"pino-pretty": "^13.1.1",
"yaml": "^2.8.1"
}
}
}
115 changes: 27 additions & 88 deletions src/api/routes/organizations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,15 @@ import {
} from "api/functions/entraId.js";
import { SecretsManagerClient } from "@aws-sdk/client-secrets-manager";
import { getRoleCredentials } from "api/functions/sts.js";
import { SQSClient } from "@aws-sdk/client-sqs";
import { SendMessageCommand, SQSClient } from "@aws-sdk/client-sqs";
import { sendSqsMessagesInBatches } from "api/functions/sqs.js";
import { retryDynamoTransactionWithBackoff } from "api/utils.js";
import {
assignIdpGroupsToTeam,
createGithubTeam,
} from "api/functions/github.js";
import { SKIP_EXTERNAL_ORG_LEAD_UPDATE } from "common/overrides.js";
import { AvailableSQSFunctions, SQSPayload } from "common/types/sqsMessage.js";

export const CLIENT_HTTP_CACHE_POLICY = `public, max-age=${ORG_DATA_CACHED_DURATION}, stale-while-revalidate=${Math.floor(ORG_DATA_CACHED_DURATION * 1.1)}, stale-if-error=3600`;

Expand Down Expand Up @@ -413,12 +414,10 @@ const organizationsPlugin: FastifyPluginAsync = async (fastify, _options) => {
? (unmarshall(metadataResponse.Item).leadsEntraGroupId as string)
: undefined;

let githubTeamId = metadataResponse.Item
? (unmarshall(metadataResponse.Item).githubTeamId as number)
const githubTeamId = metadataResponse.Item
? (unmarshall(metadataResponse.Item).leadsGithubTeamId as number)
: undefined;

let createdGithubTeam = false;

const entraIdToken = await getEntraIdToken({
clients,
clientId: fastify.environmentConfig.AadValidClientId,
Expand All @@ -428,8 +427,6 @@ const organizationsPlugin: FastifyPluginAsync = async (fastify, _options) => {

const shouldCreateNewEntraGroup =
!entraGroupId && !shouldSkipEnhancedActions;
const shouldCreateNewGithubGroup =
!githubTeamId && !shouldSkipEnhancedActions;
const grpDisplayName = `${request.params.orgId} Admin`;
const orgInfo = getOrgByName(request.params.orgId);
if (!orgInfo) {
Expand Down Expand Up @@ -524,65 +521,6 @@ const organizationsPlugin: FastifyPluginAsync = async (fastify, _options) => {
}
}

// Create GitHub team if needed
if (shouldCreateNewGithubGroup) {
request.log.info(
`No GitHub team exists for ${request.params.orgId}. Creating new team...`,
);
const suffix = fastify.environmentConfig.GroupEmailSuffix;
githubTeamId = await createGithubTeam({
orgId: fastify.environmentConfig.GithubOrgName,
githubToken: fastify.secretConfig.github_pat,
parentTeamId: fastify.environmentConfig.ExecGithubTeam,
name: `${grpShortName}${suffix === "" ? "" : `-${suffix}`}`,
description: grpDisplayName,
logger: request.log,
});
request.log.info(
`Created GitHub team "${githubTeamId}" for ${request.params.orgId} leads.`,
);
createdGithubTeam = true;

// Store GitHub team ID immediately
const logStatement = buildAuditLogTransactPut({
entry: {
module: Modules.ORG_INFO,
message: `Created GitHub team "${githubTeamId}" for organization leads.`,
actor: request.username!,
target: request.params.orgId,
},
});

const storeGithubIdOperation = async () => {
const commandTransaction = new TransactWriteItemsCommand({
TransactItems: [
...(logStatement ? [logStatement] : []),
{
Update: {
TableName: genericConfig.SigInfoTableName,
Key: marshall({
primaryKey: `DEFINE#${request.params.orgId}`,
entryId: "0",
}),
UpdateExpression:
"SET leadsGithubTeamId = :githubTeamId, updatedAt = :updatedAt",
ExpressionAttributeValues: marshall({
":githubTeamId": githubTeamId,
":updatedAt": new Date().toISOString(),
}),
},
},
],
});
return await clients.dynamoClient.send(commandTransaction);
};

await retryDynamoTransactionWithBackoff(
storeGithubIdOperation,
request.log,
`Store GitHub team ID for ${request.params.orgId}`,
);
}
const commonArgs = {
orgId: request.params.orgId,
actorUsername: request.username!,
Expand Down Expand Up @@ -628,36 +566,37 @@ const organizationsPlugin: FastifyPluginAsync = async (fastify, _options) => {
.map((r) => r.value)
.filter((p): p is SQSMessage => p !== null);

if (!fastify.sqsClient) {
fastify.sqsClient = new SQSClient({
region: genericConfig.AwsRegion,
});
}

// Queue creating GitHub team if needed
if (!githubTeamId) {
const sqsPayload: SQSPayload<AvailableSQSFunctions.CreateOrgGithubTeam> =
{
function: AvailableSQSFunctions.CreateOrgGithubTeam,
metadata: {
initiator: request.username!,
reqId: request.id,
},
payload: {
orgName: request.params.orgId,
githubTeamDescription: grpDisplayName,
githubTeamName: grpShortName,
},
};
sqsPayloads.push(sqsPayload);
}
if (sqsPayloads.length > 0) {
if (!fastify.sqsClient) {
fastify.sqsClient = new SQSClient({
region: genericConfig.AwsRegion,
});
}
await sendSqsMessagesInBatches({
sqsClient: fastify.sqsClient,
queueUrl: fastify.environmentConfig.SqsQueueUrl,
logger: request.log,
sqsPayloads,
});
}

if (
createdGithubTeam &&
githubTeamId &&
fastify.environmentConfig.GithubIdpSyncEnabled
) {
request.log.info("Setting up IDP sync for Github team!");
await assignIdpGroupsToTeam({
githubToken: fastify.secretConfig.github_pat,
teamId: githubTeamId,
logger: request.log,
groupsToSync: [entraGroupId].filter((x): x is string => !!x),
orgId: fastify.environmentConfig.GithubOrgId,
orgName: fastify.environmentConfig.GithubOrgName,
});
}

return reply.status(201).send();
},
);
Expand Down
169 changes: 169 additions & 0 deletions src/api/sqs/handlers/createOrgGithubTeam.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
import { AvailableSQSFunctions } from "common/types/sqsMessage.js";
import { currentEnvironmentConfig, SQSHandlerFunction } from "../index.js";
import {
DynamoDBClient,
GetItemCommand,
TransactWriteItemsCommand,
} from "@aws-sdk/client-dynamodb";
import { genericConfig, SecretConfig } from "common/config.js";
import { getSecretConfig } from "../utils.js";
import RedisModule from "ioredis";
import { createLock, IoredisAdapter } from "redlock-universal";
import { marshall, unmarshall } from "@aws-sdk/util-dynamodb";
import { InternalServerError } from "common/errors/index.js";
import {
assignIdpGroupsToTeam,
createGithubTeam,
} from "api/functions/github.js";
import { buildAuditLogTransactPut } from "api/functions/auditLog.js";
import { Modules } from "common/modules.js";
import { retryDynamoTransactionWithBackoff } from "api/utils.js";
import { SKIP_EXTERNAL_ORG_LEAD_UPDATE } from "common/overrides.js";
import { getOrgByName } from "@acm-uiuc/js-shared";

export const createOrgGithubTeamHandler: SQSHandlerFunction<
AvailableSQSFunctions.CreateOrgGithubTeam
> = async (payload, metadata, logger) => {
const secretConfig: SecretConfig = await getSecretConfig({
logger,
commonConfig: { region: genericConfig.AwsRegion },
});
const redisClient = new RedisModule.default(secretConfig.redis_url);
try {
const { orgName, githubTeamName, githubTeamDescription } = payload;
const orgImmutableId = getOrgByName(orgName)!.id;
if (SKIP_EXTERNAL_ORG_LEAD_UPDATE.includes(orgImmutableId)) {
logger.info(
`Organization ${orgName} has external updates disabled, exiting.`,
);
return;
}
const dynamo = new DynamoDBClient({
Comment on lines +33 to +41
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid non-null assertion on getOrgByName; harden against malformed payloads.

A bad SQS payload would crash the worker. Guard and emit a clear error.

-    const { orgName, githubTeamName, githubTeamDescription } = payload;
-    const orgImmutableId = getOrgByName(orgName)!.id;
+    const { orgName, githubTeamName, githubTeamDescription } = payload;
+    const orgInfo = getOrgByName(orgName);
+    if (!orgInfo) {
+      logger.warn({ orgName }, "Unknown organization; skipping GitHub team creation.");
+      return;
+    }
+    const orgImmutableId = orgInfo.id;
🤖 Prompt for AI Agents
In src/api/sqs/handlers/createOrgGithubTeam.ts around lines 33-41, the code uses
a non-null assertion on getOrgByName which will crash if the SQS payload is
malformed; replace the assertion with a safe lookup: first validate
payload.orgName exists, call getOrgByName(orgName) into a variable, check if the
result is undefined, and if so log a clear error including the payload/orgName
and exit early (or route to DLQ) instead of proceeding; only when org is found
read org.id and continue with the SKIP_EXTERNAL_ORG_LEAD_UPDATE check.

region: genericConfig.AwsRegion,
});
const lock = createLock({
adapter: new IoredisAdapter(redisClient),
key: `createOrgGithubTeamHandler:${orgImmutableId}`,
retryAttempts: 5,
retryDelay: 300,
});
return await lock.using(async (signal) => {
const getMetadataCommand = new GetItemCommand({
TableName: genericConfig.SigInfoTableName,
Key: marshall({
primaryKey: `DEFINE#${orgName}`,
entryId: "0",
}),
ProjectionExpression: "#entra,#gh",
ExpressionAttributeNames: {
"#entra": "leadsEntraGroupId",
"#gh": "leadsGithubTeamId",
},
ConsistentRead: true,
});
const existingData = await dynamo.send(getMetadataCommand);
if (!existingData || !existingData.Item) {
throw new InternalServerError({
message: `Could not find org entry for ${orgName}`,
});
}
const currentOrgInfo = unmarshall(existingData.Item) as {
leadsEntraGroupId?: string;
leadsGithubTeamId?: string;
};
if (!currentOrgInfo.leadsEntraGroupId) {
logger.info(`${orgName} does not have an Entra group, skipping!`);
return;
}
if (currentOrgInfo.leadsGithubTeamId) {
logger.info("This org already has a GitHub team, skipping");
return;
}
if (signal.aborted) {
throw new InternalServerError({
message:
"Checked on lock before creating GitHub team, we've lost the lock!",
});
}
logger.info(`Creating GitHub team for ${orgName}!`);
const suffix = currentEnvironmentConfig.GroupEmailSuffix;
const finalName = `${githubTeamName}${suffix === "" ? "" : `-${suffix}`}`;
const { updated, id: teamId } = await createGithubTeam({
orgId: currentEnvironmentConfig.GithubOrgName,
githubToken: secretConfig.github_pat,
parentTeamId: currentEnvironmentConfig.ExecGithubTeam,
name: finalName,
description: githubTeamDescription,
logger,
});
if (!updated) {
logger.info(
`Github team "${finalName}" already existed. We're assuming team sync was already set up (if not, please configure manually).`,
);
} else {
logger.info(
`Github team "${finalName}" created with team ID "${teamId}".`,
);
if (currentEnvironmentConfig.GithubIdpSyncEnabled) {
logger.info(
`Setting up IDP sync for Github team from Entra ID group ${currentOrgInfo.leadsEntraGroupId}`,
);
await assignIdpGroupsToTeam({
githubToken: secretConfig.github_pat,
teamId,
logger,
groupsToSync: [currentOrgInfo.leadsEntraGroupId],
orgId: currentEnvironmentConfig.GithubOrgId,
orgName: currentEnvironmentConfig.GithubOrgName,
});
}
}
logger.info("Adding updates to audit log");
const logStatement = updated
? buildAuditLogTransactPut({
entry: {
module: Modules.ORG_INFO,
message: `Created GitHub team "${finalName}" for organization leads.`,
actor: metadata.initiator,
target: orgName,
},
})
: undefined;
const storeGithubIdOperation = async () => {
const commandTransaction = new TransactWriteItemsCommand({
TransactItems: [
...(logStatement ? [logStatement] : []),
{
Update: {
TableName: genericConfig.SigInfoTableName,
Key: marshall({
primaryKey: `DEFINE#${orgName}`,
entryId: "0",
}),
UpdateExpression:
"SET leadsGithubTeamId = :githubTeamId, updatedAt = :updatedAt",
ExpressionAttributeValues: marshall({
":githubTeamId": teamId,
":updatedAt": new Date().toISOString(),
}),
},
},
],
});
return await dynamo.send(commandTransaction);
};

await retryDynamoTransactionWithBackoff(
storeGithubIdOperation,
logger,
`Store GitHub team ID for ${orgName}`,
);
});
} finally {
try {
await redisClient.quit();
} catch {
redisClient.disconnect();
}
}
};
1 change: 1 addition & 0 deletions src/api/sqs/handlers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export { emailMembershipPassHandler } from "./emailMembershipPassHandler.js";
export { provisionNewMemberHandler } from "./provisionNewMember.js";
export { sendSaleEmailHandler } from "./sendSaleEmailHandler.js";
export { emailNotificationsHandler } from "./emailNotifications.js";
export { createOrgGithubTeamHandler } from "./createOrgGithubTeam.js";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Resolve ESLint “import/extensions” on re-export

Consistent with other exports in this file, the “.js” extension is needed for NodeNext ESM. If the rule is strict, silence it inline:

-export { createOrgGithubTeamHandler } from "./createOrgGithubTeam.js";
+export { createOrgGithubTeamHandler } from "./createOrgGithubTeam.js"; // eslint-disable-line import/extensions

Alternatively, relax the rule for TS files in .eslintrc when using "moduleResolution": "nodenext".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export { createOrgGithubTeamHandler } from "./createOrgGithubTeam.js";
export { createOrgGithubTeamHandler } from "./createOrgGithubTeam.js"; // eslint-disable-line import/extensions
🧰 Tools
🪛 ESLint

[error] 6-6: Unexpected use of file extension "js" for "./createOrgGithubTeam.js"

(import/extensions)

🤖 Prompt for AI Agents
In src/api/sqs/handlers/index.ts around line 6, the re-export currently includes
the “.js” extension and ESLint's import/extensions rule is complaining; either
silence the rule inline for this line by adding an ESLint-disable comment (e.g.
// eslint-disable-next-line import/extensions) immediately above the export, or
update your project ESLint config to allow .js extensions in TS files when using
"moduleResolution": "nodenext" (adjust rules.import/extensions or override for
"*.ts" files); apply one of these fixes to keep the explicit .js re-export while
satisfying linting.

2 changes: 2 additions & 0 deletions src/api/sqs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
import { ValidationError } from "../../common/errors/index.js";
import { RunEnvironment } from "../../common/roles.js";
import { environmentConfig } from "../../common/config.js";
import { createOrgGithubTeamHandler } from "./handlers/createOrgGithubTeam.js";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix ESLint import/extensions and consolidate handler import

ESLint flags the explicit “.js” on the direct handler import. To avoid the rule while keeping NodeNext-friendly imports, pull the symbol from the existing handlers barrel import and drop the extra line.

-import { createOrgGithubTeamHandler } from "./handlers/createOrgGithubTeam.js";
 // ...
-import {
+import {
   emailMembershipPassHandler,
   pingHandler,
   provisionNewMemberHandler,
   sendSaleEmailHandler,
-  emailNotificationsHandler,
+  emailNotificationsHandler,
+  createOrgGithubTeamHandler,
 } from "./handlers/index.js";
 // ...
   [AvailableSQSFunctions.EmailNotifications]: emailNotificationsHandler,
   [AvailableSQSFunctions.CreateOrgGithubTeam]: createOrgGithubTeamHandler,

Additionally, consider awaiting the handler so the “Finished” log prints after completion:

-          const result = func(
+          const result = await func(
             parsedBody.payload,
             parsedBody.metadata,
             childLogger,
           );

Also applies to: 43-44

🧰 Tools
🪛 ESLint

[error] 25-25: Unexpected use of file extension "js" for "./handlers/createOrgGithubTeam.js"

(import/extensions)

🤖 Prompt for AI Agents
In src/api/sqs/index.ts around line 25 (and similarly lines 43-44), remove the
explicit handler import that includes the “.js” extension and instead import the
handler symbol from the existing handlers barrel (./handlers) to satisfy ESLint
import/extensions and NodeNext semantics; then when invoking the handler, await
its returned promise so the "Finished" log is emitted only after the handler
completes. Ensure you consolidate imports into the barrel import at the top and
replace the non-awaited call with an awaited call where the "Finished" log
follows.


export type SQSFunctionPayloadTypes = {
[K in keyof typeof sqsPayloadSchemas]: SQSHandlerFunction<K>;
Expand All @@ -39,6 +40,7 @@ const handlers: SQSFunctionPayloadTypes = {
[AvailableSQSFunctions.ProvisionNewMember]: provisionNewMemberHandler,
[AvailableSQSFunctions.SendSaleEmail]: sendSaleEmailHandler,
[AvailableSQSFunctions.EmailNotifications]: emailNotificationsHandler,
[AvailableSQSFunctions.CreateOrgGithubTeam]: createOrgGithubTeamHandler,
};
export const runEnvironment = process.env.RunEnvironment as RunEnvironment;
export const currentEnvironmentConfig = environmentConfig[runEnvironment];
Expand Down
Loading
Loading