From 8f5bfee82262286ad65cde60fe6f788c98d15032 Mon Sep 17 00:00:00 2001 From: Amith Murthy Date: Fri, 2 Oct 2026 09:54:20 -0700 Subject: [PATCH 1/2] syncing user gid upon group membership change --- modules/coactd.py | 21 +++++++++++++ tests/test_record_posix_group_change.py | 40 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 tests/test_record_posix_group_change.py diff --git a/modules/coactd.py b/modules/coactd.py index 17734fc..569c537 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -207,6 +207,25 @@ def do(self, req_id: str, op_type: Any, req_type: Any, approval: str, req: dict, """Process a request. Subclasses must override this method.""" raise NotImplementedError('do() is abstract') + USER_POSIX_GROUP_UPDATE_GQL = gql(""" + mutation userPosixGroupUpdate($username: String!, $gidnumber: Int!, $present: Boolean!) { + userPosixGroupUpdate(username: $username, gidnumber: $gidnumber, present: $present) { + secondaryGidNumbers + } + } + """) + + def record_posix_group_change(self, user: str, gid_number: int, present: bool) -> None: + """Tell coact that this user was just added to / removed from the posixGroup with gid_number, + so coact's stored gids reflect the playbook we just ran. Never raises: the request outcome is the + LDAP change itself, and the periodic posix sync reconciles anything missed here.""" + try: + res = self.back_channel.execute(self.USER_POSIX_GROUP_UPDATE_GQL, + {'username': user, 'gidnumber': int(gid_number), 'present': present}) + self.logger.info(f"recorded gid {gid_number} {'present' if present else 'absent'} for {user} in coact: {res['userPosixGroupUpdate']}") + except Exception as e: + self.logger.warning(f"could not record gid {gid_number} change for {user} in coact (periodic sync will reconcile): {e}") + # ============================================================================ # Create the main coactd group @@ -996,6 +1015,8 @@ def do_repo_membership(self, user: str, repo: str, facility: str, action: str, d create=True, dry_run=dry_run ) + if not dry_run: + self.record_posix_group_change(user, gid_number, present=(action == 'present')) # finish up and mark record user_req = { diff --git a/tests/test_record_posix_group_change.py b/tests/test_record_posix_group_change.py new file mode 100644 index 0000000..6b7b125 --- /dev/null +++ b/tests/test_record_posix_group_change.py @@ -0,0 +1,40 @@ +""" +Tests for Registration.record_posix_group_change: after the posixGroup playbook changes LDAP, coactd +tells coact-api which gid was added/removed for the user. It must pass the exact delta and never raise. +""" + +import sys +from unittest.mock import Mock + +sys.modules['ansible_runner'] = Mock() + +from modules.coactd import Registration # noqa: E402 + + +def make_registration(): + r = Registration.__new__(Registration) + r.logger = Mock() + r.back_channel = Mock() + return r + + +class TestRecordPosixGroupChange: + + def test_present_sends_user_gid_and_flag(self): + r = make_registration() + r.back_channel.execute.return_value = {'userPosixGroupUpdate': {'secondaryGidNumbers': [3049]}} + r.record_posix_group_change('alice', '3049', present=True) + assert r.back_channel.execute.call_count == 1 + assert r.back_channel.execute.call_args.args[1] == {'username': 'alice', 'gidnumber': 3049, 'present': True} + + def test_absent_sends_false(self): + r = make_registration() + r.back_channel.execute.return_value = {'userPosixGroupUpdate': {'secondaryGidNumbers': []}} + r.record_posix_group_change('alice', 3049, present=False) + assert r.back_channel.execute.call_args.args[1]['present'] is False + + def test_backend_error_never_raises(self): + r = make_registration() + r.back_channel.execute.side_effect = RuntimeError("coact-api down") + r.record_posix_group_change('alice', 3049, present=True) + r.logger.warning.assert_called() From d32fdcd29b9b8c3edd70c9e090de7f9938a0c7d3 Mon Sep 17 00:00:00 2001 From: Amith Murthy Date: Wed, 7 Oct 2026 15:39:11 -0700 Subject: [PATCH 2/2] adding user uid/gid during registration --- modules/coactd.py | 25 ++++++++ tests/test_record_posix_init.py | 109 ++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 tests/test_record_posix_init.py diff --git a/modules/coactd.py b/modules/coactd.py index 569c537..708a606 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -226,6 +226,27 @@ def record_posix_group_change(self, user: str, gid_number: int, present: bool) - except Exception as e: self.logger.warning(f"could not record gid {gid_number} change for {user} in coact (periodic sync will reconcile): {e}") + USER_POSIX_INIT_GQL = gql(""" + mutation userPosixInit($username: String!) { + userPosixInit(username: $username) { + primaryGid + secondaryGidNumbers + syncedAt + } + } + """) + + def record_posix_init(self, user: str) -> None: + """Ask coact to initialise this newly provisioned user's posix data (primary gid, secondary gids) from + user-lookup, so the user is served from coact's users collection and kept current by the periodic sync. + Never raises: registration must still complete. On failure the user keeps working via coact's live + user-lookup fallback, but the periodic sync will not pick them up; an admin can re-run userPosixInit.""" + try: + res = self.back_channel.execute(self.USER_POSIX_INIT_GQL, {'username': user}) + self.logger.info(f"initialised posix data for {user} in coact: {res['userPosixInit']}") + except Exception as e: + self.logger.warning(f"could not initialise posix data for {user} in coact (re-run userPosixInit for this user to fix): {e}") + # ============================================================================ # Create the main coactd group @@ -352,6 +373,10 @@ def do_new_user(self, user: str, eppn: str, facility: str, playbook: str = "coac user_id = self.back_channel.execute(self.USER_UPSERT_GQL, user_create_req) self.logger.debug(f"upserted user {user_id}") + # record the user's gids in coact now, before any repo membership change can $addToSet on top of them + if not self.dry_run: + self.record_posix_init(user) + # configure home directory runner = self.run_playbook(playbook, user=user, user_facility=facility, tags='home', force_copy_skel=False) diff --git a/tests/test_record_posix_init.py b/tests/test_record_posix_init.py new file mode 100644 index 0000000..924106e --- /dev/null +++ b/tests/test_record_posix_init.py @@ -0,0 +1,109 @@ +""" +Tests for Registration.record_posix_init and its call in UserRegistration.do_new_user: after the user is upserted +into coact, coactd asks coact-api to initialise the user's posix data from user-lookup. It must never raise +(registration still completes) and must run before repo membership changes can $addToSet on top of it. +""" + +import sys +from unittest.mock import Mock + +import pytest + +sys.modules['ansible_runner'] = Mock() + +from modules.coactd import Registration, UserRegistration # noqa: E402 + + +def make_registration(): + r = Registration.__new__(Registration) + r.logger = Mock() + r.back_channel = Mock() + return r + + +class TestRecordPosixInit: + + def test_sends_username(self): + r = make_registration() + r.back_channel.execute.return_value = {'userPosixInit': {'primaryGid': 1000, 'secondaryGidNumbers': [], 'syncedAt': 'now'}} + r.record_posix_init('alice') + assert r.back_channel.execute.call_count == 1 + assert r.back_channel.execute.call_args.args[0] is Registration.USER_POSIX_INIT_GQL + assert r.back_channel.execute.call_args.args[1] == {'username': 'alice'} + r.logger.warning.assert_not_called() + + def test_backend_error_never_raises(self): + r = make_registration() + r.back_channel.execute.side_effect = RuntimeError("user-lookup returned no primary gid") + r.record_posix_init('alice') + r.logger.warning.assert_called_once() + + def test_unexpected_response_never_raises(self): + r = make_registration() + r.back_channel.execute.return_value = {} + r.record_posix_init('alice') + r.logger.warning.assert_called_once() + + +LDAP_FACTS = { + 'ansible_facts': { + 'ldap_user_default_shell': '/bin/bash', + 'ldap_user_uidNumber': '12345', + 'ldap_user_gecos': 'Alice Example', + 'ldap_user_homedir': '/home/alice', + } +} + + +def make_user_registration(dry_run=False): + r = UserRegistration.__new__(UserRegistration) + r.logger = Mock() + r.back_channel = Mock() + r.dry_run = dry_run + r.run_playbook = Mock() + r.playbook_task_res = Mock(return_value=LDAP_FACTS) + return r + + +def executed_mutations(r): + return [c.args[0] for c in r.back_channel.execute.call_args_list] + + +class TestDoNewUserPosixInit: + + def test_init_runs_right_after_upsert_and_before_repo_membership(self): + r = make_user_registration() + assert r.do_new_user('alice', 'alice@example.org', 'somefacility') is True + assert executed_mutations(r) == [ + UserRegistration.USER_UPSERT_GQL, + UserRegistration.USER_POSIX_INIT_GQL, + UserRegistration.USER_STORAGE_GQL, + UserRegistration.REPO_ADD_USER_GQL, + ] + init_call = r.back_channel.execute.call_args_list[1] + assert init_call.args[1] == {'username': 'alice'} + + def test_dry_run_skips_init(self): + r = make_user_registration(dry_run=True) + r.do_new_user('alice', 'alice@example.org', 'somefacility') + assert UserRegistration.USER_POSIX_INIT_GQL not in executed_mutations(r) + + def test_init_failure_does_not_block_registration(self): + r = make_user_registration() + + def execute(query, variables): + if query is UserRegistration.USER_POSIX_INIT_GQL: + raise RuntimeError("coact-api down") + return {} + + r.back_channel.execute.side_effect = execute + assert r.do_new_user('alice', 'alice@example.org', 'somefacility') is True + assert executed_mutations(r)[-2:] == [UserRegistration.USER_STORAGE_GQL, UserRegistration.REPO_ADD_USER_GQL] + r.logger.warning.assert_called() + + def test_upsert_failure_skips_init(self): + r = make_user_registration() + r.back_channel.execute.side_effect = RuntimeError("userUpsert failed") + with pytest.raises(RuntimeError): + r.do_new_user('alice', 'alice@example.org', 'somefacility') + assert executed_mutations(r) == [UserRegistration.USER_UPSERT_GQL]