diff --git a/app/Jobs/SponsorServices/RemoveSponsorMemberMQJob.php b/app/Jobs/SponsorServices/RemoveSponsorMemberMQJob.php index 6fab40201..824041af9 100644 --- a/app/Jobs/SponsorServices/RemoveSponsorMemberMQJob.php +++ b/app/Jobs/SponsorServices/RemoveSponsorMemberMQJob.php @@ -54,7 +54,9 @@ public function __construct(ISponsorUserSyncService $service) public function handle(SponsorServicesMQJob $job): void { try { - $event_type = $job->getRabbitMQMessage()->getRoutingKey(); + // getEventType, not the raw routing key: a redelivered (released) + // message carries the queue name as routing key. + $event_type = $job->getEventType(); $payload = $job->payload(); $json = json_encode($payload); Log::debug("RemoveSponsorMemberMQJob::handle payload {$json}"); diff --git a/app/Jobs/SponsorServices/SponsorServicesMQJob.php b/app/Jobs/SponsorServices/SponsorServicesMQJob.php index b7fd8b8f9..9b945eb6f 100644 --- a/app/Jobs/SponsorServices/SponsorServicesMQJob.php +++ b/app/Jobs/SponsorServices/SponsorServicesMQJob.php @@ -14,20 +14,74 @@ **/ use Illuminate\Support\Facades\Log; +use PhpAmqpLib\Message\AMQPMessage; +use PhpAmqpLib\Wire\AMQPTable; use VladimirYuldashev\LaravelQueueRabbitMQ\Queue\Jobs\RabbitMQJob as BaseJob; class SponsorServicesMQJob extends BaseJob { public int $tries = 3; + /** + * Seconds to wait before the 2nd and 3rd attempt, as the comma-separated + * string Worker::calculateBackoff() explodes. + * + * These events race the IDP's own user-updated event, which is what refreshes + * a member's groups locally. Retrying immediately loses that race every time; + * for auth_user_added_to_sponsor_and_summit that is terminal, because no + * companion group event follows to repair the state. + */ + const RetryBackoff = '30,120'; + + /** + * Body key that carries the original event type across a release(): a + * released message is dead-lettered back through the default exchange, so + * it is redelivered with the QUEUE NAME as its routing key and the event + * type would otherwise be lost (see release()). + */ + const EventTypeKey = 'x_event_type'; + + private const KnownEventTypes = [ + EventTypes::AUTH_USER_ADDED_TO_GROUP, + EventTypes::AUTH_USER_REMOVED_FROM_GROUP, + EventTypes::AUTH_USER_ADDED_TO_SPONSOR_AND_SUMMIT, + EventTypes::AUTH_USER_REMOVED_FROM_SPONSOR_AND_SUMMIT, + EventTypes::AUTH_USER_REMOVED_FROM_SUMMIT, + ]; + + /** + * The event type driving handler selection. On a first delivery it is the + * message's routing key; on a redelivery after release() the routing key is + * the queue name and the original event type travels in the body. Handlers + * that branch on the event type must use this, never the raw routing key. + * + * @return string + */ + public function getEventType(): string + { + $routing_key = $this->getRabbitMQMessage()->getRoutingKey(); + if (in_array($routing_key, self::KnownEventTypes, true)) { + return $routing_key; + } + + $body = json_decode($this->getRawBody(), true); + return $body[self::EventTypeKey] ?? $routing_key; + } + /** * Get the decoded body of the job. * + * Note the maxTries/backoff keys: Job::maxTries() and Job::backoff() read the + * PAYLOAD, not the properties of the handler class this payload names. Without + * them the worker falls back to the command's own options, and the entry point + * runs `doctrine:queue:work sponsor_users_sync_consumer` with no flags - so the + * defaults of --tries=1 and --backoff=0 apply and a single failure is final. + * * @return array */ public function payload(): array { - $routing_key = $this->getRabbitMQMessage()->getRoutingKey(); + $routing_key = $this->getEventType(); switch ($routing_key) { case EventTypes::AUTH_USER_ADDED_TO_GROUP: @@ -47,7 +101,81 @@ public function payload(): array } return [ 'job' => $job, - 'data' => json_decode($this->getRawBody(), true) + 'data' => json_decode($this->getRawBody(), true), + 'maxTries' => $this->tries, + 'backoff' => self::RetryBackoff, ]; } + + /** + * Release the job back into the queue for a retry. + * + * The library implementation publishes into a delay queue whose dead-letter + * exchange is the consumer exchange (sponsor-users-api-message-broker) and + * whose dead-letter routing key is this queue's NAME. That exchange is + * direct and only binds the five auth_user_* routing keys, so the expired + * retry is unroutable and RabbitMQ silently drops it - the retry policy + * would lose every failed event instead of retrying it. + * + * Dead-letter through the DEFAULT exchange instead: it routes by queue name + * with no binding required. Redelivery rewrites the routing key to the queue + * name, so the original event type is preserved in the body (EventTypeKey) + * for getEventType() to recover. + * + * @param int $delay + */ + public function release($delay = 0): void + { + $this->released = true; + + $ttl = $this->secondsUntil($delay) * 1000; + if ($ttl <= 0) { + // Never skip the delay queue: publishing straight to the consumer + // exchange with the queue name as routing key is unroutable (above). + $ttl = 1000; + } + $delay_queue = $this->queue . '.delay.' . $ttl; + + // Declare + publish DIRECTLY on the channel, bypassing laterRaw(): + // - laterRaw() re-declares the delay queue with its own dead-letter + // arguments, which the broker rejects (PRECONDITION_FAILED, + // inequivalent args) once the queue exists with ours; + // - suppressing that re-declare by priming the declared-names cache + // would mean the queue is declared only once per worker process, + // while x-expires DELETES it when idle - every later release would + // then publish into a deleted queue and be dropped silently. + // An unconditional queue_declare per release re-creates the queue when + // it expired and is a no-op (equivalent args) when it did not. + $channel = $this->rabbitmq->getChannel(); + + $channel->queue_declare($delay_queue, false, true, false, false, false, new AMQPTable([ + 'x-dead-letter-exchange' => '', + 'x-dead-letter-routing-key' => $this->queue, + 'x-message-ttl' => $ttl, + 'x-expires' => $ttl * 2, + ])); + + // Preserve the original event type across the redelivery. ALWAYS write + // the resolved value: on a first delivery the routing key is + // authoritative, so this overwrites any forged/buggy x_event_type the + // producer body may have carried; on a redelivery getEventType() already + // resolves from the body, so rewriting it is a no-op (idempotent). + $body = json_decode($this->getRawBody(), true) ?? []; + $body[self::EventTypeKey] = $this->getEventType(); + + $channel->basic_publish( + new AMQPMessage(json_encode($body), [ + 'content_type' => 'application/json', + 'delivery_mode' => AMQPMessage::DELIVERY_MODE_PERSISTENT, + 'correlation_id' => uniqid('', true), + // attempts() reads this header on the next delivery. + 'application_headers' => new AMQPTable(['laravel' => ['attempts' => $this->attempts()]]), + ]), + '', // default exchange: routes by queue name, no binding required + $delay_queue + ); + + // The retry was republished as a new message; ack the current one. + $this->rabbitmq->ack($this); + } } diff --git a/app/Jobs/SponsorServices/UpdateSponsorMemberGroupsMQJob.php b/app/Jobs/SponsorServices/UpdateSponsorMemberGroupsMQJob.php index 2442aa732..482d816dc 100644 --- a/app/Jobs/SponsorServices/UpdateSponsorMemberGroupsMQJob.php +++ b/app/Jobs/SponsorServices/UpdateSponsorMemberGroupsMQJob.php @@ -54,7 +54,9 @@ public function __construct(ISponsorUserSyncService $service) public function handle(SponsorServicesMQJob $job): void { try { - $event_type = $job->getRabbitMQMessage()->getRoutingKey(); + // getEventType, not the raw routing key: a redelivered (released) + // message carries the queue name as routing key. + $event_type = $job->getEventType(); $payload = $job->payload(); $json = json_encode($payload); Log::debug("UpdateSponsorMemberGroupsMQJob::handle payload {$json}"); diff --git a/app/Services/Model/Imp/SponsorUserSyncService.php b/app/Services/Model/Imp/SponsorUserSyncService.php index 98708a478..4004b8cf1 100644 --- a/app/Services/Model/Imp/SponsorUserSyncService.php +++ b/app/Services/Model/Imp/SponsorUserSyncService.php @@ -12,15 +12,21 @@ * limitations under the License. **/ +use App\Services\Apis\IExternalUserApi; use App\Services\Model\AbstractService; +use App\Services\Model\IMemberService; use App\Services\Model\ISponsorUserSyncService; use Illuminate\Support\Facades\Log; use LaravelDoctrine\ORM\Facades\Registry; use libs\utils\ITransactionService; use models\exceptions\EntityNotFoundException; +use models\exceptions\ValidationException; use models\main\IGroupRepository; use models\main\IMemberRepository; +use models\main\Member; +use App\Models\Foundation\Summit\Repositories\ISponsorRepository; use models\summit\ISummitRepository; +use models\summit\Sponsor; use models\summit\Summit; use models\utils\SilverstripeBaseModel; use services\model\ISummitSponsorService; @@ -41,12 +47,21 @@ final class SponsorUserSyncService private ISummitSponsorService $summit_sponsor_service; + private IMemberService $member_service; + + private IExternalUserApi $external_user_api; + + private ISponsorRepository $sponsor_repository; + /** * SponsorUserSyncService constructor. * @param ISummitRepository $summit_repository * @param IMemberRepository $member_repository * @param IGroupRepository $group_repository * @param ISummitSponsorService $summit_sponsor_service + * @param IMemberService $member_service + * @param IExternalUserApi $external_user_api + * @param ISponsorRepository $sponsor_repository * @param ITransactionService $tx_service */ public function __construct @@ -55,6 +70,9 @@ public function __construct IMemberRepository $member_repository, IGroupRepository $group_repository, ISummitSponsorService $summit_sponsor_service, + IMemberService $member_service, + IExternalUserApi $external_user_api, + ISponsorRepository $sponsor_repository, ITransactionService $tx_service ) { @@ -63,26 +81,135 @@ public function __construct $this->member_repository = $member_repository; $this->group_repository = $group_repository; $this->summit_sponsor_service = $summit_sponsor_service; + $this->member_service = $member_service; + $this->external_user_api = $external_user_api; + $this->sponsor_repository = $sponsor_repository; + } + + /** + * Resolves the local Member for an IDP user id, registering it on demand + * from the IDP when it was never synced (brand-new user that has not + * logged in yet). Throws EntityNotFoundException when the user does not + * exist at the IDP either. + * + * @param int $user_id external (IDP) user id + * @return Member + * @throws EntityNotFoundException + */ + private function resolveMember(int $user_id): Member + { + $member = $this->member_repository->getByExternalId($user_id); + if (!is_null($member)) return $member; + + Log::warning( + "SponsorUserSyncService::resolveMember member with external id {$user_id} not found locally - registering on demand from IDP"); + + return $this->member_service->registerExternalUserById($user_id); + } + + /** + * Looks up the local Member WITHOUT registering it on demand. Revocation + * paths use this: a member that was never synced owns no Sponsor_Users row + * and no group membership, so there is nothing to revoke and provisioning + * one from the IDP would be a pure side effect (Member row, full + * synchronizeGroups, NewMember / MemberDataUpdatedExternally jobs). + * + * @param int $user_id external (IDP) user id + * @return Member|null null when the member was never synced + */ + private function findMember(int $user_id): ?Member + { + return $this->member_repository->getByExternalId($user_id); + } + + /** + * Sponsor::addUser rejects a member that belongs to none of its + * AllowedMemberGroups. sponsor-users-api grants that group at the IDP before + * publishing the membership event (_sync_user_groups), but summit-api only + * learns about it through the IDP's own user-updated event, which races this + * one. resolveMember covers the member that does not exist yet - this covers + * the member that exists with a stale local group set: re-read the groups + * from the IDP, which is the source of truth, instead of failing. + * + * Nothing downstream would repair that failure: the producers of + * auth_user_added_to_sponsor_and_summit (_import_user, _notify_approval) emit + * no companion group event, so no eager-create path ever runs and the access + * is lost once the job exhausts its tries. + * + * The sync is ADDITIVE (allow_removals = false) on purpose: this event only + * ever GRANTS access, and removals stay owned by the IDP's own user_updated + * flow (PublishUserUpdated). A full authoritative re-sync here would strip + * locally-held groups absent from the IDP payload as a side effect of a + * sponsor-membership event. + * + * @param Member $member + * @param int $user_id external (IDP) user id + * @return Member the same member, with its groups refreshed when stale + * @throws \Exception + */ + private function ensureSponsorGroupMembership(Member $member, int $user_id): Member + { + foreach (Sponsor::AllowedMemberGroups as $group_slug) { + if ($member->belongsToGroup($group_slug)) return $member; + } + + Log::warning( + "SponsorUserSyncService::ensureSponsorGroupMembership member {$member->getId()} belongs to none of the allowed sponsor groups - refreshing groups from the IDP"); + + $user_data = $this->external_user_api->getUserById($user_id); + if (is_null($user_data)) { + throw new EntityNotFoundException( + "SponsorUserSyncService::ensureSponsorGroupMembership user {$user_id} does not exist at the IDP"); + } + + return $this->member_service->synchronizeGroups($member, $user_data['groups'] ?? [], false); + } + + /** + * The MQ payload's group_slug is producer-controlled input arriving over a + * broker vhost several services can write to: without this gate a forged or + * buggy auth_user_added_to_group / auth_user_removed_from_group event could + * grant - or strip - membership of an arbitrary group like administrators. + * + * @param string $group_slug + * @throws ValidationException + */ + private function assertAllowedSponsorGroup(string $group_slug): void + { + if (!in_array($group_slug, Sponsor::AllowedMemberGroups, true)) { + throw new ValidationException( + sprintf( + "Group %s is not an allowed sponsor group (%s).", + $group_slug, + implode(', ', Sponsor::AllowedMemberGroups) + ) + ); + } } /** * @param int $summit_id - * @param int $user_id - * @return array + * @return Summit * @throws EntityNotFoundException */ - public function validateParams(int $summit_id, int $user_id): array + private function resolveSummit(int $summit_id): Summit { $summit = $this->summit_repository->getById($summit_id); if (!$summit instanceof Summit) { throw new EntityNotFoundException("Summit {$summit_id} not found"); } + return $summit; + } - $member = $this->member_repository->getByExternalId($user_id); - if (is_null($member)) { - throw new EntityNotFoundException("Member with id {$user_id} not found"); - } - return array($summit, $member); + /** + * @param int $summit_id + * @param int $user_id + * @return array + * @throws EntityNotFoundException + */ + public function validateParams(int $summit_id, int $user_id): array + { + return array($this->resolveSummit($summit_id), $this->resolveMember($user_id)); } /** @@ -90,22 +217,23 @@ public function validateParams(int $summit_id, int $user_id): array */ public function addSponsorUser(int $summit_id, int $sponsor_id, int $user_id): void { - try { - Log::debug( - "SponsorUserSyncService::addSponsorUser summit {$summit_id} sponsor {$sponsor_id} user_id {$user_id}"); + // Do NOT swallow failures here: the MQ job (tries = 3) needs the + // exception to apply its retry / failed_jobs machinery. A swallowed + // failure loses the membership event silently. + Log::debug( + "SponsorUserSyncService::addSponsorUser summit {$summit_id} sponsor {$sponsor_id} user_id {$user_id}"); - list($summit, $member) = $this->validateParams($summit_id, $user_id); + list($summit, $member) = $this->validateParams($summit_id, $user_id); - Log::debug( - "SponsorUserSyncService::addSponsorUser summit {$summit->getName()} member {$member->getEmail()}"); + Log::debug( + "SponsorUserSyncService::addSponsorUser summit {$summit->getName()} member {$member->getEmail()}"); - $this->summit_sponsor_service->addSponsorUser($summit, $sponsor_id, $member->getId()); + $member = $this->ensureSponsorGroupMembership($member, $user_id); - Log::info( - "SponsorUserSyncService::addSponsorUser member {$member->getId()} successfully added to sponsor {$sponsor_id}"); - } catch (\Exception $ex) { - Log::error($ex); - } + $this->summit_sponsor_service->addSponsorUser($summit, $sponsor_id, $member->getId()); + + Log::info( + "SponsorUserSyncService::addSponsorUser member {$member->getId()} successfully added to sponsor {$sponsor_id}"); } /** @@ -113,31 +241,57 @@ public function addSponsorUser(int $summit_id, int $sponsor_id, int $user_id): v */ public function removeSponsorUser(int $summit_id, int $user_id, ?int $sponsor_id = null): void { - try { - Log::debug( - "SponsorUserSyncService::removeSponsorUser summit {$summit_id} sponsor {$sponsor_id} user_id {$user_id}"); + // Do NOT swallow failures here: a lost removal event silently leaves + // the user with access they should have lost. Propagate so the MQ job + // (tries = 3) applies its retry / failed_jobs machinery. + Log::debug( + "SponsorUserSyncService::removeSponsorUser summit {$summit_id} sponsor {$sponsor_id} user_id {$user_id}"); - list($summit, $member) = $this->validateParams($summit_id, $user_id); + $summit = $this->resolveSummit($summit_id); - Log::debug( - "SponsorUserSyncService::removeSponsorUser summit {$summit->getName()} member {$member->getEmail()}"); + // Revocation must not provision (see findMember). Skipping is also what + // keeps a deleted IDP user from parking an unresolvable entry in + // failed_jobs: sponsor-users-api emits a removal event per access right + // when a user is deleted, and by then the local Member is gone too. + $member = $this->findMember($user_id); + if (is_null($member)) { + Log::warning( + "SponsorUserSyncService::removeSponsorUser member with external id {$user_id} was never synced - nothing to revoke, skipping"); + return; + } - if (is_null($sponsor_id)) { - foreach ($member->getSponsorMemberships() as $sponsor_membership) { - $sponsor_id = $sponsor_membership->getId(); - $this->summit_sponsor_service->removeSponsorUser($summit, $sponsor_id, $member->getId()); + Log::debug( + "SponsorUserSyncService::removeSponsorUser summit {$summit->getName()} member {$member->getEmail()}"); + + if (is_null($sponsor_id)) { + // getSponsorMemberships() is a plain ManyToMany to Sponsor with no summit + // scoping, so it also yields sponsors of OTHER summits. Those do not resolve + // against $summit and would make removeSponsorUser throw "Sponsor not found.", + // aborting the loop and leaving this summit's own memberships un-revoked. + foreach ($member->getSponsorMemberships() as $sponsor_membership) { + if ($sponsor_membership->getSummit()->getId() !== $summit->getId()) continue; + + $current_sponsor_id = $sponsor_membership->getId(); + $this->summit_sponsor_service->removeSponsorUser($summit, $current_sponsor_id, $member->getId()); - Log::info( - "SponsorUserSyncService::removeSponsorUser: member {$member->getId()} successfully removed from summit {$summit->getId()} for sponsor {$sponsor_id}" - ); - } - } else { - $this->summit_sponsor_service->removeSponsorUser($summit, $sponsor_id, $member->getId()); Log::info( - "SponsorUserSyncService::removeSponsorUser: member {$member->getId()} successfully removed from to summit {$summit_id} for sponsor {$sponsor_id}"); + "SponsorUserSyncService::removeSponsorUser: member {$member->getId()} successfully removed from summit {$summit->getId()} for sponsor {$current_sponsor_id}" + ); + } + } else { + // sponsor-users-api's metamodel reconciler emits this event precisely + // when the sponsor no longer exists here (it reaps sponsors summit-api + // stopped returning, routing them through remove_sponsor_show_permissions + // so the domain events still fire). Nothing left to revoke - not a + // failure to burn retries on and park in failed_jobs. + if (is_null($summit->getSummitSponsorById($sponsor_id))) { + Log::warning( + "SponsorUserSyncService::removeSponsorUser sponsor {$sponsor_id} no longer exists on summit {$summit->getId()} - nothing to revoke, skipping"); + return; } - } catch (\Exception $ex) { - Log::error($ex); + $this->summit_sponsor_service->removeSponsorUser($summit, $sponsor_id, $member->getId()); + Log::info( + "SponsorUserSyncService::removeSponsorUser: member {$member->getId()} successfully removed from summit {$summit->getId()} for sponsor {$sponsor_id}"); } } @@ -146,13 +300,51 @@ public function removeSponsorUser(int $summit_id, int $user_id, ?int $sponsor_id */ public function addSponsorUserToGroup(int $user_id, string $group_slug, int $sponsor_id, int $summit_id): void { - $this->tx_service->transaction(function () use ($user_id, $group_slug, $sponsor_id, $summit_id) { - Log::debug( - "SponsorUserSyncService::addSponsorUserToGroup user_id {$user_id} group_slug {$group_slug} sponsor_id {$sponsor_id} summit_id {$summit_id}"); + Log::debug( + "SponsorUserSyncService::addSponsorUserToGroup user_id {$user_id} group_slug {$group_slug} sponsor_id {$sponsor_id} summit_id {$summit_id}"); + + // Gate BEFORE resolveMember: a rejected slug must not provision a member + // from the IDP as a side effect. + $this->assertAllowedSponsorGroup($group_slug); + + // The producer derives sponsor_id and summit_id from the same AccessRight, + // so a mismatched pair is a forged or buggy event - and a deleted sponsor + // leaves nothing to grant onto. Fail here, BEFORE resolveMember (same + // no-side-effect rule as above) and loudly (failed_jobs), rather than ever + // writing onto another summit's sponsor row. + $summit = $this->resolveSummit($summit_id); + if (is_null($summit->getSummitSponsorById($sponsor_id))) { + throw new ValidationException( + "Sponsor {$sponsor_id} does not belong to summit {$summit_id}."); + } + + // Resolve (and, if needed, register from the IDP) OUTSIDE the transaction below. + // registerExternalUserById opens its own transaction and dispatches NewMember / + // MemberDataUpdatedExternally right after it. Those jobs are pushed immediately: + // afterCommit only defers dispatch for Eloquent-managed transactions, and this + // service uses the Doctrine DBAL connection directly. Keeping the registration + // outside guarantees the Member row is committed before any job references its id. + $member_id = $this->resolveMember($user_id)->getId(); + + $this->tx_service->transaction(function () use ($member_id, $group_slug, $sponsor_id, $summit_id) { + + // Re-load inside the transaction: the tx service may have reset the entity + // manager, which would leave an entity resolved outside it detached. + $member = $this->member_repository->getById($member_id); + if (!$member instanceof Member) { + throw new EntityNotFoundException("Member with id {$member_id} not found"); + } - $member = $this->member_repository->getByExternalId($user_id); - if (is_null($member)) { - throw new EntityNotFoundException("Member with id {$user_id} not found"); + // Grant the global group FIRST: Sponsor::addUser (reached through the + // eager-create path below) validates the member already belongs to a + // sponsor group, and for a brand-new sponsor user this very event is + // what delivers that group. + if (!$member->belongsToGroup($group_slug)) { + $group = $this->group_repository->getBySlug($group_slug); + if (is_null($group)) { + throw new EntityNotFoundException("Group {$group_slug} not found"); + } + $member->add2Group($group); } // Add permission entry to the Sponsor_Users JSON column for this sponsor-member pair. @@ -184,15 +376,6 @@ public function addSponsorUserToGroup(int $user_id, string $group_slug, int $spo } } - // Add to global group only if not already a member. - if (!$member->belongsToGroup($group_slug)) { - $group = $this->group_repository->getBySlug($group_slug); - if (is_null($group)) { - throw new EntityNotFoundException("Group {$group_slug} not found"); - } - $member->add2Group($group); - } - Log::info( "SponsorUserSyncService::addSponsorUserToGroup member {$member->getId()} added to group {$group_slug} via sponsor {$sponsor_id}"); }); @@ -203,13 +386,41 @@ public function addSponsorUserToGroup(int $user_id, string $group_slug, int $spo */ public function removeSponsorUserFromGroup(int $user_id, string $group_slug, int $sponsor_id, int $summit_id): void { - $this->tx_service->transaction(function () use ($user_id, $group_slug, $sponsor_id, $summit_id) { - Log::debug( - "SponsorUserSyncService::removeSponsorUserFromGroup user_id {$user_id} group_slug {$group_slug} sponsor_id {$sponsor_id} summit_id {$summit_id}"); + Log::debug( + "SponsorUserSyncService::removeSponsorUserFromGroup user_id {$user_id} group_slug {$group_slug} sponsor_id {$sponsor_id} summit_id {$summit_id}"); + + $this->assertAllowedSponsorGroup($group_slug); + + // Reject only a sponsor that exists on a DIFFERENT summit (forged or buggy + // event - the producer derives both ids from the same AccessRight). A + // sponsor deleted ENTIRELY must NOT skip: its Sponsor_Users rows are gone, + // and running the removal is exactly what recomputes the remaining + // permission count and strips the global sponsors group when this was the + // member's last sponsor - skipping would leave residual show-admin access. + $sponsor = $this->sponsor_repository->getById($sponsor_id); + if ($sponsor instanceof Sponsor && $sponsor->getSummitId() !== $summit_id) { + Log::warning( + "SponsorUserSyncService::removeSponsorUserFromGroup sponsor {$sponsor_id} belongs to summit {$sponsor->getSummitId()}, not event summit {$summit_id} - skipping"); + return; + } + + // Revocation must not provision (see findMember): a member that was never + // synced holds no permission entry and no group membership to remove. + $member = $this->findMember($user_id); + if (is_null($member)) { + Log::warning( + "SponsorUserSyncService::removeSponsorUserFromGroup member with external id {$user_id} was never synced - nothing to revoke, skipping"); + return; + } + $member_id = $member->getId(); + + $this->tx_service->transaction(function () use ($member_id, $group_slug, $sponsor_id, $summit_id) { - $member = $this->member_repository->getByExternalId($user_id); - if (is_null($member)) { - throw new EntityNotFoundException("Member with id {$user_id} not found"); + // Re-load inside the transaction: the tx service may have reset the entity + // manager, which would leave an entity resolved outside it detached. + $member = $this->member_repository->getById($member_id); + if (!$member instanceof Member) { + throw new EntityNotFoundException("Member with id {$member_id} not found"); } // Remove permission entry from JSON and get remaining sponsor count. diff --git a/tests/InsertMemberTestData.php b/tests/InsertMemberTestData.php index a329f9b40..d916dc546 100644 --- a/tests/InsertMemberTestData.php +++ b/tests/InsertMemberTestData.php @@ -162,6 +162,23 @@ protected static function clearMemberTestData() self::$em = Registry::resetManager(SilverstripeBaseModel::EntityManager); } + // Re-resolve the repositories UNCONDITIONALLY: a failed tx_service + // transaction closes and resets the manager mid-test, and a test + // finally that already reopened it leaves self::$em fresh and OPEN - + // an isOpen() check here would then skip the refresh while the + // repositories captured at setup still point at the closed manager, + // making the finds below fail and the fixtures LEAK. Leaked duplicate + // Group rows make getBySlug() resolve a stale row and turn group + // removals into silent no-ops on every later run. + self::$group_repository = self::$em->getRepository(Group::class); + self::$member_repository = self::$em->getRepository(Member::class); + + // Detach whatever the test left in the unit of work: entity unit + // tests build unpersisted object graphs hanging off managed + // fixtures, and flushing them here throws "non-persisted new + // entities found" - the cleanup must only flush its own removals. + self::$em->clear(); + self::$member = self::$member_repository->find(self::$member->getId()); self::$group = self::$group_repository->find(self::$group->getId()); @@ -185,7 +202,15 @@ protected static function clearMemberTestData() self::$em->flush(); } catch (\Exception $ex) { - + // Do NOT let a cleanup failure stay invisible: leaked fixtures + // poison the shared test database for every subsequent run (leaked + // duplicate Group rows once turned group removals into silent + // no-ops suite-wide). Log AND rethrow so the test fails loudly. + fwrite(STDERR, sprintf( + "clearMemberTestData failed - fixtures may have leaked: %s\n", + $ex->getMessage() + )); + throw $ex; } } } \ No newline at end of file diff --git a/tests/InsertSummitTestData.php b/tests/InsertSummitTestData.php index 9a7def1e0..ff76a40f3 100644 --- a/tests/InsertSummitTestData.php +++ b/tests/InsertSummitTestData.php @@ -1052,6 +1052,18 @@ protected static function clearSummitTestData(){ if (!self::$em ->isOpen()) { self::$em = Registry::resetManager(SilverstripeBaseModel::EntityManager); } + // Unconditional refresh - same staleness trap as clearMemberTestData + // (see the note there): the manager may have been reset mid-test while + // self::$em was already reopened, so isOpen() alone cannot tell whether + // the repositories captured at setup are still usable. + self::$summit_repository = self::$em->getRepository(Summit::class); + self::$summit_permission_group_repository = self::$em->getRepository(SummitAdministratorPermissionGroup::class); + self::$media_file_type_repository = self::$em->getRepository(SummitMediaFileType::class); + + // Detach whatever the test left in the unit of work (see the note in + // clearMemberTestData) - the cleanup must only flush its own removals. + self::$em->clear(); + self::$summit = self::$summit_repository->find(self::$summit->getId()); self::$summit2 = self::$summit_repository->find(self::$summit2->getId()); self::$default_media_file_type = self::$media_file_type_repository->find(self::$default_media_file_type->getId()); diff --git a/tests/Unit/Jobs/SponsorServicesMQJobReleaseIntegrationTest.php b/tests/Unit/Jobs/SponsorServicesMQJobReleaseIntegrationTest.php new file mode 100644 index 000000000..52ecf7091 --- /dev/null +++ b/tests/Unit/Jobs/SponsorServicesMQJobReleaseIntegrationTest.php @@ -0,0 +1,182 @@ +getChannel(); // force the lazy connection open + } catch (\Throwable $ex) { + $this->markTestSkipped("RabbitMQ broker not reachable: {$ex->getMessage()}"); + } + + $this->queue = $queue; + + $suffix = uniqid(); + $this->queue_name = "test-sponsor-users-release-{$suffix}"; + $this->exchange_name = "test-sponsor-users-ex-{$suffix}"; + + // A direct exchange bound only by the event-type routing key, mirroring + // the production topology (sponsor-users-api-message-broker). + $channel->exchange_declare($this->exchange_name, 'direct', false, false, true); + $channel->queue_declare($this->queue_name, false, true, false, false); + $channel->queue_bind($this->queue_name, $this->exchange_name, EventTypes::AUTH_USER_ADDED_TO_GROUP); + } + + protected function tearDown(): void + { + if (!is_null($this->queue)) { + foreach ([$this->queue_name, $this->queue_name . '.delay.1000'] as $queue) { + try { + $this->queue->getChannel(true)->queue_delete($queue); + } catch (\Throwable $ex) { + // already gone (x-expires) or never created - nothing to clean + } + } + try { + $this->queue->getChannel(true)->exchange_delete($this->exchange_name); + } catch (\Throwable $ex) { + } + } + parent::tearDown(); + } + + public function testReleasedJobIsRedeliveredWithItsOriginalEventType(): void + { + $body = [ + 'user_external_id' => 1, + 'sponsor_id' => 2, + 'summit_id' => 3, + 'group_slug' => 'sponsors', + ]; + + $this->queue->getChannel()->basic_publish( + new AMQPMessage(json_encode($body), [ + 'content_type' => 'application/json', + 'delivery_mode' => AMQPMessage::DELIVERY_MODE_PERSISTENT, + ]), + $this->exchange_name, + EventTypes::AUTH_USER_ADDED_TO_GROUP + ); + + // First delivery: the routing key IS the event type. + $job = $this->popWithinSeconds(5); + $this->assertInstanceOf(SponsorServicesMQJob::class, $job); + $this->assertSame(EventTypes::AUTH_USER_ADDED_TO_GROUP, $job->getEventType()); + $this->assertSame(1, $job->attempts()); + + // The real broker enforces queue-argument equivalence and binding + // resolution - the two failure modes the mocked unit tests cannot see. + $job->release(1); + + $redelivered = $this->popWithinSeconds(10); + $this->assertNotNull($redelivered, 'the released message must be redelivered after the delay'); + $this->assertSame( + EventTypes::AUTH_USER_ADDED_TO_GROUP, + $redelivered->getEventType(), + 'the original event type must survive redelivery' + ); + $this->assertSame(2, $redelivered->attempts(), 'the attempt count must carry over'); + $this->assertSame(3, $redelivered->maxTries()); + + $data = $redelivered->payload()['data']; + foreach ($body as $key => $value) { + $this->assertSame($value, $data[$key] ?? null, "original payload key {$key} must be preserved"); + } + + $redelivered->delete(); // ack, leave the queue clean + } + + /** + * The delay queue carries x-expires, so the broker DELETES it once idle. + * A later release in the same long-lived worker process must re-create it + * - an implementation that only declares once per process (e.g. by caching + * the declared name) publishes the retry into a deleted queue and the + * default exchange drops it silently. + */ + public function testASecondReleaseAfterTheDelayQueueExpiredStillRedelivers(): void + { + $this->queue->getChannel()->basic_publish( + new AMQPMessage(json_encode(['user_external_id' => 1]), [ + 'content_type' => 'application/json', + 'delivery_mode' => AMQPMessage::DELIVERY_MODE_PERSISTENT, + ]), + $this->exchange_name, + EventTypes::AUTH_USER_ADDED_TO_GROUP + ); + + $job = $this->popWithinSeconds(5); + $this->assertNotNull($job); + $job->release(1); // creates .delay.1000 (x-expires 2000) + + $redelivered = $this->popWithinSeconds(10); + $this->assertNotNull($redelivered, 'first retry must be redelivered'); + + // Let the (now idle) delay queue hit its x-expires and be deleted. + sleep(3); + + $redelivered->release(1); + + $third = $this->popWithinSeconds(10); + $this->assertNotNull($third, 'a release after the delay queue expired must re-create it and still redeliver'); + $this->assertSame(EventTypes::AUTH_USER_ADDED_TO_GROUP, $third->getEventType()); + $this->assertSame(3, $third->attempts()); + + $third->delete(); + } + + private function popWithinSeconds(int $max_seconds): ?SponsorServicesMQJob + { + $deadline = microtime(true) + $max_seconds; + do { + $job = $this->queue->pop($this->queue_name); + if (!is_null($job)) { + return $job; + } + usleep(200000); + } while (microtime(true) < $deadline); + + return null; + } +} diff --git a/tests/Unit/Jobs/SponsorServicesMQJobRetryTest.php b/tests/Unit/Jobs/SponsorServicesMQJobRetryTest.php new file mode 100644 index 000000000..dae3277d1 --- /dev/null +++ b/tests/Unit/Jobs/SponsorServicesMQJobRetryTest.php @@ -0,0 +1,291 @@ +setDeliveryInfo(1, false, 'sponsor_users', $routing_key); + + $job = Mockery::mock(SponsorServicesMQJob::class)->makePartial(); + $job->shouldReceive('getRabbitMQMessage')->andReturn($message); + $job->shouldReceive('getRawBody')->andReturn(json_encode(array_merge([ + 'data' => [ + 'user_external_id' => 1, + 'sponsor_id' => 2, + 'summit_id' => 3, + 'group_slug' => 'sponsors', + ], + ], $extra_body))); + + return $job; + } + + public static function routingKeyProvider(): array + { + return [ + 'added to sponsor and summit' => [EventTypes::AUTH_USER_ADDED_TO_SPONSOR_AND_SUMMIT], + 'added to group' => [EventTypes::AUTH_USER_ADDED_TO_GROUP], + 'removed from group' => [EventTypes::AUTH_USER_REMOVED_FROM_GROUP], + 'removed from summit' => [EventTypes::AUTH_USER_REMOVED_FROM_SUMMIT], + ]; + } + + #[DataProvider('routingKeyProvider')] + public function testWorkerSeesMoreThanASingleAttempt(string $routing_key): void + { + $this->assertSame(3, $this->jobForRoutingKey($routing_key)->maxTries()); + } + + #[DataProvider('routingKeyProvider')] + public function testWorkerSeesAnIncreasingBackoff(string $routing_key): void + { + $backoff = $this->jobForRoutingKey($routing_key)->backoff(); + + // Worker::calculateBackoff() explodes this on commas and indexes it by attempt. + $delays = array_map('intval', explode(',', (string)$backoff)); + + $this->assertCount(2, $delays, 'one delay per retry after the first attempt'); + $this->assertGreaterThan(0, $delays[0], 'the first retry must not be immediate'); + $this->assertGreaterThan($delays[0], $delays[1], 'the backoff must grow'); + } + + /** + * An unknown routing key returns an empty payload and must not claim a retry + * policy - there is no handler to retry. + */ + public function testUnknownRoutingKeyCarriesNoRetryPolicy(): void + { + $job = $this->jobForRoutingKey('some_unknown_routing_key'); + + $this->assertNull($job->maxTries()); + $this->assertNull($job->backoff()); + } + + // ------------------------------------------------------------------------- + // Redelivery after release() + // ------------------------------------------------------------------------- + + public static function redeliveredEventProvider(): array + { + return [ + 'added to sponsor and summit' => [EventTypes::AUTH_USER_ADDED_TO_SPONSOR_AND_SUMMIT, 'App\Jobs\SponsorServices\AddSponsorMemberMQJob@handle'], + 'added to group' => [EventTypes::AUTH_USER_ADDED_TO_GROUP, 'App\Jobs\SponsorServices\UpdateSponsorMemberGroupsMQJob@handle'], + 'removed from group' => [EventTypes::AUTH_USER_REMOVED_FROM_GROUP, 'App\Jobs\SponsorServices\UpdateSponsorMemberGroupsMQJob@handle'], + 'removed from summit' => [EventTypes::AUTH_USER_REMOVED_FROM_SUMMIT, 'App\Jobs\SponsorServices\RemoveSponsorMemberMQJob@handle'], + 'removed from sponsor + summit' => [EventTypes::AUTH_USER_REMOVED_FROM_SPONSOR_AND_SUMMIT, 'App\Jobs\SponsorServices\RemoveSponsorMemberMQJob@handle'], + ]; + } + + /** + * A released message is dead-lettered back through the DEFAULT exchange, so + * it is redelivered with the QUEUE NAME as its routing key - the original + * event type only survives inside the body (release() puts it there). The + * job must still resolve the right handler and keep its retry policy. + */ + #[DataProvider('redeliveredEventProvider')] + public function testRedeliveredMessageResolvesOriginalEventType(string $event_type, string $expected_handler): void + { + $job = $this->jobForRoutingKey( + 'sponsor-users-api-summit-api-badge-scans-queue', // != any EventTypes constant + [SponsorServicesMQJob::EventTypeKey => $event_type] + ); + + $this->assertSame($event_type, $job->getEventType()); + $this->assertSame($expected_handler, $job->payload()['job'] ?? null); + $this->assertSame(3, $job->maxTries()); + $this->assertNotNull($job->backoff()); + } + + /** + * release() must NOT use the library's delay-queue arguments: those + * dead-letter into the consumer exchange (direct, only the five auth_user_* + * bindings) with the queue name as routing key - unroutable, silently + * dropped. It must dead-letter through the DEFAULT exchange (routes by + * queue name, no binding needed) and preserve the original event type in + * the republished body, because redelivery overwrites the routing key. + */ + public function testReleaseDeadLettersBackThroughTheDefaultExchange(): void + { + $queue_name = 'sponsor-users-api-summit-api-badge-scans-queue'; + $original_body = ['user_external_id' => 1, 'sponsor_id' => 2, 'summit_id' => 3]; + + $message = new AMQPMessage(json_encode($original_body)); + $message->setDeliveryInfo(1, false, 'sponsor_users', EventTypes::AUTH_USER_ADDED_TO_SPONSOR_AND_SUMMIT); + + $rabbitmq = Mockery::mock(RabbitMQQueue::class); + $channel = Mockery::mock(\PhpAmqpLib\Channel\AMQPChannel::class); + $rabbitmq->shouldReceive('getChannel')->andReturn($channel); + + // Re-declared on EVERY release (x-expires deletes the idle queue), with + // dead-lettering through the default exchange - the consumer exchange is + // direct and only binds the auth_user_* keys, so anything else drops the + // retried message. + $channel->shouldReceive('queue_declare')->once()->with( + $queue_name . '.delay.30000', + false, + true, + false, + false, + false, + Mockery::on(function ($arguments) use ($queue_name) { + return $arguments instanceof \PhpAmqpLib\Wire\AMQPTable + && $arguments->getNativeData() == [ + 'x-dead-letter-exchange' => '', + 'x-dead-letter-routing-key' => $queue_name, + 'x-message-ttl' => 30000, + 'x-expires' => 60000, + ]; + }) + ); + + $republished = null; + $channel->shouldReceive('basic_publish')->once()->with( + Mockery::on(function ($msg) use (&$republished) { + $republished = $msg instanceof AMQPMessage ? $msg->getBody() : null; + return $msg instanceof AMQPMessage; + }), + '', // default exchange + $queue_name . '.delay.30000' + ); + + $rabbitmq->shouldReceive('ack')->once(); + + $job = new SponsorServicesMQJob(app(), $rabbitmq, $message, 'rabbitmq', $queue_name); + $job->release(30); + + $this->assertTrue($job->isReleased()); + + $body = json_decode($republished, true); + $this->assertSame( + EventTypes::AUTH_USER_ADDED_TO_SPONSOR_AND_SUMMIT, + $body[SponsorServicesMQJob::EventTypeKey] ?? null, + 'the original event type must survive redelivery in the body' + ); + foreach ($original_body as $key => $value) { + $this->assertSame($value, $body[$key] ?? null, "original payload key {$key} must be preserved"); + } + } + + /** + * A forged/buggy x_event_type in a FIRST-delivery body must not survive the + * release: on first delivery the routing key is authoritative (getEventType + * ignores the body), so release() must write the RESOLVED event type over + * whatever the body carried - otherwise the retry would run a different + * handler than the original delivery did. + */ + public function testReleaseOverwritesAForgedEventTypeWithTheResolvedOne(): void + { + $queue_name = 'sponsor-users-api-summit-api-badge-scans-queue'; + + // Routing key says ADDED_TO_GROUP; the body smuggles a conflicting type. + $message = new AMQPMessage(json_encode([ + 'user_external_id' => 1, + SponsorServicesMQJob::EventTypeKey => EventTypes::AUTH_USER_REMOVED_FROM_SUMMIT, + ])); + $message->setDeliveryInfo(1, false, 'sponsor_users', EventTypes::AUTH_USER_ADDED_TO_GROUP); + + $rabbitmq = Mockery::mock(RabbitMQQueue::class); + $channel = Mockery::mock(\PhpAmqpLib\Channel\AMQPChannel::class); + $rabbitmq->shouldReceive('getChannel')->andReturn($channel); + $channel->shouldReceive('queue_declare')->once(); + + $republished = null; + $channel->shouldReceive('basic_publish')->once()->with( + Mockery::on(function ($msg) use (&$republished) { + $republished = $msg instanceof AMQPMessage ? $msg->getBody() : null; + return $msg instanceof AMQPMessage; + }), + '', + Mockery::any() + ); + $rabbitmq->shouldReceive('ack')->once(); + + $job = new SponsorServicesMQJob(app(), $rabbitmq, $message, 'rabbitmq', $queue_name); + $job->release(30); + + $body = json_decode($republished, true); + $this->assertSame( + EventTypes::AUTH_USER_ADDED_TO_GROUP, + $body[SponsorServicesMQJob::EventTypeKey] ?? null, + 'release must overwrite a smuggled x_event_type with the routing-key-resolved event type' + ); + } + + /** + * A second release (the message already carries the event type from the + * first one) must keep the ORIGINAL event type, not overwrite it with the + * redelivered routing key (the queue name). + */ + public function testReleaseOfARedeliveredMessageKeepsTheOriginalEventType(): void + { + $queue_name = 'sponsor-users-api-summit-api-badge-scans-queue'; + + // As redelivered: routing key already the queue name, event type in body. + $message = new AMQPMessage(json_encode([ + 'user_external_id' => 1, + SponsorServicesMQJob::EventTypeKey => EventTypes::AUTH_USER_ADDED_TO_GROUP, + ])); + $message->setDeliveryInfo(1, false, 'sponsor_users', $queue_name); + + $rabbitmq = Mockery::mock(RabbitMQQueue::class); + $channel = Mockery::mock(\PhpAmqpLib\Channel\AMQPChannel::class); + $rabbitmq->shouldReceive('getChannel')->andReturn($channel); + $channel->shouldReceive('queue_declare')->once(); + + $republished = null; + $channel->shouldReceive('basic_publish')->once()->with( + Mockery::on(function ($msg) use (&$republished) { + $republished = $msg instanceof AMQPMessage ? $msg->getBody() : null; + return $msg instanceof AMQPMessage; + }), + '', + $queue_name . '.delay.120000' + ); + $rabbitmq->shouldReceive('ack')->once(); + + $job = new SponsorServicesMQJob(app(), $rabbitmq, $message, 'rabbitmq', $queue_name); + $job->release(120); + + $body = json_decode($republished, true); + $this->assertSame(EventTypes::AUTH_USER_ADDED_TO_GROUP, $body[SponsorServicesMQJob::EventTypeKey] ?? null); + } +} diff --git a/tests/Unit/Services/SponsorUserPermissionTrackingTest.php b/tests/Unit/Services/SponsorUserPermissionTrackingTest.php index 68a3acef6..915f54dfa 100644 --- a/tests/Unit/Services/SponsorUserPermissionTrackingTest.php +++ b/tests/Unit/Services/SponsorUserPermissionTrackingTest.php @@ -71,6 +71,37 @@ private function getService(): ISponsorUserSyncService return app(ISponsorUserSyncService::class); } + /** + * Replaces the IDP user API with a mock returning $user_data (null = the + * user does not exist at the IDP) and forces the services that may have + * been resolved against the real API to rebuild. + */ + private function mockExternalUserApi(?array $user_data): void + { + $api = \Mockery::mock(\App\Services\Apis\IExternalUserApi::class) + ->shouldIgnoreMissing(); + $api->shouldReceive('getUserById')->andReturn($user_data); + $this->app->instance(\App\Services\Apis\IExternalUserApi::class, $api); + $this->app->forgetInstance(\App\Services\Model\IMemberService::class); + $this->app->forgetInstance(\App\Services\Model\ISponsorUserSyncService::class); + } + + /** + * A failed tx_service transaction closes the entity manager and resets it in the + * registry (see DoctrineTransactionService::transaction), leaving the static one + * captured at setUp time unusable. Returns a usable manager either way. + */ + private static function reopenEntityManager(): \Doctrine\ORM\EntityManagerInterface + { + if (!self::$em->isOpen()) { + return \LaravelDoctrine\ORM\Facades\Registry::resetManager( + \models\utils\SilverstripeBaseModel::EntityManager + ); + } + self::$em->clear(); + return self::$em; + } + /** * Returns the decoded Permissions JSON array for a given (SponsorID, MemberID) * row in Sponsor_Users, or an empty array when the column is NULL. @@ -89,6 +120,18 @@ private function getPermissions(int $sponsor_id, int $member_id): array return json_decode($raw, true) ?? []; } + /** + * Whether a Sponsor_Users row exists for the given (SponsorID, MemberID) pair, + * regardless of its Permissions value. + */ + private function hasSponsorUserRow(int $sponsor_id, int $member_id): bool + { + return (bool)self::$em->getConnection()->executeQuery( + 'SELECT 1 FROM Sponsor_Users WHERE SponsorID = ? AND MemberID = ?', + [$sponsor_id, $member_id] + )->fetchOne(); + } + // ------------------------------------------------------------------------- // addSponsorUserToGroup // ------------------------------------------------------------------------- @@ -123,6 +166,285 @@ public function testAddSponsorUserToGroupEagerlyCreatesRowAndWritesPermissionOnR $this->assertContains(IGroup::Sponsors, $this->getPermissions($sponsor_id, $member_id)); } + /** + * Brand-new sponsor user: the member exists but does NOT belong to any + * sponsor group yet - the group grant is exactly what this event delivers. + * The eager-create path must not fail Sponsor::addUser's group validation + * (chicken-and-egg: the validation requires the group this handler grants). + */ + public function testAddSponsorUserToGroupCreatesRowWhenMemberHasNoSponsorGroupYet(): void + { + // member2 belongs only to SummitAdministrators - no sponsor group, + // exactly the state of a brand-new sponsor user's member row. + $member_id = self::$member2->getId(); + $external_id = self::$member2->getUserExternalId(); + $sponsor_id = self::$sponsors[1]->getId(); // no Sponsor_Users row + $summit_id = self::$summit->getId(); + + // Pre-condition: the member must NOT belong to any sponsor group. + $this->assertFalse( + self::$member_repository->find($member_id)->belongsToGroup(IGroup::Sponsors), + 'Pre-condition: member should not belong to the sponsors group' + ); + + $this->getService()->addSponsorUserToGroup( + $external_id, + IGroup::Sponsors, + $sponsor_id, + $summit_id + ); + + // Row created + permission written... + $this->assertContains(IGroup::Sponsors, $this->getPermissions($sponsor_id, $member_id)); + + // ...and the member ended up in the global group. + self::$em->clear(); + $this->assertTrue( + self::$member_repository->find($member_id)->belongsToGroup(IGroup::Sponsors) + ); + } + + /** + * The MQ payload's group_slug is attacker/producer-controlled input: the + * shared broker vhost grants write access to several services, so a forged + * or buggy auth_user_added_to_group with e.g. 'administrators' must never + * be granted. Only Sponsor::AllowedMemberGroups may flow through this sync. + */ + public function testAddSponsorUserToGroupRejectsNonSponsorGroup(): void + { + $this->expectException(\models\exceptions\ValidationException::class); + + $this->getService()->addSponsorUserToGroup( + self::$member->getUserExternalId(), + IGroup::Administrators, + self::$sponsors[0]->getId(), + self::$summit->getId() + ); + } + + /** + * Same contract on the removal path: a forged removal event must not be + * able to strip a member from an arbitrary group like 'administrators'. + */ + public function testRemoveSponsorUserFromGroupRejectsNonSponsorGroup(): void + { + $this->expectException(\models\exceptions\ValidationException::class); + + $this->getService()->removeSponsorUserFromGroup( + self::$member->getUserExternalId(), + IGroup::Administrators, + self::$sponsors[0]->getId(), + self::$summit->getId() + ); + } + + /** + * Builds a second sponsor belonging to summit2 (a DIFFERENT summit than the + * event summit used by these tests). Caller must clean it up with + * deleteCrossSummitSponsor() in a finally block. + */ + private function createCrossSummitSponsor(): \models\summit\Sponsor + { + $summit2 = self::$summit_repository->getById(self::$summit2->getId()); + $company = self::$em->find(\models\main\Company::class, self::$companies[1]->getId()); + + $other_sponsor = new \models\summit\Sponsor(); + $other_sponsor->setCompany($company); + $summit2->addSummitSponsor($other_sponsor); + self::$em->persist($other_sponsor); + self::$em->flush(); + + return $other_sponsor; + } + + /** + * Removes the cross-summit sponsor THROUGH the EntityManager. A raw-SQL + * delete would leave the managed entity dangling in the unit of work, + * referencing a Summit the teardown is about to remove - the next flush + * then fails with "new entity found through relationship" and the + * member/group fixtures leak into the shared test database. + */ + private function deleteCrossSummitSponsor(\models\summit\Sponsor $sponsor): void + { + $managed = self::$em->find(\models\summit\Sponsor::class, $sponsor->getId()); + if (!is_null($managed)) { + self::$em->remove($managed); // owning side: Sponsor_Users rows go with it + self::$em->flush(); + } + } + + /** + * The producer derives sponsor_id and summit_id from the same AccessRight, + * so a mismatched pair is a forged or buggy event. When the member already + * holds a Sponsor_Users row on the foreign sponsor, the permission must NOT + * be written onto another summit's sponsor row. + */ + public function testAddSponsorUserToGroupRejectsSponsorFromAnotherSummit(): void + { + $member = self::$member_repository->find(self::$member->getId()); + $other_sponsor = $this->createCrossSummitSponsor(); + $other_sponsor->addUser($member); // row on the FOREIGN sponsor exists + self::$em->flush(); + + $member_id = $member->getId(); + $external_id = $member->getUserExternalId(); + $other_sponsor_id = $other_sponsor->getId(); + + try { + $thrown = null; + try { + // Event claims the FIRST summit but carries summit2's sponsor. + $this->getService()->addSponsorUserToGroup( + $external_id, + IGroup::Sponsors, + $other_sponsor_id, + self::$summit->getId() + ); + } catch (\models\exceptions\ValidationException $ex) { + $thrown = $ex; + } + + $this->assertNotNull($thrown, 'a sponsor from another summit must be rejected'); + $this->assertEmpty( + $this->getPermissions($other_sponsor_id, $member_id), + 'no permission may be written onto another summit\'s sponsor row' + ); + } finally { + $this->deleteCrossSummitSponsor($other_sponsor); + } + } + + /** + * The ownership gate must run BEFORE resolveMember: a rejected event must + * not provision a member from the IDP as a side effect. + */ + public function testAddSponsorUserToGroupRejectedCrossSummitEventDoesNotProvisionMember(): void + { + $other_sponsor = $this->createCrossSummitSponsor(); + $other_sponsor_id = $other_sponsor->getId(); + $external_id = mt_rand(1500000000, 2000000000); // no local Member row + $email = sprintf("smarcet+xsummit_%s@gmail.com", str_random(8)); + + // The user DOES exist at the IDP - on-demand registration would succeed. + $this->mockExternalUserApi([ + 'id' => $external_id, + 'email' => $email, + 'first_name' => 'Cross', + 'last_name' => 'Summit', + 'bio' => '', + 'active' => true, + 'email_verified' => true, + 'groups' => [], + 'public_profile_show_photo' => false, + 'public_profile_show_fullname' => false, + 'public_profile_show_email' => false, + 'public_profile_show_telephone_number' => false, + 'public_profile_show_bio' => false, + 'public_profile_show_social_media_info' => false, + 'public_profile_allow_chat_with_me' => false, + ]); + + try { + $thrown = null; + try { + $this->getService()->addSponsorUserToGroup( + $external_id, + IGroup::Sponsors, + $other_sponsor_id, + self::$summit->getId() + ); + } catch (\models\exceptions\ValidationException $ex) { + $thrown = $ex; + } + + $this->assertNotNull($thrown, 'a sponsor from another summit must be rejected'); + $this->assertNull( + self::$member_repository->getByExternalId($external_id), + 'a rejected event must not provision a member from the IDP' + ); + } finally { + $this->deleteCrossSummitSponsor($other_sponsor); + $leftover = self::$member_repository->getByExternalId($external_id); + if (!is_null($leftover)) { + self::$em->remove($leftover); + self::$em->flush(); + } + } + } + + /** + * A removal event carrying a sponsor of ANOTHER summit must be a no-op: it + * must neither touch the foreign sponsor's permission entry nor strip the + * member's global group. + */ + public function testRemoveSponsorUserFromGroupSkipsSponsorFromAnotherSummit(): void + { + $member = self::$member_repository->find(self::$member->getId()); + $other_sponsor = $this->createCrossSummitSponsor(); + $other_sponsor->addUser($member); + self::$em->flush(); + + $member_id = $member->getId(); + $external_id = $member->getUserExternalId(); + $other_sponsor_id = $other_sponsor->getId(); + + try { + // Legit grant on summit2 writes the permission on the foreign row. + $this->getService()->addSponsorUserToGroup( + $external_id, IGroup::Sponsors, $other_sponsor_id, self::$summit2->getId()); + $this->assertContains(IGroup::Sponsors, $this->getPermissions($other_sponsor_id, $member_id)); + + // Forged/buggy removal: summit1's event carrying summit2's sponsor. + $this->getService()->removeSponsorUserFromGroup( + $external_id, IGroup::Sponsors, $other_sponsor_id, self::$summit->getId()); + + self::$em->clear(); + $this->assertContains( + IGroup::Sponsors, + $this->getPermissions($other_sponsor_id, $member_id), + 'a cross-summit removal must not touch the foreign sponsor\'s permission entry' + ); + $this->assertTrue( + self::$member_repository->find($member_id)->belongsToGroup(IGroup::Sponsors), + 'a cross-summit removal must not strip the global group' + ); + } finally { + $this->deleteCrossSummitSponsor($other_sponsor); + } + } + + /** + * A sponsor deleted ENTIRELY must not skip the removal: its Sponsor_Users + * rows are gone, and running the removal is exactly what recomputes the + * remaining permission count and strips the global sponsors group when this + * was the member's last sponsor. Skipping would leave the member with + * residual show-admin access forever. + */ + public function testRemoveSponsorUserFromGroupStillCleansUpWhenSponsorWasDeleted(): void + { + $member_id = self::$member->getId(); + $external_id = self::$member->getUserExternalId(); + + // Pre-condition: member holds the global group and NO remaining + // permission entries (the fixture row has a NULL Permissions column). + $this->assertTrue( + self::$member_repository->find($member_id)->belongsToGroup(IGroup::Sponsors) + ); + + $this->getService()->removeSponsorUserFromGroup( + $external_id, + IGroup::Sponsors, + PHP_INT_MAX, // sponsor no longer exists anywhere + self::$summit->getId() + ); + + self::$em->clear(); + $this->assertFalse( + self::$member_repository->find($member_id)->belongsToGroup(IGroup::Sponsors), + 'the last-sponsor cleanup must still strip the global group when the sponsor row is gone' + ); + } + /** * The group slug must be written into the Sponsor_Users.Permissions JSON * column for the correct (SponsorID, MemberID) row. @@ -164,6 +486,470 @@ public function testAddSponsorUserToGroupIsIdempotent(): void $this->assertCount(1, $occurrences); } + // ------------------------------------------------------------------------- + // addSponsorUser (membership event) + // ------------------------------------------------------------------------- + + /** + * When the member does not exist yet in summit-api (brand-new IDP user whose + * member row has not been synced), the failure must PROPAGATE so the MQ job's + * retry/failed_jobs machinery applies - not be swallowed and lost silently. + */ + public function testAddSponsorUserPropagatesErrorWhenMemberDoesNotExist(): void + { + // User does not exist locally NOR at the IDP. + $this->mockExternalUserApi(null); + + $this->expectException(\models\exceptions\EntityNotFoundException::class); + + $this->getService()->addSponsorUser( + self::$summit->getId(), + self::$sponsors[1]->getId(), + PHP_INT_MAX // external user id with no matching Member row + ); + } + + /** + * Brand-new IDP user whose Member row was never synced to summit-api + * (the user never logged in): the sync must register the member on demand + * from the IDP instead of failing - otherwise both MQ events exhaust their + * retries against a missing member and the access grant is lost for good. + */ + public function testAddSponsorUserToGroupRegistersMemberOnDemandWhenMissing(): void + { + $external_id = mt_rand(1500000000, 2000000000); // no local Member row + $sponsor_id = self::$sponsors[1]->getId(); + $summit_id = self::$summit->getId(); + $email = sprintf("smarcet+ondemand_%s@gmail.com", str_random(8)); + + $this->mockExternalUserApi([ + 'id' => $external_id, + 'email' => $email, + 'first_name' => 'On', + 'last_name' => 'Demand', + 'bio' => '', + 'active' => true, + 'email_verified' => true, + 'groups' => [], + 'public_profile_show_photo' => false, + 'public_profile_show_fullname' => false, + 'public_profile_show_email' => false, + 'public_profile_show_telephone_number' => false, + 'public_profile_show_bio' => false, + 'public_profile_show_social_media_info' => false, + 'public_profile_allow_chat_with_me' => false, + ]); + + try { + $this->getService()->addSponsorUserToGroup( + $external_id, + IGroup::Sponsors, + $sponsor_id, + $summit_id + ); + + // Member must have been registered on demand from the IDP... + // (clear first: the in-service instance memoizes a pre-grant + // belongsToGroup(false) in its groupMembershipCache) + self::$em->clear(); + $member = self::$member_repository->getByExternalId($external_id); + $this->assertNotNull($member, 'Member should have been registered on demand'); + + // ...with the Sponsor_Users row + permission written and the group granted. + $this->assertContains(IGroup::Sponsors, $this->getPermissions($sponsor_id, $member->getId())); + $this->assertTrue($member->belongsToGroup(IGroup::Sponsors)); + } finally { + // This member is created on demand, outside the trait's tearDown scope, + // so it has to be removed here - and in a finally, or a failing assertion + // above leaks it into the next test's database. Look it up again instead + // of reusing $member: the failure may have happened before it was set. + $leftover = self::$member_repository->getByExternalId($external_id); + if (!is_null($leftover)) { + self::$em->remove($leftover); + self::$em->flush(); + } + } + } + + /** + * On-demand registration must happen OUTSIDE addSponsorUserToGroup's transaction. + * + * registerExternalUserById dispatches NewMember / MemberDataUpdatedExternally, whose + * listeners enqueue MemberAssocSummitOrders, UpdateAttendeeInfo and CleanMemberCacheJob. + * Those pushes are NOT deferred to commit: afterCommit only works for transactions that + * Laravel's DatabaseTransactionsManager can see, and this service opens its transaction + * straight on the Doctrine DBAL connection. So if the member were registered inside the + * transaction and the transaction later rolled back, the Member row would vanish while + * the already-queued jobs kept pointing at its id - they would fail forever. + * + * Here the group row for an ALLOWED slug does not exist, so the transaction + * throws AFTER the member was resolved (the allowlist and sponsor-ownership + * gates both pass, and the missing Group row is only discovered inside the + * transaction). The member must still be present afterwards. + */ + public function testAddSponsorUserToGroupKeepsOnDemandMemberWhenTransactionFails(): void + { + $external_id = mt_rand(1500000000, 2000000000); // no local Member row + $sponsor_id = self::$sponsors[1]->getId(); // real sponsor: passes the ownership gate + $summit_id = self::$summit->getId(); + $email = sprintf("smarcet+rollback_%s@gmail.com", str_random(8)); + + // Ensure no Group row exists for the allowed slug used below, so the + // in-transaction getBySlug lookup fails AFTER resolveMember succeeded. + // (Fixtures create their own groups per test, so deleting is safe.) + $conn = self::$em->getConnection(); + $conn->executeStatement( + 'DELETE gm FROM Group_Members gm INNER JOIN `Group` g ON g.ID = gm.GroupID WHERE g.Code = ?', + [IGroup::SponsorExternalUsers] + ); + $conn->executeStatement('DELETE FROM `Group` WHERE Code = ?', [IGroup::SponsorExternalUsers]); + + $this->mockExternalUserApi([ + 'id' => $external_id, + 'email' => $email, + 'first_name' => 'Roll', + 'last_name' => 'Back', + 'bio' => '', + 'active' => true, + 'email_verified' => true, + 'groups' => [], + 'public_profile_show_photo' => false, + 'public_profile_show_fullname' => false, + 'public_profile_show_email' => false, + 'public_profile_show_telephone_number' => false, + 'public_profile_show_bio' => false, + 'public_profile_show_social_media_info' => false, + 'public_profile_allow_chat_with_me' => false, + ]); + + try { + $thrown = null; + try { + $this->getService()->addSponsorUserToGroup( + $external_id, + IGroup::SponsorExternalUsers, // allowed slug with no Group row: throws inside the tx + $sponsor_id, + $summit_id + ); + } catch (\models\exceptions\EntityNotFoundException $ex) { + $thrown = $ex; + } + + $this->assertNotNull($thrown, 'The missing group row should have failed the transaction'); + + // The on-demand member was committed by its own transaction, so the jobs + // already dispatched for it reference a row that exists. + self::$em = self::reopenEntityManager(); + $this->assertNotNull( + self::$member_repository->getByExternalId($external_id), + 'On-demand member must survive the rolled back outer transaction' + ); + } finally { + self::$em = self::reopenEntityManager(); + $leftover = self::$member_repository->getByExternalId($external_id); + if (!is_null($leftover)) { + self::$em->remove($leftover); + self::$em->flush(); + } + } + } + + /** + * removeSponsorUser with a null sponsor_id (the auth_user_removed_from_summit + * event, which carries no sponsor_id) must revoke ONLY the memberships that + * belong to the summit the event is about. + * + * Member::getSponsorMemberships() is a plain ManyToMany to Sponsor with no + * summit scoping, so it also returns sponsors of other summits. Those do not + * resolve against $summit and make SummitSponsorService::removeSponsorUser + * throw "Sponsor not found.", aborting the loop and leaving the memberships + * of the event's own summit un-revoked. + */ + public function testRemoveSponsorUserWithoutSponsorIdOnlyRevokesTheEventSummit(): void + { + // setUp() ends with an em->clear(), so the static fixture entities are + // detached: re-fetch them or Doctrine treats them as new on persist. + $member = self::$member_repository->find(self::$member->getId()); + $summit2 = self::$summit_repository->getById(self::$summit2->getId()); + $company = self::$em->find(\models\main\Company::class, self::$companies[1]->getId()); + + // A second sponsor, belonging to a DIFFERENT summit, with the same member. + $other_sponsor = new \models\summit\Sponsor(); + $other_sponsor->setCompany($company); + $summit2->addSummitSponsor($other_sponsor); + $other_sponsor->addUser($member); + self::$em->persist($other_sponsor); + self::$em->flush(); + + $member_id = $member->getId(); + $external_id = $member->getUserExternalId(); + $event_sponsor_id = self::$sponsors[0]->getId(); // belongs to self::$summit + $other_sponsor_id = $other_sponsor->getId(); // belongs to self::$summit2 + + // Pre-condition: the member holds a membership in BOTH summits. + $this->assertTrue($this->hasSponsorUserRow($event_sponsor_id, $member_id)); + $this->assertTrue($this->hasSponsorUserRow($other_sponsor_id, $member_id)); + + try { + // auth_user_removed_from_summit: no sponsor_id, only the summit. + $this->getService()->removeSponsorUser( + self::$summit->getId(), + $external_id, + null + ); + + self::$em->clear(); + + $this->assertFalse( + $this->hasSponsorUserRow($event_sponsor_id, $member_id), + 'the membership of the event summit must have been revoked' + ); + $this->assertTrue( + $this->hasSponsorUserRow($other_sponsor_id, $member_id), + 'the membership of an unrelated summit must be left untouched' + ); + } finally { + $conn = self::$em->getConnection(); + $conn->executeStatement('DELETE FROM Sponsor_Users WHERE SponsorID = ?', [$other_sponsor_id]); + $conn->executeStatement('DELETE FROM Sponsor WHERE ID = ?', [$other_sponsor_id]); + } + } + + /** + * The membership event (auth_user_added_to_sponsor_and_summit) is emitted by + * sponsor-users-api WITHOUT any companion group event - see _import_user and + * _notify_approval, neither of which publishes auth_user_added_to_group. So + * nothing downstream repairs a failure here: if Sponsor::addUser rejects the + * member for not belonging to an allowed sponsor group, the Sponsor_Users row + * is never created and the access is lost for good. + * + * The producer does add the group at the IDP before publishing, so the IDP is + * already correct; it is the local copy that is stale. A member that does not + * exist yet is covered by resolveMember's on-demand registration - this covers + * the member that DOES exist locally with outdated groups. + */ + public function testAddSponsorUserRefreshesStaleGroupsFromIdp(): void + { + // member2 belongs only to SummitAdministrators - not an allowed sponsor group. + $member_id = self::$member2->getId(); + $external_id = self::$member2->getUserExternalId(); + $sponsor_id = self::$sponsors[1]->getId(); // no Sponsor_Users row yet + + $this->assertFalse( + self::$member_repository->find($member_id)->belongsToGroup(IGroup::Sponsors), + 'Pre-condition: member must not belong to an allowed sponsor group' + ); + $this->assertFalse($this->hasSponsorUserRow($sponsor_id, $member_id)); + + // The IDP already carries the group (the producer syncs it before publishing). + $this->mockExternalUserApi([ + 'id' => $external_id, + 'email' => self::$member2->getEmail(), + 'first_name' => self::$member2->getFirstName(), + 'last_name' => self::$member2->getLastName(), + 'bio' => '', + 'active' => true, + 'email_verified' => true, + 'groups' => [IGroup::Sponsors], + 'public_profile_show_photo' => false, + 'public_profile_show_fullname' => false, + 'public_profile_show_email' => false, + 'public_profile_show_telephone_number' => false, + 'public_profile_show_bio' => false, + 'public_profile_show_social_media_info' => false, + 'public_profile_allow_chat_with_me' => false, + ]); + + $this->getService()->addSponsorUser( + self::$summit->getId(), + $sponsor_id, + $external_id + ); + + self::$em->clear(); + $this->assertTrue( + $this->hasSponsorUserRow($sponsor_id, $member_id), + 'the Sponsor_Users row must have been created after refreshing groups from the IDP' + ); + + $member = self::$member_repository->find($member_id); + $this->assertTrue( + $member->belongsToGroup(IGroup::Sponsors), + 'the sponsor group must have been refreshed from the IDP' + ); + // The refresh must be ADDITIVE: this event only ever grants access, and + // removals stay owned by the IDP's own user_updated flow. A full + // authoritative re-sync here would strip locally-held groups absent + // from the IDP payload (like this one) as a side effect. + $this->assertTrue( + $member->belongsToGroup(IGroup::SummitAdministrators), + 'unrelated local groups must not be stripped by the refresh' + ); + } + + /** + * A member that was never synced holds no Sponsor_Users row and no group + * membership, so a revocation event for it has nothing to revoke: it is a + * no-op, not a failure. Turning it into an exception would burn the job's + * 3 tries and park a permanently unresolvable entry in failed_jobs. + */ + public function testRemoveSponsorUserIsNoOpWhenMemberWasNeverSynced(): void + { + // User does not exist locally NOR at the IDP. + $this->mockExternalUserApi(null); + + $this->getService()->removeSponsorUser( + self::$summit->getId(), + PHP_INT_MAX, // external user id with no matching Member row + self::$sponsors[0]->getId() + ); + + $this->assertNull(self::$member_repository->getByExternalId(PHP_INT_MAX)); + } + + /** + * Revocation must never PROVISION. When the member was never synced but the + * user does still exist at the IDP, registering it on demand would create a + * Member row, run a full synchronizeGroups and dispatch NewMember / + * MemberDataUpdatedExternally jobs - all to then revoke nothing, since a + * member that did not exist owns no Sponsor_Users row. + */ + public function testRemoveSponsorUserDoesNotProvisionMemberFromIdp(): void + { + $external_id = mt_rand(1500000000, 2000000000); // no local Member row + $email = sprintf("smarcet+norevokeprov_%s@gmail.com", str_random(8)); + + // The user DOES exist at the IDP - on-demand registration would succeed. + $this->mockExternalUserApi([ + 'id' => $external_id, + 'email' => $email, + 'first_name' => 'No', + 'last_name' => 'Provision', + 'bio' => '', + 'active' => true, + 'email_verified' => true, + 'groups' => [], + 'public_profile_show_photo' => false, + 'public_profile_show_fullname' => false, + 'public_profile_show_email' => false, + 'public_profile_show_telephone_number' => false, + 'public_profile_show_bio' => false, + 'public_profile_show_social_media_info' => false, + 'public_profile_allow_chat_with_me' => false, + ]); + + try { + $this->getService()->removeSponsorUser( + self::$summit->getId(), + $external_id, + self::$sponsors[0]->getId() + ); + + self::$em->clear(); + $this->assertNull( + self::$member_repository->getByExternalId($external_id), + 'a revocation event must not create a Member row' + ); + } finally { + $leftover = self::$member_repository->getByExternalId($external_id); + if (!is_null($leftover)) { + self::$em->remove($leftover); + self::$em->flush(); + } + } + } + + /** + * Same contract for the group-scoped revocation entry point. + */ + public function testRemoveSponsorUserFromGroupDoesNotProvisionMemberFromIdp(): void + { + $external_id = mt_rand(1500000000, 2000000000); // no local Member row + $email = sprintf("smarcet+norevokegrp_%s@gmail.com", str_random(8)); + + $this->mockExternalUserApi([ + 'id' => $external_id, + 'email' => $email, + 'first_name' => 'No', + 'last_name' => 'Provision', + 'bio' => '', + 'active' => true, + 'email_verified' => true, + 'groups' => [], + 'public_profile_show_photo' => false, + 'public_profile_show_fullname' => false, + 'public_profile_show_email' => false, + 'public_profile_show_telephone_number' => false, + 'public_profile_show_bio' => false, + 'public_profile_show_social_media_info' => false, + 'public_profile_allow_chat_with_me' => false, + ]); + + try { + $this->getService()->removeSponsorUserFromGroup( + $external_id, + IGroup::Sponsors, + self::$sponsors[0]->getId(), + self::$summit->getId() + ); + + self::$em->clear(); + $this->assertNull( + self::$member_repository->getByExternalId($external_id), + 'a group revocation event must not create a Member row' + ); + } finally { + $leftover = self::$member_repository->getByExternalId($external_id); + if (!is_null($leftover)) { + self::$em->remove($leftover); + self::$em->flush(); + } + } + } + + /** + * sponsor-users-api's metamodel reconciler emits removal events precisely + * when the sponsor no longer exists here (it reaps the sponsors summit-api + * stopped returning, routing them through remove_sponsor_show_permissions + * so the domain events still fire). A missing sponsor is therefore + * nothing-to-revoke - not a failure that burns the job's retries and parks + * a permanently unresolvable entry in failed_jobs. + */ + public function testRemoveSponsorUserIsNoOpWhenSponsorNoLongerExists(): void + { + $member_id = self::$member->getId(); + $sponsor_id = self::$sponsors[0]->getId(); + + // Pre-condition: the member holds a membership on this summit. + $this->assertTrue($this->hasSponsorUserRow($sponsor_id, $member_id)); + + // Removal event pointing at a sponsor that no longer exists on the summit. + $this->getService()->removeSponsorUser( + self::$summit->getId(), + self::$member->getUserExternalId(), + PHP_INT_MAX + ); + + // No exception, and the member's existing membership is untouched. + $this->assertTrue($this->hasSponsorUserRow($sponsor_id, $member_id)); + } + + /** + * Skipping an unknown member must not turn removeSponsorUser into a + * swallow-everything handler: a genuine failure still has to propagate so + * the MQ job's retry / failed_jobs machinery applies. + */ + public function testRemoveSponsorUserPropagatesErrorWhenSummitDoesNotExist(): void + { + $this->expectException(\models\exceptions\EntityNotFoundException::class); + + $this->getService()->removeSponsorUser( + PHP_INT_MAX, // no such summit + self::$member->getUserExternalId(), + self::$sponsors[0]->getId() + ); + } + // ------------------------------------------------------------------------- // removeSponsorUserFromGroup // ------------------------------------------------------------------------- @@ -185,13 +971,6 @@ public function testRemoveSponsorUserFromGroupRemovesGlobalGroupWhenLastSponsor( $service->addSponsorUserToGroup($external_id, IGroup::Sponsors, $sponsor_id, $summit_id); $this->assertContains(IGroup::Sponsors, $this->getPermissions($sponsor_id, $member_id)); - // Doctrine ORM 3 EXTRA_LAZY PersistentCollection::removeElement() delegates to - // parent::removeElement() on the in-memory ArrayCollection first. If the collection - // is still uninitialized (addSponsorUserToGroup leaves it that way), that call - // returns false and changed() is never called, so the flush issues no DELETE. - // Force-initialize through the same model EM so removeFromGroup works correctly. - self::$em->find(\models\main\Member::class, $member_id)->getGroups()->toArray(); - // Remove — no other sponsor holds the permission. $service->removeSponsorUserFromGroup($external_id, IGroup::Sponsors, $sponsor_id, $summit_id);