summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames Lopez <james@jameslopez.es>2017-06-13 11:32:21 +0200
committerJames Lopez <james@jameslopez.es>2017-06-23 11:41:41 +0200
commit5f0e7873ae71a1f4d23a1c564bf7eb8830ebd888 (patch)
tree7748001ffd88cb5656ebad5897a34434e5847af7
parent801cf92310e9f6950dddba848ef1e6a3d1e48ef0 (diff)
downloadgitlab-ce-5f0e7873ae71a1f4d23a1c564bf7eb8830ebd888.tar.gz
ported EE user service to CE
-rw-r--r--app/services/users/update_service.rb24
-rw-r--r--spec/features/profiles/password_spec.rb2
-rw-r--r--spec/models/user_spec.rb13
-rw-r--r--spec/requests/api/users_spec.rb10
-rw-r--r--spec/services/users/update_service_spec.rb33
5 files changed, 80 insertions, 2 deletions
diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb
new file mode 100644
index 00000000000..ac20473b6eb
--- /dev/null
+++ b/app/services/users/update_service.rb
@@ -0,0 +1,24 @@
+module Users
+ # Service for creating a new user.
+ class UpdateService < BaseService
+ def initialize(current_user, user, params = {})
+ @current_user = current_user
+ @user = user
+ @params = params.dup
+ end
+
+ def execute(skip_authorization: false)
+ raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_update_user?
+
+ if @user.update_attributes(params)
+ success
+ else
+ error('Project could not be updated')
+ end
+ end
+
+ def can_update_user?
+ current_user == @user || current_user&.admin?
+ end
+ end
+end
diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb
index 2d36f3d020f..86c9df5ff86 100644
--- a/spec/features/profiles/password_spec.rb
+++ b/spec/features/profiles/password_spec.rb
@@ -25,7 +25,7 @@ describe 'Profile > Password', feature: true do
end
end
- it 'does not contains the current password field after an error' do
+ it 'does not contain the current password field after an error' do
fill_passwords('mypassword', 'mypassword2')
expect(page).to have_no_field('user[current_password]')
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 314f8781867..25ce545a1d7 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1899,4 +1899,17 @@ describe User, models: true do
user.invalidate_merge_request_cache_counts
end
end
+
+
+ describe 'audit changes' do
+ let!(:user) { create(:user) }
+
+ it 'audits an email change' do
+ expect { user.update!(email: 'test@example.com') }.to change { AuditEvent.count }.by(1)
+ end
+
+ it 'audits a password change' do
+ expect { user.update!(password: 'asdfasdf', password_confirmation: 'asdfasdf') }.to change { AuditEvent.count }.by(1)
+ end
+ end
end
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 18000d91795..a34c277112b 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -374,6 +374,7 @@ describe API::Users do
expect(response).to have_http_status(200)
expect(user.reload.password_expires_at).to be <= Time.now
+ expect(AuditEvent.count).to eq(1)
end
it "updates user with organization" do
@@ -401,6 +402,13 @@ describe API::Users do
expect(user.reload.email).to eq(user.email)
end
+ it 'updates user with a new email' do
+ put api("/users/#{user.id}", admin), email: 'new@email.com'
+ expect(response).to have_http_status(200)
+ expect(user.reload.notification_email).to eq('new@email.com')
+ expect(AuditEvent.count).to eq(1)
+ end
+
it 'updates user with his own username' do
put api("/users/#{user.id}", admin), username: user.username
expect(response).to have_http_status(200)
@@ -643,7 +651,7 @@ describe API::Users do
email_attrs = attributes_for :email
expect do
post api("/users/#{user.id}/emails", admin), email_attrs
- end.to change { user.emails.count }.by(1)
+ end.to change { user.emails.count }.by(1).and change { AuditEvent.count }.by(1)
end
it "returns a 400 for invalid ID" do
diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb
new file mode 100644
index 00000000000..73af9af7507
--- /dev/null
+++ b/spec/services/users/update_service_spec.rb
@@ -0,0 +1,33 @@
+require 'spec_helper'
+
+describe Users::UpdateService, services: true do
+ let(:user) { create(:user) }
+ let(:admin) { create(:admin) }
+ let(:user) { create(:empty_user, creator_id: user.id, namespace: user.namespace) }
+
+ describe '#execute' do
+ it 'updates the name' do
+ result = update_user(user, user, name: 'New Name')
+ expect(result).to eq({ status: :success })
+ expect(user.name).to eq('New Name')
+ end
+
+ context 'when updated by an admin' do
+ it 'updates the name' do
+ result = update_user(user, admin, name: 'New Name')
+ expect(result).to eq({ status: :success })
+ expect(user.name).to eq('New Name')
+ end
+ end
+
+ it 'returns an error result when record cannot be updated' do
+ result = update_user(user, create(:user), { name: 'New Name' })
+
+ expect(result).to eq({ status: :error, message: 'User could not be updated' })
+ end
+
+ def update_user(current_user, user, opts)
+ described_class.new(user, user, opts).execute
+ end
+ end
+end