[ARVADOS] updated: 76dd22c7c1b577ef22df59e5cc12ff0d659a3aa1
git at public.curoverse.com
git at public.curoverse.com
Mon Jun 23 11:13:30 EDT 2014
Summary of changes:
.../app/controllers/application_controller.rb | 130 +++++++++------------
.../app/controllers/collections_controller.rb | 2 +-
.../app/controllers/sessions_controller.rb | 4 +-
3 files changed, 59 insertions(+), 77 deletions(-)
via 76dd22c7c1b577ef22df59e5cc12ff0d659a3aa1 (commit)
via a0ea42d48bcde4128c9dcd7d278ef3628f36f536 (commit)
from 3a8ef07d64112657ce8a8ee5fad39368b8a35c74 (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 76dd22c7c1b577ef22df59e5cc12ff0d659a3aa1
Author: Brett Smith <brett at curoverse.com>
Date: Mon Jun 23 11:14:29 2014 -0400
2891: Rename and comment Workbench API token filters.
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index 0f4b137..f215eac 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -8,8 +8,10 @@ class ApplicationController < ActionController::Base
ERROR_ACTIONS = [:render_error, :render_not_found]
around_filter :thread_clear
- around_filter :thread_with_api_token
- around_filter :thread_with_mandatory_api_token, except: ERROR_ACTIONS
+ around_filter :set_thread_api_token
+ # Methods that don't require login should
+ # skip_around_filter :require_thread_api_token
+ around_filter :require_thread_api_token, except: ERROR_ACTIONS
before_filter :check_user_agreements, except: ERROR_ACTIONS
before_filter :check_user_notifications, except: ERROR_ACTIONS
before_filter :find_object_by_uuid, except: [:index, :choose] + ERROR_ACTIONS
@@ -58,7 +60,7 @@ class ApplicationController < ActionController::Base
if e.is_a? ArvadosApiClient::NotLoggedInException
self.render_error status: 422
else
- thread_with_api_token do
+ set_thread_api_token do
self.render_error status: 422
end
end
@@ -67,7 +69,7 @@ class ApplicationController < ActionController::Base
def render_not_found(e=ActionController::RoutingError.new("Path not found"))
logger.error e.inspect
@errors = ["Path not found"]
- thread_with_api_token do
+ set_thread_api_token do
self.render_error status: 404
end
end
@@ -371,7 +373,12 @@ class ApplicationController < ActionController::Base
Rails.cache.delete_matched(/^request_#{Thread.current.object_id}_/)
end
- def thread_with_api_token
+ # 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
@@ -420,7 +427,8 @@ class ApplicationController < ActionController::Base
end
end
- def thread_with_mandatory_api_token
+ # Reroute this request if an API token is unavailable.
+ def require_thread_api_token
if Thread.current[:arvados_api_token]
yield
elsif session[:arvados_api_token]
diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index 9179848..95aee92 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -1,5 +1,5 @@
class CollectionsController < ApplicationController
- skip_around_filter(:thread_with_mandatory_api_token,
+ skip_around_filter(:require_thread_api_token,
only: [:show_file, :show_file_links])
skip_before_filter(:find_object_by_uuid,
only: [:provenance, :show_file, :show_file_links])
diff --git a/apps/workbench/app/controllers/sessions_controller.rb b/apps/workbench/app/controllers/sessions_controller.rb
index 9cd1e1c..97c8d5a 100644
--- a/apps/workbench/app/controllers/sessions_controller.rb
+++ b/apps/workbench/app/controllers/sessions_controller.rb
@@ -1,6 +1,6 @@
class SessionsController < ApplicationController
- skip_around_filter :thread_with_mandatory_api_token, :only => [:destroy, :index]
- skip_around_filter :thread_with_api_token, :only => [:destroy, :index]
+ skip_around_filter :require_thread_api_token, :only => [:destroy, :index]
+ skip_around_filter :set_thread_api_token, :only => [:destroy, :index]
skip_before_filter :find_object_by_uuid, :only => [:destroy, :index]
def destroy
commit a0ea42d48bcde4128c9dcd7d278ef3628f36f536
Author: Brett Smith <brett at curoverse.com>
Date: Mon Jun 23 10:43:02 2014 -0400
2891: Refactor thread_with_api_token and friends.
Remove dead code and simplify the implementation, especially the
relationship between the "required" and "optional" filters, per
discussion with Peter.
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index af1d73f..0f4b137 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -8,8 +8,8 @@ class ApplicationController < ActionController::Base
ERROR_ACTIONS = [:render_error, :render_not_found]
around_filter :thread_clear
+ around_filter :thread_with_api_token
around_filter :thread_with_mandatory_api_token, except: ERROR_ACTIONS
- around_filter :thread_with_optional_api_token
before_filter :check_user_agreements, except: ERROR_ACTIONS
before_filter :check_user_notifications, except: ERROR_ACTIONS
before_filter :find_object_by_uuid, except: [:index, :choose] + ERROR_ACTIONS
@@ -58,7 +58,7 @@ class ApplicationController < ActionController::Base
if e.is_a? ArvadosApiClient::NotLoggedInException
self.render_error status: 422
else
- thread_with_optional_api_token do
+ thread_with_api_token do
self.render_error status: 422
end
end
@@ -67,7 +67,7 @@ class ApplicationController < ActionController::Base
def render_not_found(e=ActionController::RoutingError.new("Path not found"))
logger.error e.inspect
@errors = ["Path not found"]
- thread_with_optional_api_token do
+ thread_with_api_token do
self.render_error status: 404
end
end
@@ -371,56 +371,45 @@ class ApplicationController < ActionController::Base
Rails.cache.delete_matched(/^request_#{Thread.current.object_id}_/)
end
- def thread_with_api_token(login_optional = false)
+ def thread_with_api_token
+ # If an API token has already been found, pass it through.
+ if Thread.current[:arvados_api_token]
+ yield
+ return
+ end
+
begin
- try_redirect_to_login = true
- if params[:api_token]
- Thread.current[:arvados_api_token] = params[:api_token]
- # Before copying the token into session[], do a simple API
- # call to verify its authenticity.
- if verify_api_token
- try_redirect_to_login = false
- 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)
- else
- yield
- end
- end
- elsif session[:arvados_api_token]
- # In this case, the token must have already verified at some
- # point, but it might have been revoked since. We'll try
- # using it, and catch the exception if it doesn't work.
- Thread.current[:arvados_api_token] = session[:arvados_api_token]
- begin
- yield
- rescue ArvadosApiClient::NotLoggedInException
- # We'll try to redirect to login later.
- else
- try_redirect_to_login = false
+ # 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)
end
- else
- logger.debug "No token received, session is #{session.inspect}"
end
- if try_redirect_to_login
- unless login_optional
- redirect_to_login
- else
- # login is optional for this route so go on to the regular controller
+
+ # With setup done, handle the request using the session token.
+ Thread.current[:arvados_api_token] = session[:arvados_api_token]
+ begin
+ 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
@@ -432,31 +421,16 @@ class ApplicationController < ActionController::Base
end
def thread_with_mandatory_api_token
- thread_with_api_token(true) do
- if Thread.current[:arvados_api_token]
- yield
- elsif session[:arvados_api_token]
- # Expired session. Clear it before refreshing login so that,
- # if this login procedure fails, we end up showing the "please
- # log in" page instead of getting stuck in a redirect loop.
- session.delete :arvados_api_token
- redirect_to_login
- else
- render 'users/welcome'
- end
- end
- end
-
- # This runs after thread_with_mandatory_api_token in the filter chain.
- def thread_with_optional_api_token
if Thread.current[:arvados_api_token]
- # We are already inside thread_with_mandatory_api_token.
yield
+ elsif session[:arvados_api_token]
+ # Expired session. Clear it before refreshing login so that,
+ # if this login procedure fails, we end up showing the "please
+ # log in" page instead of getting stuck in a redirect loop.
+ session.delete :arvados_api_token
+ redirect_to_login
else
- # We skipped thread_with_mandatory_api_token. Use the optional version.
- thread_with_api_token(true) do
- yield
- end
+ render 'users/welcome'
end
end
diff --git a/apps/workbench/app/controllers/sessions_controller.rb b/apps/workbench/app/controllers/sessions_controller.rb
index 591373b..9cd1e1c 100644
--- a/apps/workbench/app/controllers/sessions_controller.rb
+++ b/apps/workbench/app/controllers/sessions_controller.rb
@@ -1,6 +1,6 @@
class SessionsController < ApplicationController
skip_around_filter :thread_with_mandatory_api_token, :only => [:destroy, :index]
- skip_around_filter :thread_with_optional_api_token, :only => [:destroy, :index]
+ skip_around_filter :thread_with_api_token, :only => [:destroy, :index]
skip_before_filter :find_object_by_uuid, :only => [:destroy, :index]
def destroy
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list