From 48fce933fec998722e669d7d33a62f3c0a10b05e Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Tue, 22 Mar 2016 16:31:56 -0400 Subject: [PATCH] 8767: Make offset work properly in ApiClientAuthorizationsController#index. Before this, #index was ignoring the "offset" request param and was not providing an "items_available" attribute in the response. This made Workbench's "get all pages" routine an infinite loop. --- .../api_client_authorizations_controller.rb | 33 ++++++++++----- ...i_client_authorizations_controller_test.rb | 40 +++++++++++++++---- 2 files changed, 56 insertions(+), 17 deletions(-) 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 56d0d85a82..83968be752 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 @@ -69,14 +69,27 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController val.is_a?(String) && (attr == 'uuid' || attr == 'api_token') } end - @objects = model_class. - includes(:user, :api_client). - 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 } + @objects = model_class.where('user_id=?', current_user.id) + if wanted_scopes.compact.any? + # We can't filter on scopes effectively using AR/postgres. + # Instead we get the entire result set, do our own filtering on + # scopes to get a list of UUIDs, then start a new query + # (restricted to the selected UUIDs) so super can apply the + # offset/limit/order params in the usual way. + @request_limit = @limit + @request_offset = @offset + @limit = @objects.count + @offset = 0 + super + wanted_scopes.compact.each do |scope_list| + sorted_scopes = scope_list.sort + @objects = @objects.select { |auth| auth.scopes.sort == sorted_scopes } + end + @limit = @request_limit + @offset = @request_offset + @objects = model_class.where('uuid in (?)', @objects.collect(&:uuid)) end + super end def find_object_by_uuid @@ -110,8 +123,10 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController # The @filters test here also prevents a non-trusted token from # filtering on its own scopes, and discovering whether any _other_ # equally scoped tokens exist (403=yes, 200=no). - if (@objects.andand.count == 1 and - @objects.first.uuid == current_api_client_authorization.andand.uuid and + return forbidden if !@objects + full_set = @objects.except(:limit).except(:offset) if @objects + if (full_set.count == 1 and + full_set.first.uuid == current_api_client_authorization.andand.uuid and (@filters.map(&:first) & %w(uuid api_token)).any?) return true 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 192e6b956d..9f0f555d55 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 @@ -38,9 +38,11 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes assert_response 403 end - def assert_found_tokens(auth, search_params, *expected_tokens) + def assert_found_tokens(auth, search_params, expected) authorize_with auth - expected_tokens.map! { |name| api_client_authorizations(name).api_token } + expected_tokens = expected.map do |name| + api_client_authorizations(name).api_token + end get :index, search_params assert_response :success got_tokens = JSON.parse(@response.body)['items'] @@ -52,19 +54,26 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes # 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], + [[: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| + [: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) + 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) + assert_found_tokens(auth, {filters: [['scopes', '=', scopes]]}, expected) + end + + test "#{auth.to_s} offset works with filter scopes=#{scopes.inspect}" do + assert_found_tokens(auth, { + offset: expected.length, + filters: [['scopes', '=', scopes]] + }, []) end end @@ -112,6 +121,20 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes assert_response expect_list_response if expect_list_items assert_equal assigns(:objects).length, expect_list_items + assert_equal json_response['items_available'], expect_list_items + end + end + + if expect_list_items + test "using '#{user}', list '#{token}' by uuid with offset" do + authorize_with user + get :index, { + filters: [['uuid','=',api_client_authorizations(token).uuid]], + offset: expect_list_items, + } + assert_response expect_list_response + assert_equal json_response['items_available'], expect_list_items + assert_equal json_response['items'].length, 0 end end @@ -123,6 +146,7 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes assert_response expect_list_response if expect_list_items assert_equal assigns(:objects).length, expect_list_items + assert_equal json_response['items_available'], expect_list_items end end end -- 2.30.2