From: Peter Amstutz Date: Wed, 17 Jun 2020 21:34:34 +0000 (-0400) Subject: 16007: Update docs, restore empty_collection, fix tests X-Git-Tag: 2.1.0~188^2~3 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/5502559ac286dcf807261cec86b983f061788908 16007: Update docs, restore empty_collection, fix tests Also include link uuid in migration log message Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid index edd92fa0ea..bf584d8b60 100644 --- a/doc/admin/upgrading.html.textile.liquid +++ b/doc/admin/upgrading.html.textile.liquid @@ -34,7 +34,7 @@ TODO: extract this information based on git commit messages and generate changel
-h2(#master). development master (as of 2020-02-07) +h2(#master). development master (as of 2020-06-17) "Upgrading from 2.0.0":#v2_0_0 @@ -48,6 +48,39 @@ h3. S3 signatures Keepstore now uses "V4 signatures":https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-authenticating-requests.html by default for S3 requests. If you are using Amazon S3, no action is needed; all regions support V4 signatures. If you are using a different S3-compatible service that does not support V4 signatures, add @V2Signature: true@ to your volume driver parameters to preserve the old behavior. See "configuring S3 object storage":{{site.baseurl}}/install/configure-s3-object-storage.html. +h3. New permission system constraints + +Some constraints on the permission system have been added, in particular @role@ and @project@ group types now have distinct behavior. These constraints were already de-facto imposed by the Workbench UI, so on most installations the only effect of this migration will be to reassign @role@ groups to the system user and create a @can_manage@ permission link for the previous owner. + +# The @group_class@ field must be either @role@ or @project@. Invalid group_class are migrated to @role@. +# A @role@ cannot own things. Anything owned by a role is migrated to a @can_manage@ link and reassigned to the system user. +# Only @role@ and @user@ can have outgoing permission links. Permission links originating from projects are deleted by the migration. +# A @role@ is always owned by the system_user. When a group is created, it creates a @can_manage@ link for the object that would have been assigned to @owner_uuid@. Migration adds @can_manage@ links and reassigns roles to the system user. This also has the effect of requiring that all @role@ groups have unique names on the system. +# A permission link can have the permission level (@name@) updated but not @head_uuid@, @tail_uuid@ or @link_class@. + +The @arvados-sync-groups@ tool has been updated to reflect these constraints. + +To determine which groups have invalid @group_class@ with this command (these will be migrated to @role@ groups): + +
+arv group list --filters '[["group_class", "not in", ["project", "role"]]]'
+
+ +To list all @role@ groups, which will be reassigned to the system user (unless @owner_uuid@ is already the system user): + +
+arv group list --filters '[["group_class", "=", "role"]]'
+
+ +To list which @project@ groups have outgoing permission links. Such links are now invalid and will be deleted by the migration: + +
+for uuid in $(arv link list --filters '[["link_class", "=", "permission"], ["tail_uuid", "like", "%-j7d0g-%"]]' |
+              jq -r .items[].tail_uuid | sort | uniq) ; do
+   arv group list --filters '[["group_class", "=", "project"], ["uuid", "=", "'$uuid'"]]' | jq .items
+done
+
+ h2(#v2_0_0). v2.0.0 (2020-02-07) "Upgrading from 1.4":#v1_4_1 diff --git a/doc/api/permission-model.html.textile.liquid b/doc/api/permission-model.html.textile.liquid index 1f08ea4195..4e04699fcd 100644 --- a/doc/api/permission-model.html.textile.liquid +++ b/doc/api/permission-model.html.textile.liquid @@ -10,59 +10,84 @@ Copyright (C) The Arvados Authors. All rights reserved. SPDX-License-Identifier: CC-BY-SA-3.0 {% endcomment %} -* There are four levels of permission: *none*, *can_read*, *can_write*, and *can_manage*. -** *none* is the default state when there are no other permission grants. -*** the object is not included in any list query response. -*** direct queries of the object by uuid return 404 Not Found. -*** Link objects require valid identifiers in @head_uuid@ and @tail_uuid@, so an attempt to create a Link that references an unreadable object will return an error indicating the object is not found. -** *can_read* grants read-only access to the record. Attempting to update or delete the record returns an error. *can_read* does not allow a reader to see any permission grants on the object except the object's owner_uuid and the reader's own permissions. -** *can_write* permits changes to the record (but not permission links). *can_write* permits the user to delete the object. *can_write* also implies *can_read*. -** *can_manage* permits the user to read, create, update and delete permission links whose @head_uuid@ is this object's @uuid@. *can_manage* also implies *can_write* and *can_read*. +There are four levels of permission: *none*, *can_read*, *can_write*, and *can_manage*. + +* *none* is the default state when there are no other permission grants. +** the object is not included in any list query response. +** direct queries of the object by uuid return 404 Not Found. +** Link objects require valid identifiers in @head_uuid@ and @tail_uuid@, so an attempt to create a Link that references an unreadable object will return an error indicating the object is not found. +* *can_read* grants read-only access to the record. Attempting to update or delete the record returns an error. *can_read* does not allow a reader to see any permission grants on the object except the object's owner_uuid and the reader's own permissions. +* *can_write* permits changes to the record (but not permission links). *can_write* permits the user to delete the object. *can_write* also implies *can_read*. +* *can_manage* permits the user to read, create, update and delete permission links whose @head_uuid@ is this object's @uuid@. *can_manage* also implies *can_write* and *can_read*. h2. Ownership -* All Arvados objects have an @owner_uuid@ field. Valid uuid types for @owner_uuid@ are "User" and "Group". -* The User or Group specified by @owner_uuid@ has *can_manage* permission on the object. -** This permission is one way: A User or Group's @owner_uuid@ being equal to @X@ does not imply any permission for that User/Group to read, write, or manage an object whose @uuid@ is equal to @X@. -* Applications should represent each object as belonging to, or being "inside", the Group/User referenced by its @owner_uuid@. -** A "project" is a subtype of Group that is treated as a "Project" in Workbench, and as a directory by @arv-mount@. -** A "role" is a subtype of Group that is treated in Workbench as a group of users who have permissions in common (typically an organizational group). -* To change the @owner_uuid@ field, it is necessary to have @can_write@ permission on both the current owner and the new owner. +All Arvados objects have an @owner_uuid@ field. Valid uuid types for @owner_uuid@ are "User" and "Group". For Group, the @group_class@ must be a "project". + +The User or Group specified by @owner_uuid@ has *can_manage* permission on the object. This permission is one way: an object that is owned does not get any special permissions on the User or Group that owns it. + +To change the @owner_uuid@ field, it is necessary to have @can_write@ permission on both the current owner and the new owner. h2(#links). Permission links -A link object with +A permission link is a link object with: * @owner_uuid@ of the system user. -* @link_class@ "permission" +* @link_class@ *permission* * @name@ one of *can_read*, *can_write* or *can_manage* * @head_uuid@ of some Arvados object * @tail_uuid@ of a User or Group -grants the @name@ permission for @tail_uuid@ accessing @head_uuid@ +This grants the permission in @name@ for @tail_uuid@ accessing @head_uuid@. -* If a User has *can_manage* permission on some object, this grants permission to read, create, update and delete permission links where the @head_uuid@ is the object under management. +If a User has *can_manage* permission on some object, the user has the ability to read, create, update and delete permission links with @head_uuid@ of the managed object (i.e. permission grants on the object). h3. Transitive permissions -Permissions can be obtained indirectly through Groups. +Permissions can be obtained indirectly by following multiple permission links or nested ownership. + * If a User X *can_read* Group A, and Group A *can_read* Object B, then User X *can_read* Object B. * Permissions are narrowed to the least powerful permission on the path. ** If User X *can_write* Group A, and Group A *can_read* Object B, then User X *can_read* Object B. ** If User X *can_read* Group A, and Group A *can_write* Object B, then User X *can_read* Object B. -h2. Group Membership +h2. Projects and Roles + +A "project" is a subtype of Group that is displayed as a "Project" in Workbench, and as a directory by @arv-mount@. +* A project can own things (appear in @owner_uuid@) +* A project can be owned by a user or another project. +* The name of a project is unique only among projects with the same owner_uuid. +* Projects can be the target (@head_uuid@) of a permission link, but not the origin (@tail_uuid@). Putting a project in a @tail_uuid@ field is an error. + +A "role" is a subtype of Group that is treated in Workbench as a group of users who have permissions in common (typically an organizational group). +* A role cannot own things (cannot appear in @owner_uuid@). Putting a role in an @owner_uuid@ field is an error. +* All roles are owned by the system user. +* The name of a role is unique across an instance. +* A role can be both the target (head_uuid) and origin (tail_uuid) of a permission link. + +h3. Access through Roles -Group membership is determined by whether the group has *can_read* permission on an object. If a group G *can_read* an object A, then we say A is a member of G. +A "role" consists of a set of users or other roles that have that role, and a set of permissions (primarily read/write/manage access to projects) the role grants. -For some kinds of groups, like roles, it is natural for users who are members of a group to also have *can_manage* permission on the group, i.e., G *can_read* A and A *can_manage* G ("A can do anything G can do"). However, this is not necessary: A can be a member of a group while being unable to even read it. +If there is a permission link stating that user A *can_write* role R, then we say A has role R. This means user A has up to *can_write* access to everything the role has access to. + +Because permissions are one-way, the links A *can_write* R and B *can_write* R does not imply that user A and B will be able to see each other. For users in a role to see each other, read permission should be added going in the opposite direction: R *can_read* A and R *can_read* B. + +If a user needs to be able to manipulate permissions of objects that are accessed through the role (for example, to share project P with a user outside the role), then role R must have *can_manage* permission on project P (R *can_manage* P) and the user must be granted *can_manage* permission on R (A *can_manage* R). h2. Special cases -* Log table objects are additionally readable based on whether the User has *can_read* permission on @object_uuid@ (User can access log history about objects it can read). To retain the integrity of the log, the log table should deny all update or delete operations. -* Permission links where @tail_uuid@ is a User permit @can_read@ on the link by that user. (User can discover her own permission grants.) -* *can_read* on a Collection grants permission to read the blocks that make up the collection (API server returns signed blocks) -* If User or Group X *can_FOO* Group A, and Group A *can_manage* User B, then X *can_FOO* _everything that User B can_FOO_. +Log table objects are additionally readable based on whether the User has *can_read* permission on @object_uuid@ (User can access log history about objects it can read). To retain the integrity of the log, the log table denies all update or delete operations. + +Permission links where @tail_uuid@ is a User allow @can_read@ on the link record by that user. (User can discover her own permission grants.) + +At least *can_read* on a Collection grants permission to read the blocks that make up the collection (API server returns signed blocks). + +A user can only read a container record if the user has read permission to a container_request with that container_uuid. + +*can_read* and *can_write* access on a user grants access to the user record, but not anything owned by the user. +*can_manage* access to a user grants can_manage access to the user, _and everything owned by that user_. +If a user A *can_read* role R, and role R *can_manage* user B, then user A *can_read* user B _and everything owned by that user_. h2(#system). System user and group @@ -70,7 +95,7 @@ A privileged user account exists for the use by internal Arvados components. Th h2. Anoymous user and group -An Arvados site may be configured to allow users to browse resources without requiring a login. In this case, permissions for non-logged-in users are associated with the "anonymous" user. To make objects visible to the public, they can be shared with the "anonymous" group. The anonymous user uuid is @{siteprefix}-tpzed-anonymouspublic@. The anonymous group uuid is @{siteprefix}-j7d0g-anonymouspublic@. +An Arvados site may be configured to allow users to browse resources without requiring a login. In this case, permissions for non-logged-in users are associated with the "anonymous" user. To make objects visible to the public, they can be shared with the "anonymous" role. The anonymous user uuid is @{siteprefix}-tpzed-anonymouspublic@. The anonymous group uuid is @{siteprefix}-j7d0g-anonymouspublic@. h2. Example diff --git a/services/api/app/models/database_seeds.rb b/services/api/app/models/database_seeds.rb index a86a2c8544..39f491503e 100644 --- a/services/api/app/models/database_seeds.rb +++ b/services/api/app/models/database_seeds.rb @@ -13,6 +13,7 @@ class DatabaseSeeds anonymous_group anonymous_group_read_permission anonymous_user + empty_collection refresh_permissions refresh_trashed end diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb index 21e57e143e..02c6a242f9 100644 --- a/services/api/app/models/group.rb +++ b/services/api/app/models/group.rb @@ -119,9 +119,9 @@ delete from trashed_groups where group_uuid=$1 def ensure_owner_uuid_is_permitted if group_class == "role" - @role_creator = nil + @requested_manager_uuid = nil if new_record? - @role_creator = owner_uuid + @requested_manager_uuid = owner_uuid self.owner_uuid = system_user_uuid return true end @@ -136,9 +136,9 @@ delete from trashed_groups where group_uuid=$1 end def add_role_manage_link - if group_class == "role" && @role_creator + if group_class == "role" && @requested_manager_uuid act_as_system_user do - Link.create!(tail_uuid: @role_creator, + Link.create!(tail_uuid: @requested_manager_uuid, head_uuid: self.uuid, link_class: "permission", name: "can_manage") diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb index 60a94c7e23..e4ba7f3de1 100644 --- a/services/api/app/models/link.rb +++ b/services/api/app/models/link.rb @@ -63,7 +63,7 @@ class Link < ArvadosModel return false end if tail_obj.group_class != "role" - errors.add(:tail_uuid, "must be a role, was #{tail_obj.group_class}") + errors.add(:tail_uuid, "must be a user or role, was group with group_class #{tail_obj.group_class}") return false end elsif rsc_class != User diff --git a/services/api/lib/current_api_client.rb b/services/api/lib/current_api_client.rb index f3ec0a03e2..98112c858b 100644 --- a/services/api/lib/current_api_client.rb +++ b/services/api/lib/current_api_client.rb @@ -8,6 +8,7 @@ $all_users_group = nil $anonymous_user = nil $anonymous_group = nil $anonymous_group_read_permission = nil +$empty_collection = nil module CurrentApiClient def current_user @@ -192,6 +193,26 @@ module CurrentApiClient 'd41d8cd98f00b204e9800998ecf8427e+0' end + def empty_collection + $empty_collection = check_cache $empty_collection do + act_as_system_user do + ActiveRecord::Base.transaction do + Collection. + where(portable_data_hash: empty_collection_pdh). + first_or_create(manifest_text: '', owner_uuid: system_user.uuid, name: "empty collection") do |c| + c.save! + Link.where(tail_uuid: anonymous_group.uuid, + head_uuid: c.uuid, + link_class: 'permission', + name: 'can_read'). + first_or_create! + c + end + end + end + end + end + private # If the given value is nil, or the cache has been cleared since it diff --git a/services/api/lib/fix_roles_projects.rb b/services/api/lib/fix_roles_projects.rb index 448c50cee2..5dd127b3e2 100644 --- a/services/api/lib/fix_roles_projects.rb +++ b/services/api/lib/fix_roles_projects.rb @@ -11,23 +11,20 @@ def fix_roles_projects # shouldn't be anything to do at all. act_as_system_user do ActiveRecord::Base.transaction do - q = ActiveRecord::Base.connection.exec_query %{ -select uuid from groups limit 1 -} - - # 1) any group not group_class != project becomes a 'role' (both empty and invalid groups) - ActiveRecord::Base.connection.exec_query %{ -UPDATE groups set group_class='role' where group_class != 'project' or group_class is null - } - - Group.where("group_class='role' and owner_uuid != '#{system_user_uuid}'").each do |g| - # 2) Ownership of a role becomes a can_manage link - Link.create!(link_class: 'permission', - name: 'can_manage', - tail_uuid: g.owner_uuid, - head_uuid: g.uuid) + Group.where("group_class != 'project' or group_class is null").each do |g| + # 1) any group not group_class != project becomes a 'role' (both empty and invalid groups) + old_owner = g.owner_uuid g.owner_uuid = system_user_uuid + g.group_class = 'role' g.save_with_unique_name! + + if old_owner != system_user_uuid + # 2) Ownership of a role becomes a can_manage link + Link.create!(link_class: 'permission', + name: 'can_manage', + tail_uuid: old_owner, + head_uuid: g.uuid) + end end ActiveRecord::Base.descendants.reject(&:abstract_class?).each do |klass| @@ -66,8 +63,8 @@ select links.uuid from links, groups where groups.uuid = links.tail_uuid and } q.each do |lu| ln = Link.find_by_uuid(lu['uuid']) - puts "WARNING: Projects cannot have outgoing permission links, '#{ln.name}' link from #{ln.tail_uuid} to #{ln.head_uuid} will be removed" - Rails.logger.warn "Projects cannot have outgoing permission links, '#{ln.name}' link from #{ln.tail_uuid} to #{ln.head_uuid} will be removed" + puts "WARNING: Projects cannot have outgoing permission links, removing '#{ln.name}' link #{ln.uuid} from #{ln.tail_uuid} to #{ln.head_uuid}" + Rails.logger.warn "Projects cannot have outgoing permission links, removing '#{ln.name}' link #{ln.uuid} from #{ln.tail_uuid} to #{ln.head_uuid}" ln.destroy! end end diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml index 1581039bb3..a16ee8763f 100644 --- a/services/api/test/fixtures/collections.yml +++ b/services/api/test/fixtures/collections.yml @@ -199,7 +199,6 @@ unlinked_docker_image: empty: uuid: zzzzz-4zz18-gs9ooj1h9sd5mde current_version_uuid: zzzzz-4zz18-gs9ooj1h9sd5mde - # Empty collection owned by anonymous_group is added with rake db:seed. portable_data_hash: d41d8cd98f00b204e9800998ecf8427e+0 owner_uuid: zzzzz-tpzed-000000000000000 created_at: 2014-06-11T17:22:54Z @@ -208,7 +207,7 @@ empty: modified_at: 2014-06-11T17:22:54Z updated_at: 2014-06-11T17:22:54Z manifest_text: "" - name: empty_collection + name: "empty collection for python test" foo_collection_in_aproject: uuid: zzzzz-4zz18-fy296fx3hot09f7 diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb index 07709b7528..10664474c6 100644 --- a/services/api/test/unit/permission_test.rb +++ b/services/api/test/unit/permission_test.rb @@ -466,7 +466,7 @@ class PermissionTest < ActiveSupport::TestCase test "add user to group, then change permission level" do set_user_from_auth :admin - grp = Group.create!(owner_uuid: system_user_uuid, group_class: "role") + grp = Group.create!(owner_uuid: system_user_uuid, group_class: "project") col = Collection.create!(owner_uuid: grp.uuid) assert_empty Collection.readable_by(users(:active)).where(uuid: col.uuid) assert_empty User.readable_by(users(:active)).where(uuid: users(:project_viewer).uuid) @@ -475,10 +475,6 @@ class PermissionTest < ActiveSupport::TestCase head_uuid: grp.uuid, link_class: 'permission', name: 'can_manage') - l2 = Link.create!(tail_uuid: grp.uuid, - head_uuid: users(:active).uuid, - link_class: 'permission', - name: 'can_read') assert Collection.readable_by(users(:active)).where(uuid: col.uuid).first assert users(:active).can?(read: col.uuid) @@ -505,7 +501,7 @@ class PermissionTest < ActiveSupport::TestCase test "add user to group, then add overlapping permission link to group" do set_user_from_auth :admin - grp = Group.create!(owner_uuid: system_user_uuid, group_class: "role") + grp = Group.create!(owner_uuid: system_user_uuid, group_class: "project") col = Collection.create!(owner_uuid: grp.uuid) assert_empty Collection.readable_by(users(:active)).where(uuid: col.uuid) assert_empty User.readable_by(users(:active)).where(uuid: users(:project_viewer).uuid) @@ -514,10 +510,6 @@ class PermissionTest < ActiveSupport::TestCase head_uuid: grp.uuid, link_class: 'permission', name: 'can_manage') - l2 = Link.create!(tail_uuid: grp.uuid, - head_uuid: users(:active).uuid, - link_class: 'permission', - name: 'can_read') assert Collection.readable_by(users(:active)).where(uuid: col.uuid).first assert users(:active).can?(read: col.uuid) @@ -545,8 +537,14 @@ class PermissionTest < ActiveSupport::TestCase test "add user to group, then add overlapping permission link to subproject" do set_user_from_auth :admin - grp = Group.create!(owner_uuid: system_user_uuid, group_class: "project") - prj = Group.create!(owner_uuid: grp.uuid, group_class: "project") + grp = Group.create!(owner_uuid: system_user_uuid, group_class: "role") + prj = Group.create!(owner_uuid: system_user_uuid, group_class: "project") + + l0 = Link.create!(tail_uuid: grp.uuid, + head_uuid: prj.uuid, + link_class: 'permission', + name: 'can_manage') + assert_empty Group.readable_by(users(:active)).where(uuid: prj.uuid) assert_empty User.readable_by(users(:active)).where(uuid: users(:project_viewer).uuid)