Repository navigation
feat(roles): add Presidente and Segretario built-in roles - #82
Conversation
Both are managed roles proven by direct membership of an Entra group (PN_ENTRA_PRESIDENTE_GROUP_ID, PN_ENTRA_SEGRETARIO_GROUP_ID), like Direttivo, and always inherit from Direttivo. Migration 0024 seeds them and the inheritance edge.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 47 minutes. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Role catalog and inheritance src/auth/rbac.ts, drizzle/0024_presidente_segretario_roles.sql, drizzle/meta/_journal.json, src/db/rbac.ts, src/auth/rbac.test.ts, README.md |
The catalog and migration add Presidente and Segretario with Direttivo as their parent. Validation requires intrinsic parents that remain in the catalog. Tests cover role inheritance and parent validation. |
Entra group configuration and evidence src/env.ts, scripts/security-config.mjs, src/auth/access-snapshot.ts, src/auth/membership.ts, src/auth/identity-subject.ts, src/auth/policy.ts, src/auth/account-overview.ts, .env.example, README.md |
Optional group IDs configure the office-role mappings and require complete Entra credentials when set. Successful membership checks add the corresponding states. The policy trusts those states as Entra evidence, and account overview treats them as groups. |
Possibly related PRs
- PoliNetworkOrg/auth#73: Adds People role bindings and excludes managed roles from People binding. This change adds Presidente and Segretario to the managed role catalog.
Change: Feature
Merge Risk: 
ce5dd
-
The new offices use direct Entra membership and inherit Direttivo access. No actionable merge-blocking risk was identified.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title "feat(roles): add Presidente and Segretario built-in roles" matches the main change in the changeset. The raw summary and PR objectives both show that the PR adds the two built-in roles, the… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "feat(roles): add Presidente and Segretario built-in roles" matches the main change in the changeset. The raw summary and PR objectives both show that the PR adds the two built-in roles, their Entra group mappings, the migration 0024 that seeds them, and their inheritance from Direttivo. The title is short, specific, and uses a conventional commit prefix. It does not cover the inheritance or the optional group configuration, which is acceptable for a title.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 10 files. (4 skipped: 4 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
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.
…n only Drop the Entra group wiring. The two board offices stay built-in (fixed key, cannot be deleted, always inherit from Direttivo), but are given and taken back by hand, and only by a Master Admin. Migration 0024 lets user_role hold these two managed roles.
Adds two built-in roles for the board offices:
presidente(Presidente) andsegretario(Segretario). Both always inherit fromdirettivo.Who assigns them
Only a Master Admin, by hand. Holding
idp:roles:assignis not enough, even if you hold everything the office grants. This matters because Direttivo holdsidp:roles:assigntoday.They are still built in: the key is fixed and they can't be deleted. They are the only built-in roles that take hand assignments. The others still come from evidence or config.
What changed to allow this:
0024updates theguard_managed_role_linkstrigger souser_roleaccepts these two keys and still rejects the other built-in roles.get_rolemembers.Inheriting Direttivo
0024adds the parent link from each office todirettivo.direttivoin their roles and all its permissions. They don't get thedirettivostate, which still comes only from the Entra group.Migration
0024adds the two roles (static-role-presidente,static-role-segretario, nosource_state), the parent links, and the updated trigger. If a hand-made role already uses one of these keys, it stops with an error instead of quietly treating it as the office. Production has no such role (checked read-only).Not in this PR
regulations:manage,regulations:read,regulations:write): the code only definesidp:*permissions. Everything else, and which role gets what, is set at runtime by admins. Create these and grant them in the admin UI when the regulations editor ships.Checks
vp checkandvp testpass.presidenteand rejectsdirettivo, and the full PostgreSQL suite passes.idp:roles:assigngets 403, a Master Admin can assign and revoke, and the holder getsdirettivo.denies bootstrapping new authority with both writer permissions, failed once in a full parallel run and passed in every other run, including 4 runs on its own. It looks flaky when it shares the DB with other test files.