From 6fdd4a29969be8423f31890b807d27d13627c50c Mon Sep 17 00:00:00 2001
From: Sojan Jose <sojan@pepalo.com>
Date: Wed, 1 Sep 2021 15:08:05 +0530
Subject: [PATCH] chore: Security Improvements to the API (#2893)

- Devise auth tokens are reset on password update
- Avatar attachment file type is limited to jpeg,gif and png
- Avatar attachment file size is limited to 15 mb
- Widget Message attachments are limited to types ['image/png', 'image/jpeg', 'image/gif', 'image/bmp', 'image/tiff', 'application/pdf', 'audio/mpeg', 'video/mp4', 'audio/ogg', 'text/csv']
- Widget Message attachments are limited to 40Mb size limit.
---
 app/builders/messages/message_builder.rb      | 24 +++++++++++--------
 .../api/v1/widget/messages_controller.rb      |  9 ++++---
 .../api/v1/inboxes/messages_controller.rb     |  9 ++++---
 app/helpers/file_type_helper.rb               |  2 +-
 .../widgets/WootWriter/ReplyBottomPanel.vue   |  3 ++-
 .../widgets/forms/AvatarUploader.vue          |  3 ++-
 app/models/attachment.rb                      | 19 +++++++++++++++
 app/models/concerns/avatarable.rb             | 10 ++++++++
 config/initializers/devise_token_auth.rb      |  4 ++++
 9 files changed, 60 insertions(+), 23 deletions(-)

diff --git a/app/builders/messages/message_builder.rb b/app/builders/messages/message_builder.rb
index ba78d76d7..b6ae5409d 100644
--- a/app/builders/messages/message_builder.rb
+++ b/app/builders/messages/message_builder.rb
@@ -15,21 +15,25 @@ class Messages::MessageBuilder
 
   def perform
     @message = @conversation.messages.build(message_params)
-    if @attachments.present?
-      @attachments.each do |uploaded_attachment|
-        attachment = @message.attachments.new(
-          account_id: @message.account_id,
-          file_type: file_type(uploaded_attachment&.content_type)
-        )
-        attachment.file.attach(uploaded_attachment)
-      end
-    end
-    @message.save
+    process_attachments
+    @message.save!
     @message
   end
 
   private
 
+  def process_attachments
+    return if @attachments.blank?
+
+    @attachments.each do |uploaded_attachment|
+      @message.attachments.build(
+        account_id: @message.account_id,
+        file_type: file_type(uploaded_attachment&.content_type),
+        file: uploaded_attachment
+      )
+    end
+  end
+
   def message_type
     if @conversation.inbox.channel_type != 'Channel::Api' && @message_type == 'incoming'
       raise StandardError, 'Incoming messages are only allowed in Api inboxes'
diff --git a/app/controllers/api/v1/widget/messages_controller.rb b/app/controllers/api/v1/widget/messages_controller.rb
index 9e6f770ad..0d9ab6323 100644
--- a/app/controllers/api/v1/widget/messages_controller.rb
+++ b/app/controllers/api/v1/widget/messages_controller.rb
@@ -8,8 +8,8 @@ class Api::V1::Widget::MessagesController < Api::V1::Widget::BaseController
 
   def create
     @message = conversation.messages.new(message_params)
-    @message.save
     build_attachment
+    @message.save!
   end
 
   def update
@@ -29,13 +29,12 @@ class Api::V1::Widget::MessagesController < Api::V1::Widget::BaseController
     return if params[:message][:attachments].blank?
 
     params[:message][:attachments].each do |uploaded_attachment|
-      attachment = @message.attachments.new(
+      @message.attachments.new(
         account_id: @message.account_id,
-        file_type: helpers.file_type(uploaded_attachment&.content_type)
+        file_type: helpers.file_type(uploaded_attachment&.content_type),
+        file: uploaded_attachment
       )
-      attachment.file.attach(uploaded_attachment)
     end
-    @message.save!
   end
 
   def set_conversation
diff --git a/app/controllers/public/api/v1/inboxes/messages_controller.rb b/app/controllers/public/api/v1/inboxes/messages_controller.rb
index 68c0f5223..925c16d38 100644
--- a/app/controllers/public/api/v1/inboxes/messages_controller.rb
+++ b/app/controllers/public/api/v1/inboxes/messages_controller.rb
@@ -7,8 +7,8 @@ class Public::Api::V1::Inboxes::MessagesController < Public::Api::V1::InboxesCon
 
   def create
     @message = @conversation.messages.new(message_params)
-    @message.save
     build_attachment
+    @message.save!
   end
 
   def update
@@ -23,13 +23,12 @@ class Public::Api::V1::Inboxes::MessagesController < Public::Api::V1::InboxesCon
     return if params[:attachments].blank?
 
     params[:attachments].each do |uploaded_attachment|
-      attachment = @message.attachments.new(
+      @message.attachments.new(
         account_id: @message.account_id,
-        file_type: helpers.file_type(uploaded_attachment&.content_type)
+        file_type: helpers.file_type(uploaded_attachment&.content_type),
+        file: uploaded_attachment
       )
-      attachment.file.attach(uploaded_attachment)
     end
-    @message.save!
   end
 
   def message_finder_params
diff --git a/app/helpers/file_type_helper.rb b/app/helpers/file_type_helper.rb
index 4936fa155..64d67701a 100644
--- a/app/helpers/file_type_helper.rb
+++ b/app/helpers/file_type_helper.rb
@@ -3,12 +3,12 @@ module FileTypeHelper
     return :image if [
       'image/jpeg',
       'image/png',
-      'image/svg+xml',
       'image/gif',
       'image/tiff',
       'image/bmp'
     ].include?(content_type)
 
+    return :video if content_type.include?('video/')
     return :audio if content_type.include?('audio/')
 
     :file
diff --git a/app/javascript/dashboard/components/widgets/WootWriter/ReplyBottomPanel.vue b/app/javascript/dashboard/components/widgets/WootWriter/ReplyBottomPanel.vue
index 1ffb09e04..1ffc9be65 100644
--- a/app/javascript/dashboard/components/widgets/WootWriter/ReplyBottomPanel.vue
+++ b/app/javascript/dashboard/components/widgets/WootWriter/ReplyBottomPanel.vue
@@ -11,10 +11,11 @@
         @click="toggleEmojiPicker"
       />
 
+      <!-- ensure the same validations for attachment types are implemented in  backend models as well -->
       <file-upload
         ref="upload"
         :size="4096 * 4096"
-        accept="image/*, application/pdf, audio/mpeg, video/mp4, audio/ogg, text/csv"
+        accept="image/png, image/jpeg, image/gif, image/bmp, image/tiff, application/pdf, audio/mpeg, video/mp4, audio/ogg, text/csv"
         :drop="true"
         :drop-directory="false"
         @input-file="onFileUpload"
diff --git a/app/javascript/dashboard/components/widgets/forms/AvatarUploader.vue b/app/javascript/dashboard/components/widgets/forms/AvatarUploader.vue
index 89ca4f56a..705d15ad8 100644
--- a/app/javascript/dashboard/components/widgets/forms/AvatarUploader.vue
+++ b/app/javascript/dashboard/components/widgets/forms/AvatarUploader.vue
@@ -20,12 +20,13 @@
         id="file"
         ref="file"
         type="file"
-        accept="image/*"
+        accept="image/png, image/jpeg, image/gif"
         @change="handleImageUpload"
       />
       <slot></slot>
     </label>
   </div>
+
 </template>
 
 <script>
diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 697de2767..cb1f21e33 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -20,6 +20,7 @@ class Attachment < ApplicationRecord
   belongs_to :account
   belongs_to :message
   has_one_attached :file
+  validate :acceptable_file
 
   enum file_type: [:image, :audio, :video, :file, :location, :fallback]
 
@@ -76,4 +77,22 @@ class Attachment < ApplicationRecord
       account_id: account_id
     }
   end
+
+  def should_validate_file?
+    return unless file.attached?
+    # we are only limiting attachment types in case of website widget
+    return unless message.inbox.channel_type == 'Channel::WebWidget'
+
+    true
+  end
+
+  def acceptable_file
+    should_validate_file?
+
+    errors.add(:file, 'is too big') if file.byte_size > 40.megabytes
+
+    acceptable_types = ['image/png', 'image/jpeg', 'image/gif', 'image/bmp', 'image/tiff', 'application/pdf', 'audio/mpeg', 'video/mp4', 'audio/ogg',
+                        'text/csv'].freeze
+    errors.add(:file, 'filetype not supported') unless acceptable_types.include?(file.content_type)
+  end
 end
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index a44beb28b..d5bcb862d 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -6,6 +6,7 @@ module Avatarable
 
   included do
     has_one_attached :avatar
+    validate :acceptable_avatar
   end
 
   def avatar_url
@@ -18,4 +19,13 @@ module Avatarable
 
     ''
   end
+
+  def acceptable_avatar
+    return unless avatar.attached?
+
+    errors.add(:avatar, 'is too big') if avatar.byte_size > 15.megabytes
+
+    acceptable_types = ['image/jpeg', 'image/png', 'image/gif'].freeze
+    errors.add(:avatar, 'filetype not supported') unless acceptable_types.include?(avatar.content_type)
+  end
 end
diff --git a/config/initializers/devise_token_auth.rb b/config/initializers/devise_token_auth.rb
index 07904e621..f222bd7ba 100644
--- a/config/initializers/devise_token_auth.rb
+++ b/config/initializers/devise_token_auth.rb
@@ -9,6 +9,10 @@ DeviseTokenAuth.setup do |config|
   # determines how long tokens will remain valid after they are issued.
   config.token_lifespan = 2.months
 
+  # By default, old tokens are not invalidated when password is changed.
+  # Enable this option if you want to make passwords updates to logout other devices.
+  config.remove_tokens_after_password_reset = true
+
   # Sets the max number of concurrent devices per user, which is 10 by default.
   # After this limit is reached, the oldest tokens will be removed.
   # config.max_number_of_devices = 10