SECURITY: Delete email tokens when a user's email is changed or deleted (#19735)

Co-authored-by: OsamaSayegh <asooomaasoooma90@gmail.com>
This commit is contained in:
Alan Guo Xiang Tan 2023-01-05 06:08:55 +08:00 committed by GitHub
parent bf6b08670a
commit 83944213b2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 10 deletions

View File

@ -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

View File

@ -3656,17 +3656,18 @@ RSpec.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
@ -3922,8 +3923,7 @@ RSpec.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
@ -4013,8 +4013,7 @@ RSpec.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

View File

@ -242,6 +242,28 @@ RSpec.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