-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Azure groups #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,52 @@ | ||||||
| import type { Group as TGroup, User as TUser } from "@microsoft/microsoft-graph-types" | ||||||
| import { logger } from "@/logger" | ||||||
| import { withRetry } from "@/utils/wait" | ||||||
| import { client } from "../client" | ||||||
| import type { ParsedGroup } from "../types" | ||||||
|
|
||||||
| export type Group = Pick<Required<TGroup>, "id" | "displayName" | "mailNickname" | "mailEnabled"> & { | ||||||
| members: Array<Pick<Required<TUser>, "id" | "displayName">> | ||||||
| } | ||||||
|
|
||||||
| export async function getAllGroups(): Promise<ParsedGroup[]> { | ||||||
| try { | ||||||
| const res: Group[] = await client | ||||||
| .api("/groups?$select=id,displayName,mailNickname,mailEnabled&$expand=members($select=id,displayName)") | ||||||
| .get() | ||||||
| .then((r) => r.value) | ||||||
| return res.map(({ mailNickname, mailEnabled, displayName, ...g }) => ({ | ||||||
| ...g, | ||||||
| displayName: displayName ?? "", | ||||||
| mailAddress: mailEnabled ? `${mailNickname}@polinetwork.org` : null, | ||||||
| members: g.members.map((m) => ({ ...m, displayName: m.displayName ?? "" })), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Safeguard against missing If a group has no members, the Graph API may omit the 🛡️ Proposed defensive fallback- members: g.members.map((m) => ({ ...m, displayName: m.displayName ?? "" })),
+ members: g.members?.map((m) => ({ ...m, displayName: m.displayName ?? "" })) ?? [],📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| })) | ||||||
| } catch (error) { | ||||||
| logger.error({ error }, "[MS Graph API] Error in getAllGroups call") | ||||||
| return [] | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| export async function addGroupMember(groupId: string, userId: string): Promise<boolean> { | ||||||
| try { | ||||||
| const res = withRetry(() => | ||||||
| client.api(`/groups/${groupId}/members/$ref`).post({ | ||||||
| "@odata.id": `https://graph.microsoft.com/v1.0/directoryObjects/${userId}`, | ||||||
| }) | ||||||
| ) | ||||||
|
Comment on lines
+31
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win Await the asynchronous The
📍 Affects 1 file
🤖 Prompt for AI Agents |
||||||
| logger.debug({ res, userId, groupId }, "[MS Graph API] OK addGroupMember call") | ||||||
| return true | ||||||
| } catch (error) { | ||||||
| logger.error({ error, userId, groupId }, "[MS Graph API] Error in addGroupMember call") | ||||||
| return false | ||||||
| } | ||||||
| } | ||||||
| export async function removeGroupMember(groupId: string, userId: string): Promise<boolean> { | ||||||
| try { | ||||||
| withRetry(() => client.api(`/groups/${groupId}/members/${userId}/$ref`).delete()) | ||||||
| logger.debug({ userId, groupId }, "[MS Graph API] OK removeGroupMember call") | ||||||
| return true | ||||||
| } catch (error) { | ||||||
| logger.error({ error, userId, groupId }, "[MS Graph API] Error in removeGroupMember call") | ||||||
| return false | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { z } from "zod" | ||
| import { addGroupMember, getAllGroups, removeGroupMember } from "@/azure/functions/groups" | ||
| import { createTRPCRouter, publicProcedure } from "@/trpc" | ||
|
|
||
| export default createTRPCRouter({ | ||
| getAll: publicProcedure.query(async () => { | ||
| return await getAllGroups() | ||
| }), | ||
| addMember: publicProcedure | ||
| .input(z.object({ groupId: z.string(), userId: z.string() })) | ||
| .mutation(async ({ input }) => { | ||
| return await addGroupMember(input.groupId, input.userId) | ||
| }), | ||
| removeMember: publicProcedure | ||
| .input(z.object({ groupId: z.string(), userId: z.string() })) | ||
| .mutation(async ({ input }) => { | ||
| return await removeGroupMember(input.groupId, input.userId) | ||
| }), | ||
|
Comment on lines
+6
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Find available secured TRPC procedures.
rg -o 'export const [a-zA-Z0-9_]+Procedure' src/trpc.tsRepository: PoliNetworkOrg/backend Length of output: 221 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== src/routers/azure/groups.ts ==\n'
cat -n src/routers/azure/groups.ts
printf '\n== src/trpc.ts ==\n'
cat -n src/trpc.ts
printf '\n== group helper references ==\n'
rg -n "getAllGroups|addGroupMember|removeGroupMember|protectedProcedure|publicProcedure" src -SRepository: PoliNetworkOrg/backend Length of output: 10947 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,220p' src/routers/azure/groups.ts
printf '\n---\n'
sed -n '1,220p' src/trpc.ts
printf '\n---\n'
rg -n "function (getAllGroups|addGroupMember|removeGroupMember)|const (getAllGroups|addGroupMember|removeGroupMember)|export .* (getAllGroups|addGroupMember|removeGroupMember)" src -SRepository: PoliNetworkOrg/backend Length of output: 3108 Restrict Azure group routes to authenticated access. 🤖 Prompt for AI Agents |
||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| import { createTRPCRouter } from "@/trpc" | ||
| import groups from "./groups" | ||
| import members from "./members" | ||
|
|
||
| export const azureRouter = createTRPCRouter({ | ||
| members, | ||
| groups, | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,9 +30,10 @@ export default createTRPCRouter({ | |
| }), | ||
|
|
||
| getLatestGuide: publicProcedure.output(guideSchema.nullable()).query(async () => { | ||
| const [latestGuide] = await DB.select().from(GUIDES_MATRICOLE).orderBy(desc(GUIDES_MATRICOLE.date)).limit(1) | ||
| const res = await DB.select().from(GUIDES_MATRICOLE).orderBy(desc(GUIDES_MATRICOLE.date)).limit(1) | ||
| console.log(res, res[0]) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Remove raw database-row logging from the public procedure.
🤖 Prompt for AI Agents |
||
|
|
||
| return latestGuide || null | ||
| return res[0] || null | ||
| }), | ||
|
|
||
| addGuide: publicProcedure | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import z from "zod" | |
| import { auth } from "./auth" | ||
| import { getMembers } from "./azure/functions/members" | ||
| import "./azure/blob" | ||
| import { addGroupMember, getAllGroups, removeGroupMember } from "./azure/functions/groups" | ||
| import { AUTH_PATH, TRPC_PATH, WS_PATH } from "./constants" | ||
| import { cron } from "./cron" | ||
| import { DB, SCHEMA } from "./db" | ||
|
|
@@ -87,6 +88,25 @@ app.get("/test/members", async (c) => { | |
| return c.json({ users }) | ||
| }) | ||
|
|
||
| app.get("/test/azure-groups", async (c) => { | ||
| if (env.NODE_ENV === "production") return c.status(500) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Return 404 instead of 500 for disabled dev routes. Returning a 500 (Internal Server Error) for routes that are intentionally disabled in production is semantically incorrect and will falsely trigger error monitors. Use a 403 or 404 response instead.
📍 Affects 1 file
🤖 Prompt for AI Agents |
||
|
|
||
| const groups = await getAllGroups() | ||
| return c.json({ groups }) | ||
| }) | ||
|
|
||
| app.post( | ||
| "/test/azure-group-member", | ||
| zValidator("json", z.object({ groupId: z.string(), userId: z.string(), mode: z.enum(["add", "remove"]) })), | ||
| async (c) => { | ||
| if (env.NODE_ENV === "production") return c.status(500) | ||
| const { userId, groupId, mode } = c.req.valid("json") | ||
|
|
||
| const ok = mode === "add" ? await addGroupMember(groupId, userId) : await removeGroupMember(groupId, userId) | ||
| return c.json({ ok }) | ||
| } | ||
| ) | ||
|
|
||
| app.all(`${WS_PATH}/`, (c) => { | ||
| return wssEngine.handleRequest(c.req.raw, server) | ||
| }) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: PoliNetworkOrg/backend
Length of output: 160
🏁 Script executed:
Repository: PoliNetworkOrg/backend
Length of output: 4364
Handle Graph pagination for groups and members. This only reads the first
value; later/groupspages and expandedmemberspages from@odata.nextLinkwill be dropped.🤖 Prompt for AI Agents