THREESCALE-12434: Migrate from protected attributes to strong parameters - Part 2

app/controllers/admin/api/buyers_users_controller.rb

@@ -1,6 +1,11 @@
 class Admin::Api::BuyersUsersController < Admin::Api::BuyersBaseController
   representer User
 
+  before_action :find_user, except: %i[create index]
+  before_action :build_new_user, only: %i[create]
+
+  attr_reader :user
+
   # User List
   # GET /admin/api/accounts/{account_id}/users.xml
   def index
@@ -12,14 +17,9 @@ def index
   # User Create
   # POST /admin/api/accounts/{account_id}/users.xml
   def create
-    user = new_user
-
     authorize! :create, user
 
-    user.unflattened_attributes = flat_params
-    user.signup_type = :api
-
-    user.save
+    user.update(user_params.merge(signup_type: :api))
 
     respond_with(user)
   end
@@ -37,7 +37,7 @@ def show
   def update
     authorize! :update, user
 
-    user.update_with_flattened_attributes(flat_params)
+    user.update(user_params)
 
     respond_with(user)
   end
@@ -108,19 +108,24 @@ def authorize!(*args)
     current_user ? super : logged_in?
   end
 
-  def new_user
-    @new_user ||= buyer.users.new
-  end
-
   def users
     @users ||= begin
       conditions = params.slice(:state, :role)
       buyer.users.where(conditions)
     end
   end
 
-  def user
-    @user ||= buyer.users.find(params[:id])
+  private
+
+  def build_new_user
+    @user = buyer.users.new
   end
 
+  def find_user
+    @user = buyer.users.find(params[:id])
+  end
+
+  def user_params
+    params.permit(*user.defined_fields_names, :password, :password_confirmation)
+  end
 end

app/controllers/admin/api/users_controller.rb

@@ -2,6 +2,10 @@ class Admin::Api::UsersController < Admin::Api::BaseController
   representer User
 
   before_action :can_create, only: :create
+  before_action :build_new_user, only: %i[create]
+  before_action :find_user, except: %i[create index]
+
+  attr_reader :user
 
   # User List (provider account)
   # GET /admin/api/users.xml
@@ -14,14 +18,9 @@ def index
   # User Create (provider account)
   # POST /admin/api/users.xml
   def create
-    user = new_user
-
     authorize! :create, user
 
-    user.unflattened_attributes = flat_params
-    user.signup_type = :api
-
-    user.save
+    user.update(user_params.merge(signup_type: :api))
 
     respond_with(user)
   end
@@ -39,7 +38,7 @@ def show
   def update
     authorize! :update, user
 
-    user.update_with_flattened_attributes(flat_params, as: current_user.try(:role))
+    user.update(user_params)
 
     respond_with(user)
   end
@@ -110,19 +109,19 @@ def authorize!(*args)
     current_user ? super : logged_in?
   end
 
-  def new_user
-    @new_user ||= current_account.users.new
-  end
-
   def users
     @users ||= begin
       conditions = params.slice(:state, :role)
       current_account.users.but_impersonation_admin.where(conditions)
     end
   end
 
-  def user
-    @user ||= current_account.users.but_impersonation_admin.find(params[:id])
+  def build_new_user
+    @user = current_account.users.new
+  end
+
+  def find_user
+    @user = current_account.users.but_impersonation_admin.find(params[:id])
   end
 
   def can_create
@@ -131,7 +130,10 @@ def can_create
 
   private
 
-  def flat_params
-    super.except(:id)
+  def user_params
+    permission_attrs = [:member_permission_service_ids, { member_permission_service_ids: [], member_permission_ids: [] }]
+    allowed_attrs = user.defined_fields_names + %w[password password_confirmation cas_identifier]
+    allowed_attrs += permission_attrs if provider_key.present? || current_user.admin?
+    params.permit(*allowed_attrs)
   end
 end

app/controllers/buyers/users_controller.rb

@@ -22,10 +22,7 @@ def show
   def edit; end
 
   def update
-    # TODO: I think this controller is used only on provider side
-    user.validate_fields! if current_account.buyer?
-
-    user.attributes = user_params
+    user.assign_attributes(permitted_user_params)
     user.role = user_params.fetch(:role, user.role) if can?(:update_role, user)
 
     if user.save
@@ -86,10 +83,13 @@ def find_user
     @user = @account.users.find(params[:id]).decorate
   end
 
-  DEFAULT_PARAMS = %i[username email password password_confirmation role].freeze
-
   def user_params
-    @user_params ||= params.require(:user).permit(*DEFAULT_PARAMS, extra_fields: [*user.defined_extra_fields_names])
+    @user_params ||= params.require(:user)
+  end
+
+  def permitted_user_params
+    user_params.permit(*user.defined_builtin_fields_names, :password, :password_confirmation,
+                       extra_fields: user.defined_extra_fields_names)
   end
 
   def redirect_back_or_show_detail(**opts)

app/controllers/partners/users_controller.rb

@@ -20,13 +20,7 @@ def destroy
   end
 
   def create
-    @user = @account.users.build
-    @user.email = params[:email]
-    @user.password = params[:password].presence
-    @user.first_name = params[:first_name].presence
-    @user.last_name = params[:last_name].presence
-    @user.open_id = params[:open_id].presence
-    @user.username = params[:username]
+    @user = @account.users.build(user_params)
     @user.signup_type = :partner
     @user.role = :admin
     @user.activate!
@@ -42,4 +36,9 @@ def create
   def find_account
     @account = @partner.providers.find(params[:provider_id])
   end
+
+  def user_params
+    allowed_attrs = %i[email password first_name last_name open_id username]
+    params.permit(*allowed_attrs)
+  end
 end

app/controllers/provider/admin/account/users_controller.rb

@@ -25,7 +25,6 @@ def update
     @user.validate_fields!
 
     @user.assign_attributes(user_params)
-    @user.role = user_params.fetch(:role, @user.role)
 
     if @user.save
       redirect_to provider_admin_account_users_path, success: t('.success')
@@ -57,7 +56,7 @@ def users
   end
 
   def user_params
-    allowed_attrs = @user.defined_builtin_fields.map(&:name) + @user.special_fields
+    allowed_attrs = @user.defined_builtin_fields_names + %w[password password_confirmation]
 
     if can?(:update_role, @user)
       allowed_attrs += [:role, { member_permission_ids: [] }]

app/controllers/provider/admin/user/personal_details_controller.rb

@@ -5,7 +5,7 @@ class Provider::Admin::User::PersonalDetailsController < Provider::Admin::User::
   def edit; end
 
   def update
-    if current_user.update(user_params.except(:current_password))
+    if current_user.update(permitted_user_params)
       if current_user.just_changed_password?
         current_user.kill_user_sessions(user_session)
       end
@@ -26,7 +26,12 @@ def redirect_path
   end
 
   def user_params
-    params.require(:user)
+    @user_params ||= params.require(:user)
+  end
+
+  def permitted_user_params
+    allowed_attrs = current_user.defined_builtin_fields_names + %w[password]
+    user_params.permit(*allowed_attrs, extra_fields: current_user.defined_extra_fields_names)
   end
 
   def current_password_verification

app/controllers/provider/invitee_signups_controller.rb

@@ -1,19 +1,21 @@
+# frozen_string_literal: true
+
 class Provider::InviteeSignupsController < FrontendController
   skip_before_action :login_required
 
   before_action :redirect_if_logged_in
   before_action :ask_for_upgrade
 
   before_action :find_invitation
-  before_action :build_user
+  before_action :build_new_user
   before_action :instantiate_sessions_presenter
 
   layout 'provider/login'
 
-  def show
-  end
+  def show; end
 
   def create
+    @user.assign_attributes(user_params)
     @user.admin_sections = domain_account.provider_can_use?(:service_permissions) ? [] : ['monitoring']
 
     if can_create? && @user.save
@@ -23,11 +25,7 @@ def create
 
       redirect_to provider_login_path, success: t('.success')
     else
-      errors = @user.errors.full_messages.reduce do |result, error|
-        "#{result}\n#{error}"
-      end
-
-      flash.now[:danger] = t('.error', errors: errors)
+      flash.now[:danger] = t('.error', errors: user_errors)
       render 'show'
     end
   end
@@ -52,12 +50,12 @@ def can_create?
     account.provider_constraints.can_create_user?
   end
 
-  def build_user
-    @user = @invitation.make_user(params[:user] || {})
+  def build_new_user
+    @user = @invitation.make_user
+  end
 
-    # This is just a sanity guard added when splitting invitation
-    # controllers. Remove when SURE.
-    raise 'Developer invitation used and worked on provider side!' unless @user.account.provider?
+  def user_params
+    params.require(:user).permit(@user.defined_fields_names, :password, :password_confirmation)
   end
 
   def invitation_token
@@ -67,4 +65,12 @@ def invitation_token
   def instantiate_sessions_presenter
     @presenter = Provider::SessionsPresenter.new(domain_account)
   end
+
+  def user_errors
+    return "" unless @user
+
+    @user.errors.full_messages.reduce do |result, error|
+      "#{result}\n#{error}"
+    end
+  end
 end

app/lib/fields/fields.rb

@@ -275,6 +275,14 @@ def defined_extra_fields_names
     defined_extra_fields.map(&:name)
   end
 
+  def defined_fields_names
+    defined_fields.map(&:name)
+  end
+
+  def defined_builtin_fields_names
+    defined_builtin_fields.map(&:name)
+  end
+
   def defined_fields_hash
     @defined_fields_hash ||= Hash[defined_fields.map { |f| [f.name.to_sym, f]}]
   end

app/models/invitation.rb

@@ -24,7 +24,7 @@ class Invitation < ApplicationRecord
 
   # Build new user on information in this invitation.
   def make_user(params = {})
-    self.user= account.users.build_with_fields params.reverse_merge(:email => email, :invitation => self)
+    self.user = account.users.build_with_fields params.reverse_merge(email: email, invitation: self)
   end
 
   def accepted?

app/models/user.rb

@@ -109,14 +109,6 @@ def moderatable
   validate :username_is_unique
   validates :open_id, uniqueness: { case_sensitive: true }, allow_nil: true
 
-  attr_accessible :title, :username, :email, :first_name, :last_name,
-                  :conditions, :cas_identifier, :open_id, :service_conditions,
-                  :job_role, :extra_fields, as: %i[default member admin]
-
-  attr_accessible :member_permission_service_ids, :member_permission_ids, as: %i[admin]
-
-
-
   def self.search_states
     %w(pending active)
   end

app/models/user/invitations.rb

@@ -5,12 +5,11 @@ module User::Invitations
     after_commit :accept_invitation, :on => :create
 
     attr_accessor :invitation
-    attr_accessible :invitation
 
     # TODO: refactor to make this work removing above attribute.
     # has_one :invitation
 
-    before_destroy  :destroy_invitation
+    before_destroy :destroy_invitation
   end
 
   def accept_invitation

app/models/user/permissions.rb

@@ -8,8 +8,6 @@ module User::Permissions
   included do
     has_many :member_permissions, dependent: :destroy, autosave: true
 
-    attr_accessible :member_permission_service_ids, :member_permission_ids, :allowed_sections, :allowed_service_ids
-
     alias_method :allowed_sections, :member_permission_ids
     alias_method :allowed_sections=, :member_permission_ids=
     alias_method :allowed_service_ids, :member_permission_service_ids