3171: Add tests for desired behavior. Start using FactoryGirl.

diff --git a/services/api/Gemfile b/services/api/Gemfile
index fa2ce5ae6966e2c63adafe24d024e509d52e07b1..20d51520cfdfec57ae50476c6ffc2a4d26bcd60b 100644 (file)
--- 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 18fce168191a762672c379fe184153a60a5641b7..d27f2bf427693c6b52ec6e192ec52f1c5aad69d2 100644 (file)
--- 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 7bd475278ce7d320e643cd2e78e9636f54511995..37039ee65444650c94470dd48e9b166bc67568b0 100644 (file)
--- 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 (file)
index 0000000..70358e6
--- /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 (file)
index 0000000..8a4649d
--- /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 (file)
index 0000000..7c48fc0
--- /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 47c6b613c2b85ba7f1f96fa52402fcb8bf3ab7e8..cd535d2e5a1c4faaa03745df7f9ff16d45b8c75e 100644 (file)
--- 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 1ea1419147686e5117e0f5d2b794777f350854d7..24399f500e224151d175f94049cb0eb4181cf129 100644 (file)
--- 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