[ARVADOS] created: 2.1.0-1977-g209ab9fc8
Git user
git at public.arvados.org
Thu Mar 3 21:28:27 UTC 2022
at 209ab9fc8bcdc6451bf5fdc73a5023057dd7291f (commit)
commit 209ab9fc8bcdc6451bf5fdc73a5023057dd7291f
Author: Tom Clegg <tom at curii.com>
Date: Thu Mar 3 16:28:04 2022 -0500
18691: Update database indexes for frozen_by_uuid.
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 e3df46029..216397a1b 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -250,4 +250,8 @@ on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
return true
end
end
+
+ def self.full_text_searchable_columns
+ super - ["frozen_by_uuid"]
+ end
end
diff --git a/services/api/db/migrate/20220303204419_add_frozen_by_uuid_to_group_search_index.rb b/services/api/db/migrate/20220303204419_add_frozen_by_uuid_to_group_search_index.rb
new file mode 100644
index 000000000..52fb16b2a
--- /dev/null
+++ b/services/api/db/migrate/20220303204419_add_frozen_by_uuid_to_group_search_index.rb
@@ -0,0 +1,17 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class AddFrozenByUuidToGroupSearchIndex < ActiveRecord::Migration[5.0]
+ disable_ddl_transaction!
+
+ def up
+ remove_index :groups, :name => 'groups_search_index'
+ add_index :groups, ["uuid", "owner_uuid", "modified_by_client_uuid", "modified_by_user_uuid", "name", "group_class", "frozen_by_uuid"], name: 'groups_search_index', algorithm: :concurrently
+ end
+
+ def down
+ remove_index :groups, :name => 'groups_search_index'
+ add_index :groups, ["uuid", "owner_uuid", "modified_by_client_uuid", "modified_by_user_uuid", "name", "group_class"], name: 'groups_search_index', algorithm: :concurrently
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index cf92bcad9..cfe21f7c9 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -1803,7 +1803,7 @@ CREATE INDEX group_index_on_properties ON public.groups USING gin (properties);
-- Name: groups_search_index; Type: INDEX; Schema: public; Owner: -
--
-CREATE INDEX groups_search_index ON public.groups USING btree (uuid, owner_uuid, modified_by_client_uuid, modified_by_user_uuid, name, group_class);
+CREATE INDEX groups_search_index ON public.groups USING btree (uuid, owner_uuid, modified_by_client_uuid, modified_by_user_uuid, name, group_class, frozen_by_uuid);
--
@@ -3184,6 +3184,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20210816191509'),
('20211027154300'),
('20220224203102'),
-('20220301155729');
+('20220301155729'),
+('20220303204419');
commit bc078b7d031968061e439309a6a6dbeb1938a407
Author: Tom Clegg <tom at curii.com>
Date: Thu Mar 3 15:23:06 2022 -0500
18691: Fix wrong column name in clear_permissions.
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 93bafc45a..e3df46029 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -194,11 +194,14 @@ on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
def clear_permissions_trash_frozen
MaterializedPermission.where(target_uuid: uuid).delete_all
- ['trashed_groups', 'frozen_groups'].each do |table|
- ActiveRecord::Base.connection.exec_delete %{
- delete from #{table} where group_uuid=$1
- }, "Group.clear_permissions_trash_frozen", [[nil, self.uuid]]
- end
+ ActiveRecord::Base.connection.exec_delete(
+ "delete from trashed_groups where group_uuid=$1",
+ "Group.clear_permissions_trash_frozen",
+ [[nil, self.uuid]])
+ ActiveRecord::Base.connection.exec_delete(
+ "delete from frozen_groups where uuid=$1",
+ "Group.clear_permissions_trash_frozen",
+ [[nil, self.uuid]])
end
def assign_name
commit 3725c555044cdd0c137df387c1af5f1a17966227
Author: Tom Clegg <tom at curii.com>
Date: Thu Mar 3 13:30:37 2022 -0500
18691: Document groups.frozen_by_uuid attribute.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/doc/api/methods/groups.html.textile.liquid b/doc/api/methods/groups.html.textile.liquid
index e4b8594dd..6f5b4bbb8 100644
--- a/doc/api/methods/groups.html.textile.liquid
+++ b/doc/api/methods/groups.html.textile.liquid
@@ -34,6 +34,17 @@ table(table table-bordered table-condensed).
|trash_at|datetime|If @trash_at@ is non-null and in the past, this group and all objects directly or indirectly owned by the group will be hidden from API calls. May be untrashed.||
|delete_at|datetime|If @delete_at@ is non-null and in the past, the group and all objects directly or indirectly owned by the group may be permanently deleted.||
|is_trashed|datetime|True if @trash_at@ is in the past, false if not.||
+|frozen_by_uuid|string|For a frozen project, indicates the user who froze the project. Empty in all other cases. When a project is frozen, no further changes can be made to the project or its contents.||
+
+h3. Frozen projects
+
+A user with @manage@ permission can set the @frozen_by_uuid@ attribute of a @project@ group to their own user UUID. Once this is done, no further changes can be made to the project or its contents, including subprojects.
+
+The @frozen_by_uuid@ attribute can be cleared by an admin user. It can also be cleared by a user with @manage@ permission, unless the @API.UnfreezeProjectRequiresAdmin@ configuration setting is active.
+
+The optional @API.FreezeProjectRequiresDescription@ and @API.FreezeProjectRequiresProperties@ configuration settings can be used to prevent users from freezing projects that have empty @description@ and/or specified @properties@ entries.
+
+h3. Filter groups
@filter@ groups are virtual groups; they can not own other objects. Filter groups have a special @properties@ field named @filters@, which must be an array of filter conditions. See "list method filters":{{site.baseurl}}/api/methods.html#filters for details on the syntax of valid filters, but keep in mind that the attributes must include the object type (@collections@, @container_requests@, @groups@, @workflows@), separated with a dot from the field to be filtered on.
commit d409f51a533cedd76a8cfe91357b13608bb9050b
Author: Tom Clegg <tom at curii.com>
Date: Thu Mar 3 13:30:18 2022 -0500
18691: Export new frozen-project configs.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/config/export.go b/lib/config/export.go
index 4e903a8b3..db413b97b 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -62,6 +62,8 @@ var whitelist = map[string]bool{
"API": true,
"API.AsyncPermissionsUpdateInterval": false,
"API.DisabledAPIs": false,
+ "API.FreezeProjectRequiresDescription": true,
+ "API.FreezeProjectRequiresProperties": true,
"API.KeepServiceRequestTimeout": false,
"API.MaxConcurrentRequests": false,
"API.MaxIndexDatabaseRead": false,
@@ -72,6 +74,7 @@ var whitelist = map[string]bool{
"API.MaxTokenLifetime": false,
"API.RequestTimeout": true,
"API.SendTimeout": true,
+ "API.UnfreezeProjectRequiresAdmin": true,
"API.VocabularyPath": false,
"API.WebsocketClientEventQueue": false,
"API.WebsocketServerEventQueue": false,
commit 8d623fb99bd24edaf1d70571179bfe6ecac7936a
Author: Tom Clegg <tom at curii.com>
Date: Thu Mar 3 13:09:57 2022 -0500
18691: Add frozen_by_uuid field to Go SDK.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/sdk/go/arvados/group.go b/sdk/go/arvados/group.go
index d5243dc17..ad7ac1ee2 100644
--- a/sdk/go/arvados/group.go
+++ b/sdk/go/arvados/group.go
@@ -26,6 +26,7 @@ type Group struct {
Properties map[string]interface{} `json:"properties"`
WritableBy []string `json:"writable_by,omitempty"`
Description string `json:"description"`
+ FrozenByUUID string `json:"frozen_by_uuid"`
}
// GroupList is an arvados#groupList resource.
commit 11889bda63f5c4fff89ff54f92437178b22bacc8
Author: Tom Clegg <tom at curii.com>
Date: Thu Mar 3 13:09:34 2022 -0500
18691: Test moving items into/out of frozen project.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index ff079e1b5..bbb0f1957 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -404,19 +404,29 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
cr.destroy
end
- # Once project is frozen, cannot change name/contents, trash, or
- # delete the project or anything beneath it
+ # Once project is frozen, cannot change name/contents, move,
+ # trash, or delete the project or anything beneath it
[proj, proj_inner, coll].each do |frozen|
assert_raises(StandardError, "should reject rename of #{frozen.uuid} with parent #{frozen.owner_uuid}") do
frozen.update_attributes!(name: 'foo2')
end
frozen.reload
+
if frozen.is_a?(Collection)
assert_raises(StandardError, "should reject manifest change of #{frozen.uuid}") do
frozen.update_attributes!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n")
end
- frozen.reload
+ else
+ assert_raises(StandardError, "should reject moving a project into #{frozen.uuid}") do
+ groups(:private).update_attributes!(owner_uuid: frozen.uuid)
+ end
+ end
+ frozen.reload
+
+ assert_raises(StandardError, "should reject moving #{frozen.uuid} to a different parent project") do
+ frozen.update_attributes!(owner_uuid: groups(:private).uuid)
end
+ frozen.reload
assert_raises(StandardError, "should reject setting trash_at of #{frozen.uuid}") do
frozen.update_attributes!(trash_at: db_current_time)
end
commit f73b71e88b15214683a5483e76254a9795c8b915
Author: Tom Clegg <tom at curii.com>
Date: Thu Mar 3 11:29:25 2022 -0500
18691: Return empty writable_by for items inside frozen projects.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 029034d8b..a979338e4 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -246,6 +246,15 @@ class ArvadosModel < ApplicationRecord
# If current user cannot write this object, just return
# [self.owner_uuid].
def writable_by
+ # Return [] if this is a frozen project and the current user can't
+ # unfreeze
+ return [] if respond_to?(:frozen_by_uuid) && frozen_by_uuid &&
+ !(Rails.configuration.API.UnfreezeProjectRequiresAdmin ?
+ current_user.andand.is_admin :
+ current_user.can?(manage: uuid))
+ # Return [] if nobody can write because this object is inside a
+ # frozen project
+ return [] if FrozenGroup.where(uuid: owner_uuid).any?
return [owner_uuid] if not current_user
unless (owner_uuid == current_user.uuid or
current_user.is_admin or
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index 513c1dae6..ff079e1b5 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -425,6 +425,9 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
frozen.destroy
end
frozen.reload
+ if frozen != proj
+ assert_equal [], frozen.writable_by
+ end
end
# User with manage permission can unfreeze, then create items
commit 823c4953e20d8408c203e554913a7852bbc60a65
Author: Tom Clegg <tom at curii.com>
Date: Thu Mar 3 11:26:07 2022 -0500
18691: Test freeze requires manage permission.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index d71a0b5f9..513c1dae6 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -327,6 +327,19 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
end
proj.reload
+ # Cannot set frozen_by_uuid without can_manage permission
+ act_as_system_user do
+ Link.create!(link_class: 'permission', name: 'can_write', tail_uuid: users(:spectator).uuid, head_uuid: proj.uuid)
+ end
+ act_as_user users(:spectator) do
+ # First confirm we have write permission
+ assert Collection.create(name: 'bar', owner_uuid: proj.uuid)
+ assert_raises(ArvadosModel::PermissionDeniedError) do
+ proj.update_attributes!(frozen_by_uuid: users(:spectator).uuid)
+ end
+ end
+ proj.reload
+
# Cannot set frozen_by_uuid without description (if so configured)
Rails.configuration.API.FreezeProjectRequiresDescription = true
err = assert_raises do
commit a74c5dc699722769537455504d2f59455100cd29
Author: Tom Clegg <tom at curii.com>
Date: Wed Mar 2 14:32:42 2022 -0500
18691: Prevent changes in frozen projects.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 1c842c48d..029034d8b 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -643,7 +643,14 @@ class ArvadosModel < ApplicationRecord
end
def permission_to_create
- current_user.andand.is_active
+ if !current_user.andand.is_active
+ return false
+ end
+ if self.respond_to?(:owner_uuid) && FrozenGroup.where(uuid: owner_uuid).any?
+ errors.add :owner_uuid, "#{owner_uuid} is frozen"
+ return false
+ end
+ return true
end
def permission_to_update
@@ -660,6 +667,13 @@ class ArvadosModel < ApplicationRecord
logger.warn "User #{current_user.uuid} tried to change uuid of #{self.class.to_s} #{self.uuid_was} to #{self.uuid}"
return false
end
+ if self.respond_to?(:owner_uuid)
+ frozen = FrozenGroup.where(uuid: [owner_uuid, owner_uuid_in_database]).to_a
+ if frozen.any?
+ errors.add :uuid, "#{uuid} cannot be modified in frozen project #{frozen[0]}"
+ return false
+ end
+ end
return true
end
diff --git a/services/api/app/models/frozen_group.rb b/services/api/app/models/frozen_group.rb
new file mode 100644
index 000000000..bf4ee5d0b
--- /dev/null
+++ b/services/api/app/models/frozen_group.rb
@@ -0,0 +1,6 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class FrozenGroup < ApplicationRecord
+end
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index d6f698432..93bafc45a 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -23,6 +23,7 @@ class Group < ArvadosModel
before_create :assign_name
after_create :after_ownership_change
after_create :update_trash
+ after_create :update_frozen
before_update :before_ownership_change
after_update :after_ownership_change
@@ -30,7 +31,8 @@ class Group < ArvadosModel
after_create :add_role_manage_link
after_update :update_trash
- before_destroy :clear_permissions_and_trash
+ after_update :update_frozen
+ before_destroy :clear_permissions_trash_frozen
api_accessible :user, extend: :common do |t|
t.add :name
@@ -161,6 +163,22 @@ on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
end
end
+ def update_frozen
+ return unless saved_change_to_frozen_by_uuid? || saved_change_to_owner_uuid?
+ temptable = "group_subtree_#{rand(2**64).to_s(10)}"
+ ActiveRecord::Base.connection.exec_query(
+ "create temporary table #{temptable} on commit drop as select * from project_subtree_with_is_frozen($1,$2)",
+ "Group.update_frozen.select",
+ [[nil, self.uuid],
+ [nil, !self.frozen_by_uuid.nil?]])
+ ActiveRecord::Base.connection.exec_delete(
+ "delete from frozen_groups where uuid in (select uuid from #{temptable} where not is_frozen)",
+ "Group.update_frozen.delete")
+ ActiveRecord::Base.connection.exec_query(
+ "insert into frozen_groups (uuid) select uuid from #{temptable} where is_frozen on conflict do nothing",
+ "Group.update_frozen.insert")
+ end
+
def before_ownership_change
if owner_uuid_changed? and !self.owner_uuid_was.nil?
MaterializedPermission.where(user_uuid: owner_uuid_was, target_uuid: uuid).delete_all
@@ -174,12 +192,13 @@ on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
end
end
- def clear_permissions_and_trash
+ def clear_permissions_trash_frozen
MaterializedPermission.where(target_uuid: uuid).delete_all
- ActiveRecord::Base.connection.exec_delete %{
-delete from trashed_groups where group_uuid=$1
-}, "Group.clear_permissions_and_trash", [[nil, self.uuid]]
-
+ ['trashed_groups', 'frozen_groups'].each do |table|
+ ActiveRecord::Base.connection.exec_delete %{
+ delete from #{table} where group_uuid=$1
+ }, "Group.clear_permissions_trash_frozen", [[nil, self.uuid]]
+ end
end
def assign_name
@@ -217,4 +236,15 @@ delete from trashed_groups where group_uuid=$1
end
end
end
+
+ def permission_to_update
+ if !super
+ return false
+ elsif frozen_by_uuid && frozen_by_uuid_in_database
+ errors.add :uuid, "#{uuid} is frozen and cannot be modified"
+ return false
+ else
+ return true
+ end
+ end
end
diff --git a/services/api/db/migrate/20220301155729_frozen_groups.rb b/services/api/db/migrate/20220301155729_frozen_groups.rb
new file mode 100644
index 000000000..730d051ce
--- /dev/null
+++ b/services/api/db/migrate/20220301155729_frozen_groups.rb
@@ -0,0 +1,39 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require '20200501150153_permission_table_constants'
+
+class FrozenGroups < ActiveRecord::Migration[5.0]
+ def up
+ create_table :frozen_groups, :id => false do |t|
+ t.string :uuid
+ end
+ add_index :frozen_groups, :uuid, :unique => true
+
+ ActiveRecord::Base.connection.execute %{
+create or replace function project_subtree_with_is_frozen (starting_uuid varchar(27), starting_is_frozen boolean)
+returns table (uuid varchar(27), is_frozen boolean)
+STABLE
+language SQL
+as $$
+WITH RECURSIVE
+ project_subtree(uuid, is_frozen) as (
+ values (starting_uuid, starting_is_frozen)
+ union
+ select groups.uuid, project_subtree.is_frozen or groups.frozen_by_uuid is not null
+ from groups join project_subtree on project_subtree.uuid = groups.owner_uuid
+ )
+ select uuid, is_frozen from project_subtree;
+$$;
+}
+
+ # Initialize the table. After this, it is updated incrementally.
+ # See app/models/group.rb#update_frozen_groups
+ refresh_frozen
+ end
+
+ def down
+ drop_table :frozen_groups
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 573a4375c..cf92bcad9 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -190,6 +190,24 @@ case (edges.edge_id = perm_edge_id)
$$;
+--
+-- Name: project_subtree_with_is_frozen(character varying, boolean); Type: FUNCTION; Schema: public; Owner: -
+--
+
+CREATE FUNCTION public.project_subtree_with_is_frozen(starting_uuid character varying, starting_is_frozen boolean) RETURNS TABLE(uuid character varying, is_frozen boolean)
+ LANGUAGE sql STABLE
+ AS $$
+WITH RECURSIVE
+ project_subtree(uuid, is_frozen) as (
+ values (starting_uuid, starting_is_frozen)
+ union
+ select groups.uuid, project_subtree.is_frozen or groups.frozen_by_uuid is not null
+ from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid)
+ )
+ select uuid, is_frozen from project_subtree;
+$$;
+
+
--
-- Name: project_subtree_with_trash_at(character varying, timestamp without time zone); Type: FUNCTION; Schema: public; Owner: -
--
@@ -548,6 +566,15 @@ CREATE SEQUENCE public.containers_id_seq
ALTER SEQUENCE public.containers_id_seq OWNED BY public.containers.id;
+--
+-- Name: frozen_groups; Type: TABLE; Schema: public; Owner: -
+--
+
+CREATE TABLE public.frozen_groups (
+ uuid character varying
+);
+
+
--
-- Name: groups; Type: TABLE; Schema: public; Owner: -
--
@@ -2059,6 +2086,13 @@ CREATE INDEX index_containers_on_secret_mounts_md5 ON public.containers USING bt
CREATE UNIQUE INDEX index_containers_on_uuid ON public.containers USING btree (uuid);
+--
+-- Name: index_frozen_groups_on_uuid; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE UNIQUE INDEX index_frozen_groups_on_uuid ON public.frozen_groups USING btree (uuid);
+
+
--
-- Name: index_groups_on_created_at; Type: INDEX; Schema: public; Owner: -
--
@@ -3149,6 +3183,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20210621204455'),
('20210816191509'),
('20211027154300'),
-('20220224203102');
+('20220224203102'),
+('20220301155729');
diff --git a/services/api/lib/20200501150153_permission_table_constants.rb b/services/api/lib/20200501150153_permission_table_constants.rb
index 74c15bc2e..7ee503936 100644
--- a/services/api/lib/20200501150153_permission_table_constants.rb
+++ b/services/api/lib/20200501150153_permission_table_constants.rb
@@ -15,8 +15,8 @@
# update_permissions reference that file instead.
PERMISSION_VIEW = "materialized_permissions"
-
TRASHED_GROUPS = "trashed_groups"
+FROZEN_GROUPS = "frozen_groups"
# We need to use this parameterized query in a few different places,
# including as a subquery in a larger query.
@@ -83,3 +83,21 @@ INSERT INTO materialized_permissions
}, "refresh_permission_view.do"
end
end
+
+def refresh_frozen
+ ActiveRecord::Base.transaction do
+ ActiveRecord::Base.connection.execute("LOCK TABLE #{FROZEN_GROUPS}")
+ ActiveRecord::Base.connection.execute("DELETE FROM #{FROZEN_GROUPS}")
+
+ # Compute entire frozen_groups table, starting with top-level
+ # projects (i.e., project groups owned by a user).
+ ActiveRecord::Base.connection.execute(%{
+INSERT INTO #{FROZEN_GROUPS}
+select ps.uuid from groups,
+ lateral project_subtree_with_is_frozen(groups.uuid, groups.frozen_by_uuid is not null) ps
+ where groups.owner_uuid like '_____-tpzed-_______________'
+ and group_class = 'project'
+ and ps.is_frozen
+})
+ end
+end
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index f3367118c..d71a0b5f9 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -319,7 +319,7 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
Rails.configuration.API.UnfreezeProjectRequiresAdmin = false
proj = Group.create!(group_class: 'project', name: 'freeze-test', owner_uuid: users(:active).uuid)
proj_inner = Group.create!(group_class: 'project', name: 'freeze-test-inner', owner_uuid: proj.uuid)
- coll = Collection.create!(name: 'foo', manifest_text: '')
+ coll = Collection.create!(name: 'foo', manifest_text: '', owner_uuid: proj_inner.uuid)
# Cannot set frozen_by_uuid to a different user
assert_raises do
@@ -357,35 +357,19 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
properties: {'frobity' => 'bar baz'})
assert ok, proj.errors.messages.inspect
- # Once project is frozen, cannot change name/contents, trash, or
- # delete the project or anything beneath it
- [proj, proj_inner, coll].each do |frozen|
- assert_raises do
- frozen.update_attributes!(name: 'foo2')
- end
- if frozen.is_a?(Collection)
- assert_raises do
- frozen.update_attributes!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n")
- end
- end
- assert_raises do
- frozen.update_attributes!(trash_at: db_current_time)
- end
- assert_raises do
- frozen.destroy
- end
- end
-
# Once project is frozen, cannot create new items inside it or
# its descendants
- [proj, proj_inner].each do |parent|
+ [proj, proj_inner].each do |frozen|
assert_raises do
- Collection.create!(owner_uuid: parent.uuid, name: 'inside-frozen-project')
+ collections(:collection_owned_by_active).update_attributes!(owner_uuid: frozen.uuid)
end
assert_raises do
- Group.create!(owner_uuid: parent.uuid, group_class: 'project', name: 'inside-frozen-project')
+ Collection.create!(owner_uuid: frozen.uuid, name: 'inside-frozen-project')
end
- cr = ContainerRequest.create(
+ assert_raises do
+ Group.create!(owner_uuid: frozen.uuid, group_class: 'project', name: 'inside-frozen-project')
+ end
+ cr = ContainerRequest.new(
command: ["echo", "foo"],
container_image: links(:docker_image_collection_tag).name,
cwd: "/tmp",
@@ -395,12 +379,39 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
runtime_constraints: {"vcpus" => 1, "ram" => 2},
name: "foo",
description: "bar",
- owner_uuid: parent.uuid,
+ owner_uuid: frozen.uuid,
)
- assert_raises do
- cr.save!
+ assert_raises ArvadosModel::PermissionDeniedError do
+ cr.save
+ end
+ assert_match /frozen/, cr.errors.inspect
+ # Check the frozen-parent condition is the only reason save failed.
+ cr.owner_uuid = users(:active).uuid
+ assert cr.save
+ cr.destroy
+ end
+
+ # Once project is frozen, cannot change name/contents, trash, or
+ # delete the project or anything beneath it
+ [proj, proj_inner, coll].each do |frozen|
+ assert_raises(StandardError, "should reject rename of #{frozen.uuid} with parent #{frozen.owner_uuid}") do
+ frozen.update_attributes!(name: 'foo2')
+ end
+ frozen.reload
+ if frozen.is_a?(Collection)
+ assert_raises(StandardError, "should reject manifest change of #{frozen.uuid}") do
+ frozen.update_attributes!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n")
+ end
+ frozen.reload
+ end
+ assert_raises(StandardError, "should reject setting trash_at of #{frozen.uuid}") do
+ frozen.update_attributes!(trash_at: db_current_time)
+ end
+ frozen.reload
+ assert_raises(StandardError, "should reject delete of #{frozen.uuid}") do
+ frozen.destroy
end
- assert_match cr.errors.inspect, /frozen/
+ frozen.reload
end
# User with manage permission can unfreeze, then create items
commit b586b7eece173fe0cf0ce5e53aa8969f1239ef16
Author: Tom Clegg <tom at curii.com>
Date: Mon Feb 28 11:32:39 2022 -0500
18691: Add groups.frozen_by_uuid attribute.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 9800be704..656385cc1 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -240,6 +240,18 @@ Clusters:
# https://doc.arvados.org/admin/metadata-vocabulary.html
VocabularyPath: ""
+ # If true, a project must have a non-empty description field in
+ # order to be frozen.
+ FreezeProjectRequiresDescription: false
+
+ # Project properties that must have non-empty values in order to
+ # freeze a project. Example: {"property_name": true}
+ FreezeProjectRequiresProperties: {}
+
+ # If true, only an admin user can un-freeze a project. If false,
+ # any user with "manage" permission can un-freeze.
+ UnfreezeProjectRequiresAdmin: false
+
Users:
# Config parameters to automatically setup new users. If enabled,
# this users will be able to self-activate. Enable this if you want
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index b8c8269f1..e0750bd8c 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -94,21 +94,24 @@ type Cluster struct {
PostgreSQL PostgreSQL
API struct {
- AsyncPermissionsUpdateInterval Duration
- DisabledAPIs StringSet
- MaxIndexDatabaseRead int
- MaxItemsPerResponse int
- MaxConcurrentRequests int
- MaxKeepBlobBuffers int
- MaxRequestAmplification int
- MaxRequestSize int
- MaxTokenLifetime Duration
- RequestTimeout Duration
- SendTimeout Duration
- WebsocketClientEventQueue int
- WebsocketServerEventQueue int
- KeepServiceRequestTimeout Duration
- VocabularyPath string
+ AsyncPermissionsUpdateInterval Duration
+ DisabledAPIs StringSet
+ MaxIndexDatabaseRead int
+ MaxItemsPerResponse int
+ MaxConcurrentRequests int
+ MaxKeepBlobBuffers int
+ MaxRequestAmplification int
+ MaxRequestSize int
+ MaxTokenLifetime Duration
+ RequestTimeout Duration
+ SendTimeout Duration
+ WebsocketClientEventQueue int
+ WebsocketServerEventQueue int
+ KeepServiceRequestTimeout Duration
+ VocabularyPath string
+ FreezeProjectRequiresDescription bool
+ FreezeProjectRequiresProperties StringSet
+ UnfreezeProjectRequiresAdmin bool
}
AuditLogs struct {
MaxAge Duration
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index 8565b2a41..d6f698432 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -19,6 +19,7 @@ class Group < ArvadosModel
validate :ensure_filesystem_compatible_name
validate :check_group_class
validate :check_filter_group_filters
+ validate :check_frozen_state_change_allowed
before_create :assign_name
after_create :after_ownership_change
after_create :update_trash
@@ -40,6 +41,7 @@ class Group < ArvadosModel
t.add :trash_at
t.add :is_trashed
t.add :properties
+ t.add :frozen_by_uuid
end
def ensure_filesystem_compatible_name
@@ -92,6 +94,40 @@ class Group < ArvadosModel
end
end
+ def check_frozen_state_change_allowed
+ if frozen_by_uuid == ""
+ self.frozen_by_uuid = nil
+ end
+ if frozen_by_uuid_changed? || (new_record? && frozen_by_uuid)
+ if group_class != "project"
+ errors.add(:frozen_by_uuid, "cannot be modified on a non-project group")
+ return
+ end
+ if frozen_by_uuid_was && Rails.configuration.API.UnfreezeProjectRequiresAdmin && !current_user.is_admin
+ errors.add(:frozen_by_uuid, "can only be changed by an admin user, once set")
+ return
+ end
+ if frozen_by_uuid && frozen_by_uuid != current_user.uuid
+ errors.add(:frozen_by_uuid, "can only be set to the current user's UUID")
+ return
+ end
+ if !new_record? && !current_user.can?(manage: uuid)
+ raise PermissionDeniedError
+ end
+ if frozen_by_uuid_was.nil?
+ if Rails.configuration.API.FreezeProjectRequiresDescription && !attribute_present?(:description)
+ errors.add(:frozen_by_uuid, "can only be set if description is non-empty")
+ end
+ Rails.configuration.API.FreezeProjectRequiresProperties.andand.each do |key, _|
+ key = key.to_s
+ if !properties[key] || properties[key] == ""
+ errors.add(:frozen_by_uuid, "can only be set if properties[#{key}] value is non-empty")
+ end
+ end
+ end
+ end
+ end
+
def update_trash
if saved_change_to_trash_at? or saved_change_to_owner_uuid?
# The group was added or removed from the trash.
diff --git a/services/api/db/migrate/20220224203102_add_frozen_by_uuid_to_groups.rb b/services/api/db/migrate/20220224203102_add_frozen_by_uuid_to_groups.rb
new file mode 100644
index 000000000..d730b2535
--- /dev/null
+++ b/services/api/db/migrate/20220224203102_add_frozen_by_uuid_to_groups.rb
@@ -0,0 +1,9 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class AddFrozenByUuidToGroups < ActiveRecord::Migration[5.2]
+ def change
+ add_column :groups, :frozen_by_uuid, :string
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index da9959593..573a4375c 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -567,7 +567,8 @@ CREATE TABLE public.groups (
trash_at timestamp without time zone,
is_trashed boolean DEFAULT false NOT NULL,
delete_at timestamp without time zone,
- properties jsonb DEFAULT '{}'::jsonb
+ properties jsonb DEFAULT '{}'::jsonb,
+ frozen_by_uuid character varying
);
@@ -3147,6 +3148,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20210126183521'),
('20210621204455'),
('20210816191509'),
-('20211027154300');
+('20211027154300'),
+('20220224203102');
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index 10932e116..f3367118c 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -313,4 +313,124 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
assert Link.where(tail_uuid: g3, head_uuid: g4, link_class: "permission", name: "can_manage").any?
assert !Link.where(link_class: 'permission', name: 'can_manage', tail_uuid: g5, head_uuid: g4).any?
end
+
+ test "freeze project" do
+ act_as_user users(:active) do
+ Rails.configuration.API.UnfreezeProjectRequiresAdmin = false
+ proj = Group.create!(group_class: 'project', name: 'freeze-test', owner_uuid: users(:active).uuid)
+ proj_inner = Group.create!(group_class: 'project', name: 'freeze-test-inner', owner_uuid: proj.uuid)
+ coll = Collection.create!(name: 'foo', manifest_text: '')
+
+ # Cannot set frozen_by_uuid to a different user
+ assert_raises do
+ proj.update_attributes!(frozen_by_uuid: users(:spectator).uuid)
+ end
+ proj.reload
+
+ # Cannot set frozen_by_uuid without description (if so configured)
+ Rails.configuration.API.FreezeProjectRequiresDescription = true
+ err = assert_raises do
+ proj.update_attributes!(frozen_by_uuid: users(:active).uuid)
+ end
+ assert_match /can only be set if description is non-empty/, err.inspect
+ proj.reload
+ err = assert_raises do
+ proj.update_attributes!(frozen_by_uuid: users(:active).uuid, description: '')
+ end
+ assert_match /can only be set if description is non-empty/, err.inspect
+ proj.reload
+
+ # Cannot set frozen_by_uuid without properties (if so configured)
+ Rails.configuration.API.FreezeProjectRequiresProperties['frobity'] = true
+ err = assert_raises do
+ proj.update_attributes!(
+ frozen_by_uuid: users(:active).uuid,
+ description: 'ready to freeze')
+ end
+ assert_match /can only be set if properties\[frobity\] value is non-empty/, err.inspect
+ proj.reload
+
+ # Can set frozen_by_uuid if all conditions are met
+ ok = proj.update_attributes(
+ frozen_by_uuid: users(:active).uuid,
+ description: 'ready to freeze',
+ properties: {'frobity' => 'bar baz'})
+ assert ok, proj.errors.messages.inspect
+
+ # Once project is frozen, cannot change name/contents, trash, or
+ # delete the project or anything beneath it
+ [proj, proj_inner, coll].each do |frozen|
+ assert_raises do
+ frozen.update_attributes!(name: 'foo2')
+ end
+ if frozen.is_a?(Collection)
+ assert_raises do
+ frozen.update_attributes!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n")
+ end
+ end
+ assert_raises do
+ frozen.update_attributes!(trash_at: db_current_time)
+ end
+ assert_raises do
+ frozen.destroy
+ end
+ end
+
+ # Once project is frozen, cannot create new items inside it or
+ # its descendants
+ [proj, proj_inner].each do |parent|
+ assert_raises do
+ Collection.create!(owner_uuid: parent.uuid, name: 'inside-frozen-project')
+ end
+ assert_raises do
+ Group.create!(owner_uuid: parent.uuid, group_class: 'project', name: 'inside-frozen-project')
+ end
+ cr = ContainerRequest.create(
+ command: ["echo", "foo"],
+ container_image: links(:docker_image_collection_tag).name,
+ cwd: "/tmp",
+ environment: {},
+ mounts: {"/out" => {"kind" => "tmp", "capacity" => 1000000}},
+ output_path: "/out",
+ runtime_constraints: {"vcpus" => 1, "ram" => 2},
+ name: "foo",
+ description: "bar",
+ owner_uuid: parent.uuid,
+ )
+ assert_raises do
+ cr.save!
+ end
+ assert_match cr.errors.inspect, /frozen/
+ end
+
+ # User with manage permission can unfreeze, then create items
+ # inside it and its children
+ assert proj.update_attributes(frozen_by_uuid: nil)
+ assert Collection.create!(owner_uuid: proj.uuid, name: 'inside-unfrozen-project')
+ assert Collection.create!(owner_uuid: proj_inner.uuid, name: 'inside-inner-unfrozen-project')
+
+ # Re-freeze, and reconfigure so only admins can unfreeze.
+ assert proj.update_attributes(frozen_by_uuid: users(:active).uuid)
+ Rails.configuration.API.UnfreezeProjectRequiresAdmin = true
+
+ # Owner cannot unfreeze, because not admin.
+ err = assert_raises do
+ proj.update_attributes!(frozen_by_uuid: nil)
+ end
+ assert_match /can only be changed by an admin user, once set/, err.inspect
+ proj.reload
+
+ act_as_user users(:admin) do
+ # Even admin cannot change frozen_by_uuid to someone else's UUID.
+ err = assert_raises do
+ proj.update_attributes!(frozen_by_uuid: users(:project_viewer).uuid)
+ end
+ assert_match /can only be set to the current user's UUID/, err.inspect
+ proj.reload
+
+ # Admin can unfreeze.
+ assert proj.update_attributes(frozen_by_uuid: nil), proj.errors.messages
+ end
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list