Publish module grants in the permission manifest

docs/02-iam/rbac-model.md

@@ -25,6 +25,51 @@ grants are how an access-control system becomes unauditable: within a year nobod
 revoke an offer?" without a full-table scan and a guess. If someone needs a capability, either their role
 gets it or a new role exists.
 
+### One basic role, any number of additional roles
+
+Roles are not interchangeable, and treating them as one flat list is what lets a student be made Dean
+Academic.
+
+| | Basic role | Additional roles |
+|---|---|---|
+| How many | exactly one | any number, including none |
+| Where from | ERP `globals_extrainfo.user_type` | ERP `globals_holdsdesignation` |
+| Assignable | no — it is what a person *is* | yes |
+| Values | `student`, `faculty`, `staff` | the 113 designations |
+
+The basic role is a role like any other in the grant tables — `RolePermission(designation="faculty", …)`
+resolves for every faculty member without an ERP row saying so. Before this rule existed, 14 permissions
+and 6 module grants were declared against `faculty` and reached nobody, and 121 staff who hold no
+designation had no role at all.
+
+`active_role` defaults to the first additional role and falls back to the basic role, so a Junior
+Assistant lands in their office rather than on a generic staff view.
+
+### Who may hold what
+
+`iam_role` catalogues each designation as a **rank**, an **office** or a **functional** role, and records
+which basic roles may hold it:
+
+| Category | Example | May be held by |
+|---|---|---|
+| `basic` | `student` | itself |
+| `rank` | Professor, Associate Professor | faculty |
+| `office` | Dean Academic, HOD (CSE) | faculty |
+| `office` | acadadmin, Registrar, Junior Assistant | faculty or staff |
+| `functional` | co-ordinator, Convenor, mess_committee | anyone, students included |
+
+The functional row is the point: a student cannot be a Professor but can be a club co-ordinator.
+
+`sync_identity` checks every projected assignment against the catalogue and writes what fails to
+`iam_role_violation`. It **reports** by default and only withholds the role when
+`IAM_ENFORCE_ROLE_POLICY` is on — the catalogue is a claim about institute practice, and refusing before
+that claim is confirmed would revoke access from whoever it is wrong about. An uncatalogued designation
+is allowed, because the academic office adds designations without telling this service.
+
+Two live violations exist today: `23BCS265` (student) holds Dean Academic, which grants 54 permissions
+including `curriculum.course.manage`; `ntripathi` (staff) holds Assistant Professor. Enforcing reduces
+the first to 18.
+
 **Two independent checks on every request:**
 
 | Check | Question | Source | Cost |

modules/accesscontrol/management/commands/permission_manifest.py

@@ -4,6 +4,7 @@
     manage.py permission_manifest --check    fail if stale or ungrantable
 """
 import json
+import re
 from importlib import import_module
 from pathlib import Path
 
@@ -37,11 +38,24 @@ def collect() -> dict:
                 for code, label in getattr(reg, "PERMISSIONS", [])
             ],
             "system_permissions": sorted(getattr(reg, "SYSTEM_PERMISSIONS", [])),
+            # Enforced by narrowing a queryset rather than by refusing a
+            # request. Holding one widens what you see; not holding it shows
+            # you less. There is no endpoint to check it in, so the
+            # unenforced check has to be told.
+            "scope_permissions": sorted(getattr(reg, "SCOPE_PERMISSIONS", [])),
             "grants": {
                 designation: sorted(set(codes))
                 for designation, codes
                 in sorted(getattr(reg, "ROLE_GRANTS", {}).items())
             },
+            # Derived, never listed separately. A designation holding any of a
+            # module's permissions must be able to enter the module — the two
+            # gates are separate checks, and seeding only one leaves every screen
+            # 403 with the permissions apparently correct.
+            "module_grants": sorted(
+                designation
+                for designation, codes in getattr(reg, "ROLE_GRANTS", {}).items()
+                if codes),
         }
     return {"version": VERSION, "modules": modules}
 
@@ -51,6 +65,7 @@ def problems(manifest: dict) -> list[str]:
     for code, spec in manifest["modules"].items():
         declared = {p["code"] for p in spec["permissions"]}
         system = set(spec["system_permissions"])
+        scoped = set(spec.get("scope_permissions", []))
         granted = {c for codes in spec["grants"].values() for c in codes}
 
         found.extend(
@@ -71,6 +86,97 @@ def problems(manifest: dict) -> list[str]:
             f"{code}: {both} is both a system permission and granted to a "
             f"designation. Pick one."
             for both in sorted(system & granted))
+        # None means the module's code could not be located, so there is
+        # nothing to judge. Reporting every permission as unenforced in that
+        # case is exactly the failure this check is meant to prevent.
+        # A system permission is enforced by whichever service calls it, which
+        # need not be inside this module, so it is out of scope here.
+        enforced = _enforced(code)
+        if enforced is not None:
+            found.extend(
+                f"{code}: {unused} is declared and granted, but no endpoint or "
+                f"service checks it — so the thing it names cannot be done at "
+                f"all. Either the feature is missing, or the permission is."
+                for unused in sorted(
+                    declared - system - scoped - enforced - KNOWN_UNENFORCED))
+    return found
+
+
+def known_gaps(manifest: dict) -> list[str]:
+    """The allowlisted ones that are still true, so they stay countable.
+
+    Reported on every run rather than suppressed. An allowlist nobody sees is
+    how a temporary exception becomes permanent.
+    """
+    return sorted(
+        code for spec_code, spec in manifest["modules"].items()
+        for code in ({p["code"] for p in spec["permissions"]}
+                     & KNOWN_UNENFORCED) - (_enforced(spec_code) or set()))
+
+
+
+
+
+#: Permissions declared before the check existed, whose feature has not
+#: been built yet. Each is a real gap, listed so a NEW one fails CI
+#: immediately instead of joining a pile nobody counts. Deleting a line
+#: here is the definition of done for that feature.
+KNOWN_UNENFORCED = frozenset({
+})
+
+
+def _module_paths() -> dict[str, Path]:
+    """Module code -> the directory its code lives in.
+
+    Resolved through the app registry, not by joining the code onto a path: the
+    placement module's code is `placement_cell` while its package is
+    `modules.placement`, and guessing the directory found nothing and reported
+    every one of its 21 permissions as unenforced.
+    """
+    out: dict[str, Path] = {}
+    for cfg in apps.get_app_configs():
+        if not cfg.name.startswith("modules."):
+            continue
+        try:
+            reg = import_module(f"{cfg.name}.registry")
+        except ModuleNotFoundError:
+            continue
+        spec = getattr(reg, "MODULE", None)
+        out[spec["code"] if spec else cfg.label] = Path(cfg.path)
+    return out
+
+
+def _enforced(module_code: str) -> set[str] | None:
+    """Permission codes that appear anywhere in the module's own code.
+
+    A grep, deliberately: a permission is enforced by being passed to
+    HasPermission or authz.require, and both take a plain string. Anything
+    cleverer would be a static analysis that the next way of checking defeats.
+
+    `selectors/` does not count. That is the architecture's own line: a
+    selector decides what you can see and authorises nothing, so a permission
+    appearing only there hid three `*.manage` permissions whose write path did
+    not exist.
+
+    `domain/` does count. It holds the tables that say which permission a step
+    needs — STEP_PERMISSIONS, OFFICE_PERMISSIONS — and the service reads them
+    to make the check. That is authority stated once rather than inlined.
+
+    registry.py is skipped because that is where they are declared, and tests
+    because a test naming a permission is not an endpoint enforcing it.
+    """
+    root = _module_paths().get(module_code)
+    if root is None:
+        return None
+    found: set[str] = set()
+    for path in root.rglob("*.py"):
+        parts = set(path.parts)
+        if "tests" in parts or path.name == "registry.py":
+            continue
+        if "selectors" in parts:
+            continue
+        found.update(re.findall(rf"{re.escape(module_code)}\.[a-z_]+\.[a-z_]+",
+                                path.read_text()))
     return found
 
 

registry/permissions.json

@@ -18,12 +18,20 @@
           "directory.user.search"
         ]
       },
+      "module_grants": [
+        "Dean Academic",
+        "acadadmin",
+        "placement_chairman",
+        "placement_coordinator",
+        "placement_officer"
+      ],
       "permissions": [
         {
           "code": "directory.user.search",
           "label": "Search the institute directory"
         }
       ],
+      "scope_permissions": [],
       "system_permissions": []
     },
     "placement_cell": {
@@ -81,6 +89,14 @@
           "placement_cell.registration.self"
         ]
       },
+      "module_grants": [
+        "Dean Academic",
+        "acadadmin",
+        "placement_chairman",
+        "placement_coordinator",
+        "placement_officer",
+        "student"
+      ],
       "permissions": [
         {
           "code": "placement_cell.job_posting.view",
@@ -167,6 +183,7 @@
           "label": "Browse every student's declared CPI"
         }
       ],
+      "scope_permissions": [],
       "system_permissions": [
         "placement_cell.application.auto_withdraw",
         "placement_cell.offer.expire"