From 7721ba87c5eaede8397a0c47421fb964ef21991f Mon Sep 17 00:00:00 2001 From: Sergey Antropoff Date: Mon, 13 Jul 2026 01:40:49 +0300 Subject: [PATCH] feat: complete group ACL permission matrix --- README.md | 6 ++ app/api/registry.py | 7 ++- app/db/migrations/006_group_acl.sql | 9 +++ app/simulation/seed.py | 79 +++++++++++++++++++++++++-- docs/original-prompt-gap-plan.md | 4 +- tests/compatibility/test_proxmoxer.py | 29 +++++++++- tests/integration/test_migrations.py | 8 +++ tests/unit/test_migrations.py | 1 + 8 files changed, 133 insertions(+), 10 deletions(-) create mode 100644 app/db/migrations/006_group_acl.sql diff --git a/README.md b/README.md index c3f92ad..e8e7d56 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,12 @@ work, while QEMU power operations return 403. API-token requests do not require CSRF; ticket-authenticated mutations still do. These are disposable local test credentials only. +The permission acceptance matrix additionally seeds an audit-only user through +an inherited group ACL, a VM operator, and a storage-scoped user. Compatibility +tests verify root access, inherited `Sys.Audit`/`VM.Audit`, operator power +management, token privilege intersection, denial, and identical denial for an +existing and a nonexistent VM when the principal lacks `VM.Audit`. + Token lifecycle is available at `/access/users/{userid}/token[/{tokenid}]`. A generated secret is returned only by create or explicit regenerate; only its scrypt hash is stored. List/read never diff --git a/app/api/registry.py b/app/api/registry.py index 0020c58..851c866 100644 --- a/app/api/registry.py +++ b/app/api/registry.py @@ -178,7 +178,12 @@ async def _authorize( rows = await database.pool.fetch( """SELECT a.path, a.propagate, r.privileges FROM acl_entries a JOIN roles r ON r.name=a.role_name - JOIN principals p ON p.id=a.principal_id WHERE p.name=$1""", + JOIN principals p ON p.id=a.principal_id WHERE p.name=$1 + UNION ALL + SELECT a.path, a.propagate, r.privileges + FROM group_acl_entries a JOIN roles r ON r.name=a.role_name + JOIN identity_group_members m ON m.group_id=a.group_id + JOIN principals p ON p.id=m.principal_id WHERE p.name=$1""", principal, ) entries = tuple( diff --git a/app/db/migrations/006_group_acl.sql b/app/db/migrations/006_group_acl.sql new file mode 100644 index 0000000..8b7d374 --- /dev/null +++ b/app/db/migrations/006_group_acl.sql @@ -0,0 +1,9 @@ +CREATE TABLE group_acl_entries ( + group_id uuid NOT NULL REFERENCES identity_groups(id) ON DELETE CASCADE, + role_name text NOT NULL REFERENCES roles(name) ON DELETE RESTRICT, + path text NOT NULL, + propagate boolean NOT NULL DEFAULT true, + PRIMARY KEY (group_id, role_name, path) +); +CREATE INDEX identity_group_members_principal_idx + ON identity_group_members(principal_id, group_id); diff --git a/app/simulation/seed.py b/app/simulation/seed.py index 1959cf1..969b9df 100644 --- a/app/simulation/seed.py +++ b/app/simulation/seed.py @@ -357,12 +357,29 @@ async def apply_seed(connection: Connection, profile: SeedProfile) -> None: ["Sys.Audit", "VM.Audit"], ) await connection.execute( - """INSERT INTO acl_entries(principal_id, role_name, path, propagate) - VALUES($1, 'PVEAuditor', '/', true) - ON CONFLICT (principal_id, role_name, path) DO UPDATE - SET propagate=EXCLUDED.propagate""", + "DELETE FROM acl_entries WHERE principal_id=$1 AND role_name='PVEAuditor'", auditor_id, ) + auditor_group_id = stable_id("group:auditors") + await connection.execute( + """INSERT INTO identity_groups(id, group_id, comment) + VALUES($1, 'auditors', 'Read-only operators') + ON CONFLICT (group_id) DO UPDATE SET comment=EXCLUDED.comment""", + auditor_group_id, + ) + await connection.execute( + """INSERT INTO identity_group_members(group_id, principal_id) + VALUES($1, $2) ON CONFLICT DO NOTHING""", + auditor_group_id, + auditor_id, + ) + await connection.execute( + """INSERT INTO group_acl_entries(group_id, role_name, path, propagate) + VALUES($1, 'PVEAuditor', '/', true) + ON CONFLICT (group_id, role_name, path) DO UPDATE + SET propagate=EXCLUDED.propagate""", + auditor_group_id, + ) await connection.execute( """INSERT INTO api_tokens(principal_id, token_id, secret_hash, privileges) VALUES($1, 'readonly', $2, $3) @@ -372,6 +389,60 @@ async def apply_seed(connection: Connection, profile: SeedProfile) -> None: hash_secret("readonly-secret", salt=b"pve-readonly-v1"), ["Sys.Audit", "VM.Audit"], ) + for username, role_name, privileges, acl_path, token_id, token_secret in ( + ( + "operator@pve", + "PVEVMOperator", + ["VM.Audit", "VM.PowerMgmt"], + "/vms", + "operator", + "operator-secret", + ), + ( + "storage@pve", + "PVEStorageUser", + ["Datastore.Audit", "Datastore.AllocateSpace"], + "/storage", + "storage", + "storage-secret", + ), + ): + principal_id = stable_id(f"principal:{username}") + await connection.execute( + """INSERT INTO principals(id, name, password_hash, realm_name) + VALUES($1, $2, $3, 'pve') + ON CONFLICT (name) DO UPDATE SET password_hash=EXCLUDED.password_hash, + realm_name=EXCLUDED.realm_name""", + principal_id, + username, + hash_secret(f"{username}-password", salt=f"seed:{username}".encode()), + ) + await connection.execute( + """INSERT INTO roles(name, privileges) VALUES($1, $2) + ON CONFLICT (name) DO UPDATE SET privileges=EXCLUDED.privileges""", + role_name, + privileges, + ) + await connection.execute( + """INSERT INTO acl_entries(principal_id, role_name, path, propagate) + VALUES($1, $2, $3, true) + ON CONFLICT (principal_id, role_name, path) DO UPDATE + SET propagate=EXCLUDED.propagate""", + principal_id, + role_name, + acl_path, + ) + await connection.execute( + """INSERT INTO api_tokens(principal_id, token_id, secret_hash, privileges) + VALUES($1, $2, $3, $4) + ON CONFLICT (principal_id, token_id) DO UPDATE + SET secret_hash=EXCLUDED.secret_hash, privileges=EXCLUDED.privileges, + privilege_separation=true""", + principal_id, + token_id, + hash_secret(token_secret, salt=f"token:{username}".encode()), + privileges, + ) async def seed_url( diff --git a/docs/original-prompt-gap-plan.md b/docs/original-prompt-gap-plan.md index 0cb6baf..0e6475c 100644 --- a/docs/original-prompt-gap-plan.md +++ b/docs/original-prompt-gap-plan.md @@ -45,9 +45,9 @@ large seeding proves bounded batch operations rather than row-at-a-time inserts. `PVEAPIToken=USER@REALM!TOKENID=SECRET` without CSRF. - [x] Complete pam, pve, and test realm behavior, ticket skew/expiry and credential redaction. -- [ ] Wire users, groups, roles, ACL propagation, route-derived permissions and +- [x] Wire users, groups, roles, ACL propagation, route-derived permissions and token privilege separation into every semantic handler. -- [ ] Test root, audit-only, VM operator, storage user, separated token, +- [x] Test root, audit-only, VM operator, storage user, separated token, inheritance, denial, and existence-hiding behavior. Exit: the complete credential/permission matrix passes through HTTP and no diff --git a/tests/compatibility/test_proxmoxer.py b/tests/compatibility/test_proxmoxer.py index 64f5d8d..661e896 100644 --- a/tests/compatibility/test_proxmoxer.py +++ b/tests/compatibility/test_proxmoxer.py @@ -44,6 +44,8 @@ def test_proxmoxer_read_and_qemu_task_flow() -> None: verify_ssl=False, ) assert readonly_api.nodes.get() + assert readonly_api.nodes("pve1").status.get()["status"] == "online" + assert readonly_api.nodes("pve1").qemu("101").config.get()["vmid"] == 101 with pytest.raises(ResourceException) as denied: readonly_api.nodes("pve1").qemu("101").status.start.post() assert denied.value.status_code == 403 @@ -68,13 +70,34 @@ def test_proxmoxer_read_and_qemu_task_flow() -> None: ephemeral.nodes.get() assert removed.value.status_code == 401 + storage_api = ProxmoxAPI( + os.environ["PROXMOXER_HOST"], + port=int(os.getenv("PROXMOXER_PORT", "8007")), + user="storage@pve", + token_name=os.getenv("PROXMOXER_STORAGE_TOKEN_NAME", "storage"), + token_value=os.getenv("PROXMOXER_STORAGE_TOKEN_SECRET", "storage-secret"), + verify_ssl=False, + ) + for vmid in ("101", "999999"): + with pytest.raises(ResourceException) as hidden: + storage_api.nodes("pve1").qemu(vmid).config.get() + assert hidden.value.status_code == 403 + if os.getenv("PROXMOXER_MUTATION_TEST") == "1": - status = proxmox.nodes("pve1").qemu("101").status.current.get() + operator_api = ProxmoxAPI( + os.environ["PROXMOXER_HOST"], + port=int(os.getenv("PROXMOXER_PORT", "8007")), + user="operator@pve", + token_name=os.getenv("PROXMOXER_OPERATOR_TOKEN_NAME", "operator"), + token_value=os.getenv("PROXMOXER_OPERATOR_TOKEN_SECRET", "operator-secret"), + verify_ssl=False, + ) + status = operator_api.nodes("pve1").qemu("101").status.current.get() operation = "start" if status["status"] == "stopped" else "stop" - endpoint = proxmox.nodes("pve1").qemu("101").status(operation) + endpoint = operator_api.nodes("pve1").qemu("101").status(operation) upid = endpoint.post() for _attempt in range(100): - task = proxmox.nodes("pve1").tasks(upid).status.get() + task = operator_api.nodes("pve1").tasks(upid).status.get() if task["status"] == "stopped": break Event().wait(0.05) diff --git a/tests/integration/test_migrations.py b/tests/integration/test_migrations.py index c63a166..268938e 100644 --- a/tests/integration/test_migrations.py +++ b/tests/integration/test_migrations.py @@ -60,6 +60,14 @@ async def test_small_seed_is_idempotent() -> None: assert await connection.fetchval("SELECT count(*) FROM containers") == 1 assert await connection.fetchval("SELECT count(*) FROM storages") == 2 assert await connection.fetchval("SELECT count(*) FROM storage_contents") == 4 + assert await connection.fetchval("SELECT count(*) FROM identity_groups") == 1 + assert await connection.fetchval("SELECT count(*) FROM identity_group_members") == 1 + assert await connection.fetchval("SELECT count(*) FROM group_acl_entries") == 1 + assert await connection.fetchval("SELECT count(*) FROM roles") == 3 + assert await connection.fetchval("SELECT count(*) FROM api_tokens") == 4 + secrets = await connection.fetch("SELECT secret_hash FROM api_tokens") + assert all(str(row["secret_hash"]).startswith("scrypt$") for row in secrets) + assert all("-secret" not in str(row["secret_hash"]) for row in secrets) finally: await connection.close() diff --git a/tests/unit/test_migrations.py b/tests/unit/test_migrations.py index 566821a..464b6b4 100644 --- a/tests/unit/test_migrations.py +++ b/tests/unit/test_migrations.py @@ -50,3 +50,4 @@ def test_repository_migration_defines_required_planes() -> None: "fault_injections", ): assert f"CREATE TABLE {table}" in domain + assert "CREATE TABLE group_acl_entries" in migrations[5].sql