diff --git a/cloudfile_ext/v2/memberships.py b/cloudfile_ext/v2/memberships.py index 21033d1741..c9af2bdc43 100644 --- a/cloudfile_ext/v2/memberships.py +++ b/cloudfile_ext/v2/memberships.py @@ -175,7 +175,7 @@ def _project(self, uid, username, desired): cursor.execute('INSERT INTO ' + self.members + '(group_id,user_name,is_staff) VALUES(%s,%s,0)', (group, username)) cursor.execute('SELECT group_id,is_staff FROM ' + self.members + ' WHERE user_name=%s ORDER BY group_id FOR UPDATE', (username,)) actual = cursor.fetchall() - if ({group for group, _ in actual} & owned) != required or any(group in owned and staff != 0 for group, staff in actual): + if ({group for group, _ in actual} & (owned | required)) != required or any(group in (owned | required) and staff != 0 for group, staff in actual): raise IdentityUnavailable('native membership readback failed') self.connection.commit() finally: diff --git a/cloudfile_ext/v2/tests/test_direct_groupuser_sql_mock.py b/cloudfile_ext/v2/tests/test_direct_groupuser_sql_mock.py index 004147ab86..f89c195799 100644 --- a/cloudfile_ext/v2/tests/test_direct_groupuser_sql_mock.py +++ b/cloudfile_ext/v2/tests/test_direct_groupuser_sql_mock.py @@ -107,6 +107,18 @@ def test_ancestor_is_revoked_direct_role_and_manual_group_preserved(self): self.assertEqual(conn.members, {10: 0, 12: 0, 99: 0}) self.assertEqual(conn.commits, 2) + def test_new_user_memberships_are_added_and_verified(self): + # The old readback checked only groups already owned before INSERT, + # rejecting a perfectly valid first login with new Org/Role rows. + for original in ({99: 0}, {10: 0, 99: 0}): + with self.subTest(original=original): + conn = Connection(original) + projector, identity = self.projector(conn) + projector._project(UID, USERNAME, {DIRECT, ROLE}) + self.assertEqual(conn.members, {10: 0, 12: 0, 99: 0}) + self.assertEqual((conn.commits, conn.rollbacks), (1, 0)) + identity.active.assert_called_once_with(USERNAME, UID, locked=True) + def test_failure_rolls_back_ancestor_revocation(self): conn = Connection({10: 0, 11: 0, 12: 0, 99: 0}, fail_delete=True) projector, _ = self.projector(conn)