Skip to content

Commit

Permalink
FEATURE: hide_email_address_taken forces use of email in forgot passw…
Browse files Browse the repository at this point in the history
…ord form (discourse#15362)

* FEATURE: hide_email_address_taken forces use of email in forgot password form

This strengthens this site setting which is meant to be used to harden sites
that are experiencing abuse on forgot password routes.

Previously we would only deny letting people know if forgot password worked on not
New change also bans usage of username for forgot password when enabled
  • Loading branch information
SamSaffron authored Dec 20, 2021
1 parent 1cdb5b7 commit b6c3e9a
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,15 @@ export default Controller.extend(ModalFunctionality, {

@discourseComputed("accountEmailOrUsername", "disabled")
submitDisabled(accountEmailOrUsername, disabled) {
return isEmpty((accountEmailOrUsername || "").trim()) || disabled;
if (disabled) {
return true;
}

if (this.siteSettings.hide_email_address_taken) {
return (accountEmailOrUsername || "").indexOf("@") === -1;
} else {
return isEmpty((accountEmailOrUsername || "").trim());
}
},

onShow() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@
{{#if offerHelp}}
{{html-safe offerHelp}}
{{else}}
<label for="username-or-email">{{i18n "forgot_password.invite"}}</label>
{{text-field value=accountEmailOrUsername placeholderKey="login.email_placeholder" id="username-or-email" autocorrect="off" autocapitalize="off"}}
{{#if siteSettings.hide_email_address_taken}}
<label for="username-or-email">{{i18n "forgot_password.invite_no_username"}}</label>
{{text-field value=accountEmailOrUsername placeholderKey="email" id="username-or-email" autocorrect="off" autocapitalize="off"}}
{{else}}
<label for="username-or-email">{{i18n "forgot_password.invite"}}</label>
{{text-field value=accountEmailOrUsername placeholderKey="login.email_placeholder" id="username-or-email" autocorrect="off" autocapitalize="off"}}
{{/if}}
{{/if}}
{{/d-modal-body}}
<div class="modal-footer">
Expand Down
9 changes: 7 additions & 2 deletions app/controllers/session_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,11 @@ def forgot_password
RateLimiter.new(nil, "forgot-password-hr-#{request.remote_ip}", 6, 1.hour).performed!
RateLimiter.new(nil, "forgot-password-min-#{request.remote_ip}", 3, 1.minute).performed!

user = User.find_by_username_or_email(normalized_login_param)
if SiteSetting.hide_email_address_taken
user = User.find_by_email(Email.downcase(normalized_login_param))
else
user = User.find_by_username_or_email(normalized_login_param)
end

if user
RateLimiter.new(nil, "forgot-password-login-day-#{user.username}", 6, 1.day).performed!
Expand All @@ -449,7 +453,8 @@ def forgot_password
end

json = success_json
unless SiteSetting.hide_email_address_taken

if !SiteSetting.hide_email_address_taken
json[:user_found] = user_presence
end

Expand Down
1 change: 1 addition & 0 deletions config/locales/client.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1870,6 +1870,7 @@ en:
title: "Password Reset"
action: "I forgot my password"
invite: "Enter your username or email address, and we'll send you a password reset email."
invite_no_username: "Enter your email address, and we'll send you a password reset email."
reset: "Reset Password"
complete_username: "If an account matches the username <b>%{username}</b>, you should receive an email with instructions on how to reset your password shortly."
complete_email: "If an account matches <b>%{email}</b>, you should receive an email with instructions on how to reset your password shortly."
Expand Down
2 changes: 1 addition & 1 deletion config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,7 @@ en:
allowed_email_domains: "A pipe-delimited list of email domains that users MUST register accounts with. WARNING: Users with email domains other than those listed will not be allowed!"
normalize_emails: "Check if normalized email is unique. Normalized email removes all dots from the username and everything between + and @ symbols."
auto_approve_email_domains: "Users with email addresses from this list of domains will be automatically approved."
hide_email_address_taken: "Don't inform users that an account exists with a given email address during signup and from the forgot password form."
hide_email_address_taken: "Don't inform users that an account exists with a given email address during signup or during forgot password flow. Require full email for 'forgotten password' requests."
log_out_strict: "When logging out, log out ALL sessions for the user on all devices"
version_checks: "Ping the Discourse Hub for version updates and show new version messages on the <a href='%{base_path}/admin' target='_blank'>/admin</a> dashboard"
new_version_emails: "Send an email to the contact_email address when a new version of Discourse is available."
Expand Down
4 changes: 3 additions & 1 deletion config/site_settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,9 @@ login:
default: ""
type: list
list_type: simple
hide_email_address_taken: false
hide_email_address_taken:
client: true
default: false
log_out_strict: false
pending_users_reminder_delay_minutes:
min: -1
Expand Down
23 changes: 23 additions & 0 deletions spec/requests/session_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2058,6 +2058,29 @@ def post_login
end

describe '#forgot_password' do

context 'when hide_email_address_taken is set' do
before do
SiteSetting.hide_email_address_taken = true
end

it 'denies for username' do
post "/session/forgot_password.json",
params: { login: user.username }

expect(response.status).to eq(200)
expect(Jobs::CriticalUserEmail.jobs.size).to eq(0)
end

it 'allows for email' do
post "/session/forgot_password.json",
params: { login: user.email }

expect(response.status).to eq(200)
expect(Jobs::CriticalUserEmail.jobs.size).to eq(1)
end
end

it 'raises an error without a username parameter' do
post "/session/forgot_password.json"
expect(response.status).to eq(400)
Expand Down

0 comments on commit b6c3e9a

Please sign in to comment.