diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0bd0378134b..08564f52fb6 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -7,7 +7,7 @@ class UsersController < ApplicationController skip_before_filter :authorize_mini_profiler, only: [:avatar] skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :account_created, :activate_account, :perform_account_activation, :authorize_email, :user_preferences_redirect, :avatar, :my_redirect, :toggle_anon, :admin_login] - before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect, :upload_user_image, :pick_avatar, :destroy_user_image, :destroy, :check_emails] + before_filter :ensure_logged_in, only: [:username, :update, :user_preferences_redirect, :upload_user_image, :pick_avatar, :destroy_user_image, :destroy, :check_emails] before_filter :respond_to_suspicious_request, only: [:create] # we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the @@ -471,35 +471,6 @@ class UsersController < ApplicationController end end - def change_email - params.require(:email) - user = fetch_user_from_params - guardian.ensure_can_edit_email!(user) - lower_email = Email.downcase(params[:email]).strip - - RateLimiter.new(user, "change-email-hr-#{request.remote_ip}", 6, 1.hour).performed! - RateLimiter.new(user, "change-email-min-#{request.remote_ip}", 3, 1.minute).performed! - - EmailValidator.new(attributes: :email).validate_each(user, :email, lower_email) - return render_json_error(user.errors.full_messages) if user.errors[:email].present? - - # Raise an error if the email is already in use - return render_json_error(I18n.t('change_email.error')) if User.find_by_email(lower_email) - - email_token = user.email_tokens.create(email: lower_email) - Jobs.enqueue( - :user_email, - to_address: lower_email, - type: :authorize_email, - user_id: user.id, - email_token: email_token.token - ) - - render nothing: true - rescue RateLimiter::LimitExceeded - render_json_error(I18n.t("rate_limiter.slow_down")) - end - def authorize_email expires_now() if @user = EmailToken.confirm(params[:token]) diff --git a/app/controllers/users_email_controller.rb b/app/controllers/users_email_controller.rb new file mode 100644 index 00000000000..c7e501f52a7 --- /dev/null +++ b/app/controllers/users_email_controller.rb @@ -0,0 +1,41 @@ +require_dependency 'rate_limiter' +require_dependency 'email_validator' + +class UsersEmailController < ApplicationController + + before_filter :ensure_logged_in + + def index + end + + def update + params.require(:email) + user = fetch_user_from_params + guardian.ensure_can_edit_email!(user) + lower_email = Email.downcase(params[:email]).strip + + RateLimiter.new(user, "change-email-hr-#{request.remote_ip}", 6, 1.hour).performed! + RateLimiter.new(user, "change-email-min-#{request.remote_ip}", 3, 1.minute).performed! + + EmailValidator.new(attributes: :email).validate_each(user, :email, lower_email) + return render_json_error(user.errors.full_messages) if user.errors[:email].present? + + # Raise an error if the email is already in use + return render_json_error(I18n.t('change_email.error')) if User.find_by_email(lower_email) + + email_token = user.email_tokens.create(email: lower_email) + Jobs.enqueue( + :user_email, + to_address: lower_email, + type: :authorize_email, + user_id: user.id, + email_token: email_token.token + ) + + render nothing: true + rescue RateLimiter::LimitExceeded + render_json_error(I18n.t("rate_limiter.slow_down")) + end + +end + diff --git a/config/routes.rb b/config/routes.rb index f324bca3ae3..a33b0b80e82 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -292,8 +292,8 @@ Discourse::Application.routes.draw do put "users/:username" => "users#update", constraints: {username: USERNAME_ROUTE_FORMAT} put "users/:username/emails" => "users#check_emails", constraints: {username: USERNAME_ROUTE_FORMAT} get "users/:username/preferences" => "users#preferences", constraints: {username: USERNAME_ROUTE_FORMAT}, as: :email_preferences - get "users/:username/preferences/email" => "users#preferences", constraints: {username: USERNAME_ROUTE_FORMAT} - put "users/:username/preferences/email" => "users#change_email", constraints: {username: USERNAME_ROUTE_FORMAT} + get "users/:username/preferences/email" => "users_email#index", constraints: {username: USERNAME_ROUTE_FORMAT} + put "users/:username/preferences/email" => "users_email#update", constraints: {username: USERNAME_ROUTE_FORMAT} get "users/:username/preferences/about-me" => "users#preferences", constraints: {username: USERNAME_ROUTE_FORMAT} get "users/:username/preferences/badge_title" => "users#preferences", constraints: {username: USERNAME_ROUTE_FORMAT} put "users/:username/preferences/badge_title" => "users#badge_title", constraints: {username: USERNAME_ROUTE_FORMAT} diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 8bc4130839e..4f753041c68 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -219,75 +219,6 @@ describe UsersController do end end - describe '.change_email' do - let(:new_email) { 'bubblegum@adventuretime.ooo' } - - it "requires you to be logged in" do - expect { xhr :put, :change_email, username: 'asdf', email: new_email }.to raise_error(Discourse::NotLoggedIn) - end - - context 'when logged in' do - let!(:user) { log_in } - - it 'raises an error without an email parameter' do - expect { xhr :put, :change_email, username: user.username }.to raise_error(ActionController::ParameterMissing) - end - - it "raises an error if you can't edit the user's email" do - Guardian.any_instance.expects(:can_edit_email?).with(user).returns(false) - xhr :put, :change_email, username: user.username, email: new_email - expect(response).to be_forbidden - end - - context 'when the new email address is taken' do - let!(:other_user) { Fabricate(:coding_horror) } - it 'raises an error' do - xhr :put, :change_email, username: user.username, email: other_user.email - expect(response).to_not be_success - end - - it 'raises an error if there is whitespace too' do - xhr :put, :change_email, username: user.username, email: other_user.email + ' ' - expect(response).to_not be_success - end - end - - context 'when new email is different case of existing email' do - let!(:other_user) { Fabricate(:user, email: 'case.insensitive@gmail.com')} - - it 'raises an error' do - xhr :put, :change_email, username: user.username, email: other_user.email.upcase - expect(response).to_not be_success - end - end - - it 'raises an error when new email domain is present in email_domains_blacklist site setting' do - SiteSetting.email_domains_blacklist = "mailinator.com" - xhr :put, :change_email, username: user.username, email: "not_good@mailinator.com" - expect(response).to_not be_success - end - - it 'raises an error when new email domain is not present in email_domains_whitelist site setting' do - SiteSetting.email_domains_whitelist = "discourse.org" - xhr :put, :change_email, username: user.username, email: new_email - expect(response).to_not be_success - end - - context 'success' do - - it 'has an email token' do - expect { xhr :put, :change_email, username: user.username, email: new_email }.to change(EmailToken, :count) - end - - it 'enqueues an email authorization' do - Jobs.expects(:enqueue).with(:user_email, has_entries(type: :authorize_email, user_id: user.id, to_address: new_email)) - xhr :put, :change_email, username: user.username, email: new_email - end - end - end - - end - describe '.password_reset' do let(:user) { Fabricate(:user) } diff --git a/spec/controllers/users_email_controller_spec.rb b/spec/controllers/users_email_controller_spec.rb new file mode 100644 index 00000000000..a470246a501 --- /dev/null +++ b/spec/controllers/users_email_controller_spec.rb @@ -0,0 +1,74 @@ +require 'rails_helper' + +describe UsersEmailController do + + describe '.update' do + let(:new_email) { 'bubblegum@adventuretime.ooo' } + + it "requires you to be logged in" do + expect { xhr :put, :update, username: 'asdf', email: new_email }.to raise_error(Discourse::NotLoggedIn) + end + + context 'when logged in' do + let!(:user) { log_in } + + it 'raises an error without an email parameter' do + expect { xhr :put, :update, username: user.username }.to raise_error(ActionController::ParameterMissing) + end + + it "raises an error if you can't edit the user's email" do + Guardian.any_instance.expects(:can_edit_email?).with(user).returns(false) + xhr :put, :update, username: user.username, email: new_email + expect(response).to be_forbidden + end + + context 'when the new email address is taken' do + let!(:other_user) { Fabricate(:coding_horror) } + it 'raises an error' do + xhr :put, :update, username: user.username, email: other_user.email + expect(response).to_not be_success + end + + it 'raises an error if there is whitespace too' do + xhr :put, :update, username: user.username, email: other_user.email + ' ' + expect(response).to_not be_success + end + end + + context 'when new email is different case of existing email' do + let!(:other_user) { Fabricate(:user, email: 'case.insensitive@gmail.com')} + + it 'raises an error' do + xhr :put, :update, username: user.username, email: other_user.email.upcase + expect(response).to_not be_success + end + end + + it 'raises an error when new email domain is present in email_domains_blacklist site setting' do + SiteSetting.email_domains_blacklist = "mailinator.com" + xhr :put, :update, username: user.username, email: "not_good@mailinator.com" + expect(response).to_not be_success + end + + it 'raises an error when new email domain is not present in email_domains_whitelist site setting' do + SiteSetting.email_domains_whitelist = "discourse.org" + xhr :put, :update, username: user.username, email: new_email + expect(response).to_not be_success + end + + context 'success' do + + it 'has an email token' do + expect { xhr :put, :update, username: user.username, email: new_email }.to change(EmailToken, :count) + end + + it 'enqueues an email authorization' do + Jobs.expects(:enqueue).with(:user_email, has_entries(type: :authorize_email, user_id: user.id, to_address: new_email)) + xhr :put, :update, username: user.username, email: new_email + end + end + end + + end + +end