From 431076fc6d8d8b6482490d3ab4a7a5815cd151cf Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 22 Feb 2020 19:03:48 +0000 Subject: [PATCH 1/6] Add strong parameters permits to admin users_controller --- app/controllers/spree/admin/users_controller.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/spree/admin/users_controller.rb b/app/controllers/spree/admin/users_controller.rb index a1db20605c..6d8337a5ce 100644 --- a/app/controllers/spree/admin/users_controller.rb +++ b/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 + params.require(:user).permit(:email, :enterprise_limit, :password, :password_confirmation) + end end end end From d0bd2818c22f2c203d4e97af78f170f194061606 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sun, 23 Feb 2020 17:34:39 +0000 Subject: [PATCH 2/6] Handle strong params on users_controller --- app/controllers/spree/users_controller.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/spree/users_controller.rb b/app/controllers/spree/users_controller.rb index c620136a94..99b7fa73de 100644 --- a/app/controllers/spree/users_controller.rb +++ b/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 + params.require(:user).permit(:email, :password, :password_confirmation) + end end end From 6ed93da3f130e3760bbb1583d4ba768ef7be56b7 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 5 Mar 2020 18:43:35 +0000 Subject: [PATCH 3/6] Fix case with empty spree_user in user_registrations controller --- app/controllers/user_registrations_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/user_registrations_controller.rb b/app/controllers/user_registrations_controller.rb index 258d990828..0482fc39c7 100644 --- a/app/controllers/user_registrations_controller.rb +++ b/app/controllers/user_registrations_controller.rb @@ -33,6 +33,8 @@ class UserRegistrationsController < Spree::UserRegistrationsController private def spree_user_params + return params[:spree_user] if params[:spree_user].empty? + params.require(:spree_user). permit(:email, :password, :password_confirmation, :remember_me) end From aec7f12f5aecc31aa222ec72a35495b443a3890e Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 21 Mar 2020 18:08:30 +0000 Subject: [PATCH 4/6] Extract common user permitted attributes to a separate class --- .../spree/admin/users_controller.rb | 2 +- app/controllers/spree/users_controller.rb | 2 +- .../user_registrations_controller.rb | 3 +-- app/services/permitted_attributes/user.rb | 21 +++++++++++++++++++ 4 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 app/services/permitted_attributes/user.rb diff --git a/app/controllers/spree/admin/users_controller.rb b/app/controllers/spree/admin/users_controller.rb index 6d8337a5ce..d5525936a3 100644 --- a/app/controllers/spree/admin/users_controller.rb +++ b/app/controllers/spree/admin/users_controller.rb @@ -138,7 +138,7 @@ module Spree end def user_params - params.require(:user).permit(:email, :enterprise_limit, :password, :password_confirmation) + PermittedAttributes::User.new(params).call([:enterprise_limit]) end end end diff --git a/app/controllers/spree/users_controller.rb b/app/controllers/spree/users_controller.rb index 99b7fa73de..03aced66c8 100644 --- a/app/controllers/spree/users_controller.rb +++ b/app/controllers/spree/users_controller.rb @@ -72,7 +72,7 @@ module Spree end def user_params - params.require(:user).permit(:email, :password, :password_confirmation) + ::PermittedAttributes::User.new(params).call end end end diff --git a/app/controllers/user_registrations_controller.rb b/app/controllers/user_registrations_controller.rb index 0482fc39c7..2e9870b37e 100644 --- a/app/controllers/user_registrations_controller.rb +++ b/app/controllers/user_registrations_controller.rb @@ -35,8 +35,7 @@ class UserRegistrationsController < Spree::UserRegistrationsController def spree_user_params return params[:spree_user] if params[:spree_user].empty? - params.require(:spree_user). - permit(:email, :password, :password_confirmation, :remember_me) + PermittedAttributes::User.new(params, :spree_user).call([:remember_me]) end def render_error(errors = {}) diff --git a/app/services/permitted_attributes/user.rb b/app/services/permitted_attributes/user.rb new file mode 100644 index 0000000000..4efbf0e3b5 --- /dev/null +++ b/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 From 0ee562c718a20155aa47a97937ad68330749a685 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 21 Mar 2020 18:47:25 +0000 Subject: [PATCH 5/6] Add test coverage for PermittedAttributes::User --- .../permitted_attributes/user_spec.rb | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 spec/services/permitted_attributes/user_spec.rb diff --git a/spec/services/permitted_attributes/user_spec.rb b/spec/services/permitted_attributes/user_spec.rb new file mode 100644 index 0000000000..8b98311efa --- /dev/null +++ b/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 From 5e6a210632ad54bd2556514c592d437a8b52f582 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 21 Mar 2020 22:34:40 +0000 Subject: [PATCH 6/6] Fix problem in PermittedAttributes::User namespace --- app/controllers/spree/admin/users_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/users_controller.rb b/app/controllers/spree/admin/users_controller.rb index d5525936a3..d70aaa8d9b 100644 --- a/app/controllers/spree/admin/users_controller.rb +++ b/app/controllers/spree/admin/users_controller.rb @@ -138,7 +138,7 @@ module Spree end def user_params - PermittedAttributes::User.new(params).call([:enterprise_limit]) + ::PermittedAttributes::User.new(params).call([:enterprise_limit]) end end end