From 33f55eed348769e1d14b283ec36b8f1bfc2d3c98 Mon Sep 17 00:00:00 2001 From: elijah Date: Wed, 3 Jul 2013 11:21:04 -0700 Subject: fixed security vulnerability with ticket searching --- help/app/models/ticket.rb | 5 +---- help/app/models/ticket_selection.rb | 41 ++++++++++++++++++++++++++----------- 2 files changed, 30 insertions(+), 16 deletions(-) (limited to 'help') 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 -- cgit v1.2.3