From e47f5cedd23d5979e3e8dadb51cf308bc747a536 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 3 Oct 2017 14:08:37 -0400 Subject: [PATCH] FEATURE: forgot_password_strict setting also prevents reporting that an email address is taken during signup --- app/controllers/users_controller.rb | 13 +++++++++++++ app/mailers/user_notifications.rb | 9 +++++++++ app/services/user_activator.rb | 17 +++++++++++++++++ config/locales/server.en.yml | 13 +++++++++++++ spec/controllers/users_controller_spec.rb | 22 ++++++++++++++++++++++ 5 files changed, 74 insertions(+) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9a064933d46..69ab62df520 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -372,6 +372,19 @@ class UsersController < ApplicationController message: activation.message, user_id: user.id } + elsif SiteSetting.forgot_password_strict && user.errors[:primary_email]&.include?(I18n.t('errors.messages.taken')) + session["user_created_message"] = activation.success_message + + if existing_user = User.find_by_email(user.primary_email&.email) + Jobs.enqueue(:critical_user_email, type: :account_exists, user_id: existing_user.id) + end + + render json: { + success: true, + active: user.active?, + message: activation.success_message, + user_id: user.id + } else errors = user.errors.to_hash errors[:email] = errors.delete(:primary_email) if errors[:primary_email] diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 83ba60ad085..e1d24b37c6a 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -83,6 +83,15 @@ class UserNotifications < ActionMailer::Base ) end + def account_exists(user, opts = {}) + build_email( + user.email, + template: 'user_notifications.account_exists', + locale: user_locale(user), + email: user.email + ) + end + def short_date(dt) if dt.year == Time.now.year I18n.l(dt, format: :short_no_year) diff --git a/app/services/user_activator.rb b/app/services/user_activator.rb index d0c380db95b..d65fba83c29 100644 --- a/app/services/user_activator.rb +++ b/app/services/user_activator.rb @@ -16,6 +16,10 @@ class UserActivator @message = activator.activate end + def success_message + activator.success_message + end + private def activator @@ -38,6 +42,10 @@ end class ApprovalActivator < UserActivator def activate + success_message + end + + def success_message I18n.t("login.wait_approval") end end @@ -52,6 +60,11 @@ class EmailActivator < UserActivator user_id: user.id, email_token: email_token.token ) + + success_message + end + + def success_message I18n.t("login.activate_email", email: Rack::Utils.escape_html(user.email)) end end @@ -62,6 +75,10 @@ class LoginActivator < UserActivator def activate log_on_user(user) user.enqueue_welcome_message('welcome_user') + success_message + end + + def success_message I18n.t("login.active") end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 374123e632a..d28b259caa7 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2656,6 +2656,19 @@ en: %{message} + account_exists: + title: "Account already exists" + subject_template: "[%{email_prefix}] Account already exists" + text_body_template: | + You just tried to create an account at %{site_name}. However, an account already exists for %{email}. + + If you forgot your password, [reset it now](%{base_url}/password-reset). + + If you didn’t try to create an account for %{email}, don’t worry – you can safely ignore this message. + + If you have any questions, [contact our friendly staff](%{base_url}/about). + + digest: why: "A brief summary of %{site_link} since your last visit on %{last_seen_at}" diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index a16779d4c29..fcd265fef3f 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -621,6 +621,28 @@ describe UsersController do expect(session[SessionController::ACTIVATE_USER_KEY]).to be_present end end + + context 'users already exists with given email' do + let!(:existing) { Fabricate(:user, email: post_user_params[:email]) } + + it 'returns an error if forgot_password_strict is disabled' do + SiteSetting.forgot_password_strict = false + post_user + json = JSON.parse(response.body) + expect(json['success']).to eq(false) + expect(json['message']).to be_present + end + + it 'returns success if forgot_password_strict is enabled' do + SiteSetting.forgot_password_strict = true + expect { + post_user + }.to_not change { User.count } + json = JSON.parse(response.body) + expect(json['active']).to be_falsey + expect(session["user_created_message"]).to be_present + end + end end context "creating as active" do