[arvados] created: 2.1.0-2786-ge4c83a3eb

git repository hosting git at public.arvados.org
Thu Aug 25 14:55:28 UTC 2022


        at  e4c83a3ebe3b16c16f604b3b0968ce5600b7ab64 (commit)


commit e4c83a3ebe3b16c16f604b3b0968ce5600b7ab64
Author: Tom Clegg <tom at curii.com>
Date:   Wed Aug 24 16:48:17 2022 -0400

    19269: Update error text re modifying group role without can_manage.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index 0c36a048d..e44e605b1 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -250,7 +250,7 @@ class Group < ArvadosModel
       if self.owner_uuid != system_user_uuid
         raise "Owner uuid for role must be system user"
       end
-      raise PermissionDeniedError unless current_user.can?(manage: uuid)
+      raise PermissionDeniedError.new("role group cannot be modified without can_manage permission") unless current_user.can?(manage: uuid)
       true
     else
       super

commit 86a78a669f01d15ec8fa5da8a3dfa29f5ef28128
Author: Tom Clegg <tom at curii.com>
Date:   Wed Aug 24 16:20:30 2022 -0400

    19269: Upgrade user->all_users group membership links.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 1662278cc..8c8039f1b 100644
--- 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
index 000000000..30e6463be
--- /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 c5f6d567b..525300833 100644
--- 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 b7f1aaa1f..99b97510d 100644
--- 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 6a7b00a00..b7d683df2 100644
--- 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 6ca9977a5..e106d994c 100644
--- 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 430f0d385..f7fddb44d 100644
--- 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 at 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 at 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 9a0e1dbf9..12384cba9 100644
--- 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

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list