fixed security vulnerability with ticket searching

2 files changed, 30 insertions, 16 deletions

diff --git a/help/app/models/ticket.rb b/help/app/models/ticket.rb
index 09bc64d..8066d0d 100644
--- a/help/app/models/ticket.rb
+++ b/help/app/models/ticket.rb
@@ -35,10 +35,7 @@ class Ticket < CouchRest::Model::Base
   validates :title, :presence => true
   validates :email, :allow_blank => true, :format => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/
 
-  def self.for_user(user, options = {}, is_admin = false)
-    options[:user_id] = user.id
-    options[:is_admin] = is_admin
-
+  def self.search(options = {})
     @selection = TicketSelection.new(options)
     @selection.tickets
   end
diff --git a/help/app/models/ticket_selection.rb b/help/app/models/ticket_selection.rb
index bebe5fc..74d5b78 100644
--- a/help/app/models/ticket_selection.rb
+++ b/help/app/models/ticket_selection.rb
@@ -1,10 +1,20 @@
 class TicketSelection
 
+  #
+  # supported options:
+  #
+  # user_id:      id of the user (uuid string)
+  # open_status:  open | closed | all
+  # sort_order:   updated_at_desc | updated_at_asc | created_at_desc | created_at_asc
+  # admin_status: mine | all
+  # is_admin:     true | false
+  #
   def initialize(options = {})
-    @options = options
-    @options[:open_status] ||= 'open'
-    @options[:sort_order] ||= 'updated_at_desc'
-
+    @user_id      = options[:user_id].gsub /[^a-z0-9]/, ''
+    @open_status  = allow options[:open_status],  'open', 'closed', 'all'
+    @sort_order   = allow options[:sort_order],   'updated_at_desc', 'updated_at_asc', 'created_at_desc', 'created_at_asc'
+    @admin_status = allow options[:admin_status], 'mine', 'all'
+    @is_admin     = allow options[:is_admin],     false, true
   end
 
   def tickets
@@ -13,25 +23,32 @@ class TicketSelection
 
   protected
 
+  def allow(source, *allowed)
+    if allowed.include?(source)
+      source
+    else
+      allowed.first
+    end
+  end
 
   def finder_method
     method = 'by_'
     method += 'includes_post_by_and_' if only_mine?
-    method += 'is_open_and_' if @options[:open_status] != 'all'
-    method += @options[:sort_order].sub(/_(de|a)sc$/, '')
+    method += 'is_open_and_' if @open_status != 'all'
+    method += @sort_order.sub(/_(de|a)sc$/, '')
   end
 
   def startkey
     startkeys = []
-    startkeys << @options[:user_id] if only_mine?
-    startkeys << (@options[:open_status] == 'open') if @options[:open_status] != 'all'
+    startkeys << @user_id if only_mine?
+    startkeys << (@open_status == 'open') if @open_status != 'all'
     startkeys << 0
-    startkeys = startkeys.join if startkeys.length == 1 #want string not array if just one thing in array
+    startkeys = startkeys.join if startkeys.length == 1 # want string not array if just one thing in array
     startkeys
   end
 
   def endkey
-    endtime = Time.now + 2.days #TODO. this obviously isn't ideal
+    endtime = Time.now + 2.days # TODO. this obviously isn't ideal
     if self.startkey.is_a?(Array)
       endkeys = self.startkey
       endkeys.pop
@@ -43,12 +60,12 @@ class TicketSelection
 
   def order
     # we have defined the ascending method to return the view itself:
-    (@options[:sort_order].end_with? 'desc') ? 'descending' : 'ascending'
+    (@sort_order.end_with? 'desc') ? 'descending' : 'ascending'
   end
 
 
   def only_mine?
-    !@options[:is_admin] or (@options[:admin_status] == 'mine')
+    !@is_admin || @admin_status == 'mine'
   end
 
 end