Skip to content

Commit 4316175

Browse files
authored
IAM Group Management updates (#32)
* update logging * prevent adding non-paid members to groups which require paid membership * fix linting * fix lint * fix lockfile * ui: use correct group in correct environment, and fix error notifications for partial errors * add gpt-generated tests for the group management panel * add health check to pipeline
1 parent 75a086d commit 4316175

File tree

12 files changed

+1243
-37
lines changed

12 files changed

+1243
-37
lines changed

.github/workflows/deploy-dev.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,5 +88,7 @@ jobs:
8888
uses: actions/setup-python@v5
8989
with:
9090
python-version: 3.11
91+
- name: Run health check
92+
run: make dev_health_check
9193
- name: Run live testing
9294
run: make test_live_integration

.husky/pre-commit

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,17 @@
1-
yarn lint --fix
1+
#!/bin/sh
2+
. "$(dirname "$0")/_/husky.sh"
3+
4+
# Get all staged files
5+
STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACMR)
6+
7+
if [ -n "$STAGED_FILES" ]; then
8+
echo "Running lint with fix on staged files..."
9+
# Run lint on all files (modifies files in the working directory)
10+
yarn lint --fix
11+
12+
echo "Re-adding originally staged files to the staging area..."
13+
# Re-add only the originally staged files
14+
echo "$STAGED_FILES" | xargs git add
15+
else
16+
echo "No staged files to process."
17+
fi

src/api/functions/entraId.ts

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
1-
import { genericConfig } from "../../common/config.js";
21
import {
2+
execCouncilGroupId,
3+
execCouncilTestingGroupId,
4+
genericConfig,
5+
officersGroupId,
6+
officersGroupTestingId,
7+
} from "../../common/config.js";
8+
import {
9+
BaseError,
310
EntraGroupError,
411
EntraInvitationError,
512
InternalServerError,
@@ -190,7 +197,32 @@ export async function modifyGroup(
190197
message: "User's domain must be illinois.edu to be added to the group.",
191198
});
192199
}
193-
200+
// if adding to exec group, check that all exec members we want to add are paid members
201+
const paidMemberRequiredGroups = [
202+
execCouncilGroupId,
203+
execCouncilTestingGroupId,
204+
officersGroupId,
205+
officersGroupTestingId,
206+
];
207+
if (
208+
paidMemberRequiredGroups.includes(group) &&
209+
action === EntraGroupActions.ADD
210+
) {
211+
const netId = email.split("@")[0];
212+
const response = await fetch(
213+
`https://membership.acm.illinois.edu/api/v1/checkMembership?netId=${netId}`,
214+
);
215+
const membershipStatus = (await response.json()) as {
216+
netId: string;
217+
isPaidMember: boolean;
218+
};
219+
if (!membershipStatus["isPaidMember"]) {
220+
throw new EntraGroupError({
221+
message: `${netId} is not a paid member. This group requires that all members are paid members.`,
222+
group,
223+
});
224+
}
225+
}
194226
try {
195227
const oid = await resolveEmailToOid(token, email);
196228
const methodMapper = {
@@ -220,6 +252,12 @@ export async function modifyGroup(
220252
const errorData = (await response.json()) as {
221253
error?: { message?: string };
222254
};
255+
if (
256+
errorData?.error?.message ===
257+
"One or more added object references already exist for the following modified properties: 'members'."
258+
) {
259+
return true;
260+
}
223261
throw new EntraGroupError({
224262
message: errorData?.error?.message ?? response.statusText,
225263
group,
@@ -231,12 +269,15 @@ export async function modifyGroup(
231269
if (error instanceof EntraGroupError) {
232270
throw error;
233271
}
234-
235-
throw new EntraGroupError({
236-
message: error instanceof Error ? error.message : String(error),
237-
group,
238-
});
272+
const message = error instanceof Error ? error.message : String(error);
273+
if (message) {
274+
throw new EntraGroupError({
275+
message,
276+
group,
277+
});
278+
}
239279
}
280+
return false;
240281
}
241282

242283
/**

src/api/index.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,24 @@ import { InternalServerError } from "../common/errors/index.js";
1010
import eventsPlugin from "./routes/events.js";
1111
import cors from "@fastify/cors";
1212
import fastifyZodValidationPlugin from "./plugins/validate.js";
13-
import { environmentConfig } from "../common/config.js";
13+
import { environmentConfig, genericConfig } from "../common/config.js";
1414
import organizationsPlugin from "./routes/organizations.js";
1515
import icalPlugin from "./routes/ics.js";
1616
import vendingPlugin from "./routes/vending.js";
1717
import * as dotenv from "dotenv";
1818
import iamRoutes from "./routes/iam.js";
1919
import ticketsPlugin from "./routes/tickets.js";
20+
import { STSClient, GetCallerIdentityCommand } from "@aws-sdk/client-sts";
21+
2022
dotenv.config();
2123

2224
const now = () => Date.now();
2325

2426
async function init() {
2527
const app: FastifyInstance = fastify({
26-
logger: true,
28+
logger: {
29+
level: process.env.LOG_LEVEL || "info",
30+
},
2731
disableRequestLogging: true,
2832
genReqId: (request) => {
2933
const header = request.headers["x-apigateway-event"];
@@ -90,12 +94,22 @@ async function init() {
9094
}
9195

9296
if (import.meta.url === `file://${process.argv[1]}`) {
93-
// local development
97+
console.log(`Logging level set to ${process.env.LOG_LEVEL || "info"}`);
98+
const client = new STSClient({ region: genericConfig.AwsRegion });
99+
const command = new GetCallerIdentityCommand({});
100+
try {
101+
const data = await client.send(command);
102+
console.log(`Logged in to AWS as ${data.Arn} on account ${data.Account}.`);
103+
} catch {
104+
console.error(
105+
`Could not get AWS STS credentials: are you logged in to AWS? Run "aws configure sso" to log in.`,
106+
);
107+
process.exit(1);
108+
}
94109
const app = await init();
95-
app.listen({ port: 8080 }, (err) => {
110+
app.listen({ port: 8080 }, async (err) => {
96111
/* eslint no-console: ["error", {"allow": ["log", "error"]}] */
97112
if (err) console.error(err);
98-
console.log("Server listening on 8080");
99113
});
100114
}
101115
export default init;

src/api/package.json

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,13 @@
88
"type": "module",
99
"scripts": {
1010
"build": "tsc",
11-
"dev": "tsx watch index.ts",
11+
"dev": "cross-env LOG_LEVEL=debug tsx watch index.ts",
1212
"typecheck": "tsc --noEmit",
1313
"lint": "eslint . --ext .ts --cache",
1414
"prettier": "prettier --check *.ts **/*.ts",
1515
"prettier:write": "prettier --write *.ts **/*.ts"
16+
},
17+
"dependencies": {
18+
"@aws-sdk/client-sts": "^3.726.0"
1619
}
17-
}
20+
}

src/api/routes/iam.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,18 @@ import {
1515
EntraInvitationError,
1616
InternalServerError,
1717
NotFoundError,
18+
ValidationError,
1819
} from "../../common/errors/index.js";
1920
import {
2021
DynamoDBClient,
2122
GetItemCommand,
2223
PutItemCommand,
2324
} from "@aws-sdk/client-dynamodb";
24-
import { genericConfig } from "../../common/config.js";
25+
import {
26+
execCouncilGroupId,
27+
execCouncilTestingGroupId,
28+
genericConfig,
29+
} from "../../common/config.js";
2530
import { marshall, unmarshall } from "@aws-sdk/util-dynamodb";
2631
import {
2732
InviteUserPostRequest,

src/common/config.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ type EnvironmentConfigType = {
3939

4040
export const infraChairsGroupId = "48591dbc-cdcb-4544-9f63-e6b92b067e33";
4141
export const officersGroupId = "ff49e948-4587-416b-8224-65147540d5fc";
42+
export const officersGroupTestingId = "0e6e9199-506f-4ede-9d1b-e73f6811c9e5";
4243
export const execCouncilGroupId = "ad81254b-4eeb-4c96-8191-3acdce9194b1";
44+
export const execCouncilTestingGroupId = "dbe18eb2-9675-46c4-b1ef-749a6db4fedd";
4345

4446
const genericConfig: GenericConfigType = {
4547
EventsDynamoTableName: "infra-core-api-events",
@@ -109,7 +111,7 @@ const environmentConfig: EnvironmentConfigType = {
109111
/^https:\/\/(?:.*\.)?acmuiuc\.pages\.dev$/,
110112
],
111113
AadValidClientId: "5e08cf0f-53bb-4e09-9df2-e9bdc3467296",
112-
},
114+
}
113115
};
114116

115117
export type SecretConfig = {

src/ui/config.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { execCouncilGroupId, execCouncilTestingGroupId } from '@common/config';
2+
13
export const runEnvironments = ['dev', 'prod', 'local-dev'] as const;
24
// local dev should be used when you want to test against a local instance of the API
35

@@ -9,6 +11,9 @@ export type ValidService = ValidServices;
911
export type ConfigType = {
1012
AadValidClientId: string;
1113
ServiceConfiguration: Record<ValidServices, ServiceConfiguration>;
14+
KnownGroupMappings: {
15+
Exec: string;
16+
};
1217
};
1318

1419
export type ServiceConfiguration = {
@@ -45,6 +50,9 @@ const environmentConfig: EnvironmentConfigType = {
4550
baseEndpoint: 'https://merchapi.acm.illinois.edu',
4651
},
4752
},
53+
KnownGroupMappings: {
54+
Exec: execCouncilTestingGroupId,
55+
},
4856
},
4957
dev: {
5058
AadValidClientId: 'd1978c23-6455-426a-be4d-528b2d2e4026',
@@ -65,6 +73,9 @@ const environmentConfig: EnvironmentConfigType = {
6573
baseEndpoint: 'https://merchapi.acm.illinois.edu',
6674
},
6775
},
76+
KnownGroupMappings: {
77+
Exec: execCouncilTestingGroupId,
78+
},
6879
},
6980
prod: {
7081
AadValidClientId: '43fee67e-e383-4071-9233-ef33110e9386',
@@ -85,6 +96,9 @@ const environmentConfig: EnvironmentConfigType = {
8596
baseEndpoint: 'https://merchapi.acm.illinois.edu',
8697
},
8798
},
99+
KnownGroupMappings: {
100+
Exec: execCouncilGroupId,
101+
},
88102
},
89103
} as const;
90104

0 commit comments

Comments
 (0)