Secure attachments - closes freescout-help-desk#308

alinmbb · Mar 7, 2020 · b4133db · b4133db
1 parent 37646b4
commit b4133db
Show file tree

Hide file tree

Showing 6 changed files with 146 additions and 32 deletions.
diff --git a/app/Attachment.php b/app/Attachment.php
@@ -19,6 +19,8 @@ class Attachment extends Model
 
     const DIRECTORY = 'attachment';
 
+    CONST DISK = 'private';
+
     // https://github.com/Webklex/laravel-imap/blob/master/src/IMAP/Attachment.php
     public static $types = [
         'message'     => self::TYPE_MESSAGE,
@@ -93,18 +95,18 @@ public static function create($file_name, $mime_type, $type, $content, $uploaded
         do {
             $i++;
             $file_path = self::DIRECTORY.DIRECTORY_SEPARATOR.$file_dir.$i.DIRECTORY_SEPARATOR.$file_name;
-        } while (Storage::exists($file_path));
+        } while (Storage::disk(self::DISK)->exists($file_path));
 
         $file_dir .= $i.DIRECTORY_SEPARATOR;
 
         if ($uploaded_file) {
-            $uploaded_file->storeAs(self::DIRECTORY.DIRECTORY_SEPARATOR.$file_dir, $file_name);
+            $uploaded_file->storeAs(self::DIRECTORY.DIRECTORY_SEPARATOR.$file_dir, $file_name, ['disk' => self::DISK]);
         } else {
-            Storage::put($file_path, $content);
+            Storage::disk(self::DISK)->put($file_path, $content);
         }
 
         $attachment->file_dir = $file_dir;
-        $attachment->size = Storage::size($file_path);
+        $attachment->size = Storage::disk(self::DISK)->size($file_path);
         $attachment->save();
 
         return $attachment;
@@ -189,7 +191,28 @@ public static function typeNameToInt($type_name)
      */
     public function url()
     {
-        return Storage::url($this->getStorageFilePath());
+        return Storage::url($this->getStorageFilePath()).'?id='.$this->id.'&token='.$this->getToken();
+    }
+
+    /**
+     * Get hashed security token for the attachment.
+     */
+    public function getToken()
+    {
+        // \Hash::make() may contain . and / symbols which may cause problems.
+        return md5(config('app.key').$this->id.$this->size);
+    }
+
+    /**
+     * Outputs the current Attachment as download
+     */
+    public function download()
+    {
+        return $this->getDisk()->download($this->getStorageFilePath(), $this->file_name);
+    }
+
+    private function getDisk() {
+        return Storage::disk(self::DISK);
     }
 
     /**
@@ -202,14 +225,24 @@ public function getSizeName()
         return self::formatBytes($this->size);
     }
 
+    /**
+     * attachment/...
+     */
     public function getStorageFilePath()
     {
         return self::DIRECTORY.DIRECTORY_SEPARATOR.$this->file_dir.$this->file_name;
     }
 
-    public function getLocalFilePath()
+    /**
+     * /var/html/storage/app/attachment/...
+     */
+    public function getLocalFilePath($full = true)
     {
-        return Storage::path(self::DIRECTORY.DIRECTORY_SEPARATOR.$this->file_dir.$this->file_name);
+        if ($full) {
+            return $this->getDisk()->path(self::DIRECTORY.DIRECTORY_SEPARATOR.$this->file_dir.$this->file_name);
+        } else {
+            return DIRECTORY_SEPARATOR.'storage'.DIRECTORY_SEPARATOR.'app'.DIRECTORY_SEPARATOR.self::DIRECTORY.DIRECTORY_SEPARATOR.$this->file_dir.$this->file_name;
+        }
     }
 
     public static function formatBytes($size, $precision = 0)
@@ -260,29 +293,10 @@ public static function deleteForever($attachments)
     {
         // Delete from disk
         foreach ($attachments as $attachment) {
-            Storage::delete($attachment->getStorageFilePath());
+            $attachment->getDisk()->delete($attachment->getStorageFilePath());
         }
 
         // Delete from DB
         self::whereIn('id', $attachments->pluck('id')->toArray())->delete();
     }
-
-    /**
-     * Generate dummy ID for attachment.
-     * Not used for now.
-     */
-    /*public static function genrateDummyId()
-    {
-        if (!$this->id) {
-            // Math.random should be unique because of its seeding algorithm.
-            // Convert it to base 36 (numbers + letters), and grab the first 9 characters
-            // after the decimal.
-            $hash = mt_rand() / mt_getrandmax();
-            $hash = base_convert($hash, 10, 36);
-            $id = substr($hash, 2, 9);
-        } else {
-            $id = substr(md5($this->id), 0, 9);
-        }
-        return $id;
-    }*/
 }
diff --git a/app/Http/Controllers/PublicController.php b/app/Http/Controllers/PublicController.php
@@ -2,6 +2,7 @@
 
 namespace App\Http\Controllers;
 
+use App\Attachment;
 use App\Conversation;
 use App\Option;
 use App\Thread;
@@ -134,4 +135,51 @@ public function setThreadAsRead($conversation_id, $thread_id)
 
         return $response;
     }
+
+    /**
+     * Download an attachment.
+     */
+    public function downloadAttachment($dir_1, $dir_2, $dir_3, $file_name, Request $request)
+    {
+        $id = $request->query('id', '');
+        $token = $request->query('token', '');
+        $attachment = null;
+
+        // Old attachments can not be requested by id.
+        if (!$token && $id) {
+            return \Helper::denyAccess();
+        }
+
+        // Get attachment by id.
+        if ($id) {
+            $attachment = Attachment::findOrFail($id);
+        }
+
+        if (!$attachment) {
+            $attachment = Attachment::where('file_dir', $dir_1.DIRECTORY_SEPARATOR.$dir_2.DIRECTORY_SEPARATOR.$dir_3.DIRECTORY_SEPARATOR)
+                ->where('file_name', $file_name)
+                ->firstOrFail();
+        }
+
+        // Only allow download if the attachment is public or if the token matches the hash of the contents
+        if ($token != $attachment->getToken() && (bool)$attachment->public !== true) {
+            return \Helper::denyAccess();
+        }
+
+        if (config('app.attachments_download_mode') == 'apache') {
+            // Send using Apache mod_xsendfile.
+            return response(null)
+               ->header('Content-Type' , $attachment->mime_type)
+               ->header('Content-Disposition', 'attachment; filename="'.$attachment->file_name.'"')
+               ->header('X-Sendfile', $attachment->getLocalFilePath());
+        } elseif (config('app.attachments_download_mode') == 'nginx') {
+            // Send using Nginx.
+            return response(null)
+               ->header('Content-Type' , $attachment->mime_type)
+               ->header('Content-Disposition', 'attachment; filename="'.$attachment->file_name.'"')
+               ->header('X-Accel-Redirect', $attachment->getLocalFilePath(false));
+        } else {
+            return $attachment->download();
+        }
+    }
 }
diff --git a/config/app.php b/config/app.php
@@ -247,6 +247,17 @@
         'open_tracking'      => ['default' => true],
     ],
 
+    /*
+    |--------------------------------------------------------------------------
+    | php - attachments are downloaded via PHP.
+    |
+    | apache - attachments are downloaded via Apache's mod_xsendfile.
+    |
+    | nginx - attachments are downloaded via nginx's X-Accel-Redirect.
+    |-------------------------------------------------------------------------
+    */
+    'attachments_download_mode'    => env('APP_ATTACHMENTS_DOWNLOAD_MODE', 'php'),
+
     /*
     |--------------------------------------------------------------------------
     | Autoloaded Service Providers

diff --git a/database/migrations/2020_03_06_100100_add_public_column_to_attachments_table.php b/database/migrations/2020_03_06_100100_add_public_column_to_attachments_table.php
@@ -0,0 +1,45 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class AddPublicColumnToAttachmentsTable extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('attachments', function (Blueprint $table) {
+            $table->boolean('public')->default(false);
+        });
+        DB::table('attachments')->update(['public' => true]);
+
+        $old_path = storage_path('app'.DIRECTORY_SEPARATOR.'public'.DIRECTORY_SEPARATOR.'attachment');
+        $new_path = storage_path('app'.DIRECTORY_SEPARATOR.'attachment');
+
+        // Move attachments.
+        try {
+            if (File::exists($old_path) && File::isDirectory($old_path) && !File::exists($new_path)) {
+                File::move($old_path, $new_path);
+            }
+        } catch (\Exception $e) {
+            \Log::error('Migration Error: '.$e->getMessage());
+        }
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('attachments', function (Blueprint $table) {
+            $table->dropColumn('public');
+        });
+    }
+}
diff --git a/routes/web.php b/routes/web.php
@@ -21,6 +21,7 @@
 // Public routes
 Route::get('/user-setup/{hash}', 'PublicController@userSetup')->name('user_setup');
 Route::post('/user-setup/{hash}', 'PublicController@userSetupSave');
+Route::get('/storage/attachment/{dir_1}/{dir_2}/{dir_3}/{file_name}', 'PublicController@downloadAttachment')->name('attachment.download');
 
 // General routes for logged in users
 Route::get('/', 'SecureController@dashboard')->name('dashboard');

diff --git a/storage/app/public/attachment/.htaccess b/storage/app/public/attachment/.htaccess