[arvados] created: 2.1.0-2786-g2f0641a6a
git repository hosting
git at public.arvados.org
Wed Aug 24 20:53:13 UTC 2022
at 2f0641a6a4afc16e143475b7d13a93336752b49b (commit)
commit 2f0641a6a4afc16e143475b7d13a93336752b49b
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 fbb854599d733bf50cdb258c778c25afe9240a8f
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_manage_via_all_users.rb b/services/api/db/migrate/20220726034131_write_manage_via_all_users.rb
new file mode 100644
index 000000000..d2f55c77a
--- /dev/null
+++ b/services/api/db/migrate/20220726034131_write_manage_via_all_users.rb
@@ -0,0 +1,24 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class WriteManageViaAllUsers < 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..a1011353f 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_manage',
'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