19269: Upgrade user->all_users group membership links.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 1662278cc3edc0c4b9c681d51b1696c195b27a98..8c8039f1b842a7fa1242f0a822d964800bdf3f29 100644 (file)
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -311,28 +311,27 @@ SELECT target_uuid, perm_level
     # note: these permission links are obsolete, 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
+               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
+               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
+               link_class: 'permission',
+               name: 'can_login').destroy_all
 
     # delete "All users" group read permissions for this user
     Link.where(tail_uuid: self.uuid,
-                     head_uuid: all_users_group_uuid,
-                     link_class: 'permission',
-                     name: 'can_read').destroy_all
+               head_uuid: all_users_group_uuid,
+               link_class: 'permission').destroy_all
 
     # delete any signatures by this user
     Link.where(link_class: 'signature',
-                     tail_uuid: self.uuid).destroy_all
+               tail_uuid: self.uuid).destroy_all
 
     # delete tokens for this user
     ApiClientAuthorization.where(user_id: self.id).destroy_all
@@ -381,8 +380,7 @@ SELECT target_uuid, perm_level
       #
       if Link.where(tail_uuid: self.uuid,
                     head_uuid: all_users_group_uuid,
-                    link_class: 'permission',
-                    name: 'can_read').any?
+                    link_class: 'permission').any?
         errors.add :is_active, "cannot be set to false directly, use the 'Deactivate' button on Workbench, or the 'unsetup' API call"
       end
     end
@@ -785,11 +783,11 @@ SELECT target_uuid, perm_level
     resp = [Link.where(tail_uuid: self.uuid,
                        head_uuid: all_users_group_uuid,
                        link_class: 'permission',
-                       name: 'can_read').first ||
+                       name: 'can_write').first ||
             Link.create(tail_uuid: self.uuid,
                         head_uuid: all_users_group_uuid,
                         link_class: 'permission',
-                        name: 'can_read')]
+                        name: 'can_write')]
     if Rails.configuration.Users.ActivatedUsersAreVisibleToOthers
       resp += [Link.where(tail_uuid: all_users_group_uuid,
                           head_uuid: self.uuid,

diff --git a/services/api/db/migrate/20220726034131_write_via_all_users.rb b/services/api/db/migrate/20220726034131_write_via_all_users.rb
new file mode 100644 (file)
index 0000000..30e6463
--- /dev/null
+++ b/services/api/db/migrate/20220726034131_write_via_all_users.rb
@@ -0,0 +1,24 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class WriteViaAllUsers < ActiveRecord::Migration[5.2]
+  include CurrentApiClient
+  def up
+    changelinks(from: "can_read", to: "can_write")
+  end
+  def down
+    changelinks(from: "can_write", to: "can_read")
+  end
+  def changelinks(from:, to:)
+    ActiveRecord::Base.connection.exec_query(
+      "update links set name=$1 where link_class=$2 and name=$3 and tail_uuid like $4 and head_uuid = $5",
+      "migrate", [
+        [nil, to],
+        [nil, "permission"],
+        [nil, from],
+        [nil, "_____-tpzed-_______________"],
+        [nil, all_users_group_uuid],
+      ])
+  end
+end

diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index c5f6d567bfe5dc6585da96ee52afbd7dd3da948e..525300833e7bf55069687eee5fe14a0ad3b6dd4f 100644 (file)
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -254,8 +254,6 @@ $$;
 
 SET default_tablespace = '';
 
-SET default_with_oids = false;
-
 --
 -- Name: api_client_authorizations; Type: TABLE; Schema: public; Owner: -
 --
@@ -3182,6 +3180,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20220301155729'),
 ('20220303204419'),
 ('20220401153101'),
-('20220505112900');
+('20220505112900'),
+('20220726034131');
 
 

diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index b7f1aaa1faff4fdf6897fb964567c75407daa9d3..99b97510db99951575052e88703b5d962d6716c7 100644 (file)
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -54,7 +54,7 @@ active_user_member_of_all_users_group:
   updated_at: 2014-01-24 20:42:26 -0800
   tail_uuid: zzzzz-tpzed-xurymjxw79nv3jz
   link_class: permission
-  name: can_read
+  name: can_write
   head_uuid: zzzzz-j7d0g-fffffffffffffff
   properties: {}
 
@@ -110,7 +110,7 @@ spectator_user_member_of_all_users_group:
   updated_at: 2014-01-24 20:42:26 -0800
   tail_uuid: zzzzz-tpzed-l1s2piq4t4mps8r
   link_class: permission
-  name: can_read
+  name: can_write
   head_uuid: zzzzz-j7d0g-fffffffffffffff
   properties: {}
 
@@ -124,7 +124,7 @@ inactive_user_member_of_all_users_group:
   updated_at: 2013-12-26T20:52:21Z
   tail_uuid: zzzzz-tpzed-x9kqpd79egh49c7
   link_class: permission
-  name: can_read
+  name: can_write
   head_uuid: zzzzz-j7d0g-fffffffffffffff
   properties: {}
 
@@ -138,7 +138,7 @@ inactive_signed_ua_user_member_of_all_users_group:
   updated_at: 2013-12-26T20:52:21Z
   tail_uuid: zzzzz-tpzed-7sg468ezxwnodxs
   link_class: permission
-  name: can_read
+  name: can_write
   head_uuid: zzzzz-j7d0g-fffffffffffffff
   properties: {}
 
@@ -433,7 +433,7 @@ project_viewer_member_of_all_users_group:
   updated_at: 2015-07-28T21:34:41.361747000Z
   tail_uuid: zzzzz-tpzed-projectviewer1a
   link_class: permission
-  name: can_read
+  name: can_write
   head_uuid: zzzzz-j7d0g-fffffffffffffff
   properties: {}
 
@@ -1044,7 +1044,7 @@ user1-with-load_member_of_all_users_group:
   updated_at: 2014-01-24 20:42:26 -0800
   tail_uuid: zzzzz-tpzed-user1withloadab
   link_class: permission
-  name: can_read
+  name: can_write
   head_uuid: zzzzz-j7d0g-fffffffffffffff
   properties: {}
 

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 6a7b00a00573f9413a61856d66fd6e6eb3900c65..b7d683df29b16df8eeb31b5443252a60b1742be0 100644 (file)
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -151,7 +151,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage',
         "foo/#{repo_name}", created['uuid'], 'arvados#repository', true, 'Repository'
 
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#virtualMachine', false, 'permission', 'can_login',
@@ -335,7 +335,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     # two extra links; system_group, and group
     verify_links_added 2
 
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', response_object['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#repository', false, 'permission', 'can_manage',
@@ -420,7 +420,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage',
         'foo/usertestrepo', created['uuid'], 'arvados#repository', true, 'Repository'
 
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#virtualMachine', false, 'permission', 'can_login',
@@ -458,7 +458,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage',
         'foo/usertestrepo', created['uuid'], 'arvados#repository', true, 'Repository'
 
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#virtualMachine', true, 'permission', 'can_login',
@@ -511,7 +511,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     assert_equal active_user[:email], created['email'], 'expected input email'
 
      # verify links
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage',
@@ -545,7 +545,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     assert_equal active_user['email'], created['email'], 'expected original email'
 
     # verify links
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     assert_equal(repos_count, repos_query.count)
@@ -666,7 +666,7 @@ The Arvados team.
     assert_equal active_user['uuid'], json_response['uuid']
     updated = User.where(uuid: active_user['uuid']).first
     assert_equal(true, updated.is_active)
-    assert_equal({read: true}, updated.group_permissions[all_users_group_uuid])
+    assert_equal({read: true, write: true}, updated.group_permissions[all_users_group_uuid])
   end
 
   test "non-admin user can get basic information about readable users" do

diff --git a/services/api/test/helpers/users_test_helper.rb b/services/api/test/helpers/users_test_helper.rb
index 6ca9977a5ebaa6b8ae672d015365b57a22b0d889..e106d994cd9c9808e5a73358356e71c59f8855b4 100644 (file)
--- a/services/api/test/helpers/users_test_helper.rb
+++ b/services/api/test/helpers/users_test_helper.rb
@@ -3,6 +3,8 @@
 # SPDX-License-Identifier: AGPL-3.0
 
 module UsersTestHelper
+  include CurrentApiClient
+
   def verify_link(response_items, link_object_name, expect_link, link_class,
         link_name, head_uuid, tail_uuid, head_kind, fetch_object, class_name)
     link = find_obj_in_resp response_items, 'arvados#link', link_object_name
@@ -75,17 +77,14 @@ module UsersTestHelper
       assert !vm_login_perms.any?, "expected all vm_login_perms deleted"
     end
 
-    group = Group.where(name: 'All users').select do |g|
-      g[:uuid].match(/-f+$/)
-    end.first
-    group_read_perms = Link.where(tail_uuid: uuid,
-                                  head_uuid: group[:uuid],
+    group_write_perms = Link.where(tail_uuid: uuid,
+                                  head_uuid: all_users_group_uuid,
                                   link_class: 'permission',
-                                  name: 'can_read')
+                                  name: 'can_write')
     if expect_group_perms
-      assert group_read_perms.any?, "expected all users group read perms"
+      assert group_write_perms.any?, "expected all users group write perms"
     else
-      assert !group_read_perms.any?, "expected all users group perm deleted"
+      assert !group_write_perms.any?, "expected all users group write perms deleted"
     end
 
     signed_uuids = Link.where(link_class: 'signature',

diff --git a/services/api/test/integration/users_test.rb b/services/api/test/integration/users_test.rb
index 430f0d385d7e3789995af57219ece58eeec59367..f7fddb44d371c727a0da1b9ef314004fe67f1d6d 100644 (file)
--- a/services/api/test/integration/users_test.rb
+++ b/services/api/test/integration/users_test.rb
@@ -40,7 +40,7 @@ class UsersTest < ActionDispatch::IntegrationTest
     verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage',
         'foo/usertestrepo', created['uuid'], 'arvados#repository', true, 'Repository'
 
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#virtualMachine', false, 'permission', 'can_login',
@@ -85,7 +85,7 @@ class UsersTest < ActionDispatch::IntegrationTest
     verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage',
         'foo/usertestrepo', created['uuid'], 'arvados#repository', true, 'Repository'
 
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#virtualMachine', true, 'permission', 'can_login',
@@ -113,7 +113,7 @@ class UsersTest < ActionDispatch::IntegrationTest
 
     # two new links: system_group, and 'All users' group.
 
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#virtualMachine', false, 'permission', 'can_login',
@@ -135,7 +135,7 @@ class UsersTest < ActionDispatch::IntegrationTest
     assert_equal 'foo@example.com', created['email'], 'expected input email'
 
      # verify links
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage',
@@ -163,7 +163,7 @@ class UsersTest < ActionDispatch::IntegrationTest
     assert_equal created['email'], 'foo@example.com', 'expected original email'
 
     # verify links
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#virtualMachine', true, 'permission', 'can_login',
@@ -187,7 +187,7 @@ class UsersTest < ActionDispatch::IntegrationTest
 
     # four extra links: system_group, login, group, repo and vm
 
-    verify_link response_items, 'arvados#group', true, 'permission', 'can_read',
+    verify_link response_items, 'arvados#group', true, 'permission', 'can_write',
         'All users', created['uuid'], 'arvados#group', true, 'Group'
 
     verify_link response_items, 'arvados#repository', true, 'permission', 'can_manage',

diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index 9a0e1dbf9cc2427be71613b1ed939bc125f07351..12384cba923d27da98e7d72897b1d644ce24f5cd 100644 (file)
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -465,7 +465,7 @@ class UserTest < ActiveSupport::TestCase
       verify_user resp_user, email
 
       group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
-      verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
+      verify_link group_perm, 'permission', 'can_write', resp_user[:uuid], groups(:all_users).uuid
 
       group_perm2 = find_obj_in_resp response, 'Link', 'arvados#user'
       if visible
@@ -499,7 +499,7 @@ class UserTest < ActiveSupport::TestCase
     verify_user resp_user, email
 
     group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
-    verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
+    verify_link group_perm, 'permission', 'can_write', resp_user[:uuid], groups(:all_users).uuid
 
     repo_perm = find_obj_in_resp response, 'Link', 'arvados#repository'
     verify_link repo_perm, 'permission', 'can_manage', resp_user[:uuid], nil
@@ -522,7 +522,7 @@ class UserTest < ActiveSupport::TestCase
     verify_user resp_user, email
 
     group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
-    verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
+    verify_link group_perm, 'permission', 'can_write', resp_user[:uuid], groups(:all_users).uuid
 
     group_perm2 = find_obj_in_resp response, 'Link', 'arvados#user'
     verify_link group_perm2, 'permission', 'can_read', groups(:all_users).uuid, nil
@@ -534,7 +534,7 @@ class UserTest < ActiveSupport::TestCase
     assert_equal user.uuid, resp_user[:uuid], 'expected uuid not found'
 
     group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
-    verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
+    verify_link group_perm, 'permission', 'can_write', resp_user[:uuid], groups(:all_users).uuid
 
     repo_perm = find_obj_in_resp response, 'Link', 'arvados#repository'
     verify_link repo_perm, 'permission', 'can_manage', resp_user[:uuid], nil
@@ -550,7 +550,7 @@ class UserTest < ActiveSupport::TestCase
     assert_equal user.uuid, resp_user[:uuid], 'expected uuid not found'
 
     group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
-    verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
+    verify_link group_perm, 'permission', 'can_write', resp_user[:uuid], groups(:all_users).uuid
 
     repo_perm = find_obj_in_resp response, 'Link', 'arvados#repository'
     verify_link repo_perm, 'permission', 'can_manage', resp_user[:uuid], nil
@@ -625,7 +625,7 @@ class UserTest < ActiveSupport::TestCase
     # check user setup
     verify_link_exists(Rails.configuration.Users.AutoSetupNewUsers || active,
                        groups(:all_users).uuid, user.uuid,
-                       "permission", "can_read")
+                       "permission", "can_write")
 
     # Check for repository.
     if named_repo = (prior_repo or