Implement new service for creating user

5 files changed, 209 insertions, 13 deletions

diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 8cc216445eb..902911071c4 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -30,6 +30,15 @@ describe RegistrationsController do
           expect(subject.current_user).to be_nil
         end
       end
+
+      context 'when signup_enabled? is false' do
+        it 'redirects to sign_in' do
+          allow_any_instance_of(ApplicationSetting).to receive(:signup_enabled?).and_return(false)
+
+          expect { post(:create, user_params) }.not_to change(User, :count)
+          expect(response).to redirect_to(new_user_session_path)
+        end
+      end
     end
 
     context 'when reCAPTCHA is enabled' do
diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb
index e8caad00c44..8acec805584 100644
--- a/spec/models/hooks/system_hook_spec.rb
+++ b/spec/models/hooks/system_hook_spec.rb
@@ -6,6 +6,9 @@ describe SystemHook, models: true do
     let(:user)        { create(:user) }
     let(:project)     { create(:empty_project, namespace: user.namespace) }
     let(:group)       { create(:group) }
+    let(:params) do
+      { name: 'John Doe', username: 'jduser', email: 'jg@example.com', password: 'mydummypass' }
+    end
 
     before do
       WebMock.stub_request(:post, system_hook.url)
@@ -29,7 +32,7 @@ describe SystemHook, models: true do
     end
 
     it "user_create hook" do
-      create(:user)
+      Users::CreateService.new(nil, params).execute
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_create/,
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 570abd44dc6..a9e37be1157 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -361,22 +361,10 @@ describe User, models: true do
   end
 
   describe '#generate_password' do
-    it "executes callback when force_random_password specified" do
-      user = build(:user, force_random_password: true)
-      expect(user).to receive(:generate_password)
-      user.save
-    end
-
     it "does not generate password by default" do
       user = create(:user, password: 'abcdefghe')
       expect(user.password).to eq('abcdefghe')
     end
-
-    it "generates password when forcing random password" do
-      allow(Devise).to receive(:friendly_token).and_return('123456789')
-      user = create(:user, password: 'abcdefg', force_random_password: true)
-      expect(user.password).to eq('12345678')
-    end
   end
 
   describe 'authentication token' do
diff --git a/spec/requests/api/v3/users_spec.rb b/spec/requests/api/v3/users_spec.rb
index 17bbb0b53c1..b38cbe74b85 100644
--- a/spec/requests/api/v3/users_spec.rb
+++ b/spec/requests/api/v3/users_spec.rb
@@ -263,4 +263,18 @@ describe API::V3::Users, api: true  do
       expect(json_response['message']).to eq('404 User Not Found')
     end
   end
+
+  describe 'POST /users' do
+    it 'creates confirmed user when confirm parameter is false' do
+      optional_attributes = { confirm: false }
+      attributes = attributes_for(:user).merge(optional_attributes)
+
+      post v3_api('/users', admin), attributes
+
+      user_id = json_response['id']
+      new_user = User.find(user_id)
+
+      expect(new_user).to be_confirmed
+    end
+  end
 end
diff --git a/spec/services/users/create_service_spec.rb b/spec/services/users/create_service_spec.rb
new file mode 100644
index 00000000000..5f79203701a
--- /dev/null
+++ b/spec/services/users/create_service_spec.rb
@@ -0,0 +1,182 @@
+require 'spec_helper'
+
+describe Users::CreateService, services: true do
+  describe '#build' do
+    let(:params) do
+      { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass' }
+    end
+
+    context 'with an admin user' do
+      let(:admin_user) { create(:admin) }
+      let(:service) { described_class.new(admin_user, params) }
+
+      it 'returns a valid user' do
+        expect(service.build).to be_valid
+      end
+    end
+
+    context 'with non admin user' do
+      let(:user) { create(:user) }
+      let(:service) { described_class.new(user, params) }
+
+      it 'raises AccessDeniedError exception' do
+        expect { service.build }.to raise_error Gitlab::Access::AccessDeniedError
+      end
+    end
+
+    context 'with nil user' do
+      let(:service) { described_class.new(nil, params) }
+
+      it 'returns a valid user' do
+        expect(service.build).to be_valid
+      end
+    end
+  end
+
+  describe '#execute' do
+    let(:admin_user) { create(:admin) }
+
+    context 'with an admin user' do
+      let(:service) { described_class.new(admin_user, params) }
+
+      context 'when required parameters are provided' do
+        let(:params) do
+          { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass' }
+        end
+
+        it 'returns a persisted user' do
+          expect(service.execute).to be_persisted
+        end
+
+        it 'persists the given attributes' do
+          user = service.execute
+          user.reload
+
+          expect(user).to have_attributes(
+            name: params[:name],
+            username: params[:username],
+            email: params[:email],
+            password: params[:password],
+            created_by_id: admin_user.id
+          )
+        end
+
+        it 'user is not confirmed if skip_confirmation param is not present' do
+          expect(service.execute).not_to be_confirmed
+        end
+
+        it 'logs the user creation' do
+          expect(service).to receive(:log_info).with("User \"John Doe\" (jd@example.com) was created")
+
+          service.execute
+        end
+
+        it 'executes system hooks ' do
+          system_hook_service = spy(:system_hook_service)
+
+          expect(service).to receive(:system_hook_service).and_return(system_hook_service)
+
+          user = service.execute
+
+          expect(system_hook_service).to have_received(:execute_hooks_for).with(user, :create)
+        end
+
+        it 'does not send a notification email' do
+          notification_service = spy(:notification_service)
+
+          expect(service).not_to receive(:notification_service)
+
+          service.execute
+
+          expect(notification_service).not_to have_received(:new_user)
+        end
+      end
+
+      context 'when force_random_password parameter is true' do
+        let(:params) do
+          { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', force_random_password: true }
+        end
+
+        it 'generates random password' do
+          user = service.execute
+
+          expect(user.password).not_to eq 'mydummypass'
+          expect(user.password).to be_present
+        end
+      end
+
+      context 'when skip_confirmation parameter is true' do
+        let(:params) do
+          { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', skip_confirmation: true }
+        end
+
+        it 'confirms the user' do
+          expect(service.execute).to be_confirmed
+        end
+      end
+
+      context 'when reset_password parameter is true' do
+        let(:params) do
+          { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', reset_password: true }
+        end
+
+        it 'resets password even if a password parameter is given' do
+          expect(service.execute).to be_recently_sent_password_reset
+        end
+
+        it 'sends a notification email' do
+          notification_service = spy(:notification_service)
+
+          expect(service).to receive(:notification_service).and_return(notification_service)
+
+          user = service.execute
+
+          expect(notification_service).to have_received(:new_user).with(user, an_instance_of(String))
+        end
+      end
+    end
+
+    context 'with nil user' do
+      let(:params) do
+        { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', skip_confirmation: true }
+      end
+      let(:service) { described_class.new(nil, params) }
+
+      context 'when "send_user_confirmation_email" application setting is true' do
+        before do
+          current_application_settings = double(:current_application_settings, send_user_confirmation_email: true, signup_enabled?: true)
+          allow(service).to receive(:current_application_settings).and_return(current_application_settings)
+        end
+
+        it 'does not confirm the user' do
+          expect(service.execute).not_to be_confirmed
+        end
+      end
+
+      context 'when "send_user_confirmation_email" application setting is false' do
+        before do
+          current_application_settings = double(:current_application_settings, send_user_confirmation_email: false, signup_enabled?: true)
+          allow(service).to receive(:current_application_settings).and_return(current_application_settings)
+        end
+
+        it 'confirms the user' do
+          expect(service.execute).to be_confirmed
+        end
+
+        it 'persists the given attributes' do
+          user = service.execute
+          user.reload
+
+          expect(user).to have_attributes(
+            name: params[:name],
+            username: params[:username],
+            email: params[:email],
+            password: params[:password],
+            created_by_id: nil,
+            admin: false
+          )
+        end
+      end
+    end
+  end
+end