From 7e8f99556391cc81c014b517a9fa6efed8fe8113 Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Mon, 28 Apr 2014 13:01:18 -0400 Subject: [PATCH] api: Support filters in API client auths index. 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. --- .../app/controllers/application_controller.rb | 6 ++- .../api_client_authorizations_controller.rb | 35 ++++++++++----- ...i_client_authorizations_controller_test.rb | 43 ++++++++++++------- 3 files changed, 56 insertions(+), 28 deletions(-) diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb index 9674bb784c..73d14050d5 100644 --- a/services/api/app/controllers/application_controller.rb +++ b/services/api/app/controllers/application_controller.rb @@ -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 diff --git a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb index ff322a7fb8..dc95b2f01d 100644 --- a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb +++ b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb @@ -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 diff --git a/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb b/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb index 0072792563..8877719b5b 100644 --- a/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb +++ b/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb @@ -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 -- 2.30.2