Add server-side validation to rules

lib/discourse_chat/rule.rb

@@ -6,9 +6,30 @@
     attr_accessor :id, :provider, :channel, :category_id, :tags, :filter
 
     def initialize(h={})
+      @provider = ''
+      @channel = ''
+      @category_id = nil
+      @tags = []
+      @filter = 'watch'
       h.each {|k,v| public_send("#{k}=",v)}
     end
 
+    def tags=(array)
+      if array.nil? or array.empty?
+        @tags = nil
+      else
+        @tags = array
+      end
+    end
+
+    def category_id=(val)
+      if val.nil? or val.empty?
+        @category_id = nil
+      else
+        @category_id = val.to_i
+      end
+    end
+
     # saving/loading functions
     def self.alloc_id
       DistributedMutex.synchronize('discourse-chat_rule-id') do
@@ -59,6 +80,8 @@
     end
 
     def save
+      return false if not valid?
+
       unless @id && @id > 0
         @id = self.class.alloc_id
       end
@@ -66,6 +89,37 @@
       return self
     end
 
+    def save!
+      if not save
+        raise 'Rule not valid'
+      end
+      return self
+    end
+
+    def valid?
+      
+      # Validate provider
+      return false if not ::DiscourseChat::Provider.providers.map {|x| x::PROVIDER_NAME}.include? @provider
+      
+      # Validate channel
+      return false if @channel.blank?
+      
+      # Validate category
+      return false if not (@category_id.nil? or Category.where(id: @category_id).exists?)
+
+      # Validate tags
+      if not @tags.nil?
+        @tags.each do |tag|
+          return false if not Tag.where(name: tag).exists?
+        end
+      end
+
+      # Validate filter
+      return false if not ['watch','follow','mute'].include? @filter
+
+      return true
+    end
+
     def destroy
       DiscourseChat.pstore_delete "rule:#{id}"
     end

plugin.rb

@@ -86,21 +86,33 @@ after_initialize do
     end
 
     def create_rule
-      rule = DiscourseChat::Rule.new()
+      begin
-      hash = params.require(:rule)
+        rule = DiscourseChat::Rule.new()
+        hash = params.require(:rule)
 
-      rule.update(hash)
+        if not rule.update(hash)
+          raise Discourse::InvalidParameters, 'Rule is not valid'
+        end
 
-      render_serialized rule, DiscourseChat::RuleSerializer, root: 'rule'
+        render_serialized rule, DiscourseChat::RuleSerializer, root: 'rule'
+      rescue Discourse::InvalidParameters => e
+        render json: {errors: [e.message]}, status: 422
+      end
     end
 
     def update_rule
-      rule = DiscourseChat::Rule.find(params[:id].to_i)
+      begin
-      hash = params.require(:rule)
+        rule = DiscourseChat::Rule.find(params[:id].to_i)
+        hash = params.require(:rule)
 
-      rule.update(hash)
+        if not rule.update(hash)
+          raise Discourse::InvalidParameters, 'Rule is not valid'
+        end
 
-      render_serialized rule, DiscourseChat::RuleSerializer, root: 'rule'
+        render_serialized rule, DiscourseChat::RuleSerializer, root: 'rule'
+      rescue Discourse::InvalidParameters => e
+        render json: {errors: [e.message]}, status: 422
+      end
     end
 
     def destroy_rule

spec/lib/discourse_chat/manager_spec.rb

@@ -37,7 +37,7 @@ RSpec.describe DiscourseChat::Manager do
     let(:provider) {::DiscourseChat::Provider::DummyProvider}
 
     def create_rule(provider, channel, filter, category_id, tags) # Just shorthand for testing purposes
-      DiscourseChat::Rule.new({provider: provider, channel: channel, filter:filter, category_id:category_id, tags:tags}).save
+      DiscourseChat::Rule.new({provider: provider, channel: channel, filter:filter, category_id:category_id, tags:tags}).save!
     end
 
     it "should only send notifications when provider is enabled" do

spec/lib/discourse_chat/rule_spec.rb

@@ -1,6 +1,11 @@
 require 'rails_helper'
 
+
+
 RSpec.describe DiscourseChat::Rule do
+  let(:tag1){Fabricate(:tag)}
+  let(:tag2){Fabricate(:tag)}
+
   describe '.alloc_id' do
     it 'should return sequential numbers' do 
       expect( DiscourseChat::Rule.alloc_id ).to eq(1)
@@ -15,10 +20,10 @@ RSpec.describe DiscourseChat::Rule do
     rule = DiscourseChat::Rule.new({
           provider:"slack",
           channel: "#general",
-          category_id: 2,
+          category_id: 1,
-          tags: ['hello', 'world'],
+          tags: [tag1.name, tag2.name],
           filter: 'watch'
-        }).save
+        }).save!
 
     expect(DiscourseChat::Rule.all.length).to eq(1)
 
@@ -26,8 +31,8 @@ RSpec.describe DiscourseChat::Rule do
 
     expect(loadedRule.provider).to eq('slack')
     expect(loadedRule.channel).to eq('#general')
-    expect(loadedRule.category_id).to eq(2)
+    expect(loadedRule.category_id).to eq(1)
-    expect(loadedRule.tags).to contain_exactly('hello','world')
+    expect(loadedRule.tags).to contain_exactly(tag1.name,tag2.name)
     expect(loadedRule.filter).to eq('watch')
 
   end
@@ -37,23 +42,23 @@ RSpec.describe DiscourseChat::Rule do
       rule = DiscourseChat::Rule.new({
           provider:"slack",
           channel: "#general",
-          category_id: 2,
+          category_id: 1,
-          tags: ['hello', 'world']
+          tags: [tag1.name, tag2.name]
-        }).save
+        }).save!
     end
 
     it 'can be modified' do
       rule = DiscourseChat::Rule.all.first
       rule.channel = "#random"
 
-      rule.save
+      rule.save!
 
       rule = DiscourseChat::Rule.all.first
       expect(rule.channel).to eq('#random')
     end
 
     it 'can be deleted' do
-      DiscourseChat::Rule.new.save
+      DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save!
       expect(DiscourseChat::Rule.all.length).to eq(2)
 
       rule = DiscourseChat::Rule.all.first
@@ -63,10 +68,10 @@ RSpec.describe DiscourseChat::Rule do
     end
 
     it 'can delete all' do
-      DiscourseChat::Rule.new.save
+      DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save!
-      DiscourseChat::Rule.new.save
+      DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save!
-      DiscourseChat::Rule.new.save
+      DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save!
-      DiscourseChat::Rule.new.save
+      DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save!
       
       expect(DiscourseChat::Rule.all.length).to eq(5)
 
@@ -76,8 +81,8 @@ RSpec.describe DiscourseChat::Rule do
     end
 
     it 'can be filtered by provider' do
-      rule2 = DiscourseChat::Rule.new({provider:'telegram'}).save
+      rule2 = DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save!
-      rule3 = DiscourseChat::Rule.new({provider:'slack'}).save
+      rule3 = DiscourseChat::Rule.new({provider:'slack', channel:'blah'}).save!
 
       expect(DiscourseChat::Rule.all.length).to eq(3)
 
@@ -86,16 +91,73 @@ RSpec.describe DiscourseChat::Rule do
     end
 
     it 'can be filtered by category' do
-      rule2 = DiscourseChat::Rule.new({category_id: 1}).save      
+      rule2 = DiscourseChat::Rule.new({provider:'slack', channel:'blah', category_id: 1}).save!      
-      rule3 = DiscourseChat::Rule.new({category_id: nil}).save
+      rule3 = DiscourseChat::Rule.new({provider:'slack', channel:'blah', category_id: nil}).save!
 
       expect(DiscourseChat::Rule.all.length).to eq(3)
 
-      expect(DiscourseChat::Rule.all_for_category(2).length).to eq(1)
+      expect(DiscourseChat::Rule.all_for_category(1).length).to eq(2)
-      expect(DiscourseChat::Rule.all_for_category(1).length).to eq(1)
       expect(DiscourseChat::Rule.all_for_category(nil).length).to eq(1)
     end
+  end
+
+  describe 'validations' do
+    
+    let(:rule) do
+      DiscourseChat::Rule.new({
+          filter: 'watch',
+          provider:"slack",
+          channel: "#general",
+          category_id: 1,
+        }).save!
+    end
+
+    it 'validates provider correctly' do
+      expect(rule.valid?).to eq(true)
+      rule.provider = 'somerandomprovider'
+      expect(rule.valid?).to eq(false)
+    end
+
+    it 'validates channel correctly' do
+      expect(rule.valid?).to eq(true)
+      rule.channel = ''
+      expect(rule.valid?).to eq(false)
+    end
+
+    it 'validates category correctly' do
+      expect(rule.valid?).to eq(true)
+      rule.category_id = 99
+      expect(rule.valid?).to eq(false)
+    end
+
+    it 'validates filter correctly' do
+      expect(rule.valid?).to eq(true)
+      rule.filter = 'follow'
+      expect(rule.valid?).to eq(true)
+      rule.filter = 'mute'
+      expect(rule.valid?).to eq(true)
+      rule.filter = ''
+      expect(rule.valid?).to eq(false)
+      rule.filter = 'somerandomstring'
+      expect(rule.valid?).to eq(false)
+    end
+
+    it 'validates tags correctly' do
+      expect(rule.valid?).to eq(true)
+      rule.tags = []
+      expect(rule.valid?).to eq(true)
+      rule.tags = [tag1.name]
+      expect(rule.valid?).to eq(true)
+      rule.tags = [tag1.name, 'blah']
+      expect(rule.valid?).to eq(false)
+    end
+
+    it "doesn't allow save when invalid" do
+      expect(rule.valid?).to eq(true)
+      rule.provider = 'somerandomprovider'
+      expect(rule.valid?).to eq(false)
+      expect(rule.save).to eq(false)
+    end
 
   end
-
 end