19501: Remove all sharing permissions during unsetup. 19501-unsetup-permissions
authorTom Clegg <tom@curii.com>
Mon, 17 Oct 2022 14:31:37 +0000 (10:31 -0400)
committerTom Clegg <tom@curii.com>
Mon, 17 Oct 2022 14:31:37 +0000 (10:31 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

services/api/app/models/user.rb
services/api/test/integration/users_test.rb

index 8c8039f1b842a7fa1242f0a822d964800bdf3f29..bbdd9c2843d4d810439e1f9ecafce1b0835b02ae 100644 (file)
@@ -308,25 +308,20 @@ SELECT target_uuid, perm_level
 
     # delete oid_login_perms for this user
     #
-    # note: these permission links are obsolete, they have no effect
-    # on anything and they are not created for new users.
+    # note: these permission links are obsolete anyway: they have no
+    # effect on anything and they are not created for new users.
     Link.where(tail_uuid: self.email,
                link_class: 'permission',
                name: 'can_login').destroy_all
 
-    # delete repo_perms for this user
-    Link.where(tail_uuid: self.uuid,
-               link_class: 'permission',
-               name: 'can_manage').destroy_all
-
-    # delete vm_login_perms for this user
-    Link.where(tail_uuid: self.uuid,
-               link_class: 'permission',
-               name: 'can_login').destroy_all
-
-    # delete "All users" group read permissions for this user
+    # Delete all sharing permissions so (a) the user doesn't
+    # automatically regain access to anything if re-setup in future,
+    # (b) the user doesn't appear in "currently shared with" lists
+    # shown to other users.
+    #
+    # Notably this includes the can_read -> "all users" group
+    # permission.
     Link.where(tail_uuid: self.uuid,
-               head_uuid: all_users_group_uuid,
                link_class: 'permission').destroy_all
 
     # delete any signatures by this user
index f7fddb44d371c727a0da1b9ef314004fe67f1d6d..ca143363892cad7065e65d704d1c76bbd7551c83 100644 (file)
@@ -203,6 +203,22 @@ class UsersTest < ActionDispatch::IntegrationTest
       ApiClientAuthorization.create!(user: User.find_by_uuid(created['uuid']), api_client: ApiClient.all.first).api_token
     end
 
+    # share project and collections with the new user
+    act_as_system_user do
+      Link.create!(tail_uuid: created['uuid'],
+                   head_uuid: groups(:aproject).uuid,
+                   link_class: 'permission',
+                   name: 'can_manage')
+      Link.create!(tail_uuid: created['uuid'],
+                   head_uuid: collections(:collection_owned_by_active).uuid,
+                   link_class: 'permission',
+                   name: 'can_read')
+      Link.create!(tail_uuid: created['uuid'],
+                   head_uuid: collections(:collection_owned_by_active_with_file_stats).uuid,
+                   link_class: 'permission',
+                   name: 'can_write')
+    end
+
     assert_equal 1, ApiClientAuthorization.where(user_id: User.find_by_uuid(created['uuid']).id).size, 'expected token not found'
 
     post "/arvados/v1/users/#{created['uuid']}/unsetup", params: {}, headers: auth(:admin)
@@ -213,6 +229,8 @@ class UsersTest < ActionDispatch::IntegrationTest
     assert_not_nil created2['uuid'], 'expected uuid for the newly created user'
     assert_equal created['uuid'], created2['uuid'], 'expected uuid not found'
     assert_equal 0, ApiClientAuthorization.where(user_id: User.find_by_uuid(created['uuid']).id).size, 'token should have been deleted by user unsetup'
+    # check permissions are deleted
+    assert_empty Link.where(tail_uuid: created['uuid'])
 
     verify_link_existence created['uuid'], created['email'], false, false, false, false, false
   end