[ARVADOS] updated: 1059fd1f9e22ebcae51413d2b0fde0416c71c79b

[git at public.curoverse.com]

Summary of changes:
 .../app/controllers/arvados/v1/users_controller.rb |    2 +-
 services/api/lib/current_api_client.rb             |    8 +++--
 .../functional/arvados/v1/users_controller_test.rb |   25 +++++++++++++++
 services/api/test/unit/application_test.rb         |   32 ++++++++++++++++++++
 4 files changed, 63 insertions(+), 4 deletions(-)
 create mode 100644 services/api/test/unit/application_test.rb

       via  1059fd1f9e22ebcae51413d2b0fde0416c71c79b (commit)
      from  49aaa9f26dfaf50056abd7527976b681be26208b (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 1059fd1f9e22ebcae51413d2b0fde0416c71c79b
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Wed Apr 23 13:00:03 2014 -0400

    Security bug fix to act_as_system_user to restore correct user if an
    exception is raised.  Also adds tests for act_as_system_user.
    
    Also a minor style cleanup of "== []" to ".empty?" and some additional
    checks to test "refuse to activate a user before signing UA".

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index f5255bc..c2a32f0 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -74,7 +74,7 @@ class Arvados::V1::UsersController < ApplicationController
                                   head_uuid: required_uuids).
           collect(&:head_uuid)
         todo_uuids = required_uuids - signed_uuids
-        if todo_uuids == []
+        if todo_uuids.empty?
           @object.update_attributes is_active: true
           logger.info "User #{@object.uuid} activated"
         else
diff --git a/services/api/lib/current_api_client.rb b/services/api/lib/current_api_client.rb
index 401be16..bbba4dc 100644
--- a/services/api/lib/current_api_client.rb
+++ b/services/api/lib/current_api_client.rb
@@ -98,9 +98,11 @@ module CurrentApiClient
     if block_given?
       user_was = Thread.current[:user]
       Thread.current[:user] = system_user
-      ret = yield
-      Thread.current[:user] = user_was
-      ret
+      begin
+        yield
+      ensure
+        Thread.current[:user] = user_was
+      end
     else
       Thread.current[:user] = system_user
     end
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index abf1ba1..b6bb205 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -21,12 +21,37 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
   end
 
   test "refuse to activate a user before signing UA" do
+    act_as_system_user do
+    required_uuids = Link.where("owner_uuid = ? and link_class = ? and name = ? and tail_uuid = ? and head_uuid like ?",
+                                system_user_uuid,
+                                'signature',
+                                'require',
+                                system_user_uuid,
+                                Collection.uuid_like_pattern).
+      collect(&:head_uuid)
+
+      assert required_uuids.length > 0
+
+      signed_uuids = Link.where(owner_uuid: system_user_uuid,
+                                link_class: 'signature',
+                                name: 'click',
+                                tail_uuid: users(:inactive).uuid,
+                                head_uuid: required_uuids).
+        collect(&:head_uuid)
+
+      assert_equal 0, signed_uuids.length
+    end
+
     authorize_with :inactive
+
     get :current
     assert_response :success
     me = JSON.parse(@response.body)
+    assert_equal false, me['is_active']
+
     post :activate, uuid: me['uuid']
     assert_response 403
+
     get :current
     assert_response :success
     me = JSON.parse(@response.body)
diff --git a/services/api/test/unit/application_test.rb b/services/api/test/unit/application_test.rb
new file mode 100644
index 0000000..ca80319
--- /dev/null
+++ b/services/api/test/unit/application_test.rb
@@ -0,0 +1,32 @@
+require 'test_helper'
+
+class ApplicationTest < ActiveSupport::TestCase
+  include CurrentApiClient
+
+  test "test act_as_system_user" do
+    Thread.current[:user] = users(:active)
+    assert_equal users(:active), Thread.current[:user]
+    act_as_system_user do
+      assert_not_equal users(:active), Thread.current[:user]
+      assert_equal system_user, Thread.current[:user]
+    end
+    assert_equal users(:active), Thread.current[:user]
+  end
+
+  test "test act_as_system_user is exception safe" do
+    Thread.current[:user] = users(:active)
+    assert_equal users(:active), Thread.current[:user]
+    caught = false
+    begin
+      act_as_system_user do
+        assert_not_equal users(:active), Thread.current[:user]
+        assert_equal system_user, Thread.current[:user]
+        raise "Fail"
+      end
+    rescue
+      caught = true
+    end
+    assert caught
+    assert_equal users(:active), Thread.current[:user]
+  end
+end

-----------------------------------------------------------------------


hooks/post-receive
--