Moved attachment form submit logic to a seperate service.

config/services.yaml

@@ -12,8 +12,10 @@ parameters:
     use_gravatar: true          # Set to false, if no Gravatar images should be used for user profiles.
     default_currency: 'EUR'     # The currency that should be used
     media_directory: 'public/media/' # The folder where uploaded attachment files are saved
+    secure_media_directory: 'media/' # The folder where secured attachment files are saved (must not be in public/)
     db_version_fallback: '5.6'  # Be sure to override this, in your .env with your real DB version
     global_theme: ''            # The theme to use globally (see public/build/themes/ for choices). Set to '' for default bootstrap theme
+    allow_attachments_downloads: true # Allow users to download attachments to server
 
 services:
     # default configuration for services in *this* file
@@ -75,6 +77,15 @@ services:
       arguments:
         $base_currency: '%default_currency%'
 
+    App\Form\AttachmentFormType:
+        arguments:
+            $allow_attachments_downloads: '%allow_attachments_downloads%'
+
+    App\Services\Attachments\AttachmentSubmitHandler:
+        arguments:
+            $allow_attachments_downloads: '%allow_attachments_downloads%'
+
+
     App\EventSubscriber\TimezoneListener:
         arguments:
             $timezone: '%timezone%'
@@ -83,6 +94,7 @@ services:
         arguments:
             $project_dir: '%kernel.project_dir%'
             $media_path: '%media_directory%'
+            $secure_path: '%secure_media_directory%'
             $footprints_path: 'public/img/footprints'
             $models_path: null
 

src/Controller/AdminPages/BaseAdminController.php

@@ -37,6 +37,7 @@ use App\Entity\UserSystem\User;
 use App\Form\AdminPages\ImportType;
 use App\Form\AdminPages\MassCreationForm;
 use App\Services\AttachmentHelper;
+use App\Services\Attachments\AttachmentSubmitHandler;
 use App\Services\EntityExporter;
 use App\Services\EntityImporter;
 use App\Services\StructuralElementRecursionHelper;
@@ -64,8 +65,10 @@ abstract class BaseAdminController extends AbstractController
     protected $passwordEncoder;
     protected $translator;
     protected $attachmentHelper;
+    protected $attachmentSubmitHandler;
 
-    public function __construct(TranslatorInterface $translator, UserPasswordEncoderInterface $passwordEncoder, AttachmentHelper $attachmentHelper)
+    public function __construct(TranslatorInterface $translator, UserPasswordEncoderInterface $passwordEncoder,
+                                AttachmentHelper $attachmentHelper, AttachmentSubmitHandler $attachmentSubmitHandler)
     {
         if ($this->entity_class === '' || $this->form_class === '' || $this->twig_template === '' || $this->route_base === '') {
             throw new \InvalidArgumentException('You have to override the $entity_class, $form_class, $route_base and $twig_template value in your subclasss!');
@@ -78,6 +81,7 @@ abstract class BaseAdminController extends AbstractController
         $this->translator = $translator;
         $this->passwordEncoder = $passwordEncoder;
         $this->attachmentHelper = $attachmentHelper;
+        $this->attachmentSubmitHandler = $attachmentSubmitHandler;
     }
 
     protected function _edit(NamedDBElement $entity, Request $request, EntityManagerInterface $em)
@@ -101,7 +105,7 @@ abstract class BaseAdminController extends AbstractController
             $attachments = $form['attachments'];
             foreach ($attachments as $attachment) {
                 /** @var $attachment FormInterface */
-                $this->attachmentHelper->upload( $attachment->getData(), $attachment['file']->getData());
+                $this->attachmentSubmitHandler->handleFormSubmit($attachment->getData(), $attachment['file']->getData());
             }
 
             $em->persist($entity);
@@ -146,7 +150,7 @@ abstract class BaseAdminController extends AbstractController
             $attachments = $form['attachments'];
             foreach ($attachments as $attachment) {
                 /** @var $attachment FormInterface */
-                $this->attachmentHelper->upload( $attachment->getData(), $attachment['file']->getData());
+                $this->attachmentSubmitHandler->handleFormSubmit($attachment->getData(), $attachment['file']->getData());
             }
 
             $em->persist($new_entity);

src/Controller/PartController.php

@@ -34,6 +34,7 @@ use App\Entity\Parts\Category;
 use App\Entity\Parts\Part;
 use App\Form\Part\PartBaseType;
 use App\Services\AttachmentHelper;
+use App\Services\Attachments\AttachmentSubmitHandler;
 use App\Services\Attachments\PartPreviewGenerator;
 use App\Services\PricedetailHelper;
 use Doctrine\ORM\EntityManagerInterface;
@@ -81,7 +82,7 @@ class PartController extends AbstractController
      * @return \Symfony\Component\HttpFoundation\Response
      */
     public function edit(Part $part, Request $request, EntityManagerInterface $em, TranslatorInterface $translator,
-                         AttachmentHelper $attachmentHelper)
+                         AttachmentHelper $attachmentHelper, AttachmentSubmitHandler $attachmentSubmitHandler)
     {
         $this->denyAccessUnlessGranted('edit', $part);
 
@@ -93,7 +94,7 @@ class PartController extends AbstractController
             $attachments = $form['attachments'];
             foreach ($attachments as $attachment) {
                 /** @var $attachment FormInterface */
-                $attachmentHelper->upload( $attachment->getData(), $attachment['file']->getData());
+                $attachmentSubmitHandler->handleFormSubmit($attachment->getData(), $attachment['file']->getData());
             }
 
 
@@ -148,7 +149,7 @@ class PartController extends AbstractController
      * @return \Symfony\Component\HttpFoundation\Response
      */
     public function new(Request $request, EntityManagerInterface $em, TranslatorInterface $translator,
-                        AttachmentHelper $attachmentHelper)
+                        AttachmentHelper $attachmentHelper, AttachmentSubmitHandler $attachmentSubmitHandler)
     {
         $new_part = new Part();
 
@@ -168,7 +169,7 @@ class PartController extends AbstractController
             $attachments = $form['attachments'];
             foreach ($attachments as $attachment) {
                 /** @var $attachment FormInterface */
-                $attachmentHelper->upload( $attachment->getData(), $attachment['file']->getData());
+                $attachmentSubmitHandler->handleFormSubmit($attachment->getData(), $attachment['file']->getData());
             }
 
             $em->persist($new_part);

src/Entity/Attachments/Attachment.php

@@ -146,6 +146,11 @@ abstract class Attachment extends NamedDBElement
      */
     public function isExternal() : bool
     {
+        //When path is empty, this attachment can not be external
+        if (empty($this->path)) {
+            return false;
+        }
+
         //After the %PLACEHOLDER% comes a slash, so we can check if we have a placholder via explode
         $tmp = explode("/", $this->path);
 

src/Form/AttachmentFormType.php

@@ -56,12 +56,15 @@ class AttachmentFormType extends AbstractType
     protected $attachment_helper;
     protected $trans;
     protected $urlGenerator;
+    protected $allow_attachments_download;
 
-    public function __construct(AttachmentHelper $attachmentHelper, TranslatorInterface $trans, UrlGeneratorInterface $urlGenerator)
+    public function __construct(AttachmentHelper $attachmentHelper, TranslatorInterface $trans,
+                                UrlGeneratorInterface $urlGenerator, bool $allow_attachments_downloads)
     {
         $this->attachment_helper = $attachmentHelper;
         $this->trans = $trans;
         $this->urlGenerator = $urlGenerator;
+        $this->allow_attachments_download = $allow_attachments_downloads;
     }
 
     public function buildForm(FormBuilderInterface $builder, array $options)
@@ -94,6 +97,13 @@ class AttachmentFormType extends AbstractType
             ]
         ]);
 
+        $builder->add('downloadURL', CheckboxType::class, ['required' => false,
+            'label' =>  $this->trans->trans('attachment.edit.download_url'),
+            'mapped' => false,
+            'disabled' => !$this->allow_attachments_download,
+            'attr' => ['class' => 'form-control-sm'],
+            'label_attr' => ['class' => 'checkbox-custom']]);
+
         $builder->add('file', FileType::class, [
             'label' =>  $this->trans->trans('attachment.edit.file'),
             'mapped' => false,

src/Services/AttachmentHelper.php

@@ -48,6 +48,7 @@ use App\Entity\Attachments\SupplierAttachment;
 use App\Entity\Attachments\UserAttachment;
 use App\Services\Attachments\AttachmentPathResolver;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
+use Symfony\Component\OptionsResolver\OptionsResolver;
 
 class AttachmentHelper
 {
@@ -167,66 +168,7 @@ class AttachmentHelper
         return sprintf("%.{$decimals}f", $bytes / 1024 ** $factor) . @$sz[$factor];
     }
 
-    /**
-     * Generate a path to a folder, where this attachment can save its file.
-     * @param Attachment $attachment The attachment for which the folder should be generated
-     * @return string The path to the folder (without trailing slash)
-     */
-    public function generateFolderForAttachment(Attachment $attachment) : string
-    {
-        $mapping = [PartAttachment::class => 'part', AttachmentTypeAttachment::class => 'attachment_type',
-            CategoryAttachment::class => 'category', CurrencyAttachment::class => 'currency',
-            DeviceAttachment::class => 'device', FootprintAttachment::class => 'footprint',
-            GroupAttachment::class => 'group', ManufacturerAttachment::class => 'manufacturer',
-            MeasurementUnitAttachment::class => 'measurement_unit', StorelocationAttachment::class => 'storelocation',
-            SupplierAttachment::class => 'supplier', UserAttachment::class => 'user'];
 
-        $path = $this->pathResolver->getMediaPath() . DIRECTORY_SEPARATOR . $mapping[get_class($attachment)] . DIRECTORY_SEPARATOR . $attachment->getElement()->getID();
-        return $path;
-    }
 
-    /**
-     * Moves the given uploaded file to a permanent place and saves it into the attachment
-     * @param Attachment $attachment The attachment in which the file should be saved
-     * @param UploadedFile|null $file The file which was uploaded
-     * @param bool $become_preview_if_empty If this is true, the uploaded attachment can become the preview picture
-     * if the of the element, if no was set already.
-     * @return Attachment The attachment with the new filepath
-     */
-    public function upload(Attachment $attachment, ?UploadedFile $file, bool $become_preview_if_empty = true) : Attachment
-    {
-        //If file is null, do nothing (helpful, so we dont have to check if the file was reuploaded in controller)
-        if (!$file) {
-            return $attachment;
-        }
-
-        $folder = $this->generateFolderForAttachment($attachment);
-
-        //Sanatize filename
-        $safeName = transliterator_transliterate('Any-Latin; Latin-ASCII; [^A-Za-z0-9_] remove; Lower()', $attachment->getName());
-        $newFilename = $safeName . '-' . uniqid('', false) . '.' . $file->getClientOriginalExtension();
-
-        //Move our temporay attachment to its final location
-        $file_path = $file->move($folder, $newFilename)->getRealPath();
-
-        //Make our file path relative to %BASE%
-        $file_path = $this->pathResolver->realPathToPlaceholder($file_path);
-
-        //Save the path to the attachment
-        $attachment->setPath($file_path);
-        //And save original filename
-        $attachment->setFilename($file->getClientOriginalName());
-
-        //Check if we should assign this to master picture
-        //this is only possible if the attachment is new (not yet persisted to DB)
-        if ($become_preview_if_empty && $attachment->getID() === null && $attachment->isPicture()) {
-            $element = $attachment->getElement();
-            if ($element instanceof AttachmentContainingDBElement && $element->getMasterPictureAttachment() === null) {
-                $element->setMasterPictureAttachment($attachment);
-            }
-        }
-
-        return $attachment;
-    }
 
 }

src/Services/Attachments/AttachmentPathResolver.php

@@ -47,6 +47,7 @@ class AttachmentPathResolver
     protected $media_path;
     protected $footprints_path;
     protected $models_path;
+    protected $secure_path;
 
     protected $placeholders;
     protected $pathes;
@@ -61,21 +62,19 @@ class AttachmentPathResolver
      * Set to null if this ressource should be disabled.
      * @param string|null $models_path Set to null if this ressource should be disabled.
      */
-    public function __construct(string $project_dir, string $media_path, ?string $footprints_path, ?string $models_path)
+    public function __construct(string $project_dir, string $media_path, string $secure_path, ?string $footprints_path, ?string $models_path)
     {
         $this->project_dir = $project_dir;
 
         //Determine the path for our ressources
         $this->media_path = $this->parameterToAbsolutePath($media_path);
-        /* if ($this->media_path === null) {
-            throw new \RuntimeException("MediaPath is not existing/valid! This parameter is not allowed!");
-        } */
         $this->footprints_path = $this->parameterToAbsolutePath($footprints_path);
         $this->models_path = $this->parameterToAbsolutePath($models_path);
+        $this->secure_path = $this->parameterToAbsolutePath($secure_path);
 
         //Here we define the valid placeholders and their replacement values
-        $this->placeholders = ['%MEDIA%', '%BASE%/data/media', '%FOOTPRINTS%', '%FOOTPRINTS_3D%'];
+        $this->placeholders = ['%MEDIA%', '%BASE%/data/media', '%FOOTPRINTS%', '%FOOTPRINTS_3D%', '%SECURE%'];
-        $this->pathes = [$this->media_path, $this->media_path, $this->footprints_path, $this->models_path];
+        $this->pathes = [$this->media_path, $this->media_path, $this->footprints_path, $this->models_path, $this->secure_path];
 
         //Remove all disabled placeholders
         foreach ($this->pathes as $key => $path) {
@@ -225,6 +224,16 @@ class AttachmentPathResolver
         return $this->media_path;
     }
 
+    /**
+     * The path where secured attachments are stored. Must not be located in public/ folder, so it can only be accessed
+     * via the attachment controller.
+     * @return string The absolute path to the secure path.
+     */
+    public function getSecurePath() : string
+    {
+        return $this->secure_path;
+    }
+
     /**
      * The string where the builtin footprints are stored
      * @return string|null The absolute path to the footprints folder. Null if built footprints were disabled.

src/Services/Attachments/AttachmentSubmitHandler.php

@@ -0,0 +1,219 @@
+<?php
+/**
+ *
+ * part-db version 0.1
+ * Copyright (C) 2005 Christoph Lechner
+ * http://www.cl-projects.de/
+ *
+ * part-db version 0.2+
+ * Copyright (C) 2009 K. Jacobs and others (see authors.php)
+ * http://code.google.com/p/part-db/
+ *
+ * Part-DB Version 0.4+
+ * Copyright (C) 2016 - 2019 Jan Böhmer
+ * https://github.com/jbtronics
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ *
+ */
+
+namespace App\Services\Attachments;
+
+
+use App\Entity\Attachments\Attachment;
+use App\Entity\Attachments\AttachmentContainingDBElement;
+use App\Entity\Attachments\AttachmentTypeAttachment;
+use App\Entity\Attachments\CategoryAttachment;
+use App\Entity\Attachments\CurrencyAttachment;
+use App\Entity\Attachments\DeviceAttachment;
+use App\Entity\Attachments\FootprintAttachment;
+use App\Entity\Attachments\GroupAttachment;
+use App\Entity\Attachments\ManufacturerAttachment;
+use App\Entity\Attachments\MeasurementUnitAttachment;
+use App\Entity\Attachments\PartAttachment;
+use App\Entity\Attachments\StorelocationAttachment;
+use App\Entity\Attachments\SupplierAttachment;
+use App\Entity\Attachments\UserAttachment;
+use App\Services\AttachmentHelper;
+use Doctrine\Common\Annotations\IndexedReader;
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+
+/**
+ * This service handles the form submitting of an attachment and handles things like file uploading and downloading.
+ * @package App\Services\Attachments
+ */
+class AttachmentSubmitHandler
+{
+    protected $pathResolver;
+    protected $folder_mapping;
+    protected $allow_attachments_downloads;
+
+    public function __construct(AttachmentPathResolver $pathResolver, bool $allow_attachments_downloads)
+    {
+        $this->pathResolver = $pathResolver;
+        $this->allow_attachments_downloads = $allow_attachments_downloads;
+
+        //The mapping used to determine which folder will be used for an attachment type
+        $this->folder_mapping = [PartAttachment::class => 'part', AttachmentTypeAttachment::class => 'attachment_type',
+            CategoryAttachment::class => 'category', CurrencyAttachment::class => 'currency',
+            DeviceAttachment::class => 'device', FootprintAttachment::class => 'footprint',
+            GroupAttachment::class => 'group', ManufacturerAttachment::class => 'manufacturer',
+            MeasurementUnitAttachment::class => 'measurement_unit', StorelocationAttachment::class => 'storelocation',
+            SupplierAttachment::class => 'supplier', UserAttachment::class => 'user'];
+    }
+
+    protected function configureOptions(OptionsResolver $resolver) : void
+    {
+        $resolver->setDefaults([
+            //If no preview image was set yet, the new uploaded file will become the preview image
+            'become_preview_if_empty' => true,
+            //When an URL is given download the URL
+            'download_url' => false,
+            'secure_attachment' => false,
+        ]);
+    }
+
+    /**
+     * Generates a filename for the given attachment and extension.
+     * The filename contains a random id, so every time this function is called you get an unique name.
+     * @param Attachment $attachment The attachment that should be used for generating an attachment
+     * @param string $extension The extension that the new file should have (must only contain chars allowed in pathes)
+     * @return string The new filename.
+     */
+    public function generateAttachmentFilename(Attachment $attachment, string $extension) : string
+    {
+        //Normalize extension
+        $extension = transliterator_transliterate(
+            'Any-Latin; Latin-ASCII; [^A-Za-z0-9_] remove; Lower()',
+            $extension
+        );
+
+        //Use the (sanatized) attachment name as an filename part
+        $safeName = transliterator_transliterate(
+            'Any-Latin; Latin-ASCII; [^A-Za-z0-9_] remove; Lower()',
+            $attachment->getName()
+        );
+
+        return $safeName . '-' . uniqid('', false) . '.' . $extension;
+    }
+
+    /**
+     * Generates an (absolute) path to a folder where the given attachment should be stored.
+     * @param Attachment $attachment The attachment that should be used for
+     * @param bool $secure_upload True if the file path should be located in a safe location
+     * @return string The absolute path for the attachment folder.
+     */
+    public function generateAttachmentPath(Attachment $attachment, bool $secure_upload = false) : string
+    {
+        if ($secure_upload) {
+            $base_path = $this->pathResolver->getSecurePath();
+        } else {
+            $base_path = $this->pathResolver->getMediaPath();
+        }
+
+        //Ensure the given attachment class is known to mapping
+        if (!isset($this->folder_mapping[get_class($attachment)])) {
+            throw new \InvalidArgumentException(
+                'The given attachment class is not known! The passed class was: ' . get_class($attachment)
+            );
+        }
+        //Ensure the attachment has an assigned element
+        if ($attachment->getElement() === null) {
+            throw new \InvalidArgumentException(
+                'The given attachment is not assigned to an element! An element is needed to generate a path!'
+            );
+        }
+
+        //Build path
+        return
+            $base_path . DIRECTORY_SEPARATOR //Base path
+            . $this->folder_mapping[get_class($attachment)] . DIRECTORY_SEPARATOR . $attachment->getElement()->getID();
+    }
+
+    /**
+     * Handle the submit of an attachment form.
+     * This function will move the uploaded file or download the URL file to server, if needed.
+     * @param Attachment $attachment The attachment that should be used for handling.
+     * @param UploadedFile|null $file If given, that file will be moved to the right location
+     * @param array $options The options to use with the upload. Here you can specify that an URL should be downloaded,
+     * or an file should be moved to a secure location.
+     * @return Attachment The attachment with the new filename (same instance as passed $attachment)
+     */
+    public function handleFormSubmit(Attachment $attachment, ?UploadedFile $file, array $options = []) : Attachment
+    {
+        $resolver = new OptionsResolver();
+        $this->configureOptions($resolver);
+        $options = $resolver->resolve($options);
+
+        //When a file is given then upload it, otherwise check if we need to download the URL
+        if ($file) {
+            $this->upload($attachment, $file, $options);
+        } elseif ($options['download_url'] && $attachment->isExternal()) {
+            $this->downloadURL($attachment, $options);
+        }
+
+        //Check if we should assign this attachment to master picture
+        //this is only possible if the attachment is new (not yet persisted to DB)
+        if ($options['become_preview_if_empty'] && $attachment->getID() === null && $attachment->isPicture()) {
+            $element = $attachment->getElement();
+            if ($element instanceof AttachmentContainingDBElement && $element->getMasterPictureAttachment() === null) {
+                $element->setMasterPictureAttachment($attachment);
+            }
+        }
+
+        return $attachment;
+    }
+
+    /**
+     * Download the URL set in the attachment and save it on the server
+     * @param Attachment $attachment
+     * @param array $options The options from the handleFormSubmit function
+     * @return Attachment The attachment with the new filepath
+     */
+    protected function downloadURL(Attachment $attachment, array $options) : Attachment
+    {
+        //Check if we are allowed to download files
+        if (!$this->allow_attachments_downloads) {
+            throw new \RuntimeException('Download of attachments is not allowed!');
+        }
+    }
+
+    /**
+     * Moves the given uploaded file to a permanent place and saves it into the attachment
+     * @param Attachment $attachment The attachment in which the file should be saved
+     * @param UploadedFile $file The file which was uploaded
+     * @param array $options The options from the handleFormSubmit function
+     * @return Attachment The attachment with the new filepath
+     */
+    protected function upload(Attachment $attachment, UploadedFile $file, array $options) : Attachment
+    {
+
+        //Move our temporay attachment to its final location
+        $file_path = $file->move(
+            $this->generateAttachmentPath($attachment, $options['secure_attachment']),
+            $this->generateAttachmentFilename($attachment, $file->getClientOriginalExtension())
+        )->getRealPath();
+
+        //Make our file path relative to %BASE%
+        $file_path = $this->pathResolver->realPathToPlaceholder($file_path);
+        //Save the path to the attachment
+        $attachment->setPath($file_path);
+        //And save original filename
+        $attachment->setFilename($file->getClientOriginalName());
+
+        return $attachment;
+    }
+}

tests/Entity/AttachmentTest.php

@@ -42,6 +42,10 @@ class AttachmentTest extends TestCase
     public function testIsExternal()
     {
         $attachment = new PartAttachment();
+
+        $this->setProtectedProperty($attachment, 'path', '');
+        $this->assertFalse($attachment->isExternal());
+
         $this->setProtectedProperty($attachment, 'path', '%MEDIA%/foo/bar.txt');
         $this->assertFalse($attachment->isExternal());