api: Support filters in API client auths index.
authorBrett Smith <brett@curoverse.com>
Mon, 28 Apr 2014 17:01:18 +0000 (13:01 -0400)
committerTom Clegg <tom@curoverse.com>
Tue, 29 Apr 2014 00:12:47 +0000 (20:12 -0400)
Per comments on Refs #1904.  filters is generally the preferred way to
do searching now.  I maintained existing limits on what can be
searched with this method.

services/api/app/controllers/application_controller.rb
services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb

index 9674bb784cb0b6b55cbdb126efcd8551bba21b33..73d14050d5cb054e9ad5ca8d0f5cad773057afdf 100644 (file)
@@ -208,6 +208,10 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  def default_orders
+    ["#{table_name}.modified_at desc"]
+  end
+
   def load_limit_offset_order_params
     if params[:limit]
       unless params[:limit].to_s.match(/^\d+$/)
@@ -240,7 +244,7 @@ class ApplicationController < ActionController::Base
       end
     end
     if @orders.empty?
-      @orders << "#{table_name}.modified_at desc"
+      @orders = default_orders
     end
   end
 
index ff322a7fb8981741a37f64c23c3f22ec8a12056a..dc95b2f01da12c00645226f60824468a73eac6ed 100644 (file)
@@ -34,21 +34,34 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
 
   protected
 
+  def default_orders
+    ["#{table_name}.created_at desc"]
+  end
+
   def find_objects_for_index
     # Here we are deliberately less helpful about searching for client
-    # authorizations. Rather than use the generic index/where/order
-    # features, we look up tokens belonging to the current user and
-    # filter by exact match on api_token (which we expect in the form
-    # of a where[uuid] parameter to make things easier for API client
-    # libraries).
+    # authorizations.  We look up tokens belonging to the current user
+    # and filter by exact matches on api_token and scopes.
+    wanted_scopes = []
+    if @filters
+      wanted_scopes.concat(@filters.map { |attr, operator, operand|
+        ((attr == 'scopes') and (operator == '=')) ? operand : nil
+      })
+      @filters.select! { |attr, operator, operand|
+        (attr == 'uuid') and (operator == '=')
+      }
+    end
+    if @where
+      wanted_scopes << @where['scopes']
+      @where.select! { |attr, val| attr == 'uuid' }
+    end
     @objects = model_class.
       includes(:user, :api_client).
-      where('user_id=? and (? or api_token=?)', current_user.id, !@where['uuid'], @where['uuid']).
-      order('created_at desc')
-    unless @where['scopes'].nil?
-      @objects = @objects.select { |auth|
-        (auth.scopes & @where['scopes']) == (auth.scopes | @where['scopes'])
-      }
+      where('user_id=?', current_user.id)
+    super
+    wanted_scopes.compact.each do |scope_list|
+      sorted_scopes = scope_list.sort
+      @objects = @objects.select { |auth| auth.scopes.sort == sorted_scopes }
     end
   end
 
index 0072792563fa20fdae7a4957a8c6fae8aa9948e6..8877719b5bd613673581a9b118c5cf3ae9e41f9e 100644 (file)
@@ -37,22 +37,33 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes
     assert_response 403
   end
 
-  test "admin search filters where scopes exactly match" do
-    def check_tokens_by_scopes(scopes, *expected_tokens)
-      expected_tokens.map! { |name| api_client_authorizations(name).api_token }
-      get :index, where: {scopes: scopes}
-      assert_response :success
-      got_tokens = JSON.parse(@response.body)['items']
-        .map { |auth| auth['api_token'] }
-      assert_equal(expected_tokens.sort, got_tokens.sort,
-                   "wrong results for scopes = #{scopes}")
+  def assert_found_tokens(auth, search_params, *expected_tokens)
+    authorize_with auth
+    expected_tokens.map! { |name| api_client_authorizations(name).api_token }
+    get :index, search_params
+    assert_response :success
+    got_tokens = JSON.parse(@response.body)['items']
+      .map { |auth| auth['api_token'] }
+    assert_equal(expected_tokens.sort, got_tokens.sort,
+                 "wrong results for #{search_params.inspect}")
+  end
+
+  # Three-tuples with auth to use, scopes to find, and expected tokens.
+  # Make two tests for each tuple, one searching with where and the other
+  # with filter.
+  [[:admin_trustedclient, [], :admin_noscope],
+   [:active_trustedclient, ["GET /arvados/v1/users"], :active_userlist],
+   [:active_trustedclient,
+    ["POST /arvados/v1/api_client_authorizations",
+     "GET /arvados/v1/api_client_authorizations"],
+    :active_apitokens],
+  ].each do |auth, scopes, *expected|
+    test "#{auth.to_s} can find auths where scopes=#{scopes.inspect}" do
+      assert_found_tokens(auth, {where: {scopes: scopes}}, *expected)
+    end
+
+    test "#{auth.to_s} can find auths filtered with scopes=#{scopes.inspect}" do
+      assert_found_tokens(auth, {filters: [['scopes', '=', scopes]]}, *expected)
     end
-    authorize_with :admin_trustedclient
-    check_tokens_by_scopes([], :admin_noscope)
-    authorize_with :active_trustedclient
-    check_tokens_by_scopes(["GET /arvados/v1/users"], :active_userlist)
-    check_tokens_by_scopes(["POST /arvados/v1/api_client_authorizations",
-                            "GET /arvados/v1/api_client_authorizations"],
-                           :active_apitokens)
   end
 end