Skip to content

fix(azure)!: remove group membership writes - #96

Merged
toto04 merged 1 commit into
mainfrom
fix/azure-remove-group-writes
Oct 26, 2026
Merged

toto04 merged 1 commit into
mainfrom
fix/azure-remove-group-writes

Conversation

@toto04

@toto04 toto04 commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Removes azure.groups.addMember / azure.groups.removeMember and the Graph/mock directory functions behind them. Any azure:groups:write holder 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 is azure.members.create adding the newly created user to Soci (RFC v3 §4 decision 9).
  • idp-router.test.ts now asserts azure.groups.getAll is the only azure.groups.* procedure.

The azure:groups:write permission has already been deleted from the IdP as an immediate mitigation; the matching admin dashboard PR removes the UI.

Breaking for @polinetwork/backend consumers: the two procedures are gone from the router type.

Test plan

  • pnpm check, pnpm typecheck, pnpm test, pnpm build

`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.
@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e88f503c-5580-430f-aa43-cd19ab2d6d73
Commits

Reviewing files that changed from the base of the PR and between b14229f and baf3a18.

Files selected for processing: 7
  • src/azure/directory.ts
  • src/azure/functions/groups.ts
  • src/azure/mock-directory.ts
  • src/azure/types.ts
  • src/routers/azure/groups.ts
  • tests/azure-router.test.ts
  • tests/idp-router.test.ts
Files with no reviewable changes: 3
  • src/azure/functions/groups.ts
  • src/azure/mock-directory.ts
  • src/azure/types.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The Azure directory and groups router no longer expose generic group membership add and remove operations. The router retains group listing, and the related tests now cover read authorization and the remaining groups procedure.

Changes

Azure group membership API

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: Normal

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: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: removing Azure group membership write operations. The breaking-change marker is appropriate because the procedures are removed.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@toto04
toto04 merged commit bc59023 into main Oct 26, 2026
2 checks passed
@toto04
toto04 deleted the fix/azure-remove-group-writes branch October 26, 2026 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant