NK-WP-0048-T08: identity-provisioner service-access groups ensure/revoke

POST /v1/groups/ensure and /v1/groups/revoke: reserved prefixes refused, no
user creation, validate-before-mutate, read-back before success, delete only
empty groups on request. The oidc-browser-relying-party type now names them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Assistant: claude-code
Assistant-Model: opus
Assistant-Process: 5457@bnt-lap001
Assistant-Session: 792bf84a-3783-4f5d-a811-cded61dd2012

identity-provisioner/provisioner.py

@@ -227,6 +227,58 @@ mutation Remove($userId: String!, $groupId: Int!) {
             timeout=timeout,
         )
 
+    def ensure_service_groups(self, payload: dict[str, Any]) -> Result:
+        """Create the listed service-access groups if absent and add the listed
+        memberships. Never removes anything and never creates users."""
+        groups, members = _service_group_request(payload)
+        token = self._login()
+        users, directory_groups = self._directory(token)
+        known = {str(u.get("id")) for u in users}
+        unknown = sorted({u for us in members.values() for u in us} - known)
+        if unknown:
+            raise ValueError(f"unknown directory users: {unknown}")
+        existed = {str(g.get("displayName")) for g in directory_groups}
+        ids = {name: self._ensure_group(token, directory_groups, name) for name in groups}
+        for group, users_ in members.items():
+            for username in users_:
+                self._add_group(token, username, ids[group])
+        for group, users_ in members.items():
+            for username in users_:
+                user = self._user(token, username) or {}
+                if group not in {str(g.get("displayName")) for g in user.get("groups", ())}:
+                    raise RuntimeError(f"membership of {username!r} in {group!r} did not read back")
+        return Result("netkingdom-lldap", ",".join(groups), "groups_ensured", set(groups) <= existed)
+
+    def revoke_service_groups(self, payload: dict[str, Any]) -> Result:
+        """Rollback of ensure_service_groups: remove exactly the listed memberships,
+        and delete a listed group only when delete_if_empty is true and no member
+        remains."""
+        groups, members = _service_group_request(payload)
+        delete_if_empty = payload.get("delete_if_empty", False)
+        if not isinstance(delete_if_empty, bool):
+            raise ValueError("delete_if_empty must be true or false")
+        token = self._login()
+        _, directory_groups = self._directory(token)
+        ids = {str(g.get("displayName")): int(g["id"]) for g in directory_groups}
+        for group, users_ in members.items():
+            if group not in ids:
+                continue
+            for username in users_:
+                user = self._user(token, username) or {}
+                if group in {str(g.get("displayName")) for g in user.get("groups", ())}:
+                    self._remove_group(token, username, ids[group])
+        deleted = []
+        if delete_if_empty:
+            for group in groups:
+                if group in ids and not self._group_members(token, ids[group]):
+                    self._gql(token, "mutation Del($id: Int!) { deleteGroup(groupId: $id) { ok } }", {"id": ids[group]})
+                    deleted.append(group)
+        return Result("netkingdom-lldap", ",".join(groups), "groups_revoked" if not deleted else "groups_revoked_and_deleted", False)
+
+    def _group_members(self, token: str, group_id: int) -> list[str]:
+        value = self._gql(token, "query G($id: Int!) { group(groupId: $id) { users { id } } }", {"id": group_id})
+        return [str(u["id"]) for u in (value.get("group") or {}).get("users", ())]
+
     def _directory(self, token: str) -> tuple[list[dict], list[dict]]:
         value = self._gql(token, "query { users { id email displayName } groups { id displayName } }", {})
         return list(value["users"]), list(value["groups"])
@@ -372,9 +424,45 @@ def _discard(exc: BaseException) -> None:
             pass
 
 
+SERVICE_GROUP = re.compile(r"[a-z][a-z0-9-]{1,62}")
+RESERVED_GROUP_PREFIXES = ("tenant", "netkingdom-", "lldap_")
+DIRECTORY_USERNAME = re.compile(r"[a-z][a-z0-9._-]{0,31}")
+
+
+def _service_group_request(payload: dict[str, Any]) -> tuple[list[str], dict[str, list[str]]]:
+    """Validate a service-access group request before anything is changed.
+
+    Service-access groups gate sign-in to an internal service (SecurityCaseType
+    oidc-browser-relying-party). Tenant, platform-suspension and LLDAP admin
+    groups are reserved and can never be touched through this path.
+    """
+    _required(payload, "groups", "idempotency_key", "correlation_id")
+    groups = payload["groups"]
+    if not isinstance(groups, list) or not groups or len(groups) > 16:
+        raise ValueError("groups must be a list of 1 to 16 names")
+    for name in groups:
+        if not isinstance(name, str) or not SERVICE_GROUP.fullmatch(name):
+            raise ValueError(f"invalid service group name {name!r}")
+        if name.startswith(RESERVED_GROUP_PREFIXES):
+            raise ValueError(f"group {name!r} is reserved")
+    members = payload.get("members") or {}
+    if not isinstance(members, dict):
+        raise ValueError("members must map group names to usernames")
+    for group, users in members.items():
+        if group not in groups:
+            raise ValueError(f"members names group {group!r}, which is not in groups")
+        if not isinstance(users, list) or not all(isinstance(u, str) and DIRECTORY_USERNAME.fullmatch(u) for u in users):
+            raise ValueError(f"members of {group!r} must be directory usernames")
+    return list(dict.fromkeys(groups)), {g: list(dict.fromkeys(u)) for g, u in members.items()}
+
+
 def dispatch(
     provisioner: LLDAPProvisioner, path: str, payload: dict[str, Any]
 ) -> Result | DriftResult:
+    if path == "/v1/groups/ensure":
+        return provisioner.ensure_service_groups(payload)
+    if path == "/v1/groups/revoke":
+        return provisioner.revoke_service_groups(payload)
     if path == "/v1/identities/tenant-access":
         return provisioner.tenant_access(payload)
     if path == "/v1/identities/provision":

identity-provisioner/tests/test_service_groups.py

@@ -0,0 +1,131 @@
+import unittest
+
+from provisioner import LLDAPProvisioner, dispatch
+
+
+class Directory(LLDAPProvisioner):
+    """In-memory LLDAP: users, groups and memberships."""
+
+    def __init__(self):
+        self.users = {"tegwick", "agent-codex"}
+        self.groups = {"lldap_admin": 1, "tenant:trial:one:users": 2, "forge-admins": 3}
+        self.members = {1: {"tegwick"}, 2: set(), 3: set()}
+        self.calls = []
+        self.drop_add = False
+
+    def _login(self):
+        return "test-token"
+
+    def _directory(self, token):
+        return ([{"id": u} for u in sorted(self.users)],
+                [{"id": i, "displayName": n} for n, i in self.groups.items()])
+
+    def _user(self, token, subject):
+        return {"id": subject, "groups": [{"id": i, "displayName": n} for n, i in self.groups.items()
+                                           if subject in self.members[i]]}
+
+    def _ensure_group(self, token, groups, name):
+        if name not in self.groups:
+            self.calls.append(("create", name))
+            self.groups[name] = max(self.groups.values()) + 1
+            self.members[self.groups[name]] = set()
+        return self.groups[name]
+
+    def _add_group(self, token, username, group_id):
+        self.calls.append(("add", username, group_id))
+        if not self.drop_add:
+            self.members[group_id].add(username)
+
+    def _remove_group(self, token, username, group_id):
+        self.calls.append(("remove", username, group_id))
+        self.members[group_id].discard(username)
+
+    def _group_members(self, token, group_id):
+        return sorted(self.members[group_id])
+
+    def _gql(self, token, query, variables):
+        if "deleteGroup" in query:
+            self.calls.append(("delete", variables["id"]))
+            name = next(n for n, i in self.groups.items() if i == variables["id"])
+            del self.groups[name]
+            return {"deleteGroup": {"ok": True}}
+        raise AssertionError(query)
+
+
+def req(**kw):
+    p = {"groups": ["forge-admins", "forge-coulomb-owners", "forge-coulomb-readers"],
+         "members": {"forge-admins": ["tegwick"], "forge-coulomb-owners": ["tegwick"]},
+         "idempotency_key": "forgejo-netkingdom-oidc-1", "correlation_id": "test"}
+    p.update(kw)
+    return p
+
+
+class EnsureTests(unittest.TestCase):
+    def test_creates_missing_groups_and_adds_members(self):
+        d = Directory()
+        r = dispatch(d, "/v1/groups/ensure", req())
+        self.assertEqual("groups_ensured", r.status)
+        self.assertFalse(r.resumed)
+        self.assertEqual({"forge-coulomb-owners", "forge-coulomb-readers"},
+                         {c[1] for c in d.calls if c[0] == "create"})
+        self.assertIn("tegwick", d.members[d.groups["forge-admins"]])
+        self.assertIn("tegwick", d.members[d.groups["forge-coulomb-owners"]])
+
+    def test_idempotent_and_reports_resumed(self):
+        d = Directory()
+        dispatch(d, "/v1/groups/ensure", req())
+        d.calls.clear()
+        r = dispatch(d, "/v1/groups/ensure", req())
+        self.assertTrue(r.resumed)
+        self.assertEqual([], [c for c in d.calls if c[0] == "create"])
+
+    def test_never_removes_other_memberships(self):
+        d = Directory()
+        d.members[3].add("agent-codex")
+        dispatch(d, "/v1/groups/ensure", req())
+        self.assertIn("agent-codex", d.members[3])
+        self.assertEqual([], [c for c in d.calls if c[0] == "remove"])
+
+    def test_reserved_and_invalid_requests_change_nothing(self):
+        bad = [req(groups=["lldap_admin"]), req(groups=["tenant-x"]), req(groups=["netkingdom-suspended"]),
+               req(groups=["Forge Admins"]), req(groups=[]),
+               req(members={"not-listed": ["tegwick"]}),
+               req(members={"forge-admins": ["Not A User"]}),
+               req(members={"forge-admins": ["nobody"]})]
+        for payload in bad:
+            d = Directory()
+            with self.assertRaises(ValueError, msg=payload):
+                dispatch(d, "/v1/groups/ensure", payload)
+            self.assertEqual([], d.calls, payload)
+
+    def test_failed_readback_never_reports_success(self):
+        d = Directory()
+        d.drop_add = True
+        with self.assertRaises(RuntimeError):
+            dispatch(d, "/v1/groups/ensure", req())
+
+
+class RevokeTests(unittest.TestCase):
+    def test_removes_only_listed_memberships_and_keeps_groups_by_default(self):
+        d = Directory()
+        dispatch(d, "/v1/groups/ensure", req())
+        d.members[d.groups["forge-admins"]].add("agent-codex")
+        r = dispatch(d, "/v1/groups/revoke", req())
+        self.assertEqual("groups_revoked", r.status)
+        self.assertEqual({"agent-codex"}, d.members[d.groups["forge-admins"]])
+        self.assertIn("forge-coulomb-owners", d.groups)
+
+    def test_deletes_only_empty_listed_groups_when_asked(self):
+        d = Directory()
+        dispatch(d, "/v1/groups/ensure", req())
+        d.members[d.groups["forge-admins"]].add("agent-codex")
+        r = dispatch(d, "/v1/groups/revoke", req(delete_if_empty=True))
+        self.assertEqual("groups_revoked_and_deleted", r.status)
+        self.assertIn("forge-admins", d.groups)
+        self.assertNotIn("forge-coulomb-owners", d.groups)
+        self.assertNotIn("forge-coulomb-readers", d.groups)
+        self.assertIn("lldap_admin", d.groups)
+
+
+if __name__ == "__main__":
+    unittest.main()

security-case-types/oidc-browser-relying-party/README.md

@@ -80,8 +80,9 @@ confidential secrets from custody. That would be a 1.1 or 2.0 of this type.
 - No loopback or plain-http redirects (CLI and local development clients).
 - No resource `audience`, registration-bound `tenant` or custom scopes.
 - No service (`client_credentials`) clients: that is the `oidc-service-client` type.
-- Group creation assumes the identity-provisioner exposes `groups ensure`; until
-  it does, `ensure-groups` is the owner's open item (NK-WP-0048-T04).
+- Groups are service-access groups only: the identity-provisioner refuses
+  `tenant*`, `netkingdom-*` and `lldap_*` names on this path, and never creates
+  users (members must already exist).
 - `custody` uses the railiance-platform credential broker; that owner has not yet
   declared a `c3.runtime-secrets` capability, so `mint-secret` is a follow-through
   step rather than a composed capability. When it is declared, compose it.

security-case-types/oidc-browser-relying-party/type.yaml

@@ -124,8 +124,8 @@ spec:
   follow_through:
     - id: ensure-groups
       owner: net-kingdom
-      entry_point: "identity-provisioner groups ensure --groups {access_groups} --members '{initial_members}'"
-      auth: "NetKingdom identity-provisioner service identity with LLDAP group administration, via the ops-warden proxy"
+      entry_point: "identity-provisioner POST /v1/groups/ensure (groups {access_groups}; members {initial_members})"
+      auth: "identity-provisioner service token (bearer), via the ops-warden proxy; the provisioner holds the LLDAP admin credential"
       warden_executes: true
       attended: false
     - id: mint-secret
@@ -187,7 +187,7 @@ spec:
       when: "client_type == confidential"
       action: "destroy the secret at {secret_ref}"
     - reverses: ensure-groups
-      action: "remove the case's initial memberships; delete groups {access_groups} only if this case created them and no other case uses them"
+      action: "identity-provisioner POST /v1/groups/revoke with the case's memberships; delete_if_empty only if no other case uses {access_groups}"
     - reverses: client-secret-path
       when: "client_type == confidential"
       action: "remove the KV path {secret_ref}"

workplans/NK-WP-0048-security-case-types.md

@@ -223,7 +223,7 @@ rewriting them in place.
 
 ```task
 id: NK-WP-0048-T08
-status: todo
+status: done
 priority: high
 state_hub_task_id: "8e519082-dc8b-5e46-b660-03d03e55dcf6"
 ```
@@ -237,3 +237,13 @@ the general operation (create if absent, add the listed memberships, never
 remove others), its rollback (remove only this case's memberships), tests, and
 the ops-warden route for its service identity.
 
+2026-10-08: done. `identity-provisioner/provisioner.py` gained
+`ensure_service_groups` (`POST /v1/groups/ensure`) and `revoke_service_groups`
+(`POST /v1/groups/revoke`), behind the existing service-token check. Names
+matching `tenant*`, `netkingdom-*` and `lldap_*` are reserved; members must
+already exist; everything is validated before any change; memberships are read
+back before success; nothing else is ever removed; groups are deleted only when
+asked and empty. 7 new tests (39 in the provisioner suite). The type's
+`ensure-groups` step and its rollback now name these endpoints. The ops-warden
+route for the provisioner's service token belongs to WARDEN-WP-0044-T01.
+