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