From 96584403cdd1a3098888321ece254941db7535ea Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Thu, 14 Dec 2017 17:16:49 +1100
Subject: [PATCH] SECURITY: prevent staged accounts from changing email

---
 app/controllers/users_controller.rb       |  9 ++++-
 spec/controllers/users_controller_spec.rb | 18 ----------
 spec/requests/users_controller_spec.rb    | 42 +++++++++++++++++++++++
 3 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index bede58eba15..593a402ea70 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -298,6 +298,8 @@ class UsersController < ApplicationController
     params[:for_user_id] ? User.find(params[:for_user_id]) : current_user
   end
 
+  FROM_STAGED = "from_staged".freeze
+
   def create
     params.require(:email)
     params.permit(:user_fields)
@@ -322,6 +324,7 @@ class UsersController < ApplicationController
       user_params.each { |k, v| user.send("#{k}=", v) }
       user.staged = false
       user.active = false
+      user.custom_fields[FROM_STAGED] = true
     else
       user = User.new(user_params)
     end
@@ -590,7 +593,7 @@ class UsersController < ApplicationController
       if user = User.where(id: session_user_id.to_i).first
         @account_created[:username] = user.username
         @account_created[:email] = user.email
-        @account_created[:show_controls] = true
+        @account_created[:show_controls] = !user.custom_fields[FROM_STAGED]
       end
     end
 
@@ -648,6 +651,10 @@ class UsersController < ApplicationController
       raise Discourse::InvalidAccess.new
     end
 
+    if @user.custom_fields[FROM_STAGED]
+      raise Discourse::InvalidAccess.new
+    end
+
     User.transaction do
       primary_email = @user.primary_email
 
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index ca244ffca6e..b1c527b29fa 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -1034,24 +1034,6 @@ describe UsersController do
       end
     end
 
-    context "when taking over a staged account" do
-      let!(:staged) { Fabricate(:staged, email: "staged@account.com", active: true) }
-
-      it "succeeds" do
-        post :create, params: {
-          email: staged.email, username: "zogstrip", password: "P4ssw0rd$$"
-        }, format: :json
-
-        result = ::JSON.parse(response.body)
-        expect(result["success"]).to eq(true)
-
-        created_user = User.find_by_email(staged.email)
-        expect(created_user.staged).to eq(false)
-        expect(created_user.active).to eq(false)
-        expect(created_user.registration_ip_address).to be_present
-      end
-    end
-
   end
 
   context '#username' do
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index 7f3da8e9e42..5040be8e6c8 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -3,6 +3,48 @@ require 'rails_helper'
 RSpec.describe UsersController do
   let(:user) { Fabricate(:user) }
 
+  def honeypot_magic(params)
+    get '/u/hp.json'
+    json = JSON.parse(response.body)
+    params[:password_confirmation] = json["value"]
+    params[:challenge] = json["challenge"].reverse
+    params
+  end
+
+  describe '#create' do
+
+    context "when taking over a staged account" do
+      let!(:staged) { Fabricate(:staged, email: "staged@account.com", active: true) }
+
+      it "succeeds" do
+        post '/u.json', params: honeypot_magic(
+          email: staged.email,
+          username: "zogstrip",
+          password: "P4ssw0rd$$"
+        )
+
+        expect(response.status).to eq(200)
+        result = ::JSON.parse(response.body)
+        expect(result["success"]).to eq(true)
+
+        created_user = User.find_by_email(staged.email)
+        expect(created_user.staged).to eq(false)
+        expect(created_user.active).to eq(false)
+        expect(created_user.registration_ip_address).to be_present
+        expect(!!created_user.custom_fields["from_staged"]).to eq(true)
+
+        # do not allow emails changes please
+
+        put "/u/update-activation-email.json", params: { email: 'bob@bob.com' }
+
+        created_user.reload
+        expect(created_user.email).to eq("staged@account.com")
+        expect(response.status).not_to eq(200)
+      end
+    end
+
+  end
+
   describe '#show' do
 
     it "should be able to view a user" do