diff --git a/auth/cache.cc b/auth/cache.cc index bed290a288..856c29ac73 100644 --- a/auth/cache.cc +++ b/auth/cache.cc @@ -185,24 +185,14 @@ future> cache::fetch_role(const role_name_t& r static const sstring q = format("SELECT role, name, value FROM {}.{} WHERE role = ?", db::system_keyspace::NAME, ROLE_ATTRIBUTES_CF); auto rs = co_await fetch(q); for (const auto& r : *rs) { + if (!r.has("value")) { + continue; + } rec->attributes[r.get_as("name")] = r.get_as("value"); co_await coroutine::maybe_yield(); } } - // permissions - { - static const sstring q = format("SELECT role, resource, permissions FROM {}.{} WHERE role = ?", db::system_keyspace::NAME, PERMISSIONS_CF); - auto rs = co_await fetch(q); - for (const auto& r : *rs) { - auto resource = r.get_as("resource"); - auto perms_strings = r.get_set("permissions"); - std::unordered_set perms_set(perms_strings.begin(), perms_strings.end()); - auto pset = permissions::from_strings(perms_set); - rec->permissions[std::move(resource)] = std::move(pset); - co_await coroutine::maybe_yield(); - } - } co_return rec; } diff --git a/auth/cache.hh b/auth/cache.hh index d63c02466f..305ec01f3f 100644 --- a/auth/cache.hh +++ b/auth/cache.hh @@ -44,7 +44,6 @@ public: std::unordered_set members; sstring salted_hash; std::unordered_map attributes; - std::unordered_map permissions; private: friend cache; // cached permissions include effects of role's inheritance diff --git a/auth/default_authorizer.cc b/auth/default_authorizer.cc index 02c96d3778..d00e518ef6 100644 --- a/auth/default_authorizer.cc +++ b/auth/default_authorizer.cc @@ -76,7 +76,11 @@ default_authorizer::authorize(const role_or_anonymous& maybe_role, const resourc if (results->empty()) { co_return permissions::NONE; } - co_return permissions::from_strings(results->one().get_set(PERMISSIONS_NAME)); + const auto& row = results->one(); + if (!row.has(PERMISSIONS_NAME)) { + co_return permissions::NONE; + } + co_return permissions::from_strings(row.get_set(PERMISSIONS_NAME)); } future<> diff --git a/test/cluster/auth_cluster/test_permissions_removal_restart.py b/test/cluster/auth_cluster/test_permissions_removal_restart.py new file mode 100644 index 0000000000..d17f4d3e7b --- /dev/null +++ b/test/cluster/auth_cluster/test_permissions_removal_restart.py @@ -0,0 +1,65 @@ +# +# Copyright (C) 2026-present ScyllaDB +# +# SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.1 +# + +import pytest +import logging + +from test.pylib.manager_client import ManagerClient +from test.cluster.auth_cluster import extra_scylla_config_options as auth_config + +logger = logging.getLogger(__name__) + + +@pytest.mark.asyncio +async def test_permissions_removal_and_restart(manager: ManagerClient) -> None: + """Test that a node boots successfully when role_permissions contains a + ghost row with role and resource set but the permissions column missing. + + The auth v2 migration (now removed) used INSERT to copy permission rows + from the legacy table, which created CQL row markers. Normal GRANT uses + UPDATE, which only writes collection cells without row markers. When + permissions were later revoked, the collection cells were tombstoned but + the row marker from the migration INSERT persisted. That leaves a row + with role and resource but no permissions column. + + This test simulates that scenario: + 1. INSERT permissions with row marker (simulating auth v2 migration) + 2. REVOKE ALL permissions (tombstones the cells, marker survives) + 3. Restart and verify the node boots successfully + """ + servers = await manager.servers_add(1, config=auth_config) + cql, _ = await manager.get_ready_cql(servers) + server = servers[0] + + await cql.run_async("CREATE ROLE scylla_admin WITH PASSWORD = 'x' AND LOGIN = true") + await cql.run_async("CREATE ROLE scylla_manager WITH PASSWORD = 'x' AND LOGIN = true") + + # Simulate auth v2 migration: INSERT creates a row marker alongside the + # permission cells, unlike GRANT which uses UPDATE (no row marker). + await cql.run_async( + "INSERT INTO system.role_permissions (role, resource, permissions) " + "VALUES ('scylla_admin', 'roles/scylla_manager', {'ALTER', 'AUTHORIZE', 'DROP'})") + + # Revoke all permissions — tombstones the collection cells, but the + # row marker from the INSERT survives, creating a ghost row. + await cql.run_async("REVOKE ALL ON ROLE scylla_manager FROM scylla_admin") + + # Additional check: a row with an explicitly empty permissions set. + await cql.run_async("CREATE ROLE test_empty_perms WITH PASSWORD = 'x' AND LOGIN = true") + await cql.run_async( + "INSERT INTO system.role_permissions (role, resource) " + "VALUES ('test_empty_perms', 'roles/scylla_manager')") + + # Restart — the auth cache loads the ghost row and must not crash + logger.info("Restarting node") + await manager.server_stop_gracefully(server.server_id) + await manager.server_start(server.server_id) + + await manager.driver_connect() + cql, _ = await manager.get_ready_cql(servers) + rows = await cql.run_async("SELECT * FROM system.local") + assert len(rows) == 1, "Node should be functional after restart" + logger.info("Node restarted successfully")