DEV: Change hide_email_address_taken default to true (#30293)

We're changing the default of hide_email_address_taken to true. This is a trade-off we want to make, as it prevents account enumeration with minimal impact on legitimate users. If you forget you have an account and try to sign up again with the same e-mail you'll receive an e-mail letting you know.
This commit is contained in:
Ted Johansson 2024-12-17 10:46:04 +08:00 committed by GitHub
parent 0410c07342
commit c1c7ea8959
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 55 additions and 12 deletions

View File

@ -7,6 +7,10 @@ let userFound = false;
acceptance("Forgot password", function (needs) { acceptance("Forgot password", function (needs) {
needs.pretender((server, helper) => { needs.pretender((server, helper) => {
needs.settings({
hide_email_address_taken: false,
});
server.post("/session/forgot_password", () => { server.post("/session/forgot_password", () => {
return helper.response({ return helper.response({
user_found: userFound, user_found: userFound,
@ -78,6 +82,10 @@ acceptance("Forgot password", function (needs) {
acceptance( acceptance(
"Forgot password - hide_email_address_taken enabled", "Forgot password - hide_email_address_taken enabled",
function (needs) { function (needs) {
needs.settings({
hide_email_address_taken: true,
});
needs.pretender((server, helper) => { needs.pretender((server, helper) => {
server.post("/session/forgot_password", () => { server.post("/session/forgot_password", () => {
return helper.response({}); return helper.response({});
@ -93,12 +101,12 @@ acceptance(
.dom(".forgot-password-reset") .dom(".forgot-password-reset")
.isDisabled("disables the button until the field is filled"); .isDisabled("disables the button until the field is filled");
await fillIn("#username-or-email", "someuser"); await fillIn("#username-or-email", "someuser@discourse.org");
await click(".forgot-password-reset"); await click(".forgot-password-reset");
assert.dom(".d-modal__body").hasHtml( assert.dom(".d-modal__body").hasHtml(
i18n("forgot_password.complete_username", { i18n("forgot_password.complete_email", {
username: "someuser", email: "someuser@discourse.org",
}), }),
"displays a success message" "displays a success message"
); );

View File

@ -621,7 +621,7 @@ login:
list_type: simple list_type: simple
hide_email_address_taken: hide_email_address_taken:
client: true client: true
default: false default: true
log_out_strict: false log_out_strict: false
pending_users_reminder_delay_minutes: pending_users_reminder_delay_minutes:
min: -1 min: -1

View File

@ -119,6 +119,8 @@ describe "Core extensions" do
describe "user custom fields" do describe "user custom fields" do
it "supports discourse_automation_ids" do it "supports discourse_automation_ids" do
SiteSetting.hide_email_address_taken = false
user = create_user user = create_user
automation_1.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) automation_1.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD)

View File

@ -5,6 +5,7 @@ RSpec.describe "invite only" do
describe "#create invite only" do describe "#create invite only" do
it "can create user via API" do it "can create user via API" do
SiteSetting.invite_only = true SiteSetting.invite_only = true
SiteSetting.hide_email_address_taken = false
Jobs.run_immediately! Jobs.run_immediately!
admin = Fabricate(:admin) admin = Fabricate(:admin)

View File

@ -5,6 +5,8 @@ RSpec.describe EmailUpdater do
let(:new_email) { "new.email@example.com" } let(:new_email) { "new.email@example.com" }
it "provides better error message when a staged user has the same email" do it "provides better error message when a staged user has the same email" do
SiteSetting.hide_email_address_taken = false
Fabricate(:user, staged: true, email: new_email) Fabricate(:user, staged: true, email: new_email)
user = Fabricate(:user, email: old_email) user = Fabricate(:user, email: old_email)

View File

@ -117,6 +117,8 @@ RSpec.describe Invite do
end end
it "escapes the email_address when raising an existing user error" do it "escapes the email_address when raising an existing user error" do
SiteSetting.hide_email_address_taken = false
user.email = xss_email user.email = xss_email
user.save(validate: false) user.save(validate: false)

View File

@ -656,6 +656,8 @@ RSpec.describe "users" do
end end
path "/session/forgot_password.json" do path "/session/forgot_password.json" do
SiteSetting.hide_email_address_taken = false
post "Send password reset email" do post "Send password reset email" do
tags "Users" tags "Users"
operationId "sendPasswordResetEmail" operationId "sendPasswordResetEmail"

View File

@ -1013,6 +1013,8 @@ RSpec.describe UsersController do
end end
context "when creating as active" do context "when creating as active" do
before { SiteSetting.hide_email_address_taken = false }
it "won't create the user as active" do it "won't create the user as active" do
post "/u.json", params: post_user_params.merge(active: true) post "/u.json", params: post_user_params.merge(active: true)
expect(response.status).to eq(200) expect(response.status).to eq(200)
@ -2042,6 +2044,8 @@ RSpec.describe UsersController do
end end
describe "#check_email" do describe "#check_email" do
before { SiteSetting.hide_email_address_taken = false }
it "returns success if hide_email_address_taken is true" do it "returns success if hide_email_address_taken is true" do
SiteSetting.hide_email_address_taken = true SiteSetting.hide_email_address_taken = true

View File

@ -207,12 +207,26 @@ RSpec.describe UsersEmailController do
context "when new email is different case of existing email" do context "when new email is different case of existing email" do
fab!(:other_user) { Fabricate(:user, email: "case.insensitive@gmail.com") } fab!(:other_user) { Fabricate(:user, email: "case.insensitive@gmail.com") }
it "raises an error" do context "when hiding taken e-mails" do
put "/u/#{user.username}/preferences/email.json", it "raises an error" do
params: { put "/u/#{user.username}/preferences/email.json",
email: other_user.email.upcase, params: {
} email: other_user.email.upcase,
expect(response).to_not be_successful }
expect(response).to be_successful
end
end
context "when revealing taken e-mails" do
before { SiteSetting.hide_email_address_taken = false }
it "raises an error" do
put "/u/#{user.username}/preferences/email.json",
params: {
email: other_user.email.upcase,
}
expect(response).to_not be_successful
end
end end
end end

View File

@ -51,6 +51,8 @@ describe "Changing email", type: :system do
end end
it "works when user has totp 2fa" do it "works when user has totp 2fa" do
SiteSetting.hide_email_address_taken = false
second_factor = Fabricate(:user_second_factor_totp, user: user) second_factor = Fabricate(:user_second_factor_totp, user: user)
sign_in user sign_in user

View File

@ -10,7 +10,10 @@ shared_examples "login scenarios" do |login_page_object|
fab!(:admin) { Fabricate(:admin, username: "admin", password: "supersecurepassword") } fab!(:admin) { Fabricate(:admin, username: "admin", password: "supersecurepassword") }
let(:user_menu) { PageObjects::Components::UserMenu.new } let(:user_menu) { PageObjects::Components::UserMenu.new }
before { Jobs.run_immediately! } before do
SiteSetting.hide_email_address_taken = false
Jobs.run_immediately!
end
def wait_for_email_link(user, type) def wait_for_email_link(user, type)
wait_for(timeout: 5) { ActionMailer::Base.deliveries.count != 0 } wait_for(timeout: 5) { ActionMailer::Base.deliveries.count != 0 }

View File

@ -221,7 +221,10 @@ shared_examples "signup scenarios" do |signup_page_object, login_page_object|
end end
context "when the email domain is blocked" do context "when the email domain is blocked" do
before { SiteSetting.blocked_email_domains = "example.com" } before do
SiteSetting.hide_email_address_taken = false
SiteSetting.blocked_email_domains = "example.com"
end
it "cannot signup" do it "cannot signup" do
signup_form signup_form