From 7ebfa3c901b75aee2e294819bfc0aaddaeebcd84 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Mon, 13 Mar 2017 19:19:42 +0800 Subject: [PATCH] SECURITY: Only allow users to resend activation email with a valid session. * Improve error when an active user tries to request for an activation email. --- .../controllers/not-activated.js.es6 | 11 ++++++--- app/controllers/session_controller.rb | 4 ++++ app/controllers/users_controller.rb | 18 ++++++++++++--- config/locales/server.en.yml | 1 + spec/controllers/users_controller_spec.rb | 23 +++++++++++++++++++ 5 files changed, 51 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/not-activated.js.es6 b/app/assets/javascripts/discourse/controllers/not-activated.js.es6 index 1c38cc58e59..86afe2ddd7c 100644 --- a/app/assets/javascripts/discourse/controllers/not-activated.js.es6 +++ b/app/assets/javascripts/discourse/controllers/not-activated.js.es6 @@ -1,4 +1,5 @@ import { ajax } from 'discourse/lib/ajax'; +import { popupAjaxError } from 'discourse/lib/ajax-error'; import ModalFunctionality from 'discourse/mixins/modal-functionality'; export default Ember.Controller.extend(ModalFunctionality, { @@ -9,9 +10,13 @@ export default Ember.Controller.extend(ModalFunctionality, { }, actions: { - sendActivationEmail: function() { - ajax('/users/action/send_activation_email', {data: {username: this.get('username')}, type: 'POST'}); - this.set('emailSent', true); + sendActivationEmail() { + ajax('/users/action/send_activation_email', { + data: { username: this.get('username') }, + type: 'POST' + }).then(() => { + this.set('emailSent', true); + }).catch(popupAjaxError); } } diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 0a18b3c539a..8d4b2250bf5 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -6,6 +6,8 @@ class SessionController < ApplicationController skip_before_filter :redirect_to_login_if_required skip_before_filter :preload_json, :check_xhr, only: ['sso', 'sso_login', 'become', 'sso_provider', 'destroy'] + ACTIVATE_USER_KEY = "activate_user" + def csrf render json: {csrf: form_authenticity_token } end @@ -276,6 +278,7 @@ class SessionController < ApplicationController end def not_activated(user) + session[ACTIVATE_USER_KEY] = user.username render json: { error: I18n.t("login.not_activated"), reason: 'not_activated', @@ -303,6 +306,7 @@ class SessionController < ApplicationController end def login(user) + session.delete(ACTIVATE_USER_KEY) log_on_user(user) if payload = session.delete(:sso_payload) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index b06e6721efa..0e429e8c8c1 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -567,13 +567,25 @@ class UsersController < ApplicationController RateLimiter.new(nil, "activate-min-#{request.remote_ip}", 6, 1.minute).performed! end + if (current_user && !current_user.staff?) || + (params[:username] != session[SessionController::ACTIVATE_USER_KEY]) + + raise Discourse::InvalidAccess + end + @user = User.find_by_username_or_email(params[:username].to_s) raise Discourse::NotFound unless @user - @email_token = @user.email_tokens.unconfirmed.active.first - enqueue_activation_email if @user - render nothing: true + session.delete(SessionController::ACTIVATE_USER_KEY) + + if @user.active + render_json_error(I18n.t('activation.activated'), status: 409) + elsif @user + @email_token = @user.email_tokens.unconfirmed.active.first + enqueue_activation_email + render nothing: true + end end def enqueue_activation_email diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index aa6423dccec..1619309dd96 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -614,6 +614,7 @@ en: welcome_to: "Welcome to %{site_name}!" approval_required: "A moderator must manually approve your new account before you can access this forum. You'll get an email when your account is approved!" missing_session: "We cannot detect if your account was created, please ensure you have cookies enabled." + activated: "Sorry, this account has already been activated." post_action_types: off_topic: title: 'Off-Topic' diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 746d26a0712..4bb4c871bc7 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1403,10 +1403,29 @@ describe UsersController do context 'for an existing user' do let(:user) { Fabricate(:user, active: false) } + context 'for an activated account' do + it 'fails' do + active_user = Fabricate(:user, active: true) + session[SessionController::ACTIVATE_USER_KEY] = active_user.username + xhr :post, :send_activation_email, username: active_user.username + + expect(response.status).to eq(409) + + expect(JSON.parse(response.body)['errors']).to include(I18n.t( + 'activation.activated' + )) + + expect(session[SessionController::ACTIVATE_USER_KEY]).to eq(nil) + end + end + context 'with a valid email_token' do it 'should send the activation email' do + session["activate_user"] = user.username Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) xhr :post, :send_activation_email, username: user.username + + expect(session[SessionController::ACTIVATE_USER_KEY]).to eq(nil) end end @@ -1418,13 +1437,17 @@ describe UsersController do it 'should generate a new token' do expect { + session["activate_user"] = user.username xhr :post, :send_activation_email, username: user.username }.to change{ user.email_tokens(true).count }.by(1) end it 'should send an email' do + session["activate_user"] = user.username Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) xhr :post, :send_activation_email, username: user.username + + expect(session[SessionController::ACTIVATE_USER_KEY]).to eq(nil) end end end