Move Permited Atributes to Policy (#1200)

* Intial commit

* implement suggestions

* fix lint

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix permitted attributes for order update

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: lodewiges

app/controllers/activities_controller.rb

@@ -45,7 +45,7 @@ def show # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
   end
 
   def create
-    @activity = Activity.new(permitted_attributes.merge(created_by: current_user))
+    @activity = Activity.new(activity_params.merge(created_by: current_user))
     authorize @activity
 
     if @activity.save
@@ -61,7 +61,7 @@ def update
     @activity = Activity.find(params[:id])
     authorize @activity
 
-    if @activity.update(params.require(:activity).permit(%i[title]))
+    if @activity.update(activity_params_for_update)
       flash[:success] = 'Activiteit hernoemd'
     else
       flash[:error] = "Activiteit hernoemen mislukt; #{@activity.errors.full_messages.join(', ')}"
@@ -177,7 +177,11 @@ def sorted_product_price(activity)
     activity.price_list.product_price.sort_by { |p| p.product.id }
   end
 
-  def permitted_attributes
-    params.require(:activity).permit(%i[title start_time end_time price_list_id])
+  def activity_params
+    params.require(:activity).permit(policy(Activity.new).permitted_attributes)
+  end
+
+  def activity_params_for_update
+    params.require(:activity).permit(policy(@activity).permitted_attributes_for_update)
   end
 end

app/controllers/credit_mutations_controller.rb

@@ -14,7 +14,7 @@ def index
   end
 
   def create # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
-    @mutation = CreditMutation.new(permitted_attributes.merge(created_by: current_user))
+    @mutation = CreditMutation.new(credit_mutation_params.merge(created_by: current_user))
     authorize @mutation
 
     respond_to do |format|
@@ -40,7 +40,7 @@ def model_includes
     %i[user activity created_by]
   end
 
-  def permitted_attributes
-    params.require(:credit_mutation).permit(%i[description amount user_id activity_id])
+  def credit_mutation_params
+    params.require(:credit_mutation).permit(policy(CreditMutation).permitted_attributes)
   end
 end

app/controllers/invoices_controller.rb

@@ -31,7 +31,7 @@ def show
   end
 
   def create
-    attributes = remove_empty(permitted_attributes.to_h)
+    attributes = remove_empty(invoice_params.to_h)
     @invoice = Invoice.new(attributes)
     authorize @invoice
 
@@ -93,8 +93,8 @@ def invoice
     @invoice = Invoice.find_by!(token: params[:id])
   end
 
-  def permitted_attributes
-    params.require(:invoice).permit(%i[user_id activity_id name_override email_override rows], rows_attributes: %i[name amount price])
+  def invoice_params
+    params.require(:invoice).permit(policy(Invoice.new).permitted_attributes)
   end
 
   # rubocop:disable Metrics/AbcSize, Metrics/MethodLength

app/controllers/orders_controller.rb

@@ -16,7 +16,7 @@ def index
   end
 
   def create # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
-    @order = Order.new(permitted_attributes.merge(created_by: current_user))
+    @order = Order.new(order_params.merge(created_by: current_user))
     authorize @order
 
     current_credit = @order.user&.credit
@@ -43,7 +43,7 @@ def update
 
     authorize @order
 
-    if @order.update(permitted_attributes_on_update)
+    if @order.update(order_params_for_update)
       render json: @order.to_json(proper_json)
     else
       render json: @order.errors, status: :unprocessable_content
@@ -83,13 +83,12 @@ def send_insufficient_credit_mail?(user, old_credit)
     user.provider.in?(%w[amber_oauth2 sofia_account]) && user.credit.negative? && old_credit.positive?
   end
 
-  def permitted_attributes
-    params.require(:order).permit(%i[user_id paid_with_cash paid_with_pin activity_id],
-                                  order_rows_attributes: %i[id product_id product_count])
+  def order_params
+    params.require(:order).permit(policy(Order.new).permitted_attributes_for_create)
   end
 
-  def permitted_attributes_on_update
-    params.require(:order).permit(:id, order_rows_attributes: %i[id product_count])
+  def order_params_for_update
+    params.require(:order).permit(policy(@order).permitted_attributes_for_update)
   end
 
   def proper_json

app/controllers/price_lists_controller.rb

@@ -21,7 +21,7 @@ def index
   end
 
   def create
-    @price_list = PriceList.new(permitted_attributes)
+    @price_list = PriceList.new(price_list_params)
     authorize @price_list
 
     if @price_list.save
@@ -36,7 +36,7 @@ def update
     @price_list = PriceList.find(params[:id])
     authorize @price_list
 
-    if @price_list.update(permitted_attributes)
+    if @price_list.update(price_list_params)
       flash[:success] = 'Prijslijst opgeslagen'
     else
       flash[:error] = "Prijslijst wijzigen mislukt; #{@price_list.errors.full_messages.join(', ')}"
@@ -76,7 +76,7 @@ def unarchive
 
   private
 
-  def permitted_attributes
-    params.require(:price_list).permit(:name)
+  def price_list_params
+    params.require(:price_list).permit(policy(PriceList.new).permitted_attributes)
   end
 end

app/controllers/products_controller.rb

@@ -5,7 +5,7 @@ class ProductsController < ApplicationController
   after_action :verify_authorized
 
   def create
-    @product = Product.new(permitted_attributes)
+    @product = Product.new(product_params)
     authorize @product
 
     if @product.save
@@ -18,7 +18,7 @@ def create
   def update
     authorize @product
 
-    if @product.update(permitted_attributes)
+    if @product.update(product_params)
       render json: @product, include: json_includes, except: json_exludes, methods: :t_category
     else
       render json: @product.errors, status: :unprocessable_content
@@ -31,10 +31,8 @@ def set_model
     @product = Product.find(params[:id])
   end
 
-  def permitted_attributes
-    params.require(:product)
-          .permit(%i[name category color requires_age id],
-                  product_prices_attributes: %i[id product_id price_list_id price _destroy])
+  def product_params
+    params.require(:product).permit(policy(Product.new).permitted_attributes)
   end
 
   def json_includes

app/controllers/sofia_accounts_controller.rb

@@ -15,7 +15,7 @@ def create # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
     user = User.find_by(id: user_id)
     validate_user(user)
 
-    sofia_account = SofiaAccount.new(permitted_attributes.merge(user_id:))
+    sofia_account = SofiaAccount.new(sofia_account_params.merge(user_id:))
     raise normalize_error_messages(sofia_account.errors.full_messages) unless sofia_account.save
 
     update_user_after_creation(user, sofia_account)
@@ -256,7 +256,7 @@ def update_user_after_creation(user, sofia_account) # rubocop:disable Metrics/Ab
     raise normalize_error_messages(user.errors.full_messages)
   end
 
-  def permitted_attributes
-    params.require(:sofia_account).permit(%i[username password password_confirmation])
+  def sofia_account_params
+    params.require(:sofia_account).permit(policy(SofiaAccount.new).permitted_attributes)
   end
 end

app/controllers/users_controller.rb

@@ -72,7 +72,7 @@ def json
   end
 
   def create
-    @user = User.new(permitted_attributes)
+    @user = User.new(user_params)
     authorize @user
 
     if @user.save
@@ -88,7 +88,7 @@ def update
     @user = User.find(params[:id])
     authorize @user
 
-    if @user.update(params.require(:user).permit(%i[name email deactivated]))
+    if update_user
       flash[:success] = 'Gebruiker geupdate'
     else
       flash[:error] = "Gebruiker updaten mislukt; #{@user.errors.full_messages.join(', ')}"
@@ -140,8 +140,7 @@ def update_with_sofia_account # rubocop:disable Metrics/AbcSize, Metrics/MethodL
     end
     authorize @sofia_account
 
-    if @user.update(params.require(:user).permit(%i[email sub_provider] + (current_user.treasurer? ? %i[name deactivated] : []),
-                                                 sofia_account_attributes: %i[id username]))
+    if @user.update(params.require(:user).permit(policy(@user).permitted_attributes_for_update_with_sofia_account))
       flash[:success] = 'Gegevens gewijzigd'
     else
       flash[:error] = "Gegevens wijzigen mislukt; #{@user.errors.full_messages.join(', ')}"
@@ -152,6 +151,11 @@ def update_with_sofia_account # rubocop:disable Metrics/AbcSize, Metrics/MethodL
 
   private
 
+  def update_user
+    permitted_params = params.require(:user).permit(policy(@user).permitted_attributes_for_update)
+    @user.update(permitted_params)
+  end
+
   def find_or_create_user(user_json) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
     fields = user_json['attributes']
     u = User.find_or_initialize_by(uid: user_json['id'])
@@ -166,7 +170,7 @@ def find_or_create_user(user_json) # rubocop:disable Metrics/AbcSize, Metrics/Me
     u.save
   end
 
-  def permitted_attributes
-    params.require(:user).permit(%w[name email provider sub_provider])
+  def user_params
+    params.require(:user).permit(policy(User.new).permitted_attributes)
   end
 end

app/policies/activity_policy.rb

@@ -48,4 +48,12 @@ def orders?
   def credit_mutations?
     user&.treasurer?
   end
+
+  def permitted_attributes
+    %i[title start_time end_time price_list_id]
+  end
+
+  def permitted_attributes_for_update
+    %i[title]
+  end
 end

app/policies/application_policy.rb

@@ -38,6 +38,10 @@ def scope
     Pundit.policy_scope!(user, record.class)
   end
 
+  def permitted_attributes
+    []
+  end
+
   class Scope
     attr_reader :user, :scope