From: Tom Clegg <tom@curoverse.com>
Date: Mon, 6 Oct 2014 15:15:05 +0000 (-0400)
Subject: 3687: Merge branch 'master' into 3687-grant-permission-on-repo-TC
X-Git-Tag: 1.1.0~2134^2
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/86b14bac0b6413dd033285a4357cc7edd05a22b3?hp=b344d16c97448505432c0f0e9f9f964bfae07d50

3687: Merge branch 'master' into 3687-grant-permission-on-repo-TC
---

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 446a61f255..6e7facd5d5 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -70,6 +70,30 @@ class User < ArvadosModel
         next if (group_permissions[target.owner_uuid] and
                  group_permissions[target.owner_uuid][action])
       end
+      sufficient_perms = case action
+                         when :manage
+                           ['can_manage']
+                         when :write
+                           ['can_manage', 'can_write']
+                         when :read
+                           ['can_manage', 'can_write', 'can_read']
+                         else
+                           # (Skip this kind of permission opportunity
+                           # if action is an unknown permission type)
+                         end
+      if sufficient_perms
+        # Check permission links with head_uuid pointing directly at
+        # the target object. If target is a Group, this is redundant
+        # and will fail except [a] if permission caching is broken or
+        # [b] during a race condition, where a permission link has
+        # *just* been added.
+        if Link.where(link_class: 'permission',
+                      name: sufficient_perms,
+                      tail_uuid: groups_i_can(action) + [self.uuid],
+                      head_uuid: target_uuid).any?
+          next
+        end
+      end
       return false
     end
     true
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index c0bce93daa..32560c3397 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -174,3 +174,9 @@ active_no_prefs_profile:
   user: active_no_prefs_profile
   api_token: 3kg612cdc0f3415c242856758f33bdfb07bc3561b00e86qdmi
   expires_at: 2038-01-01 00:00:00
+
+user_foo_in_sharing_group:
+  api_client: untrusted
+  user: user_foo_in_sharing_group
+  api_token: 2p1pou8p4ls208mcbedeewlotghppenobcyrmyhq8pyf51xd8u
+  expires_at: 2038-01-01 00:00:00
diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml
index 860bb987b6..2fb235c708 100644
--- a/services/api/test/fixtures/collections.yml
+++ b/services/api/test/fixtures/collections.yml
@@ -249,3 +249,11 @@ collection_in_aproject_with_same_name_as_in_home_project:
   updated_at: 2014-02-03T17:22:54Z
   manifest_text: ". 73feffa4b7f6bb68e44cf984c85f6e88+3 0:3:baz\n"
   name: collection_with_same_name_in_aproject_and_home_project
+
+collection_owned_by_foo:
+  uuid: zzzzz-4zz18-50surkhkbhsp31b
+  portable_data_hash: ea10d51bcf88862dbcc36eb292017dfd+45
+  manifest_text: ". 73feffa4b7f6bb68e44cf984c85f6e88+3 0:3:baz\n"
+  owner_uuid: zzzzz-tpzed-81hsbo6mk8nl05c
+  created_at: 2014-02-03T17:22:54Z
+  name: collection_owned_by_foo
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index 3100a2c953..8f209e9b9c 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -128,3 +128,12 @@ active_user_has_can_manage:
   uuid: zzzzz-j7d0g-ptt1ou6a9lxrv07
   owner_uuid: zzzzz-tpzed-d9tiejq69daie8f
   name: Active user has can_manage
+
+# Group for testing granting permission between users who share a group.
+#
+group_for_sharing_tests:
+  uuid: zzzzz-j7d0g-t4ucgncwteul7zt
+  owner_uuid: zzzzz-tpzed-000000000000000
+  name: Group for sharing tests
+  description: Users who can share objects with each other
+  group_class: role
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 5243dfabbc..9e13971491 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -702,3 +702,36 @@ auto_setup_vm_login_username_can_login_to_test_vm:
   head_uuid: zzzzz-2x53u-382brsig8rp3064
   properties: {username: 'auto_setup_vm_login'}
   updated_at: 2014-08-06 22:11:51.242010312 Z
+
+user_foo_can_read_sharing_group:
+  uuid: zzzzz-o0j2j-gdpvwvpj9kjs5in
+  owner_uuid: zzzzz-tpzed-000000000000000
+  tail_uuid: zzzzz-tpzed-81hsbo6mk8nl05c
+  link_class: permission
+  name: can_read
+  head_uuid: zzzzz-j7d0g-t4ucgncwteul7zt
+
+user_foo_is_in_sharing_group:
+  uuid: zzzzz-o0j2j-bwmcf9nqwomvtny
+  owner_uuid: zzzzz-tpzed-000000000000000
+  tail_uuid: zzzzz-j7d0g-t4ucgncwteul7zt
+  link_class: permission
+  name: can_read
+  head_uuid: zzzzz-tpzed-81hsbo6mk8nl05c
+
+user_bar_can_read_sharing_group:
+  uuid: zzzzz-o0j2j-23djaoza9g2zvjx
+  owner_uuid: zzzzz-tpzed-000000000000000
+  tail_uuid: zzzzz-tpzed-n3oaj4sm5fcnwib
+  link_class: permission
+  name: can_read
+  head_uuid: zzzzz-j7d0g-t4ucgncwteul7zt
+
+user_bar_is_in_sharing_group:
+  uuid: zzzzz-o0j2j-ga7fgy3xsz4hu28
+  owner_uuid: zzzzz-tpzed-000000000000000
+  tail_uuid: zzzzz-j7d0g-t4ucgncwteul7zt
+  link_class: permission
+  name: can_read
+  head_uuid: zzzzz-tpzed-n3oaj4sm5fcnwib
+
diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index 8a82fe461a..2dd72ab4b1 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -178,3 +178,25 @@ active_no_prefs_profile:
   is_admin: false
   prefs:
     test: abc
+
+# Fixtures to test granting and removing permissions.
+
+user_foo_in_sharing_group:
+  owner_uuid: zzzzz-tpzed-000000000000000
+  uuid: zzzzz-tpzed-81hsbo6mk8nl05c
+  email: user_foo_in_sharing_group@arvados.local
+  first_name: Foo
+  last_name: Sharing
+  identity_url: https://user_foo_in_sharing_group.openid.local
+  is_active: true
+  is_admin: false
+
+user_bar_in_sharing_group:
+  owner_uuid: zzzzz-tpzed-000000000000000
+  uuid: zzzzz-tpzed-n3oaj4sm5fcnwib
+  email: user_bar_in_sharing_group@arvados.local
+  first_name: Bar
+  last_name: Sharing
+  identity_url: https://user_bar_in_sharing_group.openid.local
+  is_active: true
+  is_admin: false
diff --git a/services/api/test/functional/arvados/v1/links_controller_test.rb b/services/api/test/functional/arvados/v1/links_controller_test.rb
index 4058101501..7515e49c8f 100644
--- a/services/api/test/functional/arvados/v1/links_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/links_controller_test.rb
@@ -340,4 +340,22 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
     assert_not_nil assigns(:objects)
     assert_empty assigns(:objects)
   end
+
+  # Granting permissions.
+  test "grant can_read on project to other users in group" do
+    authorize_with :user_foo_in_sharing_group
+
+    refute users(:user_bar_in_sharing_group).can?(read: collections(:collection_owned_by_foo).uuid)
+
+    post :create, {
+      link: {
+        tail_uuid: users(:user_bar_in_sharing_group).uuid,
+        link_class: 'permission',
+        name: 'can_read',
+        head_uuid: collections(:collection_owned_by_foo).uuid,
+      }
+    }
+    assert_response :success
+    assert users(:user_bar_in_sharing_group).can?(read: collections(:collection_owned_by_foo).uuid)
+  end
 end