From 514b80236709aee2ea7ec972e4e64a66f9b90790 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 10 Oct 2026 16:41:05 -0300 Subject: [PATCH 1/3] fix(members): drop sponsor groups and Sponsor_Users access on full IDP sync MemberService::synchronizeGroups skipped IGroup::Sponsors on the full IDP sync, so removing the group at the IDP never removed it locally and the member's Sponsor_Users access stayed too. The IDP payload is the source of truth: when sponsors or sponsors-external-users is missing from it, the local group is removed and that slug is stripped from the member's Sponsor_Users.Permissions on every sponsor. Rows left without any permission lose the membership; rows that still hold another slug are kept. The additive per-request sync is unchanged. --- app/Models/Foundation/Main/Member.php | 64 +++++++ app/Services/Model/Imp/MemberService.php | 7 +- .../MemberServiceSponsorGroupSyncTest.php | 175 ++++++++++++++++++ 3 files changed, 245 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/Services/MemberServiceSponsorGroupSyncTest.php diff --git a/app/Models/Foundation/Main/Member.php b/app/Models/Foundation/Main/Member.php index 846f6407c4..a891e22b2f 100644 --- a/app/Models/Foundation/Main/Member.php +++ b/app/Models/Foundation/Main/Member.php @@ -3599,6 +3599,70 @@ public function removeSponsorPermission(int $sponsor_id, string $group_slug): in ])->executeQuery()->fetchOne()); } + /** + * Strips $group_slug from the Permissions of EVERY Sponsor_Users row of this member and drops the + * membership of the rows that held that slug and are left without any permission. Used when the + * IDP no longer carries the sponsor group, so its access cannot outlive the group. + * Rows that never held the slug (or still hold another one) are left untouched. + * + * @param string $group_slug + * @return void + */ + public function removeSponsorPermissionFromAllSponsors(string $group_slug): void + { + $params = ['member_id' => $this->getId(), 'group_slug' => $group_slug]; + + $sponsor_ids = $this->prepareRawSQL( + <<executeQuery()->fetchFirstColumn(); + + if (count($sponsor_ids) === 0) return; + + $this->prepareRawSQL( + <<executeStatement(); + + $emptied_sponsor_ids = $this->prepareRawSQL( + << $this->getId()] + )->executeQuery()->fetchFirstColumn(); + + // only the rows this call emptied, never an unrelated row that was already empty + $to_remove = array_intersect(array_map('intval', $sponsor_ids), array_map('intval', $emptied_sponsor_ids)); + + foreach ($this->sponsor_memberships->toArray() as $sponsor) { + if (in_array(intval($sponsor->getId()), $to_remove, true)) { + $sponsor->removeUser($this); + } + } + } + public function addSponsorMembership(Sponsor $sponsor):void { if($this->sponsor_memberships->contains($sponsor)) return; diff --git a/app/Services/Model/Imp/MemberService.php b/app/Services/Model/Imp/MemberService.php index ec9655e1f5..7d86a723a3 100644 --- a/app/Services/Model/Imp/MemberService.php +++ b/app/Services/Model/Imp/MemberService.php @@ -539,7 +539,7 @@ public function synchronizeGroups(Member $member, array $groups, bool $allow_rem continue; // skipping this groups bc are managed by SS side - if(in_array($group->getCode(), [IGroup::FoundationMembers, IGroup::CommunityMembers, IGroup::TrackChairs, IGroup::Sponsors])){ + if(in_array($group->getCode(), [IGroup::FoundationMembers, IGroup::CommunityMembers, IGroup::TrackChairs])){ Log::debug(sprintf("MemberService::synchronizeGroups skipping group %s removal", $group->getCode())); continue; } @@ -558,6 +558,11 @@ public function synchronizeGroups(Member $member, array $groups, bool $allow_rem $member->removeFromGroup($group); } } + // the IDP is the source of truth: a sponsor group that is gone takes its Sponsor_Users access with it + if(in_array($externalGroup->getCode(), [IGroup::Sponsors, IGroup::SponsorExternalUsers])){ + Log::debug(sprintf("MemberService::synchronizeGroups member %s email %s removing Sponsor_Users access for group %s", $member->getId(), $member->getEmail(), $externalGroup->getCode())); + $member->removeSponsorPermissionFromAllSponsors($externalGroup->getCode()); + } Log::debug(sprintf("MemberService::synchronizeGroups member %s email %s removing from group %s", $member->getId(), $member->getEmail(), $externalGroup->getCode())); $member->removeFromGroup($externalGroup); } diff --git a/tests/Unit/Services/MemberServiceSponsorGroupSyncTest.php b/tests/Unit/Services/MemberServiceSponsorGroupSyncTest.php new file mode 100644 index 0000000000..a0480cf3f5 --- /dev/null +++ b/tests/Unit/Services/MemberServiceSponsorGroupSyncTest.php @@ -0,0 +1,175 @@ +addUser(self::$member); + + self::$em->flush(); + self::$em->clear(); + } + + public function tearDown(): void + { + self::clearSummitTestData(); + self::clearMemberTestData(); + parent::tearDown(); + } + + private function setPermissions(int $sponsor_id, int $member_id, array $slugs): void + { + self::$em->getConnection()->executeStatement( + 'UPDATE Sponsor_Users SET Permissions = ? WHERE SponsorID = ? AND MemberID = ?', + [json_encode($slugs), $sponsor_id, $member_id] + ); + } + + /** + * @return array|null decoded Permissions of the row, null when the row does not exist + */ + private function getPermissions(int $sponsor_id, int $member_id): ?array + { + $res = self::$em->getConnection()->fetchOne( + 'SELECT Permissions FROM Sponsor_Users WHERE SponsorID = ? AND MemberID = ?', + [$sponsor_id, $member_id] + ); + return $res === false ? null : json_decode($res ?? '[]', true); + } + + private function sync(array $groups, bool $allow_removals): void + { + $member = self::$member_repository->find(self::$member->getId()); + App::make(IMemberService::class)->synchronizeGroups($member, $groups, $allow_removals); + self::$em->clear(); + } + + public function testFullSyncWithoutSponsorsGroupRemovesLocalGroupAndSponsorUsersRow(): void + { + $member_id = self::$member->getId(); + $sponsor_id = self::$sponsors[0]->getId(); + $this->setPermissions($sponsor_id, $member_id, [IGroup::Sponsors]); + + $this->sync([], true); + + $member = self::$member_repository->find($member_id); + $this->assertFalse($member->belongsToGroup(IGroup::Sponsors), "local sponsors group must be removed"); + $this->assertNull($this->getPermissions($sponsor_id, $member_id), "Sponsor_Users row must be removed"); + } + + public function testFullSyncKeepsSponsorsGroupAndSponsorUsersRowWhenPayloadStillHasIt(): void + { + $member_id = self::$member->getId(); + $sponsor_id = self::$sponsors[0]->getId(); + $this->setPermissions($sponsor_id, $member_id, [IGroup::Sponsors]); + + $this->sync([IGroup::Sponsors], true); + + $member = self::$member_repository->find($member_id); + $this->assertTrue($member->belongsToGroup(IGroup::Sponsors)); + $this->assertEquals([IGroup::Sponsors], $this->getPermissions($sponsor_id, $member_id)); + } + + public function testAdditiveSyncNeverRemovesSponsorsGroupNorSponsorUsersRow(): void + { + $member_id = self::$member->getId(); + $sponsor_id = self::$sponsors[0]->getId(); + $this->setPermissions($sponsor_id, $member_id, [IGroup::Sponsors]); + + $this->sync([], false); + + $member = self::$member_repository->find($member_id); + $this->assertTrue($member->belongsToGroup(IGroup::Sponsors)); + $this->assertEquals([IGroup::Sponsors], $this->getPermissions($sponsor_id, $member_id)); + } + + public function testFullSyncKeepsRowAndOtherSlugsWhenRowHoldsMoreThanTheRemovedGroup(): void + { + $member_id = self::$member->getId(); + $sponsor_id = self::$sponsors[0]->getId(); + $this->setPermissions($sponsor_id, $member_id, [IGroup::Sponsors, IGroup::SponsorServices]); + + $this->sync([], true); + + $member = self::$member_repository->find($member_id); + $this->assertFalse($member->belongsToGroup(IGroup::Sponsors)); + $this->assertEquals( + [IGroup::SponsorServices], + $this->getPermissions($sponsor_id, $member_id), + "only the removed slug must be stripped; the row keeps the other one" + ); + } + + public function testFullSyncWithoutExternalUsersGroupRemovesLocalGroupAndSponsorUsersRow(): void + { + $member_id = self::$member->getId(); + $sponsor_id = self::$sponsors[0]->getId(); + + $external = new Group(); + $external->setCode(IGroup::SponsorExternalUsers); + $external->setTitle(IGroup::SponsorExternalUsers); + $external->setDescription(IGroup::SponsorExternalUsers); + $external->setExternal(); + self::$em->persist($external); + $member = self::$member_repository->find($member_id); + $member->add2Group($external); + self::$em->flush(); + self::$em->clear(); + + $this->setPermissions($sponsor_id, $member_id, [IGroup::SponsorExternalUsers]); + + try { + // payload keeps sponsors (so the row's remaining access is judged on its own slug) but drops external users + $this->sync([IGroup::Sponsors], true); + + $member = self::$member_repository->find($member_id); + $this->assertTrue($member->belongsToGroup(IGroup::Sponsors)); + $this->assertFalse($member->belongsToGroup(IGroup::SponsorExternalUsers)); + $this->assertNull($this->getPermissions($sponsor_id, $member_id)); + } finally { + $g = self::$em->getRepository(Group::class)->findOneBy(['code' => IGroup::SponsorExternalUsers]); + if (!is_null($g)) { + self::$em->remove($g); + self::$em->flush(); + } + } + } +} From e8ba800faeb6c061247e65c21a2179e814dc3b03 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 10 Oct 2026 19:14:58 -0300 Subject: [PATCH 2/3] refactor(members): strip sponsor permission slug without deleting Sponsor_Users rows Reuse Member::removeSponsorPermission per sponsor membership, the same primitive the sponsor-users-api removal event uses, instead of a new raw SQL method that also deleted the memberships left without permissions. Group absence does not mean the access right is gone: the Sponsor_Users row lifecycle belongs to the access right events (AUTH_USER_REMOVED_FROM_SPONSOR_AND_SUMMIT). Stripping the slug is enough to hide the summits, since the sponsor summit strategy reads Permissions. --- app/Services/Model/Imp/MemberService.php | 10 +++++++--- .../Services/MemberServiceSponsorGroupSyncTest.php | 12 +++++++----- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/Services/Model/Imp/MemberService.php b/app/Services/Model/Imp/MemberService.php index 7d86a723a3..60a57cff05 100644 --- a/app/Services/Model/Imp/MemberService.php +++ b/app/Services/Model/Imp/MemberService.php @@ -558,10 +558,14 @@ public function synchronizeGroups(Member $member, array $groups, bool $allow_rem $member->removeFromGroup($group); } } - // the IDP is the source of truth: a sponsor group that is gone takes its Sponsor_Users access with it + // the IDP is the source of truth: a sponsor group that is gone takes its Sponsor_Users permission slug with it if(in_array($externalGroup->getCode(), [IGroup::Sponsors, IGroup::SponsorExternalUsers])){ - Log::debug(sprintf("MemberService::synchronizeGroups member %s email %s removing Sponsor_Users access for group %s", $member->getId(), $member->getEmail(), $externalGroup->getCode())); - $member->removeSponsorPermissionFromAllSponsors($externalGroup->getCode()); + Log::debug(sprintf("MemberService::synchronizeGroups member %s email %s stripping Sponsor_Users permission for group %s", $member->getId(), $member->getEmail(), $externalGroup->getCode())); + // same primitive the sponsor-users-api removal event uses; the Sponsor_Users rows stay + // (their lifecycle belongs to the access right events), only the slug is stripped + foreach ($member->getSponsorMemberships() as $sponsor) { + $member->removeSponsorPermission($sponsor->getId(), $externalGroup->getCode()); + } } Log::debug(sprintf("MemberService::synchronizeGroups member %s email %s removing from group %s", $member->getId(), $member->getEmail(), $externalGroup->getCode())); $member->removeFromGroup($externalGroup); diff --git a/tests/Unit/Services/MemberServiceSponsorGroupSyncTest.php b/tests/Unit/Services/MemberServiceSponsorGroupSyncTest.php index a0480cf3f5..4270194003 100644 --- a/tests/Unit/Services/MemberServiceSponsorGroupSyncTest.php +++ b/tests/Unit/Services/MemberServiceSponsorGroupSyncTest.php @@ -25,7 +25,8 @@ * * The IDP payload is the source of truth for the sponsor groups: on the full * (allow_removals = true) sync a sponsor group missing from the payload is removed - * locally together with the matching Sponsor_Users access. The additive + * locally and its slug is stripped from the member's Sponsor_Users.Permissions (the + * Sponsor_Users rows stay: their lifecycle belongs to sponsor-users-api). The additive * (per-request, token claim) sync never removes either. * * @package Tests\Unit\Services @@ -82,7 +83,7 @@ private function sync(array $groups, bool $allow_removals): void self::$em->clear(); } - public function testFullSyncWithoutSponsorsGroupRemovesLocalGroupAndSponsorUsersRow(): void + public function testFullSyncWithoutSponsorsGroupRemovesLocalGroupAndStripsSlugFromSponsorUsersRow(): void { $member_id = self::$member->getId(); $sponsor_id = self::$sponsors[0]->getId(); @@ -92,7 +93,8 @@ public function testFullSyncWithoutSponsorsGroupRemovesLocalGroupAndSponsorUsers $member = self::$member_repository->find($member_id); $this->assertFalse($member->belongsToGroup(IGroup::Sponsors), "local sponsors group must be removed"); - $this->assertNull($this->getPermissions($sponsor_id, $member_id), "Sponsor_Users row must be removed"); + // the membership itself is owned by the sponsor-users-api access right events, not by the group sync + $this->assertEquals([], $this->getPermissions($sponsor_id, $member_id), "slug must be stripped, row must stay"); } public function testFullSyncKeepsSponsorsGroupAndSponsorUsersRowWhenPayloadStillHasIt(): void @@ -138,7 +140,7 @@ public function testFullSyncKeepsRowAndOtherSlugsWhenRowHoldsMoreThanTheRemovedG ); } - public function testFullSyncWithoutExternalUsersGroupRemovesLocalGroupAndSponsorUsersRow(): void + public function testFullSyncWithoutExternalUsersGroupRemovesLocalGroupAndStripsSlugFromSponsorUsersRow(): void { $member_id = self::$member->getId(); $sponsor_id = self::$sponsors[0]->getId(); @@ -163,7 +165,7 @@ public function testFullSyncWithoutExternalUsersGroupRemovesLocalGroupAndSponsor $member = self::$member_repository->find($member_id); $this->assertTrue($member->belongsToGroup(IGroup::Sponsors)); $this->assertFalse($member->belongsToGroup(IGroup::SponsorExternalUsers)); - $this->assertNull($this->getPermissions($sponsor_id, $member_id)); + $this->assertEquals([], $this->getPermissions($sponsor_id, $member_id)); } finally { $g = self::$em->getRepository(Group::class)->findOneBy(['code' => IGroup::SponsorExternalUsers]); if (!is_null($g)) { From 0e9f1e3ca9f800063049a1aefc4cdf8c15e12bcb Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 10 Oct 2026 19:15:21 -0300 Subject: [PATCH 3/3] chore(members): remove unused Member::removeSponsorPermissionFromAllSponsors --- app/Models/Foundation/Main/Member.php | 64 --------------------------- 1 file changed, 64 deletions(-) diff --git a/app/Models/Foundation/Main/Member.php b/app/Models/Foundation/Main/Member.php index a891e22b2f..846f6407c4 100644 --- a/app/Models/Foundation/Main/Member.php +++ b/app/Models/Foundation/Main/Member.php @@ -3599,70 +3599,6 @@ public function removeSponsorPermission(int $sponsor_id, string $group_slug): in ])->executeQuery()->fetchOne()); } - /** - * Strips $group_slug from the Permissions of EVERY Sponsor_Users row of this member and drops the - * membership of the rows that held that slug and are left without any permission. Used when the - * IDP no longer carries the sponsor group, so its access cannot outlive the group. - * Rows that never held the slug (or still hold another one) are left untouched. - * - * @param string $group_slug - * @return void - */ - public function removeSponsorPermissionFromAllSponsors(string $group_slug): void - { - $params = ['member_id' => $this->getId(), 'group_slug' => $group_slug]; - - $sponsor_ids = $this->prepareRawSQL( - <<executeQuery()->fetchFirstColumn(); - - if (count($sponsor_ids) === 0) return; - - $this->prepareRawSQL( - <<executeStatement(); - - $emptied_sponsor_ids = $this->prepareRawSQL( - << $this->getId()] - )->executeQuery()->fetchFirstColumn(); - - // only the rows this call emptied, never an unrelated row that was already empty - $to_remove = array_intersect(array_map('intval', $sponsor_ids), array_map('intval', $emptied_sponsor_ids)); - - foreach ($this->sponsor_memberships->toArray() as $sponsor) { - if (in_array(intval($sponsor->getId()), $to_remove, true)) { - $sponsor->removeUser($this); - } - } - } - public function addSponsorMembership(Sponsor $sponsor):void { if($this->sponsor_memberships->contains($sponsor)) return;