diff --git a/app/models/user_email.rb b/app/models/user_email.rb index 4bb2e64e9ab..50ee27318b2 100644 --- a/app/models/user_email.rb +++ b/app/models/user_email.rb @@ -18,6 +18,10 @@ class UserEmail < ActiveRecord::Base scope :secondary, -> { where(primary: false) } + before_save ->() { destroy_email_tokens(self.email_was) }, if: :will_save_change_to_email? + + after_destroy { destroy_email_tokens(self.email) } + def normalize_email self.normalized_email = if self.email.present? username, domain = self.email.split('@', 2) @@ -62,6 +66,10 @@ class UserEmail < ActiveRecord::Base ) end end + + def destroy_email_tokens(email) + EmailToken.where(email: email).destroy_all + end end # == Schema Information diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 8b29279e0b1..2d3ad1853c7 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -3063,17 +3063,18 @@ describe UsersController do expect(user1.email_change_requests).to contain_exactly(request_1) end - it "can destroy associated email tokens" do + it "destroys associated email tokens and email change requests" do new_email = 'new.n.cool@example.com' updater = EmailUpdater.new(guardian: user1.guardian, user: user1) + updater.change_to(new_email) - expect { updater.change_to(new_email) } - .to change { user1.email_tokens.count }.by(1) + email_token = updater.change_req.new_email_token + expect(email_token).to be_present - expect { delete "/u/#{user1.username}/preferences/email.json", params: { email: new_email } } - .to change { user1.email_tokens.count }.by(-1) + delete "/u/#{user1.username}/preferences/email.json", params: { email: new_email } - expect(user1.email_tokens.first.email).to eq(user1.email) + expect(EmailToken.find_by(id: email_token.id)).to eq(nil) + expect(EmailChangeRequest.find_by(id: updater.change_req.id)).to eq(nil) end end @@ -3439,8 +3440,7 @@ describe UsersController do expect(user.email).to eq('updatedemail@example.com') expect(user.email_tokens.where(email: 'updatedemail@example.com', expired: false)).to be_present - token.reload - expect(token.expired?).to eq(true) + expect(EmailToken.find_by(id: token.id)).to eq(nil) end it 'tells the user to slow down after many requests' do @@ -3530,8 +3530,7 @@ describe UsersController do expect(user.email).to eq('updatedemail@example.com') expect(user.email_tokens.where(email: 'updatedemail@example.com', expired: false)).to be_present - token.reload - expect(token.expired?).to eq(true) + expect(EmailToken.find_by(id: token.id)).to eq(nil) end it 'tells the user to slow down after many requests' do diff --git a/spec/requests/users_email_controller_spec.rb b/spec/requests/users_email_controller_spec.rb index 0b65943f7c5..82d2400494e 100644 --- a/spec/requests/users_email_controller_spec.rb +++ b/spec/requests/users_email_controller_spec.rb @@ -244,6 +244,28 @@ describe UsersEmailController do end end end + + it "destroys email tokens associated with the old email after the new email is confirmed" do + SiteSetting.enable_secondary_emails = true + + email_token = user.email_tokens.create!(email: user.email, scope: EmailToken.scopes[:password_reset]) + + updater = EmailUpdater.new(guardian: user.guardian, user: user) + updater.change_to('bubblegum@adventuretime.ooo') + + sign_in(user) + put "/u/confirm-new-email", params: { + token: "#{updater.change_req.new_email_token.token}" + } + + new_password = SecureRandom.hex + put "/u/password-reset/#{email_token.token}.json", params: { + password: new_password + } + expect(response.parsed_body["success"]).to eq(false) + expect(response.parsed_body["message"]).to eq(I18n.t("password_reset.no_token", base_url: Discourse.base_url)) + expect(user.reload.confirm_password?(new_password)).to eq(false) + end end describe '#confirm-old-email' do