From 8c1fbd04fac0ee7bc510fa9eb8b18da66f56c449 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Thu, 27 Jun 2024 06:25:02 -0700 Subject: [PATCH] Simplify API for querying directory services users and groups Cache query responses are fast enough that we should include by default. Directory services users and groups can be reasonably omitted if filters explicitly call for local or builtin users. Remove currently-disabled old LDAP test and validate that DS users and groups returned via user.query and group.query. Add useful CallErrors if someone tries to modify DS user. Sometimes API consumers are over-enthusiastic about what they can do with users provided by AD / LDAP. --- src/middlewared/middlewared/main.py | 6 +- .../middlewared/plugins/account.py | 208 +++++++-- .../middlewared/plugins/account_/2fa.py | 2 +- .../middlewared/plugins/account_/privilege.py | 8 +- src/middlewared/middlewared/plugins/etc.py | 6 +- src/middlewared/middlewared/plugins/idmap.py | 3 +- .../middlewared/plugins/keychain.py | 4 +- src/middlewared/middlewared/plugins/smb.py | 2 +- .../middlewared/plugins/smb_/groupmap.py | 6 +- .../middlewared/plugins/smb_/passdb.py | 2 +- src/middlewared/middlewared/plugins/usage.py | 2 +- .../test/integration/assets/ftp.py | 2 +- tests/api2/test_036_ad_ldap.py | 429 ------------------ tests/api2/test_040_ad_user_group_cache.py | 54 +-- 14 files changed, 209 insertions(+), 525 deletions(-) delete mode 100644 tests/api2/test_036_ad_ldap.py diff --git a/src/middlewared/middlewared/main.py b/src/middlewared/middlewared/main.py index 1d1bf2abe2ebb..f6de5297212fd 100644 --- a/src/middlewared/middlewared/main.py +++ b/src/middlewared/middlewared/main.py @@ -796,7 +796,7 @@ async def run(self, ws, request, conndata): try: user = await self.middleware.call( 'user.query', - [['username', '=', token['username']]], + [['username', '=', token['username']], ['local', '=', True]], {'get': True}, ) except MatchNotFound: @@ -806,7 +806,9 @@ async def run(self, ws, request, conndata): if 'ALL' in user['sudo_commands'] or 'ALL' in user['sudo_commands_nopasswd']: as_root = True else: - for group in await self.middleware.call('group.query', [['id', 'in', user['groups']]]): + for group in await self.middleware.call('group.query', [ + ['id', 'in', user['groups']], ['local', '=', True] + ]): if 'ALL' in group['sudo_commands'] or 'ALL' in group['sudo_commands_nopasswd']: as_root = True break diff --git a/src/middlewared/middlewared/plugins/account.py b/src/middlewared/middlewared/plugins/account.py index c24eb19490f58..32c3d19c06397 100644 --- a/src/middlewared/middlewared/plugins/account.py +++ b/src/middlewared/middlewared/plugins/account.py @@ -24,6 +24,7 @@ import middlewared.sqlalchemy as sa from middlewared.utils import run, filter_list from middlewared.utils.crypto import sha512_crypt +from middlewared.utils.directoryservices.constants import DSType, DSStatus from middlewared.utils.nss import pwd, grp from middlewared.utils.nss.nss_common import NssModule from middlewared.utils.privilege import credential_has_full_admin, privileges_group_mapping @@ -34,6 +35,7 @@ ) from middlewared.plugins.smb_.constants import SMBBuiltin from middlewared.plugins.idmap_.idmap_constants import ( + BASE_SYNTHETIC_DATASTORE_ID, TRUENAS_IDMAP_DEFAULT_LOW, SID_LOCAL_USER_PREFIX, SID_LOCAL_GROUP_PREFIX @@ -123,6 +125,30 @@ def validate_sudo_commands(commands): return verrors +def filters_include_ds_accounts(filters): + """ Check for filters limiting to local accounts """ + for f in filters: + if len(f) < 3: + # OR -- assume evaluation for this will result in including DS + continue + + # Directory services do not provide builtin accounts + # local explicitly denotes not directory service + if f[0] in ('local', 'builtin'): + match f[1]: + case '=': + if f[2] is True: + return False + case '!=': + if f[2] is False: + return False + + case _: + pass + + return True + + class UserModel(sa.Model): __tablename__ = 'account_bsdusers' @@ -167,7 +193,7 @@ async def user_extend_context(self, rows, extra): [], {'prefix': 'bsdgrpmember_'} ) - group_roles = await self.middleware.call('group.query', [], {'select': ['id', 'roles']}) + group_roles = await self.middleware.call('group.query', [['local', '=', True]], {'select': ['id', 'roles']}) for i in res: uid = i['user']['id'] @@ -251,9 +277,6 @@ async def query(self, filters, options): The following `additional_information` options are supported: `SMB` - include Windows SID and NT Name for user. If this option is not specified, then these keys will have `null` value. - `DS` - include users from Directory Service (LDAP or Active Directory) in results - - `"extra": {"search_dscache": true}` is a legacy method of querying for directory services users. """ ds_users = [] options = options or {} @@ -269,13 +292,8 @@ async def query(self, filters, options): datastore_options.pop('select', None) extra = options.get('extra', {}) - dssearch = extra.pop('search_dscache', False) additional_information = extra.get('additional_information', []) - if 'DS' in additional_information: - dssearch = True - additional_information.remove('DS') - username_sid = {} if 'SMB' in additional_information: try: @@ -289,16 +307,23 @@ async def query(self, filters, options): # broken self.logger.error('Failed to retrieve passdb information', exc_info=True) - if dssearch: - ds_state = await self.middleware.call('directoryservices.get_state') - if ds_state['activedirectory'] == 'HEALTHY' or ds_state['ldap'] == 'HEALTHY': - ds_users = await self.middleware.call('directoryservices.cache.query', 'USER', filters, options.copy()) - # For AD users, we will not have 2FA attribute normalized so let's do that - ad_users_2fa_mapping = await self.middleware.call('auth.twofactor.get_ad_users') - for index, user in enumerate(filter( - lambda u: not u['local'] and 'twofactor_auth_configured' not in u, ds_users) - ): - ds_users[index]['twofactor_auth_configured'] = bool(ad_users_2fa_mapping.get(user['sid'])) + if filters_include_ds_accounts(filters): + ds = await self.middleware.call('directoryservices.status') + if ds['type'] is not None and ds['status'] == DSStatus.HEALTHY.name: + ds_users = await self.middleware.call( + 'directoryservices.cache.query', 'USER', filters, options.copy() + ) + + match DSType(ds['type']): + case DSType.AD: + ad_users_2fa_mapping = await self.middleware.call('auth.twofactor.get_ad_users') + for index, user in enumerate(filter( + lambda u: not u['local'] and 'twofactor_auth_configured' not in u, ds_users) + ): + ds_users[index]['twofactor_auth_configured'] = bool(ad_users_2fa_mapping.get(user['sid'])) + case _: + # FIXME - map twofactor_auth_configured hint for LDAP users + pass result = await self.middleware.call( 'datastore.query', self._config.datastore, [], datastore_options @@ -521,7 +546,10 @@ def do_create(self, data): group_created = False if create: - group = self.middleware.call_sync('group.query', [('group', '=', data['username'])]) + group = self.middleware.call_sync('group.query', [ + ('group', '=', data['username']), + ('local', '=', True) + ]) if group: group = group[0] else: @@ -532,7 +560,9 @@ def do_create(self, data): 'sudo_commands_nopasswd': [], 'allow_duplicate_gid': False }, False) - group = self.middleware.call_sync('group.query', [('id', '=', group)])[0] + group = self.middleware.call_sync('group.query', [ + ('id', '=', group), ('local', '=', True) + ])[0] group_created = True data['group'] = group['id'] @@ -544,7 +574,7 @@ def do_create(self, data): if data['smb']: groups.append((self.middleware.call_sync( - 'group.query', [('group', '=', 'builtin_users')], {'get': True}, + 'group.query', [('group', '=', 'builtin_users'), ('local', '=', True)], {'get': True}, ))['id']) if data.get('uid') is None: @@ -632,6 +662,23 @@ def do_update(self, app, audit_callback, pk, data): Update attributes of an existing user. """ + if pk > BASE_SYNTHETIC_DATASTORE_ID: + # datastore ids for directory services are created by adding the + # posix ID to a base value so that we can use getpwuid / getgrgid to + # convert back to a username / group name + try: + username = self.middleware.call_sync( + 'user.get_user_obj', {'uid': pk - BASE_SYNTHETIC_DATASTORE_ID} + )['pw_name'] + except KeyError: + username = 'UNKNOWN' + + audit_callback(username) + raise CallError( + 'Users provided by a directory service must be modified through the identity provider ' + '(LDAP server or domain controller).' + ) + user = self.middleware.call_sync('user.get_instance', pk) audit_callback(user['username']) @@ -860,6 +907,23 @@ def do_delete(self, audit_callback, pk, options): The `delete_group` option deletes the user primary group if it is not being used by any other user. """ + if pk > BASE_SYNTHETIC_DATASTORE_ID: + # datastore ids for directory services are created by adding the + # posix ID to a base value so that we can use getpwuid / getgrgid to + # convert back to a username / group name + try: + username = self.middleware.call_sync( + 'user.get_user_obj', {'uid': pk - BASE_SYNTHETIC_DATASTORE_ID} + )['pw_name'] + except KeyError: + username = 'UNKNOWN' + + audit_callback(username) + raise CallError( + 'Users provided by a directory service must be deleted from the identity provider ' + '(LDAP server or domain controller).' + ) + user = self.middleware.call_sync('user.get_instance', pk) audit_callback(user['username']) @@ -1196,21 +1260,64 @@ async def setup_local_administrator(self, app, username, password, options): raise CallError('Local administrator is already set up', errno.EEXIST) if username == 'admin': - if await self.middleware.call('user.query', [['uid', '=', ADMIN_UID]]): + # first check based on NSS to catch collisions with AD / LDAP users + try: + pwd_obj = await self.middleware.call('user.get_user_obj', {'uid': ADMIN_UID}) + raise CallError( + f'A {pwd_obj["source"].lower()} user with uid={ADMIN_UID} already exists, ' + 'setting up local administrator is not possible', + errno.EEXIST, + ) + except KeyError: + pass + + try: + pwd_obj = await self.middleware.call('user.get_user_obj', {'username': 'admin'}) + raise CallError(f'"admin" {pwd_obj["source"].lower()} user already exists, ' + 'setting up local administrator is not possible', + errno.EEXIST) + except KeyError: + pass + + try: + grp_obj = await self.middleware.call('group.get_group_obj', {'gid': ADMIN_GID}) + raise CallError( + f'A {grp_obj["source"].lower()} group with gid={ADMIN_GID} already exists, ' + 'setting up local administrator is not possible', + errno.EEXIST, + ) + except KeyError: + pass + + try: + grp_obj = await self.middleware.call('get.get_group_obj', {'groupname': 'admin'}) + raise CallError(f'"admin" {grp_obj["source"].lower()} group already exists, ' + 'setting up local administrator is not possible', + errno.EEXIST) + except KeyError: + pass + + # double-check our database in case we have for some reason failed to write to passwd + local_users = await self.middleware.call('user.query', [['local', '=', True]]) + local_groups = await self.middleware.call('group.query', [['local', '=', True]]) + + if filter_list(local_users, [['uid', '=', ADMIN_UID]]): raise CallError( f'A user with uid={ADMIN_UID} already exists, setting up local administrator is not possible', errno.EEXIST, ) - if await self.middleware.call('user.query', [['username', '=', 'admin']]): + + if filter_list(local_users, [['username', '=', 'admin']]): raise CallError('"admin" user already exists, setting up local administrator is not possible', errno.EEXIST) - if await self.middleware.call('group.query', [['gid', '=', ADMIN_GID]]): + if filter_list(local_groups, [['gid', '=', ADMIN_GID]]): raise CallError( f'A group with gid={ADMIN_GID} already exists, setting up local administrator is not possible', errno.EEXIST, ) - if await self.middleware.call('group.query', [['group', '=', 'admin']]): + + if filter_list(local_groups, [['group', '=', 'admin']]): raise CallError('"admin" group already exists, setting up local administrator is not possible', errno.EEXIST) @@ -1713,9 +1820,6 @@ async def query(self, filters, options): The following `additional_information` options are supported: `SMB` - include Windows SID and NT Name for group. If this option is not specified, then these keys will have `null` value. - `DS` - include groups from Directory Service (LDAP or Active Directory) in results - - `"extra": {"search_dscache": true}` is a legacy method of querying for directory services groups. """ ds_groups = [] options = options or {} @@ -1731,16 +1835,11 @@ async def query(self, filters, options): datastore_options.pop('select', None) extra = options.get('extra', {}) - dssearch = extra.pop('search_dscache', False) additional_information = extra.get('additional_information', []) - if 'DS' in additional_information: - dssearch = True - additional_information.remove('DS') - - if dssearch: - ds_state = await self.middleware.call('directoryservices.get_state') - if ds_state['activedirectory'] == 'HEALTHY' or ds_state['ldap'] == 'HEALTHY': + if filters_include_ds_accounts(filters): + ds = await self.middleware.call('directoryservices.status') + if ds['type'] is not None and ds['status'] == DSStatus.HEALTHY.name: ds_groups = await self.middleware.call('directoryservices.cache.query', 'GROUP', filters, options) if 'SMB' in additional_information: @@ -1848,6 +1947,24 @@ async def do_update(self, audit_callback, pk, data): Update attributes of an existing group. """ + if pk > BASE_SYNTHETIC_DATASTORE_ID: + # datastore ids for directory services are created by adding the + # posix ID to a base value so that we can use getpwuid / getgrgid to + # convert back to a username / group name + try: + groupname = self.middleware.call_sync( + 'group.get_group_obj', {'gid': pk - BASE_SYNTHETIC_DATASTORE_ID} + )['gr_name'] + except KeyError: + groupname = 'UNKNOWN' + + + audit_callback(groupname) + raise CallError( + 'Groups provided by a directory service must be modified through the identity provider ' + '(LDAP server or domain controller).' + ) + group = await self.get_instance(pk) audit_callback(group['name']) @@ -1925,6 +2042,23 @@ async def do_delete(self, audit_callback, pk, options): The `delete_users` option deletes all users that have this group as their primary group. """ + if pk > BASE_SYNTHETIC_DATASTORE_ID: + # datastore ids for directory services are created by adding the + # posix ID to a base value so that we can use getpwuid / getgrgid to + # convert back to a username / group name + try: + groupname = self.middleware.call_sync( + 'group.get_group_obj', {'gid': pk - BASE_SYNTHETIC_DATASTORE_ID} + )['gr_name'] + except KeyError: + groupname = 'UNKNOWN' + + audit_callback(groupname) + raise CallError( + 'Groups provided by a directory service must be deleted from the identity provider ' + '(LDAP server or domain controller).' + ) + group = await self.get_instance(pk) audit_callback(group['name'] + (' and all its users' if options['delete_users'] else '')) diff --git a/src/middlewared/middlewared/plugins/account_/2fa.py b/src/middlewared/middlewared/plugins/account_/2fa.py index b61f6e7b8e596..2d1f0c2f3bf36 100644 --- a/src/middlewared/middlewared/plugins/account_/2fa.py +++ b/src/middlewared/middlewared/plugins/account_/2fa.py @@ -103,7 +103,7 @@ async def translate_username(self, username): return await self.middleware.call( 'user.query', [['username', '=', user['pw_name']]], { 'get': True, - 'extra': {'additional_information': ['DS', 'SMB']}, + 'extra': {'additional_information': ['SMB']}, } ) diff --git a/src/middlewared/middlewared/plugins/account_/privilege.py b/src/middlewared/middlewared/plugins/account_/privilege.py index 4efc7c077b4da..cd14ea606ca8e 100644 --- a/src/middlewared/middlewared/plugins/account_/privilege.py +++ b/src/middlewared/middlewared/plugins/account_/privilege.py @@ -430,9 +430,9 @@ async def full_privilege(self): @private async def always_has_root_password_enabled(self, users=None, groups=None): if users is None: - users = await self.middleware.call('user.query') + users = await self.middleware.call('user.query', [['local', '=', True]]) if groups is None: - groups = await self.middleware.call('group.query') + groups = await self.middleware.call('group.query', [['local', '=', True]]) root_user = filter_list( users, @@ -458,9 +458,9 @@ async def always_has_root_password_enabled(self, users=None, groups=None): async def local_administrators(self, exclude_user_ids=None, users=None, groups=None): exclude_user_ids = exclude_user_ids or [] if users is None: - users = await self.middleware.call('user.query') + users = await self.middleware.call('user.query', [['local', '=', True]]) if groups is None: - groups = await self.middleware.call('group.query') + groups = await self.middleware.call('group.query', [['local', '=', True]]) local_administrator_privilege = await self.middleware.call( 'datastore.query', diff --git a/src/middlewared/middlewared/plugins/etc.py b/src/middlewared/middlewared/plugins/etc.py index 1ecb27b258206..fc6ecc99d430e 100644 --- a/src/middlewared/middlewared/plugins/etc.py +++ b/src/middlewared/middlewared/plugins/etc.py @@ -76,7 +76,7 @@ class EtcService(Service): ], 'shadow': { 'ctx': [ - {'method': 'user.query'}, + {'method': 'user.query', 'args': [[['local', '=', True]]]}, ], 'entries': [ {'type': 'mako', 'path': 'shadow', 'group': 'shadow', 'mode': 0o0640}, @@ -84,8 +84,8 @@ class EtcService(Service): }, 'user': { 'ctx': [ - {'method': 'user.query'}, - {'method': 'group.query'}, + {'method': 'user.query', 'args': [[['local', '=', True]]]}, + {'method': 'group.query', 'args': [[['local', '=', True]]]}, ], 'entries': [ {'type': 'mako', 'path': 'group'}, diff --git a/src/middlewared/middlewared/plugins/idmap.py b/src/middlewared/middlewared/plugins/idmap.py index 88de2c04b4c63..fa63b9b5b49e4 100644 --- a/src/middlewared/middlewared/plugins/idmap.py +++ b/src/middlewared/middlewared/plugins/idmap.py @@ -1102,7 +1102,6 @@ async def id_to_name(self, xid, id_type): """ idtype = IDType[id_type] idmap_timeout = 5.0 - options = {'extra': {'additional_information': ['DS']}, 'get': True} match idtype: # IDType.BOTH is possible return by nss_winbind / nss_sss @@ -1122,7 +1121,7 @@ async def id_to_name(self, xid, id_type): try: ret = await asyncio.wait_for( - self.middleware.create_task(self.middleware.call(method, filters, options)), + self.middleware.create_task(self.middleware.call(method, filters, {'get': True})), timeout=idmap_timeout ) name = ret[key] diff --git a/src/middlewared/middlewared/plugins/keychain.py b/src/middlewared/middlewared/plugins/keychain.py index 711164024931b..2d62d0a547bec 100644 --- a/src/middlewared/middlewared/plugins/keychain.py +++ b/src/middlewared/middlewared/plugins/keychain.py @@ -637,7 +637,7 @@ def remote_ssh_semiautomatic_setup(self, data): except Exception as e: raise CallError(f"Semi-automatic SSH connection setup failed: {e!r}") - user = c.call("user.query", [["username", "=", data["username"]]], {"get": True}) + user = c.call("user.query", [["username", "=", data["username"]], ['local', '=', True]], {"get": True}) user_update = {} if user["shell"] == "/usr/sbin/nologin": user_update["shell"] = "/usr/bin/bash" @@ -679,7 +679,7 @@ def ssh_pair(self, data): service = self.middleware.call_sync("service.query", [("service", "=", "ssh")], {"get": True}) ssh = self.middleware.call_sync("ssh.config") try: - user = self.middleware.call_sync("user.query", [("username", "=", data["username"])], {"get": True}) + user = self.middleware.call_sync("user.query", [("username", "=", data["username"]), ("local", "=", True)], {"get": True}) except MatchNotFound: raise CallError(f"User {data['username']} does not exist") diff --git a/src/middlewared/middlewared/plugins/smb.py b/src/middlewared/middlewared/plugins/smb.py index 637ae6bc5c82b..5218f32d528f4 100644 --- a/src/middlewared/middlewared/plugins/smb.py +++ b/src/middlewared/middlewared/plugins/smb.py @@ -1455,7 +1455,7 @@ async def share_precheck(self, data): if not ad_enabled: local_smb_user_cnt = await self.middleware.call( 'user.query', - [['smb', '=', True]], + [['smb', '=', True], ['local', '=', True]], {'count': True} ) if local_smb_user_cnt == 0: diff --git a/src/middlewared/middlewared/plugins/smb_/groupmap.py b/src/middlewared/middlewared/plugins/smb_/groupmap.py index 1ce8937d907a9..09d6fe806d16b 100644 --- a/src/middlewared/middlewared/plugins/smb_/groupmap.py +++ b/src/middlewared/middlewared/plugins/smb_/groupmap.py @@ -150,7 +150,7 @@ async def sync_foreign_groups(self): admin_sid = None grp_obj = await self.middleware.call( 'group.query', - [('group', '=', admin_group)], + [('group', '=', admin_group), ('local', '=', True)], {'extra': {'additional_information': ['SMB', 'DS']}} ) if grp_obj: @@ -431,9 +431,9 @@ async def synchronize_group_mappings(self, job, bypass_sentinel_check=False): groupmap = await self.groupmap_list() must_remove_cache = False - groups = await self.middleware.call('group.query', [('builtin', '=', False), ('smb', '=', True)]) + groups = await self.middleware.call('group.query', [('builtin', '=', False), ('local', '=', True), ('smb', '=', True)]) g_dict = {x["gid"]: x for x in groups} - g_dict[545] = await self.middleware.call('group.query', [('gid', '=', 545)], {'get': True}) + g_dict[545] = await self.middleware.call('group.query', [('gid', '=', 545), ('local', '=', True)], {'get': True}) intersect = set(g_dict.keys()).intersection(set(groupmap["local"].keys())) diff --git a/src/middlewared/middlewared/plugins/smb_/passdb.py b/src/middlewared/middlewared/plugins/smb_/passdb.py index 74818c68f1bed..68c55111bc6b0 100644 --- a/src/middlewared/middlewared/plugins/smb_/passdb.py +++ b/src/middlewared/middlewared/plugins/smb_/passdb.py @@ -238,5 +238,5 @@ async def synchronize_passdb(self, job, bypass_sentinel_check=False): "This may indicate system dataset setup failure." ) - conf_users = await self.middleware.call('user.query', [("smb", "=", True)]) + conf_users = await self.middleware.call('user.query', [("smb", "=", True), ('local', '=', True)]) await self.passdb_sync_impl(conf_users) diff --git a/src/middlewared/middlewared/plugins/usage.py b/src/middlewared/middlewared/plugins/usage.py index f0282ca49303f..07800728fe420 100644 --- a/src/middlewared/middlewared/plugins/usage.py +++ b/src/middlewared/middlewared/plugins/usage.py @@ -266,7 +266,7 @@ async def gather_system(self, context): 'system_hash': await self.middleware.call('system.host_id'), 'usage_version': 1, 'system': [{ - 'users': await self.middleware.call('user.query', [], {'count': True}), + 'users': await self.middleware.call('user.query', [['local', '=', True]], {'count': True}), 'snapshots': context['total_snapshots'], 'zvols': context['total_zvols'], 'datasets': context['total_datasets'], diff --git a/src/middlewared/middlewared/test/integration/assets/ftp.py b/src/middlewared/middlewared/test/integration/assets/ftp.py index 4633525a87b67..ceb5a9f085dab 100644 --- a/src/middlewared/middlewared/test/integration/assets/ftp.py +++ b/src/middlewared/middlewared/test/integration/assets/ftp.py @@ -36,7 +36,7 @@ def anonymous_ftp_server(config=None, dataset_name="anonftp"): @contextlib.contextmanager def ftp_server_with_user_account(config=None): config = config or {} - ftp_id = call("group.query", [["name", "=", "ftp"]], {"get": True})["id"] + ftp_id = call("group.query", [["name", "=", "ftp"], ['local', '=', True]], {"get": True})["id"] with dataset("ftptest") as ds: with user({ diff --git a/tests/api2/test_036_ad_ldap.py b/tests/api2/test_036_ad_ldap.py deleted file mode 100644 index 5a44f5bff7a46..0000000000000 --- a/tests/api2/test_036_ad_ldap.py +++ /dev/null @@ -1,429 +0,0 @@ -from contextlib import contextmanager -import os -import sys - -import pytest - -from middlewared.test.integration.assets.pool import dataset -from middlewared.test.integration.utils import call - -apifolder = os.getcwd() -sys.path.append(apifolder) - -from middlewared.test.integration.assets.directory_service import active_directory, ldap -from auto_config import hostname, password, pool_name, user, ha -from functions import GET, POST, PUT, SSH_TEST, make_ws_request, wait_on_job -from protocols import nfs_share, SSH_NFS -from pytest_dependency import depends - -try: - from config import AD_DOMAIN, ADPASSWORD, ADUSERNAME, ADNameServer, AD_COMPUTER_OU -except ImportError: - Reason = 'ADNameServer AD_DOMAIN, ADPASSWORD, or/and ADUSERNAME are missing in config.py"' - pytestmark = pytest.mark.skip(reason=Reason) - -pytestmark = pytest.mark.skip("LDAP KRB5 NFS tests disabled pending CI framework changes") - - -test_perms = { - "READ_DATA": True, - "WRITE_DATA": True, - "EXECUTE": True, - "APPEND_DATA": True, - "DELETE_CHILD": False, - "DELETE": True, - "READ_ATTRIBUTES": True, - "WRITE_ATTRIBUTES": True, - "READ_NAMED_ATTRS": True, - "WRITE_NAMED_ATTRS": True, - "READ_ACL": True, - "WRITE_ACL": True, - "WRITE_OWNER": True, - "SYNCHRONIZE": False, -} - -test_flags = { - "FILE_INHERIT": True, - "DIRECTORY_INHERIT": True, - "INHERIT_ONLY": False, - "NO_PROPAGATE_INHERIT": False, - "INHERITED": False -} - - -@pytest.fixture(scope="module") -def kerberos_config(request): - # DNS in automation domain is often broken. - # Setting rdns helps to pass this - results = PUT("/kerberos/", {"libdefaults_aux": "rdns = false"}) - assert results.status_code == 200, results.text - - results = PUT("/nfs/", {"v4_krb": True}) - assert results.status_code == 200, results.text - try: - yield (request, results.json()) - finally: - results = POST('/service/stop/', {'service': 'nfs'}) - assert results.status_code == 200, results.text - - results = PUT("/nfs/", {"v4_krb": False}) - assert results.status_code == 200, results.text - - results = PUT("/kerberos/", {"libdefaults_aux": ""}) - assert results.status_code == 200, results.text - - -@pytest.fixture(scope="module") -def do_ad_connection(request): - with active_directory( - AD_DOMAIN, - ADUSERNAME, - ADPASSWORD, - netbiosname=hostname, - createcomputer=AD_COMPUTER_OU, - ) as ad: - yield (request, ad) - - -@contextmanager -def stop_activedirectory(request): - results = PUT("/activedirectory/", {"enable": False}) - assert results.status_code == 200, results.text - job_id = results.json() - job_status = wait_on_job(job_id, 180) - assert job_status['state'] == 'SUCCESS', str(job_status['results']) - try: - yield results.json() - finally: - results = PUT("/activedirectory/", {"enable": True}) - assert results.status_code == 200, results.text - job_id = results.json() - job_status = wait_on_job(job_id, 180) - assert job_status['state'] == 'SUCCESS', str(job_status['results']) - - -@pytest.fixture(scope="module") -def do_ldap_connection(request): - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.keytab.kerberos_principal_choices', - 'params': [], - }) - error = res.get('error') - assert error is None, str(error) - - kerberos_principal = res['result'][0] - - results = GET("/kerberos/realm/") - assert results.status_code == 200, results.text - - realm_id = results.json()[0]['id'] - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.check_ticket', - 'params': [], - }) - error = res.get('error') - assert error is None, str(error) - assert res['result'] is True - - results = POST("/activedirectory/domain_info/", AD_DOMAIN) - assert results.status_code == 200, results.text - domain_info = results.json() - - with stop_activedirectory(request) as ad: - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.get_cred', - 'params': [{ - 'dstype': 'DS_TYPE_LDAP', - 'conf': { - 'kerberos_realm': realm_id, - 'kerberos_principal': kerberos_principal, - } - }], - }) - error = res.get('error') - assert error is None, str(error) - cred = res['result'] - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.do_kinit', - 'params': [{ - 'krb5_cred': cred, - 'kinit-options': { - 'kdc_override': { - 'domain': AD_DOMAIN.upper(), - 'kdc': domain_info['KDC server'] - }, - } - }], - }) - error = res.get('error') - assert error is None, str(error) - - with ldap( - domain_info['Bind Path'], - '', '', f'{domain_info["LDAP server name"].upper()}.', - has_samba_schema=False, - ssl="OFF", - kerberos_realm=realm_id, - kerberos_principal=kerberos_principal, - validate_certificates=False, - enable=True - ) as ldap_conn: - yield (request, ldap_conn) - - -@pytest.fixture(scope="module") -def setup_nfs_share(request): - results = POST("/user/get_user_obj/", {'username': f'{ADUSERNAME}@{AD_DOMAIN}'}) - assert results.status_code == 200, results.text - target_uid = results.json()['pw_uid'] - - target_acl = [ - {'tag': 'owner@', 'id': -1, 'perms': test_perms, 'flags': test_flags, 'type': 'ALLOW'}, - {'tag': 'group@', 'id': -1, 'perms': test_perms, 'flags': test_flags, 'type': 'ALLOW'}, - {'tag': 'everyone@', 'id': -1, 'perms': test_perms, 'flags': test_flags, 'type': 'ALLOW'}, - {'tag': 'USER', 'id': target_uid, 'perms': test_perms, 'flags': test_flags, 'type': 'ALLOW'}, - ] - with dataset( - 'NFSKRB5', - {'acltype': 'NFSV4'}, - acl=target_acl - ) as ds: - with nfs_share(f'/mnt/{ds}', options={ - 'comment': 'KRB Functional Test Share', - 'security': ['KRB5', 'KRB5I', 'KRB5P'], - }) as share: - yield (request, {'share': share, 'uid': target_uid}) - - -@pytest.mark.dependency(name="AD_CONFIGURED") -def test_02_enabling_activedirectory(do_ad_connection): - results = GET('/activedirectory/started/') - assert results.status_code == 200, results.text - assert results.json() is True, results.text - - results = GET('/activedirectory/get_state/') - assert results.status_code == 200, results.text - assert results.json() == 'HEALTHY', results.text - - -def test_03_kerberos_nfs4_spn_add(kerberos_config): - depends(kerberos_config[0], ["AD_CONFIGURED"], scope="session") - assert kerberos_config[1]['v4_krb_enabled'] - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.keytab.has_nfs_principal', - 'params': [], - }) - error = res.get('error') - assert error is None, str(error) - assert res['result'] is False - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'nfs.add_principal', - 'params': [{ - 'username': ADUSERNAME, - 'password': ADPASSWORD - }], - }) - error = res.get('error') - assert error is None, str(error) - assert res['result'] is True - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.keytab.has_nfs_principal', - 'params': [], - }) - error = res.get('error') - assert error is None, str(error) - assert res['result'] is True - - results = POST('/service/reload/', {'service': 'idmap'}) - assert results.status_code == 200, results.text - - results = POST('/service/restart/', {'service': 'ssh'}) - assert results.status_code == 200, results.text - - -@pytest.mark.dependency(name="AD_LDAP_USER_CCACHE") -def test_05_kinit_as_ad_user(setup_nfs_share): - """ - Set up an NFS share and ensure that permissions are - set correctly to allow writes via out test user. - - This test does kinit as our test user so that we have - kerberos ticket that we will use to verify NFS4 + KRB5 - work correctly. - """ - depends(setup_nfs_share[0], ["AD_CONFIGURED"], scope="session") - - kinit_opts = {'ccache': 'USER', 'ccache_uid': setup_nfs_share[1]['uid']} - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.get_cred', - 'params': [{ - 'dstype': 'DS_TYPE_ACTIVEDIRECTORY', - 'conf': { - 'domainname': AD_DOMAIN, - 'bindname': ADUSERNAME, - 'bindpw': ADPASSWORD, - } - }], - }) - error = res.get('error') - assert error is None, str(error) - cred = res['result'] - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.do_kinit', - 'params': [{ - 'krb5_cred': cred, - 'kinit-options': kinit_opts - }], - }) - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.check_ticket', - 'params': [kinit_opts], - }) - error = res.get('error') - assert error is None, str(error) - assert res['result'] is True - - results = POST('/service/restart/', {'service': 'nfs'}) - assert results.status_code == 200, results.text - - -if not ha: - """ - we skip this test for a myriad of profoundly complex reasons - on our HA pipeline. If you cherish your sanity, don't try to - understand, just accept and move on :) - """ - def test_06_krb5nfs_ops_with_ad(request): - my_fqdn = f'{hostname.strip()}.{AD_DOMAIN}' - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'dnsclient.forward_lookup', - 'params': [{'names': [my_fqdn]}], - }) - error = res.get('error') - assert error is None, str(error) - - addresses = [rdata['address'] for rdata in res['result']] - assert ip in addresses - - """ - The following creates a loopback mount using our kerberos - keytab (AD computer account) and then performs ops via SSH - using a limited AD account for which we generated a kerberos - ticket above. Due to the odd nature of this setup, the loopback - mount gets mapped as the guest account on the NFS server. - This is fine for our purposes as we're validating that - sec=krb5 works. - """ - userobj = call('user.get_user_obj', {'username': f'{ADUSERNAME}@{AD_DOMAIN}'}) - groupobj = call('group.get_group_obj', {'gid': userobj['pw_gid']}) - call('ssh.update', {"password_login_groups": [groupobj['gr_name']]}) - with SSH_NFS( - my_fqdn, - f'/mnt/{pool_name}/NFSKRB5', - vers=4, - mount_user=user, - mount_password=password, - ip=ip, - kerberos=True, - user=ADUSERNAME, - password=ADPASSWORD, - ) as n: - n.create('testfile') - n.mkdir('testdir') - contents = n.ls('.') - - assert 'testdir' in contents - assert 'testfile' in contents - - file_acl = n.getacl('testfile') - for idx, ace in enumerate(file_acl): - assert ace['perms'] == test_perms, str(ace) - - dir_acl = n.getacl('testdir') - for idx, ace in enumerate(dir_acl): - assert ace['perms'] == test_perms, str(ace) - assert ace['flags'] == test_flags, str(ace) - - n.unlink('testfile') - n.rmdir('testdir') - contents = n.ls('.') - assert 'testdir' not in contents - assert 'testfile' not in contents - - -@pytest.mark.dependency(name="SET_UP_AD_VIA_LDAP") -def test_07_setup_and_enabling_ldap(do_ldap_connection): - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.stop', - 'params': [], - }) - error = res.get('error') - assert error is None, str(error) - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.start', - 'params': [], - }) - error = res.get('error') - assert error is None, str(error) - - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos._klist_test', - 'params': [], - }) - error = res.get('error') - assert error is None, str(error) - assert res['result'] is True - - # Verify that our NFS kerberos principal is - # still present - res = make_ws_request(ip, { - 'msg': 'method', - 'method': 'kerberos.keytab.has_nfs_principal', - 'params': [], - }) - error = res.get('error') - assert error is None, str(error) - assert res['result'] is True - - -def test_08_verify_ldap_users(request): - depends(request, ["SET_UP_AD_VIA_LDAP"], scope="session") - - results = GET('/user', payload={ - 'query-filters': [['local', '=', False]], - 'query-options': {'extra': {"search_dscache": True}}, - }) - assert results.status_code == 200, results.text - assert len(results.json()) > 0, results.text - - results = GET('/group', payload={ - 'query-filters': [['local', '=', False]], - 'query-options': {'extra': {"search_dscache": True}}, - }) - assert results.status_code == 200, results.text - assert len(results.json()) > 0, results.text diff --git a/tests/api2/test_040_ad_user_group_cache.py b/tests/api2/test_040_ad_user_group_cache.py index a323004ff7544..12a85262f646e 100644 --- a/tests/api2/test_040_ad_user_group_cache.py +++ b/tests/api2/test_040_ad_user_group_cache.py @@ -27,15 +27,11 @@ def do_ad_connection(request): call('core.job_wait', cache_fill_job['id'], job=True) users = set([x['username'] for x in call( - 'user.query', - [['local', '=', False]], - {'extra': {'search_dscache': True}} + 'user.query', [['local', '=', False]], )]) groups = set([x['name'] for x in call( - 'group.query', - [['local', '=', False]], - {'extra': {'search_dscache': True}} + 'group.query', [['local', '=', False]], )]) yield ad | {'users': users, 'groups': groups} @@ -50,12 +46,12 @@ def get_ad_user_and_group(ad_connection): user = call( 'user.query', [['username', '=', ad_user]], - {'extra': {'search_dscache': True}, 'get': True} + {'get': True} ) group = call( 'group.query', [['name', '=', ad_group]], - {'extra': {'search_dscache': True}, 'get': True} + {'get': True} ) return (user, group) @@ -63,8 +59,8 @@ def get_ad_user_and_group(ad_connection): def test_check_for_ad_users(do_ad_connection): """ - This test validates that we can query AD users using - filter-option {"extra": {"search_dscache": True}} + This test validates that wbinfo -u output matches entries + we get through user.query """ cmd = "wbinfo -u" results = SSH_TEST(cmd, user, password) @@ -76,8 +72,8 @@ def test_check_for_ad_users(do_ad_connection): def test_check_for_ad_groups(do_ad_connection): """ - This test validates that we can query AD groups using - filter-option {"extra": {"search_dscache": True}} + This test validates that wbinfo -g output matches entries + we get through group.query """ cmd = "wbinfo -g" results = SSH_TEST(cmd, user, password) @@ -105,17 +101,13 @@ def test_check_directoryservices_cache_refresh(do_ad_connection): call('directoryservices.cache.refresh_impl', job=True) users = set([x['username'] for x in call( - 'user.query', - [['local', '=', False]], - {'extra': {'search_dscache': True}} + 'user.query', [['local', '=', False]] )]) assert users == do_ad_connection['users'] groups = set([x['name'] for x in call( - 'group.query', - [['local', '=', False]], - {'extra': {'search_dscache': True}} + 'group.query', [['local', '=', False]], )]) assert groups == do_ad_connection['groups'] @@ -139,17 +131,13 @@ def test_check_lazy_initialization_of_users_and_groups_by_name(do_ad_connection) ad_user, ad_group = get_ad_user_and_group(do_ad_connection) cache_names = set([x['username'] for x in call( - 'user.query', - [['local', '=', False]], - {'extra': {'search_dscache': True}} + 'user.query', [['local', '=', False]], )]) assert cache_names == {ad_user['username']} cache_names = set([x['name'] for x in call( - 'group.query', - [['local', '=', False]], - {'extra': {'search_dscache': True}} + 'group.query', [['local', '=', False]], )]) assert cache_names == {ad_group['name']} @@ -172,28 +160,18 @@ def test_check_lazy_initialization_of_users_and_groups_by_id(do_ad_connection): results = SSH_TEST(cmd, user, password) assert results['result'] is True, results['output'] - call( - 'user.query', [['uid', '=', ad_user['uid']]], - {'extra': {'search_dscache': True, 'get': True}} - ) + call('user.query', [['uid', '=', ad_user['uid']]], {'get': True}}) - call( - 'group.query', [['gid', '=', ad_group['gid']]], - {'extra': {'search_dscache': True, 'get': True}} - ) + call('group.query', [['gid', '=', ad_group['gid']]], {'get': True}}) cache_names = set([x['username'] for x in call( - 'user.query', - [['local', '=', False]], - {'extra': {'search_dscache': True}} + 'user.query', [['local', '=', False]], )]) assert cache_names == {ad_user['username']} cache_names = set([x['name'] for x in call( - 'group.query', - [['local', '=', False]], - {'extra': {'search_dscache': True}} + 'group.query', [['local', '=', False]], )]) assert cache_names == {ad_group['name']}