diff --git a/ydb/core/tx/tx_proxy/schemereq.cpp b/ydb/core/tx/tx_proxy/schemereq.cpp index c16abd2785d1..adb81886b032 100644 --- a/ydb/core/tx/tx_proxy/schemereq.cpp +++ b/ydb/core/tx/tx_proxy/schemereq.cpp @@ -1107,19 +1107,20 @@ struct TBaseSchemeReq: public TActorBootstrapped { // Check admin restrictions and special cases if (modifyScheme.GetOperationType() == NKikimrSchemeOp::ESchemeOpAlterLogin) { - // User management is allowed to any user or (if configured so) to admins only - if (checkAdmin && !isAdmin) { - const auto errString = MakeAccessDeniedError(ctx, "attempt to manage user"); - auto issue = MakeIssue(NKikimrIssues::TIssuesIds::ACCESS_DENIED, errString); - ReportStatus(TEvTxUserProxy::TEvProposeTransactionStatus::EStatus::AccessDenied, nullptr, &issue, ctx); - return false; - } - allowACLBypass = checkAdmin && isAdmin; - + // EnableStrictUserManagement == false: + // - any user can manage users|groups, but require AlterSchema right + // + // EnableStrictUserManagement == true: + // - user can change password for himself, and does not require AlterSchema right + // - only admins can manage users|groups, and does not require AlterSchema right + // - database admin can't change other database admins + // - database admin can't change database admin group + // - database admin can't change database owner + // const auto& alterLogin = modifyScheme.GetAlterLogin(); - // Any user can change their own password (but nothing else) - auto isUserChangesOwnPassword = [](const auto& alterLogin, const NACLib::TSID& subjectSid) { + // User changes password for himself (and only password) + bool isUserChangesOwnPassword = [](const auto& alterLogin, const NACLib::TSID& subjectSid) { if (alterLogin.GetAlterCase() == NKikimrSchemeOp::TAlterLogin::kModifyUser) { const auto& targetUser = alterLogin.GetModifyUser(); if (targetUser.HasPassword() && !targetUser.HasCanLogin()) { @@ -1127,11 +1128,21 @@ struct TBaseSchemeReq: public TActorBootstrapped { } } return false; - }; - allowACLBypass = allowACLBypass || isUserChangesOwnPassword(alterLogin, UserToken->GetUserSID()); + }(alterLogin, UserToken->GetUserSID()); + + bool allowManageUser = !checkAdmin || isAdmin || isUserChangesOwnPassword; + + if (!allowManageUser) { + const auto errString = MakeAccessDeniedError(ctx, "attempt to manage user"); + auto issue = MakeIssue(NKikimrIssues::TIssuesIds::ACCESS_DENIED, errString); + ReportStatus(TEvTxUserProxy::TEvProposeTransactionStatus::EStatus::AccessDenied, nullptr, &issue, ctx); + return false; + } + + allowACLBypass = (checkAdmin && isAdmin) || isUserChangesOwnPassword; // Database admin is not allowed to manage group of database admins (its the privilege of cluster admins). - if (IsDatabaseAdministrator) { + if (checkAdmin && IsDatabaseAdministrator) { TString group; switch (alterLogin.GetAlterCase()) { case NKikimrSchemeOp::TAlterLogin::kAddGroupMembership: diff --git a/ydb/tests/functional/tenants/test_user_administration.py b/ydb/tests/functional/tenants/test_user_administration.py new file mode 100644 index 000000000000..21497c81f88e --- /dev/null +++ b/ydb/tests/functional/tenants/test_user_administration.py @@ -0,0 +1,188 @@ +# -*- coding: utf-8 -*- +import copy +import logging + +import pytest + +from ydb.tests.library.harness.kikimr_config import KikimrConfigGenerator +from ydb.tests.library.harness.util import LogLevels +from ydb.tests.oss.ydb_sdk_import import ydb + +logger = logging.getLogger(__name__) + + +# local configuration for the ydb cluster (fetched by ydb_cluster_configuration fixture) +CLUSTER_CONFIG = dict( + additional_log_configs={ + # more logs + 'TX_PROXY': LogLevels.DEBUG, + # less logs + 'FLAT_TX_SCHEMESHARD': LogLevels.INFO, + 'KQP_PROXY': LogLevels.CRIT, + 'KQP_WORKER': LogLevels.CRIT, + 'KQP_GATEWAY': LogLevels.CRIT, + 'GRPC_PROXY': LogLevels.ERROR, + 'TX_DATASHARD': LogLevels.ERROR, + 'TX_PROXY_SCHEME_CACHE': LogLevels.ERROR, + 'GRPC_SERVER': LogLevels.CRIT, + 'KQP_YQL': LogLevels.ERROR, + 'KQP_SESSION': LogLevels.CRIT, + 'KQP_COMPILE_ACTOR': LogLevels.CRIT, + 'PERSQUEUE_CLUSTER_TRACKER': LogLevels.CRIT, + 'SCHEME_BOARD_SUBSCRIBER': LogLevels.CRIT, + 'SCHEME_BOARD_POPULATOR': LogLevels.CRIT, + }, + extra_feature_flags=[ + 'enable_strict_acl_check', + 'enable_strict_user_management', + 'enable_database_admin', + ], + # do not clutter logs with resource pools auto creation + enable_resource_pools=False, + enforce_user_token_requirement=True, + # make all bootstrap and setup under this user + default_clusteradmin='root@builtin', +) + + +# ydb_fixtures.ydb_cluster_configuration local override +@pytest.fixture(scope='module') +def ydb_cluster_configuration(): + conf = copy.deepcopy(CLUSTER_CONFIG) + return conf + + +@pytest.fixture(scope='module') +def ydb_configurator(ydb_cluster_configuration): + config_generator = KikimrConfigGenerator(**ydb_cluster_configuration) + config_generator.yaml_config['auth_config'] = { + 'domain_login_only': False, + } + config_generator.yaml_config['domains_config']['disable_builtin_security'] = True + security_config = config_generator.yaml_config['domains_config']['security_config'] + security_config['administration_allowed_sids'].append('clusteradmin') + return config_generator + + +@pytest.fixture(scope='module') +def prepared_root_db(ydb_cluster, ydb_root, ydb_endpoint): + cluster_admin = ydb.AuthTokenCredentials(ydb_cluster.config.default_clusteradmin) + + # prepare root database + driver_config = ydb.DriverConfig(ydb_endpoint, ydb_root, credentials=cluster_admin) + with ydb.Driver(driver_config) as driver: + pool = ydb.SessionPool(driver) + with pool.checkout() as session: + session.execute_scheme("create user clusteradmin password '1234'") + + +@pytest.fixture(scope='module') +def prepared_tenant_db(ydb_cluster, ydb_endpoint, ydb_database_module_scope): + cluster_admin = ydb.AuthTokenCredentials(ydb_cluster.config.default_clusteradmin) + + # prepare tenant database + database_path = ydb_database_module_scope + driver_config = ydb.DriverConfig(ydb_endpoint, database_path, credentials=cluster_admin) + with ydb.Driver(driver_config) as driver: + pool = ydb.SessionPool(driver) + with pool.checkout() as session: + # setup for database admins, first + session.execute_scheme("create user dbadmin password '1234'") + session.execute_scheme('create group dbadmins') + session.execute_scheme('alter group dbadmins add user dbadmin') + + # additional setup for individual tests + session.execute_scheme("create user ordinaryuser password '1234'") + + session.execute_scheme("create group ordinarygroup") + session.execute_scheme("create user dbadmin2 password '1234'") + session.execute_scheme("create group dbsubadmins") + session.execute_scheme('alter group dbadmins add user dbadmin2, dbsubadmins') + + # setup for database admins, second + # make dbadmin the real admin of the database + driver.scheme_client.modify_permissions(database_path, ydb.ModifyPermissionsSettings().change_owner('dbadmins')) + + return database_path + + +# python sdk does not legally expose login method, so that is a crude way +# to get auth-token in exchange to credentials +def login_user(endpoint, database, user, password): + driver_config = ydb.DriverConfig(endpoint, database) + credentials = ydb.StaticCredentials(driver_config, user, password) + return credentials._make_token_request()['access_token'] + + +def test_ordinaryuser_can_change_password_for_himself(ydb_endpoint, prepared_root_db, prepared_tenant_db, ydb_client): + database_path = prepared_tenant_db + + user_auth_token = login_user(ydb_endpoint, database_path, 'ordinaryuser', '1234') + credentials = ydb.AuthTokenCredentials(user_auth_token) + + with ydb_client(database_path, credentials=credentials) as driver: + driver.wait() + + pool = ydb.SessionPool(driver) + with pool.checkout() as session: + session.execute_scheme("alter user ordinaryuser password '4321'") + + user_auth_token = login_user(ydb_endpoint, database_path, 'ordinaryuser', '4321') + + +def test_database_admin_cant_change_database_owner(ydb_endpoint, prepared_root_db, prepared_tenant_db, ydb_client): + database_path = prepared_tenant_db + + user_auth_token = login_user(ydb_endpoint, database_path, 'dbadmin', '1234') + credentials = ydb.AuthTokenCredentials(user_auth_token) + + with ydb_client(database_path, credentials=credentials) as driver: + driver.wait() + + with pytest.raises(ydb.issues.Error) as exc_info: + driver.scheme_client.modify_permissions(database_path, ydb.ModifyPermissionsSettings().change_owner('ordinaryuser')) + + assert exc_info.type is ydb.issues.Unauthorized + assert 'Access denied for dbadmin' in exc_info.value.message + + +@pytest.mark.parametrize('query', [ + pytest.param('alter group dbadmins add user ordinaryuser', id='add-user'), + pytest.param('alter group dbadmins drop user dbadmin', id='remove-himself'), + pytest.param('alter group dbadmins drop user dbadmin2', id='remove-other-admin'), + pytest.param('alter group dbadmins add user ordinarygroup', id='add-subgroup'), + pytest.param('alter group dbadmins drop user dbsubadmins', id='remove-subgroup'), + pytest.param('drop group dbadmins', id='remove-admin-group'), + pytest.param('alter group dbadmins rename to dbadminsdemoted', id='rename-admin-group'), + +]) +def test_database_admin_cant_change_database_admin_group(ydb_endpoint, prepared_root_db, prepared_tenant_db, ydb_client, query): + database_path = prepared_tenant_db + + user_auth_token = login_user(ydb_endpoint, database_path, 'dbadmin', '1234') + credentials = ydb.AuthTokenCredentials(user_auth_token) + + with ydb_client(database_path, credentials=credentials) as driver: + driver.wait() + + pool = ydb.SessionPool(driver) + with pool.checkout() as session: + with pytest.raises(ydb.issues.Error) as exc_info: + session.execute_scheme(query) + + assert exc_info.type is ydb.issues.Unauthorized + assert 'Access denied.' in exc_info.value.message + + +def test_database_admin_can_create_user(ydb_endpoint, prepared_root_db, prepared_tenant_db, ydb_client): + database_path = prepared_tenant_db + + user_auth_token = login_user(ydb_endpoint, database_path, 'dbadmin', '1234') + credentials = ydb.AuthTokenCredentials(user_auth_token) + + with ydb_client(database_path, credentials=credentials) as driver: + driver.wait() + + pool = ydb.SessionPool(driver) + with pool.checkout() as session: + session.execute_scheme("create user testuser password '1234'") diff --git a/ydb/tests/functional/tenants/ya.make b/ydb/tests/functional/tenants/ya.make index 1e12a21e946f..bed91ee88d66 100644 --- a/ydb/tests/functional/tenants/ya.make +++ b/ydb/tests/functional/tenants/ya.make @@ -13,6 +13,7 @@ TEST_SRCS( test_system_views.py test_auth_system_views.py test_publish_into_schemeboard_with_common_ssring.py + test_user_administration.py test_users_groups_with_acl.py )