[ARVADOS] updated: 1.3.0-2622-ge37d3046e

Git user git at public.arvados.org
Tue Jun 2 20:47:16 UTC 2020


Summary of changes:
 .../migrate/20200602141328_fix_roles_projects.rb   | 13 +++++
 services/api/lib/fix_roles_projects.rb             | 63 ++++++++++++++++++++++
 services/api/test/fixtures/links.yml               | 14 -----
 services/api/test/unit/group_test.rb               | 50 +++++++++++++++++
 4 files changed, 126 insertions(+), 14 deletions(-)
 create mode 100644 services/api/db/migrate/20200602141328_fix_roles_projects.rb
 create mode 100644 services/api/lib/fix_roles_projects.rb

       via  e37d3046e74508a54e171ae2661ce1e38c8f0edc (commit)
      from  0c1b46dcfb5fce2f8fc73587adaeebfde8fa9268 (commit)

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 e37d3046e74508a54e171ae2661ce1e38c8f0edc
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Tue Jun 2 16:45:21 2020 -0400

    16007: Add migration to fix invalid groups & permission links
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/services/api/db/migrate/20200602141328_fix_roles_projects.rb b/services/api/db/migrate/20200602141328_fix_roles_projects.rb
new file mode 100644
index 000000000..f9b2bd6fa
--- /dev/null
+++ b/services/api/db/migrate/20200602141328_fix_roles_projects.rb
@@ -0,0 +1,13 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'fix_roles_projects'
+
+class FixRolesProjects < ActiveRecord::Migration[5.0]
+  def change
+    # This migration is not reversible.  However, the results are
+    # backwards compatible.
+    fix_roles_projects
+  end
+end
diff --git a/services/api/lib/fix_roles_projects.rb b/services/api/lib/fix_roles_projects.rb
new file mode 100644
index 000000000..dafef61aa
--- /dev/null
+++ b/services/api/lib/fix_roles_projects.rb
@@ -0,0 +1,63 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+def fix_roles_projects
+  # This migration is not reversible.  However, the behavior it
+  # enforces is backwards-compatible, and most of the time there
+  # 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").each do |g|
+        if g.owner_uuid != system_user_uuid
+          # 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)
+          g.owner_uuid = system_user_uuid
+          g.save_with_unique_name!
+        end
+
+        # 3) If a role owns anything, give it to system user and it
+        # becomes a can_manage link
+        ActiveRecord::Base.descendants.reject(&:abstract_class?).each do |klass|
+          next if [ApiClientAuthorization,
+                   AuthorizedKey,
+                   Log].include?(klass)
+          next if !klass.columns.collect(&:name).include?('owner_uuid')
+
+          klass.where(owner_uuid: g.uuid).each do |owned|
+            Link.create!(link_class: 'permission',
+                         name: 'can_manage',
+                         tail_uuid: g.uuid,
+                         head_uuid: owned.uuid)
+            owned.owner_uuid = system_user_uuid
+            owned.save_with_unique_name!
+          end
+        end
+      end
+
+      # 4) Projects can't have outgoing permission links.  Just delete them.
+      q = ActiveRecord::Base.connection.exec_query %{
+select links.uuid from links, groups where groups.uuid = links.tail_uuid and
+       links.link_class = 'permission' and groups.group_class = 'project'
+}
+      q.each do |lu|
+        ln = Link.find_by_uuid(lu['uuid'])
+        puts "Projects cannot have outgoing permission links, '#{ln.name}' link from #{ln.tail_uuid} to #{ln.head_uuid} will be removed"
+        Rails.logger.warn "Destroying invalid permission link from project #{ln.tail_uuid} to #{ln.head_uuid}"
+        ln.destroy!
+      end
+    end
+  end
+end
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 4293b0466..b30d1edc2 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -198,20 +198,6 @@ foo_file_readable_by_active_redundant_permission_via_private_group:
   head_uuid: zzzzz-4zz18-znfnqtbbv4spc3w
   properties: {}
 
-foo_file_readable_by_aproject:
-  uuid: zzzzz-o0j2j-fp1d8395ldqw22p
-  owner_uuid: zzzzz-tpzed-000000000000000
-  created_at: 2014-01-24 20:42:26 -0800
-  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
-  modified_by_user_uuid: zzzzz-tpzed-000000000000000
-  modified_at: 2014-01-24 20:42:26 -0800
-  updated_at: 2014-01-24 20:42:26 -0800
-  tail_uuid: zzzzz-j7d0g-v955i6s2oi1cbso
-  link_class: permission
-  name: can_read
-  head_uuid: zzzzz-4zz18-znfnqtbbv4spc3w
-  properties: {}
-
 bar_file_readable_by_active:
   uuid: zzzzz-o0j2j-8hppiuduf8eqdng
   owner_uuid: zzzzz-tpzed-000000000000000
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index 631e0137c..3d1fda927 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: AGPL-3.0
 
 require 'test_helper'
+require 'fix_roles_projects'
 
 class GroupTest < ActiveSupport::TestCase
 
@@ -293,4 +294,53 @@ class GroupTest < ActiveSupport::TestCase
       end
     end
   end
+
+  def insert_group uuid, owner_uuid, name, group_class
+    q = ActiveRecord::Base.connection.exec_query %{
+insert into groups (uuid, owner_uuid, name, group_class, created_at, updated_at)
+       values ('#{uuid}', '#{owner_uuid}',
+               '#{name}', #{if group_class then "'"+group_class+"'" else 'NULL' end},
+               statement_timestamp(), statement_timestamp())
+}
+    uuid
+  end
+
+  test "migration to fix roles and projects" do
+    g1 = insert_group Group.generate_uuid, system_user_uuid, 'group with no class', nil
+    g2 = insert_group Group.generate_uuid, users(:active).uuid, 'role owned by a user', 'role'
+
+    g3 = insert_group Group.generate_uuid, system_user_uuid, 'role that owns a project', 'role'
+    g4 = insert_group Group.generate_uuid, g3, 'the project', 'project'
+
+    g5 = insert_group Group.generate_uuid, users(:active).uuid, 'a project with an outgoing permission link', 'project'
+
+    g6 = insert_group Group.generate_uuid, system_user_uuid, 'name collision', 'role'
+    g7 = insert_group Group.generate_uuid, users(:active).uuid, 'name collision', 'role'
+
+    refresh_permissions
+
+    act_as_system_user do
+      l1 = Link.create!(link_class: 'permission', name: 'can_manage', tail_uuid: g3, head_uuid: g4)
+      q = ActiveRecord::Base.connection.exec_query %{
+update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
+}
+    refresh_permissions
+    end
+
+    assert_equal nil, Group.find_by_uuid(g1).group_class
+    assert_equal users(:active).uuid, Group.find_by_uuid(g2).owner_uuid
+    assert_equal g3, Group.find_by_uuid(g4).owner_uuid
+    assert !Link.where(tail_uuid: users(:active).uuid, head_uuid: g2, link_class: "permission", name: "can_manage").any?
+    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?
+
+    fix_roles_projects
+
+    assert_equal 'role', Group.find_by_uuid(g1).group_class
+    assert_equal system_user_uuid, Group.find_by_uuid(g2).owner_uuid
+    assert_equal system_user_uuid, Group.find_by_uuid(g4).owner_uuid
+    assert Link.where(tail_uuid: users(:active).uuid, head_uuid: g2, link_class: "permission", name: "can_manage").any?
+    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
 end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list