SECURITY: Avoid mass assignment on user create

app/controllers/users_controller.rb

@@ -343,7 +343,7 @@ class UsersController < ApplicationController
         ),
         errors: user.errors.to_hash,
         values: user.attributes.slice('name', 'username', 'email'),
-        is_developer: UsernameCheckerService.new.is_developer?(user.email)
+        is_developer: UsernameCheckerService.is_developer?(user.email)
       }
     end
   rescue ActiveRecord::StatementInvalid
@@ -674,9 +674,20 @@ class UsersController < ApplicationController
     end
 
     def user_params
-      params.permit(:name, :email, :password, :username, :active)
+      result = params.permit(:name, :email, :password, :username)
-            .merge(ip_address: request.remote_ip, registration_ip_address: request.remote_ip,
+                     .merge(ip_address: request.remote_ip,
-                   locale: user_locale)
+                            registration_ip_address: request.remote_ip,
+                            locale: user_locale)
+
+      if !UsernameCheckerService.is_developer?(result['email']) &&
+          is_api? &&
+          current_user.present? &&
+          current_user.admin?
+
+        result.merge!(params.permit(:active, :staged))
+      end
+
+      result
     end
 
     def user_locale

app/serializers/search_post_serializer.rb

@@ -1,5 +1,4 @@
 class SearchPostSerializer < PostSerializer
-
   has_one :topic, serializer: TopicListItemSerializer
 
   attributes :like_count

app/services/username_checker_service.rb

@@ -23,4 +23,9 @@ class UsernameCheckerService
     Rails.configuration.respond_to?(:developer_emails) && Rails.configuration.developer_emails.include?(value)
   end
 
+
+  def self.is_developer?(email)
+    UsernameCheckerService.new.is_developer?(email)
+  end
+
 end

spec/controllers/users_controller_spec.rb

@@ -382,12 +382,15 @@ describe UsersController do
       @user.password = "strongpassword"
     end
 
-    def post_user
+    let(:post_user_params) do
-      xhr :post, :create,
+      { name: @user.name,
-        name: @user.name,
         username: @user.username,
         password: "strongpassword",
-        email: @user.email
+        email: @user.email }
+    end
+
+    def post_user
+      xhr :post, :create, post_user_params
     end
 
     context 'when creating a user' do
@@ -453,6 +456,79 @@ describe UsersController do
       end
     end
 
+    context "creating as active" do
+      it "won't create the user as active" do
+        xhr :post, :create, post_user_params.merge(active: true)
+        expect(JSON.parse(response.body)['active']).to be_falsey
+      end
+
+      context "with a regular api key" do
+        let(:user) { Fabricate(:user) }
+        let(:api_key) { Fabricate(:api_key, user: user) }
+
+        it "won't create the user as active with a regular key" do
+          xhr :post, :create, post_user_params.merge(active: true, api_key: api_key.key)
+          expect(JSON.parse(response.body)['active']).to be_falsey
+        end
+      end
+
+      context "with an admin api key" do
+        let(:user) { Fabricate(:admin) }
+        let(:api_key) { Fabricate(:api_key, user: user) }
+
+        it "creates the user as active with a regular key" do
+          xhr :post, :create, post_user_params.merge(active: true, api_key: api_key.key)
+          expect(JSON.parse(response.body)['active']).to be_truthy
+        end
+
+        it "won't create the developer as active" do
+          UsernameCheckerService.expects(:is_developer?).returns(true)
+
+          xhr :post, :create, post_user_params.merge(active: true, api_key: api_key.key)
+          expect(JSON.parse(response.body)['active']).to be_falsy
+        end
+      end
+    end
+
+    context "creating as staged" do
+      it "won't create the user as staged" do
+        xhr :post, :create, post_user_params.merge(staged: true)
+        new_user = User.where(username: post_user_params[:username]).first
+        expect(new_user.staged?).to eq(false)
+      end
+
+      context "with a regular api key" do
+        let(:user) { Fabricate(:user) }
+        let(:api_key) { Fabricate(:api_key, user: user) }
+
+        it "won't create the user as staged with a regular key" do
+          xhr :post, :create, post_user_params.merge(staged: true, api_key: api_key.key)
+          new_user = User.where(username: post_user_params[:username]).first
+          expect(new_user.staged?).to eq(false)
+        end
+      end
+
+      context "with an admin api key" do
+        let(:user) { Fabricate(:admin) }
+        let(:api_key) { Fabricate(:api_key, user: user) }
+
+        it "creates the user as staged with a regular key" do
+          xhr :post, :create, post_user_params.merge(staged: true, api_key: api_key.key)
+
+          new_user = User.where(username: post_user_params[:username]).first
+          expect(new_user.staged?).to eq(true)
+        end
+
+        it "won't create the developer as staged" do
+          UsernameCheckerService.expects(:is_developer?).returns(true)
+          xhr :post, :create, post_user_params.merge(staged: true, api_key: api_key.key)
+
+          new_user = User.where(username: post_user_params[:username]).first
+          expect(new_user.staged?).to eq(false)
+        end
+      end
+    end
+
     context 'when creating an active user (confirmed email)' do
       before { User.any_instance.stubs(:active?).returns(true) }