Release 1.30.4 (#3770)

* prevent empty invoice number * prevent broken invoice filename * fix recent timesheets for user * allow to upload twig invoice template
kimai · Jan 19, 2023 · 7ea3eb1 · 7ea3eb1
1 parent 559868f
commit 7ea3eb1
Show file tree

Hide file tree

Showing 37 changed files with 190 additions and 49 deletions.
diff --git a/src/API/TimesheetController.php b/src/API/TimesheetController.php
@@ -21,6 +21,7 @@
 use App\Repository\Query\TimesheetQuery;
 use App\Repository\TagRepository;
 use App\Repository\TimesheetRepository;
+use App\Repository\UserRepository;
 use App\Timesheet\TimesheetService;
 use App\Timesheet\TrackingMode\TrackingModeInterface;
 use App\Utils\SearchTerm;
@@ -75,19 +76,22 @@ class TimesheetController extends BaseApiController
      * @var TimesheetService
      */
     private $service;
+    private $userRepository;
 
     public function __construct(
         ViewHandlerInterface $viewHandler,
         TimesheetRepository $repository,
         TagRepository $tagRepository,
         EventDispatcherInterface $dispatcher,
-        TimesheetService $service
+        TimesheetService $service,
+        UserRepository $userRepository
     ) {
         $this->viewHandler = $viewHandler;
         $this->repository = $repository;
         $this->tagRepository = $tagRepository;
         $this->dispatcher = $dispatcher;
         $this->service = $service;
+        $this->userRepository = $userRepository;
     }
 
     protected function getTrackingMode(): TrackingModeInterface
@@ -491,9 +495,13 @@ public function recentAction(ParamFetcherInterface $paramFetcher): Response
 
         if ($this->isGranted('view_other_timesheet') && null !== ($reqUser = $paramFetcher->get('user'))) {
             if ('all' === $reqUser) {
-                $reqUser = null;
+                $user = null;
+            } else {
+                $user = $this->userRepository->getUserById($reqUser);
+                if ($user === null) {
+                    throw $this->createNotFoundException('Unknown User ID');
+                }
             }
-            $user = $reqUser;
         }
 
         if (null !== ($reqLimit = $paramFetcher->get('size'))) {

diff --git a/src/Constants.php b/src/Constants.php
@@ -17,11 +17,11 @@ class Constants
     /**
      * The current release version
      */
-    public const VERSION = '1.30.3';
+    public const VERSION = '1.30.4';
     /**
      * The current release: major * 10000 + minor * 100 + patch
      */
-    public const VERSION_ID = 13003;
+    public const VERSION_ID = 13004;
     /**
      * The current release status, either "stable" or "dev"
      */

diff --git a/src/Controller/InvoiceController.php b/src/Controller/InvoiceController.php
@@ -44,6 +44,7 @@
 use Symfony\Component\Security\Csrf\CsrfToken;
 use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
+use Twig\Environment;
 
 /**
  * Controller used to create invoices and manage invoice templates.
@@ -409,7 +410,7 @@ public function editTemplateAction(InvoiceTemplate $template, Request $request):
      * @Route(path="/document_upload", name="admin_invoice_document_upload", methods={"GET", "POST"})
      * @Security("is_granted('upload_invoice_template')")
      */
-    public function uploadDocumentAction(Request $request, string $projectDirectory, InvoiceDocumentRepository $documentRepository)
+    public function uploadDocumentAction(Request $request, string $projectDirectory, InvoiceDocumentRepository $documentRepository, Environment $twig, SystemConfiguration $systemConfiguration)
     {
         $dir = $documentRepository->getUploadDirectory();
         $invoiceDir = $dir;
@@ -418,6 +419,7 @@ public function uploadDocumentAction(Request $request, string $projectDirectory,
         if ($invoiceDir[0] !== '/') {
             $invoiceDir = $projectDirectory . DIRECTORY_SEPARATOR . $dir;
         }
+        $invoiceDir = rtrim($invoiceDir, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR;
 
         $used = [];
         foreach ($this->templateRepository->findAll() as $template) {
@@ -473,23 +475,56 @@ public function uploadDocumentAction(Request $request, string $projectDirectory,
                 /** @var UploadedFile $uploadedFile */
                 $uploadedFile = $form->get('document')->getData();
 
-                $originalFilename = pathinfo($uploadedFile->getClientOriginalName(), PATHINFO_FILENAME);
-                $safeFilename = transliterator_transliterate(
-                    'Any-Latin; Latin-ASCII; [^A-Za-z0-9_] remove; Lower()',
-                    $originalFilename
-                );
+                $originalName = $uploadedFile->getClientOriginalName();
+                $safeFilename = null;
+                $extension = null;
+                $success = true;
 
-                $extension = $uploadedFile->guessExtension();
+                $allowed = InvoiceDocumentUploadForm::EXTENSIONS_NO_TWIG;
+                if ((bool) $systemConfiguration->find('invoice.upload_twig') === true) {
+                    $allowed = InvoiceDocumentUploadForm::EXTENSIONS;
+                }
 
-                $newFilename = substr($safeFilename, 0, 20) . '.' . $extension;
+                foreach ($allowed as $ext) {
+                    $len = \strlen($ext);
+                    if (substr_compare($originalName, $ext, -$len) === 0) {
+                        $extension = $ext;
+                        $withoutExtension = str_replace($ext, '', $originalName);
+                        $safeFilename = transliterator_transliterate(InvoiceDocumentUploadForm::FILENAME_RULE, $withoutExtension);
+                        break;
+                    }
+                }
 
-                try {
-                    $uploadedFile->move($invoiceDir, $newFilename);
+                if ($safeFilename === null || $extension === null) {
+                    $success = false;
+                    $this->flashError('Invalid file given');
+                } else {
+                    $newFilename = substr($safeFilename, 0, 20) . $extension;
+
+                    try {
+                        $uploadedFile->move($invoiceDir, $newFilename);
+
+                        // if this is a twig file, we directly try to compile the template
+                        if (stripos($newFilename, '.twig') !== false) {
+                            try {
+                                $twig->enableAutoReload();
+                                $twig->load('@invoice/' . $newFilename);
+                                $twig->disableAutoReload();
+                            } catch (Exception $ex) {
+                                unlink($invoiceDir . $newFilename);
+                                $success = false;
+                                $this->flashException($ex, 'File was deleted, as Twig template is broken: ' . $ex->getMessage());
+                            }
+                        }
+                    } catch (Exception $ex) {
+                        $this->flashException($ex, 'action.upload.error');
+                    }
+                }
+
+                if ($success) {
                     $this->flashSuccess('action.update.success');
 
                     return $this->redirectToRoute('admin_invoice_document_upload');
-                } catch (Exception $ex) {
-                    $this->flashException($ex, 'action.upload.error');
                 }
             }
         }

diff --git a/src/Controller/SystemConfigurationController.php b/src/Controller/SystemConfigurationController.php
@@ -495,6 +495,7 @@ protected function getConfigurationTypes()
                         ->setLabel('invoice.number_format')
                         ->setRequired(true)
                         ->setType(TextType::class)
+                        ->setConstraints([new NotBlank()])
                         ->setTranslationDomain('system-configuration'),
                     (new Configuration())
                         ->setName('invoice.simple_form')

diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -333,6 +333,9 @@ private function getInvoiceNode(): ArrayNodeDefinition
                 ->scalarNode('number_format')
                     ->defaultValue('{Y}/{cy,3}')
                 ->end()
+                ->booleanNode('upload_twig')
+                    ->defaultTrue()
+                ->end()
             ->end()
         ;
 

diff --git a/src/Form/InvoiceDocumentUploadForm.php b/src/Form/InvoiceDocumentUploadForm.php
@@ -9,6 +9,7 @@
 
 namespace App\Form;
 
+use App\Configuration\SystemConfiguration;
 use App\Repository\InvoiceDocumentRepository;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\FileType;
@@ -21,34 +22,51 @@
 
 class InvoiceDocumentUploadForm extends AbstractType
 {
+    public const EXTENSIONS = ['.html.twig', '.pdf.twig', '.docx', '.xlsx', '.ods'];
+    public const EXTENSIONS_NO_TWIG = ['.docx', '.xlsx', '.ods'];
+    public const FILENAME_RULE = 'Any-Latin; Latin-ASCII; [^A-Za-z0-9_\-] remove; Lower()';
+
     private $repository;
+    private $systemConfiguration;
+    private $extensions = [];
 
-    public function __construct(InvoiceDocumentRepository $repository)
+    public function __construct(InvoiceDocumentRepository $repository, SystemConfiguration $systemConfiguration)
     {
         $this->repository = $repository;
+        $this->systemConfiguration = $systemConfiguration;
     }
 
     /**
      * {@inheritdoc}
      */
     public function buildForm(FormBuilderInterface $builder, array $options)
     {
+        $this->extensions = self::EXTENSIONS_NO_TWIG;
+        $extensions = 'DOCX, ODS, XLSX';
         $mimetypes = [
             'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
             'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
             'application/vnd.oasis.opendocument.spreadsheet',
         ];
 
+        if ((bool) $this->systemConfiguration->find('invoice.upload_twig') === true) {
+            $this->extensions = self::EXTENSIONS;
+            $extensions = 'DOCX, ODS, XLSX, TWIG (PDF & HTML)';
+            $mimetypes = array_merge($mimetypes, [
+                'application/octet-stream',  // needed for twig templates
+                'text/html', // needed for twig templates
+                'text/plain', // needed for twig templates
+            ]);
+        }
+
         $builder
             ->add('document', FileType::class, [
                 'label' => 'label.invoice_renderer',
                 'translation_domain' => 'invoice-renderer',
                 'help' => 'help.upload',
+                'help_translation_parameters' => ['%extensions%' => $extensions],
                 'mapped' => false,
                 'required' => true,
-                'attr' => [
-                    'accept' => implode(',', $mimetypes)
-                ],
                 'constraints' => [
                     new File([
                         'mimeTypes' => $mimetypes,
@@ -77,6 +95,48 @@ public function validateDocument($value, ExecutionContextInterface $context)
                 ->setTranslationDomain('validators')
                 ->setCode('kimai-invoice-document-upload-01')
                 ->addViolation();
+
+            return;
+        }
+
+        $extension = null;
+        $nameWithoutExtension = null;
+
+        foreach ($this->extensions as $ext) {
+            $len = \strlen($ext);
+            if (substr_compare($name, $ext, -$len) === 0) {
+                $extension = $ext;
+                $nameWithoutExtension = str_replace($ext, '', $name);
+                break;
+            }
+        }
+
+        if ($extension === null) {
+            $context->buildViolation('This invoice document cannot be used, allowed file extensions are: %extensions%')
+                ->setParameters(['%extensions%' => implode(', ', $this->extensions)])
+                ->setTranslationDomain('validators')
+                ->setCode('kimai-invoice-document-upload-02')
+                ->addViolation();
+
+            return;
+        }
+
+        $safeFilename = transliterator_transliterate(self::FILENAME_RULE, $nameWithoutExtension);
+
+        if ($safeFilename !== $nameWithoutExtension) {
+            $context->buildViolation('This invoice document cannot be used, filename may only contain the following ascii character: %character%')
+                ->setParameters(['%character%' => 'A-Z a-z 0-9 _ -'])
+                ->setTranslationDomain('validators')
+                ->setCode('kimai-invoice-document-upload-03')
+                ->addViolation();
+        }
+
+        if (mb_strlen($nameWithoutExtension) > 20) {
+            $context->buildViolation('This invoice document cannot be used, allowed filename length without extension is %character% character.')
+                ->setParameters(['%character%' => 20])
+                ->setTranslationDomain('validators')
+                ->setCode('kimai-invoice-document-upload-04')
+                ->addViolation();
         }
     }
 

diff --git a/src/Invoice/Renderer/PdfRenderer.php b/src/Invoice/Renderer/PdfRenderer.php
@@ -15,6 +15,7 @@
 use App\Export\ExportContext;
 use App\Invoice\InvoiceFilename;
 use App\Invoice\InvoiceModel;
+use App\Utils\FileHelper;
 use App\Utils\HtmlToPdfConverter;
 use Symfony\Component\HttpFoundation\Response;
 use Twig\Environment;
@@ -61,6 +62,8 @@ public function render(InvoiceDocument $document, InvoiceModel $model): Response
 
         $response = new Response($content);
 
+        $filename = FileHelper::convertToAsciiFilename($filename);
+
         $disposition = $response->headers->makeDisposition($this->getDisposition(), $filename . '.pdf');
 
         $response->headers->set('Content-Type', 'application/pdf');

diff --git a/tests/API/TimesheetControllerTest.php b/tests/API/TimesheetControllerTest.php
@@ -773,6 +773,36 @@ public function testGetRecentAction()
         self::assertApiResponseTypeStructure('TimesheetCollectionFull', $result[0]);
     }
 
+    public function testGetRecentActionForUser(): void
+    {
+        $client = $this->getClientForAuthenticatedUser(User::ROLE_TEAMLEAD);
+
+        $start = new \DateTime('-10 days');
+
+        $user = $this->getUserByRole(User::ROLE_ADMIN);
+        $fixture = new TimesheetFixtures();
+        $fixture->setFixedRate(true);
+        $fixture->setHourlyRate(true);
+        $fixture->setAmount(10);
+        $fixture->setUser($user);
+        $fixture->setStartDate($start);
+        $this->importFixture($fixture);
+
+        $query = [
+            'user' => $user->getId(),
+            'size' => 2,
+            'begin' => $start->format(self::DATE_FORMAT_HTML5),
+        ];
+
+        $this->assertAccessIsGranted($client, '/api/timesheets/recent', 'GET', $query);
+        $result = json_decode($client->getResponse()->getContent(), true);
+
+        $this->assertIsArray($result);
+        $this->assertNotEmpty($result);
+        $this->assertEquals(1, \count($result));
+        self::assertApiResponseTypeStructure('TimesheetCollectionFull', $result[0]);
+    }
+
     public function testActiveAction()
     {
         $client = $this->getClientForAuthenticatedUser(User::ROLE_USER);

diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php
@@ -325,6 +325,7 @@ public function testFullDefaultConfig()
                 ],
                 'simple_form' => false,
                 'number_format' => '{Y}/{cy,3}',
+                'upload_twig' => true,
             ],
             'export' => [
                 'documents' => [

diff --git a/translations/invoice-renderer.ar.xlf b/translations/invoice-renderer.ar.xlf
@@ -60,7 +60,7 @@
       </trans-unit>
       <trans-unit id="7mEUv6C" resname="help.upload">
         <source>help.upload</source>
-        <target state="translated">انتباه: سيتم الكتابة فوق الملفات الموجودة. أنواع الملفات المسموح بها هي: DOCX و ODS و XLSX.</target>
+        <target state="translated">انتباه: سيتم الكتابة فوق الملفات الموجودة. أنواع الملفات المسموح بها هي: %extensions%.</target>
       </trans-unit>
       <trans-unit id="E5BwZHT" resname="programmatic">
         <source>programmatic</source>

diff --git a/translations/invoice-renderer.cs.xlf b/translations/invoice-renderer.cs.xlf
@@ -36,7 +36,7 @@
       </trans-unit>
       <trans-unit id="7mEUv6C" resname="help.upload">
         <source>help.upload</source>
-        <target state="translated">Pozor: existující soubory budou přepsány. Povolené typy jsou: DOCX, ODS, XLSX.</target>
+        <target state="translated">Pozor: existující soubory budou přepsány. Povolené typy jsou: %extensions%.</target>
       </trans-unit>
       <trans-unit id="E5BwZHT" resname="programmatic">
         <source>programmatic</source>

diff --git a/translations/invoice-renderer.de.xlf b/translations/invoice-renderer.de.xlf
@@ -8,7 +8,7 @@
       </trans-unit>
       <trans-unit id="7mEUv6C" resname="help.upload">
         <source>help.upload</source>
-        <target state="translated">Achtung: existierende Dateien werden überschrieben. Erlaubte Dateitypen sind: DOCX, ODS, XLSX.</target>
+        <target state="translated">Achtung: existierende Dateien werden überschrieben. Erlaubte Dateitypen sind: %extensions%.</target>
       </trans-unit>
       <!-- Optgroups -->
       <trans-unit id="E5BwZHT" resname="programmatic">

diff --git a/translations/invoice-renderer.de_CH.xlf b/translations/invoice-renderer.de_CH.xlf
@@ -8,7 +8,7 @@
       </trans-unit>
       <trans-unit id="7mEUv6C" resname="help.upload">
         <source>help.upload</source>
-        <target state="needs-translation">Achtung: existierende Dateien werden überschrieben. Erlaubte Dateitypen sind: DOCX, ODS, XLSX</target>
+        <target state="needs-translation">Achtung: existierende Dateien werden überschrieben. Erlaubte Dateitypen sind: %extensions%</target>
       </trans-unit>
       <!-- Optgroups -->
       <trans-unit id="E5BwZHT" resname="programmatic">

diff --git a/translations/invoice-renderer.el.xlf b/translations/invoice-renderer.el.xlf
@@ -8,7 +8,7 @@
       </trans-unit>
       <trans-unit id="7mEUv6C" resname="help.upload">
         <source>help.upload</source>
-        <target state="translated">Προσοχή: τα υπάρχοντα αρχεία θα αντικατασταθούν. Επιτρεπόμενοι τύποι αρχείων είναι: DOCX, ODS, XLSX.</target>
+        <target state="translated">Προσοχή: τα υπάρχοντα αρχεία θα αντικατασταθούν. Επιτρεπόμενοι τύποι αρχείων είναι: %extensions%.</target>
       </trans-unit>
       <!-- Optgroups -->
       <trans-unit id="E5BwZHT" resname="programmatic">