[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