feat: identify accounts by a uuid in the oauth2 sub claim

docs/api-consumer-guide.md

@@ -380,7 +380,7 @@ Responses:
 
 OIDC-style claims for the authenticated user. The response contains only the claims covered by the granted scopes:
 
-- always: `sub` (string), `email`
+- always: `sub` (UUID string, see below), `email`
 - scope `id`: `person_id`, `address_id`
 - scope `roles`: `roles` (array; only the roles that actually grant something are exported — the implicit baseline role is stripped, and so are the `*_PENDING` markers of roles the BusPro CRM claims but nobody has approved yet, see `docs/buspronet-schema/crm-selection-queries.md#from-claim-to-role`)
 - scope `profile`: `profile` object:
@@ -407,8 +407,10 @@ All of those accounts report the **same** `person_id` and `address_id`, and the
 `profile.communication.email` — that value is the first contact address on the BusPro record and
 is **not necessarily** the one signed in with. So:
 
-- **`sub`** — opaque, stable, unique per MyE&P account. The only claim that identifies an account.
+- **`sub`** — a **UUID (v4, RFC 4122, 36 characters)**, stable for the life of the account and
-  Match your local user on it and store it.
+  unique across accounts. The only claim that identifies an account. Match your local user on it
+  and store it. Size the column at 36 characters and treat the value as opaque: it is random, it
+  carries no timestamp and no ordering, and nothing about it should be parsed or derived from.
 - **`email`** — the address this session actually authenticated with. Distinct per account, but
   treat it as a display and contact value; it is not the account key.
 - **`person_id` / `address_id`** — the BusPro person behind the account. Shared between that

migrations/Version20260923150000.php

@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+use Symfony\Component\Uid\Uuid;
+
+/**
+ * Gives every account a uuid, which replaces the primary key as the OAuth2 `sub` claim.
+ *
+ * The UUIDs are generated here in PHP rather than with MariaDB's UUID(): that function
+ * returns v1, which encodes a timestamp and the server's node id, so a whole backfill
+ * would come out time-ordered and share a suffix. Guessing one account's identifier from
+ * another's is the property this column exists to remove, so the backfill has to be as
+ * random as the ones App\Entity\User generates from now on.
+ *
+ * One statement per row, which is why it is emitted rather than written out: the values
+ * cannot exist before the migration runs. Added nullable and tightened afterwards, since
+ * there is no single default that could satisfy a unique column.
+ */
+final class Version20260923150000 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Adds user.uuid and backfills it; the OAuth2 sub claim moves off the primary key.';
+    }
+
+    public function up(Schema $schema): void
+    {
+        $this->addSql('ALTER TABLE user ADD uuid VARCHAR(36) DEFAULT NULL');
+
+        foreach ($this->connection->fetchFirstColumn('SELECT id FROM user') as $id) {
+            $this->addSql('UPDATE user SET uuid = :uuid WHERE id = :id', [
+                'uuid' => Uuid::v4()->toRfc4122(),
+                'id' => $id,
+            ]);
+        }
+
+        $this->addSql('ALTER TABLE user MODIFY uuid VARCHAR(36) NOT NULL');
+        $this->addSql('CREATE UNIQUE INDEX UNIQ_8D93D649D17F50A6 ON user (uuid)');
+    }
+
+    public function down(Schema $schema): void
+    {
+        $this->addSql('DROP INDEX UNIQ_8D93D649D17F50A6 ON user');
+        $this->addSql('ALTER TABLE user DROP uuid');
+    }
+}

src/Controller/Api/UserinfoController.php

@@ -58,7 +58,11 @@ class UserinfoController extends AbstractController
             // addresses on a person's record as a login and answers all of them with the same
             // ids and the same first contact address, so only the local account tells the staff
             // account and the private one apart — and they hold different roles.
-            $data->subject = (string) $user->getId();
+            //
+            // The uuid, not the primary key: consumers store this value and match their own
+            // accounts on it, so it must not disclose how many accounts exist or let one
+            // account's identifier be guessed from another's.
+            $data->subject = $user->getUuid();
             $data->loginEmail = $user->getEmail();
 
             // Patch current user's roles. The implicit ROLE_USER says nothing about the

src/Entity/User.php

@@ -6,6 +6,7 @@ use App\Repository\UserRepository;
 use Doctrine\ORM\Mapping as ORM;
 use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
 use Symfony\Component\Security\Core\User\UserInterface;
+use Symfony\Component\Uid\Uuid;
 
 #[ORM\Entity(repositoryClass: UserRepository::class)]
 class User implements UserInterface, PasswordAuthenticatedUserInterface
@@ -15,6 +16,22 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
     #[ORM\Column(type: 'integer')]
     private ?int $id = null;
 
+    /**
+     * The account's external identifier, exported as the OAuth2 `sub` claim.
+     *
+     * Separate from the primary key on purpose: `sub` is handed to other applications, which
+     * store it and match their own accounts on it, so it has to be stable, unique and free of
+     * anything the row's position in the table implies. The auto-increment id satisfied the
+     * first two and neither of the last: it disclosed how many accounts exist and made one
+     * account's identifier a guess away from the next.
+     *
+     * v4 rather than the framework's default v7 (see config/packages/uid.yaml): v7 embeds its
+     * creation timestamp and sorts, which is useful for a key and is exactly the leak this
+     * column exists to avoid. Matches Groups\AccommodationBooking::$uuid in shape and version.
+     */
+    #[ORM\Column(type: 'string', length: 36, unique: true)]
+    private string $uuid;
+
     #[ORM\Column(type: 'string', length: 180, unique: true)]
     private ?string $email;
 
@@ -54,6 +71,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
     public function __construct(string $email)
     {
         $this->email = $email;
+        $this->uuid = Uuid::v4()->toRfc4122();
     }
 
     public function getId(): ?int
@@ -61,6 +79,11 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
         return $this->id;
     }
 
+    public function getUuid(): string
+    {
+        return $this->uuid;
+    }
+
     public function getEmail(): ?string
     {
         return $this->email;

tests/Controller/Api/UserinfoControllerTest.php

@@ -27,13 +27,14 @@ class UserinfoControllerTest extends TestCase
 {
     private const LOGIN_EMAIL = '[email protected]';
     private const CONTACT_EMAIL = '[email protected]';
+    private const UUID = '018f3c2a-7b4d-4e91-a3c5-6d2f8b1e4a07';
 
     public function testSubAndEmailAreExportedWithoutAnyOptionalScope(): void
     {
         $claims = $this->claims();
 
         self::assertSame(['sub', 'email'], array_keys($claims));
-        self::assertSame('42', $claims['sub']);
+        self::assertSame(self::UUID, $claims['sub']);
         self::assertSame(self::LOGIN_EMAIL, $claims['email']);
     }
 
@@ -43,11 +44,27 @@ class UserinfoControllerTest extends TestCase
 
         // The ids identify the human, the subject identifies the account. A second account of the
         // same person reports these same two ids and must still be told apart.
-        self::assertSame('42', $claims['sub']);
+        self::assertSame(self::UUID, $claims['sub']);
         self::assertSame(7, $claims['person_id']);
         self::assertSame(9, $claims['address_id']);
     }
 
+    /**
+     * Consumers store `sub` and match their own accounts on it, so it must not disclose how many
+     * accounts exist or let one account's identifier be guessed from the next. The primary key
+     * did both.
+     */
+    public function testSubIsTheAccountUuidRatherThanItsPrimaryKey(): void
+    {
+        $claims = $this->claims();
+
+        self::assertNotSame('42', $claims['sub']);
+        self::assertMatchesRegularExpression(
+            '/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/',
+            $claims['sub']
+        );
+    }
+
     public function testEmailIsTheAuthenticatedAddressAndNotTheFirstContactOnTheBusProRecord(): void
     {
         $claims = $this->claims(['ROLE_OAUTH2_PROFILE']);
@@ -160,10 +177,15 @@ class UserinfoControllerTest extends TestCase
             ->setRoles($roles)
         ;
 
-        // The id is generated by Doctrine and has no setter, but it is what `sub` exports.
+        // The id is generated by Doctrine and has no setter. It is deliberately not what `sub`
+        // exports -- see testSubIsTheAccountUuidRatherThanItsPrimaryKey.
         $property = new \ReflectionProperty(User::class, 'id');
         $property->setValue($user, 42);
 
+        // The constructor generates a fresh uuid; pin it so the claim can be asserted literally.
+        $property = new \ReflectionProperty(User::class, 'uuid');
+        $property->setValue($user, self::UUID);
+
         return $user;
     }
 

tests/Entity/UserTest.php

@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Entity;
+
+use App\Entity\User;
+use PHPUnit\Framework\TestCase;
+
+class UserTest extends TestCase
+{
+    /**
+     * The uuid is what /api/userinfo exports as `sub`, and consumers key their own accounts on
+     * it, so an account has to carry one from the moment it exists -- there is no later step
+     * that could assign it.
+     */
+    public function testAnAccountIsGivenAUuidOnCreation(): void
+    {
+        $user = new User('[email protected]');
+
+        self::assertMatchesRegularExpression(
+            '/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/',
+            $user->getUuid()
+        );
+    }
+
+    /**
+     * v4, not the framework's default v7: a v7 would sort by creation time and carry that
+     * timestamp in the claim, which is the disclosure the column exists to avoid.
+     */
+    public function testUuidsAreRandomRatherThanOrdered(): void
+    {
+        $uuids = [];
+        for ($i = 0; $i < 50; ++$i) {
+            $uuids[] = (new User("someone{$i}@ep-reisen.de"))->getUuid();
+        }
+
+        self::assertCount(50, array_unique($uuids));
+
+        $sorted = $uuids;
+        sort($sorted);
+        self::assertNotSame($sorted, $uuids, 'v4 uuids must not come out in creation order');
+    }
+}