syncing user gid upon group membership change

modules/coactd.py

@@ -207,6 +207,46 @@ 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}")
+
+    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
@@ -333,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)
 
@@ -996,6 +1040,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 = {

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()

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]