[Ovirt-devel] [PATCH] add permissions checks to search results.

Scott Seago sseago at redhat.com
Wed Aug 6 17:45:26 UTC 2008


To do so, I've enabled term-based parameters to each of the searchable types. At the query level, appending search_users:foo limits results to items viewable by user foo. This involved:

1) added :terms parameter to acts_as_xapian declaration for the method search_users
2) added search_users method to searchable models which return an array of usernames that have 'monitor' access
3) modified the acts_as_xapian plugin to handle prefix searches for which the object provides multiple values (since we have multiple users with access to each object) -- this is a change which may be suitable for upstream inclusion
4) When performing the search, search for "(#{terms}) AND search_users:#{user}" instead of simply searching for terms.

This patch is dependant on the prior search denormalization patch.

Signed-off-by: Scott Seago <sseago at redhat.com>
---
 wui/src/app/controllers/hardware_controller.rb     |    1 +
 wui/src/app/controllers/search_controller.rb       |   16 ++++++++++------
 wui/src/app/models/host.rb                         |    8 +++++++-
 wui/src/app/models/pool.rb                         |    8 +++++++-
 wui/src/app/models/storage_pool.rb                 |    6 +++++-
 wui/src/app/models/vm.rb                           |    8 +++++++-
 .../plugins/acts_as_xapian/lib/acts_as_xapian.rb   |    6 +++++-
 7 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/wui/src/app/controllers/hardware_controller.rb b/wui/src/app/controllers/hardware_controller.rb
index 5f473db..019fdd8 100644
--- a/wui/src/app/controllers/hardware_controller.rb
+++ b/wui/src/app/controllers/hardware_controller.rb
@@ -110,6 +110,7 @@ class HardwareController < ApplicationController
     unless @can_view
       flash[:notice] = 'You do not have permission to view this Hardware Pool: redirecting to top level'
       redirect_to :action => 'list'
+      return
     end
     render :layout => 'selection'    
   end
diff --git a/wui/src/app/controllers/search_controller.rb b/wui/src/app/controllers/search_controller.rb
index ca4fbb1..cc7e527 100644
--- a/wui/src/app/controllers/search_controller.rb
+++ b/wui/src/app/controllers/search_controller.rb
@@ -56,6 +56,11 @@ class SearchController < ApplicationController
       @models ||= [@model_param]
     end
     @user = get_login_user
+    #filter terms on permissions
+    filtered_terms = "search_users:#{@user}"
+    if @terms and !@terms.empty?
+      filtered_terms = "(#{@terms}) AND #{filtered_terms}"
+    end
 
     @page = params[:page].to_i
     @page ||= 1
@@ -63,12 +68,11 @@ class SearchController < ApplicationController
     @per_page ||= 20
     @offset = (@page-1)*@per_page
     @results = ActsAsXapian::Search.new(@models,
-                                        @terms,
-                                       :offset => @offset,
-                                       :limit => @per_page,
-                                       :sort_by_prefix => nil,
-                                       :collapse_by_prefix => nil)
-    #FIXME filter on permissions
+                                        filtered_terms,
+                                        :offset => @offset,
+                                        :limit => @per_page,
+                                        :sort_by_prefix => nil,
+                                        :collapse_by_prefix => nil)
   end
 
   def results
diff --git a/wui/src/app/models/host.rb b/wui/src/app/models/host.rb
index fc3b236..5b32653 100644
--- a/wui/src/app/models/host.rb
+++ b/wui/src/app/models/host.rb
@@ -43,7 +43,8 @@ class Host < ActiveRecord::Base
   acts_as_xapian :texts => [ :hostname, :uuid, :hypervisor_type, :arch ],
                  :values => [ [ :created_at, 0, "created_at", :date ],
                               [ :updated_at, 1, "updated_at", :date ] ],
-                 :terms => [ [ :hostname, 'H', "hostname" ] ]
+                 :terms => [ [ :hostname, 'H', "hostname" ],
+                             [ :search_users, 'U', "search_users" ] ]
 
 
   KVM_HYPERVISOR_TYPE = "KVM"
@@ -88,4 +89,9 @@ class Host < ActiveRecord::Base
   def display_class
     "Host"
   end
+
+  def search_users
+    hardware_pool.search_users
+  end
+
 end
diff --git a/wui/src/app/models/pool.rb b/wui/src/app/models/pool.rb
index 1870027..6599c72 100644
--- a/wui/src/app/models/pool.rb
+++ b/wui/src/app/models/pool.rb
@@ -85,7 +85,8 @@ class Pool < ActiveRecord::Base
     end
   end
 
-  acts_as_xapian :texts => [ :name ]
+  acts_as_xapian :texts => [ :name ],
+                 :terms => [ [ :search_users, 'U', "search_users" ] ]
 
   # this method lists pools with direct permission grants, but by default does
   #  not include implied permissions (i.e. subtrees)
@@ -246,6 +247,11 @@ class Pool < ActiveRecord::Base
   def display_class
     get_type_label
   end
+
+  def search_users
+    permissions.collect {|perm| perm.uid}
+  end
+
   protected
   def traverse_parents
     if id
diff --git a/wui/src/app/models/storage_pool.rb b/wui/src/app/models/storage_pool.rb
index 3e3f58f..460b49d 100644
--- a/wui/src/app/models/storage_pool.rb
+++ b/wui/src/app/models/storage_pool.rb
@@ -32,7 +32,8 @@ class StoragePool < ActiveRecord::Base
 
   validates_presence_of :ip_addr, :hardware_pool_id
 
-  acts_as_xapian :texts => [ :ip_addr, :target, :export_path, :type ]
+  acts_as_xapian :texts => [ :ip_addr, :target, :export_path, :type ],
+                 :terms => [ [ :search_users, 'U', "search_users" ] ]
   ISCSI = "iSCSI"
   NFS   = "NFS"
   STORAGE_TYPES = { ISCSI => "Iscsi",
@@ -60,4 +61,7 @@ class StoragePool < ActiveRecord::Base
     "Storage Pool"
   end
 
+  def search_users
+    hardware_pool.search_users
+  end
 end
diff --git a/wui/src/app/models/vm.rb b/wui/src/app/models/vm.rb
index 34d5bf4..9ac24d9 100644
--- a/wui/src/app/models/vm.rb
+++ b/wui/src/app/models/vm.rb
@@ -31,7 +31,8 @@ class Vm < ActiveRecord::Base
   validates_presence_of :uuid, :description, :num_vcpus_allocated,
                         :memory_allocated_in_mb, :memory_allocated, :vnic_mac_addr
 
-  acts_as_xapian :texts => [ :uuid, :description, :vnic_mac_addr, :state ]
+  acts_as_xapian :texts => [ :uuid, :description, :vnic_mac_addr, :state ],
+                 :terms => [ [ :search_users, 'U', "search_users" ] ]
 
   BOOT_DEV_HD          = "hd"
   BOOT_DEV_NETWORK     = "network"
@@ -193,6 +194,11 @@ class Vm < ActiveRecord::Base
   def display_class
     "VM"
   end
+
+  def search_users
+    vm_resource_pool.search_users
+  end
+
   protected
   def validate
     resources = vm_resource_pool.max_resources_for_vm(self)
diff --git a/wui/src/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb b/wui/src/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
index eda0b1b..566fe82 100644
--- a/wui/src/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
+++ b/wui/src/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
@@ -523,6 +523,8 @@ module ActsAsXapian
                 value.utc.strftime("%Y%m%d")
             elsif type == :boolean
                 value ? true : false
+            elsif type == :array
+                (value.class == Array) ? value.collect {|x| x.to_s} : value.to_s
             else
                 value.to_s
             end
@@ -549,7 +551,9 @@ module ActsAsXapian
             doc.add_term("I" + doc.data)
             if self.xapian_options[:terms]
               for term in self.xapian_options[:terms]
-                  doc.add_term(term[1] + xapian_value(term[0]))
+                  xapian_value(term[0], :array).each do |val|
+                    doc.add_term(term[1] + val)
+                  end
               end
             end
             if self.xapian_options[:values]
-- 
1.5.5.1




More information about the ovirt-devel mailing list