X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/425f97db9a84540d608b7717b8df3befce687c8f..668a799436295a17223096392898cf257d8dfc76:/apps/workbench/test/controllers/collections_controller_test.rb diff --git a/apps/workbench/test/controllers/collections_controller_test.rb b/apps/workbench/test/controllers/collections_controller_test.rb index 14db674292..88287cd3f3 100644 --- a/apps/workbench/test/controllers/collections_controller_test.rb +++ b/apps/workbench/test/controllers/collections_controller_test.rb @@ -1,3 +1,7 @@ +# Copyright (C) The Arvados Authors. All rights reserved. +# +# SPDX-License-Identifier: AGPL-3.0 + require 'test_helper' class CollectionsControllerTest < ActionController::TestCase @@ -10,13 +14,13 @@ class CollectionsControllerTest < ActionController::TestCase NONEXISTENT_COLLECTION = "ffffffffffffffffffffffffffffffff+0" - def stub_file_content - # For the duration of the current test case, stub file download - # content with a randomized (but recognizable) string. Return the - # string, the test case can use it in assertions. - txt = 'the quick brown fox ' + rand(2**32).to_s - @controller.stubs(:file_enumerator).returns([txt]) - txt + def config_anonymous enable + Rails.configuration.anonymous_user_token = + if enable + api_token('anonymous') + else + false + end end def collection_params(collection_name, file_name=nil) @@ -28,7 +32,11 @@ class CollectionsControllerTest < ActionController::TestCase def assert_hash_includes(actual_hash, expected_hash, msg=nil) expected_hash.each do |key, value| - assert_equal(value, actual_hash[key], msg) + if value.nil? + assert_nil(actual_hash[key], msg) + else + assert_equal(value, actual_hash[key], msg) + end end end @@ -39,7 +47,7 @@ class CollectionsControllerTest < ActionController::TestCase def assert_session_for_auth(client_auth) api_token = - api_fixture('api_client_authorizations')[client_auth.to_s]['api_token'] + self.api_token(client_auth.to_s) assert_hash_includes(session, {arvados_api_token: api_token}, "session token does not belong to #{client_auth}") end @@ -47,7 +55,7 @@ class CollectionsControllerTest < ActionController::TestCase def show_collection(params, session={}, response=:success) params = collection_params(params) if not params.is_a? Hash session = session_for(session) if not session.is_a? Hash - get(:show, params, session) + get(:show, params: params, session: session) assert_response response end @@ -56,6 +64,22 @@ class CollectionsControllerTest < ActionController::TestCase assert_equal([['.', 'foo', 3]], assigns(:object).files) end + test "viewing a collection with spaces in filename" do + show_collection(:w_a_z_file, :active) + assert_equal([['.', 'w a z', 5]], assigns(:object).files) + end + + test "download a file with spaces in filename" do + setup_for_keep_web + collection = api_fixture('collections')['w_a_z_file'] + get :show_file, params: { + uuid: collection['uuid'], + file: 'w a z' + }, session: session_for(:active) + assert_response :redirect + assert_match /w%20a%20z/, response.redirect_url + end + test "viewing a collection fetches related projects" do show_collection({id: api_fixture('collections')["foo_file"]['portable_data_hash']}, :active) assert_includes(assigns(:same_pdh).map(&:owner_uuid), @@ -84,13 +108,6 @@ class CollectionsControllerTest < ActionController::TestCase "controller did not find logger job") end - test "viewing a collection fetches logs about it" do - show_collection(:foo_file, :active) - assert_includes(assigns(:logs).map(&:uuid), - api_fixture('logs')['log4']['uuid'], - "controller did not find related log") - end - test "sharing auths available to admin" do show_collection("collection_owned_by_active", "admin_trustedclient") assert_not_nil assigns(:search_sharing) @@ -109,107 +126,112 @@ class CollectionsControllerTest < ActionController::TestCase test "viewing collection files with a reader token" do params = collection_params(:foo_file) - params[:reader_token] = api_fixture("api_client_authorizations", - "active_all_collections", "api_token") - get(:show_file_links, params) - assert_response :success - assert_equal([['.', 'foo', 3]], assigns(:object).files) + params[:reader_token] = api_token("active_all_collections") + get(:show_file_links, params: params) + assert_response :redirect assert_no_session end test "fetching collection file with reader token" do - expected = stub_file_content + setup_for_keep_web params = collection_params(:foo_file, "foo") - params[:reader_token] = api_fixture("api_client_authorizations", - "active_all_collections", "api_token") - get(:show_file, params) - assert_response :success - assert_equal(expected, @response.body, - "failed to fetch a Collection file with a reader token") + params[:reader_token] = api_token("active_all_collections") + get(:show_file, params: params) + assert_response :redirect + assert_match /foo/, response.redirect_url assert_no_session end test "reader token Collection links end with trailing slash" do # Testing the fix for #2937. session = session_for(:active_trustedclient) - post(:share, collection_params(:foo_file), session) + post(:share, params: collection_params(:foo_file), session: session) assert(@controller.download_link.ends_with? '/', "Collection share link does not end with slash for wget") end test "getting a file from Keep" do + setup_for_keep_web params = collection_params(:foo_file, 'foo') sess = session_for(:active) - expect_content = stub_file_content - get(:show_file, params, sess) - assert_response :success - assert_equal(expect_content, @response.body, - "failed to get a correct file from Keep") + get(:show_file, params: params, session: sess) + assert_response :redirect + assert_match /foo/, response.redirect_url + end + + test 'anonymous download' do + setup_for_keep_web + config_anonymous true + get :show_file, params: { + uuid: api_fixture('collections')['user_agreement_in_anonymously_accessible_project']['uuid'], + file: 'GNU_General_Public_License,_version_3.pdf', + } + assert_response :redirect + assert_match /GNU_General_Public_License/, response.redirect_url end test "can't get a file from Keep without permission" do params = collection_params(:foo_file, 'foo') sess = session_for(:spectator) - get(:show_file, params, sess) - assert_response 404 - end - - test "trying to get a nonexistent file from Keep returns a 404" do - params = collection_params(:foo_file, 'gone') - sess = session_for(:admin) - get(:show_file, params, sess) + get(:show_file, params: params, session: sess) assert_response 404 end test "getting a file from Keep with a good reader token" do + setup_for_keep_web params = collection_params(:foo_file, 'foo') - read_token = api_fixture('api_client_authorizations')['active']['api_token'] + read_token = api_token('active') params[:reader_token] = read_token - expect_content = stub_file_content - get(:show_file, params) - assert_response :success - assert_equal(expect_content, @response.body, - "failed to get a correct file from Keep using a reader token") + get(:show_file, params: params) + assert_response :redirect + assert_match /foo/, response.redirect_url assert_not_equal(read_token, session[:arvados_api_token], "using a reader token set the session's API token") end - test "trying to get from Keep with an unscoped reader token prompts login" do - params = collection_params(:foo_file, 'foo') - params[:reader_token] = - api_fixture('api_client_authorizations')['active_noscope']['api_token'] - get(:show_file, params) - assert_response :redirect + [false, true].each do |anon| + test "download a file using a reader token with insufficient scope, anon #{anon}" do + config_anonymous anon + params = collection_params(:foo_file, 'foo') + params[:reader_token] = + api_token('active_noscope') + get(:show_file, params: params) + if anon + # Some files can be shown without a valid token, but not this one. + assert_response 404 + else + # No files will ever be shown without a valid token. You + # should log in and try again. + assert_response :redirect + end + end end test "can get a file with an unpermissioned auth but in-scope reader token" do + setup_for_keep_web params = collection_params(:foo_file, 'foo') sess = session_for(:expired) - read_token = api_fixture('api_client_authorizations')['active']['api_token'] + read_token = api_token('active') params[:reader_token] = read_token - expect_content = stub_file_content - get(:show_file, params, sess) - assert_response :success - assert_equal(expect_content, @response.body, - "failed to get a correct file from Keep using a reader token") + get(:show_file, params: params, session: sess) + assert_response :redirect assert_not_equal(read_token, session[:arvados_api_token], "using a reader token set the session's API token") end test "inactive user can retrieve user agreement" do + setup_for_keep_web ua_collection = api_fixture('collections')['user_agreement'] # Here we don't test whether the agreement can be retrieved from - # Keep. We only test that show_file decides to send file content, - # so we use the file content stub. - stub_file_content - get :show_file, { + # Keep. We only test that show_file decides to send file content. + get :show_file, params: { uuid: ua_collection['uuid'], file: ua_collection['manifest_text'].match(/ \d+:\d+:(\S+)/)[1] - }, session_for(:inactive) + }, session: session_for(:inactive) assert_nil(assigns(:unsigned_user_agreements), "Did not skip check_user_agreements filter " + "when showing the user agreement.") - assert_response :success + assert_response :redirect end test "requesting nonexistent Collection returns 404" do @@ -217,37 +239,12 @@ class CollectionsControllerTest < ActionController::TestCase :active, 404) end - test "use a reasonable read buffer even if client requests a huge range" do - fakefiledata = mock - IO.expects(:popen).returns(fakefiledata) - fakefiledata.expects(:read).twice.with() do |length| - # Fail the test if read() is called with length>1MiB: - length < 2**20 - ## Force the ActionController::Live thread to lose the race to - ## verify that @response.body.length actually waits for the - ## response (see below): - # sleep 3 - end.returns("foo\n", nil) - fakefiledata.expects(:close) - foo_file = api_fixture('collections')['foo_file'] - @request.headers['Range'] = 'bytes=0-4294967296/*' - get :show_file, { - uuid: foo_file['uuid'], - file: foo_file['manifest_text'].match(/ \d+:\d+:(\S+)/)[1] - }, session_for(:active) - # Wait for the whole response to arrive before deciding whether - # mocks' expectations were met. Otherwise, Mocha will fail the - # test depending on how slowly the ActionController::Live thread - # runs. - @response.body.length - end - test "show file in a subdirectory of a collection" do + setup_for_keep_web params = collection_params(:collection_with_files_in_subdir, 'subdir2/subdir3/subdir4/file1_in_subdir4.txt') - expect_content = stub_file_content - get(:show_file, params, session_for(:user1_with_load)) - assert_response :success - assert_equal(expect_content, @response.body, "failed to get a correct file from Keep") + get(:show_file, params: params, session: session_for(:user1_with_load)) + assert_response :redirect + assert_match /subdir2\/subdir3\/subdir4\/file1_in_subdir4\.txt/, response.redirect_url end test 'provenance graph' do @@ -313,4 +310,436 @@ class CollectionsControllerTest < ActionController::TestCase assert /#{stage3_id}->#{stage3_out}/.match(used_by_svg) end + + test "view collection with empty properties" do + fixture_name = :collection_with_empty_properties + show_collection(fixture_name, :active) + assert_equal(api_fixture('collections')[fixture_name.to_s]['name'], assigns(:object).name) + assert_not_nil(assigns(:object).properties) + assert_empty(assigns(:object).properties) + end + + test "view collection with one property" do + fixture_name = :collection_with_one_property + show_collection(fixture_name, :active) + fixture = api_fixture('collections')[fixture_name.to_s] + assert_equal(fixture['name'], assigns(:object).name) + assert_equal(fixture['properties'].values[0], assigns(:object).properties.values[0]) + end + + test "create collection with properties" do + post :create, params: { + collection: { + name: 'collection created with properties', + manifest_text: '', + properties: { + property_1: 'value_1' + }, + }, + format: :json + }, session: session_for(:active) + assert_response :success + assert_not_nil assigns(:object).uuid + assert_equal 'collection created with properties', assigns(:object).name + assert_equal 'value_1', assigns(:object).properties[:property_1] + end + + test "update description and check manifest_text is not lost" do + collection = api_fixture("collections")["multilevel_collection_1"] + post :update, params: { + id: collection["uuid"], + collection: { + description: 'test description update' + }, + format: :json + }, session: session_for(:active) + assert_response :success + assert_not_nil assigns(:object) + # Ensure the Workbench response still has the original manifest_text + assert_equal 'test description update', assigns(:object).description + assert_equal true, strip_signatures_and_compare(collection['manifest_text'], assigns(:object).manifest_text) + # Ensure the API server still has the original manifest_text after + # we called arvados.v1.collections.update + use_token :active do + assert_equal true, strip_signatures_and_compare(Collection.find(collection['uuid']).manifest_text, + collection['manifest_text']) + end + end + + # Since we got the initial collection from fixture, there are no signatures in manifest_text. + # However, after update or find, the collection retrieved will have singed manifest_text. + # Hence, let's compare each line after excluding signatures. + def strip_signatures_and_compare m1, m2 + m1_lines = m1.split "\n" + m2_lines = m2.split "\n" + + return false if m1_lines.size != m2_lines.size + + m1_lines.each_with_index do |line, i| + m1_words = [] + line.split.each do |word| + m1_words << word.split('+A')[0] + end + m2_words = [] + m2_lines[i].split.each do |word| + m2_words << word.split('+A')[0] + end + return false if !m1_words.join(' ').eql?(m2_words.join(' ')) + end + + return true + end + + test "view collection and verify none of the file types listed are disabled" do + show_collection(:collection_with_several_supported_file_types, :active) + + files = assigns(:object).files + assert_equal true, files.length>0, "Expected one or more files in collection" + + disabled = css_select('[disabled="disabled"]').collect do |el| + el + end + assert_equal 0, disabled.length, "Expected no disabled files in collection viewables list" + end + + test "view collection and verify file types listed are all disabled" do + show_collection(:collection_with_several_unsupported_file_types, :active) + + files = assigns(:object).files.collect do |_, file, _| + file + end + assert_equal true, files.length>0, "Expected one or more files in collection" + + disabled = css_select('[disabled="disabled"]').collect do |el| + el.attributes['title'].value.split[-1] + end + + assert_equal files.sort, disabled.sort, "Expected to see all collection files in disabled list of files" + end + + test "anonymous user accesses collection in shared project" do + config_anonymous true + collection = api_fixture('collections')['public_text_file'] + get(:show, params: {id: collection['uuid']}) + + response_object = assigns(:object) + assert_equal collection['name'], response_object['name'] + assert_equal collection['uuid'], response_object['uuid'] + assert_includes @response.body, 'Hello world' + assert_includes @response.body, 'Content address' + refute_nil css_select('[href="#Advanced"]') + end + + test "can view empty collection" do + get :show, params: {id: 'd41d8cd98f00b204e9800998ecf8427e+0'}, session: session_for(:active) + assert_includes @response.body, 'The following collections have this content' + end + + test "collection portable data hash redirect" do + di = api_fixture('collections')['docker_image'] + get :show, params: {id: di['portable_data_hash']}, session: session_for(:active) + assert_match /\/collections\/#{di['uuid']}/, @response.redirect_url + end + + test "collection portable data hash with multiple matches" do + pdh = api_fixture('collections')['foo_file']['portable_data_hash'] + get :show, params: {id: pdh}, session: session_for(:admin) + matches = api_fixture('collections').select {|k,v| v["portable_data_hash"] == pdh} + assert matches.size > 1 + + matches.each do |k,v| + assert_match /href="\/collections\/#{v['uuid']}">.*#{v['name']}<\/a>/, @response.body + end + + assert_includes @response.body, 'The following collections have this content:' + assert_not_includes @response.body, 'more results are not shown' + assert_not_includes @response.body, 'Activity' + assert_not_includes @response.body, 'Sharing and permissions' + end + + test "collection page renders name" do + collection = api_fixture('collections')['foo_file'] + get :show, params: {id: collection['uuid']}, session: session_for(:active) + assert_includes @response.body, collection['name'] + assert_match /not authorized to manage collection sharing links/, @response.body + end + + test "No Upload tab on non-writable collection" do + get :show, + params: {id: api_fixture('collections')['user_agreement']['uuid']}, + session: session_for(:active) + assert_not_includes @response.body, ' 'file1renamed' + }, + format: :json + }, session: session_for(:active) + assert_response :success + + use_token :active + collection = Collection.select([:uuid, :manifest_text]).where(uuid: collection['uuid']).first + assert_match /. d41d8cd98f00b204e9800998ecf8427e\+0\+A(.*) 0:0:file1renamed 0:0:file2\n.\/dir1 d41d8cd98f00b204e9800998ecf8427e\+0\+A(.*) 0:0:dir1file1 0:0:dir1file2 0:0:dir1imagefile.png\n$/, collection['manifest_text'] + + # now rename 'file2' such that it is moved into 'dir1' + @test_counter = 0 + post :update, params: { + id: collection['uuid'], + collection: { + 'rename-file-path:file2' => 'dir1/file2' + }, + format: :json + }, session: session_for(:active) + assert_response :success + + use_token :active + collection = Collection.select([:uuid, :manifest_text]).where(uuid: collection['uuid']).first + assert_match /. d41d8cd98f00b204e9800998ecf8427e\+0\+A(.*) 0:0:file1renamed\n.\/dir1 d41d8cd98f00b204e9800998ecf8427e\+0\+A(.*) 0:0:dir1file1 0:0:dir1file2 0:0:dir1imagefile.png 0:0:file2\n$/, collection['manifest_text'] + + # now rename 'dir1/dir1file1' such that it is moved into a new subdir + @test_counter = 0 + post :update, params: { + id: collection['uuid'], + collection: { + 'rename-file-path:dir1/dir1file1' => 'dir2/dir3/dir1file1moved' + }, + format: :json + }, session: session_for(:active) + assert_response :success + + use_token :active + collection = Collection.select([:uuid, :manifest_text]).where(uuid: collection['uuid']).first + assert_match /. d41d8cd98f00b204e9800998ecf8427e\+0\+A(.*) 0:0:file1renamed\n.\/dir1 d41d8cd98f00b204e9800998ecf8427e\+0\+A(.*) 0:0:dir1file2 0:0:dir1imagefile.png 0:0:file2\n.\/dir2\/dir3 d41d8cd98f00b204e9800998ecf8427e\+0\+A(.*) 0:0:dir1file1moved\n$/, collection['manifest_text'] + + # now rename the image file 'dir1/dir1imagefile.png' + @test_counter = 0 + post :update, params: { + id: collection['uuid'], + collection: { + 'rename-file-path:dir1/dir1imagefile.png' => 'dir1/dir1imagefilerenamed.png' + }, + format: :json + }, session: session_for(:active) + assert_response :success + + use_token :active + collection = Collection.select([:uuid, :manifest_text]).where(uuid: collection['uuid']).first + assert_match /. d41d8cd98f00b204e9800998ecf8427e\+0\+A(.*) 0:0:file1renamed\n.\/dir1 d41d8cd98f00b204e9800998ecf8427e\+0\+A(.*) 0:0:dir1file2 0:0:dir1imagefilerenamed.png 0:0:file2\n.\/dir2\/dir3 d41d8cd98f00b204e9800998ecf8427e\+0\+A(.*) 0:0:dir1file1moved\n$/, collection['manifest_text'] + end + + test "renaming file with a duplicate name in same stream not allowed" do + use_token :active + + # rename 'file2' as 'file1' and expect error + post :update, params: { + id: 'zzzzz-4zz18-pyw8yp9g3pr7irn', + collection: { + 'rename-file-path:file2' => 'file1' + }, + format: :json + }, session: session_for(:active) + assert_response 422 + assert_includes json_response['errors'], 'Duplicate file path' + end + + test "renaming file with a duplicate name as another stream not allowed" do + use_token :active + + # rename 'file1' as 'dir1/file1' and expect error + post :update, params: { + id: 'zzzzz-4zz18-pyw8yp9g3pr7irn', + collection: { + 'rename-file-path:file1' => 'dir1/file1' + }, + format: :json + }, session: session_for(:active) + assert_response 422 + assert_includes json_response['errors'], 'Duplicate file path' + end end