Is this an un-secure way of using strong params for rails?

Question

I'm relatively new to rails (and coding) and my understanding isn't 100% on this, I've tried to read strong params documentation on require, and am unable to solve the issue.

I've created users and admins using Pundit for a site. I've created a dashboard which only admins can access, which provides an overview of all the users, a way to delete them, and a way to change their role. From what I understand this uses the users_controller update method which passes in user_params.

The user update method is also, from what I understand, used for users to edit their own profile (change email, etc.) and I'm concerned that I've added a way for them to pass a role change in through the form somehow.

Is this the right way to go about this? Or should I create a new set of params for the role change, which are solely used on the user dashboard page - if so, how would I do this?

I feel like I'm missing one, or a few things here.

user_controller.rb: class UsersController < ApplicationController

  before_filter :authenticate_user!
  after_action :verify_authorized
  before_action :set_user, only: [:edit, :update, :show, :destroy]


def update
    if @user.update(user_params)
       authorize @user
      flash[:success] = "Profile updated successfully!"
      redirect_to @user
    else
      render 'edit'
    end
end

   def create
    @user = User.new(user_params)
    if @user.save
      session[:user_id] = @user.id
      cookies.signed[:user_id] = @user.id
      flash[:success] = "Welcome #{@user.username} to Day One"
      redirect_to user_path(@user)
    else
      render 'new'
    end
   end

  def show 
    authorize @user
  end

  def destroy
      authorize @user
      @user.destroy
      flash[:danger] = "User and all their related goals have been deleted"
      redirect_to users_path
  end

  def user_dashboard
    @users = User.all
    authorize User
  end

  private

  def user_params
    params.require(:user).permit(:username, :email, :password, :password_confirmation, :profileimage, :role)
  end


  def set_user
    @user = User.find(params[:id])
  end

end

dashboard page:

 <% @users.each do |user| %>
          <tr>
            <td>
              <%= link_to user.email, user %>
            </td>
            <td>
              <%= form_for(user) do |f| %>
                <%= f.select(:role, User.roles.keys.map {|role| [role.titleize,role]}) %>
                <td>
                <button><%= f.submit 'Change Role' %></button>
                </td>
              <% end %>

and finally user_policy.rb:

class UserPolicy < ApplicationPolicy
  attr_reader :current_user, :model

  def initialize(current_user, model)
    @current_user = current_user
    @user = model
  end  

  def index?
    @current_user.admin?
  end

  def show?
    # scope.where(:id => record.id).exists?
  end

  def create?
    false
  end

  def new?
    create?
  end

  def update?
    @current_user.admin? || @current_user == @user
  end

  def edit?
    @current_user.admin? || @current_user == @user
  end

  def destroy?
    @current_user.admin? 
  end

  def user_dashboard?
    @current_user.admin? 
  end


end

Answer 1

You're right - if users pass a role, they'll be able to change it. There is a way to specify different permitted params for different roles described in the pundit README (https://github.com/elabs/pundit#user-content-strong-parameters)

I've also noticed that in the #update action you first update and then authorize user. You should switch the order:

def update
  authorize @user
  if @user.update(user_params)
    ...
  end
end