[ARVADOS] updated: 2.1.0-1282-g0c90b88be

Git user git at public.arvados.org
Fri Sep 3 18:39:33 UTC 2021


Summary of changes:
 .../app/controllers/arvados/v1/users_controller.rb | 17 +-----
 services/api/app/models/user.rb                    | 31 ----------
 services/api/config/routes.rb                      |  1 -
 .../functional/arvados/v1/users_controller_test.rb | 21 -------
 services/api/test/unit/owner_test.rb               | 25 +-------
 services/api/test/unit/user_test.rb                | 66 ----------------------
 6 files changed, 3 insertions(+), 158 deletions(-)

       via  0c90b88beddd9d5d6dcf777af43afe1817d37d61 (commit)
      from  88c0535c0cd110f75f1c5256401c1831e4cf3052 (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 0c90b88beddd9d5d6dcf777af43afe1817d37d61
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Fri Sep 3 15:39:05 2021 -0300

    18094: Removes update_uuid code & tests from railsAPI.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index f4d42edf6..52f4b1e5e 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -10,7 +10,7 @@ class Arvados::V1::UsersController < ApplicationController
     [:activate, :current, :system, :setup, :merge, :batch_update]
   skip_before_action :render_404_if_no_object, only:
     [:activate, :current, :system, :setup, :merge, :batch_update]
-  before_action :admin_required, only: [:setup, :unsetup, :update_uuid, :batch_update]
+  before_action :admin_required, only: [:setup, :unsetup, :batch_update]
 
   # Internal API used by controller to update local cache of user
   # records from LoginCluster.
@@ -145,13 +145,6 @@ class Arvados::V1::UsersController < ApplicationController
     show
   end
 
-  # Change UUID to a new (unused) uuid and transfer all owned/linked
-  # objects accordingly.
-  def update_uuid
-    @object.update_uuid(new_uuid: params[:new_uuid])
-    show
-  end
-
   def merge
     if (params[:old_user_uuid] || params[:new_user_uuid])
       if !current_user.andand.is_admin
@@ -261,14 +254,6 @@ class Arvados::V1::UsersController < ApplicationController
     })
   end
 
-  def self._update_uuid_requires_parameters
-    {
-      new_uuid: {
-        type: 'string', required: true,
-      },
-    }
-  end
-
   def apply_filters(model_class=nil)
     return super if @read_users.any?(&:is_admin)
     if params[:uuid] != current_user.andand.uuid
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index da7e7b310..2e862d3ae 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -362,37 +362,6 @@ SELECT target_uuid, perm_level
     end
   end
 
-  def update_uuid(new_uuid:)
-    if !current_user.andand.is_admin
-      raise PermissionDeniedError
-    end
-    if uuid == system_user_uuid || uuid == anonymous_user_uuid
-      raise "update_uuid cannot update system accounts"
-    end
-    if self.class != self.class.resource_class_for_uuid(new_uuid)
-      raise "invalid new_uuid #{new_uuid.inspect}"
-    end
-    transaction(requires_new: true) do
-      reload
-      old_uuid = self.uuid
-      self.uuid = new_uuid
-      save!(validate: false)
-      change_all_uuid_refs(old_uuid: old_uuid, new_uuid: new_uuid)
-    ActiveRecord::Base.connection.exec_update %{
-update #{PERMISSION_VIEW} set user_uuid=$1 where user_uuid = $2
-},
-                                             'User.update_uuid.update_permissions_user_uuid',
-                                             [[nil, new_uuid],
-                                              [nil, old_uuid]]
-      ActiveRecord::Base.connection.exec_update %{
-update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
-},
-                                            'User.update_uuid.update_permissions_target_uuid',
-                                             [[nil, new_uuid],
-                                              [nil, old_uuid]]
-    end
-  end
-
   # Move this user's (i.e., self's) owned items to new_owner_uuid and
   # new_user_uuid (for things normally owned directly by the user).
   #
diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb
index 697585803..738426b1d 100644
--- a/services/api/config/routes.rb
+++ b/services/api/config/routes.rb
@@ -81,7 +81,6 @@ Rails.application.routes.draw do
         post 'activate', on: :member
         post 'setup', on: :collection
         post 'unsetup', on: :member
-        post 'update_uuid', on: :member
         post 'merge', on: :collection
         patch 'batch_update', on: :collection
       end
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index e0f7b8970..c807a7d6c 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -800,27 +800,6 @@ The Arvados team.
                     "user's writable_by should include its owner_uuid")
   end
 
-  [
-    [:admin, true],
-    [:active, false],
-  ].each do |auth_user, expect_success|
-    test "update_uuid as #{auth_user}" do
-      authorize_with auth_user
-      orig_uuid = users(:active).uuid
-      post :update_uuid, params: {
-             id: orig_uuid,
-             new_uuid: 'zbbbb-tpzed-abcde12345abcde',
-           }
-      if expect_success
-        assert_response :success
-        assert_empty User.where(uuid: orig_uuid)
-      else
-        assert_response 403
-        assert_not_empty User.where(uuid: orig_uuid)
-      end
-    end
-  end
-
   test "merge with redirect_to_user_uuid=false" do
     authorize_with :project_viewer_trustedclient
     tok = api_client_authorizations(:project_viewer).api_token
diff --git a/services/api/test/unit/owner_test.rb b/services/api/test/unit/owner_test.rb
index e356f4d9f..aa0ac5f36 100644
--- a/services/api/test/unit/owner_test.rb
+++ b/services/api/test/unit/owner_test.rb
@@ -92,12 +92,8 @@ class OwnerTest < ActiveSupport::TestCase
              "new #{o_class} should really be in DB")
       old_uuid = o.uuid
       new_uuid = o.uuid.sub(/..........$/, rand(2**256).to_s(36)[0..9])
-      if o.respond_to? :update_uuid
-        o.update_uuid(new_uuid: new_uuid)
-      else
-        assert(o.update_attributes(uuid: new_uuid),
-               "should change #{o_class} uuid from #{old_uuid} to #{new_uuid}")
-      end
+      assert(o.update_attributes(uuid: new_uuid),
+              "should change #{o_class} uuid from #{old_uuid} to #{new_uuid}")
       assert_equal(false, o_class.where(uuid: old_uuid).any?,
                    "#{old_uuid} should disappear when renamed to #{new_uuid}")
     end
@@ -142,21 +138,4 @@ class OwnerTest < ActiveSupport::TestCase
     check_permissions_against_full_refresh
   end
 
-  test "change uuid of User that owns self" do
-    o = User.create!
-    assert User.where(uuid: o.uuid).any?, "new User should really be in DB"
-    assert_equal(true, o.update_attributes(owner_uuid: o.uuid),
-                 "setting owner to self should work")
-    old_uuid = o.uuid
-    new_uuid = o.uuid.sub(/..........$/, rand(2**256).to_s(36)[0..9])
-    o.update_uuid(new_uuid: new_uuid)
-    o = User.find_by_uuid(new_uuid)
-    assert_equal(false, User.where(uuid: old_uuid).any?,
-                 "#{old_uuid} should not be in DB after deleting")
-    assert_equal(true, User.where(uuid: new_uuid).any?,
-                 "#{new_uuid} should be in DB after renaming")
-    assert_equal(new_uuid, User.where(uuid: new_uuid).first.owner_uuid,
-                 "#{new_uuid} should be its own owner in DB after renaming")
-  end
-
 end
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index f973c6ba1..c00164c0a 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -686,72 +686,6 @@ class UserTest < ActiveSupport::TestCase
     end
   end
 
-  [
-    [:active, 'zzzzz-borkd-abcde12345abcde'],
-    [:active, 'zzzzz-j7d0g-abcde12345abcde'],
-    [:active, 'zzzzz-tpzed-borkd'],
-    [:system_user, 'zzzzz-tpzed-abcde12345abcde'],
-    [:anonymous, 'zzzzz-tpzed-abcde12345abcde'],
-  ].each do |fixture, new_uuid|
-    test "disallow update_uuid #{fixture} -> #{new_uuid}" do
-      u = users(fixture)
-      orig_uuid = u.uuid
-      act_as_system_user do
-        assert_raises do
-          u.update_uuid(new_uuid: new_uuid)
-        end
-      end
-      # "Successfully aborted orig->new" outcome looks the same as
-      # "successfully updated new->orig".
-      assert_update_success(old_uuid: new_uuid,
-                            new_uuid: orig_uuid,
-                            expect_owned_objects: fixture == :active)
-    end
-  end
-
-  [:active, :spectator, :admin].each do |target|
-    test "update_uuid on #{target} as non-admin user" do
-      act_as_user users(:active) do
-        assert_raises(ArvadosModel::PermissionDeniedError) do
-          users(target).update_uuid(new_uuid: 'zzzzz-tpzed-abcde12345abcde')
-        end
-      end
-    end
-  end
-
-  test "update_uuid to existing uuid" do
-    u = users(:active)
-    orig_uuid = u.uuid
-    new_uuid = users(:admin).uuid
-    act_as_system_user do
-      assert_raises do
-        u.update_uuid(new_uuid: new_uuid)
-      end
-    end
-    u.reload
-    assert_equal u.uuid, orig_uuid
-    assert_not_empty Collection.where(owner_uuid: orig_uuid)
-    assert_not_empty Group.where(owner_uuid: orig_uuid)
-  end
-
-  [
-    [:active, 'zbbbb-tpzed-abcde12345abcde'],
-    [:active, 'zzzzz-tpzed-abcde12345abcde'],
-    [:admin, 'zbbbb-tpzed-abcde12345abcde'],
-    [:admin, 'zzzzz-tpzed-abcde12345abcde'],
-  ].each do |fixture, new_uuid|
-    test "update_uuid #{fixture} to unused uuid #{new_uuid}" do
-      u = users(fixture)
-      orig_uuid = u.uuid
-      act_as_system_user do
-        u.update_uuid(new_uuid: new_uuid)
-      end
-      assert_update_success(old_uuid: orig_uuid,
-                            new_uuid: new_uuid,
-                            expect_owned_objects: fixture == :active)
-    end
-  end
-
   def assert_update_success(old_uuid:, new_uuid:, expect_owned_objects: true)
     [[User, :uuid],
      [Link, :head_uuid],

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list