Rails 5 - Pundit - authorisation for nested resource

Question

I am trying to figure out how to use Pundit with my Rails 5 app.

I have models for Proposal, Potential and User. The associations are:

Proposal

has_many :potentials, inverse_of: :proposal
    accepts_nested_attributes_for :potentials, reject_if: :all_blank, allow_destroy: true

belongs_to :user

Potential

belongs_to :proposal, inverse_of: :potentials
  belongs_to :user

User

has_many :proposals, dependent: :destroy
  has_many :potentials

I have pundit policies for each of these resources.

I am currently struggling to figure out how to implement rules so that potentials are only shown if rules are applied.

My potentials are rendered in a partial saved in my proposals view folder. It has:

<% @proposal.potentials.each do | pot | %>
        <div class="panel">
            <% if policy(pot).show? %>
                <% if pot.private_comment == true %>
                    <p> <%= render :text => 'CONFIDENTIAL - NOT FOR PUBLIC DISCLOSURE' %></p>
                    <% end %>
                        <p><%=  pot.comment %>
                        </p>
                        <p style = "color: navy; text-align:right"><%= pot.user.full_name %>, <%= pot.user.organisation.title.titleize %></p>

                        <p style="font-style:italic; color: #FFFFFF; float:right"><%= text_for_status(pot)%></p>
                    </div>
                <% end %>
        <% end %>

In my proposal controller, show action, I have:

    before_action :set_proposal, only: [:show, :edit, :update, :destroy ]

def show
        @potentials = @proposal.potentials
end

private
    # Use callbacks to share common setup or constraints between actions.
    def set_proposal
      @proposal = Proposal.find(params[:id])
      authorize @proposal
    end

In my potential policy, I define the rules for show as any of the 3 criteria listed being true:

class PotentialPolicy < ApplicationPolicy


  def index?
    true
  end

    def show?
true  if record.private_comment != true ||
      if record.private_comment == true && @current_user == record.user_id ||
      if record.private_comment == true && @current_user  == record.proposal.user_id
      else false
      end
      end

end

  def new?
    true
  end

  def create?
    true
  end

  def edit?
    update?
  end

  def update?
    true if record.user_id == current_user.id
  end

  def destroy?
    false
  end

end

My expectation is that since I ask for the potential policy to be checked in the views/proposals/temporary_proposals.html.erb partial (above) and extracted below,

<% if policy(pot).show? %> 

Note: pot is defined as @proposal.potential.

The only logical error I can see in this is that current user is the user rather than the user id. But, if I append ".id" to the end of current_user, I get an error that says "id is nil".

Pundit will look at the potential policy for that proposal and figure out whether to display the record or not.

This isnt right because when I save all this and try to render the proposal show, I can only see potentials where the :private_comment attribute is not true (but I do meet the criteria for the second and third qualifying permission (i.e. I created the potential and the proposal) - so I should be able to view that record).

My application policy has:

class ApplicationPolicy
  attr_reader :user, :record

  def initialize(user, record)
    @user = user
    @record = record
  end

I understand this to mean that since my potential policy inherits from my application policy, I should be able to refer to @record to mean the record this policy is trying to handle. To me, that means I should be able to define the show action as:

def show?
    true  if record.private_comment != true ||
          if record.private_comment == true && @current_user == @record.user ||
          if record.private_comment == true && @current_user  == @record.proposal.user
          else false
          end
    end
  end

But this gives the same incorrect result as the attempt above.

Can anyone see where I've got this wrong?

Answer 1

Unless I'm missing something, @current_user isn't defined in your policies. How about simplifying your show? method:

def show?
  return true unless record.private_comment?

  return [ record.user_id, record.proposal.user_id ].include? user.id
end

---

I think building huge conditionals like you've done is a common trap when thinking through the guards for an action.

I find policy methods much easier to read and write if you consider the shortest possible paths to a failure or success of the guard independently up front. Follow these by weird edge cases, and finally a default (usually false) if necessary.

This will lead to less duplication (like your repeated evaluation of record.private_comment == true) and cleaner code.

It's also worth pointing out query methods exist for boolean attributes on rails models. That is how you are able to do record.private_comment? instead of record.private_comment == true.