Merge pull request vtsykun#120 from vtsykun/feat/webhooks-engine

Moved webhooks payload render to new expr lang engine
okvpn · May 28, 2023 · 42e4f23 · 42e4f23
2 parents 38e666c + e710a8b
commit 42e4f23
Show file tree

Hide file tree

Showing 12 changed files with 145 additions and 124 deletions.
diff --git a/composer.json b/composer.json
@@ -46,6 +46,7 @@
         "nelmio/cors-bundle": "^2.2",
         "nelmio/security-bundle": "^3.0",
         "okvpn/cron-bundle": "^1.0",
+        "okvpn/expression-language": "^1.0",
         "oro/doctrine-extensions": "^2.0",
         "pagerfanta/core": "^3.7",
         "pagerfanta/doctrine-orm-adapter": "^3.7",

diff --git a/composer.lock b/composer.lock
diff --git a/config/services.yaml b/config/services.yaml
@@ -14,12 +14,13 @@ parameters:
     env(PUBLIC_ACCESS): false
 
     # twig sandbox
-    security_policy_tags: ['app', 'for', 'if', 'spaceless', 'set', 'do', 'apply', 'verbatim']
+    security_policy_tags: ['app', 'for', 'if', 'spaceless', 'set', 'do', 'apply', 'verbatim', 'return']
     security_policy_functions: ['attribute', 'cycle', 'date', 'max', 'min', 'random', 'range', 'constant']
     security_policy_methods: []
     security_policy_forbidden_classes:
         - 'Packeton\Entity\Job'
         - 'Packeton\Entity\SshCredentials'
+        - 'Packeton\Entity\ApiToken'
     security_policy_forbidden_properties:
         'Packeton\Entity\User': ['apiToken', 'githubToken', 'password', 'salt']
         'Packeton\Entity\Package': ['credentials']
@@ -199,12 +200,13 @@ services:
 
     Packeton\Webhook\Twig\PayloadRenderer:
         arguments:
-            - !tagged trusted_extension
-            -
-                autoescape: false
+            $extensions: !tagged trusted_extension
+            $options:
+                cache: '%kernel.cache_dir%/webhook'
+                clear_text_tokens: false
         calls:
             - [addExtension, ['@packeton.twig.webhook_sandbox']]
-    
+
     Packeton\Security\CheckLdapCredentialsListener:
         autoconfigure: false
 

diff --git a/src/Controller/ZipballController.php b/src/Controller/ZipballController.php
@@ -46,9 +46,9 @@ public function upload(Request $request): Response
     }
 
     #[Route('/archive/remove/{id}', name: 'archive_remove', methods: ["DELETE"])]
+    #[IsGranted('ROLE_MAINTAINER')]
     public function remove(#[Vars] Zipball $zip, Request $request): Response
     {
-        $this->denyAccessUnlessGranted('ROLE_MAINTAINER');
         if (!$this->isCsrfTokenValid('archive_upload', $request->get('token'))) {
             return new JsonResponse(['error' => 'Csrf token is not a valid'], 400);
         }

diff --git a/src/Form/Type/WebhookType.php b/src/Form/Type/WebhookType.php
@@ -30,7 +30,8 @@ public function __construct(
         private readonly TokenStorageInterface $tokenStorage,
         private readonly PayloadRenderer $renderer,
         private readonly HmacOpensslCrypter $crypter,
-    ) {}
+    ) {
+    }
 
     /**
      * {@inheritdoc}
@@ -173,8 +174,7 @@ public function checkPayload($value, ExecutionContextInterface $context): void
         }
 
         try {
-            $this->renderer->init();
-            $this->renderer->createTemplate($value);
+            $this->renderer->validateScript($value);
         } catch (\Throwable $exception) {
             $context->addViolation('This value is not a valid twig. ' . $exception->getMessage());
         }

diff --git a/src/Webhook/HookRequest.php b/src/Webhook/HookRequest.php
@@ -6,17 +6,12 @@
 
 class HookRequest implements \JsonSerializable
 {
-    private $method;
-    private $options;
-    private $url;
-    private $body;
-
-    public function __construct(string $url, string $method, array $options = [], $body = null)
-    {
-        $this->url =  $url;
-        $this->method = $method;
-        $this->options = $options;
-        $this->body = $body;
+    public function __construct(
+        protected string $url,
+        protected string $method,
+        protected array $options = [],
+        protected mixed $body = null
+    ) {
     }
 
     /**
@@ -52,7 +47,7 @@ public function getUrl(): string
     /**
      * @return null|mixed
      */
-    public function getBody()
+    public function getBody(): mixed
     {
         return $this->body;
     }

diff --git a/src/Webhook/HookRequestExecutor.php b/src/Webhook/HookRequestExecutor.php
@@ -53,13 +53,15 @@ public function executeWebhook(Webhook $webhook, array $variables = [], HttpClie
             unset($options['secrets']);
 
             if ($body = $request->getBody()) {
-                $options['body'] = $request->getBody();
-                try {
+                if (is_string($body)) {
                     if (is_array($json = @json_decode($body, true))) {
                         $options['json'] = $json;
-                        unset($options['body']);
+                    } else {
+                        $options['body'] = $body;
                     }
-                } catch (\Throwable) {}
+                } else {
+                    $options['json'] = $body;
+                }
             }
 
             try {

diff --git a/src/Webhook/HookTestAction.php b/src/Webhook/HookTestAction.php
@@ -107,10 +107,10 @@ public function runTest(Webhook $webhook, array $data)
         $client = null;
         if (($data['sendReal'] ?? false) !== true) {
             $callback = function () {
-                $responseTime = rand(0, 900000);
+                $responseTime = rand(50000, 250000);
                 usleep($responseTime);
                 return new MockResponse('true', [
-                    'total_time' => $responseTime/1000.0,
+                    'total_time' => $responseTime/1000000.0,
                     'response_headers' => [
                         'Content-type' => 'application/json',
                         'Pragma' => 'no-cache',

diff --git a/src/Webhook/RequestResolver.php b/src/Webhook/RequestResolver.php
@@ -16,17 +16,9 @@
 class RequestResolver implements ContextAwareInterface, LoggerAwareInterface
 {
     private $logger;
-    private $renderer;
-    private $storedPrefix;
 
-    /**
-     * @param PayloadRenderer $renderer
-     * @param string $rootDir
-     */
-    public function __construct(PayloadRenderer $renderer, string $rootDir = null)
+    public function __construct(private readonly PayloadRenderer $renderer)
     {
-        $this->renderer = $renderer;
-        $this->storedPrefix = $rootDir ? rtrim($rootDir, '/') . '/var/webhooks/' : null;
     }
 
     /**
@@ -35,42 +27,47 @@ public function __construct(PayloadRenderer $renderer, string $rootDir = null)
      *
      * @return HookRequest[]
      */
-    public function resolveHook(Webhook $webhook, array $context = [])
+    public function resolveHook(Webhook $webhook, array $context = []): array
     {
         return iterator_to_array($this->doResolveHook($webhook, $context));
     }
 
     /**
      * @param Webhook $webhook
      * @param array $context
+     *
      * @return \Generator|void
      */
-    private function doResolveHook(Webhook $webhook, array $context = [])
+    private function doResolveHook(Webhook $webhook, array $context = []): iterable
     {
-        $separator = '-------------' . sha1(random_bytes(10)) . '---------------';
         $context[PlaceholderExtension::VARIABLE_NAME] = $placeholder = new PlaceholderContext();
+        $this->renderer->setLogger($this->logger);
 
-        $this->renderer->init();
-        if (null !== $this->logger) {
-            $this->renderer->setLogger($this->logger);
-        }
-
+        $content = null;
         if ($payload = $webhook->getPayload()) {
-            if (preg_match('/^@\w+$/', trim($payload)) && null !== $this->storedPrefix) {
-                $filename = $this->storedPrefix . substr(trim($payload), 1) . '.twig';
-                if (@file_exists($filename)) {
-                    $payload = file_get_contents($filename);
+            $legacy = '';
+            $this->renderer->setLogHandler(static function ($result) use (&$legacy) {
+                if (is_string($result)) {
+                    $legacy .= $result;
                 }
-            }
-            $payload = (string) $this->renderer->createTemplate($payload)->render($context);
-            $content = $webhook->getUrl() . $separator . trim($payload);
-        } else {
-            $content = $webhook->getUrl() . $separator;
+            });
+
+            $result = $this->renderer->execute(trim($payload), $context);
+            $result = is_string($result) ? trim($result) : $result;
+
+            $content = $result === null ? trim($legacy) : $result;
         }
 
+        $content = [$webhook->getUrl(), $webhook->getOptions()['headers'] ?? null, $content === '' ? null : $content];
+
         foreach ($placeholder->walkContent($content) as $content) {
-            list($url, $content) = explode($separator, $content);
-            yield new HookRequest($url, $webhook->getMethod(), $webhook->getOptions() ?: [], $content ? trim($content) : null);
+            [$url, $headers, $content] = $content;
+            $options = $webhook->getOptions() ?: [];
+            if ($headers) {
+                $options['headers'] = $headers;
+            }
+
+            yield new HookRequest($url, $webhook->getMethod(), $options, $content);
         }
     }
 

diff --git a/src/Webhook/Twig/PayloadRenderer.php b/src/Webhook/Twig/PayloadRenderer.php
@@ -4,24 +4,12 @@
 
 namespace Packeton\Webhook\Twig;
 
+use Okvpn\Expression\TwigLanguage;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
-use Twig\Environment;
-use Twig\Loader\ArrayLoader;
 
-class PayloadRenderer extends Environment implements LoggerAwareInterface
+class PayloadRenderer extends TwigLanguage
 {
-    private $init = false;
-
-    public function __construct(private readonly iterable $extensions, $options = [])
-    {
-        $loader = new ArrayLoader();
-        parent::__construct($loader, $options);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
     public function setContext(WebhookContext $context = null): void
     {
         foreach ($this->extensions as $extension) {
@@ -31,28 +19,12 @@ public function setContext(WebhookContext $context = null): void
         }
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function setLogger(LoggerInterface $logger): void
+    public function setLogger(LoggerInterface $logger = null): void
     {
         foreach ($this->extensions as $extension) {
-            if ($extension instanceof LoggerAwareInterface) {
+            if (null !== $logger && $extension instanceof LoggerAwareInterface) {
                 $extension->setLogger($logger);
             }
         }
     }
-
-    public function init()
-    {
-        if ($this->init === true) {
-            return;
-        }
-
-        foreach ($this->extensions as $extension) {
-            $this->addExtension($extension);
-        }
-
-        $this->init = true;
-    }
 }