Repository navigation
fix(azure)!: remove group membership writes - #96
Conversation
`azure.groups.addMember` and `azure.groups.removeMember` let any holder of `azure:groups:write` add any directory user to any Entra group, including the groups the IdP derives the Socio and Direttivo roles from. RFC v3 §4 decision 9 allows only one Azure group write: `azure.members.create` adding the user it just created to Soci. The procedures came back by mistake when read-only group browsing was restored. Groups stay readable through `azure.groups.getAll` (`azure:groups:read`), and the directory no longer exposes a general add/remove member function. BREAKING CHANGE: `azure.groups.addMember` and `azure.groups.removeMember` are removed. The `azure:groups:write` permission is no longer checked.
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Remove directory membership methods src/azure/types.ts, src/azure/functions/groups.ts, src/azure/mock-directory.ts, src/azure/directory.ts |
The directory contract, Graph functions, and mock directory no longer provide group membership add and remove methods. |
Retain group listing and update coverage src/routers/azure/groups.ts, tests/azure-router.test.ts, tests/idp-router.test.ts |
The router retains getAll and removes addMember and removeMember. Tests cover listing authorization and assert that getAll is the only azure.groups.* procedure. |
Possibly related PRs
- PoliNetworkOrg/backend#95: This PR synchronizes Microsoft 365 Soci membership and PoliNetwork Auth roles into People records; this change removes generic group membership writes that could alter IdP-derived roles.
Priority:
Change: Bug fix
Merge Risk: 
Minimal · up to baf3a
-
The removed group-write procedures are a breaking API change, but no actionable merge-blocking risk remains after normal checks.
Pre-merge checks: 
3 · 
1
-
Failed checks: 1 warning Passed checks: 3 passed -
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Summary
azure.groups.addMember/azure.groups.removeMemberand the Graph/mock directory functions behind them. Anyazure:groups:writeholder could add any user to any Entra group, including the Soci and Direttivo groups the IdP derives managed roles from (privilege escalation).azure.groups.getAll(azure:groups:read) stays: browsing groups is intended. The only group write left isazure.members.createadding the newly created user to Soci (RFC v3 §4 decision 9).idp-router.test.tsnow assertsazure.groups.getAllis the onlyazure.groups.*procedure.The
azure:groups:writepermission has already been deleted from the IdP as an immediate mitigation; the matching admin dashboard PR removes the UI.Breaking for
@polinetwork/backendconsumers: the two procedures are gone from the router type.Test plan
pnpm check,pnpm typecheck,pnpm test,pnpm build