diff --git a/identity-provisioner/provisioner.py b/identity-provisioner/provisioner.py index 10a742a..1ae6d54 100644 --- a/identity-provisioner/provisioner.py +++ b/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": diff --git a/identity-provisioner/tests/test_service_groups.py b/identity-provisioner/tests/test_service_groups.py new file mode 100644 index 0000000..07a38ff --- /dev/null +++ b/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() diff --git a/security-case-types/oidc-browser-relying-party/README.md b/security-case-types/oidc-browser-relying-party/README.md index c9f2806..a5dbb5b 100644 --- a/security-case-types/oidc-browser-relying-party/README.md +++ b/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. diff --git a/security-case-types/oidc-browser-relying-party/type.yaml b/security-case-types/oidc-browser-relying-party/type.yaml index 1cce94d..cdf5310 100644 --- a/security-case-types/oidc-browser-relying-party/type.yaml +++ b/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}" diff --git a/workplans/NK-WP-0048-security-case-types.md b/workplans/NK-WP-0048-security-case-types.md index 81fce4b..85997eb 100644 --- a/workplans/NK-WP-0048-security-case-types.md +++ b/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. +