summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGrzegorz Bizon <grzegorz@gitlab.com>2017-08-27 04:54:22 +0000
committerGrzegorz Bizon <grzegorz@gitlab.com>2017-08-27 04:54:22 +0000
commitef8eb3f6f7c1af167017ecc854e9fe385cee9354 (patch)
tree9a6a19be034620c5a388d2285b228949ce20f3b8
parent2be34630623711fc20ef8c101b5cef688f207cc1 (diff)
parente94a2fde0dbb1802ad3e01486c2c8f6d8a3ee4da (diff)
downloadgitlab-ce-ef8eb3f6f7c1af167017ecc854e9fe385cee9354.tar.gz
Merge branch 'sh-system-hooks-ldap-users' into 'master'
Fire system hooks when a user is created via LDAP or OAuth Closes #37073 See merge request !13846
-rw-r--r--app/services/concerns/users/new_user_notifier.rb9
-rw-r--r--app/services/users/create_service.rb8
-rw-r--r--app/services/users/update_service.rb6
-rw-r--r--changelogs/unreleased/sh-system-hooks-ldap-users.yml5
-rw-r--r--spec/services/users/update_service_spec.rb17
5 files changed, 39 insertions, 6 deletions
diff --git a/app/services/concerns/users/new_user_notifier.rb b/app/services/concerns/users/new_user_notifier.rb
new file mode 100644
index 00000000000..231693ce7a9
--- /dev/null
+++ b/app/services/concerns/users/new_user_notifier.rb
@@ -0,0 +1,9 @@
+module Users
+ module NewUserNotifier
+ def notify_new_user(user, reset_token)
+ log_info("User \"#{user.name}\" (#{user.email}) was created")
+ notification_service.new_user(user, reset_token) if reset_token
+ system_hook_service.execute_hooks_for(user, :create)
+ end
+ end
+end
diff --git a/app/services/users/create_service.rb b/app/services/users/create_service.rb
index 74abc017cea..c8a3c461d60 100644
--- a/app/services/users/create_service.rb
+++ b/app/services/users/create_service.rb
@@ -1,5 +1,7 @@
module Users
class CreateService < BaseService
+ include NewUserNotifier
+
def initialize(current_user, params = {})
@current_user = current_user
@params = params.dup
@@ -10,11 +12,7 @@ module Users
@reset_token = user.generate_reset_token if user.recently_sent_password_reset?
- if user.save
- log_info("User \"#{user.name}\" (#{user.email}) was created")
- notification_service.new_user(user, @reset_token) if @reset_token
- system_hook_service.execute_hooks_for(user, :create)
- end
+ notify_new_user(user, @reset_token) if user.save
user
end
diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb
index dfbd6016c3f..2f9855273dc 100644
--- a/app/services/users/update_service.rb
+++ b/app/services/users/update_service.rb
@@ -1,5 +1,7 @@
module Users
class UpdateService < BaseService
+ include NewUserNotifier
+
def initialize(user, params = {})
@user = user
@params = params.dup
@@ -10,7 +12,11 @@ module Users
assign_attributes(&block)
+ user_exists = @user.persisted?
+
if @user.save(validate: validate)
+ notify_new_user(@user, nil) unless user_exists
+
success
else
error(@user.errors.full_messages.uniq.join('. '))
diff --git a/changelogs/unreleased/sh-system-hooks-ldap-users.yml b/changelogs/unreleased/sh-system-hooks-ldap-users.yml
new file mode 100644
index 00000000000..87514ec00ea
--- /dev/null
+++ b/changelogs/unreleased/sh-system-hooks-ldap-users.yml
@@ -0,0 +1,5 @@
+---
+title: Fire system hooks when a user is created via LDAP
+merge_request:
+author:
+type: fixed
diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb
index 985f6d94876..6ee35a33b2d 100644
--- a/spec/services/users/update_service_spec.rb
+++ b/spec/services/users/update_service_spec.rb
@@ -37,7 +37,10 @@ describe Users::UpdateService do
describe '#execute!' do
it 'updates the name' do
- result = update_user(user, name: 'New Name')
+ service = described_class.new(user, name: 'New Name')
+ expect(service).not_to receive(:notify_new_user)
+
+ result = service.execute!
expect(result).to be true
expect(user.name).to eq('New Name')
@@ -49,6 +52,18 @@ describe Users::UpdateService do
end.to raise_error(ActiveRecord::RecordInvalid)
end
+ it 'fires system hooks when a new user is saved' do
+ system_hook_service = spy(:system_hook_service)
+ user = build(:user)
+ service = described_class.new(user, name: 'John Doe')
+ expect(service).to receive(:notify_new_user).and_call_original
+ expect(service).to receive(:system_hook_service).and_return(system_hook_service)
+
+ service.execute
+
+ expect(system_hook_service).to have_received(:execute_hooks_for).with(user, :create)
+ end
+
def update_user(user, opts)
described_class.new(user, opts).execute!
end