From a19f34a2e948d1e8945ca7262abafeb632bd41ae Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Mon, 5 May 2014 10:54:53 -0400 Subject: [PATCH] workbench: Add initial reader tokens support. The API server recently gained support for reader tokens, which optionally supplement the user's permissions for read-only operations. This commit extends Workbench to pass on reader tokens for general index and show operations. --- .../app/controllers/application_controller.rb | 79 ++++++++++++++----- .../app/models/arvados_api_client.rb | 8 +- .../functional/collections_controller_test.rb | 44 +++++++++++ 3 files changed, 108 insertions(+), 23 deletions(-) diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb index d2b90c5476..f9de62d60e 100644 --- a/apps/workbench/app/controllers/application_controller.rb +++ b/apps/workbench/app/controllers/application_controller.rb @@ -1,12 +1,17 @@ class ApplicationController < ActionController::Base respond_to :html, :json, :js protect_from_forgery + + ERROR_ACTIONS = [:render_error, :render_not_found] + around_filter :thread_clear - around_filter :thread_with_mandatory_api_token, :except => [:render_exception, :render_not_found] + around_filter(:thread_with_mandatory_api_token, + except: [:index, :show] + ERROR_ACTIONS) around_filter :thread_with_optional_api_token - before_filter :find_object_by_uuid, :except => [:index, :render_exception, :render_not_found] - before_filter :check_user_agreements, :except => [:render_exception, :render_not_found] - before_filter :check_user_notifications, :except => [:render_exception, :render_not_found] + before_filter :check_user_agreements, except: ERROR_ACTIONS + before_filter :check_user_notifications, except: ERROR_ACTIONS + around_filter :using_reader_tokens, only: [:index, :show] + before_filter :find_object_by_uuid, except: [:index] + ERROR_ACTIONS theme :select_theme begin @@ -183,7 +188,56 @@ class ApplicationController < ActionController::Base end protected - + + def redirect_to_login + respond_to do |f| + f.html { + if request.method == 'GET' + redirect_to $arvados_api_client.arvados_login_url(return_to: request.url) + else + flash[:error] = "Either you are not logged in, or your session has timed out. I can't automatically log you in and re-attempt this request." + redirect_to :back + end + } + f.json { + @errors = ['You do not seem to be logged in. You did not supply an API token with this request, and your session (if any) has timed out.'] + self.render_error status: 422 + } + end + false # For convenience to return from callbacks + end + + def using_reader_tokens(login_optional=false) + if params[:reader_tokens].is_a?(Array) and params[:reader_tokens].any? + Thread.current[:reader_tokens] = params[:reader_tokens] + end + begin + yield + rescue ArvadosApiClient::NotLoggedInException + if login_optional + raise + else + return redirect_to_login + end + ensure + Thread.current[:reader_tokens] = nil + end + end + + def using_specific_api_token(api_token) + start_values = {} + [:arvados_api_token, :user].each do |key| + start_values[key] = Thread.current[key] + end + Thread.current[:arvados_api_token] = api_token + Thread.current[:user] = nil + begin + yield + ensure + start_values.each_key { |key| Thread.current[key] = start_values[key] } + end + end + def find_object_by_uuid if params[:id] and params[:id].match /\D/ params[:uuid] = params.delete :id @@ -242,20 +296,7 @@ class ApplicationController < ActionController::Base end if try_redirect_to_login unless login_optional - respond_to do |f| - f.html { - if request.method == 'GET' - redirect_to $arvados_api_client.arvados_login_url(return_to: request.url) - else - flash[:error] = "Either you are not logged in, or your session has timed out. I can't automatically log you in and re-attempt this request." - redirect_to :back - end - } - f.json { - @errors = ['You do not seem to be logged in. You did not supply an API token with this request, and your session (if any) has timed out.'] - self.render_error status: 422 - } - end + redirect_to_login else # login is optional for this route so go on to the regular controller Thread.current[:arvados_api_token] = nil diff --git a/apps/workbench/app/models/arvados_api_client.rb b/apps/workbench/app/models/arvados_api_client.rb index efe8b7eaf1..b6aeeca381 100644 --- a/apps/workbench/app/models/arvados_api_client.rb +++ b/apps/workbench/app/models/arvados_api_client.rb @@ -26,16 +26,16 @@ class ArvadosApiClient end end - api_token = Thread.current[:arvados_api_token] - api_token ||= '' - resources_kind = class_kind(resources_kind).pluralize if resources_kind.is_a? Class url = "#{self.arvados_v1_base}/#{resources_kind}#{action}" # Clean up /arvados/v1/../../discovery/v1 to /discovery/v1 url.sub! '/arvados/v1/../../', '/' - query = {"api_token" => api_token} + query = { + 'api_token' => Thread.current[:arvados_api_token] || '', + 'reader_tokens' => (Thread.current[:reader_tokens] || []).to_json, + } if !data.nil? data.each do |k,v| if v.is_a? String or v.nil? diff --git a/apps/workbench/test/functional/collections_controller_test.rb b/apps/workbench/test/functional/collections_controller_test.rb index d82ba6bb01..ea42b158eb 100644 --- a/apps/workbench/test/functional/collections_controller_test.rb +++ b/apps/workbench/test/functional/collections_controller_test.rb @@ -15,6 +15,24 @@ class CollectionsControllerTest < ActionController::TestCase [token, params[:uuid], params[:file]].join('/') end + def assert_hash_includes(actual_hash, expected_hash, msg=nil) + expected_hash.each do |key, value| + assert_equal(value, actual_hash[key], msg) + end + end + + def assert_no_session + assert_hash_includes(session, {arvados_api_token: nil}, + "session includes unexpected API token") + end + + def assert_session_for_auth(client_auth) + api_token = + api_fixture('api_client_authorizations')[client_auth.to_s]['api_token'] + assert_hash_includes(session, {arvados_api_token: api_token}, + "session token does not belong to #{client_auth}") + end + # Mock the collection file reader to avoid external calls and return # a predictable string. CollectionsController.class_eval do @@ -31,6 +49,32 @@ class CollectionsControllerTest < ActionController::TestCase assert_equal([['.', 'foo', 3]], assigns(:object).files) end + test "viewing a collection with a reader token" do + params = collection_params(:foo_file) + params[:reader_tokens] = + [api_fixture('api_client_authorizations')['active']['api_token']] + get(:show, params) + assert_response :success + assert_equal([['.', 'foo', 3]], assigns(:object).files) + assert_no_session + end + + test "viewing the index with a reader token" do + params = {reader_tokens: + [api_fixture('api_client_authorizations')['spectator']['api_token']] + } + get(:index, params) + assert_response :success + assert_no_session + listed_collections = assigns(:collections).map { |c| c.uuid } + assert_includes(listed_collections, + api_fixture('collections')['bar_file']['uuid'], + "spectator reader token didn't list bar file") + refute_includes(listed_collections, + api_fixture('collections')['foo_file']['uuid'], + "spectator reader token listed foo file") + end + test "getting a file from Keep" do params = collection_params(:foo_file, 'foo') sess = session_for(:active) -- 2.30.2