From a49edbc2c9d4d906f0403bdf4c20f2fa49256497 Mon Sep 17 00:00:00 2001 From: Marcin Maliszkiewicz Date: Thu, 25 Sep 2025 10:42:40 +0200 Subject: [PATCH] auth: use old keyspace during auth-v1 consistently Before this patch we may trigger assertion on legacy_mode(_qp). That's because some auth startup is done in the background and assumes that auth version doesn't change in the middle of the startup. But topology coordinator may decide to do the migration at any time, regadless if auth service is fully started on all nodes. This change makes sure that in legacy startup flow we'll always use old auth-v1 keyspace and therefore auth version change in the middle won't negatively affect the flow. --- auth/password_authenticator.cc | 13 ++++++++++--- auth/roles-metadata.cc | 4 ++-- auth/standard_role_manager.cc | 13 ++++++------- auth/standard_role_manager.hh | 2 +- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/auth/password_authenticator.cc b/auth/password_authenticator.cc index 3cbc25d66a..4972fc719d 100644 --- a/auth/password_authenticator.cc +++ b/auth/password_authenticator.cc @@ -101,7 +101,11 @@ future<> password_authenticator::migrate_legacy_metadata() const { return do_for_each(*results, [this](const cql3::untyped_result_set_row& row) { auto username = row.get_as("username"); auto salted_hash = row.get_as(SALTED_HASH); - static const auto query = update_row_query(); + static const auto query = seastar::format("UPDATE {}.{} SET {} = ? WHERE {} = ?", + meta::legacy::AUTH_KS, + meta::roles_table::name, + SALTED_HASH, + meta::roles_table::role_col_name); return _qp.execute_internal( query, consistency_for_user(username), @@ -118,7 +122,6 @@ future<> password_authenticator::migrate_legacy_metadata() const { } future<> password_authenticator::legacy_create_default_if_missing() { - SCYLLA_ASSERT(legacy_mode(_qp)); const auto exists = co_await default_role_row_satisfies(_qp, &has_salted_hash, _superuser); if (exists) { co_return; @@ -127,7 +130,11 @@ future<> password_authenticator::legacy_create_default_if_missing() { if (salted_pwd.empty()) { salted_pwd = passwords::hash(DEFAULT_USER_PASSWORD, rng_for_salt); } - const auto query = update_row_query(); + const auto query = seastar::format("UPDATE {}.{} SET {} = ? WHERE {} = ?", + meta::legacy::AUTH_KS, + meta::roles_table::name, + SALTED_HASH, + meta::roles_table::role_col_name); co_await _qp.execute_internal( query, db::consistency_level::QUORUM, diff --git a/auth/roles-metadata.cc b/auth/roles-metadata.cc index 1f13151c58..2ddd30ea39 100644 --- a/auth/roles-metadata.cc +++ b/auth/roles-metadata.cc @@ -47,7 +47,7 @@ future default_role_row_satisfies( std::function p, std::optional rolename) { const sstring query = seastar::format("SELECT * FROM {}.{} WHERE {} = ?", - get_auth_ks_name(qp), + meta::legacy::AUTH_KS, meta::roles_table::name, meta::roles_table::role_col_name); @@ -68,7 +68,7 @@ future any_nondefault_role_row_satisfies( cql3::query_processor& qp, std::function p, std::optional rolename) { - const sstring query = seastar::format("SELECT * FROM {}.{}", get_auth_ks_name(qp), meta::roles_table::name); + const sstring query = seastar::format("SELECT * FROM {}.{}", meta::legacy::AUTH_KS, meta::roles_table::name); auto results = co_await qp.execute_internal(query, db::consistency_level::QUORUM , internal_distributed_query_state(), cql3::query_processor::cache_internal::no diff --git a/auth/standard_role_manager.cc b/auth/standard_role_manager.cc index a4340953fe..fba9b5b8c8 100644 --- a/auth/standard_role_manager.cc +++ b/auth/standard_role_manager.cc @@ -181,14 +181,13 @@ future<> standard_role_manager::create_legacy_metadata_tables_if_missing() const } future<> standard_role_manager::legacy_create_default_role_if_missing() { - SCYLLA_ASSERT(legacy_mode(_qp)); try { const auto exists = co_await default_role_row_satisfies(_qp, &has_can_login, _superuser); if (exists) { co_return; } const sstring query = seastar::format("INSERT INTO {}.{} ({}, is_superuser, can_login) VALUES (?, true, true)", - get_auth_ks_name(_qp), + meta::legacy::AUTH_KS, meta::roles_table::name, meta::roles_table::role_col_name); co_await _qp.execute_internal( @@ -283,7 +282,7 @@ future<> standard_role_manager::migrate_legacy_metadata() { std::move(config), ::service::group0_batch::unused(), [this](const auto& name, const auto& config, auto& mc) { - return create_or_replace(name, config, mc); + return create_or_replace(meta::legacy::AUTH_KS, name, config, mc); }); }).finally([results] {}); }).then([] { @@ -344,12 +343,12 @@ future<> standard_role_manager::ensure_superuser_is_created() { return _superuser_created_promise.get_shared_future(); } -future<> standard_role_manager::create_or_replace(std::string_view role_name, const role_config& c, ::service::group0_batch& mc) { +future<> standard_role_manager::create_or_replace(std::string_view auth_ks_name, std::string_view role_name, const role_config& c, ::service::group0_batch& mc) { const sstring query = seastar::format("INSERT INTO {}.{} ({}, is_superuser, can_login) VALUES (?, ?, ?)", - get_auth_ks_name(_qp), + auth_ks_name, meta::roles_table::name, meta::roles_table::role_col_name); - if (legacy_mode(_qp)) { + if (auth_ks_name == meta::legacy::AUTH_KS) { co_await _qp.execute_internal( query, consistency_for_role(role_name), @@ -368,7 +367,7 @@ standard_role_manager::create(std::string_view role_name, const role_config& c, throw role_already_exists(role_name); } - return create_or_replace(role_name, c, mc); + return create_or_replace(get_auth_ks_name(_qp), role_name, c, mc); }); } diff --git a/auth/standard_role_manager.hh b/auth/standard_role_manager.hh index e06737fe39..dac6471d9f 100644 --- a/auth/standard_role_manager.hh +++ b/auth/standard_role_manager.hh @@ -100,7 +100,7 @@ private: future<> maybe_create_default_role(); future<> maybe_create_default_role_with_retries(); - future<> create_or_replace(std::string_view role_name, const role_config&, ::service::group0_batch&); + future<> create_or_replace(std::string_view auth_ks_name, std::string_view role_name, const role_config&, ::service::group0_batch&); future<> legacy_modify_membership(std::string_view role_name, std::string_view grantee_name, membership_change);