15531: Implement merge with redirect_to_new_user=false

When false, delete credentials of old user instead of migrating them.

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 4a345f363be8da15055f52d54dcfb929f6687298..0ef129481492c9aad9ba06114b2ad12304e2cffd 100644 (file)
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -181,12 +181,10 @@ class Arvados::V1::UsersController < ApplicationController
       return send_error("cannot merge remote user to other with redirect_to_new_user=true", status: 422)
     end
 
-    if !redirect
-      return send_error("merge with redirect_to_new_user=false is not yet supported", status: 422)
-    end
-
     act_as_system_user do
-      @object.merge(new_owner_uuid: params[:new_owner_uuid], redirect_to_user_uuid: redirect && new_user.uuid)
+      @object.merge(new_owner_uuid: params[:new_owner_uuid],
+                    redirect_to_user_uuid: new_user.uuid,
+                    redirect_auth: redirect)
     end
     show
   end

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 4493f038cd1c03e5e265d973ed774e7223eb43e4..65cb75306f384a3eb4c3c863889b034e45988814 100644 (file)
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -280,7 +280,7 @@ class User < ArvadosModel
   #
   # current_user must have admin privileges, i.e., the caller is
   # responsible for checking permission to do this.
-  def merge(new_owner_uuid:, redirect_to_user_uuid:)
+  def merge(new_owner_uuid:, redirect_to_user_uuid:, redirect_auth:)
     raise PermissionDeniedError if !current_user.andand.is_admin
     raise "not implemented" if !redirect_to_user_uuid
     transaction(requires_new: true) do
@@ -291,23 +291,39 @@ class User < ArvadosModel
       raise "user does not exist" if !new_user
       raise "cannot merge to an already merged user" if new_user.redirect_to_user_uuid
 
-      # Existing API tokens are updated to authenticate to the new
-      # user.
-      ApiClientAuthorization.
-        where(user_id: id).
-        update_all(user_id: new_user.id)
+      if redirect_auth
+        # Existing API tokens and ssh keys are updated to authenticate
+        # to the new user.
+        ApiClientAuthorization.
+          where(user_id: id).
+          update_all(user_id: new_user.id)
+
+        user_updates = [
+          [AuthorizedKey, :owner_uuid],
+          [AuthorizedKey, :authorized_user_uuid],
+          [Repository, :owner_uuid],
+          [Link, :owner_uuid],
+          [Link, :tail_uuid],
+          [Link, :head_uuid],
+        ]
+      else
+        # Destroy API tokens and ssh keys associated with the old
+        # user.
+        ApiClientAuthorization.where(user_id: id).destroy_all
+        AuthorizedKey.where(owner_uuid: uuid).destroy_all
+        AuthorizedKey.where(authorized_user_uuid: uuid).destroy_all
+        user_updates = [
+          [Repository, :owner_uuid],
+          [Link, :owner_uuid],
+          [Link, :tail_uuid],
+          [Link, :head_uuid],
+        ]
+      end
 
       # References to the old user UUID in the context of a user ID
       # (rather than a "home project" in the project hierarchy) are
       # updated to point to the new user.
-      [
-        [AuthorizedKey, :owner_uuid],
-        [AuthorizedKey, :authorized_user_uuid],
-        [Repository, :owner_uuid],
-        [Link, :owner_uuid],
-        [Link, :tail_uuid],
-        [Link, :head_uuid],
-      ].each do |klass, column|
+      user_updates.each do |klass, column|
         klass.where(column => uuid).update_all(column => new_user.uuid)
       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 60696b98a9c998be7e270fe8bd3fea8cc72bd450..4e19988de1bee16528ccc410a4225509e4a40368 100644 (file)
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -817,14 +817,21 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     end
   end
 
-  test "refuse to merge with redirect_to_user_uuid=false (not yet supported)" do
+  test "merge with redirect_to_user_uuid=false" do
     authorize_with :project_viewer_trustedclient
+    tok = api_client_authorizations(:project_viewer).api_token
     post :merge, params: {
            new_user_token: api_client_authorizations(:active_trustedclient).api_token,
            new_owner_uuid: users(:active).uuid,
            redirect_to_new_user: false,
          }
-    assert_response(422)
+    assert_response(:success)
+    assert_equal(users(:active).uuid, User.unscoped.find_by_uuid(users(:project_viewer).uuid).redirect_to_user_uuid)
+
+    # because redirect_to_new_user=false, token owned by
+    # project_viewer should be deleted
+    auth = ApiClientAuthorization.validate(token: tok)
+    assert_nil(auth)
   end
 
   test "refuse to merge user into self" do