[WIP] [PoC] Migrate from protected attributes to strong parameters

app/controllers/admin/api/application_plan_metric_limits_controller.rb

@@ -61,7 +61,7 @@ def usage_limit
   end
 
   def usage_limit_params
-    params.fetch(:usage_limit)
+    params.fetch(:usage_limit).permit(:period, :value)
   end
 
 end

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,28 @@ 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
+    @user_params ||= begin
+      allowed_attrs = user.defined_fields_names | %i(password password_confirmation signup_type)
+      flat_params.permit(*allowed_attrs)
+    end
   end
 
 end

app/controllers/admin/api/providers_controller.rb

@@ -13,7 +13,7 @@ def show
   # Provider Account Update
   # PUT /admin/api/provider.xml
   def update
-    current_account.update(provider_params, without_protection: true)
+    current_account.update(provider_params)
     respond_with current_account
   end
 

app/controllers/admin/api/signups_controller.rb

@@ -7,16 +7,30 @@ class Admin::Api::SignupsController < Admin::Api::BaseController
   def create
     authorize!(:create, Account) if current_user
 
-    @signup_result = Signup::DeveloperAccountManager.new(current_account).create(signup_params)
+    @signup_result = account_manager.create(signup_params)
 
     check_creation_errors
     respond_with(@signup_result.account, user_options: { with_apps: true })
   end
 
   private
 
+  def account_manager
+    @account_manager ||= Signup::DeveloperAccountManager.new(current_account)
+  end
+
   def user_params
-    flat_params.merge({signup_type: :minimal})
+    @user_params ||= begin
+      allowed_attrs = account_manager.user.defined_fields_names | %i(password signup_type)
+      flat_params.permit(*allowed_attrs).merge(signup_type: :minimal)
+    end
+  end
+
+  def account_params
+    @account_params ||= begin
+      allowed_attrs = account_manager.account.defined_fields_names - %i(billing_address country) + %i(country_id)
+      flat_params.permit(*allowed_attrs)
+    end
   end
 
   def check_creation_errors
@@ -31,7 +45,7 @@ def check_creation_errors
   end
 
   def signup_params
-    Signup::SignupParams.new(plans: plans, user_attributes: user_params, account_attributes: flat_params, defaults: defaults)
+    Signup::SignupParams.new(plans: plans, user_attributes: user_params, account_attributes: account_params, defaults: defaults)
   end
 
   def defaults

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
@@ -134,4 +133,13 @@ def can_create
   def flat_params
     super.except(:id)
   end
+
+  def user_params
+    @user_params ||= begin
+      permission_attrs = [member_permission_service_ids: [], member_permission_ids: [], allowed_sections: [], allowed_service_ids: []]
+      allowed_attrs = user.defined_fields_names | %i(password password_confirmation cas_identifier signup_type)
+      allowed_attrs |= permission_attrs if (provider_key.present? || current_user.admin?)
+      flat_params.permit(*allowed_attrs)
+    end
+  end
 end

app/controllers/api/metric_visibilities_controller.rb

@@ -76,8 +76,4 @@ def find_metric
   def authorize_section
     authorize! :manage, :plans
   end
-
-  def usage_limit_params
-    params.require(:usage_limit)
-  end
 end

app/controllers/api/usage_limits_controller.rb

@@ -79,6 +79,6 @@ def authorize_action
   end
 
   def usage_limit_params
-    params.require(:usage_limit)
+    params.require(:usage_limit).permit(:period, :value)
   end
 end

app/controllers/buyers/accounts_controller.rb

@@ -9,6 +9,7 @@ class Buyers::AccountsController < Buyers::BaseController
 
   before_action :set_plans, :only => %i[new create]
   before_action :find_account, except: %i[index new create]
+  before_action :init_signup_account_manager, only: %i[create]
 
   activate_menu :buyers, :accounts, :listing
 
@@ -41,7 +42,7 @@ def update
   end
 
   def create
-    signup_result = Signup::DeveloperAccountManager.new(current_account).create(signup_params)
+    signup_result = @account_manager.create(signup_params)
     @buyer = signup_result.account
 
     if signup_result.persisted?
@@ -84,7 +85,7 @@ def show
 
   protected
 
-  attr_reader :account
+  attr_reader :account, :user
 
   def find_account
     with_deleted = %w[show resume].include?(action_name)
@@ -109,13 +110,19 @@ def signup_params
     Signup::SignupParams.new(plans: [], user_attributes: user_params.merge(signup_type: :created_by_provider), account_attributes: account_params, validate_fields: false)
   end
 
-  # TODO: using `permit` later
   def account_params
-    @account_params ||= params.require(:account).except(:user)
+    @account_params ||= begin
+      allowed_attrs = account.defined_builtin_fields_names - %i(billing_address country) + %i(country_id)
+      params.require(:account).permit(*allowed_attrs, extra_fields: account.defined_extra_fields_names)
+    end
   end
 
   def user_params
-    params.require(:account).fetch(:user, {})
+    @user_params ||= begin
+      allowed_attrs = user.defined_builtin_fields_names | %i(password signup_type)
+      params.require(:account).fetch(:user, ActionController::Parameters.new)
+            .permit(*allowed_attrs, extra_fields: user.defined_extra_fields_names)
+    end
   end
 
   def set_plans
@@ -131,4 +138,10 @@ def presenter
                                                       user: current_user,
                                                       params: params)
   end
+
+  def init_signup_account_manager
+    @account_manager = Signup::DeveloperAccountManager.new(current_account)
+    @account = @account_manager.account
+    @user = @account_manager.user
+  end
 end

app/controllers/buyers/groups_controller.rb

@@ -10,7 +10,7 @@ class Buyers::GroupsController < Buyers::BaseController
   def show; end
 
   def update
-    flash[:success] = t('.success') if @account.update(params[:account])
+    flash[:success] = t('.success') if @account.update(groups_params)
 
     redirect_to action: :show, id: @account.id
   end
@@ -28,4 +28,8 @@ def authorize_groups
   def find_account
     @account = current_account.buyer_accounts.find(params[:account_id])
   end
+
+  def groups_params
+    @groups_params ||= params.require(:account).permit(group_ids: [])
+  end
 end

app/controllers/buyers/invoices_controller.rb

@@ -40,7 +40,7 @@ def create
   def update
     @invoice = @account.invoices.find(params[:id])
 
-    if @invoice.update(params[:invoice])
+    if @invoice.update(invoice_params)
       redirect_to admin_buyers_account_invoice_url(@account, @invoice), success: t('.success')
     else
       render :edit
@@ -52,4 +52,8 @@ def update
   def find_account
     @account = current_account.buyers.find(params[:account_id])
   end
+
+  def invoice_params
+    params.require(:invoice).permit(:period)
+  end
 end

app/controllers/buyers/users_controller.rb

@@ -15,7 +15,7 @@ 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.attributes = permitted_user_params
     user.role = user_params.fetch(:role, user.role) if can?(:update_role, user)
 
     if user.save
@@ -80,10 +80,15 @@ 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
+    @permitted_user_params ||= begin
+      allowed_attrs = user.defined_builtin_fields_names | %i(password password_confirmation)
+      user_params.permit(*allowed_attrs, extra_fields: user.defined_extra_fields_names)
+    end
   end
 
   def redirect_back_or_show_detail(**opts)

app/controllers/finance/api/invoices_controller.rb

@@ -51,7 +51,7 @@ def charge
   # Invoice Update
   # PUT /api/invoices/{id}.xml
   def update
-    invoice.update(invoice_params_update, without_protection: true)
+    invoice.update(invoice_params_update)
     respond_with(invoice)
   end
 

app/controllers/finance/provider/invoices_controller.rb

@@ -61,7 +61,7 @@ def create
   end
 
   def update
-    if @invoice.update(params[:invoice])
+    if @invoice.update(invoice_params)
       redirect_to admin_finance_invoice_url(@invoice), success: t('.success')
     else
       render :edit
@@ -96,4 +96,8 @@ def find_buyer(options = {})
   def find_invoice
     @invoice = collection.find(params[:id])
   end
+
+  def invoice_params
+    @invoice_params ||= params.require(:invoice).permit(:friendly_id, :period)
+  end
 end