Merge branch 'trunk' into settings_for_DC

app/controllers/new/templates_controller.rb

@@ -8,6 +8,9 @@ class TemplatesController < ::ApplicationController
     protect_from_forgery except: [:new], with: :exception
     layout 'application'
 
+    before_action :sanatize_template_body, only: [:create, :update]
+    before_action :sanatize_instance_preview_params, only: [:instant_preview]
+
     def index
       @notice_kinds = Templates::TemplateModel.all
       @datatable = Effective::Datatables::NoticesDatatable.new
@@ -48,9 +51,8 @@ def create
           redirect_to action: :edit, id: record.success.id
         else
           @errors = Array.wrap(record.failure)
-          @templates = Templates::TemplateModel.all
-
-          render action: 'index'
+          flash[:error] = "Unable to create template due to #{@errors}"
+          redirect_to action: :index
         end
       else
         flash[:error] = "Unable to create template due to #{result.errors}"
@@ -62,8 +64,13 @@ def update
       result = Templates::TemplateContract.new.call(template_params.to_h)
 
       if result.success?
-        Templates::Template.new(result.to_h).update_model(params['id'])
-        flash[:notice] = 'Notice content updated successfully'
+        record = Templates::Template.new(result.to_h).update_model(params['id'])
+        if record.success?
+          flash[:notice] = 'Notice content updated successfully'
+        else
+          @errors = Array.wrap(record.failure)
+          flash[:error] = "Unable to create template due to #{@errors}"
+        end
         redirect_to action: :index
       else
         flash[:error] = "Unable to update template due to #{result.errors}"
@@ -228,5 +235,24 @@ def builder_param
       entities_contracts_mapping[params['builder']] ||
         '::AcaEntities::MagiMedicaid::Contracts::ApplicationContract'
     end
+
+    def sanatize_instance_preview_params
+      template = instant_preview_params
+      raw_text = [template['title'], template['subject'], template['body']].join('\n\n')
+      validate_params(raw_text)
+    end
+
+    def sanatize_template_body
+      template = template_params
+      raw_text = [template['title'], template['description'], template['body']].join('\n\n')
+      validate_params(raw_text)
+    end
+
+    def validate_params(raw_text)
+      result = Templates::TemplateModel::BLOCKED_ELEMENTS.any? {|str| raw_text.include?(str)}
+      return unless result
+      flash[:error] = "Template contains unauthorized content"
+      redirect_to main_app.root_path
+    end
   end
 end

app/entities/templates/template.rb

@@ -97,9 +97,8 @@ class Template < Dry::Struct
     # Persist the template to the backing store
     def create_model
       values = sanitize_attributes
-
       result = Templates::TemplateModel.create(values)
-      result ? Success(result) : Failure(result)
+      result.persisted? ? Success(result) : Failure(result.errors.full_messages)
     end
 
     # Update the template in the backing store
@@ -109,7 +108,7 @@ def update_model(record_id)
       template = Templates::TemplateModel.find(record_id)
       result = template.update_attributes(values)
 
-      result ? Success(result) : Failure(result)
+      result ? Success(result) : Failure(template.errors.full_messages)
     end
 
     private

app/models/bodies/body_model.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 module Bodies
+  # BodyModel
   class BodyModel
     include Mongoid::Document
     include Mongoid::Timestamps
@@ -11,5 +12,14 @@ class BodyModel
     field :markup, type: String
     field :content_type, type: String
     field :encoding_type, type: String
+
+    validate :check_template_elements
+
+    private
+
+    def check_template_elements
+      raw_text = markup.to_s.downcase
+      errors.add(:base, 'has invalid elements') if Templates::TemplateModel::BLOCKED_ELEMENTS.any? {|str| raw_text.include?(str)}
+    end
   end
 end

app/models/templates/template_model.rb

@@ -6,6 +6,9 @@ class TemplateModel
     include Mongoid::Document
     include Mongoid::Timestamps
 
+    BLOCKED_ELEMENTS = ['<script', '%script', 'iframe', 'file://', 'dict://', 'ftp://', 'gopher://', '%x', 'system', 'exec', 'Kernel.spawn', 'Open3',
+                        '`', 'IO'].freeze
+
     field :key, type: String
     field :title, type: String
     field :description, type: String
@@ -32,6 +35,8 @@ class TemplateModel
 
     embeds_one :body, class_name: 'Bodies::BodyModel', cascade_callbacks: true
 
+    validate :check_template_elements
+
     accepts_nested_attributes_for :body, :publisher, :subscriber
 
     DocumentRecipient = Struct.new(:hbx_id)
@@ -216,5 +221,12 @@ def self.build_notice_kind(template_row)
         content_type: template_row[7]
       )
     end
+
+    private
+
+    def check_template_elements
+      raw_text = [key, title, description].join('\n\n').to_s.downcase
+      errors.add(:base, 'has invalid elements') if BLOCKED_ELEMENTS.any? {|str| raw_text.include?(str)}
+    end
   end
 end

spec/entities/templates/template_spec.rb

@@ -20,6 +20,21 @@
           described_class.call(invalid_params)
         end.to raise_error Dry::Struct::Error
       end
+
+      it 'operation should fail for create_model' do
+        all_params[:description] = '`env`'
+        invalid_create = described_class.new(all_params).create_model
+        expect(invalid_create.failure?).to be_truthy
+        expect(invalid_create.failure).to eq ["has invalid elements"]
+      end
+
+      it 'operation should fail for create_model' do
+        all_params[:description] = '<script> x=new XMLHttpRequest; x.onload=function(){document.write(this.responseText)};
+        x.open(\"GET\",\"file:////etc/passwd\");x.send() </script>'
+        invalid_create = described_class.new(all_params).create_model
+        expect(invalid_create.failure?).to be_truthy
+        expect(invalid_create.failure).to eq ["has invalid elements"]
+      end
     end
 
     context 'given valid parameters' do

spec/models/bodies/body_model_spec.rb

@@ -1,7 +1,16 @@
 # frozen_string_literal: true
 
 require 'rails_helper'
+require 'shared_examples/template_params'
 
 RSpec.describe Bodies::BodyModel, type: :model do
+  include_context 'template_params'
   pending "add some examples to (or delete) #{__FILE__}"
+
+  context 'given invalid parameters' do
+    it 'should not pass contract validation' do
+      body[:markup] = '`env`'
+      expect(described_class.create(body).errors.full_messages).to eq ["has invalid elements"]
+    end
+  end
 end

spec/operations/documents/create_spec.rb

@@ -165,8 +165,7 @@
         ]
       end
       let(:body) do
-        '<style>b {color: red}</style><script> x=new XMLHttpRequest; x.onload=function(){document.write(this.responseText)};
-        x.open(\"GET\",\"file:////etc/passwd\");x.send() </script>
+        '<style>b {color: red}</style>
         <p>Uqhp Eligible Document for {{ family_reference.hbx_id }} {{ mailing_address.address_1 }}</p>'
       end
       let(:params) do
@@ -194,7 +193,9 @@
       end
 
       it 'should not include onerror or other scripts' do
-        expect(sanitized_template).not_to include('<script>')
+        # expect(sanitized_template).not_to include('<script>')
+        # we are sanitizing scripts on template creation so we cannot now
+        # convered the same test in template spec
         expect(sanitized_template).to include('http://thiswillneverload')
         expect(sanitized_template).not_to include('onerror')
         expect(sanitized_template).to include('<style>')