CODEWITHSUNDEEP
ruby-on-railsrubyruby-on-rails-3activerecord
Tintin81
Tintin81

Reputation: 10215

How to validate foreign keys in Rails validations?

In my Rails app I have users who can have many projects which in turn can have many invoices.

How can I make sure that a user can only create an invoice for one of his projects and not for another user's projects?

class Invoice < ActiveRecord::Base

  attr_accessible :number, :date, :project_id

  validates :project_id,  :presence   => true,
                          :inclusion  => { :in => ????????? }

end

Thanks for any help.

class InvoicesController < ApplicationController

  def new  
    @invoice = current_user.invoices.build(:project_id => params[:project_id])
  end

  def create
    @invoice = current_user.invoices.build(params[:invoice])    
    if @invoice.save
      flash[:success] = "Invoice saved."
      redirect_to edit_invoice_path(@invoice)
    else
      render :new
    end
  end

end

Upvotes: 0

Views: 412

Answers (3)

gotva
gotva

Reputation: 5998

project_id is "sensitive" attribute - so remove it from attr_accessible. You are right that you should not believe params from the form and you must check it.

def create
  @invoice = current_user.invoices.build(params[:invoice])
  # @invoice.project_id is nil now because this attr not in attr_accessible list
  @invoice.project_id = params[:invoice][:project_id] if current_user.project_ids.include?(params[:invoice][:project_id])
  if @invoice.save
    flash[:success] = "Invoice saved."
    redirect_to edit_invoice_path(@invoice)
  else
    render :new
  end
end

If user tries to hack your app and change project_id to not owned value then method create render partial new with invalid @invoice. Do not forget to leave the validation of project_id on presence.

If you get exception Can't mass-assign protected attributes... there are several ways what to do. The simplest ways are: 1. remove line from environment configs (development, test, production)

# Raise exception on mass assignment protection for Active Record models
config.active_record.mass_assignment_sanitizer = :strict

2. Reject sensitive parameters from params before assigning. 

# changes in method create
def create
  project_id = params[:invoice].delete(:project_id)
  @invoice = current_user.invoices.build(params[:invoice])
  @invoice.project_id = project_id if current_user.project_ids.include?(project_id)
  ...
end

Upvotes: 1

Tintin81
Tintin81

Reputation: 10215

OK, luckily I managed to come up with a solution of my own this time.

I didn't make any changes to my controller ("let's keep 'em skinny"), but added a validation method to my model instead:

class Invoice < ActiveRecord::Base

  attr_accessible :number, :date, :project_id

  validates :project_id,  :presence     => true,
                          :numericality => { :only_integer => true },
                          :inclusion    => { :in => proc { |record| record.available_project_ids } }

  def available_project_ids
    user.project_ids
  end

end

I am not sure if this is good or bad coding practice. Maybe someone can shed some light on this. But for the moment it seems pretty safe to me and I haven't been able to hack it in any way so far.

Upvotes: 0

Ismael
Ismael

Reputation: 16730

I think that shouldn't be on a validation. You should ensure the project the user selected is one his projects.

You could do something on your controller like:

project = current_user.projects.find params[:project_id]
@invoice = Invoice.new(project: project)
# ...

Your create action could look something like this.

  def create
    @invoice = current_user.invoices.build(params[:invoice])
    @invoice.project = current_user.projects.find params[:invoice][:project_id]
    if @invoice.save
      flash[:success] = "Invoice saved."
      redirect_to edit_invoice_path(@invoice)
    else
      render :new
    end
  end

Upvotes: 1

Related Questions