fix: review feedback

Signed-off-by: romanetar <roman_ag@hotmail.com>

Author: romanetar

app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitBadgeScanApiController.php

@@ -126,6 +126,12 @@ protected function addChild(Summit $summit, array $payload): IEntity
         if (is_null($current_member))
             throw new HTTP403ForbiddenException();
 
+        $isSponsorUser = $current_member->isSponsorUser() || $current_member->isExternalSponsorUser();
+
+        if (!$isSponsorUser) {
+            throw new HTTP403ForbiddenException('Only sponsor users can add badge scans.');
+        }
+
         return $this->service->addBadgeScan($summit, $current_member, $payload);
     }
 

app/Jobs/SponsorServices/UpdateSponsorMemberGroupsMQJob.php

@@ -60,11 +60,19 @@ public function handle(SponsorServicesMQJob $job): void
             Log::debug("UpdateSponsorMemberGroupsMQJob::handle payload {$json}");
 
             $data = $payload['data'];
+            if (!isset($data['user_external_id'], $data['group_slug'], $data['sponsor_id'], $data['summit_id'])) {
+                throw new ValidationException('Invalid payload: user_external_id, group_slug, sponsor_id and summit_id are required.');
+            }
+
             $user_external_id = intval($data['user_external_id']);
             $group_slug = $data['group_slug'];
             $sponsor_id = intval($data['sponsor_id']);
             $summit_id = intval($data['summit_id']);
 
+            if ($user_external_id <= 0 || $sponsor_id <= 0 || $summit_id <= 0 || trim((string)$group_slug) === '') {
+                throw new ValidationException('Invalid payload: identifiers must be positive and group_slug must be non-empty.');
+            }
+
             if ($event_type === EventTypes::AUTH_USER_ADDED_TO_GROUP) {
                 $this->service->addSponsorUserToGroup($user_external_id, $group_slug, $sponsor_id, $summit_id);
             } else if ($event_type === EventTypes::AUTH_USER_REMOVED_FROM_GROUP) {
@@ -85,4 +93,4 @@ public function failed(array $data, Throwable $exception): void
     {
         Log::error("UpdateSponsorMemberGroupsMQJob::failed {$exception->getMessage()}");
     }
-}
+}

app/Models/Foundation/Main/Member.php

@@ -19,6 +19,7 @@
 use App\Models\Foundation\Main\IGroup;
 use App\Models\Foundation\Main\Strategies\MemberSummitStrategyFactory;
 use App\Models\Foundation\Summit\Events\RSVP\RSVPInvitation;
+use Doctrine\DBAL\Exception;
 use Doctrine\ORM\Query\ResultSetMappingBuilder;
 use Illuminate\Support\Facades\Config;
 use Doctrine\DBAL\ParameterType;
@@ -1860,21 +1861,20 @@ public function getActiveSummitsSponsorMemberships()
             return [];
         }
 
-        // Step 2 — load each Sponsor by PK. find() uses a different code path that avoids
-        // the ORM 3 assertion failure triggered by the OneToOne inverse associations on Sponsor
-        // (lead_report_setting, sponsorservices_statistics) when using DQL/native query hydration.
-        $sponsors = [];
-        foreach ($ids as $id) {
-            $sponsor = $this->getEM()->find(\models\summit\Sponsor::class, $id);
-            if ($sponsor !== null) {
-                $sponsors[] = $sponsor;
-            }
-        }
+        // Step 2 — load all sponsors in a single IN query. findBy() uses PK-based hydration
+        // which avoids the ORM 3 assertion failure triggered by the OneToOne inverse associations
+        // on Sponsor (lead_report_setting, sponsorservices_statistics) that DQL/native-query
+        // hydration hits. The result set is then re-sorted to match the SQL ORDER BY.
+        $position = array_flip($ids);
+        $sponsors = EntityManager::getRepository(Sponsor::class)->findBy(['id' => $ids]);
+        usort($sponsors, fn($a, $b) => $position[$a->getId()] <=> $position[$b->getId()]);
         return $sponsors;
     }
 
     /**
+     * @param Summit $summit
      * @return array
+     * @throws Exception
      */
     public function getSponsorMembershipIds(Summit $summit): array
     {
@@ -3466,9 +3466,23 @@ public function getIndividualMemberJoinDate(): ?\DateTime
      * Appends $group_slug to the Permissions JSON array on the Sponsor_Users row
      * for this member and the given sponsor. Idempotent: the slug is only added
      * when it is not already present.
+     *
+     * An exclusive row lock (SELECT … FOR UPDATE) is acquired first so that
+     * concurrent jobs for the same (member, sponsor, slug) serialize here and
+     * the second job always reads the post-first-job value, preventing duplicates.
+     *
+     * Returns the number of rows matched by the WHERE clause (0 when the
+     * Sponsor_Users row does not yet exist, 1 when it does).
      */
-    public function addSponsorPermission(int $sponsor_id, string $group_slug): void
+    public function addSponsorPermission(int $sponsor_id, string $group_slug): int
     {
+        // Lock the row before the read-modify-write so concurrent transactions
+        // serialize and the IF(JSON_CONTAINS) in the UPDATE sees the committed state.
+        $this->prepareRawSQL(
+            'SELECT Permissions FROM Sponsor_Users WHERE SponsorID = :sponsor_id AND MemberID = :member_id FOR UPDATE',
+            ['sponsor_id' => $sponsor_id, 'member_id' => $this->getId()]
+        )->executeQuery();
+
         $sql = <<<SQL
 UPDATE Sponsor_Users
 SET Permissions = IF(
@@ -3478,45 +3492,65 @@ public function addSponsorPermission(int $sponsor_id, string $group_slug): void
 )
 WHERE SponsorID = :sponsor_id AND MemberID = :member_id
 SQL;
-        $stmt = $this->prepareRawSQL($sql, [
+        return $this->prepareRawSQL($sql, [
             'group_slug' => $group_slug,
             'sponsor_id' => $sponsor_id,
             'member_id'  => $this->getId(),
-        ]);
-        $stmt->executeStatement();
+        ])->executeStatement();
     }
 
     /**
      * Removes $group_slug from the Permissions JSON array on the Sponsor_Users row
      * for this member and the given sponsor, then returns how many other Sponsor_Users
      * rows for this member still carry that slug. The caller uses the count to decide
      * whether to also revoke the global group membership.
+     *
+     * An exclusive row lock is acquired first so the remove UPDATE and the
+     * remaining-count SELECT are not interleaved with concurrent operations.
+     * All occurrences of the slug are removed (via JSON_ARRAYAGG filter) to
+     * prevent stale entries if a prior race introduced duplicates.
      */
     public function removeSponsorPermission(int $sponsor_id, string $group_slug): int
     {
+        // Serialize concurrent removals for the same row.
+        $this->prepareRawSQL(
+            'SELECT Permissions FROM Sponsor_Users WHERE SponsorID = :sponsor_id AND MemberID = :member_id FOR UPDATE',
+            ['sponsor_id' => $sponsor_id, 'member_id' => $this->getId()]
+        )->executeQuery();
+
+        // Remove ALL occurrences (not just the first) so duplicate slugs
+        // introduced by any prior race cannot leave stale entries behind.
         $removeSQL = <<<SQL
 UPDATE Sponsor_Users
-SET Permissions = JSON_REMOVE(Permissions, JSON_UNQUOTE(JSON_SEARCH(Permissions, 'one', :group_slug)))
+SET Permissions = COALESCE(
+    (
+        SELECT JSON_ARRAYAGG(element)
+        FROM JSON_TABLE(
+            COALESCE(Permissions, '[]'), '$[*]'
+            COLUMNS(element VARCHAR(255) PATH '$')
+        ) AS jt
+        WHERE element != :group_slug
+    ),
+    '[]'
+)
 WHERE SponsorID = :sponsor_id AND MemberID = :member_id
 AND JSON_CONTAINS(COALESCE(Permissions, '[]'), JSON_QUOTE(:group_slug))
 SQL;
-        $stmt = $this->prepareRawSQL($removeSQL, [
+        $this->prepareRawSQL($removeSQL, [
             'group_slug' => $group_slug,
             'sponsor_id' => $sponsor_id,
             'member_id'  => $this->getId(),
-        ]);
-        $stmt->executeStatement();
+        ])->executeStatement();
 
         $countSQL = <<<SQL
 SELECT COUNT(*) FROM Sponsor_Users
 WHERE MemberID = :member_id
 AND JSON_CONTAINS(COALESCE(Permissions, '[]'), JSON_QUOTE(:group_slug))
 SQL;
-        $stmt = $this->prepareRawSQL($countSQL, [
+        return intval($this->prepareRawSQL($countSQL, [
             'member_id'  => $this->getId(),
             'group_slug' => $group_slug,
-        ]);
-        return intval($stmt->executeQuery()->fetchOne());
+        ])->executeQuery()->fetchOne());
     }
 
     public function addSponsorMembership(Sponsor $sponsor):void

app/Services/Model/Imp/SponsorUserInfoGrantService.php

@@ -193,6 +193,9 @@ public function addBadgeScan(Summit $summit, Member $current_member, array $data
                     throw new ValidationException("Current member does not belong to the selected summit sponsor.");
             }
 
+            if(!$current_member->hasSponsorMembershipsFor($summit, $sponsor))
+                throw new ValidationException("Current member does not have badge scan permissions for the selected sponsor.");
+
             $scan = new SponsorBadgeScan();
             $scan->setScanDate($scan_date);
             $scan->setQRCode($qr_code);

app/Services/Model/Imp/SponsorUserSyncService.php

@@ -154,7 +154,23 @@ public function addSponsorUserToGroup(int $user_id, string $group_slug, int $spo
             }
 
             // Add permission entry to the Sponsor_Users JSON column for this sponsor-member pair.
-            $member->addSponsorPermission($sponsor_id, $group_slug);
+            // If the row does not exist yet (MQ ordering race: group event arrived before membership
+            // event), create it eagerly so the permission is never silently dropped.
+            if ($member->addSponsorPermission($sponsor_id, $group_slug) === 0) {
+                Log::warning(
+                    "SponsorUserSyncService::addSponsorUserToGroup no Sponsor_Users row found for " .
+                    "member {$member->getId()} / sponsor {$sponsor_id} — creating it eagerly");
+
+                $summit = $this->summit_repository->getById($summit_id);
+                if (!$summit instanceof Summit) {
+                    throw new EntityNotFoundException("Summit {$summit_id} not found");
+                }
+
+                $this->summit_sponsor_service->addSponsorUser($summit, $sponsor_id, $member->getId());
+
+                // Retry now that the row exists.
+                $member->addSponsorPermission($sponsor_id, $group_slug);
+            }
 
             // Add to global group only if not already a member.
             if (!$member->belongsToGroup($group_slug)) {

app/Services/Model/Imp/SummitSponsorService.php

@@ -394,7 +394,9 @@ public function addSponsorUser(Summit $summit, int $sponsor_id, int $member_id):
                 // due a member could be on 2 diff places at same time ...
                 // (StartA <= EndB)  and  (EndA >= StartB)
 
-                if ($current_summit_begin_date <= $former_summit_end_date && $current_summit_end_date >= $former_summit_begin_date) {
+                if ($summit->getId() != $former_summit->getId() &&
+                    $current_summit_begin_date <= $former_summit_end_date &&
+                    $current_summit_end_date >= $former_summit_begin_date) {
                     throw new ValidationException
                     (
                         sprintf

database/migrations/model/Version20260408102410.php

@@ -0,0 +1,60 @@
+<?php namespace Database\Migrations\Model;
+/**
+ * Copyright 2026 OpenStack Foundation
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ **/
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Class Version20260408120000
+ * @package Database\Migrations\Model
+ *
+ * Backfills the Permissions JSON column on Sponsor_Users with the group codes
+ * (Group.Code) of every group the member belongs to, derived from Group_Members.
+ * Only rows where Permissions IS NULL are updated (i.e. rows that pre-date the
+ * per-sponsor permission tracking feature added in Version20260402153110).
+ */
+final class Version20260408102410 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Backfill Sponsor_Users.Permissions JSON from existing Group_Members rows.';
+    }
+
+    public function up(Schema $schema): void
+    {
+        // For each Sponsor_Users row whose Permissions have not been set yet,
+        // aggregate all Group.Code values for the member and store them as a
+        // JSON array. JSON_ARRAYAGG returns NULL when there are no matching
+        // groups, which is a valid (empty-permissions) state.
+        $this->addSql(<<<SQL
+UPDATE Sponsor_Users su
+SET su.Permissions = (
+    SELECT JSON_ARRAYAGG(g.Code)
+    FROM Group_Members gm
+    INNER JOIN `Group` g ON g.ID = gm.GroupID
+    WHERE gm.MemberID = su.MemberID
+)
+WHERE su.Permissions IS NULL
+SQL
+        );
+    }
+
+    public function down(Schema $schema): void
+    {
+        // Revert the backfill by clearing all Permissions values. This is the
+        // only safe inverse: we cannot distinguish backfilled values from those
+        // written by the application after the migration ran.
+        $this->addSql("UPDATE Sponsor_Users SET Permissions = NULL");
+    }
+}