From cb731dbecd9e02d1a58ce0d854fbdfbad70b9866 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Mon, 13 Mar 2017 20:20:25 +0800 Subject: [PATCH] FIX: Store user's id instead for sending activation email. * Email and username are both allowed to be used for logging in. Therefore, it is easier to just store the user's id rather than to store the username and email in the session. --- app/controllers/session_controller.rb | 2 +- app/controllers/users_controller.rb | 14 +++++++------- spec/controllers/users_controller_spec.rb | 17 +++++++++++++---- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 8d4b2250bf..f06022cf98 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -278,7 +278,7 @@ class SessionController < ApplicationController end def not_activated(user) - session[ACTIVATE_USER_KEY] = user.username + session[ACTIVATE_USER_KEY] = user.id render json: { error: I18n.t("login.not_activated"), reason: 'not_activated', diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0e429e8c8c..a06c6c376b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -567,21 +567,21 @@ 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 + if (current_user && !current_user.staff?) || + @user.id != session[SessionController::ACTIVATE_USER_KEY] + + raise Discourse::InvalidAccess + end + session.delete(SessionController::ACTIVATE_USER_KEY) if @user.active render_json_error(I18n.t('activation.activated'), status: 409) - elsif @user + else @user @email_token = @user.email_tokens.unconfirmed.active.first enqueue_activation_email render nothing: true diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 4bb4c871bc..e98612fe44 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1406,7 +1406,7 @@ describe UsersController do context 'for an activated account' do it 'fails' do active_user = Fabricate(:user, active: true) - session[SessionController::ACTIVATE_USER_KEY] = active_user.username + session[SessionController::ACTIVATE_USER_KEY] = active_user.id xhr :post, :send_activation_email, username: active_user.username expect(response.status).to eq(409) @@ -1419,9 +1419,18 @@ describe UsersController do end end + describe 'when user does not have a valid session' do + it 'should not be valid' do + user = Fabricate(:user) + xhr :post, :send_activation_email, username: user.username + + expect(response.status).to eq(403) + end + end + context 'with a valid email_token' do it 'should send the activation email' do - session["activate_user"] = user.username + session[SessionController::ACTIVATE_USER_KEY] = user.id Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) xhr :post, :send_activation_email, username: user.username @@ -1437,13 +1446,13 @@ describe UsersController do it 'should generate a new token' do expect { - session["activate_user"] = user.username + session[SessionController::ACTIVATE_USER_KEY] = user.id 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 + session[SessionController::ACTIVATE_USER_KEY] = user.id Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) xhr :post, :send_activation_email, username: user.username