[ARVADOS] created: 62f9bb4bb85fc644e2b005b6f953b642b832b21c
git at public.curoverse.com
git at public.curoverse.com
Fri Jun 20 14:32:55 EDT 2014
at 62f9bb4bb85fc644e2b005b6f953b642b832b21c (commit)
commit 62f9bb4bb85fc644e2b005b6f953b642b832b21c
Author: Brett Smith <brett at curoverse.com>
Date: Fri Jun 20 14:33:18 2014 -0400
2891: Workbench error page shows login status.
The tests here check that an error page shows the usual user
information for someone logged in, and renders okay for someone who
isn't.
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index 28d6593..07edc36 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -55,13 +55,21 @@ class ApplicationController < ActionController::Base
else
@errors = [e.to_s]
end
- self.render_error status: 422
+ if e.is_a? ArvadosApiClient::NotLoggedInException
+ self.render_error status: 422
+ else
+ thread_with_optional_api_token do
+ self.render_error status: 422
+ end
+ end
end
def render_not_found(e=ActionController::RoutingError.new("Path not found"))
logger.error e.inspect
@errors = ["Path not found"]
- self.render_error status: 404
+ thread_with_optional_api_token do
+ self.render_error status: 404
+ end
end
def find_objects_for_index
diff --git a/apps/workbench/test/integration/errors_test.rb b/apps/workbench/test/integration/errors_test.rb
new file mode 100644
index 0000000..092041d
--- /dev/null
+++ b/apps/workbench/test/integration/errors_test.rb
@@ -0,0 +1,19 @@
+require 'integration_helper'
+
+class ErrorsTest < ActionDispatch::IntegrationTest
+ BAD_UUID = "zzzzz-zzzzz-zzzzzzzzzzzzzzz"
+
+ test "error page renders user navigation" do
+ visit(page_with_token("active", "/collections/#{BAD_UUID}"))
+ assert(page.has_text?(@@API_AUTHS["active"]["email"]),
+ "User information missing from error page")
+ assert(page.has_no_text?(/log ?in/i),
+ "Logged in user prompted to log in on error page")
+ end
+
+ test "error page renders without login" do
+ visit "/collections/download/#{BAD_UUID}/#{@@API_AUTHS['active']['api_token']}"
+ assert(page.has_no_text?(/\b500\b/),
+ "Error page without login returned 500")
+ end
+end
commit af60f396e5a4d313bbfeb810178885b69550f7ba
Author: Brett Smith <brett at curoverse.com>
Date: Fri Jun 20 14:31:33 2014 -0400
2891: Workbench handles expired tokens more consistently.
Previously Workbench would behave differently depending on whether an
expired token was provided as a query parameter, or living in a
session. This makes it do the same thing in all cases. It also fixes
some small bugs around removing api_token from URL paths.
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index 9e918f5..28d6593 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -296,11 +296,15 @@ class ApplicationController < ActionController::Base
protected
+ def strip_token_from_path(path)
+ path.sub(/([&\?])api_token=[^&]*/, '\1')
+ 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: request.url)
+ 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
@@ -363,11 +367,11 @@ class ApplicationController < ActionController::Base
begin
try_redirect_to_login = true
if params[:api_token]
- try_redirect_to_login = false
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] = {
@@ -384,24 +388,22 @@ class ApplicationController < ActionController::Base
# 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 request.fullpath.sub(%r{([&\?]api_token=)[^&\?]*}, '')
+ redirect_to strip_token_from_path(request.fullpath)
else
yield
end
- else
- @errors = ['Invalid API token']
- self.render_error status: 401
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.
- try_redirect_to_login = false
Thread.current[:arvados_api_token] = session[:arvados_api_token]
begin
yield
rescue ArvadosApiClient::NotLoggedInException
- try_redirect_to_login = true
+ # We'll try to redirect to login later.
+ else
+ try_redirect_to_login = false
end
else
logger.debug "No token received, session is #{session.inspect}"
diff --git a/apps/workbench/test/integration/logins_test.rb b/apps/workbench/test/integration/logins_test.rb
index be7e4e1..9daf831 100644
--- a/apps/workbench/test/integration/logins_test.rb
+++ b/apps/workbench/test/integration/logins_test.rb
@@ -7,15 +7,12 @@ class LoginsTest < ActionDispatch::IntegrationTest
assert_no_match(/\bapi_token=/, current_path)
end
- test "can't use expired token" do
+ test "trying to use expired token redirects to login page" do
visit page_with_token('expired_trustedclient')
- assert page.has_text? 'Log in'
- end
-
- test "expired token yields login page, not error page" do
- visit page_with_token('expired_trustedclient')
- # Even the error page has a "Log in" link. We should look for
- # something that only appears the real login page.
- assert page.has_text? ' Log in Oh... fiddlesticks. Sorry, I had some trouble handling your request'
+ buttons = all("a.btn", text: /Log in/)
+ assert_equal(1, buttons.size, "Failed to find one login button")
+ login_link = buttons.first[:href]
+ assert_match(%r{//[^/]+/login}, login_link)
+ assert_no_match(/\bapi_token=/, login_link)
end
end
commit 7917aacd2736ab3e41613cbab3c113518964fd9d
Author: Brett Smith <brett at curoverse.com>
Date: Fri Jun 20 11:19:20 2014 -0400
2891: Workbench integration tests cope when screenshots aren't supported.
diff --git a/apps/workbench/test/integration_helper.rb b/apps/workbench/test/integration_helper.rb
index ebfbc58..81ea67c 100644
--- a/apps/workbench/test/integration_helper.rb
+++ b/apps/workbench/test/integration_helper.rb
@@ -54,11 +54,17 @@ class ActionDispatch::IntegrationTest
end
end
- @@screenshot_count = 0
+ @@screenshot_count = 1
def screenshot
- image_file = "./tmp/workbench-fail-#{@@screenshot_count += 1}.png"
- page.save_screenshot image_file
- puts "Saved #{image_file}"
+ image_file = "./tmp/workbench-fail-#{@@screenshot_count}.png"
+ begin
+ page.save_screenshot image_file
+ rescue Capybara::NotSupportedByDriverError
+ # C'est la vie.
+ else
+ puts "Saved #{image_file}"
+ @@screenshot_count += 1
+ end
end
teardown do
commit 355ebf1f21ba7344ac6e70dcf7e1717fe92f33ed
Author: Brett Smith <brett at curoverse.com>
Date: Fri Jun 20 14:33:28 2014 -0400
Remove debug prints from Workbench.
diff --git a/apps/workbench/app/views/collections/_choose_rows.html.erb b/apps/workbench/app/views/collections/_choose_rows.html.erb
index 5cce19a..d87f56f 100644
--- a/apps/workbench/app/views/collections/_choose_rows.html.erb
+++ b/apps/workbench/app/views/collections/_choose_rows.html.erb
@@ -1,7 +1,5 @@
<% @name_links.each do |name_link| %>
- <% puts "looking up #{name_link.head_uuid}" %>
<% if (object = get_object(name_link.head_uuid)) %>
- <% puts "got #{object}" %>
<div class="row filterable selectable <%= 'multiple' if multiple %>" data-object-uuid="<%= name_link.uuid %>"
data-preview-href="<%= url_for object %>?tab_pane=chooser_preview"
style="margin-left: 1em; border-bottom-style: solid; border-bottom-width: 1px; border-bottom-color: #DDDDDD">
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list