15061: Implement admin variant of user merge by uuid

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 de18144c52fc49d2bef328a8c8a5d38836b103ee..4a345f363be8da15055f52d54dcfb929f6687298 100644 (file)
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -126,40 +126,65 @@ class Arvados::V1::UsersController < ApplicationController
   end
 
   def merge
-    if !Thread.current[:api_client].andand.is_trusted
-      return send_error("supplied API token is not from a trusted client", status: 403)
-    elsif Thread.current[:api_client_authorization].scopes != ['all']
-      return send_error("cannot merge with a scoped token", status: 403)
-    end
+    if (params[:old_user_uuid] || params[:new_user_uuid])
+      if !current_user.andand.is_admin
+        return send_error("Must be admin to use old_user_uuid/new_user_uuid", status: 403)
+      end
+      if !params[:old_user_uuid] || !params[:new_user_uuid]
+        return send_error("Must supply both old_user_uuid and new_user_uuid", status: 422)
+      end
+      new_user = User.find_by_uuid(params[:new_user_uuid])
+      if !new_user
+        return send_error("User in new_user_uuid not found", status: 422)
+      end
+      @object = User.find_by_uuid(params[:old_user_uuid])
+      if !@object
+        return send_error("User in old_user_uuid not found", status: 422)
+      end
+    else
+      if !Thread.current[:api_client].andand.is_trusted
+        return send_error("supplied API token is not from a trusted client", status: 403)
+      elsif Thread.current[:api_client_authorization].scopes != ['all']
+        return send_error("cannot merge with a scoped token", status: 403)
+      end
 
-    new_auth = ApiClientAuthorization.validate(token: params[:new_user_token])
-    if !new_auth
-      return send_error("invalid new_user_token", status: 401)
-    end
+      new_auth = ApiClientAuthorization.validate(token: params[:new_user_token])
+      if !new_auth
+        return send_error("invalid new_user_token", status: 401)
+      end
 
-    if new_auth.user.uuid[0..4] == Rails.configuration.ClusterID
-      if !new_auth.api_client.andand.is_trusted
-        return send_error("supplied new_user_token is not from a trusted client", status: 403)
-      elsif new_auth.scopes != ['all']
-        return send_error("supplied new_user_token has restricted scope", status: 403)
+      if new_auth.user.uuid[0..4] == Rails.configuration.ClusterID
+        if !new_auth.api_client.andand.is_trusted
+          return send_error("supplied new_user_token is not from a trusted client", status: 403)
+        elsif new_auth.scopes != ['all']
+          return send_error("supplied new_user_token has restricted scope", status: 403)
+        end
       end
+      new_user = new_auth.user
+      @object = current_user
     end
-    new_user = new_auth.user
 
-    if current_user.uuid == new_user.uuid
+    if @object.uuid == new_user.uuid
       return send_error("cannot merge user to self", status: 422)
     end
 
+    if !params[:new_owner_uuid]
+      return send_error("missing new_owner_uuid", status: 422)
+    end
+
     if !new_user.can?(write: params[:new_owner_uuid])
       return send_error("cannot move objects into supplied new_owner_uuid: new user does not have write permission", status: 403)
     end
 
     redirect = params[:redirect_to_new_user]
+    if @object.uuid[0..4] != Rails.configuration.ClusterID && redirect
+      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
 
-    @object = current_user
     act_as_system_user do
       @object.merge(new_owner_uuid: params[:new_owner_uuid], redirect_to_user_uuid: redirect && new_user.uuid)
     end
@@ -174,11 +199,17 @@ class Arvados::V1::UsersController < ApplicationController
         type: 'string', required: true,
       },
       new_user_token: {
-        type: 'string', required: true,
+        type: 'string', required: false,
       },
       redirect_to_new_user: {
         type: 'boolean', required: false,
       },
+      old_user_uuid: {
+        type: 'string', required: false,
+      },
+      new_user_uuid: {
+        type: 'string', required: false,
+      }
     }
   end
 

diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 1e4befb05a973b5de66eee6f5a96bcc4ffb83258..ef0f8868666dfb3bb786dab263270c8911df45e6 100644 (file)
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -85,10 +85,6 @@ class UserSessionsController < ApplicationController
       # Send them to their home cluster's login
       rh = Rails.configuration.RemoteClusters[user.uuid[0..4]]
       remote, return_to_url = params[:return_to].split(',', 2)
-      if remote !~ /^[0-9a-z]{5}$/ && remote != ""
-        return send_error 'Invalid remote cluster id', status: 400
-      end
-      remote = nil if remote == ''
       @remotehomeurl = "#{rh.Scheme || "https"}://#{rh.Host}/login?remote=#{Rails.configuration.ClusterID}&return_to=#{return_to_url}"
       render
       return

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 0501da1673ebdff87c5c9900b205dfdde96dce42..60696b98a9c998be7e270fe8bd3fea8cc72bd450 100644 (file)
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -927,7 +927,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
            redirect_to_new_user: true,
          })
     assert_response(:success)
-    assert_equal(users(:project_viewer).redirect_to_user_uuid, users(:active).uuid)
+    assert_equal(users(:active).uuid, User.unscoped.find_by_uuid(users(:project_viewer).uuid).redirect_to_user_uuid)
 
     auth = ApiClientAuthorization.validate(token: api_client_authorizations(:project_viewer).api_token)
     assert_not_nil(auth)
@@ -935,6 +935,82 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     assert_equal(users(:active).uuid, auth.user.uuid)
   end
 
+
+  test "merge 'project_viewer' account into 'active' account using uuids" do
+    authorize_with(:admin)
+    post(:merge, params: {
+           old_user_uuid: users(:project_viewer).uuid,
+           new_user_uuid: users(:active).uuid,
+           new_owner_uuid: users(:active).uuid,
+           redirect_to_new_user: true,
+         })
+    assert_response(:success)
+    assert_equal(users(:active).uuid, User.unscoped.find_by_uuid(users(:project_viewer).uuid).redirect_to_user_uuid)
+
+    auth = ApiClientAuthorization.validate(token: api_client_authorizations(:project_viewer).api_token)
+    assert_not_nil(auth)
+    assert_not_nil(auth.user)
+    assert_equal(users(:active).uuid, auth.user.uuid)
+  end
+
+  test "merge 'project_viewer' account into 'active' account using uuids denied for non-admin" do
+    authorize_with(:active)
+    post(:merge, params: {
+           old_user_uuid: users(:project_viewer).uuid,
+           new_user_uuid: users(:active).uuid,
+           new_owner_uuid: users(:active).uuid,
+           redirect_to_new_user: true,
+         })
+    assert_response(403)
+    assert_nil(users(:project_viewer).redirect_to_user_uuid)
+  end
+
+  test "merge 'project_viewer' account into 'active' account using uuids denied missing old_user_uuid" do
+    authorize_with(:admin)
+    post(:merge, params: {
+           new_user_uuid: users(:active).uuid,
+           new_owner_uuid: users(:active).uuid,
+           redirect_to_new_user: true,
+         })
+    assert_response(422)
+    assert_nil(users(:project_viewer).redirect_to_user_uuid)
+  end
+
+  test "merge 'project_viewer' account into 'active' account using uuids denied missing new_user_uuid" do
+    authorize_with(:admin)
+    post(:merge, params: {
+           old_user_uuid: users(:project_viewer).uuid,
+           new_owner_uuid: users(:active).uuid,
+           redirect_to_new_user: true,
+         })
+    assert_response(422)
+    assert_nil(users(:project_viewer).redirect_to_user_uuid)
+  end
+
+  test "merge 'project_viewer' account into 'active' account using uuids denied bogus old_user_uuid" do
+    authorize_with(:admin)
+    post(:merge, params: {
+           old_user_uuid: "zzzzz-tpzed-bogusbogusbogus",
+           new_user_uuid: users(:active).uuid,
+           new_owner_uuid: users(:active).uuid,
+           redirect_to_new_user: true,
+         })
+    assert_response(422)
+    assert_nil(users(:project_viewer).redirect_to_user_uuid)
+  end
+
+  test "merge 'project_viewer' account into 'active' account using uuids denied bogus new_user_uuid" do
+    authorize_with(:admin)
+    post(:merge, params: {
+           old_user_uuid: users(:project_viewer).uuid,
+           new_user_uuid: "zzzzz-tpzed-bogusbogusbogus",
+           new_owner_uuid: users(:active).uuid,
+           redirect_to_new_user: true,
+         })
+    assert_response(422)
+    assert_nil(users(:project_viewer).redirect_to_user_uuid)
+  end
+
   NON_ADMIN_USER_DATA = ["uuid", "kind", "is_active", "email", "first_name",
                          "last_name", "username"].sort