From d3427aea4532d247b71b5ebfc5f5eb527e8835a2 Mon Sep 17 00:00:00 2001
From: Brett Smith <brett@curoverse.com>
Date: Thu, 14 Aug 2014 13:59:18 -0400
Subject: [PATCH] 3593: Another round of hardening Workbench's
 render_exception.

render_exception was counting on session[:user] to help render the
error page.  We recently excised that, which is turning exceptions
into 500s.  This commit restores the session user object, but uses it
only for error rendering, in keeping with the spirit of the previous
revert.  This commit also defends against more possible failure cases
in render_exception, and adds a basic test for it.
---
 .../app/controllers/application_controller.rb | 36 ++++++++++++++-----
 .../functional/application_controller_test.rb | 18 ++++++++++
 2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index 222888085d..bbc2a825ba 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -62,22 +62,31 @@ class ApplicationController < ActionController::Base
     else
       @errors = [e.to_s]
     end
-    # If the user has an active session, and the API server is available,
-    # make user information available on the error page.
+    # Make user information available on the error page, falling back to the
+    # session cache if the API server is unavailable.
     begin
       load_api_token(session[:arvados_api_token])
     rescue ArvadosApiClient::ApiError
-      load_api_token(nil)
+      unless session[:user].nil?
+        begin
+          Thread.current[:user] = User.new(session[:user])
+        rescue ArvadosApiClient::ApiError
+          # This can happen if User's columns are unavailable.  Nothing to do.
+        end
+      end
     end
-    # Preload projects trees for the template.  If that fails, set empty
+    # Preload projects trees for the template.  If that's not doable, set empty
     # trees so error page rendering can proceed.  (It's easier to rescue the
     # exception here than in a template.)
-    begin
-      build_project_trees
-    rescue ArvadosApiClient::ApiError
-      @my_project_tree ||= []
-      @shared_project_tree ||= []
+    unless current_user.nil?
+      begin
+        build_project_trees
+      rescue ArvadosApiClient::ApiError
+        # Fall back to the default-setting code later.
+      end
     end
+    @my_project_tree ||= []
+    @shared_project_tree ||= []
     render_error(err_opts)
   end
 
@@ -427,6 +436,15 @@ class ApplicationController < ActionController::Base
       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
diff --git a/apps/workbench/test/functional/application_controller_test.rb b/apps/workbench/test/functional/application_controller_test.rb
index 50f990aa0b..c2828020bd 100644
--- a/apps/workbench/test/functional/application_controller_test.rb
+++ b/apps/workbench/test/functional/application_controller_test.rb
@@ -303,4 +303,22 @@ class ApplicationControllerTest < ActionController::TestCase
     get(:show, {id: "zzzzz-zzzzz-zzzzzzzzzzzzzzz"}, session_for(:admin))
     assert_response 404
   end
+
+  test "Workbench returns 4xx when API server is unreachable" do
+    # We're really testing ApplicationController's render_exception.
+    # Our primary concern is that it doesn't raise an error and
+    # return 500.
+    orig_api_server = Rails.configuration.arvados_v1_base
+    begin
+      # The URL should look valid in all respects, and avoid talking over a
+      # network.  100::/64 is the IPv6 discard prefix, so it's perfect.
+      Rails.configuration.arvados_v1_base = "https://[100::f]:1/"
+      @controller = NodesController.new
+      get(:index, {}, session_for(:active))
+      assert_includes(405..422, @response.code.to_i,
+                      "bad response code when API server is unreachable")
+    ensure
+      Rails.configuration.arvados_v1_base = orig_api_server
+    end
+  end
 end
-- 
2.30.2