Преглед на файлове

fix: auto-revoke previous service tokens on rotation (SEC_REVIEW F13)

`ServiceTokenBootstrap` previously inserted a new service-kind row on
rotation but left every prior service-kind row valid forever — a
leaked old token (config snapshot, image layer) authenticated
indefinitely.

Bootstrap now wraps revoke-old + insert-new in
`Connection::transactional()` (per F4): on a `UI_SERVICE_TOKEN` value
that does not match any current row, every currently-active
service-kind row gets `revoked_at = now()` before the new row is
inserted. The revokes and the create roll back together if any step
fails — so there is never an observable window with no service token.

Each rotation emits `token.revoked`
(`details_json.reason = "rotated_by_bootstrap"`) per old row plus a
`token.created` (`source: "bootstrap"`, `rotated_from: [<prefix>...]`)
for the new row, both attributed to `actor_kind = system` via
`AuditContext::system()`. SOC tooling can split automatic from
operator-initiated revocations.

Bootstrap also refuses to silently re-enable a hash that is already
present-but-revoked: the operator must issue a fresh value rather
than rolling env back, closing the corner where a revoked token
gets re-activated by an env rollback.

Adds `TokenRepository::findActiveServiceTokens()` to enumerate the
rows to revoke.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa преди 5 дни
родител
ревизия
40be6c1875

+ 108 - 23
api/src/Infrastructure/Auth/ServiceTokenBootstrap.php

@@ -4,9 +4,14 @@ declare(strict_types=1);
 
 namespace App\Infrastructure\Auth;
 
+use App\Domain\Audit\AuditAction;
+use App\Domain\Audit\AuditContext;
+use App\Domain\Audit\AuditEmitter;
 use App\Domain\Auth\Token;
 use App\Domain\Auth\TokenHasher;
 use App\Domain\Auth\TokenKind;
+use App\Domain\Time\Clock;
+use Doctrine\DBAL\Connection;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -16,10 +21,13 @@ use Psr\Log\LoggerInterface;
  * - empty env → log a warning and skip (early-bring-up scenario).
  * - hash matches an existing service-kind row → no-op.
  * - hash absent → insert.
- * - hash absent but a *different* service-kind row exists → log warning
- *   (likely a rotation in progress), insert anyway. The operator must
- *   revoke the old hash via a future tooling change. Auto-revocation here
- *   would risk locking out a misconfigured deploy.
+ * - hash absent but a *different* service-kind row exists (rotation):
+ *   atomically revoke every previously-valid service-kind row and insert
+ *   the new one. Emits `token.revoked` (per old) and `token.created` (new)
+ *   audit rows attributed to `system` (SEC_REVIEW F13). The transaction
+ *   keeps "no service token exists" out of any observable state — if the
+ *   insert fails, the revokes roll back too, leaving the prior token
+ *   valid.
  */
 final class ServiceTokenBootstrap
 {
@@ -27,6 +35,9 @@ final class ServiceTokenBootstrap
         private readonly TokenRepository $tokens,
         private readonly TokenHasher $hasher,
         private readonly LoggerInterface $logger,
+        private readonly Connection $connection,
+        private readonly AuditEmitter $audit,
+        private readonly Clock $clock,
     ) {
     }
 
@@ -50,6 +61,18 @@ final class ServiceTokenBootstrap
 
         if ($existing !== null) {
             if ($existing->kind === TokenKind::Service) {
+                if ($existing->revokedAt !== null) {
+                    // Operator explicitly revoked this hash, then put it
+                    // back in env. Refuse to silently un-revoke; force the
+                    // operator to issue a fresh value.
+                    $this->logger->error(
+                        'UI_SERVICE_TOKEN matches a previously-revoked row; refusing to re-enable. '
+                        . 'Issue a fresh token value.'
+                    );
+
+                    return;
+                }
+
                 $this->logger->info('UI_SERVICE_TOKEN already provisioned; no-op');
 
                 return;
@@ -62,29 +85,91 @@ final class ServiceTokenBootstrap
             return;
         }
 
-        // No existing row by exact hash. Check if there is a *different*
-        // service-kind row already — typical during rotation.
-        $stmt = $this->tokens->countServiceTokens();
-        if ($stmt > 0) {
+        // No existing row by exact hash. SEC_REVIEW F13: if there are any
+        // currently-valid service-kind rows, we are in a rotation —
+        // revoke them all atomically with the new insert so the previously
+        // valid hash cannot authenticate after this bootstrap returns.
+        $previouslyActive = $this->tokens->findActiveServiceTokens();
+        $newPrefix = $parsed->prefix();
+
+        $auditCtx = AuditContext::system();
+        $now = $this->clock->now();
+
+        $this->connection->transactional(function () use ($previouslyActive, $hash, $newPrefix, $auditCtx, $now): void {
+            foreach ($previouslyActive as $old) {
+                if ($old->id === null) {
+                    // Repository hydrates persisted rows; id is always set.
+                    throw new \LogicException('persisted TokenRecord must have id');
+                }
+                $this->tokens->revoke($old->id, $now);
+                $this->audit->emitOrThrow(
+                    AuditAction::TOKEN_REVOKED,
+                    'token',
+                    $old->id,
+                    [
+                        'kind' => $old->kind->value,
+                        'prefix' => $old->prefix,
+                        'reason' => 'rotated_by_bootstrap',
+                    ],
+                    $auditCtx,
+                    self::tokenLabel($old->kind->value, $old->prefix),
+                );
+            }
+
+            $this->tokens->create(new TokenRecord(
+                id: null,
+                kind: TokenKind::Service,
+                hash: $hash,
+                prefix: $newPrefix,
+                reporterId: null,
+                consumerId: null,
+                role: null,
+                expiresAt: null,
+                revokedAt: null,
+                lastUsedAt: null,
+            ));
+
+            $created = $this->tokens->findByHashIncludingInvalid($hash);
+            if ($created === null) {
+                throw new \RuntimeException('service token not retrievable after insert');
+            }
+
+            $this->audit->emitOrThrow(
+                AuditAction::TOKEN_CREATED,
+                'token',
+                $created->id,
+                [
+                    'kind' => $created->kind->value,
+                    'prefix' => $created->prefix,
+                    'source' => 'bootstrap',
+                    'rotated_from' => array_map(static fn (TokenRecord $r): string => $r->prefix, $previouslyActive),
+                ],
+                $auditCtx,
+                self::tokenLabel($created->kind->value, $created->prefix),
+            );
+        });
+
+        if ($previouslyActive !== []) {
             $this->logger->warning(
-                'UI_SERVICE_TOKEN does not match the existing service-kind row(s); '
-                . 'inserting new service token. Operator must revoke the old hash manually.'
+                'UI_SERVICE_TOKEN rotated; revoked previously-valid service token(s) and inserted new one',
+                [
+                    'revoked_count' => count($previouslyActive),
+                    'revoked_prefixes' => array_map(
+                        static fn (TokenRecord $r): string => $r->prefix,
+                        $previouslyActive,
+                    ),
+                    'new_prefix' => $newPrefix,
+                ],
             );
-        }
 
-        $this->tokens->create(new TokenRecord(
-            id: null,
-            kind: TokenKind::Service,
-            hash: $hash,
-            prefix: $parsed->prefix(),
-            reporterId: null,
-            consumerId: null,
-            role: null,
-            expiresAt: null,
-            revokedAt: null,
-            lastUsedAt: null,
-        ));
+            return;
+        }
 
         $this->logger->info('UI_SERVICE_TOKEN provisioned');
     }
+
+    private static function tokenLabel(string $kind, string $prefix): string
+    {
+        return $kind . ':' . $prefix;
+    }
 }

+ 27 - 0
api/src/Infrastructure/Auth/TokenRepository.php

@@ -167,6 +167,33 @@ final class TokenRepository
         return (int) $value;
     }
 
+    /**
+     * Returns every currently-valid (non-revoked, non-expired) service-kind
+     * row. ServiceTokenBootstrap uses this on rotation to revoke each one
+     * after the new token has been inserted (SEC_REVIEW F13).
+     *
+     * @return list<TokenRecord>
+     */
+    public function findActiveServiceTokens(): array
+    {
+        $now = (new DateTimeImmutable('now', new DateTimeZone('UTC')))->format('Y-m-d H:i:s');
+
+        $sql = 'SELECT id, kind, token_hash, token_prefix, reporter_id, consumer_id, role, '
+            . 'expires_at, revoked_at, last_used_at FROM api_tokens '
+            . 'WHERE kind = :kind '
+            . 'AND revoked_at IS NULL '
+            . 'AND (expires_at IS NULL OR expires_at > :now) '
+            . 'ORDER BY id ASC';
+
+        /** @var list<array<string, mixed>> $rows */
+        $rows = $this->connection->fetchAllAssociative($sql, [
+            'kind' => TokenKind::Service->value,
+            'now' => $now,
+        ]);
+
+        return array_map(fn (array $r): TokenRecord => $this->hydrate($r), $rows);
+    }
+
     /**
      * @param array<string, mixed> $row
      */

+ 168 - 2
api/tests/Integration/Auth/ServiceTokenBootstrapTest.php

@@ -71,7 +71,14 @@ final class ServiceTokenBootstrapTest extends AppTestCase
         );
     }
 
-    public function testBootstrapWithDifferentTokenInsertsNewRow(): void
+    /**
+     * SEC_REVIEW F13. The previous implementation left the old service-kind
+     * row valid forever (the operator was expected to revoke it manually).
+     * Bootstrap now revokes any previously-active service-kind row when a
+     * new value is provisioned, so a leaked old token cannot authenticate
+     * after the next bootstrap of a fresh value.
+     */
+    public function testBootstrapWithDifferentTokenRevokesPreviousAndInsertsNewRow(): void
     {
         /** @var ServiceTokenBootstrap $boot */
         $boot = $this->container->get(ServiceTokenBootstrap::class);
@@ -84,10 +91,169 @@ final class ServiceTokenBootstrapTest extends AppTestCase
         $boot->bootstrap($first);
         $boot->bootstrap($second);
 
-        // Both rows should now exist; operator must revoke the old one manually.
+        // Both rows are kept (audit + traceability), but only the second is active.
         self::assertSame(
             2,
             (int) $this->db->fetchOne("SELECT COUNT(*) FROM api_tokens WHERE kind = 'service'")
         );
+        self::assertSame(
+            1,
+            (int) $this->db->fetchOne(
+                "SELECT COUNT(*) FROM api_tokens WHERE kind = 'service' AND revoked_at IS NOT NULL"
+            ),
+            'old service token must be marked revoked after rotation'
+        );
+        self::assertSame(
+            1,
+            (int) $this->db->fetchOne(
+                "SELECT COUNT(*) FROM api_tokens WHERE kind = 'service' AND revoked_at IS NULL"
+            ),
+            'exactly one service token must remain active after rotation'
+        );
+    }
+
+    /**
+     * SEC_REVIEW F13. If the operator has accumulated multiple
+     * service-kind rows (e.g. ran rotations on a pre-fix deploy),
+     * a fresh bootstrap revokes ALL previously-valid service-kind rows,
+     * not just the most recent one.
+     */
+    public function testBootstrapRotationRevokesEveryPreviouslyActiveServiceToken(): void
+    {
+        /** @var ServiceTokenBootstrap $boot */
+        $boot = $this->container->get(ServiceTokenBootstrap::class);
+        /** @var TokenIssuer $issuer */
+        $issuer = $this->container->get(TokenIssuer::class);
+
+        // Simulate a pre-fix history with three accumulated active service rows.
+        /** @var \App\Domain\Auth\TokenHasher $hasher */
+        $hasher = $this->container->get(\App\Domain\Auth\TokenHasher::class);
+        foreach (range(1, 3) as $_) {
+            $raw = $issuer->issue(TokenKind::Service);
+            $this->db->insert('api_tokens', [
+                'token_hash' => $hasher->hash($raw),
+                'token_prefix' => substr($raw, 0, 8),
+                'kind' => 'service',
+                'reporter_id' => null,
+                'consumer_id' => null,
+                'role' => null,
+                'expires_at' => null,
+                'revoked_at' => null,
+                'last_used_at' => null,
+            ]);
+        }
+        self::assertSame(
+            3,
+            (int) $this->db->fetchOne(
+                "SELECT COUNT(*) FROM api_tokens WHERE kind = 'service' AND revoked_at IS NULL"
+            )
+        );
+
+        $fresh = $issuer->issue(TokenKind::Service);
+        $boot->bootstrap($fresh);
+
+        self::assertSame(
+            3,
+            (int) $this->db->fetchOne(
+                "SELECT COUNT(*) FROM api_tokens WHERE kind = 'service' AND revoked_at IS NOT NULL"
+            ),
+            'all three pre-existing service tokens must be revoked'
+        );
+        self::assertSame(
+            1,
+            (int) $this->db->fetchOne(
+                "SELECT COUNT(*) FROM api_tokens WHERE kind = 'service' AND revoked_at IS NULL"
+            ),
+            'only the new service token must remain active'
+        );
+    }
+
+    /**
+     * SEC_REVIEW F13. Rotation must surface in the audit log so SOC tooling
+     * can attribute the change-of-state. The `token.revoked` row carries
+     * a structured `reason: rotated_by_bootstrap` so a query can split
+     * automatic-rotation from operator-initiated revoke.
+     */
+    public function testBootstrapRotationEmitsRevokedAndCreatedAuditRows(): void
+    {
+        /** @var ServiceTokenBootstrap $boot */
+        $boot = $this->container->get(ServiceTokenBootstrap::class);
+        /** @var TokenIssuer $issuer */
+        $issuer = $this->container->get(TokenIssuer::class);
+
+        $first = $issuer->issue(TokenKind::Service);
+        $second = $issuer->issue(TokenKind::Service);
+
+        $boot->bootstrap($first);
+        $this->db->executeStatement("DELETE FROM audit_log");
+
+        $boot->bootstrap($second);
+
+        $revokedDetails = $this->db->fetchOne(
+            "SELECT details_json FROM audit_log WHERE action = 'token.revoked' ORDER BY id DESC LIMIT 1"
+        );
+        self::assertIsString($revokedDetails);
+        $details = json_decode($revokedDetails, true);
+        self::assertIsArray($details);
+        self::assertSame('service', $details['kind']);
+        self::assertSame('rotated_by_bootstrap', $details['reason']);
+
+        $createdRow = $this->db->fetchAssociative(
+            "SELECT actor_kind, details_json FROM audit_log WHERE action = 'token.created' ORDER BY id DESC LIMIT 1"
+        );
+        self::assertIsArray($createdRow);
+        self::assertSame('system', $createdRow['actor_kind']);
+        $createdDetails = json_decode((string) $createdRow['details_json'], true);
+        self::assertIsArray($createdDetails);
+        self::assertSame('service', $createdDetails['kind']);
+        self::assertSame('bootstrap', $createdDetails['source']);
+        self::assertIsArray($createdDetails['rotated_from']);
+        self::assertCount(1, $createdDetails['rotated_from']);
+    }
+
+    /**
+     * SEC_REVIEW F13 corner case. If the operator put the old token back
+     * into env after explicitly revoking it (intentionally or via env-var
+     * rollback), bootstrap must NOT silently re-enable a known-bad hash.
+     * It refuses; the operator must issue a fresh value.
+     */
+    public function testBootstrapRefusesToReEnablePreviouslyRevokedToken(): void
+    {
+        /** @var ServiceTokenBootstrap $boot */
+        $boot = $this->container->get(ServiceTokenBootstrap::class);
+        /** @var TokenIssuer $issuer */
+        $issuer = $this->container->get(TokenIssuer::class);
+        /** @var \App\Domain\Auth\TokenHasher $hasher */
+        $hasher = $this->container->get(\App\Domain\Auth\TokenHasher::class);
+
+        $raw = $issuer->issue(TokenKind::Service);
+        $this->db->insert('api_tokens', [
+            'token_hash' => $hasher->hash($raw),
+            'token_prefix' => substr($raw, 0, 8),
+            'kind' => 'service',
+            'reporter_id' => null,
+            'consumer_id' => null,
+            'role' => null,
+            'expires_at' => null,
+            'revoked_at' => '2026-01-01 00:00:00',
+            'last_used_at' => null,
+        ]);
+
+        $boot->bootstrap($raw);
+
+        self::assertSame(
+            1,
+            (int) $this->db->fetchOne(
+                "SELECT COUNT(*) FROM api_tokens WHERE kind = 'service' AND revoked_at IS NOT NULL"
+            ),
+            'revoked row stays revoked'
+        );
+        self::assertSame(
+            0,
+            (int) $this->db->fetchOne(
+                "SELECT COUNT(*) FROM api_tokens WHERE kind = 'service' AND revoked_at IS NULL"
+            ),
+            'no fresh active row inserted from a revoked-hash env value'
+        );
     }
 }