implements InternalRequests management and security

.env

@@ -18,10 +18,6 @@ APP_DEBUG=0
 APP_SECRET=6a76497c8658bb23e2236f97a2627df3
 ###< symfony/framework-bundle ###
 
-###> files management ###
-INTERNAL_FILES_DOWNLOAD_URI=https://api.opentalent.fr/_internal/secure/files
-###< files management ###
-
 ###> doctrine/doctrine-bundle ###
 # => defined in the .env.<environment> files
 ###< doctrine/doctrine-bundle ###
@@ -107,3 +103,8 @@ CORS_ALLOW_ORIGIN='^https?://(localhost|127\.0\.0\.1)(:[0-9]+)?$'
 ###> filename log ###
 LOG_FILE_NAME=undefined
 ###< filename log ###
+
+### Internal requests (@see doc/internal_requests.md)
+INTERNAL_REQUESTS_TOKEN=sRyfu6SZLR9StpnSKYRdl6i9wr5qs1bJQzro4DUiVyYJ2jknl
+INTERNAL_FILES_DOWNLOAD_URI=https://api.opentalent.fr/_internal/secure/files
+###

config/packages/security.yaml

@@ -169,6 +169,8 @@ security:
     # Note: Only the *first* access control that matches will be used
     access_control:
         - { path: ^/api/public, roles: PUBLIC_ACCESS }
+        - { path: ^/api/internal, roles: INTERNAL_REQUEST }
+        - { path: ^/api/internal, roles: ROLE_NO_ACCESS }
         - { path: ^/api/, roles: IS_HAVING_MODULE }
 
 when@test:

config/services.yaml

@@ -46,6 +46,7 @@ services:
         bind:
             $opentalentConfig: '%kernel.project_dir%%env(OPENTALENT_CONFIG)%'
             $internalFilesUploadUri: '%env(INTERNAL_FILES_DOWNLOAD_URI)%'
+            $internalRequestsToken: '%env(INTERNAL_REQUESTS_TOKEN)%'
             $bindfileBufferFile: '%env(BIND_FILE_BUFFER_FILE)%'
             $persistProcessor: '@api_platform.doctrine.orm.state.persist_processor'
             $removeProcessor: '@api_platform.doctrine.orm.state.remove_processor'

doc/internal_requests.md

@@ -0,0 +1,59 @@
+# Internal Requests
+
+### Principe général
+
+Les requêtes internes sont des requêtes envoyées de ap2i vers opentalent-platform ou dans le sens inverse, par 
+exemple pour demander un fichier.
+
+Ces requêtes ne sont pas protégées par l'authentification Symfony standard, car elles doivent pouvoir être exécutées 
+en dehors du cadre d'une requête utilisateur, par exemple lors d'une exécution en ligne de commande ou lors 
+d'un processus asynchrone exécuté par messenger.
+
+Pour éviter tout risque de sécurité lié à ces routes :
+
+* on restreint leur accès aux ips internes
+* on conditionne l'autorisation à la présence d'un token
+* on limite les routes concernées
+
+Ainsi, si l'on prend l'exemple d'une requête `/internal/download/123` sur ap2i :
+
+* Un utilisateur dans le VPN qui ferait un curl à cette adresse recevra une erreur 500 à cause du token manquant
+* Un utilisateur hors VPN, même s'il connaissait le token, recevra une erreur 500, car n'ayant pas une ip autorisée
+* Une requête issue de la V1 sera autorisée sans authentification
+
+
+### Mise en oeuvre
+
+On met en place un pattern de routes de la forme `/api/internal/*` qui sera uniquement dédié aux requêtes internes entre
+les deux API ou à d'autres éventuels échanges entre systèmes.
+
+Les appels à cette route ne sont autorisés que si :
+
+1. Que l'ip du client dont émet la requête fait partie d'un pool autorisé d'ips internes
+2. Qu'un header 'internal-requests-token' est défini et que sa valeur correspond à la valeur attendue.
+
+Si ces deux conditions ne sont pas remplies, la requête est rejetée, et ce même si l'utilisateur est authentifié.
+
+
+### Valider le fonctionnement
+
+Soit `$id` l'id d'un fichier stocké sur l'environnement V2
+On part du principe que l'utilisateur authentifié a des droits suffisants pour voir ce fichier.
+
+
+Côté ap2i, les requêtes suivantes doivent donner les résultats correspondants :
+
+| query                      | header défini | authentifié | VPN activé | Résultat attendu |
+|----------------------------|---------------|-------------|------------|------------------|
+| /api/internal/download/$id | NON           | NON         | NON        | 401 Unauthorized |
+| /api/internal/download/$id | OUI           | NON         | NON        | 401 Unauthorized |
+| /api/internal/download/$id | OUI           | NON         | OUI        | 200 OK           |
+| /api/internal/download/$id | OUI           | OUI         | OUI        | 200 OK           |
+| /api/internal/download/$id | OUI           | OUI         | NON        | 403 Forbidden    |
+| /api/internal/download/$id | NON           | OUI         | OUI        | 403 Forbidden    |
+| /api/download/$id          | *             | NON         | *          | 401 Unauthorized |
+| /api/download/$id          | *             | OUI         | *          | 200 OK           |
+
+
+Les mêmes tests s'appliquent côté V1, appliqués à un fichier stocké sur l'environnement de la V1.
+

src/ApiResources/DownloadRequest.php → src/ApiResources/Core/File/DownloadRequest.php

@@ -1,12 +1,11 @@
 <?php
 declare (strict_types=1);
 
-namespace App\ApiResources;
+namespace App\ApiResources\Core\File;
 
-use ApiPlatform\Metadata\Link;
-use ApiPlatform\Metadata\Get;
-use ApiPlatform\Metadata\ApiResource;
 use ApiPlatform\Metadata\ApiProperty;
+use ApiPlatform\Metadata\ApiResource;
+use ApiPlatform\Metadata\Get;
 use App\State\Provider\Core\DownloadRequestProvider;
 
 /**
@@ -19,6 +18,11 @@ use App\State\Provider\Core\DownloadRequestProvider;
             requirements: ['fileId' => '\\d+'],
             security: 'is_granted("ROLE_FILE")',
             provider: DownloadRequestProvider::class
+        ),
+        new Get(
+            uriTemplate: '/internal/download/{fileId}',
+            requirements: ['fileId' => '\\d+'],
+            provider: DownloadRequestProvider::class
         )
     ]
 )]

src/Security/Voter/InternalRequestsVoter.php

@@ -0,0 +1,56 @@
+<?php
+declare(strict_types=1);
+
+namespace App\Security\Voter;
+
+use App\Service\Security\InternalRequestsService;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
+
+/**
+ * @package App\Security
+ */
+class InternalRequestsVoter extends Voter
+{
+    public function __construct(
+        private readonly InternalRequestsService $internalRequestsService,
+        protected RequestStack $requestStack,
+    )
+    {}
+
+    /**
+     * @param string $attribute
+     * @param mixed $subject
+     * @return bool
+     */
+    protected function supports(string $attribute, mixed $subject): bool
+    {
+        return $attribute === 'INTERNAL_REQUEST';
+    }
+
+    /**
+     * @param string $attribute
+     * @param mixed $subject
+     * @param TokenInterface $token
+     * @return bool
+     */
+    protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token): bool
+    {
+        return $this->isValidInternalRequest();
+    }
+
+    /**
+     * Is the current request a valid internal request ?
+     *
+     * @return bool
+     */
+    protected function isValidInternalRequest(): bool {
+        $request = $this->requestStack->getCurrentRequest();
+
+        $clientIp = $request->server->get('REMOTE_ADDR');
+        $internalRequestsToken = $request->headers->get('internal-requests-token')?? null;
+
+        return $internalRequestsToken && $this->internalRequestsService->isAllowed($clientIp, $internalRequestsToken);
+    }
+}

src/Service/File/FileManager.php

@@ -6,8 +6,7 @@ namespace App\Service\File;
 use ApiPlatform\Api\IriConverterInterface;
 use ApiPlatform\Api\UrlGeneratorInterface;
 use ApiPlatform\Metadata\Get;
-use App\ApiResources\DownloadRequest;
-use App\Entity\Access\Access;
+use App\ApiResources\Core\File\DownloadRequest;
 use App\Entity\Core\File;
 use App\Enum\Core\FileHostEnum;
 use App\Service\File\Exception\FileNotFoundException;
@@ -15,7 +14,6 @@ use App\Service\File\Storage\ApiLegacyStorage;
 use App\Service\File\Storage\FileStorageInterface;
 use App\Service\File\Storage\LocalStorage;
 use Mimey\MimeTypes;
-use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 
 /**
@@ -57,7 +55,7 @@ class FileManager
      * @return string
      * @throws FileNotFoundException
      */
-    public function read(File $file, ?TokenInterface $token=null): string
+    public function read(File $file): string
     {
         $storage = $this->getStorageFor($file);
         return $storage->read($file);

src/Service/Security/InternalRequestsService.php

@@ -0,0 +1,61 @@
+<?php
+
+namespace App\Service\Security;
+
+/**
+ * Identify and allow internal requests between api v1 and v2
+ *
+ * @see doc/internal_requests.md
+ */
+class InternalRequestsService
+{
+    // Internal ips allowed to access private files without being authenticated
+    const INTERNAL_IPS = [
+        '/^127\.0\.0\.[0-1]$/', // Localhost
+        '/^localhost$/',  // Localhost
+        '/^10\.8\.0\.\d{1,3}$/', // VPN
+        '/^141\.94\.117\.[33-61]$/',   // opentalent hosts public ips
+        '/^172\.20\.\d{1,3}\.\d{1,3}$/',  // docker
+    ];
+
+    public function __construct(
+        readonly private string $internalRequestsToken
+    ) {}
+
+    /**
+     * Returns true if the client Ip is allowed to access restricted content without auth
+     *
+     * @param string $clientIp
+     * @return bool
+     */
+    protected function isInternalIp(string $clientIp): bool
+    {
+        foreach (self::INTERNAL_IPS as $ipRule) {
+            if (preg_match($ipRule, $clientIp)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Compare the given token to the expected one, and return true if they are identical
+     *
+     * @param string $token
+     * @return bool
+     */
+    protected function tokenIsValid(string $token): bool {
+        return $token === $this->internalRequestsToken;
+    }
+
+    /**
+     * Is the given request a valid internal request, which shall be responded even without authentication
+     *
+     * @param string $ip
+     * @param string $token
+     * @return bool
+     */
+    public function isAllowed(string $ip, string $token): bool {
+        return $this->isInternalIp($ip) && $this->tokenIsValid($token);
+    }
+}

src/State/Provider/Core/DownloadRequestProvider.php

@@ -6,7 +6,7 @@ namespace App\State\Provider\Core;
 use ApiPlatform\Metadata\GetCollection;
 use ApiPlatform\Metadata\Operation;
 use ApiPlatform\State\ProviderInterface;
-use App\ApiResources\DownloadRequest;
+use App\ApiResources\Core\File\DownloadRequest;
 use App\Enum\Core\FileStatusEnum;
 use App\Repository\Core\FileRepository;
 use App\Service\File\Exception\FileNotFoundException;
@@ -15,33 +15,21 @@ use RuntimeException;
 use Symfony\Component\HttpFoundation\HeaderUtils;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Response;
-use Symfony\Bundle\SecurityBundle\Security;
 
 /**
  * Custom provider pour le téléchargement des fichiers du LocalStorage
  */
 final class DownloadRequestProvider implements ProviderInterface
 {
-    // Internal ips allowed to access private files without being authenticated
-    const INTERNAL_IPS = [
-        '/^127\.0\.0\.[0-1]$/',
-        '/^localhost$/',
-        '/^10\.8\.0\.\d{1,3}$/', // VPN
-        '/^141\.94\.117\.[33-61]$/',   // internal public ips
-        '/^172\.20\.\d{1,3}\.\d{1,3}$/',  // docker
-    ];
-
     public function __construct(
-        private FileRepository $fileRepository,
-        private FileManager $fileManager,
-        private Security $security,
-    )
-    {}
+        private readonly FileRepository $fileRepository,
+        private readonly FileManager    $fileManager,
+    ) {}
 
     /**
      * @param string $resourceClass
      * @param string|null $operationName
-     * @param mixed[] $context
+     * @param array $context
      * @return bool
      */
     public function supports(string $resourceClass, string $operationName = null, array $context = []): bool
@@ -49,26 +37,10 @@ final class DownloadRequestProvider implements ProviderInterface
         return DownloadRequest::class === $resourceClass;
     }
 
-    /**
-     * Returns true if the client Ip is allowed to access restricted content without auth
-     *
-     * @param string $clientIp
-     * @return bool
-     */
-    private function isInternalIp(string $clientIp): bool
-    {
-        foreach (self::INTERNAL_IPS as $ipRule) {
-            if (preg_match($ipRule, $clientIp)) {
-                return true;
-            }
-        }
-        return false;
-    }
-
     /**
      * @param Operation $operation
-     * @param mixed[] $uriVariables
-     * @param mixed[] $context
+     * @param array $uriVariables
+     * @param array $context
      * @return Response|RedirectResponse
      * @throws FileNotFoundException
      */
@@ -78,22 +50,27 @@ final class DownloadRequestProvider implements ProviderInterface
             throw new RuntimeException('not supported', 500);
         }
 
-        $id = $uriVariables['id'];
-        $file = $this->fileRepository->find($id);
+        $id = $uriVariables['fileId'];
+
+        return $this->serveFile($id);
+    }
+
+    /**
+     * @param int $fileId
+     * @return Response
+     * @throws FileNotFoundException
+     */
+    protected function serveFile(int $fileId): Response {
+        $file = $this->fileRepository->find($fileId);
+
         if (empty($file)) {
-            throw new RuntimeException("File " . $id . " does not exist; abort.");
+            throw new RuntimeException("File " . $fileId . " does not exist; abort.");
         }
         if ($file->getStatus() !== FileStatusEnum::READY()->getValue()) {
-            throw new RuntimeException("File " . $id . " has " . $file->getStatus() . " status; abort.");
+            throw new RuntimeException("File " . $fileId . " has " . $file->getStatus() . " status; abort.");
         }
 
-        // This is a request from an authorized IP
-        $clientIp = $_SERVER['REMOTE_ADDR'];
-        $internalIp = $this->isInternalIp($clientIp);
-
-        // Read the file
-        $token = $internalIp ? null : $this->security->getToken();
-        $content = $this->fileManager->read($file, $token);
+        $content = $this->fileManager->read($file);
 
         // Build the response and attach the file to it
         // @see https://symfony.com/doc/current/components/http_foundation.html#serving-files
@@ -113,4 +90,5 @@ final class DownloadRequestProvider implements ProviderInterface
 
         return $response;
     }
+
 }

tests/Service/File/FileManagerTest.php

@@ -5,7 +5,7 @@ namespace App\Tests\Service\File;
 use ApiPlatform\Api\IriConverterInterface;
 use ApiPlatform\Api\UrlGeneratorInterface;
 use ApiPlatform\Metadata\Get;
-use App\ApiResources\DownloadRequest;
+use App\ApiResources\Core\File\DownloadRequest;
 use App\Entity\Core\File;
 use App\Enum\Core\FileHostEnum;
 use App\Service\File\Exception\FileNotFoundException;