[ARVADOS] updated: 1d3987356d0ab60c4f6c80cbd11852b4209137f4
git at public.curoverse.com
git at public.curoverse.com
Thu Nov 6 15:50:11 EST 2014
Summary of changes:
apps/workbench/app/controllers/application_controller.rb | 9 +++++++--
apps/workbench/app/controllers/collections_controller.rb | 6 +++++-
.../test/functional/collections_controller_test.rb | 16 ++++++++++++++--
apps/workbench/test/integration/collections_test.rb | 11 ++++++++---
4 files changed, 34 insertions(+), 8 deletions(-)
via 1d3987356d0ab60c4f6c80cbd11852b4209137f4 (commit)
via c53b0474744b0e2e863c7aa3c4a148152f4b0e63 (commit)
from 308a6da1a9fd716f3957b116110a932c08aefafe (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 1d3987356d0ab60c4f6c80cbd11852b4209137f4
Merge: 308a6da c53b047
Author: Brett Smith <brett at curoverse.com>
Date: Thu Nov 6 15:49:32 2014 -0500
Merge branch '4408-collection-sharing-login-fix-wip'
Closes #4408.
commit c53b0474744b0e2e863c7aa3c4a148152f4b0e63
Author: Brett Smith <brett at curoverse.com>
Date: Thu Nov 6 11:59:00 2014 -0500
4408: Workbench Collection sharing allows file downloads again.
Earlier we refactored our API token loading code to load User.current
to check that it was valid. This approach doesn't work when we're
presenting shared Collections, because the token usually won't be
scoped to get user information. Skip the check for this specific
case.
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index c3fd58a..751b494 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -399,12 +399,17 @@ class ApplicationController < ActionController::Base
false # For convenience to return from callbacks
end
- def using_specific_api_token(api_token)
+ def using_specific_api_token(api_token, opts={})
start_values = {}
[:arvados_api_token, :user].each do |key|
start_values[key] = Thread.current[key]
end
- load_api_token(api_token)
+ if opts.fetch(:load_user, true)
+ load_api_token(api_token)
+ else
+ Thread.current[:arvados_api_token] = api_token
+ Thread.current[:user] = nil
+ end
begin
yield
ensure
diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index e869824..5ddf93c 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -1,3 +1,5 @@
+require "arvados/keep"
+
class CollectionsController < ApplicationController
include ActionController::Live
@@ -298,7 +300,9 @@ class CollectionsController < ApplicationController
most_specific_error = [401]
token_list.each do |api_token|
begin
- using_specific_api_token(api_token) do
+ # We can't load the corresponding user, because the token may not
+ # be scoped for that.
+ using_specific_api_token(api_token, load_user: false) do
yield
return api_token
end
diff --git a/apps/workbench/test/functional/collections_controller_test.rb b/apps/workbench/test/functional/collections_controller_test.rb
index e621010..6c64ac9 100644
--- a/apps/workbench/test/functional/collections_controller_test.rb
+++ b/apps/workbench/test/functional/collections_controller_test.rb
@@ -86,14 +86,26 @@ 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']['api_token']
+ 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)
assert_no_session
end
+ test "fetching collection file with reader token" do
+ expected = stub_file_content
+ 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")
+ assert_no_session
+ end
+
test "reader token Collection links end with trailing slash" do
# Testing the fix for #2937.
session = session_for(:active_trustedclient)
diff --git a/apps/workbench/test/integration/collections_test.rb b/apps/workbench/test/integration/collections_test.rb
index 9eb16da..f4fc4cb 100644
--- a/apps/workbench/test/integration/collections_test.rb
+++ b/apps/workbench/test/integration/collections_test.rb
@@ -45,6 +45,8 @@ class CollectionsTest < ActionDispatch::IntegrationTest
end
test "can download an entire collection with a reader token" do
+ CollectionsController.any_instance.
+ stubs(:file_enumerator).returns(["foo\n", "file\n"])
uuid = api_fixture('collections')['foo_file']['uuid']
token = api_fixture('api_client_authorizations')['active_all_collections']['api_token']
url_head = "/collections/download/#{uuid}/#{token}/"
@@ -53,9 +55,8 @@ class CollectionsTest < ActionDispatch::IntegrationTest
# a very blunt approach.
assert_no_match(/<\s*meta[^>]+\bnofollow\b/i, page.html,
"wget prohibited from recursing the collection page")
- # TODO: When we can test against a Keep server, actually follow links
- # and check their contents, rather than testing the href directly
- # (this is too closely tied to implementation details).
+ # Look at all the links that wget would recurse through using our
+ # recommended options, and check that it's exactly the file list.
hrefs = page.all('a').map do |anchor|
link = anchor[:href] || ''
if link.start_with? url_head
@@ -68,6 +69,10 @@ class CollectionsTest < ActionDispatch::IntegrationTest
end
assert_equal(['foo'], hrefs.compact.sort,
"download page did provide strictly file links")
+ within "#collection_files" do
+ click_link "foo"
+ assert_equal("foo\nfile\n", page.html)
+ end
end
test "can view empty collection" do
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list