[ARVADOS] updated: 1.3.0-2689-g5502559ac

Git user git at public.arvados.org
Wed Jun 17 23:54:12 UTC 2020


Summary of changes:
 services/api/lib/current_api_client.rb     |  2 +-
 services/api/lib/fix_roles_projects.rb     |  4 ++--
 services/api/test/fixtures/collections.yml | 13 +++++++++++++
 3 files changed, 16 insertions(+), 3 deletions(-)

  discards  7835d752ba1d320153f32e4de9b96c20b20ed8c0 (commit)
  discards  0c73874964316de30579968ed0ad8c17be065333 (commit)
       via  5502559ac286dcf807261cec86b983f061788908 (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (7835d752ba1d320153f32e4de9b96c20b20ed8c0)
            \
             N -- N -- N (5502559ac286dcf807261cec86b983f061788908)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 5502559ac286dcf807261cec86b983f061788908
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Wed Jun 17 17:34:34 2020 -0400

    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 <peter.amstutz at curii.com>

diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid
index edd92fa0e..bf584d8b6 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
 <div class="releasenotes">
 </notextile>
 
-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 at . Invalid group_class are migrated to @role at .
+# 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 at .  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 at .
+
+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):
+
+<pre>
+arv group list --filters '[["group_class", "not in", ["project", "role"]]]'
+</pre>
+
+To list all @role@ groups, which will be reassigned to the system user (unless @owner_uuid@ is already the system user):
+
+<pre>
+arv group list --filters '[["group_class", "=", "role"]]'
+</pre>
+
+To list which @project@ groups have outgoing permission links.  Such links are now invalid and will be deleted by the migration:
+
+<pre>
+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
+</pre>
+
 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 1f08ea419..4e04699fc 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 at .  *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 at .  *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 at .
-* Applications should represent each object as belonging to, or being "inside", the Group/User referenced by its @owner_uuid at .
-** A "project" is a subtype of Group that is treated as a "Project" in Workbench, and as a directory by @arv-mount at .
-** 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 at .
 
-* 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 at .
+* 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 at .  The anonymous group uuid is @{siteprefix}-j7d0g-anonymouspublic at .
+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 at .  The anonymous group uuid is @{siteprefix}-j7d0g-anonymouspublic at .
 
 h2. Example
 
diff --git a/services/api/app/models/database_seeds.rb b/services/api/app/models/database_seeds.rb
index a86a2c854..39f491503 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 21e57e143..02c6a242f 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 60a94c7e2..e4ba7f3de 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 f3ec0a03e..98112c858 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 448c50cee..5dd127b3e 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 1581039bb..a16ee8763 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 07709b752..10664474c 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)
 

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list