[ARVADOS] updated: d3d2311db3b942d41ef6746cb29d8a31a2ecbfa2
git at public.curoverse.com
git at public.curoverse.com
Fri Jun 27 14:46:00 EDT 2014
Summary of changes:
.../app/controllers/application_controller.rb | 147 ++++++++++-----------
.../app/controllers/collections_controller.rb | 12 +-
apps/workbench/app/models/arvados_base.rb | 2 +-
.../test/functional/users_controller_test.rb | 2 +-
4 files changed, 78 insertions(+), 85 deletions(-)
via d3d2311db3b942d41ef6746cb29d8a31a2ecbfa2 (commit)
via 6a62c302c86a35e8a03118ef5882d441f409a3bf (commit)
from 99832dd9fc3773bb3d0cf6dc0b1a2c79ef209fbb (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 d3d2311db3b942d41ef6746cb29d8a31a2ecbfa2
Author: Brett Smith <brett at curoverse.com>
Date: Fri Jun 27 14:37:34 2014 -0400
2891: Workbench reliably reloads model columns after failure.
ArvadosBase @columns could get stuck as [] if the API server wasn't
available during Workbench's first API request. This change ensures
that it keeps trying to reload column information until it receives
good data.
diff --git a/apps/workbench/app/models/arvados_base.rb b/apps/workbench/app/models/arvados_base.rb
index 3b5ac86..6d427fd 100644
--- a/apps/workbench/app/models/arvados_base.rb
+++ b/apps/workbench/app/models/arvados_base.rb
@@ -56,7 +56,7 @@ class ArvadosBase < ActiveRecord::Base
end
def self.columns
- return @columns unless @columns.nil?
+ return @columns if @columns.andand.any?
@columns = []
@attribute_info ||= {}
schema = arvados_api_client.discovery[:schemas][self.to_s.to_sym]
commit 6a62c302c86a35e8a03118ef5882d441f409a3bf
Author: Brett Smith <brett at curoverse.com>
Date: Fri Jun 27 14:30:38 2014 -0400
2891: Workbench renders login exceptions earlier.
These changes are designed to ensure that if there are any problems
getting information from the current API token (other than expected
401 Unauthorized responses), those problems are raised early and
propagated up to the exception handler. This helps better ensure that
thread state is consistent when we get to later stages of request
processing.
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index d2fe528..cb04fae 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -59,14 +59,14 @@ class ApplicationController < ActionController::Base
else
@errors = [e.to_s]
end
- if e.is_a? ArvadosApiClient::NotLoggedInException
- prep_token = :thread_clear
- else
- prep_token = :set_thread_api_token
- end
- send(prep_token) do
- render_error(err_opts)
+ # If the user has an active session, and the API server is available,
+ # make user information available on the error page.
+ begin
+ load_api_token(session[:arvados_api_token])
+ rescue ArvadosApiClient::ApiError
+ load_api_token(nil)
end
+ render_error(err_opts)
end
def render_not_found(e=ActionController::RoutingError.new("Path not found"))
@@ -271,26 +271,7 @@ class ApplicationController < ActionController::Base
end
def current_user
- return Thread.current[:user] if Thread.current[:user]
-
- if User.columns.empty?
- # We can't even get the discovery document from the API server.
- # We're not going to be able to instantiate any user object.
- nil
- elsif Thread.current[:arvados_api_token]
- if session[:user]
- if session[:user][:is_active] != true
- Thread.current[:user] = User.current
- else
- Thread.current[:user] = User.new(session[:user])
- end
- else
- Thread.current[:user] = User.current
- end
- else
- logger.error "No API token in Thread"
- return nil
- end
+ Thread.current[:user]
end
def model_class
@@ -340,8 +321,7 @@ class ApplicationController < ActionController::Base
[: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
+ load_api_token(api_token)
begin
yield
ensure
@@ -373,65 +353,87 @@ class ApplicationController < ActionController::Base
end
def thread_clear
- Thread.current[:arvados_api_token] = nil
- Thread.current[:user] = nil
+ load_api_token(nil)
Rails.cache.delete_matched(/^request_#{Thread.current.object_id}_/)
yield
Rails.cache.delete_matched(/^request_#{Thread.current.object_id}_/)
end
+ # Set up the thread with the given API token and associated user object.
+ def load_api_token(new_token)
+ Thread.current[:arvados_api_token] = new_token
+ if new_token.nil?
+ Thread.current[:user] = nil
+ elsif (new_token == session[:arvados_api_token]) and
+ session[:user].andand[:is_active]
+ Thread.current[:user] = User.new(session[:user])
+ else
+ Thread.current[:user] = User.current
+ end
+ end
+
+ # If there's a valid api_token parameter, set up the session with that
+ # user's information. Return true if the method redirects the request
+ # (usually a post-login redirect); false otherwise.
+ def setup_user_session
+ return false unless params[:api_token]
+ Thread.current[:arvados_api_token] = params[:api_token]
+ begin
+ user = User.current
+ rescue ArvadosApiClient::NotLoggedInException
+ false # We may redirect to login, or not, based on the current action.
+ else
+ session[:arvados_api_token] = params[:api_token]
+ session[:user] = {
+ uuid: user.uuid,
+ email: user.email,
+ first_name: user.first_name,
+ last_name: user.last_name,
+ is_active: user.is_active,
+ is_admin: user.is_admin,
+ prefs: user.prefs
+ }
+ if !request.format.json? and request.method.in? ['GET', 'HEAD']
+ # Repeat this request with api_token in the (new) session
+ # cookie instead of the query string. This prevents API
+ # tokens from appearing in (and being inadvisedly copied
+ # and pasted from) browser Location bars.
+ redirect_to strip_token_from_path(request.fullpath)
+ true
+ else
+ false
+ end
+ ensure
+ Thread.current[:arvados_api_token] = nil
+ end
+ end
+
# Save the session API token in thread-local storage, and yield.
# This method also takes care of session setup if the request
# provides a valid api_token parameter.
# If a token is unavailable or expired, the block is still run, with
# a nil token.
def set_thread_api_token
- # If an API token has already been found, pass it through.
if Thread.current[:arvados_api_token]
- yield
+ yield # An API token has already been found - pass it through.
return
+ elsif setup_user_session
+ return # A new session was set up and received a response.
end
begin
- # If there's a valid api_token parameter, use it to set up the session.
- if (Thread.current[:arvados_api_token] = params[:api_token]) and
- verify_api_token
- session[:arvados_api_token] = params[:api_token]
- u = User.current
- session[:user] = {
- uuid: u.uuid,
- email: u.email,
- first_name: u.first_name,
- last_name: u.last_name,
- is_active: u.is_active,
- is_admin: u.is_admin,
- prefs: u.prefs
- }
- if !request.format.json? and request.method.in? ['GET', 'HEAD']
- # Repeat this request with api_token in the (new) session
- # cookie instead of the query string. This prevents API
- # tokens from appearing in (and being inadvisedly copied
- # and pasted from) browser Location bars.
- redirect_to strip_token_from_path(request.fullpath)
- return
- end
- end
-
- # With setup done, handle the request using the session token.
- Thread.current[:arvados_api_token] = session[:arvados_api_token]
- begin
+ load_api_token(session[:arvados_api_token])
+ yield
+ rescue ArvadosApiClient::NotLoggedInException
+ # If we got this error with a token, it must've expired.
+ # Retry the request without a token.
+ unless Thread.current[:arvados_api_token].nil?
+ load_api_token(nil)
yield
- rescue ArvadosApiClient::NotLoggedInException
- # If we got this error with a token, it must've expired.
- # Retry the request without a token.
- unless Thread.current[:arvados_api_token].nil?
- Thread.current[:arvados_api_token] = nil
- yield
- end
end
ensure
# Remove token in case this Thread is used for anything else.
- Thread.current[:arvados_api_token] = nil
+ load_api_token(nil)
end
end
@@ -450,15 +452,6 @@ class ApplicationController < ActionController::Base
end
end
- def verify_api_token
- begin
- Link.where(uuid: 'just-verifying-my-api-token')
- true
- rescue ArvadosApiClient::NotLoggedInException
- false
- end
- end
-
def ensure_current_user_is_admin
unless current_user and current_user.is_admin
@errors = ['Permission denied']
diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index 7556f3c..dd1ac2f 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -238,14 +238,14 @@ class CollectionsController < ApplicationController
# error we encounter, and return nil.
most_specific_error = [401]
token_list.each do |api_token|
- using_specific_api_token(api_token) do
- begin
+ begin
+ using_specific_api_token(api_token) do
yield
return api_token
- rescue ArvadosApiClient::ApiError => error
- if error.api_status >= most_specific_error.first
- most_specific_error = [error.api_status, error]
- end
+ end
+ rescue ArvadosApiClient::ApiError => error
+ if error.api_status >= most_specific_error.first
+ most_specific_error = [error.api_status, error]
end
end
end
diff --git a/apps/workbench/test/functional/users_controller_test.rb b/apps/workbench/test/functional/users_controller_test.rb
index bf21a26..86778df 100644
--- a/apps/workbench/test/functional/users_controller_test.rb
+++ b/apps/workbench/test/functional/users_controller_test.rb
@@ -7,7 +7,7 @@ class UsersControllerTest < ActionController::TestCase
end
test "ignore previously valid token (for deleted user), don't crash" do
- get :welcome, {}, session_for(:valid_token_deleted_user)
+ get :activity, {}, session_for(:valid_token_deleted_user)
assert_response :redirect
assert_match /^#{Rails.configuration.arvados_login_base}/, @response.redirect_url
assert_nil assigns(:my_jobs)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list