Merge pull request #5036 from luisramos0/strong_params_users

[Spree 2.1] Implement strong params in user related controllers

app/controllers/spree/admin/users_controller.rb

@@ -22,7 +22,7 @@ module Spree
           roles = params[:user].delete("spree_role_ids")
         end
 
-        @user = Spree::User.new(params[:user])
+        @user = Spree::User.new(user_params)
         if @user.save
 
           if roles
@@ -41,7 +41,7 @@ module Spree
           roles = params[:user].delete("spree_role_ids")
         end
 
-        if @user.update_attributes(params[:user])
+        if @user.update_attributes(user_params)
           if roles
             @user.spree_roles = roles.reject(&:blank?).collect{ |r| Spree::Role.find(r) }
           end
@@ -136,6 +136,10 @@ module Spree
       def new_email_unconfirmed?
         params[:user][:email] != @user.email
       end
+
+      def user_params
+        ::PermittedAttributes::User.new(params).call([:enterprise_limit])
+      end
     end
   end
 end

app/controllers/spree/users_controller.rb

@@ -25,7 +25,7 @@ module Spree
     end
 
     def create
-      @user = Spree::User.new(params[:user])
+      @user = Spree::User.new(user_params)
       if @user.save
 
         if current_order
@@ -39,7 +39,7 @@ module Spree
     end
 
     def update
-      if @user.update_attributes(params[:user])
+      if @user.update_attributes(user_params)
         if params[:user][:password].present?
           # this logic needed b/c devise wants to log us out after password changes
           Spree::User.reset_password_by_token(params[:user])
@@ -70,5 +70,9 @@ module Spree
     def accurate_title
       Spree.t(:my_account)
     end
+
+    def user_params
+      ::PermittedAttributes::User.new(params).call
+    end
   end
 end

app/controllers/user_registrations_controller.rb

@@ -33,8 +33,9 @@ class UserRegistrationsController < Spree::UserRegistrationsController
   private
 
   def spree_user_params
-    params.require(:spree_user).
-      permit(:email, :password, :password_confirmation, :remember_me)
+    return params[:spree_user] if params[:spree_user].empty?
+
+    PermittedAttributes::User.new(params, :spree_user).call([:remember_me])
   end
 
   def render_error(errors = {})

app/services/permitted_attributes/user.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module PermittedAttributes
+  class User
+    def initialize(params, resource_name = :user)
+      @params = params
+      @resource_name = resource_name
+    end
+
+    def call(extra_permitted_attributes = [])
+      @params.require(@resource_name).
+        permit(permitted_attributes + extra_permitted_attributes)
+    end
+
+    private
+
+    def permitted_attributes
+      [:email, :password, :password_confirmation]
+    end
+  end
+end

spec/services/permitted_attributes/user_spec.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+module PermittedAttributes
+  describe User do
+    describe "simple usage" do
+      let(:user_permitted_attributes) { PermittedAttributes::User.new(params) }
+
+      describe "permits basic attributes" do
+        let(:params) {
+          ActionController::Parameters.new(user: { name: "John",
+                                                   email: "email@example.com" } )
+        }
+
+        it "keeps permitted and removes not permitted" do
+          permitted_attributes = user_permitted_attributes.call
+
+          expect(permitted_attributes[:name]).to be nil
+          expect(permitted_attributes[:email]).to eq "email@example.com"
+        end
+
+        it "keeps extra permitted attributes" do
+          permitted_attributes = user_permitted_attributes.call([:name])
+
+          expect(permitted_attributes[:name]).to eq "John"
+          expect(permitted_attributes[:email]).to eq "email@example.com"
+        end
+      end
+    end
+
+    describe "with custom resource_name" do
+      let(:user_permitted_attributes) { PermittedAttributes::User.new(params, :spree_user) }
+      let(:params) {
+        ActionController::Parameters.new(spree_user: { name: "John",
+                                                       email: "email@example.com" } )
+      }
+
+      it "keeps permitted and removes not permitted" do
+        permitted_attributes = user_permitted_attributes.call
+
+        expect(permitted_attributes[:name]).to be nil
+        expect(permitted_attributes[:email]).to eq "email@example.com"
+      end
+    end
+  end
+end