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

Author: svkrieger

app/controllers/v3/roles_controller.rb

@@ -114,12 +114,11 @@ def create_org_role(message)
     unauthorized! unless permission_queryer.can_write_to_active_org?(org.id)
     suspended! unless permission_queryer.is_org_active?(org.id)
 
-    if message.username && message.user_origin && message.user_origin != 'uaa' && org_managers_can_create_users?
-      user = create_uaa_shadow_user(message.username, message.user_origin)
-      user_guid = user['id']
-    else
-      user_guid = message.user_guid || lookup_user_guid_in_uaa(message.username, message.user_origin)
-    end
+    user_guid = if message.username && message.user_origin && message.user_origin != 'uaa' && org_managers_can_create_users?
+                  create_or_get_uaa_user(message)
+                else
+                  message.user_guid || lookup_user_guid_in_uaa(message.username, message.user_origin)
+                end
 
     user = User.first(guid: user_guid) || create_cc_user(user_guid)
 
@@ -145,9 +144,20 @@ def create_cc_user(user_guid)
     UserCreate.new.create(message:)
   end
 
-  def create_uaa_shadow_user(username, origin)
-    message = UserCreateMessage.new(username:, origin:)
-    unprocessable!(message.errors.full_messages) unless message.valid?
+  def create_or_get_uaa_user(message)
+    user_create_message = UserCreateMessage.new(username: message.username, origin: message.user_origin)
+    unprocessable!(user_create_message.errors.full_messages) unless user_create_message.valid?
+
+    existing_user_id = get_uaa_user_id(user_create_message)
+    user = create_uaa_shadow_user(user_create_message) unless existing_user_id
+    existing_user_id || user['id']
+  end
+
+  def get_uaa_user_id(message)
+    User.get_user_id_by_username_and_origin(message.username, message.origin)
+  end
+
+  def create_uaa_shadow_user(message)
     User.create_uaa_shadow_user(message.username, message.origin)
   end
 

spec/request/roles_spec.rb

@@ -814,6 +814,7 @@
           before do
             TestConfig.override(allow_user_creation_by_org_manager: true)
             allow(uaa_client).to receive(:create_shadow_user).with('bob_unaffiliated', origin).and_return({ 'id' => user_unaffiliated.guid })
+            allow(uaa_client).to receive(:ids_for_usernames_and_origins).and_return([])
           end
 
           it 'does not call create_shadow_user' do
@@ -830,10 +831,29 @@
 
             it 'calls create_shadow_user and retrieves the guid of the user from uaa' do
               post '/v3/roles', params.to_json, admin_header
+
               expect(last_response).to have_status_code(201)
               expect(parsed_response).to match_json_response(expected_response)
+
+              expect(uaa_client).to have_received(:ids_for_usernames_and_origins)
               expect(uaa_client).to have_received(:create_shadow_user)
             end
+
+            context 'user already exists in UAA' do
+              before do
+                allow(uaa_client).to receive(:ids_for_usernames_and_origins).and_return([user_unaffiliated.guid])
+              end
+
+              it 'retrieves the id from UAA and does not create a shadow user' do
+                post '/v3/roles', params.to_json, admin_header
+
+                expect(last_response).to have_status_code(201)
+                expect(parsed_response).to match_json_response(expected_response)
+
+                expect(uaa_client).to have_received(:ids_for_usernames_and_origins)
+                expect(uaa_client).not_to have_received(:create_shadow_user)
+              end
+            end
           end
         end
       end
@@ -971,6 +991,10 @@
         end
 
         context 'by user name and origin' do
+          before do
+            allow(uaa_client).to receive(:ids_for_usernames_and_origins).and_return([])
+          end
+
           let(:params) do
             {
               type: 'organization_auditor',