Merge 'auth: ensure default superuser password is set before serving CQL' from Andrzej Jackowski
Before this change, it was ensured that a default superuser is created before serving CQL. However, the mechanism didn't wait for default password initialization, so effectively, for a short period, customer couldn't authenticate as the superuser properily. The purpose of this change is to improve the superuser initialization mechanism to wait for superuser default password, just as for the superuser creation. This change: - Introduce authenticator::ensure_superuser_is_created() to allow waiting for complete initialization of super user authentication - Implement ensure_superuser_is_created in password_authenticator, so waiting for superuser password initialization is possible - Implement ensure_superuser_is_create in transitional_authenticator, so the implementation from password_authenticator is used - Implement no-op ensure_superuser_is_create for other authenticators - Extend service::ensure_superuser_is_created to wait for superuser initialization in authenticator, just as it was implemented earlier for role_manager - Add injected error (sleep) in password_authenticator::start to reproduce a case of delayed password creation - Implement test_delayed_deafult_password to verify the correctness of the fix - Ensure superuser is created in single_node_cql_env::run_in_thread to make single_node_cql more similar to scylla_main in main.cc Fixes scylladb/scylladb#20566 Backport not needed - a minor bugfix Closes scylladb/scylladb#22532 * github.com:scylladb/scylladb: test: implement test_auth_password_ensured test: implement connect_driver argument in ManagerClient::server_add auth: ensure default superuser password is set before serving CQL auth: added password_authenticator_start_pause injected error
This commit is contained in:
@@ -83,6 +83,10 @@ public:
|
||||
virtual ::shared_ptr<sasl_challenge> new_sasl_challenge() const override {
|
||||
throw std::runtime_error("Should not reach");
|
||||
}
|
||||
|
||||
virtual future<> ensure_superuser_is_created() const override {
|
||||
return make_ready_future<>();
|
||||
}
|
||||
};
|
||||
|
||||
}
|
||||
|
||||
@@ -159,6 +159,8 @@ public:
|
||||
virtual const resource_set& protected_resources() const = 0;
|
||||
|
||||
virtual ::shared_ptr<sasl_challenge> new_sasl_challenge() const = 0;
|
||||
|
||||
virtual future<> ensure_superuser_is_created() const = 0;
|
||||
};
|
||||
|
||||
}
|
||||
|
||||
@@ -56,6 +56,10 @@ public:
|
||||
const resource_set& protected_resources() const override;
|
||||
|
||||
::shared_ptr<sasl_challenge> new_sasl_challenge() const override;
|
||||
|
||||
virtual future<> ensure_superuser_is_created() const override {
|
||||
return make_ready_future<>();
|
||||
}
|
||||
private:
|
||||
};
|
||||
|
||||
|
||||
@@ -147,6 +147,9 @@ future<> password_authenticator::start() {
|
||||
_stopped = do_after_system_ready(_as, [this] {
|
||||
return async([this] {
|
||||
if (legacy_mode(_qp)) {
|
||||
if (!_superuser_created_promise.available()) {
|
||||
_superuser_created_promise.set_value();
|
||||
}
|
||||
_migration_manager.wait_for_schema_agreement(_qp.db().real_database(), db::timeout_clock::time_point::max(), &_as).get();
|
||||
|
||||
if (any_nondefault_role_row_satisfies(_qp, &has_salted_hash, _superuser).get()) {
|
||||
@@ -162,7 +165,11 @@ future<> password_authenticator::start() {
|
||||
return;
|
||||
}
|
||||
}
|
||||
utils::get_local_injector().inject("password_authenticator_start_pause", utils::wait_for_message(5min)).get();
|
||||
create_default_if_missing().get();
|
||||
if (!legacy_mode(_qp)) {
|
||||
_superuser_created_promise.set_value();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -361,4 +368,8 @@ const resource_set& password_authenticator::protected_resources() const {
|
||||
});
|
||||
}
|
||||
|
||||
future<> password_authenticator::ensure_superuser_is_created() const {
|
||||
return _superuser_created_promise.get_shared_future();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@
|
||||
#pragma once
|
||||
|
||||
#include <seastar/core/abort_source.hh>
|
||||
#include <seastar/core/shared_future.hh>
|
||||
|
||||
#include "db/consistency_level_type.hh"
|
||||
#include "auth/authenticator.hh"
|
||||
@@ -41,6 +42,7 @@ class password_authenticator : public authenticator {
|
||||
future<> _stopped;
|
||||
abort_source _as;
|
||||
std::string _superuser;
|
||||
shared_promise<> _superuser_created_promise;
|
||||
|
||||
public:
|
||||
static db::consistency_level consistency_for_user(std::string_view role_name);
|
||||
@@ -80,6 +82,8 @@ public:
|
||||
|
||||
virtual ::shared_ptr<sasl_challenge> new_sasl_challenge() const override;
|
||||
|
||||
virtual future<> ensure_superuser_is_created() const override;
|
||||
|
||||
private:
|
||||
bool legacy_metadata_exists() const;
|
||||
|
||||
|
||||
@@ -55,6 +55,10 @@ public:
|
||||
const resource_set& protected_resources() const override;
|
||||
|
||||
::shared_ptr<sasl_challenge> new_sasl_challenge() const override;
|
||||
|
||||
virtual future<> ensure_superuser_is_created() const override {
|
||||
return make_ready_future<>();
|
||||
}
|
||||
};
|
||||
|
||||
/// A set of four credential strings that saslauthd expects.
|
||||
|
||||
@@ -263,7 +263,8 @@ future<> service::stop() {
|
||||
}
|
||||
|
||||
future<> service::ensure_superuser_is_created() {
|
||||
return _role_manager->ensure_superuser_is_created();
|
||||
co_await _role_manager->ensure_superuser_is_created();
|
||||
co_await _authenticator->ensure_superuser_is_created();
|
||||
}
|
||||
|
||||
void service::update_cache_config() {
|
||||
|
||||
@@ -159,6 +159,10 @@ public:
|
||||
};
|
||||
return ::make_shared<sasl_wrapper>(_authenticator->new_sasl_challenge());
|
||||
}
|
||||
|
||||
virtual future<> ensure_superuser_is_created() const override {
|
||||
return _authenticator->ensure_superuser_is_created();
|
||||
}
|
||||
};
|
||||
|
||||
class transitional_authorizer : public authorizer {
|
||||
|
||||
53
test/auth_cluster/test_auth_password_ensured.py
Normal file
53
test/auth_cluster/test_auth_password_ensured.py
Normal file
@@ -0,0 +1,53 @@
|
||||
#
|
||||
# Copyright (C) 2025-present ScyllaDB
|
||||
#
|
||||
# SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0
|
||||
#
|
||||
|
||||
import pytest
|
||||
import logging
|
||||
import time
|
||||
|
||||
from cassandra.cluster import NoHostAvailable
|
||||
from test.auth_cluster.conftest import skip_mode
|
||||
from test.pylib.manager_client import ManagerClient, ServerUpState
|
||||
from test.pylib.util import wait_for
|
||||
|
||||
async def repeat_if_host_unavailable(f):
|
||||
async def try_execute(f):
|
||||
try:
|
||||
await f()
|
||||
return True
|
||||
except NoHostAvailable:
|
||||
return None
|
||||
return await wait_for(lambda: try_execute(f), time.time() + 60)
|
||||
|
||||
"""
|
||||
Test CQL is served only after superuser default password is created.
|
||||
After CQL is served, user is properily authenticated as superuser (not annonymous user)
|
||||
"""
|
||||
@pytest.mark.asyncio
|
||||
@skip_mode('release', 'error injection is disabled in release mode')
|
||||
async def test_auth_password_ensured(manager: ManagerClient) -> None:
|
||||
config = {
|
||||
'authenticator': "com.scylladb.auth.TransitionalAuthenticator",
|
||||
'error_injections_at_startup': ['password_authenticator_start_pause'],
|
||||
}
|
||||
server = await manager.server_add(config=config, expected_server_up_state=ServerUpState.HOST_ID_QUERIED, connect_driver=False)
|
||||
|
||||
logging.info("Waiting until PasswordAuthenticator pauses on the injected error")
|
||||
server_log = await manager.server_open_log(server.server_id)
|
||||
await server_log.wait_for("password_authenticator_start_pause: waiting for message")
|
||||
|
||||
with pytest.raises(NoHostAvailable, match="Unable to connect to any servers"):
|
||||
logging.info("Expecting driver connection failure, because password_authenticator_start_pause blocks serving CQL")
|
||||
await manager.driver_connect()
|
||||
|
||||
await manager.api.message_injection(server.ip_addr, 'password_authenticator_start_pause')
|
||||
await repeat_if_host_unavailable(manager.driver_connect)
|
||||
|
||||
cql, _ = await manager.get_ready_cql([server])
|
||||
|
||||
logging.info("Run CREATE USER to confirm successful superuser authentication")
|
||||
await cql.run_async("CREATE USER normal WITH PASSWORD '123456' NOSUPERUSER")
|
||||
|
||||
@@ -219,13 +219,13 @@ class ManagerClient():
|
||||
|
||||
async def server_start(self, server_id: ServerNum, expected_error: Optional[str] = None,
|
||||
wait_others: int = 0, wait_interval: float = 45, seeds: Optional[List[IPAddress]] = None,
|
||||
timeout: Optional[float] = None) -> None:
|
||||
timeout: Optional[float] = None, connect_driver: bool = True) -> None:
|
||||
"""Start specified server and optionally wait for it to learn of other servers"""
|
||||
logger.debug("ManagerClient starting %s", server_id)
|
||||
data = {"expected_error": expected_error, "seeds": seeds}
|
||||
await self.client.put_json(f"/cluster/server/{server_id}/start", data, timeout=timeout)
|
||||
await self.server_sees_others(server_id, wait_others, interval = wait_interval)
|
||||
if expected_error is None:
|
||||
if expected_error is None and connect_driver:
|
||||
if self.cql:
|
||||
self._driver_update()
|
||||
else:
|
||||
@@ -327,7 +327,8 @@ class ManagerClient():
|
||||
seeds: Optional[List[IPAddress]] = None,
|
||||
timeout: Optional[float] = ScyllaServer.TOPOLOGY_TIMEOUT,
|
||||
server_encryption: str = "none",
|
||||
expected_server_up_state: Optional[ServerUpState] = None) -> ServerInfo:
|
||||
expected_server_up_state: Optional[ServerUpState] = None,
|
||||
connect_driver: bool = True) -> ServerInfo:
|
||||
"""Add a new server"""
|
||||
try:
|
||||
data = self._create_server_add_data(
|
||||
@@ -364,7 +365,7 @@ class ManagerClient():
|
||||
except Exception as exc:
|
||||
raise RuntimeError(f"server_add got invalid server data {server_info}") from exc
|
||||
logger.debug("ManagerClient added %s", s_info)
|
||||
if expected_error is None:
|
||||
if expected_error is None and connect_driver:
|
||||
if self.cql:
|
||||
self._driver_update()
|
||||
elif start:
|
||||
|
||||
Reference in New Issue
Block a user