From 754d85439d5e9a835562689dee597b782932914f Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@curoverse.com>
Date: Thu, 21 Aug 2014 14:10:01 -0400
Subject: [PATCH] 3171: Add tests for desired behavior. Start using
 FactoryGirl.

---
 services/api/Gemfile                      |  1 +
 services/api/Gemfile.lock                 |  6 ++++
 services/api/lib/current_api_client.rb    | 16 ++++++---
 services/api/test/factories/group.rb      |  4 +++
 services/api/test/factories/link.rb       |  7 ++++
 services/api/test/factories/user.rb       | 29 ++++++++++++++++
 services/api/test/test_helper.rb          |  1 +
 services/api/test/unit/permission_test.rb | 42 +++++++++++++++++++++++
 8 files changed, 101 insertions(+), 5 deletions(-)
 create mode 100644 services/api/test/factories/group.rb
 create mode 100644 services/api/test/factories/link.rb
 create mode 100644 services/api/test/factories/user.rb

diff --git a/services/api/Gemfile b/services/api/Gemfile
index fa2ce5ae69..20d51520cf 100644
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -6,6 +6,7 @@ gem 'rails', '~> 3.2.0'
 # gem 'rails',     :git => 'git://github.com/rails/rails.git'
 
 group :test, :development do
+  gem 'factory_girl_rails'
   # Note: "require: false" here tells bunder not to automatically
   # 'require' the packages during application startup. Installation is
   # still mandatory.
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 18fce16819..d27f2bf427 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -76,6 +76,11 @@ GEM
     eventmachine (1.0.3)
     execjs (2.0.2)
     extlib (0.9.16)
+    factory_girl (4.4.0)
+      activesupport (>= 3.0.0)
+    factory_girl_rails (4.4.1)
+      factory_girl (~> 4.4.0)
+      railties (>= 3.0.0)
     faraday (0.8.9)
       multipart-post (~> 1.2.0)
     faye-websocket (0.7.2)
@@ -221,6 +226,7 @@ DEPENDENCIES
   arvados-cli (>= 0.1.20140708213257)
   coffee-rails (~> 3.2.0)
   database_cleaner
+  factory_girl_rails
   faye-websocket
   google-api-client (~> 0.6.3)
   jquery-rails
diff --git a/services/api/lib/current_api_client.rb b/services/api/lib/current_api_client.rb
index 7bd475278c..37039ee654 100644
--- a/services/api/lib/current_api_client.rb
+++ b/services/api/lib/current_api_client.rb
@@ -100,18 +100,24 @@ module CurrentApiClient
 
   def act_as_system_user
     if block_given?
-      user_was = Thread.current[:user]
-      Thread.current[:user] = system_user
-      begin
+      act_as_user system_user do
         yield
-      ensure
-        Thread.current[:user] = user_was
       end
     else
       Thread.current[:user] = system_user
     end
   end
 
+  def act_as_user user
+    user_was = Thread.current[:user]
+    Thread.current[:user] = user
+    begin
+      yield
+    ensure
+      Thread.current[:user] = user_was
+    end
+  end
+
   def anonymous_group
     if not $anonymous_group
       act_as_system_user do
diff --git a/services/api/test/factories/group.rb b/services/api/test/factories/group.rb
new file mode 100644
index 0000000000..70358e6f60
--- /dev/null
+++ b/services/api/test/factories/group.rb
@@ -0,0 +1,4 @@
+FactoryGirl.define do
+  factory :group do
+  end
+end
diff --git a/services/api/test/factories/link.rb b/services/api/test/factories/link.rb
new file mode 100644
index 0000000000..8a4649d480
--- /dev/null
+++ b/services/api/test/factories/link.rb
@@ -0,0 +1,7 @@
+FactoryGirl.define do
+  factory :link do
+    factory :permission_link do
+      link_class 'permission'
+    end
+  end
+end
diff --git a/services/api/test/factories/user.rb b/services/api/test/factories/user.rb
new file mode 100644
index 0000000000..7c48fc0ccc
--- /dev/null
+++ b/services/api/test/factories/user.rb
@@ -0,0 +1,29 @@
+include CurrentApiClient
+
+FactoryGirl.define do
+  factory :user do
+    before :create do
+      Thread.current[:user_was] = Thread.current[:user]
+      Thread.current[:user] = system_user
+    end
+    after :create do
+      Thread.current[:user] = Thread.current[:user_was]
+    end
+    first_name "Factory"
+    last_name "Factory"
+    identity_url do
+      "https://example.com/#{rand(2**24).to_s(36)}"
+    end
+    factory :active_user do
+      is_active true
+      after :create do |user|
+        act_as_system_user do
+          Link.create!(tail_uuid: user.uuid,
+                       head_uuid: Group.where('uuid ~ ?', '-f+$').first.uuid,
+                       link_class: 'permission',
+                       name: 'can_read')
+        end
+      end
+    end
+  end
+end
diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index 47c6b613c2..cd535d2e5a 100644
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -38,6 +38,7 @@ module ArvadosTestSupport
 end
 
 class ActiveSupport::TestCase
+  include FactoryGirl::Syntax::Methods
   fixtures :all
 
   include ArvadosTestSupport
diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb
index 1ea1419147..24399f500e 100644
--- a/services/api/test/unit/permission_test.rb
+++ b/services/api/test/unit/permission_test.rb
@@ -132,6 +132,48 @@ class PermissionTest < ActiveSupport::TestCase
     end
   end
 
+  test "users with bidirectional read permission in group can see each other, but cannot see each other's private articles" do
+    a = create :active_user first_name: "A"
+    b = create :active_user first_name: "B"
+    other = create :active_user first_name: "OTHER"
+    act_as_system_user do
+      g = create :group
+      [a,b].each do |u|
+        create(:permission_link,
+               name: 'can_read', tail_uuid: u.uuid, head_uuid: g.uuid)
+        create(:permission_link,
+               name: 'can_read', head_uuid: u.uuid, tail_uuid: g.uuid)
+      end
+    end
+    a_specimen = act_as_user a do
+      Specimen.create!
+    end
+    assert_not_empty(Specimen.readable_by(a).where(uuid: a_specimen.uuid),
+                     "A cannot read own Specimen, following test probably useless.")
+    assert_empty(Specimen.readable_by(b).where(uuid: a_specimen.uuid),
+                 "B can read A's Specimen")
+    [a,b].each do |u|
+      assert_empty(User.readable_by(u).where(uuid: other.uuid),
+                   "#{u.first_name} can see OTHER in the user list")
+      assert_empty(User.readable_by(other).where(uuid: u.uuid),
+                   "OTHER can see #{u.first_name} in the user list")
+      act_as_user u do
+        assert_raises ArvadosModel::PermissionDeniedError, "wrote without perm" do
+          other.update_attributes!(prefs: {'pwned' => true})
+        end
+        assert_equal true, u.update_attributes!(prefs: {'thisisme' => true})
+      end
+      act_as_user other do
+        ([other, a, b] - [u]).each do |x|
+          assert_raises ArvadosModel::PermissionDeniedError, "wrote without perm" do
+            x.update_attributes!(prefs: {'pwned' => true})
+          end
+        end
+        assert_equal true, other.update_attributes!(prefs: {'thisisme' => true})
+      end
+    end
+  end
+
   test "cannot create with owner = unwritable user" do
     set_user_from_auth :rominiadmin
     assert_raises ArvadosModel::PermissionDeniedError, "created with owner = unwritable user" do
-- 
2.30.2