From c1c7ea89592f3ceee2edeec544b3dcc170df87c2 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Tue, 17 Dec 2024 10:46:04 +0800 Subject: [PATCH] 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. --- .../tests/acceptance/forgot-password-test.js | 14 +++++++--- config/site_settings.yml | 2 +- .../spec/integration/core_ext_spec.rb | 2 ++ .../invite_only_registration_spec.rb | 1 + spec/lib/email_updater_spec.rb | 2 ++ spec/models/invite_spec.rb | 2 ++ spec/requests/api/users_spec.rb | 2 ++ spec/requests/users_controller_spec.rb | 4 +++ spec/requests/users_email_controller_spec.rb | 26 ++++++++++++++----- spec/system/email_change_spec.rb | 2 ++ spec/system/login_spec.rb | 5 +++- spec/system/signup_spec.rb | 5 +++- 12 files changed, 55 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/discourse/tests/acceptance/forgot-password-test.js b/app/assets/javascripts/discourse/tests/acceptance/forgot-password-test.js index 9569cd0a73b..60f24263555 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/forgot-password-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/forgot-password-test.js @@ -7,6 +7,10 @@ let userFound = false; acceptance("Forgot password", function (needs) { needs.pretender((server, helper) => { + needs.settings({ + hide_email_address_taken: false, + }); + server.post("/session/forgot_password", () => { return helper.response({ user_found: userFound, @@ -78,6 +82,10 @@ acceptance("Forgot password", function (needs) { acceptance( "Forgot password - hide_email_address_taken enabled", function (needs) { + needs.settings({ + hide_email_address_taken: true, + }); + needs.pretender((server, helper) => { server.post("/session/forgot_password", () => { return helper.response({}); @@ -93,12 +101,12 @@ acceptance( .dom(".forgot-password-reset") .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"); assert.dom(".d-modal__body").hasHtml( - i18n("forgot_password.complete_username", { - username: "someuser", + i18n("forgot_password.complete_email", { + email: "someuser@discourse.org", }), "displays a success message" ); diff --git a/config/site_settings.yml b/config/site_settings.yml index 99e95b99c9e..6705efbbabd 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -621,7 +621,7 @@ login: list_type: simple hide_email_address_taken: client: true - default: false + default: true log_out_strict: false pending_users_reminder_delay_minutes: min: -1 diff --git a/plugins/automation/spec/integration/core_ext_spec.rb b/plugins/automation/spec/integration/core_ext_spec.rb index 6a5c9ab7b4d..5ef7daa1acd 100644 --- a/plugins/automation/spec/integration/core_ext_spec.rb +++ b/plugins/automation/spec/integration/core_ext_spec.rb @@ -119,6 +119,8 @@ describe "Core extensions" do describe "user custom fields" do it "supports discourse_automation_ids" do + SiteSetting.hide_email_address_taken = false + user = create_user automation_1.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) diff --git a/spec/integration/invite_only_registration_spec.rb b/spec/integration/invite_only_registration_spec.rb index 1ea4a9c1eca..baed1f52296 100644 --- a/spec/integration/invite_only_registration_spec.rb +++ b/spec/integration/invite_only_registration_spec.rb @@ -5,6 +5,7 @@ RSpec.describe "invite only" do describe "#create invite only" do it "can create user via API" do SiteSetting.invite_only = true + SiteSetting.hide_email_address_taken = false Jobs.run_immediately! admin = Fabricate(:admin) diff --git a/spec/lib/email_updater_spec.rb b/spec/lib/email_updater_spec.rb index f96c57e85de..f664de684d7 100644 --- a/spec/lib/email_updater_spec.rb +++ b/spec/lib/email_updater_spec.rb @@ -5,6 +5,8 @@ RSpec.describe EmailUpdater do let(:new_email) { "new.email@example.com" } 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) user = Fabricate(:user, email: old_email) diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index d17de76dde1..f4fc8d949ac 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -117,6 +117,8 @@ RSpec.describe Invite do end it "escapes the email_address when raising an existing user error" do + SiteSetting.hide_email_address_taken = false + user.email = xss_email user.save(validate: false) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index a1fdac809ef..28a720c2e4a 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -656,6 +656,8 @@ RSpec.describe "users" do end path "/session/forgot_password.json" do + SiteSetting.hide_email_address_taken = false + post "Send password reset email" do tags "Users" operationId "sendPasswordResetEmail" diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 8f2aac569b4..e87d4a4bef6 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1013,6 +1013,8 @@ RSpec.describe UsersController do end context "when creating as active" do + before { SiteSetting.hide_email_address_taken = false } + it "won't create the user as active" do post "/u.json", params: post_user_params.merge(active: true) expect(response.status).to eq(200) @@ -2042,6 +2044,8 @@ RSpec.describe UsersController do end describe "#check_email" do + before { SiteSetting.hide_email_address_taken = false } + it "returns success if hide_email_address_taken is true" do SiteSetting.hide_email_address_taken = true diff --git a/spec/requests/users_email_controller_spec.rb b/spec/requests/users_email_controller_spec.rb index 076d23f5343..0dc87cf5a21 100644 --- a/spec/requests/users_email_controller_spec.rb +++ b/spec/requests/users_email_controller_spec.rb @@ -207,12 +207,26 @@ RSpec.describe UsersEmailController do context "when new email is different case of existing email" do fab!(:other_user) { Fabricate(:user, email: "case.insensitive@gmail.com") } - it "raises an error" do - put "/u/#{user.username}/preferences/email.json", - params: { - email: other_user.email.upcase, - } - expect(response).to_not be_successful + context "when hiding taken e-mails" do + it "raises an error" do + put "/u/#{user.username}/preferences/email.json", + params: { + email: other_user.email.upcase, + } + 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 diff --git a/spec/system/email_change_spec.rb b/spec/system/email_change_spec.rb index 547ea91abf2..ea60f5b616a 100644 --- a/spec/system/email_change_spec.rb +++ b/spec/system/email_change_spec.rb @@ -51,6 +51,8 @@ describe "Changing email", type: :system do end it "works when user has totp 2fa" do + SiteSetting.hide_email_address_taken = false + second_factor = Fabricate(:user_second_factor_totp, user: user) sign_in user diff --git a/spec/system/login_spec.rb b/spec/system/login_spec.rb index 56c81c642b1..a3fded60a35 100644 --- a/spec/system/login_spec.rb +++ b/spec/system/login_spec.rb @@ -10,7 +10,10 @@ shared_examples "login scenarios" do |login_page_object| fab!(:admin) { Fabricate(:admin, username: "admin", password: "supersecurepassword") } 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) wait_for(timeout: 5) { ActionMailer::Base.deliveries.count != 0 } diff --git a/spec/system/signup_spec.rb b/spec/system/signup_spec.rb index 829348f4132..a3ea97b30b8 100644 --- a/spec/system/signup_spec.rb +++ b/spec/system/signup_spec.rb @@ -221,7 +221,10 @@ shared_examples "signup scenarios" do |signup_page_object, login_page_object| end 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 signup_form