From 9364d8ce71c76814d0982769375be71545aa7710 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 8d4b2250bf5..f06022cf98d 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 0e429e8c8c1..a06c6c376b5 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 4bb4c871bc7..e98612fe442 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