[ARVADOS] created: 130511ce33222d8c4d5bea163c54e76fbf1c4e59
git at public.curoverse.com
git at public.curoverse.com
Thu Mar 5 17:46:54 EST 2015
at 130511ce33222d8c4d5bea163c54e76fbf1c4e59 (commit)
commit 130511ce33222d8c4d5bea163c54e76fbf1c4e59
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Mar 5 17:46:17 2015 -0500
5105: Treat not-logged-in AJAX requests as errors, instead of redirecting to login prompts.
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index b52591b..e078641 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -441,19 +441,14 @@ class ApplicationController < ActionController::Base
end
def redirect_to_login
- respond_to do |f|
- f.html {
- if request.method.in? ['GET', 'HEAD']
- redirect_to arvados_api_client.arvados_login_url(return_to: strip_token_from_path(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
- }
+ if request.xhr? or request.format.json?
+ @errors = ['You are not logged in. Most likely your session has timed out and you need to log in again.']
+ render_error status: 401
+ elsif request.method.in? ['GET', 'HEAD']
+ redirect_to arvados_api_client.arvados_login_url(return_to: strip_token_from_path(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
false # For convenience to return from callbacks
end
@@ -594,7 +589,8 @@ class ApplicationController < ActionController::Base
end
end
- # Redirect to login/welcome if client provided expired API token (or none at all)
+ # Redirect to login/welcome if client provided expired API token (or
+ # none at all)
def require_thread_api_token
if Thread.current[:arvados_api_token]
yield
@@ -604,15 +600,26 @@ class ApplicationController < ActionController::Base
# log in" page instead of getting stuck in a redirect loop.
session.delete :arvados_api_token
redirect_to_login
+ elsif request.xhr?
+ # If we redirect to the welcome page, the browser will handle
+ # the 302 by itself and the client code will end up rendering
+ # the "welcome" page in some content area where it doesn't make
+ # sense. Instead, we send 401 ("authenticate and try again" or
+ # "display error", depending on how smart the client side is).
+ @errors = ['You are not logged in.']
+ render_error status: 401
else
redirect_to welcome_users_path(return_to: request.fullpath)
end
end
def ensure_current_user_is_admin
- unless current_user and current_user.is_admin
+ if not current_user
+ @errors = ['Not logged in']
+ render_error status: 401
+ elsif not current_user.is_admin
@errors = ['Permission denied']
- self.render_error status: 401
+ render_error status: 403
end
end
diff --git a/apps/workbench/test/integration/ajax_errors_test.rb b/apps/workbench/test/integration/ajax_errors_test.rb
new file mode 100644
index 0000000..de2dfc7
--- /dev/null
+++ b/apps/workbench/test/integration/ajax_errors_test.rb
@@ -0,0 +1,45 @@
+require 'integration_helper'
+
+class AjaxErrorsTest < ActionDispatch::IntegrationTest
+ setup do
+ # Regrettably...
+ need_selenium 'to assert_text in iframe'
+ end
+
+ test 'load pane with deleted session' do
+ # Simulate loading a page in browser-tab A, hitting "Log out" in
+ # browser-tab B, then returning to browser-tab A and choosing a
+ # different tab. (Automatic tab refreshes will behave similarly.)
+ visit page_with_token('active', '/projects/' + api_fixture('groups')['aproject']['uuid'])
+ ActionDispatch::Request::Session.any_instance.stubs(:[]).returns(nil)
+ click_link "Subprojects"
+ wait_for_ajax
+ assert_no_selector '.container-fluid .container-fluid'
+ assert_no_text 'If you have never used'
+ assert_text 'Reload tab'
+ page.driver.browser.switch_to.frame 0
+ assert_text 'You are not logged in.'
+ end
+
+ test 'load pane with expired token' do
+ # Similar to 'deleted session'. Here, the session cookie is still
+ # alive, but it contains a token which has expired. This uses a
+ # different code path because Workbench cannot detect that
+ # anything is amiss until it actually uses the token in an API
+ # request.
+ visit page_with_token('active', '/projects/' + api_fixture('groups')['aproject']['uuid'])
+ use_token :active_trustedclient do
+ # Go behind Workbench's back to expire the "active" token.
+ token = api_fixture('api_client_authorizations')['active']['api_token']
+ auth = ApiClientAuthorization.find(token)
+ auth.update_attributes(expires_at: '1999-12-31T23:59:59Z')
+ end
+ click_link "Subprojects"
+ wait_for_ajax
+ assert_no_selector '.container-fluid .container-fluid'
+ assert_no_text 'If you have never used'
+ assert_text 'Reload tab'
+ page.driver.browser.switch_to.frame 0
+ assert_text 'You are not logged in.'
+ end
+end
diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index 335fcd0..c2ff207 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -32,8 +32,13 @@ class ActiveSupport::TestCase
# in integration tests -- they do not yet inherit this setting
fixtures :all
def use_token token_name
+ was = Thread.current[:arvados_api_token]
auth = api_fixture('api_client_authorizations')[token_name.to_s]
Thread.current[:arvados_api_token] = auth['api_token']
+ if block_given?
+ yield
+ Thread.current[:arvados_api_token] = was
+ end
end
setup do
commit 2b8a615808ba5fd0dc7cb80451622e28ab6e51a7
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Mar 5 17:20:32 2015 -0500
5105: Move displaced comment.
diff --git a/apps/workbench/app/assets/javascripts/tab_panes.js b/apps/workbench/app/assets/javascripts/tab_panes.js
index 6565ea9..c67772d 100644
--- a/apps/workbench/app/assets/javascripts/tab_panes.js
+++ b/apps/workbench/app/assets/javascripts/tab_panes.js
@@ -106,8 +106,8 @@ $(document).on('arv:pane:reload', '[data-pane-content-url]', function(e) {
var content_url = $pane.attr('data-pane-content-url');
$.ajax(content_url, {dataType: 'html', type: 'GET', context: $pane}).
done(function(data, status, jqxhr) {
- // Preserve collapsed state
var $pane = this;
+ // Preserve collapsed state
var collapsable = {};
$(".collapse", this).each(function(i, c) {
collapsable[c.id] = $(c).hasClass('in');
commit 317782960f7d53f57ac4c186140d1a753089c3c3
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Mar 5 14:39:27 2015 -0500
5105: Add API stub helpers and basic ArvadosApiClient unit tests.
diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index 078190b..335fcd0 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -86,6 +86,32 @@ module ApiFixtureLoader
end
end
+module ApiMockHelpers
+ def self.included base
+ base.class_eval do
+ def stub_api_calls_with_body body, status_code=200
+ resp = mock
+ stubbed_client = ArvadosApiClient.new
+ stubbed_client.instance_eval do
+ resp.responds_like_instance_of HTTP::Message
+ resp.stubs(:content).returns body
+ resp.stubs(:status_code).returns status_code
+ @api_client = HTTPClient.new
+ @api_client.stubs(:post).returns resp
+ end
+ ArvadosApiClient.stubs(:new_or_current).returns(stubbed_client)
+ end
+
+ def stub_api_calls_with_invalid_json
+ stub_api_calls_with_body ']"omg,bogus"['
+ end
+ end
+ end
+end
+class ActiveSupport::TestCase
+ include ApiMockHelpers
+end
+
class ActiveSupport::TestCase
include ApiFixtureLoader
def session_for api_client_auth_name
diff --git a/apps/workbench/test/unit/arvados_api_client_test.rb b/apps/workbench/test/unit/arvados_api_client_test.rb
new file mode 100644
index 0000000..faaa7e5
--- /dev/null
+++ b/apps/workbench/test/unit/arvados_api_client_test.rb
@@ -0,0 +1,20 @@
+require 'test_helper'
+
+class ArvadosApiClientTest < ActiveSupport::TestCase
+ test 'successful stubbed api request' do
+ stub_api_calls_with_body '{"foo":"bar","baz":0}'
+ use_token :active
+ resp = ArvadosApiClient.new_or_current.api Link, ''
+ assert_equal Hash, resp.class
+ assert_equal 'bar', resp['foo']
+ assert_equal 0, resp['baz']
+ end
+
+ test 'exception if server returns non-JSON' do
+ stub_api_calls_with_invalid_json
+ assert_raises ArvadosApiClient::InvalidApiResponseException do
+ use_token :active
+ resp = ArvadosApiClient.new_or_current.api Link, ''
+ end
+ end
+end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list