From 32951ca2f4c0a994e5708648fc97cd7bd49cb6ba Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 28 Jul 2021 14:07:18 +0800 Subject: [PATCH] FIX: User can change name when auth_overrides_name is enabled. --- app/services/user_updater.rb | 5 ++++- spec/requests/users_controller_spec.rb | 18 ++++++++++++++++-- spec/services/user_updater_spec.rb | 23 ++++++++++++----------- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index adb2cc6dba1..84446b98794 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -97,7 +97,10 @@ class UserUpdater end old_user_name = user.name.present? ? user.name : "" - user.name = attributes.fetch(:name) { user.name } + + if guardian.can_edit_name?(user) + user.name = attributes.fetch(:name) { user.name } + end user.locale = attributes.fetch(:locale) { user.locale } user.date_of_birth = attributes.fetch(:date_of_birth) { user.date_of_birth } diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 5fa9eb0a714..78fc05a68cd 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1083,8 +1083,11 @@ describe UsersController do expect(response.status).to eq(200) json = response.parsed_body expect(json['success']).to eq(true) - expect(User.last.username).to eq('testosama') - expect(User.last.name).to eq('Osama Test') + + user = User.last + + expect(user.username).to eq('testosama') + expect(user.name).to eq('Osama Test') end end @@ -1821,6 +1824,17 @@ describe UsersController do end end + it "does not allow name to be updated if auth auth_overrides_name is enabled" do + SiteSetting.auth_overrides_name = true + + sign_in(user) + + put "/u/#{user.username}", params: { name: 'test.test' } + + expect(response.status).to eq(200) + expect(user.reload.name).to_not eq('test.test') + end + context "when username contains a period" do before do sign_in(user) diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index 4f9545d803d..9d52d1c1317 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -43,7 +43,7 @@ describe UserUpdater do it 'saves user' do user = Fabricate(:user, name: 'Billy Bob') - updater = UserUpdater.new(acting_user, user) + updater = UserUpdater.new(user, user) updater.update(name: 'Jim Tom') @@ -547,11 +547,10 @@ describe UserUpdater do end it "logs the action" do - user_without_name = Fabricate(:user, name: nil) user = Fabricate(:user, name: 'Billy Bob') expect do - UserUpdater.new(acting_user, user).update(name: 'Jim Tom') + UserUpdater.new(user, user).update(name: 'Jim Tom') end.to change { UserHistory.count }.by(1) expect(UserHistory.last.action).to eq( @@ -559,19 +558,21 @@ describe UserUpdater do ) expect do - UserUpdater.new(acting_user, user).update(name: 'JiM TOm') + UserUpdater.new(user, user).update(name: 'JiM TOm') end.to_not change { UserHistory.count } expect do - UserUpdater.new(acting_user, user).update(bio_raw: 'foo bar') + UserUpdater.new(user, user).update(bio_raw: 'foo bar') + end.to_not change { UserHistory.count } + + user_without_name = Fabricate(:user, name: nil) + + expect do + UserUpdater.new(user_without_name, user_without_name).update(bio_raw: 'foo bar') end.to_not change { UserHistory.count } expect do - UserUpdater.new(acting_user, user_without_name).update(bio_raw: 'foo bar') - end.to_not change { UserHistory.count } - - expect do - UserUpdater.new(acting_user, user_without_name).update(name: 'Jim Tom') + UserUpdater.new(user_without_name, user_without_name).update(name: 'Jim Tom') end.to change { UserHistory.count }.by(1) expect(UserHistory.last.action).to eq( @@ -579,7 +580,7 @@ describe UserUpdater do ) expect do - UserUpdater.new(acting_user, user).update(name: '') + UserUpdater.new(user, user).update(name: '') end.to change { UserHistory.count }.by(1) expect(UserHistory.last.action).to eq(