First check whether user exists in UAA before trying to create in /v3/users

Author: svkrieger

app/actions/user_create.rb

@@ -4,9 +4,15 @@ class Error < StandardError
     end
 
     def create(message:)
-      shadow_user = User.create_uaa_shadow_user(message.username, message.origin) if message.username && message.origin
-      user_guid = shadow_user ? shadow_user['id'] : message.guid
+      if message.username && message.origin
+        existing_user_guid = User.get_user_id_by_username_and_origin(message.username, message.origin)
 
+        shadow_user = User.create_uaa_shadow_user(message.username, message.origin) unless existing_user_guid
+
+        user_guid = existing_user_guid || shadow_user['id']
+      else
+        user_guid = message.guid
+      end
       user = User.create(guid: user_guid)
       User.db.transaction do
         MetadataUpdate.update(user, message)

app/models/runtime/user.rb

@@ -252,6 +252,11 @@ def self.uaa_users_info(user_guids)
       uaa_username_lookup_client.users_for_ids(user_guids)
     end
 
+    def self.get_user_id_by_username_and_origin(username, origin)
+      uaa_username_lookup_client = CloudController::DependencyLocator.instance.uaa_username_lookup_client
+      uaa_username_lookup_client.ids_for_usernames_and_origins([username], [origin]).first
+    end
+
     def self.create_uaa_shadow_user(username, origin)
       uaa_shadow_user_creation_client = CloudController::DependencyLocator.instance.uaa_shadow_user_creation_client
       uaa_shadow_user_creation_client.create_shadow_user(username, origin)

spec/request/users_spec.rb

@@ -705,8 +705,8 @@
         end
 
         before do
-          allow(CloudController::DependencyLocator.instance).to receive(:uaa_shadow_user_creation_client).and_return(uaa_client)
-          allow(uaa_client).to receive(:create_shadow_user).and_return({ 'id' => user_guid })
+          allow(CloudController::DependencyLocator.instance).to receive_messages(uaa_shadow_user_creation_client: uaa_client, uaa_username_lookup_client: uaa_client)
+          allow(uaa_client).to receive_messages(create_shadow_user: { 'id' => user_guid }, ids_for_usernames_and_origins: [])
         end
 
         it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
@@ -867,7 +867,7 @@
     context 'when "allow_user_creation_by_org_manager" is enabled' do
       before do
         TestConfig.override(allow_user_creation_by_org_manager: true)
-        allow(CloudController::DependencyLocator.instance).to receive(:uaa_shadow_user_creation_client).and_return(uaa_client)
+        allow(CloudController::DependencyLocator.instance).to receive_messages(uaa_shadow_user_creation_client: uaa_client, uaa_username_lookup_client: uaa_client)
       end
 
       describe 'when creating a user by guid' do
@@ -954,7 +954,7 @@
         end
 
         before do
-          allow(uaa_client).to receive(:create_shadow_user).and_return({ 'id' => user_guid })
+          allow(uaa_client).to receive_messages(create_shadow_user: { 'id' => user_guid }, ids_for_usernames_and_origins: [])
         end
 
         it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
@@ -967,6 +967,21 @@
 
           expect(uaa_client).not_to have_received(:users_for_ids)
         end
+
+        context 'when user already exists in UAA' do
+          before do
+            allow(uaa_client).to receive(:ids_for_usernames_and_origins).and_return([user_guid])
+          end
+
+          it 'does not try to create a shadow user' do
+            post '/v3/users', params.to_json, admin_header
+
+            expect(last_response).to have_status_code(201)
+            expect(parsed_response).to match_json_response(user_json)
+
+            expect(uaa_client).not_to have_received(:create_shadow_user)
+          end
+        end
       end
 
       context 'when parameters are invalid' do

spec/unit/actions/user_create_spec.rb

@@ -56,7 +56,7 @@ module VCAP::CloudController
 
       describe 'creating users' do
         before do
-          allow(User).to receive(:create_uaa_shadow_user).and_return({ 'id' => guid })
+          allow(User).to receive_messages(create_uaa_shadow_user: { 'id' => guid }, get_user_id_by_username_and_origin: nil)
         end
 
         context 'when creating a UAA user by guid' do
@@ -108,6 +108,17 @@ module VCAP::CloudController
             expect(User).to have_received(:create_uaa_shadow_user).with(username, origin)
           end
 
+          context 'when user already exists in UAA' do
+            before do
+              allow(User).to receive(:get_user_id_by_username_and_origin).and_return('some-user-guid')
+            end
+
+            it 'does not try to create a shadow user' do
+              subject.create(message:)
+              expect(User).not_to have_received(:create_uaa_shadow_user)
+            end
+          end
+
           context 'when an UaaUnavailable error is raised' do
             before do
               allow(User).to receive(:create_uaa_shadow_user).and_raise(UaaUnavailable)

spec/unit/models/runtime/user_spec.rb

@@ -578,5 +578,45 @@ module VCAP::CloudController
         end
       end
     end
+
+    describe '#get_user_id_by_username_and_origin' do
+      let(:uaa_client) { instance_double(UaaClient) }
+
+      before do
+        allow(CloudController::DependencyLocator.instance).to receive(:uaa_username_lookup_client).and_return(uaa_client)
+      end
+
+      context 'when uaa client returns user id' do
+        before do
+          allow(uaa_client).to receive(:ids_for_usernames_and_origins).and_return(['some-user-id'])
+        end
+
+        it 'returns user id' do
+          user_id = User.get_user_id_by_username_and_origin(['shadow-user'], ['idp.local'])
+          expect(user_id).to eq('some-user-id')
+        end
+      end
+
+      context 'when uaa client returns empty array' do
+        before do
+          allow(uaa_client).to receive(:ids_for_usernames_and_origins).and_return([])
+        end
+
+        it 'returns nil' do
+          user_id = User.get_user_id_by_username_and_origin(['shadow-user'], ['idp.local'])
+          expect(user_id).to be_nil
+        end
+      end
+
+      context 'when uaa client raises UaaUnavailable' do
+        before do
+          allow(uaa_client).to receive(:ids_for_usernames_and_origins).and_raise(UaaUnavailable)
+        end
+
+        it 'raises UaaUnavailable' do
+          expect { User.get_user_id_by_username_and_origin(['shadow-user'], ['idp.local']) }.to raise_error(UaaUnavailable)
+        end
+      end
+    end
   end
 end