Support for custom CSS/JS by app setting or tenant

Until now, it was only possible to configure a custom CSS on the tenant. For some customizing (e.g. renaming submit button text on standalone new ticket view), custom JS is necessary. However, custom JS may open the door to malicious JS injection after getting illegitimate access to any agent's account. Time for some extra security. There are four new application settings now: enable_custom_stylesheet (default: true) Main switch for custom stylesheets. enable_custom_javascript (default: false) Main switch for custom javascript. custom_stylesheet_url (default: nil) Overrides the URL set on the tenant. This URL will be showed on the settings form as a disabled input. custom_javascript_url (default: nil) Overrides the URL set on the tenant. This URLwill be showed on the settings form as a disabled input.
ivaldi · Oct 12, 2018 · 0dc6b15 · 0dc6b15
1 parent 7d3bb54
commit 0dc6b15
Show file tree

Hide file tree

Showing 12 changed files with 140 additions and 14 deletions.
diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb
@@ -46,6 +46,7 @@ def tenant_params
       :notify_client_when_ticket_is_created,
       :ticket_creation_is_open_to_the_world,
       :stylesheet_url,
+      :javascript_url,
       :always_notify_me,
       :work_can_wait
     )
@@ -57,7 +58,7 @@ def month_names
           format: "%B"
     end
   end
-  
+
   def day_names
     @day_names = (1..7).map do |m|
       I18n.l DateTime.parse(Date::MONTHNAMES[m])

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
@@ -44,4 +44,23 @@ def tabindex
     @tabindex += 1
   end
 
+  # Tag to link the custom stylesheet if enabled and either set by application
+  # setting (higher priority) or tenant setting (lower priority)
+  def custom_stylesheet_link_tag
+    if AppSettings.enable_custom_stylesheet
+      if url = AppSettings.custom_stylesheet_url || Tenant.current_tenant.stylesheet_url
+        stylesheet_link_tag url
+      end
+    end
+  end
+
+  # Tag to include the custom javascript if enabled and either set by application
+  # setting (higher priority) or tenant setting (lower priority)
+  def custom_javascript_include_tag
+    if AppSettings.enable_custom_javascript
+      if url = AppSettings.custom_javascript_url || Tenant.current_tenant.javascript_url
+        javascript_include_tag url
+      end
+    end
+  end
 end
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
@@ -16,11 +16,8 @@
     <% else %>
       <%= stylesheet_link_tag 'application' %>
     <% end %>
-
     <%= stylesheet_link_tag 'application-print', media: 'print' %>
-    <% if Tenant.current_tenant.stylesheet_url? %>
-      <%= stylesheet_link_tag Tenant.current_tenant.stylesheet_url %>
-    <% end %>
+    <%= custom_stylesheet_link_tag %>
 
     <%= javascript_include_tag 'vendor/modernizr' %>
     <%= csrf_meta_tags %>
@@ -126,6 +123,7 @@
     </div>
 
     <%= javascript_include_tag 'application' %>
+    <%= custom_javascript_include_tag %>
 
     <% if user_signed_in? && @show_joyride %>
       <ol class="joyride-list" data-joyride data-options="expose: true;scroll_speed: 1000;">

diff --git a/app/views/settings/edit.html.erb b/app/views/settings/edit.html.erb
@@ -34,9 +34,25 @@
 
       <%= f.check_box :notify_user_when_account_is_created %>
 
-      <h5><%= t(:look_and_feel) %></h5>
-
-      <%= f.text_field :stylesheet_url %>
+      <% if AppSettings.enable_custom_stylesheet || AppSettings.enable_custom_javascript %>
+        <h5><%= t(:look_and_feel) %></h5>
+
+        <% if AppSettings.enable_custom_stylesheet %>
+          <% if AppSettings.custom_stylesheet_url %>
+            <%= f.text_field(:stylesheet_url, value: AppSettings.custom_stylesheet_url, disabled: true) %>
+          <% else %>
+            <%= f.text_field(:stylesheet_url) %>
+          <% end %>
+        <% end %>
+
+        <% if AppSettings.enable_custom_javascript %>
+          <% if AppSettings.custom_javascript_url %>
+            <%= f.text_field(:javascript_url, value: AppSettings.custom_javascript_url, disabled: true) %>
+          <% else %>
+            <%= f.text_field(:javascript_url) %>
+          <% end %>
+        <% end %>
+      <% end %>
 
     </div>
 

diff --git a/config/locales/de.yml b/config/locales/de.yml
@@ -429,4 +429,5 @@ de:
         notify_client_when_ticket_is_created: Benutzer benachrichtigen nachdem ein neues Ticket erstellt wurde.
         notify_user_when_account_is_created: Benutzer benachrichtigen nachdem das Benutzerkonto erstellt wurde.
         stylesheet_url: Pfad zu eigenem Stylesheet.
+        javascript_url: Pfad zur eigenen Javascript-Datei.
         ticket_creation_is_open_to_the_world: Jeder ist berechtigt, Tickets zu erstellen.
diff --git a/config/locales/en.yml b/config/locales/en.yml
@@ -427,5 +427,6 @@ en:
         notify_client_when_ticket_is_assigned_or_closed: Notify the client when his ticket is assigned or closed.
         notify_client_when_ticket_is_created: Notify the client when his newly created ticket is received.
         notify_user_when_account_is_created: Notify user when account is created.
-        stylesheet_url: Add the path to your own custom stylesheet.
+        stylesheet_url: Path to custom stylesheet.
+        javascript_url: Path to custom javascript file.
         ticket_creation_is_open_to_the_world: Tickets creation is open to everybody.
diff --git a/config/locales/fr-CA.yml b/config/locales/fr-CA.yml
@@ -427,5 +427,6 @@ fr-CA:
         notify_client_when_ticket_is_assigned_or_closed: Notifier l'utilisateur après avoir attribué ou fermé son billet.
         notify_client_when_ticket_is_created: Notifier l'utilisateur après avoir reçu et créé un nouveau billet pour lui.
         notify_user_when_account_is_created: Notifier l'utilisateur après avoir créé son compte utilisateur.
-        stylesheet_url: Ajouter chemin pour feuille de style personalisée.
+        stylesheet_url: Chemin pour feuille de style personalisée.
+        javascript_url: Chemin pour fichier javascript personalisé.
         ticket_creation_is_open_to_the_world: Création de billets autorisée à tout le monde.
diff --git a/config/locales/fr-FR.yml b/config/locales/fr-FR.yml
@@ -427,5 +427,6 @@ fr-FR:
         notify_client_when_ticket_is_assigned_or_closed: Notifier l'utilisateur après avoir attribué ou fermé son ticket.
         notify_client_when_ticket_is_created: Notifier l'utilisateur après avoir reçu et créé un nouveau ticket pour lui.
         notify_user_when_account_is_created: Notifier l'utilisateur après avoir créé son compte utilisateur.
-        stylesheet_url: Ajouter chemin pour feuille de style personalisée.
+        stylesheet_url: Chemin pour feuille de style personalisée.
+        javascript_url: Chemin pour fichier javascript personalisé.
         ticket_creation_is_open_to_the_world: Création de tickets autorisée à tout le monde.
diff --git a/config/settings.yml b/config/settings.yml
@@ -2,6 +2,10 @@ default: &defaults
   display_user_avatars: true
   default_user_avatar: 'mm'
   user_avatar_api_url: ''
+  enable_custom_stylesheet: true
+  custom_stylesheet_url:
+  enable_custom_javascript: false
+  custom_javascript_url:
 
 development:
   <<: *defaults

diff --git a/db/migrate/20181011062745_add_javascript_url_to_tenants.rb b/db/migrate/20181011062745_add_javascript_url_to_tenants.rb
@@ -0,0 +1,7 @@
+class AddJavascriptUrlToTenants < ActiveRecord::Migration[5.1]
+  def change
+    unless column_exists? :tenants, :javascript_url
+      add_column :tenants, :javascript_url, :string
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20171016140901) do
+ActiveRecord::Schema.define(version: 20181011062745) do
 
   create_table "attachments", force: :cascade do |t|
     t.integer "attachable_id"
@@ -147,6 +147,7 @@
     t.integer "email_template_id"
     t.boolean "ticket_creation_is_open_to_the_world", default: true
     t.string "stylesheet_url"
+    t.string "javascript_url"
     t.index ["domain"], name: "index_tenants_on_domain", unique: true
     t.index ["email_template_id"], name: "index_tenants_on_email_template_id"
   end

diff --git a/test/controllers/settings_controller_test.rb b/test/controllers/settings_controller_test.rb
@@ -37,12 +37,88 @@ class SettingsControllerTest < ActionController::TestCase
       assert_difference 'EmailTemplate.count', 2 do
         put :update, params: {
           id: @tenant.id, tenant: {
-          notify_user_when_account_is_created: true,
-          notify_client_when_ticket_is_created: true
+            notify_user_when_account_is_created: true,
+            notify_client_when_ticket_is_created: true
           }
         }
       end
     end
   end
 
+  test 'enabled custom stylesheet set by tenant' do
+    AppSettings.enable_custom_stylesheet = true
+    AppSettings.custom_stylesheet_url = nil
+    sign_in users(:alice)
+    put :update, params: {
+      id: @tenant.id, tenant: {
+        stylesheet_url: '/tenant/custom.css'
+      }
+    }
+    body = get(:edit).body
+    assert_match %r(<link[^>]+href="/tenant/custom.css"), body
+    assert_match %r(<input[^>]+stylesheet_url), body
+    refute_match %r(<input[^>]+disabled[^>]+stylesheet_url), body
+  end
+
+  test 'enabled custom stylesheet set by app settings' do
+    AppSettings.enable_custom_stylesheet = true
+    AppSettings.custom_stylesheet_url = '/appsettings/custom.css'
+    sign_in users(:alice)
+    put :update, params: {
+      id: @tenant.id, tenant: {
+        stylesheet_url: '/tenant/custom.css'
+      }
+    }
+    body = get(:edit).body
+    assert_match %r(<link[^>]+href="/appsettings/custom.css"), body
+    assert_match %r(<input[^>]+disabled[^>]+stylesheet_url), body
+  end
+
+  test 'disabled custom stylesheet' do
+    AppSettings.enable_custom_stylesheet = false
+    AppSettings.custom_stylesheet_url = '/appsettings/custom.css'
+    sign_in users(:alice)
+    body = get(:edit).body
+    refute_match %r(<link[^>]+href="/appsettings/custom.css"), body
+    refute_match %r(<input[^>]+stylesheet_url), body
+  end
+
+  test 'enabled custom javascript set by tenant' do
+    AppSettings.enable_custom_javascript = true
+    AppSettings.custom_javascript_url = nil
+    sign_in users(:alice)
+    put :update, params: {
+      id: @tenant.id, tenant: {
+        javascript_url: '/tenant/custom.js'
+      }
+    }
+    body = get(:edit).body
+    assert_match %r(<script[^>]+src="/tenant/custom.js"), body
+    assert_match %r(<input[^>]+javascript_url), body
+    refute_match %r(<input[^>]+disabled[^>]+javascript_url), body
+  end
+
+  test 'enabled custom javascript set by app settings ' do
+    AppSettings.enable_custom_javascript = true
+    AppSettings.custom_javascript_url = '/appsettings/custom.js'
+    sign_in users(:alice)
+    put :update, params: {
+      id: @tenant.id, tenant: {
+        javascript_url: '/tenant/custom.js'
+      }
+    }
+    body = get(:edit).body
+    assert_match %r(<script[^>]+src="/appsettings/custom.js"), body
+    assert_match %r(<input[^>]+disabled[^>]+javascript_url), body
+  end
+
+  test 'disabled custom javascript' do
+    AppSettings.enable_custom_javascript = false
+    AppSettings.custom_javascript_url = '/appsettings/custom.js'
+    sign_in users(:alice)
+    body = get(:edit).body
+    refute_match %r(<script[^>]+src="/appsettings/custom.js"), body
+    refute_match %r(<input[^>]+javascript_url), body
+  end
+
 end